Skip to content

Commit f0abbc1

Browse files
Backport of paginator: fix tokenizer comparison of composite index and ID (#25792) (#25793)
Co-authored-by: Chris Roberts <[email protected]>
1 parent dac7438 commit f0abbc1

File tree

3 files changed

+121
-1
lines changed

3 files changed

+121
-1
lines changed

.changelog/25792.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
api: Fixed pagination bug which could result in duplicate results
3+
```

nomad/state/paginator/tokenizer.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"cmp"
88
"fmt"
99
"strconv"
10+
"strings"
1011
)
1112

1213
// Tokenizer is the interface that must be implemented to provide pagination
@@ -45,7 +46,32 @@ func CreateIndexAndIDTokenizer[T idAndCreateIndexGetter](target string) Tokenize
4546
index := item.GetCreateIndex()
4647
id := item.GetID()
4748
token := fmt.Sprintf("%d.%s", index, id)
48-
return token, cmp.Compare(token, target)
49+
50+
// Split the target to extract the create index and the ID values.
51+
targetParts := strings.SplitN(target, ".", 2)
52+
// If the target wasn't composed of both parts, directly compare.
53+
if len(targetParts) < 2 {
54+
return token, cmp.Compare(token, target)
55+
}
56+
57+
// Convert the create index to an integer for comparison. This
58+
// prevents a lexigraphical comparison of the create index which
59+
// can cause unexpected results when comparing index values like
60+
// '12' and '102'. If the index cannot be converted to an integer,
61+
// fall back to direct comparison.
62+
targetIndex, err := strconv.Atoi(targetParts[0])
63+
if err != nil {
64+
return token, cmp.Compare(token, target)
65+
}
66+
67+
indexCmp := cmp.Compare(index, uint64(targetIndex))
68+
if indexCmp != 0 {
69+
return token, indexCmp
70+
}
71+
72+
// If the index values are equivalent use the ID values
73+
// as the comparison.
74+
return token, cmp.Compare(id, targetParts[1])
4975
}
5076
}
5177

nomad/state/paginator/tokenizer_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,94 @@ func TestTokenizer(t *testing.T) {
5252
})
5353
}
5454
}
55+
56+
func TestCreateIndexAndIDTokenizer(t *testing.T) {
57+
ci.Parallel(t)
58+
59+
cases := []struct {
60+
name string
61+
obj *mockCreateIndexObject
62+
target string
63+
expectedToken string
64+
expectedCmp int
65+
}{
66+
{
67+
name: "common index (less)",
68+
obj: newMockCreateIndexObject(12, "aaa-bbb-ccc"),
69+
target: "12.bbb-ccc-ddd",
70+
expectedToken: "12.aaa-bbb-ccc",
71+
expectedCmp: -1,
72+
},
73+
{
74+
name: "common index (greater)",
75+
obj: newMockCreateIndexObject(12, "bbb-ccc-ddd"),
76+
target: "12.aaa-bbb-ccc",
77+
expectedToken: "12.bbb-ccc-ddd",
78+
expectedCmp: 1,
79+
},
80+
{
81+
name: "common index (equal)",
82+
obj: newMockCreateIndexObject(12, "bbb-ccc-ddd"),
83+
target: "12.bbb-ccc-ddd",
84+
expectedToken: "12.bbb-ccc-ddd",
85+
expectedCmp: 0,
86+
},
87+
{
88+
name: "less index",
89+
obj: newMockCreateIndexObject(12, "aaa-bbb-ccc"),
90+
target: "89.aaa-bbb-ccc",
91+
expectedToken: "12.aaa-bbb-ccc",
92+
expectedCmp: -1,
93+
},
94+
{
95+
name: "greater index",
96+
obj: newMockCreateIndexObject(89, "aaa-bbb-ccc"),
97+
target: "12.aaa-bbb-ccc",
98+
expectedToken: "89.aaa-bbb-ccc",
99+
expectedCmp: 1,
100+
},
101+
{
102+
name: "common index start (less)",
103+
obj: newMockCreateIndexObject(12, "aaa-bbb-ccc"),
104+
target: "102.aaa-bbb-ccc",
105+
expectedToken: "12.aaa-bbb-ccc",
106+
expectedCmp: -1,
107+
},
108+
{
109+
name: "common index start (greater)",
110+
obj: newMockCreateIndexObject(102, "aaa-bbb-ccc"),
111+
target: "12.aaa-bbb-ccc",
112+
expectedToken: "102.aaa-bbb-ccc",
113+
expectedCmp: 1,
114+
},
115+
}
116+
117+
for _, tc := range cases {
118+
t.Run(tc.name, func(t *testing.T) {
119+
fn := CreateIndexAndIDTokenizer[*mockCreateIndexObject](tc.target)
120+
actualToken, actualCmp := fn(tc.obj)
121+
must.Eq(t, tc.expectedToken, actualToken)
122+
must.Eq(t, tc.expectedCmp, actualCmp)
123+
})
124+
}
125+
}
126+
127+
func newMockCreateIndexObject(createIndex uint64, id string) *mockCreateIndexObject {
128+
return &mockCreateIndexObject{
129+
createIndex: createIndex,
130+
id: id,
131+
}
132+
}
133+
134+
type mockCreateIndexObject struct {
135+
createIndex uint64
136+
id string
137+
}
138+
139+
func (m *mockCreateIndexObject) GetCreateIndex() uint64 {
140+
return m.createIndex
141+
}
142+
143+
func (m *mockCreateIndexObject) GetID() string {
144+
return m.id
145+
}

0 commit comments

Comments
 (0)