From 6493589bb44db7819eb5dd673573a12e1d791a09 Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Wed, 15 Apr 2026 19:54:15 +0700 Subject: [PATCH] types: make CheckTarget.String() unambiguous by always emitting all fields The previous implementation skipped empty fields, which meant targets differing only in which fields were populated could produce the same string (e.g. {UserId:"A"} and {DomainId:"A"} both gave "A"). This caused key collisions when the string was used in storage index keys. --- checker/types.go | 17 ++++------------- checker/types_test.go | 9 ++++++--- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/checker/types.go b/checker/types.go index 8d61f05..bd048a5 100644 --- a/checker/types.go +++ b/checker/types.go @@ -21,7 +21,6 @@ import ( "context" "encoding/json" "fmt" - "strings" "time" ) @@ -66,19 +65,11 @@ func (t CheckTarget) Scope() CheckScopeType { return CheckScopeUser } -// String returns a stable string representation of the target. +// String returns a stable, unambiguous string representation of the target. +// All three fields are always present (even when empty) so that different +// targets never produce the same string. func (t CheckTarget) String() string { - var parts []string - if t.UserId != "" { - parts = append(parts, t.UserId) - } - if t.DomainId != "" { - parts = append(parts, t.DomainId) - } - if t.ServiceId != "" { - parts = append(parts, t.ServiceId) - } - return strings.Join(parts, "/") + return t.UserId + "/" + t.DomainId + "/" + t.ServiceId } // CheckerAvailability declares on which scopes a checker can operate. diff --git a/checker/types_test.go b/checker/types_test.go index 7b66af2..b2ee5db 100644 --- a/checker/types_test.go +++ b/checker/types_test.go @@ -148,10 +148,13 @@ func TestCheckTarget_String(t *testing.T) { target CheckTarget want string }{ - {CheckTarget{}, ""}, - {CheckTarget{UserId: "u1"}, "u1"}, - {CheckTarget{UserId: "u1", DomainId: "d1"}, "u1/d1"}, + {CheckTarget{}, "//"}, + {CheckTarget{UserId: "u1"}, "u1//"}, + {CheckTarget{UserId: "u1", DomainId: "d1"}, "u1/d1/"}, {CheckTarget{UserId: "u1", DomainId: "d1", ServiceId: "s1"}, "u1/d1/s1"}, + // Ensure different targets with different empty fields don't collide. + {CheckTarget{DomainId: "d1"}, "/d1/"}, + {CheckTarget{ServiceId: "s1"}, "//s1"}, } for _, tt := range tests { if got := tt.target.String(); got != tt.want {