HDDS-16097. Add unit tests for OzoneManagerRatisServer.checkRetryCache

[jojochuang]

What changes were proposed in this pull request?

This PR adds unit test coverage for OzoneManagerRatisServer.checkRetryCache().

HDDS-13621 (#9711) fixed an NPE when a cached Ratis reply was unsuccessful: the server now returns null instead of calling getOMResponse() on a failed reply. This change adds targeted unit tests to lock in that behavior and to verify the success path still returns an OMResponse.

Changes:

• checkRetryCacheReturnsNullWhenCachedReplyFailed — mocks a failed cached RaftClientReply and asserts checkRetryCache() returns null.
• checkRetryCacheReturnsOmResponseWhenCachedReplySucceeded — mocks a successful cached reply and asserts an OMResponse with Status.OK is returned.

Related: HDDS-13621, #9711.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16097

How was this patch tested?

• Unit tests in TestOzoneManagerRatisServer (two new test methods).
• Fork CI (flaky-test-check on commit b3c339a): https://github.com/jojochuang/ozone/actions/runs/31129294251 — TestOzoneManagerRatisServer#ALL passed.

Generated-by: Cursor (Composer)

Made with Cursor

[Copilot]

Pull request overview

This PR adds JUnit coverage in TestOzoneManagerRatisServer to lock in the HDDS-13621 behavior of OzoneManagerRatisServer.checkRetryCache() returning null when a cached RaftClientReply is unsuccessful, while still returning an OMResponse on successful cached replies.

Changes:

• Adds a unit test asserting checkRetryCache() returns null when the cached RaftClientReply indicates failure.
• Adds a unit test asserting checkRetryCache() returns a cached OMResponse when the cached RaftClientReply indicates success.
• Introduces a helper to inject a mocked RetryCache entry into a spied OzoneManagerRatisServer.

Suppressed comments (3)

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java:209

• To make this test exercise the same clientId mapping used by OzoneManagerRatisServer.getClientId() (UUID.nameUUIDFromBytes(Server.getClientId())), build ratisClientId from the clientIdBytes you place into Server.Call, rather than using ClientId.randomId() and then serializing it into Server.Call.

    ClientId ratisClientId = ClientId.randomId();
    int callId = 43;
    OMResponse expected = OMResponse.newBuilder()

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java:229

• The RetryCache in injectRetryCacheEntry() is currently stubbed with a broad matcher, so these tests don’t verify that checkRetryCache() actually queries the cache using the expected ClientInvocationId (clientId + callId). Consider capturing the RetryCache mock so the test can verify the exact key used.

    OzoneManagerRatisServer spyServer = spy(omRatisServer);
    injectRetryCacheEntry(spyServer, ratisClientId, callId, successReply);
    doReturn(omRatisServer.getRaftPeerId()).when(spyServer).getLeaderId();

    Server.Call previousCall = Server.getCurCall().get();

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java:246

• injectRetryCacheEntry() currently hides the RetryCache mock inside the helper, which makes it hard for callers to verify that the correct key was used. Returning the RetryCache allows each test to assert the cache lookup was performed with the expected ClientInvocationId without loosening the stubbing.

  private static void injectRetryCacheEntry(OzoneManagerRatisServer spyServer,
      ClientId clientId, int callId, RaftClientReply reply) throws Exception {
    RetryCache.Entry cacheEntry = mock(RetryCache.Entry.class);
    when(cacheEntry.getReplyFuture())
        .thenReturn(CompletableFuture.completedFuture(reply));

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.