diff --git a/.changelog/25792.txt b/.changelog/25792.txt new file mode 100644 index 00000000000..79662302d24 --- /dev/null +++ b/.changelog/25792.txt @@ -0,0 +1,3 @@ +```release-note:bug +api: Fixed pagination bug which could result in duplicate results +``` diff --git a/nomad/state/paginator/tokenizer.go b/nomad/state/paginator/tokenizer.go index 181377a419f..db3a2d4212e 100644 --- a/nomad/state/paginator/tokenizer.go +++ b/nomad/state/paginator/tokenizer.go @@ -7,6 +7,7 @@ import ( "cmp" "fmt" "strconv" + "strings" ) // Tokenizer is the interface that must be implemented to provide pagination @@ -45,7 +46,32 @@ func CreateIndexAndIDTokenizer[T idAndCreateIndexGetter](target string) Tokenize index := item.GetCreateIndex() id := item.GetID() token := fmt.Sprintf("%d.%s", index, id) - return token, cmp.Compare(token, target) + + // Split the target to extract the create index and the ID values. + targetParts := strings.SplitN(target, ".", 2) + // If the target wasn't composed of both parts, directly compare. + if len(targetParts) < 2 { + return token, cmp.Compare(token, target) + } + + // Convert the create index to an integer for comparison. This + // prevents a lexigraphical comparison of the create index which + // can cause unexpected results when comparing index values like + // '12' and '102'. If the index cannot be converted to an integer, + // fall back to direct comparison. + targetIndex, err := strconv.Atoi(targetParts[0]) + if err != nil { + return token, cmp.Compare(token, target) + } + + indexCmp := cmp.Compare(index, uint64(targetIndex)) + if indexCmp != 0 { + return token, indexCmp + } + + // If the index values are equivalent use the ID values + // as the comparison. + return token, cmp.Compare(id, targetParts[1]) } } diff --git a/nomad/state/paginator/tokenizer_test.go b/nomad/state/paginator/tokenizer_test.go index 73f41c64042..2ac5b062976 100644 --- a/nomad/state/paginator/tokenizer_test.go +++ b/nomad/state/paginator/tokenizer_test.go @@ -52,3 +52,94 @@ func TestTokenizer(t *testing.T) { }) } } + +func TestCreateIndexAndIDTokenizer(t *testing.T) { + ci.Parallel(t) + + cases := []struct { + name string + obj *mockCreateIndexObject + target string + expectedToken string + expectedCmp int + }{ + { + name: "common index (less)", + obj: newMockCreateIndexObject(12, "aaa-bbb-ccc"), + target: "12.bbb-ccc-ddd", + expectedToken: "12.aaa-bbb-ccc", + expectedCmp: -1, + }, + { + name: "common index (greater)", + obj: newMockCreateIndexObject(12, "bbb-ccc-ddd"), + target: "12.aaa-bbb-ccc", + expectedToken: "12.bbb-ccc-ddd", + expectedCmp: 1, + }, + { + name: "common index (equal)", + obj: newMockCreateIndexObject(12, "bbb-ccc-ddd"), + target: "12.bbb-ccc-ddd", + expectedToken: "12.bbb-ccc-ddd", + expectedCmp: 0, + }, + { + name: "less index", + obj: newMockCreateIndexObject(12, "aaa-bbb-ccc"), + target: "89.aaa-bbb-ccc", + expectedToken: "12.aaa-bbb-ccc", + expectedCmp: -1, + }, + { + name: "greater index", + obj: newMockCreateIndexObject(89, "aaa-bbb-ccc"), + target: "12.aaa-bbb-ccc", + expectedToken: "89.aaa-bbb-ccc", + expectedCmp: 1, + }, + { + name: "common index start (less)", + obj: newMockCreateIndexObject(12, "aaa-bbb-ccc"), + target: "102.aaa-bbb-ccc", + expectedToken: "12.aaa-bbb-ccc", + expectedCmp: -1, + }, + { + name: "common index start (greater)", + obj: newMockCreateIndexObject(102, "aaa-bbb-ccc"), + target: "12.aaa-bbb-ccc", + expectedToken: "102.aaa-bbb-ccc", + expectedCmp: 1, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fn := CreateIndexAndIDTokenizer[*mockCreateIndexObject](tc.target) + actualToken, actualCmp := fn(tc.obj) + must.Eq(t, tc.expectedToken, actualToken) + must.Eq(t, tc.expectedCmp, actualCmp) + }) + } +} + +func newMockCreateIndexObject(createIndex uint64, id string) *mockCreateIndexObject { + return &mockCreateIndexObject{ + createIndex: createIndex, + id: id, + } +} + +type mockCreateIndexObject struct { + createIndex uint64 + id string +} + +func (m *mockCreateIndexObject) GetCreateIndex() uint64 { + return m.createIndex +} + +func (m *mockCreateIndexObject) GetID() string { + return m.id +}