fix: make mock request record lookup deterministic(#6521)

[juicewcode]

Fixes #6521

Summary

• Add deterministic ordering to mock request record queries.
• Select the most recently updated record first with date_updated DESC.
• Use id DESC as a stable tie breaker when records have the same update time.
• Add unit test for duplicate api_id records and queryByApiId(...).

Make sure that:

• You have read the contribution guidelines.
• You submit test cases (unit or integration tests) that back your changes.
• Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

[Aias00]

Summary

Small, targeted, and correctly implemented fix for #6521.

• selectByQuery in mock-request-record-sqlmap.xml now orders by date_updated DESC, id DESC, making the mock request record lookup deterministic.
• The new testSelectByQueryOrdersByDateUpdatedAndId covers both the ordering (three records with distinct / identical timestamps) and the service-level queryByApiId returning the newest record.

The change is minimal and the test exercises the real service path (MockRequestRecordService.queryByApiId), so it is not a self-fulfilling fixture.

Non-blocking

• id DESC is lexicographic on the string id; for real UUID ids this is still deterministic (which is the goal), but if a future id scheme mixes lengths the tie-break could look arbitrary. Not a problem today.

CI

All unit/build jobs pass; only e2e / e2e-storage (compose infrastructure) fail, unrelated to this change.

Approving.

[Aias00]

Review — APPROVE

Minimal, well-scoped fix for #6521 that takes the "add a deterministic ordering and selection rule" option the issue explicitly offers. I verified the root cause, the load-bearing value of the new test, the ordering semantics, and the pagination interaction. No blocking issues found.

✅ Root cause confirmed

MockRequestRecordServiceImpl.queryByApiId (MockRequestRecordServiceImpl.java:135-138) builds a query with only apiId set and takes mockRequestRecordDoList.get(0):

List<MockRequestRecordDO> mockRequestRecordDoList = mockRequestRecordMapper.selectByQuery(mockRequestRecordQuery);
return mockRequestRecordDoList.isEmpty() ? new MockRequestRecordVO()
        : MockRequestRecordVO.buildMockRequestRecordVO(mockRequestRecordDoList.get(0));

mock_request_record declares only PRIMARY KEY (id) (sql-script/h2/schema.sql:134-147) with no uniqueness on api_id, so multiple rows per api_id are legal and the "first" row was plan-dependent. Reaching the user via GET /mock-request-record/{apiId} (MockRequestRecordController.java:113-116).

✅ The regression test is genuinely load-bearing (empirically verified)

I reproduced the exact fixture against the project's own H2 (h2.version = 1.4.200, jdbc:h2:mem:...;MODE=MySQL), using the verbatim DDL from schema.sql:134-147 plus idx_mock_record_api (:1588):

=== master (no ORDER BY) ===
  [1] id=mock-order-older date_updated=2024-01-01
  [2] id=mock-order-z     date_updated=2024-01-02
  [3] id=mock-order-y     date_updated=2024-01-02
  -> first=mock-order-older ; assertion records.get(0)=="mock-order-z" : FAIL

=== PR #7666 (ORDER BY date_updated DESC, id DESC) ===
  [1] id=mock-order-z     date_updated=2024-01-02
  [2] id=mock-order-y     date_updated=2024-01-02
  [3] id=mock-order-older date_updated=2024-01-01
  -> first=mock-order-z ; assertion records.get(0)=="mock-order-z" : PASS

Note that master returns rows in primary-key order, not insertion order (mock-order-y sorts after mock-order-z despite being inserted later), so both the get(0) and get(1) assertions detect the bug. This is a real regression test, not a tautology.

✅ Ordering semantics are correct and total

• date_updated DESC = most recently updated first, matching the "latest recorded request" intent of GET /mock-request-record/{apiId}.
• id DESC is a total tiebreaker because id is the primary key (unique, NOT NULL). A total order means the result is fully deterministic — no residual plan dependence. Verified against the fixture: mock-order-z and mock-order-y share date_updated, and 'z' > 'y' puts z first, exactly as asserted.
• H2 in MySQL mode preserves the timestamp value exactly, so the 1-day gap between older and newer is unambiguous.

✅ Blast radius fully mapped

selectByQuery has exactly two call sites, both in MockRequestRecordServiceImpl:

Call site	Consumer	Effect
:86 listByPage (@Pageable)	GET /mock-request-record/findPageByQuery	now deterministic + stable paging
:135 queryByApiId (.get(0))	GET /mock-request-record/{apiId}	the issue's target — now deterministic

The sibling unordered statement queryAll in the same mapper has no production consumer (only MockRequestRecordMapperTest.queryAll), so leaving it unordered introduces no user-facing nondeterminism. I also confirmed there is only one mock-request-record-sqlmap.xml with no databaseId variants, so no per-dialect divergence.

✅ PageHelper interaction is safe

@Pageable → PageMethod.startPage (PageableAspect.java:63) rewrites the executed statement by appending LIMIT/OFFSET after the existing ORDER BY, yielding ... ORDER BY date_updated DESC, id DESC LIMIT ?, ?. Semantics are correct, and paged results become stable across page boundaries (previously a row could migrate between pages between requests). The count query is generated by PageHelper's own order-by handling, which is orthogonal.

✅ Test isolation is sound

@Transactional on the new method guarantees rollback, and apiId = "mock-order-api" is disjoint from the apiId = "1" fixture produced by buildMockRequestRecordDO() used by every other test in the class — including the two non-transactional tests (testSelectByQuery, testBatchDelete) that leak rows into the shared in-memory DB. assertEquals(3, records.size()) therefore cannot be perturbed by test ordering.

⚠️ CI attribution

Only two real failures; the rest are fail-fast artifacts:

Job	conclusion	Cause
e2e-storage (mysql)	failure	DividePluginTest.testDivide:105 endpoint '/anything/xx/yy' not exists — external httpbin.org flake, same as #7668/#7672
e2e-case (logging-rocketmq)	failure	DividePluginTest.testDivide:146 Failed to consume RocketMQ access log — RocketMQ log-consumption flake
e2e-storage (h2 / opengauss / postgres)	cancelled	fail-fast siblings; gh pr checks renders these as "fail"
e2e-case (apache-dubbo)	cancelled	fail-fast sibling
e2e	failure (3s)	fail-fast aggregator job

Master is flaky on the same workflow: run 38836735684 has postgres + cluster-jdbc failures, followed by three consecutive successes. This PR touches only an admin mapper XML and one admin test, so it cannot influence the divide-plugin or RocketMQ e2e paths.

📝 Non-blocking observations

1. The tie-breaker is stable but semantically arbitrary. MockRequestRecordServiceImpl.create() (:114-127) sets both dateCreated and dateUpdated to new Timestamp(System.currentTimeMillis()). Two records for the same api_id sharing a timestamp will tie, and the winner is decided by a random short UUID (UUIDUtils.generateShortUuid()). So "most recent" is guaranteed only at timestamp granularity. Since date_created carries no extra signal (it equals date_updated on create), id DESC is the only available total tiebreaker — the approach is the best the data model allows, but it is worth stating this limitation in the PR description.

2. queryByApiId still materialises the whole result set. selectByQuery has no LIMIT and the service does .get(0), so a hot api_id reads every row just to pick one. Pre-existing and unchanged by this PR (the ORDER BY does not make it worse), but a LIMIT 1 path or a dedicated mapper method would be the natural follow-up.

3. No index on (api_id, date_updated) — only idx_mock_record_api (api_id) (schema.sql:1588), so the ORDER BY adds a sort. Acceptable at admin mock-record volumes, and PageHelper's count query already scans all matching rows, so the asymptotic cost is unchanged. Noted for completeness only.

Verdict

APPROVE. Correct, minimal, and backed by a test I verified actually fails without the fix. The three notes above are optional polish, not blockers.

[yuluo-yx]

/retest