vidakovic commented on code in PR #5674:
URL: https://github.com/apache/fineract/pull/5674#discussion_r2972114948


##########
fineract-core/src/main/java/org/apache/fineract/commands/service/DeterministicIdempotencyKeyGenerator.java:
##########
@@ -0,0 +1,49 @@
+package org.apache.fineract.commands.service;
+
+
+import org.apache.fineract.commands.domain.CommandWrapper;
+import org.springframework.stereotype.Component;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.util.Base64;
+
+@Component
+public class DeterministicIdempotencyKeyGenerator {
+
+    private static final int BUCKET_MINUTES = 2;
+
+    public String generate(CommandWrapper wrapper) {
+        String json = wrapper.getJson();
+
+        String normalizedPayload = normalize(json);
+
+        String bucket = currentTimeBucket();
+
+        String raw = normalizedPayload + "|" + bucket;
+
+        return sha256(raw);
+    }
+
+    private String normalize(String json) {
+        // IMPORTANT: make deterministic (order, spacing, etc.)

Review Comment:
   This is not deterministic. The main issue is the order of the attributes... 
and then you have to take into account null values (aka missing attributes)... 
and then there are potentially nested objects (at least on the legacy 
commands), should you order their attributes too? This here won't do what you 
intend to do.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to