Skip to content

Commit 01a8e39

Browse files
elnafatehelnafateh
committed
Improve idempotency handling with deterministic key generation
Introduce deterministic idempotency key generation when a client-provided key is absent. The key is derived from canonical request payload with a time window to support safe retries. Enhancements: - deterministic retry behavior without client-provided keys - proper JSON canonicalization (ordering, nested objects, null handling) This change extends the existing idempotency mechanism without replacing it.
1 parent 2ee6f22 commit 01a8e39

File tree

6 files changed

+320
-9
lines changed

6 files changed

+320
-9
lines changed

‎fineract-core/src/main/java/org/apache/fineract/commands/service/DeterministicIdempotencyKeyGenerator.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package org.apache.fineract.commands.service;
2+
3+
4+
import com.fasterxml.jackson.databind.JsonNode;
5+
import com.fasterxml.jackson.databind.ObjectMapper;
6+
import com.fasterxml.jackson.databind.node.ArrayNode;
7+
import com.fasterxml.jackson.databind.node.ObjectNode;
8+
import lombok.RequiredArgsConstructor;
9+
import org.apache.fineract.commands.domain.CommandWrapper;
10+
import org.springframework.stereotype.Component;
11+
12+
import java.nio.charset.StandardCharsets;
13+
import java.security.MessageDigest;
14+
import java.time.Instant;
15+
import java.util.ArrayList;
16+
import java.util.Base64;
17+
import java.util.Collections;
18+
import java.util.List;
19+
20+
@Component
21+
@RequiredArgsConstructor
22+
public class DeterministicIdempotencyKeyGenerator {
23+
24+
private final ObjectMapper objectMapper;
25+
26+
public String generate(String json) {
27+
String canonical = toCanonicalString(json);
28+
String window = currentTimeWindow();
29+
return hash(canonical + ":" + window);
30+
}
31+
32+
private String toCanonicalString(String json) {
33+
try {
34+
JsonNode node = objectMapper.readTree(json);
35+
JsonNode canonical = canonicalize(node);
36+
return objectMapper.writeValueAsString(canonical);
37+
} catch (Exception e) {
38+
throw new RuntimeException("Failed to canonicalize JSON", e);
39+
}
40+
}
41+
42+
private JsonNode canonicalize(JsonNode node) {
43+
if (node.isObject()) {
44+
ObjectNode sorted = objectMapper.createObjectNode();
45+
46+
List<String> fieldNames = new ArrayList<>();
47+
node.fieldNames().forEachRemaining(fieldNames::add);
48+
Collections.sort(fieldNames);
49+
50+
for (String field : fieldNames) {
51+
sorted.set(field, canonicalize(node.get(field))); // recursion to resolve nested obj
52+
}
53+
54+
return sorted;
55+
}
56+
57+
if (node.isArray()) {
58+
ArrayNode arrayNode = objectMapper.createArrayNode();
59+
for (JsonNode element : node) {
60+
arrayNode.add(canonicalize(element)); // recursion inside array
61+
}
62+
return arrayNode;
63+
}
64+
65+
return node; // primitives + null
66+
}
67+
68+
private String hash(String input) {
69+
try {
70+
MessageDigest digest = MessageDigest.getInstance("SHA-256");
71+
byte[] hashed = digest.digest(input.getBytes(StandardCharsets.UTF_8));
72+
return Base64.getEncoder().encodeToString(hashed);
73+
} catch (Exception e) {
74+
throw new RuntimeException("Hashing failed", e);
75+
}
76+
}
77+
78+
private String currentTimeWindow() {
79+
Instant now = Instant.now();
80+
long window = now.getEpochSecond() / (5 * 60);
81+
return String.valueOf(window);
82+
}
83+
}

‎fineract-core/src/main/java/org/apache/fineract/commands/service/IdempotencyKeyResolver.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,23 @@ public class IdempotencyKeyResolver {
3030

3131
private final FineractRequestContextHolder fineractRequestContextHolder;
3232

33-
private final IdempotencyKeyGenerator idempotencyKeyGenerator;
33+
private final DeterministicIdempotencyKeyGenerator deterministicGenerator;
3434

3535
public String resolve(CommandWrapper wrapper) {
36-
return Optional.ofNullable(wrapper.getIdempotencyKey()).orElseGet(() -> getAttribute().orElseGet(idempotencyKeyGenerator::create));
36+
37+
// If wrapper already has key → use it
38+
if (wrapper.getIdempotencyKey() != null) {
39+
return wrapper.getIdempotencyKey();
40+
}
41+
42+
// If request attribute exists (internal retry)
43+
Optional<String> attributeKey = getAttribute();
44+
if (attributeKey.isPresent()) {
45+
return attributeKey.get();
46+
}
47+
48+
// hybrid logic (external retry)
49+
return deterministicGenerator.generate(wrapper.getJson());
3750
}
3851

3952
private Optional<String> getAttribute() {

‎fineract-core/src/test/java/org/apache/fineract/commands/service/DeterministicIdempotencyKeyGeneratorTest.java‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.fineract.commands.service;
20+
21+
import static org.junit.jupiter.api.Assertions.assertEquals;
22+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
23+
import static org.junit.jupiter.api.Assertions.assertThrows;
24+
25+
import com.fasterxml.jackson.databind.ObjectMapper;
26+
import java.time.Instant;
27+
import org.junit.jupiter.api.Test;
28+
import org.mockito.MockedStatic;
29+
import org.mockito.Mockito;
30+
31+
class DeterministicIdempotencyKeyGeneratorTest {
32+
33+
private final DeterministicIdempotencyKeyGenerator underTest = new DeterministicIdempotencyKeyGenerator(new ObjectMapper());
34+
35+
@Test
36+
void shouldGenerateSameKeyForSemanticallyEquivalentJsonWithinSameWindow() {
37+
String firstJson = "{\"businessSteps\":[{\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\",\"order\":1}]}";
38+
String secondJson = "{\"businessSteps\":[{\"order\":1,\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\"}]}";
39+
Instant firstWindowInstant = Instant.ofEpochSecond(60);
40+
Instant secondWindowInstant = Instant.ofEpochSecond(299);
41+
42+
try (MockedStatic<Instant> instant = Mockito.mockStatic(Instant.class, Mockito.CALLS_REAL_METHODS)) {
43+
instant.when(Instant::now).thenReturn(firstWindowInstant, secondWindowInstant);
44+
45+
String firstKey = underTest.generate(firstJson);
46+
String secondKey = underTest.generate(secondJson);
47+
48+
assertEquals(firstKey, secondKey);
49+
}
50+
}
51+
52+
@Test
53+
void shouldGenerateDifferentKeysForDifferentPayloadsWithinSameWindow() {
54+
String firstJson = "{\"businessSteps\":[{\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\",\"order\":1}]}";
55+
String secondJson = "{\"businessSteps\":[{\"stepName\":\"LOAN_DELINQUENCY_CLASSIFICATION\",\"order\":1}]}";
56+
Instant currentInstant = Instant.ofEpochSecond(60);
57+
58+
try (MockedStatic<Instant> instant = Mockito.mockStatic(Instant.class, Mockito.CALLS_REAL_METHODS)) {
59+
instant.when(Instant::now).thenReturn(currentInstant, currentInstant);
60+
61+
String firstKey = underTest.generate(firstJson);
62+
String secondKey = underTest.generate(secondJson);
63+
64+
assertNotEquals(firstKey, secondKey);
65+
}
66+
}
67+
68+
@Test
69+
void shouldPreserveArrayOrderingInGeneratedKey() {
70+
String firstJson = "{\"businessSteps\":[{\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\",\"order\":1},{\"stepName\":\"LOAN_DELINQUENCY_CLASSIFICATION\",\"order\":2}]}";
71+
String secondJson = "{\"businessSteps\":[{\"stepName\":\"LOAN_DELINQUENCY_CLASSIFICATION\",\"order\":2},{\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\",\"order\":1}]}";
72+
Instant currentInstant = Instant.ofEpochSecond(60);
73+
74+
try (MockedStatic<Instant> instant = Mockito.mockStatic(Instant.class, Mockito.CALLS_REAL_METHODS)) {
75+
instant.when(Instant::now).thenReturn(currentInstant, currentInstant);
76+
77+
String firstKey = underTest.generate(firstJson);
78+
String secondKey = underTest.generate(secondJson);
79+
80+
assertNotEquals(firstKey, secondKey);
81+
}
82+
}
83+
84+
@Test
85+
void shouldGenerateDifferentKeysAcrossFiveMinuteWindows() {
86+
String json = "{\"businessSteps\":[{\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\",\"order\":1}]}";
87+
Instant firstWindowInstant = Instant.ofEpochSecond(299);
88+
Instant secondWindowInstant = Instant.ofEpochSecond(300);
89+
90+
try (MockedStatic<Instant> instant = Mockito.mockStatic(Instant.class, Mockito.CALLS_REAL_METHODS)) {
91+
instant.when(Instant::now).thenReturn(firstWindowInstant, secondWindowInstant);
92+
93+
String firstKey = underTest.generate(json);
94+
String secondKey = underTest.generate(json);
95+
96+
assertNotEquals(firstKey, secondKey);
97+
}
98+
}
99+
100+
@Test
101+
void shouldFailForInvalidJson() {
102+
RuntimeException exception = assertThrows(RuntimeException.class, () -> underTest.generate("{invalid-json"));
103+
assertEquals("Failed to canonicalize JSON", exception.getMessage());
104+
}
105+
}

‎fineract-provider/src/test/java/org/apache/fineract/commands/service/IdempotencyKeyResolverTest.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
*/
1919
package org.apache.fineract.commands.service;
2020

21+
import static org.mockito.Mockito.verify;
22+
import static org.mockito.Mockito.verifyNoInteractions;
2123
import static org.mockito.Mockito.when;
2224

2325
import java.util.HashMap;
@@ -36,7 +38,7 @@
3638
public class IdempotencyKeyResolverTest {
3739

3840
@Mock
39-
private IdempotencyKeyGenerator idempotencyKeyGenerator;
41+
private DeterministicIdempotencyKeyGenerator deterministicIdempotencyKeyGenerator;
4042

4143
@InjectMocks
4244
private IdempotencyKeyResolver underTest;
@@ -62,15 +64,19 @@ public void testIPKResolveFromRequest() {
6264
CommandWrapper wrapper = CommandWrapper.wrap("act", "ent", 1L, 1L);
6365
String resolvedIdk = underTest.resolve(wrapper);
6466
Assertions.assertEquals(idk, resolvedIdk);
67+
verifyNoInteractions(deterministicIdempotencyKeyGenerator);
6568
}
6669

6770
@Test
6871
public void testIPKResolveFromGenerate() {
6972
String idk = "idk";
70-
when(idempotencyKeyGenerator.create()).thenReturn(idk);
71-
CommandWrapper wrapper = CommandWrapper.wrap("act", "ent", 1L, 1L);
73+
String json = "{\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\",\"order\":1}";
74+
when(deterministicIdempotencyKeyGenerator.generate(json)).thenReturn(idk);
75+
CommandWrapper wrapper = new CommandWrapper(null, null, null, null, null, "UPDATE", "JOB", 1L, null, "/jobs/LOAN_CLOSE_OF_BUSINESS",
76+
json, null, null, null, null, null, null, null, null, null);
7277
String resolvedIdk = underTest.resolve(wrapper);
7378
Assertions.assertEquals(idk, resolvedIdk);
79+
verify(deterministicIdempotencyKeyGenerator).generate(json);
7480
}
7581

7682
@Test
@@ -80,5 +86,6 @@ public void testIPKResolveFromWrapper() {
8086
null, null, null, idk, null, null);
8187
String resolvedIdk = underTest.resolve(wrapper);
8288
Assertions.assertEquals(idk, resolvedIdk);
89+
verifyNoInteractions(deterministicIdempotencyKeyGenerator);
8390
}
8491
}

‎integration-tests/src/test/java/org/apache/fineract/integrationtests/IdempotencyTest.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,74 @@ public void shouldTheSecondRequestWithSameIdempotencyKeyWillFailureToo() {
154154
assertEquals((Map) body1.jsonPath().get(""), response2.getBody().jsonPath().get(""));
155155
}
156156

157+
@Test
158+
public void shouldReuseDeterministicIdempotencyKeyWhenHeaderMissing() {
159+
ResponseSpecification updateResponseSpec = new ResponseSpecBuilder().expectStatusCode(204).build();
160+
JobBusinessStepConfigData originalStepConfig = IdempotencyHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec,
161+
LOAN_JOB_NAME);
162+
163+
try {
164+
String requestBody = "{\"businessSteps\":[{\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\",\"order\":1}]}";
165+
Response response = IdempotencyHelper.updateBusinessStepOrderWithoutIdempotencyKey(requestSpec, updateResponseSpec,
166+
LOAN_JOB_NAME, requestBody);
167+
Response responseSecond = IdempotencyHelper.updateBusinessStepOrderWithoutIdempotencyKey(requestSpec, updateResponseSpec,
168+
LOAN_JOB_NAME, requestBody);
169+
170+
assertEquals(response.getBody().asString(), responseSecond.getBody().asString());
171+
assertNull(response.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER));
172+
assertNotNull(responseSecond.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER));
173+
} finally {
174+
restoreOriginalStepConfig(updateResponseSpec, originalStepConfig);
175+
}
176+
}
177+
178+
@Test
179+
public void shouldReuseDeterministicIdempotencyKeyForReorderedJsonWhenHeaderMissing() {
180+
ResponseSpecification updateResponseSpec = new ResponseSpecBuilder().expectStatusCode(204).build();
181+
JobBusinessStepConfigData originalStepConfig = IdempotencyHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec,
182+
LOAN_JOB_NAME);
183+
184+
try {
185+
String firstRequestBody = "{\"businessSteps\":[{\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\",\"order\":1},"
186+
+ "{\"stepName\":\"LOAN_DELINQUENCY_CLASSIFICATION\",\"order\":2}]}";
187+
String secondRequestBody = "{\"businessSteps\":[{\"order\":1,\"stepName\":\"APPLY_CHARGE_TO_OVERDUE_LOANS\"},"
188+
+ "{\"order\":2,\"stepName\":\"LOAN_DELINQUENCY_CLASSIFICATION\"}]}";
189+
190+
Response response = IdempotencyHelper.updateBusinessStepOrderWithoutIdempotencyKey(requestSpec, updateResponseSpec,
191+
LOAN_JOB_NAME, firstRequestBody);
192+
Response responseSecond = IdempotencyHelper.updateBusinessStepOrderWithoutIdempotencyKey(requestSpec, updateResponseSpec,
193+
LOAN_JOB_NAME, secondRequestBody);
194+
195+
assertEquals(response.getBody().asString(), responseSecond.getBody().asString());
196+
assertNull(response.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER));
197+
assertNotNull(responseSecond.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER));
198+
} finally {
199+
restoreOriginalStepConfig(updateResponseSpec, originalStepConfig);
200+
}
201+
}
202+
203+
@Test
204+
public void shouldCacheFailedRequestWhenHeaderMissing() {
205+
ResponseSpecification responseSpecForError = new ResponseSpecBuilder().expectStatusCode(400).build();
206+
List<BusinessStep> requestBody = new ArrayList<>();
207+
208+
Response response1 = IdempotencyHelper.updateBusinessStepOrderWithErrorWithoutIdempotencyKey(requestSpec, responseSpecForError,
209+
LOAN_JOB_NAME, IdempotencyHelper.toJsonString(requestBody));
210+
assertNull(response1.getHeader(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER));
211+
ResponseBody body1 = response1.getBody();
212+
assertNotNull(body1);
213+
214+
Response response2 = IdempotencyHelper.updateBusinessStepOrderWithErrorWithoutIdempotencyKey(requestSpec, responseSpecForError,
215+
LOAN_JOB_NAME, IdempotencyHelper.toJsonString(requestBody));
216+
assertNotNull(response2.getHeader(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER));
217+
assertEquals((Map) body1.jsonPath().get(""), response2.getBody().jsonPath().get(""));
218+
}
219+
220+
private void restoreOriginalStepConfig(ResponseSpecification updateResponseSpec, JobBusinessStepConfigData originalStepConfig) {
221+
IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME,
222+
IdempotencyHelper.toJsonString(originalStepConfig.getBusinessSteps()), UUID.randomUUID().toString());
223+
}
224+
157225
private BusinessStep getBusinessSteps(Long order, String stepName) {
158226
BusinessStep businessStep = new BusinessStep();
159227
businessStep.setStepName(stepName);

‎integration-tests/src/test/java/org/apache/fineract/integrationtests/common/IdempotencyHelper.java‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,26 @@ public static JobBusinessStepDetail getAvailableBusinessStepsByJobName(final Req
9494
@Deprecated(forRemoval = true)
9595
public static Response updateBusinessStepOrder(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
9696
String jobName, String jsonBodyToSend, String idempotencyKey) {
97+
return updateBusinessStepOrder(requestSpec, responseSpec, jobName, jsonBodyToSend, true, idempotencyKey);
98+
}
99+
100+
@Deprecated(forRemoval = true)
101+
public static Response updateBusinessStepOrderWithoutIdempotencyKey(final RequestSpecification requestSpec,
102+
final ResponseSpecification responseSpec, String jobName, String jsonBodyToSend) {
103+
return updateBusinessStepOrder(requestSpec, responseSpec, jobName, jsonBodyToSend, false, null);
104+
}
105+
106+
@Deprecated(forRemoval = true)
107+
private static Response updateBusinessStepOrder(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
108+
String jobName, String jsonBodyToSend, boolean useIdempotencyKey, String idempotencyKey) {
97109
Response response = Utils.performServerPutRaw(requestSpec, responseSpec,
98-
BUSINESS_STEPS_API_URL_START + jobName + BUSINESS_STEPS_API_URL_END,
99-
request -> request.header("Idempotency-Key", idempotencyKey).body(jsonBodyToSend));
110+
BUSINESS_STEPS_API_URL_START + jobName + BUSINESS_STEPS_API_URL_END, request -> {
111+
RequestSpecification mappedRequest = request.body(jsonBodyToSend);
112+
if (useIdempotencyKey) {
113+
mappedRequest = mappedRequest.header("Idempotency-Key", idempotencyKey);
114+
}
115+
return mappedRequest;
116+
});
100117
log.info("BusinessStepConfigurationHelper Response: {}", response.getBody().asString());
101118
return response;
102119
}
@@ -107,9 +124,27 @@ public static Response updateBusinessStepOrder(final RequestSpecification reques
107124
@Deprecated(forRemoval = true)
108125
public static Response updateBusinessStepOrderWithError(final RequestSpecification requestSpec,
109126
final ResponseSpecification responseSpec, String jobName, String jsonBodyToSend, String idempotencyKey) {
127+
return updateBusinessStepOrderWithError(requestSpec, responseSpec, jobName, jsonBodyToSend, true, idempotencyKey);
128+
}
129+
130+
@Deprecated(forRemoval = true)
131+
public static Response updateBusinessStepOrderWithErrorWithoutIdempotencyKey(final RequestSpecification requestSpec,
132+
final ResponseSpecification responseSpec, String jobName, String jsonBodyToSend) {
133+
return updateBusinessStepOrderWithError(requestSpec, responseSpec, jobName, jsonBodyToSend, false, null);
134+
}
135+
136+
@Deprecated(forRemoval = true)
137+
private static Response updateBusinessStepOrderWithError(final RequestSpecification requestSpec,
138+
final ResponseSpecification responseSpec, String jobName, String jsonBodyToSend, boolean useIdempotencyKey,
139+
String idempotencyKey) {
110140
String url = BUSINESS_STEPS_API_URL_START + jobName + BUSINESS_STEPS_API_URL_END;
111-
return Utils.performServerPutRaw(requestSpec, responseSpec, url,
112-
request -> request.header("Idempotency-Key", idempotencyKey).body(jsonBodyToSend));
141+
return Utils.performServerPutRaw(requestSpec, responseSpec, url, request -> {
142+
RequestSpecification mappedRequest = request.body(jsonBodyToSend);
143+
if (useIdempotencyKey) {
144+
mappedRequest = mappedRequest.header("Idempotency-Key", idempotencyKey);
145+
}
146+
return mappedRequest;
147+
});
113148
}
114149

115150
private static final class BusinessStepWrapper {

0 commit comments

Comments
 (0)