Skip to content

Commit 9fe34d0

Browse files
elnafatehelnafateh
committed
FINERACT-1420: Improve idempotency fallback using deterministic key generation
FINERACT-1420: Idempotency edge case and retry impl FINERACT-1420: Idempotency test and edge cases improvement
1 parent 83fcc5c commit 9fe34d0

File tree

7 files changed

+510
-17
lines changed

7 files changed

+510
-17
lines changed

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

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
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 com.fasterxml.jackson.databind.JsonNode;
22+
import com.fasterxml.jackson.databind.ObjectMapper;
23+
import com.fasterxml.jackson.databind.node.ArrayNode;
24+
import com.fasterxml.jackson.databind.node.ObjectNode;
25+
import java.nio.charset.StandardCharsets;
26+
import java.security.MessageDigest;
27+
import java.time.Instant;
28+
import java.util.ArrayList;
29+
import java.util.Base64;
30+
import java.util.Collections;
31+
import java.util.List;
32+
import lombok.RequiredArgsConstructor;
33+
import org.springframework.stereotype.Component;
34+
35+
@Component
36+
@RequiredArgsConstructor
37+
public class DeterministicIdempotencyKeyGenerator {
38+
39+
private final ObjectMapper objectMapper;
40+
41+
public String generate(String json, String context) {
42+
43+
if (json == null || json.isBlank()) {
44+
// Shouldn't reach here after resolver guard, but defensive fallback
45+
return java.util.UUID.randomUUID().toString();
46+
}
47+
48+
String canonical = toCanonicalString(json);
49+
String window = currentTimeWindow();
50+
return hash(canonical + ":" + context + ":" + window);
51+
}
52+
53+
private String toCanonicalString(String json) {
54+
try {
55+
JsonNode node = objectMapper.readTree(json);
56+
JsonNode canonical = canonicalize(node);
57+
return objectMapper.writeValueAsString(canonical);
58+
} catch (Exception e) {
59+
throw new RuntimeException("Failed to canonicalize JSON", e);
60+
}
61+
}
62+
63+
private JsonNode canonicalize(JsonNode node) {
64+
if (node.isObject()) {
65+
ObjectNode sorted = objectMapper.createObjectNode();
66+
67+
List<String> fieldNames = new ArrayList<>();
68+
node.fieldNames().forEachRemaining(fieldNames::add);
69+
Collections.sort(fieldNames);
70+
71+
for (String field : fieldNames) {
72+
sorted.set(field, canonicalize(node.get(field))); // recursion to resolve nested obj
73+
}
74+
75+
return sorted;
76+
}
77+
78+
if (node.isArray()) {
79+
ArrayNode arrayNode = objectMapper.createArrayNode();
80+
for (JsonNode element : node) {
81+
arrayNode.add(canonicalize(element)); // recursion inside array
82+
}
83+
return arrayNode;
84+
}
85+
86+
return node; // primitives + null
87+
}
88+
89+
private String hash(String input) {
90+
try {
91+
MessageDigest digest = MessageDigest.getInstance("SHA-256");
92+
byte[] hashed = digest.digest(input.getBytes(StandardCharsets.UTF_8));
93+
return Base64.getEncoder().encodeToString(hashed);
94+
} catch (Exception e) {
95+
throw new RuntimeException("Hashing failed", e);
96+
}
97+
}
98+
99+
private String currentTimeWindow() {
100+
Instant now = Instant.now();
101+
long window = now.getEpochSecond() / (5 * 60);
102+
return String.valueOf(window);
103+
}
104+
}

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

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

3131
private final FineractRequestContextHolder fineractRequestContextHolder;
3232

33-
private final IdempotencyKeyGenerator idempotencyKeyGenerator;
33+
private final IdempotencyKeyGenerator randomKeyGenerator;
34+
35+
private final DeterministicIdempotencyKeyGenerator deterministicGenerator;
36+
37+
public record ResolvedKey(String key, boolean isDeterministic) {
38+
}
39+
40+
public ResolvedKey resolveWithMeta(CommandWrapper wrapper) {
41+
// 1. Explicit key from wrapper (client-provided header)
42+
if (wrapper.getIdempotencyKey() != null) {
43+
return new ResolvedKey(wrapper.getIdempotencyKey(), false);
44+
}
45+
// 2. Internal retry — key already stored in request context
46+
Optional<String> attributeKey = getAttribute();
47+
if (attributeKey.isPresent()) {
48+
return new ResolvedKey(attributeKey.get(), false);
49+
}
50+
// 3. No JSON body — cannot hash, use random key
51+
if (wrapper.getJson() == null || wrapper.getJson().isBlank()) {
52+
return new ResolvedKey(randomKeyGenerator.create(), false);
53+
}
54+
// 4. No clientId and no entityId — system-level operation (e.g. global
55+
// config update, business date change). These have no per-caller scope
56+
// so the same payload from different scenarios within the same 5-minute
57+
// window would collide. Fall back to random key to avoid false cache hits.
58+
if (wrapper.getClientId() == null && wrapper.getEntityId() == null && wrapper.getJobName() == null) {
59+
return new ResolvedKey(randomKeyGenerator.create(), false);
60+
}
61+
// 5. Global configuration updates — same configId + same payload (e.g.
62+
// enabled=true) collides across scenarios within the same 5-minute window
63+
// since entityId is the configId not a client-scoped resource.
64+
String href = wrapper.getHref() != null ? wrapper.getHref() : "";
65+
if (href.startsWith("/configurations/")) {
66+
return new ResolvedKey(randomKeyGenerator.create(), false);
67+
}
68+
69+
// 6. Client/entity-scoped operation — generate deterministic key
70+
String deterministicKey = deterministicGenerator.generate(wrapper.getJson(), buildContext(wrapper));
71+
fineractRequestContextHolder.setAttribute(SynchronousCommandProcessingService.IDEMPOTENCY_KEY_ATTRIBUTE, deterministicKey);
72+
return new ResolvedKey(deterministicKey, true);
73+
}
3474

3575
public String resolve(CommandWrapper wrapper) {
36-
return Optional.ofNullable(wrapper.getIdempotencyKey()).orElseGet(() -> getAttribute().orElseGet(idempotencyKeyGenerator::create));
76+
return resolveWithMeta(wrapper).key();
77+
}
78+
79+
private String buildContext(CommandWrapper wrapper) {
80+
return wrapper.getActionName() + ":" + wrapper.getEntityName() + ":" + wrapper.getHref() + ":" + wrapper.getClientId() + ":"
81+
+ wrapper.getJobName();
3782
}
3883

3984
private Optional<String> getAttribute() {

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,20 @@ public CommandProcessingResult executeCommand(final CommandWrapper wrapper, fina
109109

110110
CommandSource commandSource = null;
111111
String idempotencyKey;
112+
boolean isDeterministicKey = false;
112113
if (isRetry) {
113114
commandSource = commandSourceService.getCommandSource(commandId);
114115
idempotencyKey = commandSource.getIdempotencyKey();
115116
} else if ((commandId = command.commandId()) != null) { // action on the command itself
116117
commandSource = commandSourceService.getCommandSource(commandId);
117118
idempotencyKey = commandSource.getIdempotencyKey();
118119
} else {
119-
idempotencyKey = idempotencyKeyResolver.resolve(wrapper);
120+
IdempotencyKeyResolver.ResolvedKey resolved = idempotencyKeyResolver.resolveWithMeta(wrapper);
121+
idempotencyKey = resolved.key();
122+
isDeterministicKey = resolved.isDeterministic();
123+
// idempotencyKey = idempotencyKeyResolver.resolve(wrapper);
120124
}
121-
exceptionWhenTheRequestAlreadyProcessed(wrapper, idempotencyKey, isRetry);
125+
exceptionWhenTheRequestAlreadyProcessed(wrapper, idempotencyKey, isRetry, isDeterministicKey);
122126

123127
AppUser user = context.authenticatedUser(wrapper);
124128
if (commandSource == null) {
@@ -218,7 +222,8 @@ private void publishHookErrorEvent(CommandWrapper wrapper, JsonCommand command,
218222
}
219223
}
220224

221-
private void exceptionWhenTheRequestAlreadyProcessed(CommandWrapper wrapper, String idempotencyKey, boolean retry) {
225+
private void exceptionWhenTheRequestAlreadyProcessed(CommandWrapper wrapper, String idempotencyKey, boolean retry,
226+
boolean isDeterministicKey) {
222227
CommandSource command = commandSourceService.findCommandSource(wrapper, idempotencyKey);
223228
if (command == null) {
224229
return;
@@ -234,7 +239,7 @@ private void exceptionWhenTheRequestAlreadyProcessed(CommandWrapper wrapper, Str
234239
}
235240
case PROCESSED -> throw new IdempotentCommandProcessSucceedException(wrapper, idempotencyKey, command);
236241
case ERROR -> {
237-
if (!retry) {
242+
if (!retry && !isDeterministicKey) {
238243
throw new IdempotentCommandProcessFailedException(wrapper, idempotencyKey, command);
239244
}
240245
}

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
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 org.junit.jupiter.api.Test;
27+
28+
class DeterministicIdempotencyKeyGeneratorTest {
29+
30+
private final DeterministicIdempotencyKeyGenerator underTest = new DeterministicIdempotencyKeyGenerator(new ObjectMapper());
31+
32+
@Test
33+
void shouldGenerateSameKeyForSameInputAndContext() {
34+
String json = "{\"b\":2,\"a\":1}";
35+
String context = "action:entity:/endpoint:client1";
36+
37+
String key1 = underTest.generate(json, context);
38+
String key2 = underTest.generate("{\"a\":1,\"b\":2}", context);
39+
40+
assertEquals(key1, key2);
41+
}
42+
43+
@Test
44+
void shouldGenerateDifferentKeysForDifferentContext() {
45+
String json = "{\"a\":1}";
46+
47+
String key1 = underTest.generate(json, "context1");
48+
String key2 = underTest.generate(json, "context2");
49+
50+
assertNotEquals(key1, key2);
51+
}
52+
53+
@Test
54+
void shouldGenerateDifferentKeysForDifferentPayload() {
55+
String context = "same-context";
56+
57+
String key1 = underTest.generate("{\"a\":1}", context);
58+
String key2 = underTest.generate("{\"a\":2}", context);
59+
60+
assertNotEquals(key1, key2);
61+
}
62+
63+
@Test
64+
void shouldGenerateSameKeyWithinSameTimeWindow() {
65+
String json = "{\"a\":1}";
66+
String context = "ctx";
67+
68+
String key1 = underTest.generate(json, context);
69+
String key2 = underTest.generate(json, context);
70+
71+
assertEquals(key1, key2);
72+
}
73+
74+
@Test
75+
void shouldFailForInvalidJson() {
76+
RuntimeException exception = assertThrows(RuntimeException.class, () -> underTest.generate("{invalid-json", "test-context"));
77+
assertEquals("Failed to canonicalize JSON", exception.getMessage());
78+
}
79+
}

0 commit comments

Comments
 (0)