From 3b0b43101f297f2b1f3da4b25d94d52f3114ea0a Mon Sep 17 00:00:00 2001
From: "K.S. Yim" <point1304@gmail.com>
Date: Wed, 2 Feb 2022 13:59:13 +0900
Subject: [PATCH] Do not normalize push command with changes for JSON5 upsert

---
 .../command/NormalizingPushCommand.java       | 11 ++++++++--
 .../ZooKeeperCommandExecutorTest.java         | 21 +++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/server/src/main/java/com/linecorp/centraldogma/server/command/NormalizingPushCommand.java b/server/src/main/java/com/linecorp/centraldogma/server/command/NormalizingPushCommand.java
index 33756196bc..bebfede4df 100644
--- a/server/src/main/java/com/linecorp/centraldogma/server/command/NormalizingPushCommand.java
+++ b/server/src/main/java/com/linecorp/centraldogma/server/command/NormalizingPushCommand.java
@@ -15,6 +15,7 @@
  */
 package com.linecorp.centraldogma.server.command;
 
+import static com.linecorp.centraldogma.internal.Util.isJson5;
 import static java.util.Objects.requireNonNull;
 
 import javax.annotation.Nullable;
@@ -24,6 +25,7 @@
 
 import com.linecorp.centraldogma.common.Author;
 import com.linecorp.centraldogma.common.Change;
+import com.linecorp.centraldogma.common.ChangeType;
 import com.linecorp.centraldogma.common.Markup;
 import com.linecorp.centraldogma.common.Revision;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
@@ -57,8 +59,13 @@ public final class NormalizingPushCommand extends AbstractPushCommand<CommitResu
      */
     public PushAsIsCommand asIs(CommitResult commitResult) {
         requireNonNull(commitResult, "commitResult");
+
+        // JSON5 changes should not be normalized to preserve its content in replication log.
+        final boolean useOriginalChanges = changes().stream().anyMatch(
+                change -> change.type() == ChangeType.UPSERT_JSON && isJson5(change.path()));
+
         return new PushAsIsCommand(timestamp(), author(), projectName(), repositoryName(),
-                                   commitResult.revision().backward(1), summary(), detail(),
-                                   markup(), commitResult.changes());
+                                   commitResult.revision().backward(1), summary(), detail(), markup(),
+                                   useOriginalChanges ? changes() : commitResult.changes());
     }
 }
diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutorTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutorTest.java
index fb24584438..d20dcf32fd 100644
--- a/server/src/test/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutorTest.java
+++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutorTest.java
@@ -223,6 +223,27 @@ void testRace() throws Exception {
         }
     }
 
+    @Test
+    void testJson5UpsertReplicationLog() throws Exception {
+        try (Cluster cluster = Cluster.of(ZooKeeperCommandExecutorTest::newMockDelegate)) {
+            final Replica replica1 = cluster.get(0);
+
+            final List<Change<?>> changes =
+                    ImmutableList.of(Change.ofJsonUpsert("/foo.json5", "{a: 'b',}"));
+
+            final Command<CommitResult> command = Command.push(Author.DEFAULT, "project", "repo",
+                                                               new Revision(1), "summary", "detail",
+                                                               Markup.PLAINTEXT, changes);
+
+            replica1.commandExecutor().execute(command).join();
+            final Optional<ReplicationLog<?>> commandResult = replica1.commandExecutor().loadLog(0, false);
+
+            assertThat(commandResult).isPresent();
+            assertThat(commandResult.get().command()).isInstanceOf(PushAsIsCommand.class);
+            assertThat(((PushAsIsCommand) commandResult.get().command()).changes()).isEqualTo(changes);
+        }
+    }
+
     /**
      * Makes sure that we can stop a replica that's waiting for the initial quorum.
      */