This is an automated email from the ASF dual-hosted git repository.

hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks-controller.git


The following commit(s) were added to refs/heads/unstable by this push:
     new 2c834b4  Fix the migrating slot didn't compatible with old format 
(#310)
2c834b4 is described below

commit 2c834b4af8dab0e44b1745d4dc28618cf9328339
Author: Byron Seto <[email protected]>
AuthorDate: Mon May 12 22:10:57 2025 -0600

    Fix the migrating slot didn't compatible with old format (#310)
---
 cmd/client/command/helper.go |   2 +-
 controller/cluster.go        |  13 +---
 controller/cluster_test.go   |   4 +-
 server/api/cluster.go        |   2 +-
 server/api/cluster_test.go   |   4 +-
 store/cluster.go             |   4 +-
 store/cluster_node.go        |   9 +--
 store/cluster_shard.go       |  20 ++++--
 store/cluster_shard_test.go  |  17 +++--
 store/cluster_test.go        |   6 +-
 store/slot.go                |  95 +++++++++++++++++++++++---
 store/slot_test.go           | 155 +++++++++++++++++++++++++++++++++++++------
 12 files changed, 260 insertions(+), 71 deletions(-)

diff --git a/cmd/client/command/helper.go b/cmd/client/command/helper.go
index 1d395ac..ade4f35 100644
--- a/cmd/client/command/helper.go
+++ b/cmd/client/command/helper.go
@@ -51,7 +51,7 @@ func printCluster(cluster *store.Cluster) {
                                role = strings.ToUpper(store.RoleMaster)
                        }
                        migratingStatus := "NO"
-                       if shard.MigratingSlot != nil {
+                       if shard.IsMigrating() {
                                migratingStatus = fmt.Sprintf("%s --> %d", 
shard.MigratingSlot, shard.TargetShardIndex)
                        }
                        columns := []string{fmt.Sprintf("%d", i), node.ID(), 
node.Addr(), role, migratingStatus}
diff --git a/controller/cluster.go b/controller/cluster.go
index 2d6802e..2cb08b5 100755
--- a/controller/cluster.go
+++ b/controller/cluster.go
@@ -318,19 +318,12 @@ func (c *ClusterChecker) tryUpdateMigrationStatus(ctx 
context.Context, clonedClu
                if !shard.IsMigrating() {
                        continue
                }
-
                sourceNodeClusterInfo, err := 
shard.GetMasterNode().GetClusterInfo(ctx)
                if err != nil {
                        log.Error("Failed to get the cluster info from the 
source node", zap.Error(err))
                        return
                }
-               if sourceNodeClusterInfo.MigratingSlot == nil {
-                       log.Error("The source migration slot is empty",
-                               zap.String("migrating_slot", 
shard.MigratingSlot.String()),
-                       )
-                       return
-               }
-               if 
!sourceNodeClusterInfo.MigratingSlot.Equal(shard.MigratingSlot) {
+               if 
!sourceNodeClusterInfo.MigratingSlot.Equal(shard.MigratingSlot.SlotRange) {
                        log.Error("Mismatch migrating slot",
                                zap.String("source_migrating_slot", 
sourceNodeClusterInfo.MigratingSlot.String()),
                                zap.String("migrating_slot", 
shard.MigratingSlot.String()),
@@ -355,9 +348,9 @@ func (c *ClusterChecker) tryUpdateMigrationStatus(ctx 
context.Context, clonedClu
                        c.updateCluster(clonedCluster)
                        log.Warn("Failed to migrate the slot", 
zap.String("slot", migratingSlot.String()))
                case "success":
-                       clonedCluster.Shards[i].SlotRanges = 
store.RemoveSlotFromSlotRanges(clonedCluster.Shards[i].SlotRanges, 
*shard.MigratingSlot)
+                       clonedCluster.Shards[i].SlotRanges = 
store.RemoveSlotFromSlotRanges(clonedCluster.Shards[i].SlotRanges, 
shard.MigratingSlot.SlotRange)
                        clonedCluster.Shards[shard.TargetShardIndex].SlotRanges 
= store.AddSlotToSlotRanges(
-                               
clonedCluster.Shards[shard.TargetShardIndex].SlotRanges, *shard.MigratingSlot,
+                               
clonedCluster.Shards[shard.TargetShardIndex].SlotRanges, 
shard.MigratingSlot.SlotRange,
                        )
                        migratedSlot := shard.MigratingSlot
                        clonedCluster.Shards[i].ClearMigrateState()
diff --git a/controller/cluster_test.go b/controller/cluster_test.go
index d415f3f..a1a720a 100644
--- a/controller/cluster_test.go
+++ b/controller/cluster_test.go
@@ -109,7 +109,7 @@ func TestCluster_FailureCount(t *testing.T) {
                                mockNode0, mockNode1, mockNode2, mockNode3,
                        },
                        SlotRanges:       []store.SlotRange{{Start: 0, Stop: 
16383}},
-                       MigratingSlot:    nil,
+                       MigratingSlot:    &store.MigratingSlot{IsMigrating: 
false},
                        TargetShardIndex: -1,
                }},
        }
@@ -221,7 +221,7 @@ func TestCluster_MigrateSlot(t *testing.T) {
        }()
        slotRange, err := store.NewSlotRange(0, 0)
        require.NoError(t, err)
-       require.NoError(t, cluster.MigrateSlot(ctx, *slotRange, 1, false))
+       require.NoError(t, cluster.MigrateSlot(ctx, slotRange, 1, false))
 
        s := NewMockClusterStore()
        require.NoError(t, s.CreateCluster(ctx, ns, cluster))
diff --git a/server/api/cluster.go b/server/api/cluster.go
index b539c90..b223644 100644
--- a/server/api/cluster.go
+++ b/server/api/cluster.go
@@ -33,7 +33,7 @@ import (
 
 type MigrateSlotRequest struct {
        Target   int             `json:"target" validate:"required"`
-       Slot     store.SlotRange `json:"slot" validate:"required"`
+       Slot     store.SlotRange `json:"slot" validate:"required"` // we don't 
use store.MigratingSlot here because we expect a valid SlotRange
        SlotOnly bool            `json:"slot_only"`
 }
 
diff --git a/server/api/cluster_test.go b/server/api/cluster_test.go
index 6559a87..5d3aa8d 100644
--- a/server/api/cluster_test.go
+++ b/server/api/cluster_test.go
@@ -131,7 +131,7 @@ func TestClusterBasics(t *testing.T) {
                slotRange, err := store.NewSlotRange(3, 3)
                require.NoError(t, err)
                testMigrateReq := &MigrateSlotRequest{
-                       Slot:     *slotRange,
+                       Slot:     slotRange,
                        SlotOnly: true,
                        Target:   1,
                }
@@ -271,7 +271,7 @@ func TestClusterMigrateData(t *testing.T) {
                        currentVersion := gotCluster.Version.Load()
                        sourceSlotRanges := gotCluster.Shards[0].SlotRanges
                        targetSlotRanges := gotCluster.Shards[1].SlotRanges
-                       require.EqualValues(t, slotRange, 
*gotCluster.Shards[0].MigratingSlot)
+                       require.EqualValues(t, slotRange, 
gotCluster.Shards[0].MigratingSlot.SlotRange)
                        require.EqualValues(t, 1, 
gotCluster.Shards[0].TargetShardIndex)
 
                        // Run the controller to check and update the migration 
status
diff --git a/store/cluster.go b/store/cluster.go
index b4bfd9e..8dff6ed 100644
--- a/store/cluster.go
+++ b/store/cluster.go
@@ -181,7 +181,7 @@ func (cluster *Cluster) findShardIndexBySlot(slot 
SlotRange) (int, error) {
        for i := 0; i < len(cluster.Shards); i++ {
                slotRanges := cluster.Shards[i].SlotRanges
                for _, slotRange := range slotRanges {
-                       if slotRange.HasOverlap(&slot) {
+                       if slotRange.HasOverlap(slot) {
                                if sourceShardIdx != -1 {
                                        return sourceShardIdx, 
consts.ErrSlotRangeBelongsToMultipleShards
                                }
@@ -226,7 +226,7 @@ func (cluster *Cluster) MigrateSlot(ctx context.Context, 
slot SlotRange, targetS
        }
 
        // Will start the data migration in the background
-       cluster.Shards[sourceShardIdx].MigratingSlot = &slot
+       cluster.Shards[sourceShardIdx].MigratingSlot = FromSlotRange(slot)
        cluster.Shards[sourceShardIdx].TargetShardIndex = targetShardIdx
        return nil
 }
diff --git a/store/cluster_node.go b/store/cluster_node.go
index 4da4253..8d038d0 100644
--- a/store/cluster_node.go
+++ b/store/cluster_node.go
@@ -85,9 +85,9 @@ type ClusterNode struct {
 }
 
 type ClusterInfo struct {
-       CurrentEpoch   int64      `json:"cluster_current_epoch"`
-       MigratingSlot  *SlotRange `json:"migrating_slot"`
-       MigratingState string     `json:"migrating_state"`
+       CurrentEpoch   int64          `json:"cluster_current_epoch"`
+       MigratingSlot  *MigratingSlot `json:"migrating_slot"`
+       MigratingState string         `json:"migrating_state"`
 }
 
 type ClusterNodeInfo struct {
@@ -195,10 +195,11 @@ func (n *ClusterNode) GetClusterInfo(ctx context.Context) 
(*ClusterInfo, error)
                        }
                case "migrating_slot", "migrating_slot(s)":
                        // TODO(@git-hulk): handle multiple migrating slots
-                       clusterInfo.MigratingSlot, err = 
ParseSlotRange(fields[1])
+                       slotRange, err := ParseSlotRange(fields[1])
                        if err != nil {
                                return nil, err
                        }
+                       clusterInfo.MigratingSlot = FromSlotRange(*slotRange)
                case "migrating_state":
                        clusterInfo.MigratingState = fields[1]
                }
diff --git a/store/cluster_shard.go b/store/cluster_shard.go
index 62c8826..69ffd4a 100644
--- a/store/cluster_shard.go
+++ b/store/cluster_shard.go
@@ -33,11 +33,17 @@ import (
        "github.com/apache/kvrocks-controller/consts"
 )
 
+const (
+       // the old migrating slot was denoted by an int and -1 was
+       // used to denote a non migrating slot
+       NotMigratingInt = -1
+)
+
 type Shard struct {
-       Nodes            []Node      `json:"nodes"`
-       SlotRanges       []SlotRange `json:"slot_ranges"`
-       TargetShardIndex int         `json:"target_shard_index"`
-       MigratingSlot    *SlotRange  `json:"migrating_slot"`
+       Nodes            []Node         `json:"nodes"`
+       SlotRanges       []SlotRange    `json:"slot_ranges"`
+       TargetShardIndex int            `json:"target_shard_index"`
+       MigratingSlot    *MigratingSlot `json:"migrating_slot"`
 }
 
 type Shards []*Shard
@@ -112,7 +118,7 @@ func (shard *Shard) addNode(addr, role, password string) 
error {
 }
 
 func (shard *Shard) IsMigrating() bool {
-       return shard.MigratingSlot != nil && shard.TargetShardIndex != -1
+       return shard.MigratingSlot != nil && shard.MigratingSlot.IsMigrating && 
shard.TargetShardIndex != -1
 }
 
 func (shard *Shard) GetMasterNode() Node {
@@ -206,7 +212,7 @@ func (shard *Shard) promoteNewMaster(ctx context.Context, 
masterNodeID, preferre
        return preferredNewMasterNode.ID(), nil
 }
 
-func (shard *Shard) HasOverlap(slotRange *SlotRange) bool {
+func (shard *Shard) HasOverlap(slotRange SlotRange) bool {
        for _, shardSlotRange := range shard.SlotRanges {
                if shardSlotRange.HasOverlap(slotRange) {
                        return true
@@ -261,7 +267,7 @@ func (shard *Shard) UnmarshalJSON(bytes []byte) error {
        var data struct {
                SlotRanges       []SlotRange    `json:"slot_ranges"`
                TargetShardIndex int            `json:"target_shard_index"`
-               MigratingSlot    *SlotRange     `json:"migrating_slot"`
+               MigratingSlot    *MigratingSlot `json:"migrating_slot"`
                Nodes            []*ClusterNode `json:"nodes"`
        }
        if err := json.Unmarshal(bytes, &data); err != nil {
diff --git a/store/cluster_shard_test.go b/store/cluster_shard_test.go
index 994046f..1406f35 100644
--- a/store/cluster_shard_test.go
+++ b/store/cluster_shard_test.go
@@ -29,11 +29,11 @@ import (
 
 func TestShard_HasOverlap(t *testing.T) {
        shard := NewShard()
-       slotRange := &SlotRange{Start: 0, Stop: 100}
-       shard.SlotRanges = append(shard.SlotRanges, *slotRange)
+       slotRange := SlotRange{Start: 0, Stop: 100}
+       shard.SlotRanges = append(shard.SlotRanges, slotRange)
        require.True(t, shard.HasOverlap(slotRange))
-       require.True(t, shard.HasOverlap(&SlotRange{Start: 50, Stop: 150}))
-       require.False(t, shard.HasOverlap(&SlotRange{Start: 101, Stop: 150}))
+       require.True(t, shard.HasOverlap(SlotRange{Start: 50, Stop: 150}))
+       require.False(t, shard.HasOverlap(SlotRange{Start: 101, Stop: 150}))
 }
 
 func TestShard_Sort(t *testing.T) {
@@ -56,17 +56,22 @@ func TestShard_Sort(t *testing.T) {
 func TestShard_IsServicing(t *testing.T) {
        var err error
        shard := NewShard()
+       shard.TargetShardIndex = 0
+       shard.MigratingSlot = &MigratingSlot{IsMigrating: false}
+       require.False(t, shard.IsServicing())
+
        shard.TargetShardIndex = 0
        shard.MigratingSlot = nil
        require.False(t, shard.IsServicing())
 
        shard.TargetShardIndex = 0
-       shard.MigratingSlot, err = NewSlotRange(1, 1)
+       slotRange, err := NewSlotRange(1, 1)
        require.Nil(t, err)
+       shard.MigratingSlot = FromSlotRange(slotRange)
        require.True(t, shard.IsServicing())
 
        shard.TargetShardIndex = -1
-       shard.MigratingSlot = nil
+       shard.MigratingSlot = &MigratingSlot{IsMigrating: false}
        shard.SlotRanges = []SlotRange{{Start: 0, Stop: 100}}
        require.True(t, shard.IsServicing())
 
diff --git a/store/cluster_test.go b/store/cluster_test.go
index 6b6e45c..975f03f 100644
--- a/store/cluster_test.go
+++ b/store/cluster_test.go
@@ -44,19 +44,19 @@ func TestCluster_FindIndexShardBySlot(t *testing.T) {
 
        slotRange, err := NewSlotRange(0, 0)
        require.NoError(t, err)
-       shard, err := cluster.findShardIndexBySlot(*slotRange)
+       shard, err := cluster.findShardIndexBySlot(slotRange)
        require.NoError(t, err)
        require.Equal(t, 0, shard)
 
        slotRange, err = NewSlotRange(MaxSlotID/3+1, MaxSlotID/3+1)
        require.NoError(t, err)
-       shard, err = cluster.findShardIndexBySlot(*slotRange)
+       shard, err = cluster.findShardIndexBySlot(slotRange)
        require.NoError(t, err)
        require.Equal(t, 1, shard)
 
        slotRange, err = NewSlotRange(MaxSlotID, MaxSlotID)
        require.NoError(t, err)
-       shard, err = cluster.findShardIndexBySlot(*slotRange)
+       shard, err = cluster.findShardIndexBySlot(slotRange)
        require.NoError(t, err)
        require.Equal(t, 2, shard)
 }
diff --git a/store/slot.go b/store/slot.go
index 04a2b52..448002e 100644
--- a/store/slot.go
+++ b/store/slot.go
@@ -44,24 +44,26 @@ type SlotRange struct {
 
 type SlotRanges []SlotRange
 
-func NewSlotRange(start, stop int) (*SlotRange, error) {
+type MigratingSlot struct {
+       SlotRange
+       IsMigrating bool
+}
+
+func NewSlotRange(start, stop int) (SlotRange, error) {
        if start > stop {
-               return nil, errors.New("start was larger than stop")
+               return SlotRange{}, errors.New("start was larger than stop")
        }
        if (start < MinSlotID || start > MaxSlotID) ||
                (stop < MinSlotID || stop > MaxSlotID) {
-               return nil, ErrSlotOutOfRange
+               return SlotRange{}, ErrSlotOutOfRange
        }
-       return &SlotRange{
+       return SlotRange{
                Start: start,
                Stop:  stop,
        }, nil
 }
 
-func (slotRange *SlotRange) Equal(that *SlotRange) bool {
-       if that == nil {
-               return false
-       }
+func (slotRange *SlotRange) Equal(that SlotRange) bool {
        if slotRange.Start != that.Start {
                return false
        }
@@ -71,7 +73,7 @@ func (slotRange *SlotRange) Equal(that *SlotRange) bool {
        return true
 }
 
-func (slotRange *SlotRange) HasOverlap(that *SlotRange) bool {
+func (slotRange *SlotRange) HasOverlap(that SlotRange) bool {
        return slotRange.Stop >= that.Start && slotRange.Start <= that.Stop
 }
 
@@ -156,13 +158,84 @@ func (SlotRanges *SlotRanges) Contains(slot int) bool {
 
 func (SlotRanges *SlotRanges) HasOverlap(slotRange SlotRange) bool {
        for _, slotRange := range *SlotRanges {
-               if slotRange.HasOverlap(&slotRange) {
+               if slotRange.HasOverlap(slotRange) {
                        return true
                }
        }
        return false
 }
 
+func (s *SlotRange) Reset() {
+       s.Start = 0
+       s.Stop = 0
+}
+
+// FromSlotRange will return a MigratingSlot with the IsMigrating field set to 
true.
+// IsMigrating field would probably only be set to false from an unmarshal, 
like when
+// reading from the topology string
+func FromSlotRange(slotRange SlotRange) *MigratingSlot {
+       return &MigratingSlot{
+               SlotRange:   slotRange,
+               IsMigrating: true,
+       }
+}
+
+func (s *MigratingSlot) UnmarshalJSON(data []byte) error {
+       var slotsString any
+       if err := json.Unmarshal(data, &slotsString); err != nil {
+               return err
+       }
+       switch t := slotsString.(type) {
+       case string:
+               slotRange := SlotRange{}
+               err := json.Unmarshal(data, &slotRange)
+               if err != nil {
+                       s.Reset()
+                       return err
+               }
+               s.SlotRange = slotRange
+               s.IsMigrating = true
+       case float64:
+               // We use integer to represent the slot because we don't 
support the slot range
+               // in the past. So we need to support the integer type for 
backward compatibility.
+               // But the number in JSON is float64, so we need to convert it 
to int here.
+               if t == NotMigratingInt {
+                       s.Reset()
+                       return nil
+               }
+               if t < MinSlotID || t > MaxSlotID {
+                       s.Reset()
+                       return ErrSlotOutOfRange
+               }
+               slotID := int(t)
+               s.Start = slotID
+               s.Stop = slotID
+               s.IsMigrating = true
+       default:
+               s.Reset()
+               return fmt.Errorf("invalid slot range type: %T", slotsString)
+       }
+       return nil
+}
+
+func (s *MigratingSlot) MarshalJSON() ([]byte, error) {
+       if !s.IsMigrating {
+               // backwards compatibility. When we read from an old cluster 
that had `-1`
+               // denoting !isMigrating. The MigratingSlot field will not be 
nil. So when
+               // this field is marshal'd back into JSON format, we can keep 
it as it was
+               // which was `-1`.
+               // The only case this turns back to null is if a migration 
happens on this
+               // shard, and the function `ClearMigrateState()` is called on 
the shard.
+               return json.Marshal(NotMigratingInt)
+       }
+       return json.Marshal(s.String())
+}
+
+func (s *MigratingSlot) Reset() {
+       s.SlotRange.Reset()
+       s.IsMigrating = false
+}
+
 // CanMerge will return true if the given SlotRanges are adjacent with each 
other
 func CanMerge(a, b SlotRange) bool {
        // Ensure a starts before b for easier comparison
@@ -218,7 +291,7 @@ func RemoveSlotFromSlotRanges(source SlotRanges, slot 
SlotRange) SlotRanges {
        result := make([]SlotRange, 0, len(source))
        for _, slotRange := range source {
                // if no overlap, keep original range
-               if !slotRange.HasOverlap(&slot) {
+               if !slotRange.HasOverlap(slot) {
                        result = append(result, slotRange)
                        continue
                }
diff --git a/store/slot_test.go b/store/slot_test.go
index 9158198..119ab09 100644
--- a/store/slot_test.go
+++ b/store/slot_test.go
@@ -20,6 +20,7 @@
 package store
 
 import (
+       "encoding/json"
        "testing"
 
        "github.com/apache/kvrocks-controller/consts"
@@ -42,6 +43,116 @@ func TestSlotRange_String(t *testing.T) {
        assert.Equal(t, ErrSlotOutOfRange, err)
 }
 
+func TestMigratingSlot_UnmarshalJSON(t *testing.T) {
+       var migratingSlot MigratingSlot
+
+       migratingSlot = MigratingSlot{SlotRange: SlotRange{Start: 5, Stop: 5}, 
IsMigrating: true} // to set values to migratingSlot
+       slotBytes, err := json.Marshal(NotMigratingInt)
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &migratingSlot)
+       require.NoError(t, err, "expects no error since -1 was a valid 'not 
migrating' value")
+       assert.Equal(t, MigratingSlot{SlotRange{Start: 0, Stop: 0}, false}, 
migratingSlot)
+
+       migratingSlot = MigratingSlot{SlotRange: SlotRange{Start: 5, Stop: 5}, 
IsMigrating: true} // to set values to migratingSlot
+       slotBytes, err = json.Marshal(-5)
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &migratingSlot)
+       require.ErrorIs(t, err, ErrSlotOutOfRange, "-5 is not a valid 'not 
migrating' value")
+       assert.Equal(t, MigratingSlot{SlotRange{Start: 0, Stop: 0}, false}, 
migratingSlot)
+
+       slotBytes, err = json.Marshal("456")
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &migratingSlot)
+       require.NoError(t, err)
+       assert.Equal(t, MigratingSlot{SlotRange{Start: 456, Stop: 456}, true}, 
migratingSlot)
+
+       slotBytes, err = json.Marshal("123-456")
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &migratingSlot)
+       require.NoError(t, err)
+       assert.Equal(t, MigratingSlot{SlotRange{Start: 123, Stop: 456}, true}, 
migratingSlot)
+
+       slotBytes, err = json.Marshal("invalid-string")
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &migratingSlot)
+       require.Error(t, err)
+       assert.Equal(t, MigratingSlot{SlotRange{Start: 0, Stop: 0}, false}, 
migratingSlot)
+}
+
+// TestMigratingSlot_MarshalUnmarshalJSON will check that we can marshal and 
then unmarshal
+// back into the MigratingSlot
+func TestMigratingSlot_MarshalUnmarshalJSON(t *testing.T) {
+       migratingSlot := MigratingSlot{SlotRange: SlotRange{Start: 5, Stop: 5}, 
IsMigrating: true}
+       migratingSlotBytes, err := json.Marshal(&migratingSlot)
+       require.NoError(t, err)
+       err = json.Unmarshal(migratingSlotBytes, &migratingSlot)
+       require.NoError(t, err)
+       assert.Equal(t, MigratingSlot{SlotRange{Start: 5, Stop: 5}, true}, 
migratingSlot)
+
+       // tests that we can marshal isMigrating = false, which results in -1, 
and then unmarshal it
+       // to be isMigrating = false again
+       migratingSlot = MigratingSlot{SlotRange: SlotRange{Start: 0, Stop: 0}, 
IsMigrating: false}
+       migratingSlotBytes, err = json.Marshal(&migratingSlot)
+       require.NoError(t, err)
+       err = json.Unmarshal(migratingSlotBytes, &migratingSlot)
+       require.NoError(t, err)
+       assert.Equal(t, MigratingSlot{SlotRange{Start: 0, Stop: 0}, false}, 
migratingSlot)
+
+       // same test as earlier, but checks that it resets the start and stop
+       migratingSlot = MigratingSlot{SlotRange: SlotRange{Start: 5, Stop: 5}, 
IsMigrating: false}
+       migratingSlotBytes, err = json.Marshal(&migratingSlot)
+       require.NoError(t, err)
+       err = json.Unmarshal(migratingSlotBytes, &migratingSlot)
+       require.NoError(t, err)
+       assert.Equal(t, MigratingSlot{SlotRange{Start: 0, Stop: 0}, false}, 
migratingSlot, "expects start and stop to reset to 0")
+}
+
+// TestMigratingSlot_MarshalJSON will checks the resulting string
+func TestMigratingSlot_MarshalJSON(t *testing.T) {
+       migratingSlot := MigratingSlot{SlotRange: SlotRange{Start: 5, Stop: 5}, 
IsMigrating: true}
+       migratingSlotBytes, err := json.Marshal(&migratingSlot)
+       require.NoError(t, err)
+       assert.Equal(t, `"5"`, string(migratingSlotBytes))
+
+       migratingSlot = MigratingSlot{SlotRange: SlotRange{Start: 5, Stop: 10}, 
IsMigrating: true}
+       migratingSlotBytes, err = json.Marshal(&migratingSlot)
+       require.NoError(t, err)
+       assert.Equal(t, `"5-10"`, string(migratingSlotBytes))
+
+       migratingSlot = MigratingSlot{SlotRange: SlotRange{Start: 5, Stop: 10}, 
IsMigrating: false}
+       migratingSlotBytes, err = json.Marshal(&migratingSlot)
+       require.NoError(t, err)
+       assert.Equal(t, `-1`, string(migratingSlotBytes))
+}
+
+func TestMigrateSlotRange_MarshalAndUnmarshalJSON(t *testing.T) {
+       var slotRange SlotRange
+
+       slotBytes, err := json.Marshal("-100")
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &slotRange)
+       require.NotNil(t, err, "expects error since input is a negative number")
+       assert.Equal(t, SlotRange{Start: 0, Stop: 0}, slotRange)
+
+       slotBytes, err = json.Marshal("-100-100000")
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &slotRange)
+       require.NotNil(t, err, "expects error since input is out of range")
+       assert.Equal(t, SlotRange{Start: 0, Stop: 0}, slotRange)
+
+       slotBytes, err = json.Marshal("456")
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &slotRange)
+       require.NoError(t, err)
+       assert.Equal(t, SlotRange{Start: 456, Stop: 456}, slotRange)
+
+       slotBytes, err = json.Marshal("123-456")
+       require.NoError(t, err)
+       err = json.Unmarshal(slotBytes, &slotRange)
+       require.NoError(t, err)
+       assert.Equal(t, SlotRange{Start: 123, Stop: 456}, slotRange)
+}
+
 func TestSlotRange_Parse(t *testing.T) {
        sr, err := ParseSlotRange("1-12")
        assert.Nil(t, err)
@@ -82,31 +193,31 @@ func TestAddSlotToSlotRanges(t *testing.T) {
        }
        slotRange, err := NewSlotRange(0, 0)
        require.NoError(t, err)
-       slotRanges = AddSlotToSlotRanges(slotRanges, *slotRange)
+       slotRanges = AddSlotToSlotRanges(slotRanges, slotRange)
        require.Equal(t, 3, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 0, Stop: 20}, slotRanges[0], 
slotRanges)
 
        slotRange, err = NewSlotRange(21, 21)
        require.NoError(t, err)
-       slotRanges = AddSlotToSlotRanges(slotRanges, *slotRange)
+       slotRanges = AddSlotToSlotRanges(slotRanges, slotRange)
        require.Equal(t, 3, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 0, Stop: 21}, slotRanges[0], 
slotRanges)
 
        slotRange, err = NewSlotRange(50, 50)
        require.NoError(t, err)
-       slotRanges = AddSlotToSlotRanges(slotRanges, *slotRange)
+       slotRanges = AddSlotToSlotRanges(slotRanges, slotRange)
        require.Equal(t, 4, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 50, Stop: 50}, slotRanges[1], 
slotRanges)
 
        slotRange, err = NewSlotRange(200, 200)
        require.NoError(t, err)
-       slotRanges = AddSlotToSlotRanges(slotRanges, *slotRange)
+       slotRanges = AddSlotToSlotRanges(slotRanges, slotRange)
        require.Equal(t, 3, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 101, Stop: 300}, slotRanges[2], 
slotRanges)
 
        slotRange, err = NewSlotRange(400, 400)
        require.NoError(t, err)
-       slotRanges = AddSlotToSlotRanges(slotRanges, *slotRange)
+       slotRanges = AddSlotToSlotRanges(slotRanges, slotRange)
        require.Equal(t, 4, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 400, Stop: 400}, slotRanges[3], 
slotRanges)
 }
@@ -119,56 +230,56 @@ func TestRemoveSlotRanges(t *testing.T) {
        }
        slotRange, err := NewSlotRange(0, 0)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 3, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 1, Stop: 20}, slotRanges[0], 
slotRanges)
 
        slotRange, err = NewSlotRange(21, 21)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 3, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 1, Stop: 20}, slotRanges[0], 
slotRanges)
 
        slotRange, err = NewSlotRange(20, 20)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 3, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 1, Stop: 19}, slotRanges[0], 
slotRanges)
 
        slotRange, err = NewSlotRange(150, 150)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 4, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 101, Stop: 149}, slotRanges[1], 
slotRanges)
 
        slotRange, err = NewSlotRange(101, 101)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 4, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 102, Stop: 149}, slotRanges[1], 
slotRanges)
 
        slotRange, err = NewSlotRange(199, 199)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 4, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 151, Stop: 198}, slotRanges[2], 
slotRanges)
 
        slotRange, err = NewSlotRange(300, 300)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 4, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 201, Stop: 299}, slotRanges[3], 
slotRanges)
 
        slotRange, err = NewSlotRange(298, 298)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 5, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 201, Stop: 297}, slotRanges[3], 
slotRanges)
        require.EqualValues(t, SlotRange{Start: 299, Stop: 299}, slotRanges[4], 
slotRanges)
 
        slotRange, err = NewSlotRange(299, 299)
        require.NoError(t, err)
-       slotRanges = RemoveSlotFromSlotRanges(slotRanges, *slotRange)
+       slotRanges = RemoveSlotFromSlotRanges(slotRanges, slotRange)
        require.Equal(t, 4, len(slotRanges), slotRanges)
        require.EqualValues(t, SlotRange{Start: 201, Stop: 297}, slotRanges[3], 
slotRanges)
 }
@@ -187,7 +298,7 @@ func TestSlotRange_HasOverlap(t *testing.T) {
                Stop  int
        }
        type args struct {
-               that *SlotRange
+               that SlotRange
        }
        tests := []struct {
                name   string
@@ -198,43 +309,43 @@ func TestSlotRange_HasOverlap(t *testing.T) {
                {
                        name:   "0-5 does not overlap 6-7",
                        fields: fields{Start: 0, Stop: 5},
-                       args:   args{&SlotRange{Start: 6, Stop: 7}},
+                       args:   args{SlotRange{Start: 6, Stop: 7}},
                        want:   false,
                },
                {
                        name:   "0-5 does overlap 3-4",
                        fields: fields{Start: 0, Stop: 5},
-                       args:   args{&SlotRange{Start: 3, Stop: 4}},
+                       args:   args{SlotRange{Start: 3, Stop: 4}},
                        want:   true,
                },
                {
                        name:   "0-5 does overlap 5-8",
                        fields: fields{Start: 0, Stop: 5},
-                       args:   args{&SlotRange{Start: 5, Stop: 8}},
+                       args:   args{SlotRange{Start: 5, Stop: 8}},
                        want:   true,
                },
                {
                        name:   "0-5 does overlap 4-8",
                        fields: fields{Start: 0, Stop: 5},
-                       args:   args{&SlotRange{Start: 4, Stop: 8}},
+                       args:   args{SlotRange{Start: 4, Stop: 8}},
                        want:   true,
                },
                {
                        name:   "0-100 does not overlap 101-150",
                        fields: fields{Start: 0, Stop: 100},
-                       args:   args{&SlotRange{Start: 101, Stop: 150}},
+                       args:   args{SlotRange{Start: 101, Stop: 150}},
                        want:   false,
                },
                {
                        name:   "50-100 does overlap 30-50",
                        fields: fields{Start: 50, Stop: 100},
-                       args:   args{&SlotRange{Start: 30, Stop: 50}},
+                       args:   args{SlotRange{Start: 30, Stop: 50}},
                        want:   true,
                },
                {
                        name:   "50-100 does overlap 50-51",
                        fields: fields{Start: 50, Stop: 100},
-                       args:   args{&SlotRange{Start: 50, Stop: 51}},
+                       args:   args{SlotRange{Start: 50, Stop: 51}},
                        want:   true,
                },
        }

Reply via email to