diff --git a/core/common/nameidmapper.go b/core/common/nameidmapper.go index a2c8dd0..9577b00 100644 --- a/core/common/nameidmapper.go +++ b/core/common/nameidmapper.go @@ -15,7 +15,10 @@ package common // NameIDMapper assigns stable int32 IDs to string names on demand. -// IDs are assigned sequentially starting from 0. +// IDs are assigned sequentially starting from 1. Zero is reserved as the +// proto3 "unset" sentinel so consumers using encoding/json (which honors +// `omitempty` on int32 fields) or any client that treats GetId() == 0 as +// missing never silently lose real entries. type NameIDMapper struct { nameToID map[string]int32 nextID int32 @@ -25,7 +28,7 @@ type NameIDMapper struct { func NewNameIDMapper() *NameIDMapper { return &NameIDMapper{ nameToID: make(map[string]int32), - nextID: 0, + nextID: 1, } } diff --git a/core/common/nameidmapper_test.go b/core/common/nameidmapper_test.go index 90f0ac6..008e2fc 100644 --- a/core/common/nameidmapper_test.go +++ b/core/common/nameidmapper_test.go @@ -15,6 +15,7 @@ package common import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -24,15 +25,15 @@ func TestNameIDMapper_AssignsSequentialAndStableIDs(t *testing.T) { mapper := NewNameIDMapper() idA := mapper.ID("a") - assert.Equal(t, int32(0), idA) + assert.Equal(t, int32(1), idA, "first id must be 1; 0 is reserved as proto3 unspecified") idB := mapper.ID("b") - assert.Equal(t, int32(1), idB) + assert.Equal(t, int32(2), idB) // Re-requesting 'a' should return the same id idA2 := mapper.ID("a") assert.Equal(t, idA, idA2) // A new, third name should get the next sequential id idC := mapper.ID("c") - assert.Equal(t, int32(2), idC) + assert.Equal(t, int32(3), idC) } func TestNameIDMapper_MappingAndInvert(t *testing.T) { @@ -44,18 +45,28 @@ func TestNameIDMapper_MappingAndInvert(t *testing.T) { mapping := mapper.Mapping() assert.Equal(t, 3, len(mapping)) - assert.Equal(t, int32(0), mapping["x"]) - assert.Equal(t, int32(1), mapping["y"]) - assert.Equal(t, int32(2), mapping["z"]) + assert.Equal(t, int32(1), mapping["x"]) + assert.Equal(t, int32(2), mapping["y"]) + assert.Equal(t, int32(3), mapping["z"]) inv := mapper.Invert() assert.Equal(t, 3, len(inv)) - assert.Equal(t, "x", inv[0]) - assert.Equal(t, "y", inv[1]) - assert.Equal(t, "z", inv[2]) + assert.Equal(t, "x", inv[1]) + assert.Equal(t, "y", inv[2]) + assert.Equal(t, "z", inv[3]) + _, hasZero := inv[0] + assert.False(t, hasZero, "id 0 must remain reserved") // Mutating the inverted map should not affect the original mapping - inv[3] = "extra" + inv[4] = "extra" _, ok := mapping["extra"] assert.False(t, ok) } + +func TestNameIDMapper_NeverAssignsZero(t *testing.T) { + mapper := NewNameIDMapper() + for i := 0; i < 1000; i++ { + id := mapper.ID(fmt.Sprintf("name-%d", i)) + assert.NotEqual(t, int32(0), id) + } +} diff --git a/core/common/utils.go b/core/common/utils.go index 4dd3b7a..91a45ac 100644 --- a/core/common/utils.go +++ b/core/common/utils.go @@ -147,9 +147,11 @@ func HashRequestOptions(opts *tangopb.RequestOptions) string { // ResultToGetTargetGraphResponse converts a Result to a GetTargetGraphResponse func ResultToGetTargetGraphResponse(result targethasher.Result) ([]*tangopb.GetTargetGraphResponse, error) { // Map target names to ids. This list is topologically sorted, so the ids are stable. + // IDs start at 1 — 0 is reserved as the proto3 "unset" sentinel so consumers using + // encoding/json (which honors `omitempty` on int32 fields) never silently lose a target. targetNamesMapping := make(map[string]int32, len(result.TargetNames)) for i, name := range result.TargetNames { - targetNamesMapping[name] = int32(i) + targetNamesMapping[name] = int32(i + 1) } ruleTypeMapper := NewNameIDMapper()