Skip to content

Commit 1a42a94

Browse files
authored
fix: Remove MaxCreatedTime from DomainAuditLogFilter (#7725)
<!-- 1-2 line summary of WHAT changed technically: - Always link the relevant projects GitHub issue, unless it is a minor bugfix - Good: "Modified FailoverDomain mapper to allow null ActiveClusterName #320" - Bad: "added nil check" --> **What changed?** - Removed `MaxCreatedTime` from `DomainAuditLogFilter` <!-- Your goal is to provide all the required context for a future maintainer to understand the reasons for making this change (see https://cbea.ms/git-commit/#why-not-how). How did this work previously (and what was wrong with it)? What has changed, and why did you solve it this way? - Good: "Active-active domains have independent cluster attributes per region. Previously, modifying cluster attributes required spedifying the default ActiveClusterName which updates the global domain default. This prevents operators from updating regional configurations without affecting the primary cluster designation. This change allows attribute updates to be independent of active cluster selection." - Bad: "Improves domain handling" --> **Why?** - Remove duplicate logic between `MaxCreatedTime` and `PageMaxCreatedTime` in domain audit filtering Part of: #7602 <!-- Include specific test commands and setup. Please include the exact commands such that another maintainer or contributor can reproduce the test steps taken. - e.g Unit test commands with exact invocation `go test -v ./common/types/mapper/proto -run TestFailoverDomainRequest` - For integration tests include setup steps and test commands Example: "Started local server with `./cadence start`, then ran `make test_e2e`" - For local simulation testing include setup steps for the server and how you ran the tests - Good: Full commands that reviewers can copy-paste to verify - Bad: "Tested locally" or "Added tests" --> **How did you test it?** - Unit tests: `go test -v ./common/persistence/sql -run TestGetDomainAuditLogs TestSelectFromDomainAuditLogs` <!-- If there are risks that the release engineer should know about document them here. For example: - Has an API/IDL been modified? Is it backwards/forwards compatible? If not, what are the repecussions? - Has a schema change been introduced? Is it possible to roll back? - Has a feature flag been re-used for a new purpose? - Is there a potential performance concern? Is the change modifying core task processing logic? - If truly N/A, you can mark it as such --> **Potential risks** N/A, logic stays the same <!-- If this PR completes a user facing feature or changes functionality add release notes here. Your release notes should allow a user and the release engineer to understand the changes with little context. Always ensure that the description contains a link to the relevant GitHub issue. --> **Release notes** N/A <!-- Consider whether this change requires documentation updates in the Cadence-Docs repo - If yes: mention what needs updating (or link to docs PR in cadence-docs repo) - If in doubt, add a note about potential doc needs - Only mark N/A if you're certain no docs are affected --> **Documentation Changes** N/A --- ## Reviewer Validation **PR Description Quality** (check these before reviewing code): - [x] **"What changed"** provides a clear 1-2 line summary - [x] Project Issue is linked - [x] **"Why"** explains the full motivation with sufficient context - [ ] **Testing is documented:** - [ ] Unit test commands are included (with exact `go test` invocation) - [ ] Integration test setup/commands included (if integration tests were run) - [ ] Canary testing details included (if canary was mentioned) - [x] **Potential risks** section is thoughtfully filled out (or legitimately N/A) - [x] **Release notes** included if this completes a user-facing feature - [x] **Documentation** needs are addressed (or noted if uncertain) Signed-off-by: Joanna Lau <118241363+joannalauu@users.noreply.github.com>
1 parent 4202c52 commit 1a42a94

File tree

5 files changed

+9
-18
lines changed

5 files changed

+9
-18
lines changed

common/persistence/sql/sql_domain_audit_store.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ func (m *sqlDomainAuditStore) GetDomainAuditLogs(
119119
DomainID: request.DomainID,
120120
OperationType: request.OperationType,
121121
MinCreatedTime: &minCreatedTime,
122-
MaxCreatedTime: &maxCreatedTime,
123122
PageSize: request.PageSize,
124123
PageMaxCreatedTime: &pageMaxCreatedTime,
125124
PageMinEventID: &pageMinEventID,

common/persistence/sql/sql_domain_audit_store_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ func TestGetDomainAuditLogs(t *testing.T) {
226226
DomainID: "d1111111-1111-1111-1111-111111111111",
227227
OperationType: persistence.DomainAuditOperationTypeUpdate,
228228
MinCreatedTime: &minTime,
229-
MaxCreatedTime: &maxTime,
230229
PageSize: pageSize,
231230
PageMaxCreatedTime: &maxTime,
232231
PageMinEventID: &maxUUID,
@@ -302,7 +301,6 @@ func TestGetDomainAuditLogs(t *testing.T) {
302301
DomainID: "d1111111-1111-1111-1111-111111111111",
303302
OperationType: persistence.DomainAuditOperationTypeUpdate,
304303
MinCreatedTime: &minTime,
305-
MaxCreatedTime: &maxTime,
306304
PageSize: pageSize2,
307305
PageMaxCreatedTime: &expectedCreatedTime,
308306
PageMinEventID: &expectedEventID,

common/persistence/sql/sqlplugin/interfaces.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,6 @@ type (
604604
DomainID string
605605
OperationType persistence.DomainAuditOperationType
606606
MinCreatedTime *time.Time
607-
MaxCreatedTime *time.Time
608607
PageSize int
609608
// PageMaxCreatedTime and PageMinEventID are used to paginate Select queries
610609
PageMaxCreatedTime *time.Time

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,16 @@ const (
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
40-
WHERE domain_id = $1 AND operation_type = $2 AND created_time >= $3 AND created_time < $4
41-
AND (created_time < $5 OR (created_time = $5 AND event_id > $6))
40+
WHERE domain_id = $1 AND operation_type = $2 AND created_time >= $3
41+
AND (created_time < $4 OR (created_time = $4 AND event_id > $5))
4242
ORDER BY created_time DESC, event_id ASC
43-
LIMIT $7`
43+
LIMIT $6`
4444
_selectAllDomainAuditLogsQuery = `SELECT
4545
event_id, domain_id, state_before, state_before_encoding, state_after, state_after_encoding,
4646
operation_type, created_time, last_updated_time, identity, identity_type, comment
4747
FROM domain_audit_log
48-
WHERE domain_id = $1 AND operation_type = $2 AND created_time >= $3 AND created_time < $4
49-
AND (created_time < $5 OR (created_time = $5 AND event_id > $6))
48+
WHERE domain_id = $1 AND operation_type = $2 AND created_time >= $3
49+
AND (created_time < $4 OR (created_time = $4 AND event_id > $5))
5050
ORDER BY created_time DESC, event_id ASC`
5151
)
5252

@@ -80,7 +80,6 @@ func (pdb *db) SelectFromDomainAuditLogs(
8080
filter.DomainID,
8181
filter.OperationType,
8282
*filter.MinCreatedTime,
83-
*filter.MaxCreatedTime,
8483
*filter.PageMaxCreatedTime,
8584
*filter.PageMinEventID,
8685
}

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,6 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
171171
DomainID: domainID,
172172
OperationType: operationType,
173173
MinCreatedTime: &minTime,
174-
MaxCreatedTime: &maxTime,
175174
PageSize: 2,
176175
PageMaxCreatedTime: &defaultPageMaxCreatedTime,
177176
PageMinEventID: &defaultPageMinEventID,
@@ -182,7 +181,7 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
182181
sqlplugin.DbDefaultShard,
183182
gomock.Any(),
184183
_selectDomainAuditLogsQuery,
185-
domainID, operationType, minTime, maxTime, defaultPageMaxCreatedTime, defaultPageMinEventID, 2,
184+
domainID, operationType, minTime, defaultPageMaxCreatedTime, defaultPageMinEventID, 2,
186185
).DoAndReturn(func(ctx context.Context, shardID int, dest interface{}, query string, args ...interface{}) error {
187186
rows := dest.(*[]*sqlplugin.DomainAuditLogRow)
188187
*rows = []*sqlplugin.DomainAuditLogRow{
@@ -224,7 +223,6 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
224223
DomainID: domainID,
225224
OperationType: operationType,
226225
MinCreatedTime: &minTime,
227-
MaxCreatedTime: &maxTime,
228226
PageSize: 0,
229227
PageMaxCreatedTime: &createdTime2,
230228
PageMinEventID: &eventID2,
@@ -235,7 +233,7 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
235233
sqlplugin.DbDefaultShard,
236234
gomock.Any(),
237235
_selectAllDomainAuditLogsQuery,
238-
domainID, operationType, minTime, maxTime, createdTime2, eventID2,
236+
domainID, operationType, minTime, createdTime2, eventID2,
239237
).DoAndReturn(func(ctx context.Context, shardID int, dest interface{}, query string, args ...interface{}) error {
240238
rows := dest.(*[]*sqlplugin.DomainAuditLogRow)
241239
*rows = []*sqlplugin.DomainAuditLogRow{
@@ -277,7 +275,6 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
277275
DomainID: domainID,
278276
OperationType: operationType,
279277
MinCreatedTime: &minTime,
280-
MaxCreatedTime: &maxTime,
281278
PageMaxCreatedTime: &defaultPageMaxCreatedTime,
282279
PageMinEventID: &defaultPageMinEventID,
283280
},
@@ -287,7 +284,7 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
287284
sqlplugin.DbDefaultShard,
288285
gomock.Any(),
289286
_selectAllDomainAuditLogsQuery,
290-
domainID, operationType, minTime, maxTime, defaultPageMaxCreatedTime, defaultPageMinEventID,
287+
domainID, operationType, minTime, defaultPageMaxCreatedTime, defaultPageMinEventID,
291288
).DoAndReturn(func(ctx context.Context, shardID int, dest interface{}, query string, args ...interface{}) error {
292289
return nil
293290
})
@@ -301,7 +298,6 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
301298
DomainID: domainID,
302299
OperationType: operationType,
303300
MinCreatedTime: &minTime,
304-
MaxCreatedTime: &maxTime,
305301
PageMaxCreatedTime: &defaultPageMaxCreatedTime,
306302
PageMinEventID: &defaultPageMinEventID,
307303
},
@@ -311,7 +307,7 @@ func TestSelectFromDomainAuditLogs(t *testing.T) {
311307
sqlplugin.DbDefaultShard,
312308
gomock.Any(),
313309
_selectAllDomainAuditLogsQuery,
314-
domainID, operationType, minTime, maxTime, defaultPageMaxCreatedTime, defaultPageMinEventID,
310+
domainID, operationType, minTime, defaultPageMaxCreatedTime, defaultPageMinEventID,
315311
).Return(errors.New("select failed"))
316312
},
317313
wantRows: nil,

0 commit comments

Comments
 (0)