Skip to content

Commit 69bc8c0

Browse files
joannalauujoannalauu
committed
Return all rows if PageSize == 0
Signed-off-by: Joanna Lau <118241363+joannalauu@users.noreply.github.com>
1 parent 5dcbfe1 commit 69bc8c0

File tree

2 files changed

+51
-105
lines changed

2 files changed

+51
-105
lines changed

‎common/persistence/sql/sqlplugin/postgres/domain_audit_log.go‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,21 @@ const (
3333
operation_type, created_time, last_updated_time, identity, identity_type, comment
3434
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)`
3535

36-
_listDomainAuditLogsQuery = `SELECT
36+
_selectDomainAuditLogsQuery = `SELECT
3737
event_id, domain_id, state_before, state_before_encoding, state_after, state_after_encoding,
3838
operation_type, created_time, last_updated_time, identity, identity_type, comment
3939
FROM domain_audit_log
4040
WHERE domain_id = $1 AND operation_type = $2 AND created_time >= $3 AND created_time < $4
4141
AND (created_time < $5 OR (created_time = $5 AND event_id > $6))
4242
ORDER BY created_time DESC, event_id ASC
4343
LIMIT $7`
44-
45-
_getDomainAuditLogsQuery = `SELECT
44+
_selectAllDomainAuditLogsQuery = `SELECT
4645
event_id, domain_id, state_before, state_before_encoding, state_after, state_after_encoding,
4746
operation_type, created_time, last_updated_time, identity, identity_type, comment
4847
FROM domain_audit_log
4948
WHERE domain_id = $1 AND operation_type = $2 AND created_time >= $3 AND created_time < $4
5049
AND (created_time < $5 OR (created_time = $5 AND event_id > $6))
51-
ORDER BY created_time DESC, event_id ASC
52-
LIMIT 1`
50+
ORDER BY created_time DESC, event_id ASC`
5351
)
5452

5553
// InsertIntoDomainAuditLog inserts a single row into domain_audit_log table
@@ -90,21 +88,15 @@ func (pdb *db) SelectFromDomainAuditLogs(
9088
var rows []*sqlplugin.DomainAuditLogRow
9189
if filter.PageSize > 0 {
9290
args = append(args, filter.PageSize)
93-
err := pdb.driver.SelectContext(ctx, sqlplugin.DbDefaultShard, &rows, _listDomainAuditLogsQuery, args...)
91+
err := pdb.driver.SelectContext(ctx, sqlplugin.DbDefaultShard, &rows, _selectDomainAuditLogsQuery, args...)
9492
if err != nil {
9593
return nil, err
9694
}
9795
} else {
98-
var row *sqlplugin.DomainAuditLogRow
99-
err := pdb.driver.GetContext(ctx, sqlplugin.DbDefaultShard, &row, _getDomainAuditLogsQuery, args...)
96+
err := pdb.driver.SelectContext(ctx, sqlplugin.DbDefaultShard, &rows, _selectAllDomainAuditLogsQuery, args...)
10097
if err != nil {
10198
return nil, err
10299
}
103-
if row != nil {
104-
rows = append(rows, row)
105-
}
106-
107100
}
108-
109101
return rows, nil
110102
}

‎common/persistence/sql/sqlplugin/postgres/domain_audit_log_test.go‎

Lines changed: 46 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -148,12 +148,16 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
148148
createdTime1 := time.Date(2024, 7, 1, 12, 0, 0, 0, time.UTC)
149149
createdTime2 := time.Date(2024, 6, 1, 12, 0, 0, 0, time.UTC)
150150
createdTime3 := time.Date(2024, 5, 1, 12, 0, 0, 0, time.UTC)
151+
createdTime4 := time.Date(2024, 4, 1, 12, 0, 0, 0, time.UTC)
151152

152153
eventID1 := serialization.MustParseUUID("e1111111-1111-1111-1111-111111111111")
153154
eventID2 := serialization.MustParseUUID("e2222222-2222-2222-2222-222222222222")
154155
eventID3 := serialization.MustParseUUID("e3333333-3333-3333-3333-333333333333")
155-
maxUUID := serialization.UUID{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}
156-
pageSize := 2
156+
eventID4 := serialization.MustParseUUID("e4444444-4444-4444-4444-444444444444")
157+
158+
// Default page cursor: maxCreatedTime with max UUID (no page token)
159+
defaultPageMaxCreatedTime := maxTime
160+
defaultPageMinEventID := serialization.UUID{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}
157161

158162
tests := []struct {
159163
name string
@@ -163,30 +167,24 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
163167
wantErr bool
164168
}{
165169
{
166-
name: "first page with limit",
170+
name: "pageSize limits number of results; no pageToken",
167171
filter: &sqlplugin.DomainAuditLogFilter{
168172
DomainID: domainID,
169173
OperationType: operationType,
170174
MinCreatedTime: &minTime,
171175
MaxCreatedTime: &maxTime,
172-
PageMaxCreatedTime: &maxTime,
173-
PageMinEventID: &maxUUID,
174-
PageSize: pageSize,
176+
PageSize: 2,
177+
PageMaxCreatedTime: &defaultPageMaxCreatedTime,
178+
PageMinEventID: &defaultPageMinEventID,
175179
},
176180
mockSetup: func(mockDriver *sqldriver.MockDriver) {
177181
mockDriver.EXPECT().SelectContext(
178182
gomock.Any(),
179183
sqlplugin.DbDefaultShard,
180184
gomock.Any(),
181-
gomock.Any(),
182-
gomock.Any(),
185+
_selectDomainAuditLogsQuery,
186+
domainID, operationType, minTime, maxTime, defaultPageMaxCreatedTime, defaultPageMinEventID, 2,
183187
).DoAndReturn(func(ctx context.Context, shardID int, dest interface{}, query string, args ...interface{}) error {
184-
assert.Contains(t, query, "LIMIT $7")
185-
assert.Contains(t, query, "ORDER BY created_time DESC, event_id ASC")
186-
assert.Contains(t, query, "AND (created_time < $5 OR (created_time = $5 AND event_id > $6))")
187-
assert.Equal(t, 7, len(args))
188-
assert.Equal(t, 2, args[6], "LIMIT should be 2")
189-
190188
rows := dest.(*[]*sqlplugin.DomainAuditLogRow)
191189
*rows = []*sqlplugin.DomainAuditLogRow{
192190
{
@@ -222,34 +220,38 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
222220
wantErr: false,
223221
},
224222
{
225-
name: "second page uses page token; GetContext returns one row",
223+
name: "pageToken filters results by createdTime and eventID; pageSize is 0",
226224
filter: &sqlplugin.DomainAuditLogFilter{
227225
DomainID: domainID,
228226
OperationType: operationType,
229227
MinCreatedTime: &minTime,
230228
MaxCreatedTime: &maxTime,
229+
PageSize: 0,
231230
PageMaxCreatedTime: &createdTime2,
232231
PageMinEventID: &eventID2,
233-
PageSize: 0,
234232
},
235233
mockSetup: func(mockDriver *sqldriver.MockDriver) {
236-
mockDriver.EXPECT().GetContext(
234+
mockDriver.EXPECT().SelectContext(
237235
gomock.Any(),
238236
sqlplugin.DbDefaultShard,
239237
gomock.Any(),
240-
gomock.Any(),
241-
gomock.Any(),
238+
_selectAllDomainAuditLogsQuery,
239+
domainID, operationType, minTime, maxTime, createdTime2, eventID2,
242240
).DoAndReturn(func(ctx context.Context, shardID int, dest interface{}, query string, args ...interface{}) error {
243-
assert.Contains(t, query, "AND (created_time < $5 OR (created_time = $5 AND event_id > $6))")
244-
assert.Contains(t, query, "ORDER BY created_time DESC, event_id ASC")
245-
assert.NotContains(t, query, "LIMIT")
246-
247-
row := dest.(**sqlplugin.DomainAuditLogRow)
248-
*row = &sqlplugin.DomainAuditLogRow{
249-
EventID: eventID3,
250-
DomainID: domainID,
251-
CreatedTime: createdTime3,
252-
OperationType: operationType,
241+
rows := dest.(*[]*sqlplugin.DomainAuditLogRow)
242+
*rows = []*sqlplugin.DomainAuditLogRow{
243+
{
244+
EventID: eventID3,
245+
DomainID: domainID,
246+
CreatedTime: createdTime3,
247+
OperationType: operationType,
248+
},
249+
{
250+
EventID: eventID4,
251+
DomainID: domainID,
252+
CreatedTime: createdTime4,
253+
OperationType: operationType,
254+
},
253255
}
254256
return nil
255257
})
@@ -261,80 +263,33 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
261263
CreatedTime: createdTime3,
262264
OperationType: operationType,
263265
},
266+
{
267+
EventID: eventID4,
268+
DomainID: domainID,
269+
CreatedTime: createdTime4,
270+
OperationType: operationType,
271+
},
264272
},
265273
wantErr: false,
266274
},
267275
{
268-
name: "get success with no results",
276+
name: "success with no results",
269277
filter: &sqlplugin.DomainAuditLogFilter{
270278
DomainID: domainID,
271279
OperationType: operationType,
272280
MinCreatedTime: &minTime,
273281
MaxCreatedTime: &maxTime,
274-
PageMaxCreatedTime: &maxTime,
275-
PageMinEventID: &maxUUID,
276-
},
277-
mockSetup: func(mockDriver *sqldriver.MockDriver) {
278-
mockDriver.EXPECT().GetContext(
279-
gomock.Any(),
280-
sqlplugin.DbDefaultShard,
281-
gomock.Any(),
282-
gomock.Any(),
283-
gomock.Any(),
284-
).DoAndReturn(func(ctx context.Context, shardID int, dest interface{}, query string, args ...interface{}) error {
285-
assert.NotContains(t, query, "LIMIT")
286-
assert.Contains(t, query, "ORDER BY created_time DESC, event_id ASC")
287-
assert.Equal(t, 6, len(args), "should have 6 args")
288-
return nil
289-
})
290-
},
291-
wantRows: []*sqlplugin.DomainAuditLogRow{},
292-
wantErr: false,
293-
},
294-
{
295-
name: "error when get fails",
296-
filter: &sqlplugin.DomainAuditLogFilter{
297-
DomainID: domainID,
298-
OperationType: operationType,
299-
MinCreatedTime: &minTime,
300-
MaxCreatedTime: &maxTime,
301-
PageMaxCreatedTime: &maxTime,
302-
PageMinEventID: &maxUUID,
303-
},
304-
mockSetup: func(mockDriver *sqldriver.MockDriver) {
305-
mockDriver.EXPECT().GetContext(
306-
gomock.Any(),
307-
sqlplugin.DbDefaultShard,
308-
gomock.Any(),
309-
gomock.Any(),
310-
gomock.Any(),
311-
).Return(errors.New("get failed"))
312-
},
313-
wantRows: nil,
314-
wantErr: true,
315-
},
316-
{
317-
name: "select success with no results",
318-
filter: &sqlplugin.DomainAuditLogFilter{
319-
DomainID: domainID,
320-
OperationType: operationType,
321-
MinCreatedTime: &minTime,
322-
MaxCreatedTime: &maxTime,
323-
PageMaxCreatedTime: &maxTime,
324-
PageMinEventID: &maxUUID,
325-
PageSize: pageSize,
282+
PageMaxCreatedTime: &defaultPageMaxCreatedTime,
283+
PageMinEventID: &defaultPageMinEventID,
326284
},
327285
mockSetup: func(mockDriver *sqldriver.MockDriver) {
328286
mockDriver.EXPECT().SelectContext(
329287
gomock.Any(),
330288
sqlplugin.DbDefaultShard,
331289
gomock.Any(),
332-
gomock.Any(),
333-
gomock.Any(),
290+
_selectAllDomainAuditLogsQuery,
291+
domainID, operationType, minTime, maxTime, defaultPageMaxCreatedTime, defaultPageMinEventID,
334292
).DoAndReturn(func(ctx context.Context, shardID int, dest interface{}, query string, args ...interface{}) error {
335-
assert.Contains(t, query, "LIMIT")
336-
assert.Contains(t, query, "ORDER BY created_time DESC, event_id ASC")
337-
assert.Equal(t, 7, len(args), "should have 7 args")
338293
return nil
339294
})
340295
},
@@ -348,17 +303,16 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
348303
OperationType: operationType,
349304
MinCreatedTime: &minTime,
350305
MaxCreatedTime: &maxTime,
351-
PageMaxCreatedTime: &maxTime,
352-
PageMinEventID: &maxUUID,
353-
PageSize: pageSize,
306+
PageMaxCreatedTime: &defaultPageMaxCreatedTime,
307+
PageMinEventID: &defaultPageMinEventID,
354308
},
355309
mockSetup: func(mockDriver *sqldriver.MockDriver) {
356310
mockDriver.EXPECT().SelectContext(
357311
gomock.Any(),
358312
sqlplugin.DbDefaultShard,
359313
gomock.Any(),
360-
gomock.Any(),
361-
gomock.Any(),
314+
_selectAllDomainAuditLogsQuery,
315+
domainID, operationType, minTime, maxTime, defaultPageMaxCreatedTime, defaultPageMinEventID,
362316
).Return(errors.New("select failed"))
363317
},
364318
wantRows: nil,

0 commit comments

Comments
 (0)