From 6a69a896e5970bd92786a00c60b897664dcbffb1 Mon Sep 17 00:00:00 2001
From: Evgenii Stratonikov <evgeniy@nspcc.ru>
Date: Tue, 20 Oct 2020 11:31:45 +0300
Subject: [PATCH] [#174] netmap: process unspecified attribute as ID

When selector attribute is provided it should be
silently replaced by transparent ID, so each node
resides in a separate bucket.

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
---
 pkg/netmap/selector.go      | 56 ++++++++++++++++++++++++++-----------
 pkg/netmap/selector_test.go | 22 +++++++++++++++
 2 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/pkg/netmap/selector.go b/pkg/netmap/selector.go
index 73a0669..30985ac 100644
--- a/pkg/netmap/selector.go
+++ b/pkg/netmap/selector.go
@@ -43,23 +43,27 @@ func GetNodesCount(p *netmap.PlacementPolicy, s *netmap.Selector) (int, int) {
 // getSelection returns nodes grouped by s.attribute.
 func (c *Context) getSelection(p *netmap.PlacementPolicy, s *netmap.Selector) ([]Nodes, error) {
 	bucketCount, nodesInBucket := GetNodesCount(p, s)
-	m := c.getSelectionBase(s)
-	if len(m) < bucketCount {
+	buckets := c.getSelectionBase(s)
+	if len(buckets) < bucketCount {
 		return nil, fmt.Errorf("%w: '%s'", ErrNotEnoughNodes, s.GetName())
 	}
 
-	keys := make(sort.StringSlice, 0, len(m))
-	for k := range m {
-		keys = append(keys, k)
-	}
 	if len(c.pivot) == 0 {
-		// deterministic order in case of zero seed
-		keys.Sort()
+		// Deterministic order in case of zero seed.
+		if s.GetAttribute() == "" {
+			sort.Slice(buckets, func(i, j int) bool {
+				return buckets[i].nodes[0].ID < buckets[j].nodes[0].ID
+			})
+		} else {
+			sort.Slice(buckets, func(i, j int) bool {
+				return buckets[i].attr < buckets[j].attr
+			})
+		}
 	}
 
-	nodes := make([]Nodes, 0, len(m))
-	for i := range keys {
-		ns := m[keys[i]]
+	nodes := make([]Nodes, 0, len(buckets))
+	for i := range buckets {
+		ns := buckets[i].nodes
 		if len(ns) >= nodesInBucket {
 			nodes = append(nodes, ns[:nodesInBucket])
 		}
@@ -77,21 +81,39 @@ func (c *Context) getSelection(p *netmap.PlacementPolicy, s *netmap.Selector) ([
 	return nodes[:bucketCount], nil
 }
 
+type nodeAttrPair struct {
+	attr  string
+	nodes Nodes
+}
+
 // getSelectionBase returns nodes grouped by selector attribute.
-func (c *Context) getSelectionBase(s *netmap.Selector) map[string]Nodes {
+// It it guaranteed that each pair will contain at least one node.
+func (c *Context) getSelectionBase(s *netmap.Selector) []nodeAttrPair {
 	f := c.Filters[s.GetFilter()]
 	isMain := s.GetFilter() == MainFilterName
-	result := map[string]Nodes{}
+	result := []nodeAttrPair{}
+	nodeMap := map[string]Nodes{}
+	attr := s.GetAttribute()
 	for i := range c.Netmap.Nodes {
 		if isMain || c.match(f, c.Netmap.Nodes[i]) {
-			v := c.Netmap.Nodes[i].Attribute(s.GetAttribute())
-			result[v] = append(result[v], c.Netmap.Nodes[i])
+			if attr == "" {
+				// Default attribute is transparent identifier which is different for every node.
+				result = append(result, nodeAttrPair{attr: "", nodes: Nodes{c.Netmap.Nodes[i]}})
+			} else {
+				v := c.Netmap.Nodes[i].Attribute(attr)
+				nodeMap[v] = append(nodeMap[v], c.Netmap.Nodes[i])
+			}
+		}
+	}
+	if attr != "" {
+		for k, ns := range nodeMap {
+			result = append(result, nodeAttrPair{attr: k, nodes: ns})
 		}
 	}
 
 	if len(c.pivot) != 0 {
-		for _, ns := range result {
-			hrw.SortSliceByWeightValue(ns, ns.Weights(c.weightFunc), c.pivotHash)
+		for i := range result {
+			hrw.SortSliceByWeightValue(result[i].nodes, result[i].nodes.Weights(c.weightFunc), c.pivotHash)
 		}
 	}
 	return result
diff --git a/pkg/netmap/selector_test.go b/pkg/netmap/selector_test.go
index 79f363e..cf581d0 100644
--- a/pkg/netmap/selector_test.go
+++ b/pkg/netmap/selector_test.go
@@ -9,6 +9,28 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestPlacementPolicy_UnspecifiedClause(t *testing.T) {
+	p := newPlacementPolicy(1,
+		[]*netmap.Replica{newReplica(1, "X")},
+		[]*netmap.Selector{
+			newSelector("X", "", netmap.Distinct, 4, "*"),
+		},
+		nil,
+	)
+	nodes := []netmap.NodeInfo{
+		nodeInfoFromAttributes("ID", "1", "Country", "RU", "City", "St.Petersburg", "SSD", "0"),
+		nodeInfoFromAttributes("ID", "2", "Country", "RU", "City", "St.Petersburg", "SSD", "1"),
+		nodeInfoFromAttributes("ID", "3", "Country", "RU", "City", "Moscow", "SSD", "1"),
+		nodeInfoFromAttributes("ID", "4", "Country", "RU", "City", "Moscow", "SSD", "1"),
+	}
+
+	nm, err := NewNetmap(NodesFromV2(nodes))
+	require.NoError(t, err)
+	v, err := nm.GetContainerNodes(p, nil)
+	require.NoError(t, err)
+	require.Equal(t, 4, len(v.Flatten()))
+}
+
 func TestPlacementPolicy_GetPlacementVectors(t *testing.T) {
 	p := newPlacementPolicy(2,
 		[]*netmap.Replica{