node: Bind length of copies number to number of replicas #665

Merged
fyrchik merged 1 commit from acid-ant/frostfs-node:bugfix/compare-copies-num-with-rep into master 2023-08-30 17:11:59 +00:00
2 changed files with 21 additions and 8 deletions

View file

@ -58,6 +58,9 @@ var errNilBuilder = errors.New("placement builder is nil")
var errNilPolicy = errors.New("placement policy 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 { func defaultCfg() *cfg {
return &cfg{ return &cfg{
trackCopies: true, trackCopies: true,
@ -74,6 +77,11 @@ func NewTraverser(opts ...Option) (*Traverser, error) {
} }
} }
fyrchik marked this conversation as resolved Outdated

What about a defining a constant error (like errNilBuilder?

What about a defining a constant error (like `errNilBuilder`?

Why not, added.

Why not, added.
cnLen := len(cfg.copyNumbers)
if cnLen > 0 && cnLen != 1 && cnLen != cfg.policy.NumberOfReplicas() {
return nil, errCopiesNumberLen
}
if cfg.builder == nil { if cfg.builder == nil {
return nil, fmt.Errorf("%s: %w", invalidOptsMsg, errNilBuilder) return nil, fmt.Errorf("%s: %w", invalidOptsMsg, errNilBuilder)
} else if !cfg.policySet { } else if !cfg.policySet {

View file

@ -220,6 +220,7 @@ func TestTraverserRemValues(t *testing.T) {
name string name string
copyNumbers []uint32 copyNumbers []uint32
expectedRem []int expectedRem []int
expectedErr error
}{ }{
{ {
name: "zero copy numbers", name: "zero copy numbers",
@ -231,11 +232,6 @@ func TestTraverserRemValues(t *testing.T) {
copyNumbers: []uint32{0}, copyNumbers: []uint32{0},
expectedRem: replicas, expectedRem: replicas,
}, },
{
name: "compatible zero copy numbers, len 2",
fyrchik marked this conversation as resolved Outdated

Do we have INcompatible tests?

Do we have INcompatible tests?
Review

No, let's create them. Updated.

No, let's create them. Updated.
copyNumbers: []uint32{0, 0},
expectedRem: replicas,
},
{ {
name: "compatible zero copy numbers, len 3", name: "compatible zero copy numbers, len 3",
copyNumbers: []uint32{0, 0, 0}, copyNumbers: []uint32{0, 0, 0},
@ -253,9 +249,14 @@ func TestTraverserRemValues(t *testing.T) {
}, },
{ {
name: "multiple copy numbers for multiple replicas", name: "multiple copy numbers for multiple replicas",
copyNumbers: []uint32{1, 1}, copyNumbers: []uint32{1, 1, 4},
expectedRem: []int{1, 1, 4}, expectedRem: []int{1, 1, 4},
}, },
{
name: "incompatible copies number vector",
copyNumbers: []uint32{1, 1},
expectedErr: errCopiesNumberLen,
},
} }
for _, testCase := range testCases { for _, testCase := range testCases {
@ -265,8 +266,12 @@ func TestTraverserRemValues(t *testing.T) {
UseBuilder(&testBuilder{vectors: nodesCopy}), UseBuilder(&testBuilder{vectors: nodesCopy}),
WithCopyNumbers(testCase.copyNumbers), WithCopyNumbers(testCase.copyNumbers),
) )
require.NoError(t, err) if testCase.expectedErr == nil {
require.Equal(t, testCase.expectedRem, tr.rem) require.NoError(t, err, testCase.name)
require.Equal(t, testCase.expectedRem, tr.rem, testCase.name)
} else {
require.Error(t, err, testCase.expectedErr, testCase.name)
}
}) })
} }
} }