FINERACT-1420: Improve idempotency fallback using deterministic key generation

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

@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.commands.service;
+
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.Collections;
+import java.util.List;
+import org.springframework.stereotype.Component;
+
+@Component
+public class DeterministicIdempotencyKeyGenerator {
+
+    // Plain ObjectMapper for canonicalization — must NOT use the application ObjectMapper
+    // which has custom serializers/deserializers that cause failures on certain JSON payloads
+    private static final ObjectMapper CANONICAL_MAPPER;
+
+    static {
+        JsonFactory factory = new JsonFactory();
+        factory.enable(JsonParser.Feature.ALLOW_SINGLE_QUOTES);
+        CANONICAL_MAPPER = new ObjectMapper(factory);
+    }
+
+    public String generate(String json, String context) {
+
+        if (json == null || json.isBlank()) {
+            // Shouldn't reach here after resolver guard, but defensive fallback
+            return java.util.UUID.randomUUID().toString();
+        }
+
+        String canonical = toCanonicalString(json);
+        String window = currentTimeWindow();
+        return hash(canonical + ":" + context + ":" + window);
+    }
+
+    private String toCanonicalString(String json) {
+        try {
+            JsonNode node = CANONICAL_MAPPER.readTree(json);
+            JsonNode canonical = canonicalize(node);
+            return CANONICAL_MAPPER.writeValueAsString(canonical);
+        } catch (Exception e) {
+            throw new RuntimeException("Failed to canonicalize JSON", e);
+        }
+    }
+
+    private JsonNode canonicalize(JsonNode node) {
+        if (node.isObject()) {
+            ObjectNode sorted = CANONICAL_MAPPER.createObjectNode();
+
+            List<String> fieldNames = new ArrayList<>();
+            node.fieldNames().forEachRemaining(fieldNames::add);
+            Collections.sort(fieldNames);
+
+            for (String field : fieldNames) {
+                sorted.set(field, canonicalize(node.get(field))); // recursion to resolve nested obj
+            }
+
+            return sorted;
+        }
+
+        if (node.isArray()) {
+            ArrayNode arrayNode = CANONICAL_MAPPER.createArrayNode();
+            for (JsonNode element : node) {
+                arrayNode.add(canonicalize(element)); // recursion inside array
+            }
+            return arrayNode;
+        }
+
+        return node; // primitives + null
+    }
+
+    private String hash(String input) {
+        try {
+            MessageDigest digest = MessageDigest.getInstance("SHA-256");
+            byte[] hashed = digest.digest(input.getBytes(StandardCharsets.UTF_8));
+            return Base64.getEncoder().encodeToString(hashed);
+        } catch (Exception e) {
+            throw new RuntimeException("Hashing failed", e);
+        }
+    }
+
+    private String currentTimeWindow() {
+        Instant now = Instant.now();
+        long window = now.getEpochSecond() / (5 * 60);
+        return String.valueOf(window);
+    }
+}

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

@@ -30,10 +30,73 @@ public class IdempotencyKeyResolver {
 
     private final FineractRequestContextHolder fineractRequestContextHolder;
 
-    private final IdempotencyKeyGenerator idempotencyKeyGenerator;
+    private final IdempotencyKeyGenerator randomKeyGenerator;
+
+    private final DeterministicIdempotencyKeyGenerator deterministicGenerator;
+
+    public record ResolvedKey(String key, boolean isDeterministic) {
+    }
+
+    public ResolvedKey resolveWithMeta(CommandWrapper wrapper) {
+        // 1. Explicit key from wrapper (client-provided header)
+        if (wrapper.getIdempotencyKey() != null) {
+            return new ResolvedKey(wrapper.getIdempotencyKey(), false);
+        }
+        // 2. Internal retry — key already stored in request context
+        Optional<String> attributeKey = getAttribute();
+        if (attributeKey.isPresent()) {
+            return new ResolvedKey(attributeKey.get(), false);
+        }
+        // 3. No JSON body — cannot hash, use random key
+        if (wrapper.getJson() == null || wrapper.getJson().isBlank()) {
+            return new ResolvedKey(randomKeyGenerator.create(), false);
+        }
+        // 4. No clientId and no entityId — system-level operation (e.g. global
+        // config update, business date change). These have no per-caller scope
+        // so the same payload from different scenarios within the same 5-minute
+        // window would collide. Fall back to random key to avoid false cache hits.
+        if (wrapper.getClientId() == null && wrapper.getEntityId() == null && wrapper.getJobName() == null) {
+            return new ResolvedKey(randomKeyGenerator.create(), false);
+        }
+        // 5. Global configuration updates — same configId + same payload (e.g.
+        // enabled=true) collides across scenarios within the same 5-minute window
+        // since entityId is the configId not a client-scoped resource.
+        String href = wrapper.getHref() != null ? wrapper.getHref() : "";
+        if (href.startsWith("/configurations/")) {
+            return new ResolvedKey(randomKeyGenerator.create(), false);
+        }
+
+        // 6. Job commands — always use random key since jobs must run every invocation
+        // even with the same payload (e.g. same loan IDs for COB across different business dates)
+        if (wrapper.getJobName() != null && !wrapper.getJobName().isBlank()) {
+            return new ResolvedKey(randomKeyGenerator.create(), false);
+        }
+
+        // 7. Account transfers — the ONLY operation where deterministic idempotency
+        // is genuinely needed. A network timeout during a transfer means the client
+        // cannot know if money was moved. Retrying with a random key would create
+        // a duplicate transfer. Deterministic key ensures the retry returns the
+        // cached result instead of moving money twice.
+        String entityName = wrapper.getEntityName() != null ? wrapper.getEntityName().toUpperCase() : "";
+        String actionName = wrapper.getActionName() != null ? wrapper.getActionName().toUpperCase() : "";
+        boolean isAccountTransfer = actionName.equals("CREATE") && entityName.equals("ACCOUNTTRANSFER");
+        if (!isAccountTransfer) {
+            return new ResolvedKey(randomKeyGenerator.create(), false);
+        }
+
+        // 8. Account transfer — generate deterministic key to prevent duplicate transfers
+        String deterministicKey = deterministicGenerator.generate(wrapper.getJson(), buildContext(wrapper));
+        fineractRequestContextHolder.setAttribute(SynchronousCommandProcessingService.IDEMPOTENCY_KEY_ATTRIBUTE, deterministicKey);
+        return new ResolvedKey(deterministicKey, true);
+    }
 
     public String resolve(CommandWrapper wrapper) {
-        return Optional.ofNullable(wrapper.getIdempotencyKey()).orElseGet(() -> getAttribute().orElseGet(idempotencyKeyGenerator::create));
+        return resolveWithMeta(wrapper).key();
+    }
+
+    private String buildContext(CommandWrapper wrapper) {
+        return wrapper.getActionName() + ":" + wrapper.getEntityName() + ":" + wrapper.getHref() + ":" + wrapper.getClientId() + ":"
+                + wrapper.getJobName();
     }
 
     private Optional<String> getAttribute() {

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

@@ -109,16 +109,20 @@ public CommandProcessingResult executeCommand(final CommandWrapper wrapper, fina
 
             CommandSource commandSource = null;
             String idempotencyKey;
+            boolean isDeterministicKey = false;
             if (isRetry) {
                 commandSource = commandSourceService.getCommandSource(commandId);
                 idempotencyKey = commandSource.getIdempotencyKey();
             } else if ((commandId = command.commandId()) != null) { // action on the command itself
                 commandSource = commandSourceService.getCommandSource(commandId);
                 idempotencyKey = commandSource.getIdempotencyKey();
             } else {
-                idempotencyKey = idempotencyKeyResolver.resolve(wrapper);
+                IdempotencyKeyResolver.ResolvedKey resolved = idempotencyKeyResolver.resolveWithMeta(wrapper);
+                idempotencyKey = resolved.key();
+                isDeterministicKey = resolved.isDeterministic();
+                // idempotencyKey = idempotencyKeyResolver.resolve(wrapper);
             }
-            exceptionWhenTheRequestAlreadyProcessed(wrapper, idempotencyKey, isRetry);
+            exceptionWhenTheRequestAlreadyProcessed(wrapper, idempotencyKey, isRetry, isDeterministicKey);
 
             AppUser user = context.authenticatedUser(wrapper);
             if (commandSource == null) {
@@ -218,7 +222,8 @@ private void publishHookErrorEvent(CommandWrapper wrapper, JsonCommand command,
         }
     }
 
-    private void exceptionWhenTheRequestAlreadyProcessed(CommandWrapper wrapper, String idempotencyKey, boolean retry) {
+    private void exceptionWhenTheRequestAlreadyProcessed(CommandWrapper wrapper, String idempotencyKey, boolean retry,
+            boolean isDeterministicKey) {
         CommandSource command = commandSourceService.findCommandSource(wrapper, idempotencyKey);
         if (command == null) {
             return;
@@ -234,7 +239,7 @@ private void exceptionWhenTheRequestAlreadyProcessed(CommandWrapper wrapper, Str
             }
             case PROCESSED -> throw new IdempotentCommandProcessSucceedException(wrapper, idempotencyKey, command);
             case ERROR -> {
-                if (!retry) {
+                if (!retry && !isDeterministicKey) {
                     throw new IdempotentCommandProcessFailedException(wrapper, idempotencyKey, command);
                 }
             }

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

@@ -0,0 +1,78 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.commands.service;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.junit.jupiter.api.Test;
+
+class DeterministicIdempotencyKeyGeneratorTest {
+
+    private final DeterministicIdempotencyKeyGenerator underTest = new DeterministicIdempotencyKeyGenerator();
+
+    @Test
+    void shouldGenerateSameKeyForSameInputAndContext() {
+        String json = "{\"b\":2,\"a\":1}";
+        String context = "action:entity:/endpoint:client1";
+
+        String key1 = underTest.generate(json, context);
+        String key2 = underTest.generate("{\"a\":1,\"b\":2}", context);
+
+        assertEquals(key1, key2);
+    }
+
+    @Test
+    void shouldGenerateDifferentKeysForDifferentContext() {
+        String json = "{\"a\":1}";
+
+        String key1 = underTest.generate(json, "context1");
+        String key2 = underTest.generate(json, "context2");
+
+        assertNotEquals(key1, key2);
+    }
+
+    @Test
+    void shouldGenerateDifferentKeysForDifferentPayload() {
+        String context = "same-context";
+
+        String key1 = underTest.generate("{\"a\":1}", context);
+        String key2 = underTest.generate("{\"a\":2}", context);
+
+        assertNotEquals(key1, key2);
+    }
+
+    @Test
+    void shouldGenerateSameKeyWithinSameTimeWindow() {
+        String json = "{\"a\":1}";
+        String context = "ctx";
+
+        String key1 = underTest.generate(json, context);
+        String key2 = underTest.generate(json, context);
+
+        assertEquals(key1, key2);
+    }
+
+    @Test
+    void shouldFailForInvalidJson() {
+        RuntimeException exception = assertThrows(RuntimeException.class, () -> underTest.generate("{invalid-json", "test-context"));
+        assertEquals("Failed to canonicalize JSON", exception.getMessage());
+    }
+}