From fe5aa06a75a3afa36a2fd304e6f791d493284b35 Mon Sep 17 00:00:00 2001
From: Anton Nikiforov <an.nikiforov@yadro.com>
Date: Wed, 30 Aug 2023 13:43:43 +0300
Subject: [PATCH] [#665] node: Bind length of copies number to number of
 replicas

Allow to use one digit in copies number array for backward compatibility.

Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
---
 .../object_manager/placement/traverser.go     |  8 +++++++
 .../placement/traverser_test.go               | 21 ++++++++++++-------
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/pkg/services/object_manager/placement/traverser.go b/pkg/services/object_manager/placement/traverser.go
index ad1077047..dc9ab5e7a 100644
--- a/pkg/services/object_manager/placement/traverser.go
+++ b/pkg/services/object_manager/placement/traverser.go
@@ -58,6 +58,9 @@ var errNilBuilder = errors.New("placement builder is nil")
 
 var errNilPolicy = errors.New("placement policy is nil")
 
+var errCopiesNumberLen = errors.New("copies number accepts only one number or array with length " +
+	"equal to length of replicas")
+
 func defaultCfg() *cfg {
 	return &cfg{
 		trackCopies: true,
@@ -74,6 +77,11 @@ func NewTraverser(opts ...Option) (*Traverser, error) {
 		}
 	}
 
+	cnLen := len(cfg.copyNumbers)
+	if cnLen > 0 && cnLen != 1 && cnLen != cfg.policy.NumberOfReplicas() {
+		return nil, errCopiesNumberLen
+	}
+
 	if cfg.builder == nil {
 		return nil, fmt.Errorf("%s: %w", invalidOptsMsg, errNilBuilder)
 	} else if !cfg.policySet {
diff --git a/pkg/services/object_manager/placement/traverser_test.go b/pkg/services/object_manager/placement/traverser_test.go
index e16f33759..9b70efc73 100644
--- a/pkg/services/object_manager/placement/traverser_test.go
+++ b/pkg/services/object_manager/placement/traverser_test.go
@@ -220,6 +220,7 @@ func TestTraverserRemValues(t *testing.T) {
 		name        string
 		copyNumbers []uint32
 		expectedRem []int
+		expectedErr error
 	}{
 		{
 			name:        "zero copy numbers",
@@ -231,11 +232,6 @@ func TestTraverserRemValues(t *testing.T) {
 			copyNumbers: []uint32{0},
 			expectedRem: replicas,
 		},
-		{
-			name:        "compatible zero copy numbers, len 2",
-			copyNumbers: []uint32{0, 0},
-			expectedRem: replicas,
-		},
 		{
 			name:        "compatible zero copy numbers, len 3",
 			copyNumbers: []uint32{0, 0, 0},
@@ -253,9 +249,14 @@ func TestTraverserRemValues(t *testing.T) {
 		},
 		{
 			name:        "multiple copy numbers for multiple replicas",
-			copyNumbers: []uint32{1, 1},
+			copyNumbers: []uint32{1, 1, 4},
 			expectedRem: []int{1, 1, 4},
 		},
+		{
+			name:        "incompatible copies number vector",
+			copyNumbers: []uint32{1, 1},
+			expectedErr: errCopiesNumberLen,
+		},
 	}
 
 	for _, testCase := range testCases {
@@ -265,8 +266,12 @@ func TestTraverserRemValues(t *testing.T) {
 				UseBuilder(&testBuilder{vectors: nodesCopy}),
 				WithCopyNumbers(testCase.copyNumbers),
 			)
-			require.NoError(t, err)
-			require.Equal(t, testCase.expectedRem, tr.rem)
+			if testCase.expectedErr == nil {
+				require.NoError(t, err, testCase.name)
+				require.Equal(t, testCase.expectedRem, tr.rem, testCase.name)
+			} else {
+				require.Error(t, err, testCase.expectedErr, testCase.name)
+			}
 		})
 	}
 }