diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/converter/ChangesRequestConverter.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/converter/ChangesRequestConverter.java index f8095da53..554120099 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/converter/ChangesRequestConverter.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/converter/ChangesRequestConverter.java @@ -80,28 +80,34 @@ private static Change readChange(JsonNode node) { checkArgument(node.get("path") != null && node.get("type") != null, "a change should have a path and a type"); final ChangeType changeType = ChangeType.parse(node.get("type").textValue()); + final JsonNode content = node.get("content"); if (changeType != ChangeType.REMOVE) { - checkArgument(node.get("content") != null, "a change should have a content."); + checkArgument(content != null, "a change should have a content."); } final String path = node.get("path").textValue(); if (changeType == ChangeType.UPSERT_TEXT) { - return Change.ofTextUpsert(path, node.get("content").textValue()); + return Change.ofTextUpsert(path, content.textValue()); } if (changeType == ChangeType.UPSERT_JSON) { - return Change.ofJsonUpsert(path, node.get("content")); + if (content.isTextual()) { + // A content can be a serialized JSON so the text value needs to be parsed as JSON. + return Change.ofJsonUpsert(path, content.textValue()); + } else { + return Change.ofJsonUpsert(path, content); + } } if (changeType == ChangeType.REMOVE) { return Change.ofRemoval(path); } if (changeType == ChangeType.RENAME) { - return Change.ofRename(path, node.get("content").textValue()); + return Change.ofRename(path, content.textValue()); } if (changeType == ChangeType.APPLY_TEXT_PATCH) { - return Change.ofTextPatch(path, node.get("content").textValue()); + return Change.ofTextPatch(path, content.textValue()); } if (changeType == ChangeType.APPLY_JSON_PATCH) { - return Change.ofJsonPatch(path, node.get("content")); + return Change.ofJsonPatch(path, content); } // Should never reach here. diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1Test.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1Test.java index 05a153141..e9ee4efe7 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1Test.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1Test.java @@ -32,6 +32,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonNode; import com.linecorp.armeria.client.WebClient; @@ -44,6 +45,8 @@ import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.centraldogma.common.ChangeConflictException; +import com.linecorp.centraldogma.common.ChangeFormatException; +import com.linecorp.centraldogma.common.Entry; import com.linecorp.centraldogma.common.InvalidPushException; import com.linecorp.centraldogma.common.RedundantChangeException; import com.linecorp.centraldogma.internal.Jackson; @@ -154,6 +157,85 @@ void pushFileToMetaRepositoryShouldFail() { assertThat(res.contentUtf8()).contains(InvalidPushException.class.getName()); } + @Test + void pushEmbeddedJsonString() throws JsonParseException { + final WebClient client = dogma.httpClient(); + + final String body = + '{' + + " \"path\" : \"/embedded-string.json\"," + + " \"type\" : \"UPSERT_JSON\"," + + " \"content\" : \"\\\"json string\\\"\"," + + " \"commitMessage\" : {" + + " \"summary\" : \"Add embedded string.json\"," + + " \"detail\": \"An embedded JSON string.\"," + + " \"markup\": \"PLAINTEXT\"" + + " }" + + '}'; + final RequestHeaders headers = + RequestHeaders.of(HttpMethod.POST, "/api/v1/projects/myPro/repos/myRepo/contents", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + final AggregatedHttpResponse res = client.execute(headers, body).aggregate().join(); + assertThat(res.status()).isEqualTo(HttpStatus.OK); + + final Entry entry = dogma.client().forRepo("myPro", "myRepo") + .file("/embedded-string.json") + .get().join(); + assertThat(entry.contentAsText()).isEqualTo("\"json string\""); + final JsonNode content = entry.contentAsJson(); + assertThat(content.isTextual()).isTrue(); + assertThat(content.asText()).isEqualTo("json string"); + } + + @Test + void pushUnquoteJsonString() throws JsonParseException { + final WebClient client = dogma.httpClient(); + + final String body = + '{' + + " \"path\" : \"/string.json\"," + + " \"type\" : \"UPSERT_JSON\"," + + " \"content\" : \"json string\"," + + " \"commitMessage\" : {" + + " \"summary\" : \"Add string.json\"," + + " \"detail\": \"An JSON string.\"," + + " \"markup\": \"PLAINTEXT\"" + + " }" + + '}'; + final RequestHeaders headers = + RequestHeaders.of(HttpMethod.POST, "/api/v1/projects/myPro/repos/myRepo/contents", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + final AggregatedHttpResponse res = client.execute(headers, body).aggregate().join(); + assertThat(res.status()).isEqualTo(HttpStatus.BAD_REQUEST); + // Rejected by ChangesRequestConverter + assertThat(res.contentUtf8()).contains(ChangeFormatException.class.getName()); + } + + @Test + void pushInvalidEmbeddedJson() { + final WebClient client = dogma.httpClient(); + + // An invalid JSON containing a trailing comma. + final String body = + '{' + + " \"path\" : \"/invalid.json\"," + + " \"type\" : \"UPSERT_JSON\"," + + " \"content\" : \"{\\\"trailing\\\": \\\"comma\\\", }\"," + + " \"commitMessage\" : {" + + " \"summary\" : \"Add invalid.json\"," + + " \"detail\": \"An invalid JSON must be rejected.\"," + + " \"markup\": \"PLAINTEXT\"" + + " }" + + '}'; + final RequestHeaders headers = + RequestHeaders.of(HttpMethod.POST, "/api/v1/projects/myPro/repos/myRepo/contents", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + final AggregatedHttpResponse res = client.execute(headers, body).aggregate().join(); + assertThat(res.status()).isEqualTo(HttpStatus.BAD_REQUEST); + // Rejected by ChangesRequestConverter + assertThat(res.contentUtf8()).contains(ChangeFormatException.class.getName()); + } + @Nested class FilesTest {