diff --git a/client/java-armeria/src/main/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogma.java b/client/java-armeria/src/main/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogma.java index eaf5de7f39..129b441225 100644 --- a/client/java-armeria/src/main/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogma.java +++ b/client/java-armeria/src/main/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogma.java @@ -21,6 +21,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.linecorp.centraldogma.internal.Util.isJson5; import static com.linecorp.centraldogma.internal.Util.unsafeCast; import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.PROJECTS_PREFIX; import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.REMOVED; @@ -974,7 +975,9 @@ private static ArrayNode toJson(Iterable> changes) { changeNode.put("path", c.path()); changeNode.put("type", c.type().name()); final Class contentType = c.type().contentType(); - if (contentType == JsonNode.class) { + if (isJson5(c.path())) { + changeNode.put("content", c.contentAsText()); + } else if (contentType == JsonNode.class) { changeNode.set("content", (JsonNode) c.content()); } else if (contentType == String.class) { changeNode.put("content", (String) c.content()); @@ -1028,11 +1031,11 @@ private static Entry toEntry(Revision revision, JsonNode node, QueryType throw new CentralDogmaException("invalid entry type. entry type: " + receivedEntryType + " (expected: " + queryType + ')'); } - return entryAsJson(revision, node, entryPath); + return entryAsJson(revision, node, entryPath, queryType); case IDENTITY: switch (receivedEntryType) { case JSON: - return entryAsJson(revision, node, entryPath); + return entryAsJson(revision, node, entryPath, queryType); case TEXT: return entryAsText(revision, node, entryPath); case DIRECTORY: @@ -1053,7 +1056,18 @@ private static Entry entryAsText(Revision revision, JsonNode node, String return unsafeCast(Entry.ofText(revision, entryPath, content0)); } - private static Entry entryAsJson(Revision revision, JsonNode node, String entryPath) { + private static Entry entryAsJson(Revision revision, JsonNode node, String entryPath, + QueryType queryType) { + // 'content' of the JSON5 identity query response is always 'TextNode' to preserve JSON5 format + // while the 'content' of JSON_PATH query can be whatever 'JsonNode'. + if (isJson5(entryPath) && queryType != QueryType.JSON_PATH) { + final String json5Text = getField(node, "content").asText(); + try { + return unsafeCast(Entry.ofJson(revision, entryPath, json5Text)); + } catch (JsonParseException e) { + throw new CentralDogmaException("failed to parse JSON5 content as JSON: " + json5Text, e); + } + } return unsafeCast(Entry.ofJson(revision, entryPath, getField(node, "content"))); } diff --git a/client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaTest.java b/client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaTest.java index ec7b1c9ed8..90ec4c4600 100644 --- a/client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaTest.java +++ b/client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaTest.java @@ -15,55 +15,140 @@ */ package com.linecorp.centraldogma.client.armeria; +import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -import java.net.UnknownHostException; -import java.util.concurrent.CompletionException; +import static org.mockito.Mockito.when; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.ArgumentMatchers; +import org.mockito.Mock; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.databind.JsonNode; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.centraldogma.client.CentralDogma; -import com.linecorp.centraldogma.common.Change; -import com.linecorp.centraldogma.common.InvalidPushException; -import com.linecorp.centraldogma.common.PushResult; -import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; +import com.linecorp.centraldogma.common.Entry; +import com.linecorp.centraldogma.common.Query; +import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Json5; class ArmeriaCentralDogmaTest { - @RegisterExtension - static CentralDogmaExtension dogma = new CentralDogmaExtension() { - @Override - protected void scaffold(CentralDogma client) { - client.createProject("foo").join(); - } - }; + private static final String JSON5_STRING = + "{\n" + + " // comments\n" + + " unquoted: 'and you can quote me on that',\n" + + " singleQuotes: 'I can use \"double quotes\" here',\n" + + " leadingDecimalPoint: .8675309," + + " trailingComma: 'in objects', andIn: ['arrays',]," + + " \"backwardsCompatible\": \"with JSON\",\n" + + "}\n"; + + @Mock + private WebClient webClient; + + private CentralDogma client; + + static Entry getFile(CentralDogma client, Query query) { + return client.getFile("foo", "bar", Revision.INIT, query).join(); + } + + static Entry watchFile(CentralDogma client, Query query) { + return client.watchFile("foo", "bar", Revision.INIT, query).join(); + } + + static void validateJson5Entry(Entry entry) throws JsonParseException { + assertThat(entry.path()).isEqualTo("/foo.json5"); + assertThat(entry.content()).isEqualTo(Json5.readTree(JSON5_STRING)); + assertThat(entry.contentAsText()).isEqualTo(JSON5_STRING); + } + + @BeforeEach + void setUp() { + client = new ArmeriaCentralDogma(newSingleThreadScheduledExecutor(), webClient, "access_token"); + } + + @Test + void testGetJson5File() throws Exception { + when(webClient.execute(ArgumentMatchers.any())).thenReturn( + HttpResponse.ofJson(new MockEntryDto("/foo.json5", "JSON", JSON5_STRING))); + + final Entry entry = getFile(client, Query.ofJson("/foo.json5")); + validateJson5Entry(entry); + } @Test - void pushFileToMetaRepositoryShouldFail() throws UnknownHostException { - final CentralDogma client = new ArmeriaCentralDogmaBuilder() - .host(dogma.serverAddress().getHostString(), dogma.serverAddress().getPort()) - .build(); - - assertThatThrownBy(() -> client.forRepo("foo", "meta") - .commit("summary", Change.ofJsonUpsert("/bar.json", "{ \"a\": \"b\" }")) - .push() - .join()) - .isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(InvalidPushException.class); + void testGetJson5FileWithJsonPath() throws Exception { + final JsonNode node = Jackson.readTree("{\"a\": \"bar\"}"); + when(webClient.execute(ArgumentMatchers.any())).thenReturn( + HttpResponse.ofJson(new MockEntryDto("/foo.json5", "JSON", node))); + + final Entry entry = getFile(client, Query.ofJsonPath("/foo.json5", "$.a")); + assertThat(entry.content()).isEqualTo(node); } @Test - void pushMirrorsJsonFileToMetaRepository() throws UnknownHostException { - final CentralDogma client = new ArmeriaCentralDogmaBuilder() - .host(dogma.serverAddress().getHostString(), dogma.serverAddress().getPort()) - .build(); - - final PushResult result = client.forRepo("foo", "meta") - .commit("summary", Change.ofJsonUpsert("/mirrors.json", "[]")) - .push() - .join(); - assertThat(result.revision().major()).isPositive(); + void testWatchJson5File() throws Exception { + final MockEntryDto entryDto = new MockEntryDto("/foo.json5", "JSON", JSON5_STRING); + when(webClient.execute(ArgumentMatchers.any())).thenReturn( + HttpResponse.ofJson(new MockWatchResultDto(1, entryDto))); + + final Entry entry = watchFile(client, Query.ofJson("/foo.json5")); + validateJson5Entry(entry); + } + + static class MockEntryDto { + + private final String path; + private final String type; + private final Object content; + + MockEntryDto(String path, String type, Object content) { + this.path = path; + this.type = type; + this.content = content; + } + + @JsonProperty("path") + String path() { + return path; + } + + @JsonProperty("type") + String type() { + return type; + } + + @JsonProperty("content") + Object content() { + return content; + } + } + + static class MockWatchResultDto { + + private final int revision; + private final MockEntryDto entry; + + MockWatchResultDto(int revision, MockEntryDto entry) { + this.revision = revision; + this.entry = entry; + } + + @JsonProperty("revision") + int revision() { + return revision; + } + + @JsonProperty("entry") + MockEntryDto entry() { + return entry; + } } } diff --git a/common/src/main/java/com/linecorp/centraldogma/common/Change.java b/common/src/main/java/com/linecorp/centraldogma/common/Change.java index 06c57432f9..34c2f96a13 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/Change.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/Change.java @@ -16,6 +16,7 @@ package com.linecorp.centraldogma.common; +import static com.linecorp.centraldogma.internal.Util.isJson5; import static com.linecorp.centraldogma.internal.Util.validateDirPath; import static com.linecorp.centraldogma.internal.Util.validateFilePath; import static java.util.Objects.requireNonNull; @@ -34,10 +35,12 @@ import javax.annotation.Nullable; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Json5; import com.linecorp.centraldogma.internal.Util; import com.linecorp.centraldogma.internal.jsonpatch.JsonPatch; import com.linecorp.centraldogma.internal.jsonpatch.ReplaceMode; @@ -51,6 +54,22 @@ @JsonDeserialize(as = DefaultChange.class) public interface Change { + /** + * Returns a newly-created {@link Change} whose type is {@link ChangeType#UPSERT_TEXT}. + * + *

Note that you should use {@link #ofJsonUpsert(String, String)} if the specified {@code path} ends with + * {@code ".json"}. The {@link #ofJsonUpsert(String, String)} will check that the given {@code text} is a + * valid JSON. + * + * @param path the path of the file + * @param content UTF-8 encoded text file content + * @throws ChangeFormatException if the path ends with {@code ".json"} + */ + static Change ofTextUpsert(String path, byte[] content) { + requireNonNull(content, "content"); + return ofTextUpsert(path, new String(content, StandardCharsets.UTF_8)); + } + /** * Returns a newly-created {@link Change} whose type is {@link ChangeType#UPSERT_TEXT}. * @@ -65,13 +84,26 @@ public interface Change { static Change ofTextUpsert(String path, String text) { requireNonNull(text, "text"); validateFilePath(path, "path"); - if (EntryType.guessFromPath(path) == EntryType.JSON) { + if (EntryType.guessFromPath(path) == EntryType.JSON && !isJson5(path)) { throw new ChangeFormatException("invalid file type: " + path + " (expected: a non-JSON file). Use Change.ofJsonUpsert() instead"); } return new DefaultChange<>(path, ChangeType.UPSERT_TEXT, text); } + /** + * Returns a newly-created {@link Change} whose type is {@link ChangeType#UPSERT_JSON}. + * + * @param path the path of the file + * @param jsonContent the content of the file + * + * @throws ChangeFormatException if the specified {@code jsonText} is not a valid JSON + */ + static Change ofJsonUpsert(String path, byte[] jsonContent) { + requireNonNull(jsonContent, "jsonContent"); + return ofJsonUpsert(path, new String(jsonContent, StandardCharsets.UTF_8)); + } + /** * Returns a newly-created {@link Change} whose type is {@link ChangeType#UPSERT_JSON}. * @@ -85,12 +117,15 @@ static Change ofJsonUpsert(String path, String jsonText) { final JsonNode jsonNode; try { + if (isJson5(path)) { + jsonNode = Json5.readTree(jsonText); + return new DefaultChange<>(path, ChangeType.UPSERT_JSON, jsonNode, jsonText); + } jsonNode = Jackson.readTree(jsonText); - } catch (IOException e) { + return new DefaultChange<>(path, ChangeType.UPSERT_JSON, jsonNode); + } catch (JsonParseException e) { throw new ChangeFormatException("failed to read a value as a JSON tree", e); } - - return new DefaultChange<>(path, ChangeType.UPSERT_JSON, jsonNode); } /** @@ -140,10 +175,6 @@ static Change ofRename(String oldPath, String newPath) { static Change ofTextPatch(String path, @Nullable String oldText, String newText) { validateFilePath(path, "path"); requireNonNull(newText, "newText"); - if (EntryType.guessFromPath(path) == EntryType.JSON) { - throw new ChangeFormatException("invalid file type: " + path + - " (expected: a non-JSON file). Use Change.ofJsonPatch() instead"); - } final List oldLineList = oldText == null ? Collections.emptyList() : Util.stringToLines(oldText); @@ -152,7 +183,7 @@ static Change ofTextPatch(String path, @Nullable String oldText, String final Patch patch = DiffUtils.diff(oldLineList, newLineList); final List unifiedDiff = DiffUtils.generateUnifiedDiff(path, path, oldLineList, patch, 3); - return new DefaultChange<>(path, ChangeType.APPLY_TEXT_PATCH, String.join("\n", unifiedDiff)); + return ofTextPatch(path, String.join("\n", unifiedDiff)); } /** @@ -170,7 +201,7 @@ static Change ofTextPatch(String path, @Nullable String oldText, String static Change ofTextPatch(String path, String textPatch) { validateFilePath(path, "path"); requireNonNull(textPatch, "textPatch"); - if (EntryType.guessFromPath(path) == EntryType.JSON) { + if (EntryType.guessFromPath(path) == EntryType.JSON && !isJson5(path)) { throw new ChangeFormatException("invalid file type: " + path + " (expected: a non-JSON file). Use Change.ofJsonPatch() instead"); } diff --git a/common/src/main/java/com/linecorp/centraldogma/common/DefaultChange.java b/common/src/main/java/com/linecorp/centraldogma/common/DefaultChange.java index 0d205425c9..0f4118f828 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/DefaultChange.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/DefaultChange.java @@ -96,6 +96,10 @@ private static DefaultChange rejectIncompatibleContent(@Nullable JsonNode con private String contentAsText; DefaultChange(String path, ChangeType type, @Nullable T content) { + this(path, type, content, null); + } + + DefaultChange(String path, ChangeType type, @Nullable T content, @Nullable String contentAsText) { this.type = requireNonNull(type, "type"); if (type.contentType() == JsonNode.class) { @@ -106,6 +110,7 @@ private static DefaultChange rejectIncompatibleContent(@Nullable JsonNode con this.path = path; this.content = content; + this.contentAsText = contentAsText; } @Override diff --git a/common/src/main/java/com/linecorp/centraldogma/common/Entry.java b/common/src/main/java/com/linecorp/centraldogma/common/Entry.java index 42bb08e9e8..2e152d0ae3 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/Entry.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/Entry.java @@ -17,8 +17,10 @@ package com.linecorp.centraldogma.common; import static com.google.common.base.Preconditions.checkArgument; +import static com.linecorp.centraldogma.internal.Util.isJson5; import static java.util.Objects.requireNonNull; +import java.nio.charset.StandardCharsets; import java.util.Objects; import java.util.function.Consumer; @@ -29,6 +31,7 @@ import com.google.common.base.MoreObjects; import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Json5; /** * A file or a directory in a repository. @@ -69,7 +72,28 @@ public static Entry ofJson(Revision revision, String path, JsonNode co */ public static Entry ofJson(Revision revision, String path, String content) throws JsonParseException { - return ofJson(revision, path, Jackson.readTree(content)); + requireNonNull(content, "content"); + return isJson5(path) ? new Entry<>(revision, path, EntryType.JSON, Json5.readTree(content), content) + : new Entry<>(revision, path, EntryType.JSON, Jackson.readTree(content)); + } + + /** + * Returns a newly-created {@link Entry} of a JSON file. + * + * @param revision the revision of the JSON file + * @param path the path of the JSON file + * @param content the content of the JSON file in byte array. + * + * @throws JsonParseException if the {@code content} is not a valid JSON + */ + public static Entry ofJson(Revision revision, String path, byte[] content) + throws JsonParseException { + requireNonNull(content, "content"); + if (isJson5(path)) { + return new Entry<>(revision, path, EntryType.JSON, Json5.readTree(content), + new String(content, StandardCharsets.UTF_8)); + } + return new Entry<>(revision, path, EntryType.JSON, Jackson.readTree(content)); } /** @@ -106,6 +130,10 @@ public static Entry of(Revision revision, String path, EntryType type, @N @Nullable private String contentAsPrettyText; + private Entry(Revision revision, String path, EntryType type, @Nullable T content) { + this(revision, path, type, content, null); + } + /** * Creates a new instance. * @@ -114,7 +142,8 @@ public static Entry of(Revision revision, String path, EntryType type, @N * @param type the type of given {@code content} * @param content an object of given type {@code T} */ - private Entry(Revision revision, String path, EntryType type, @Nullable T content) { + private Entry(Revision revision, String path, EntryType type, + @Nullable T content, @Nullable String contentAsText) { requireNonNull(revision, "revision"); checkArgument(!revision.isRelative(), "revision: %s (expected: absolute revision)", revision); this.revision = revision; @@ -126,10 +155,12 @@ private Entry(Revision revision, String path, EntryType type, @Nullable T conten if (entryContentType == Void.class) { checkArgument(content == null, "content: %s (expected: null)", content); this.content = null; + this.contentAsText = null; } else { @SuppressWarnings("unchecked") final T castContent = (T) entryContentType.cast(requireNonNull(content, "content")); this.content = castContent; + this.contentAsText = contentAsText; } } @@ -211,7 +242,9 @@ public boolean equals(Object o) { final Entry that = (Entry) o; return type == that.type && revision.equals(that.revision) && path.equals(that.path) && - Objects.equals(content, that.content); + Objects.equals(content, that.content) && + // Compare 'contentAsText' for JSON5 entries. + (!isJson5(path) || Objects.equals(contentAsText, that.contentAsText)); } @Override diff --git a/common/src/main/java/com/linecorp/centraldogma/common/EntryType.java b/common/src/main/java/com/linecorp/centraldogma/common/EntryType.java index c3afe47573..b6b9f72fd3 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/EntryType.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/EntryType.java @@ -16,10 +16,11 @@ package com.linecorp.centraldogma.common; +import static com.linecorp.centraldogma.internal.Util.isJson; +import static com.linecorp.centraldogma.internal.Util.isJson5; import static java.util.Objects.requireNonNull; import com.fasterxml.jackson.databind.JsonNode; -import com.google.common.base.Ascii; /** * The type of an {@link Entry}. @@ -51,7 +52,7 @@ public static EntryType guessFromPath(String path) { return DIRECTORY; } - if (Ascii.toLowerCase(path).endsWith(".json")) { + if (isJson(path) || isJson5(path)) { return JSON; } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/Json5.java b/common/src/main/java/com/linecorp/centraldogma/internal/Json5.java new file mode 100644 index 0000000000..9b47051cf6 --- /dev/null +++ b/common/src/main/java/com/linecorp/centraldogma/internal/Json5.java @@ -0,0 +1,75 @@ +/* + * Copyright 2021 LINE Corporation + * + * LINE Corporation 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: + * + * https://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 com.linecorp.centraldogma.internal; + +import java.io.IOError; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.time.Instant; + +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.json.JsonReadFeature; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.fasterxml.jackson.datatype.jsr310.deser.InstantDeserializer; +import com.fasterxml.jackson.datatype.jsr310.ser.InstantSerializer; + +public final class Json5 { + + private static final ObjectMapper mapper = new ObjectMapper(); + + static { + // Sort the attributes when serialized via the mapper. + mapper.enable(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS); + + // Enable JSON5 feature. + mapper.enable(JsonReadFeature.ALLOW_SINGLE_QUOTES.mappedFeature(), + JsonReadFeature.ALLOW_UNQUOTED_FIELD_NAMES.mappedFeature(), + JsonReadFeature.ALLOW_JAVA_COMMENTS.mappedFeature(), + JsonReadFeature.ALLOW_TRAILING_COMMA.mappedFeature(), + JsonReadFeature.ALLOW_BACKSLASH_ESCAPING_ANY_CHARACTER.mappedFeature(), + JsonReadFeature.ALLOW_NON_NUMERIC_NUMBERS.mappedFeature(), + JsonReadFeature.ALLOW_LEADING_DECIMAL_POINT_FOR_NUMBERS.mappedFeature()); + + mapper.registerModules(new SimpleModule().addSerializer(Instant.class, InstantSerializer.INSTANCE) + .addDeserializer(Instant.class, InstantDeserializer.INSTANT)); + } + + public static JsonNode readTree(String data) throws JsonParseException { + try { + return mapper.readTree(data); + } catch (JsonParseException e) { + throw e; + } catch (IOException e) { + throw new IOError(e); + } + } + + public static JsonNode readTree(byte[] data) throws JsonParseException { + // String conversion is a workaround for Jackson bug with 'ALLOW_SINGLE_QUOTES' feature + // when it deserializes JSON5 from byte array source. + // If double quotes are used within a single quoted string, it either raises JsonParseException or + // removes double quotes while deserializing. + // e.g. 'I can use "double quotes" here' deserialized into "I can use double quotes here" + // 'I can use double quotes "here"' raises JsonParseException. + return readTree(new String(data, StandardCharsets.UTF_8)); + } + + private Json5() {} +} diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/Util.java b/common/src/main/java/com/linecorp/centraldogma/internal/Util.java index 110e9b1933..67cdc0c024 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/Util.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/Util.java @@ -29,8 +29,12 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import com.google.common.base.Ascii; import com.jayway.jsonpath.JsonPath; +import com.linecorp.centraldogma.common.Entry; +import com.linecorp.centraldogma.common.EntryType; + /** * This class borrowed some of its methods from a NetUtil class which was part of Netty project. @@ -42,7 +46,7 @@ public final class Util { private static final Pattern FILE_PATH_PATTERN = Pattern.compile( "^(?:/[-_0-9a-zA-Z](?:[-_.0-9a-zA-Z]*[-_0-9a-zA-Z])?)+$"); private static final Pattern JSON_FILE_PATH_PATTERN = Pattern.compile( - "^(?:/[-_0-9a-zA-Z](?:[-_.0-9a-zA-Z]*[-_0-9a-zA-Z])?)+\\.(?i)json$"); + "^(?:/[-_0-9a-zA-Z](?:[-_.0-9a-zA-Z]*[-_0-9a-zA-Z])?)+\\.(?i)json[5]?$"); private static final Pattern DIR_PATH_PATTERN = Pattern.compile( "^(?:/[-_0-9a-zA-Z](?:[-_.0-9a-zA-Z]*[-_0-9a-zA-Z])?)*/?$"); private static final Pattern PATH_PATTERN_PATTERN = Pattern.compile("^[- /*_.,0-9a-zA-Z]+$"); @@ -97,6 +101,21 @@ public static boolean isValidJsonFilePath(String path) { JSON_FILE_PATH_PATTERN.matcher(path).matches(); } + public static boolean isJson(String path) { + requireNonNull(path, "path"); + return Ascii.toLowerCase(path).endsWith(".json"); + } + + public static boolean isJson5(String path) { + requireNonNull(path, "path"); + return Ascii.toLowerCase(path).endsWith(".json5"); + } + + public static boolean isJson5(Entry entry) { + requireNonNull(entry, "entry"); + return entry.type() == EntryType.JSON && isJson5(entry.path()); + } + public static String validateJsonPath(String jsonPath, String paramName) { requireNonNull(jsonPath, paramName); checkArgument(isValidJsonPath(jsonPath), diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/EntryDto.java b/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/EntryDto.java index 995db34300..6e9d74fe3e 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/EntryDto.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/EntryDto.java @@ -32,7 +32,7 @@ import com.linecorp.centraldogma.common.Revision; @JsonInclude(Include.NON_NULL) -public class EntryDto { +public class EntryDto { private final Revision revision; @@ -40,12 +40,13 @@ public class EntryDto { private final EntryType type; - private final T content; + @Nullable + private final Object content; private final String url; public EntryDto(Revision revision, String path, EntryType type, - String projectName, String repoName, @Nullable T content) { + String projectName, String repoName, @Nullable Object content) { this.revision = requireNonNull(revision, "revision"); this.path = requireNonNull(path, "path"); this.type = requireNonNull(type, "type"); @@ -70,7 +71,7 @@ public EntryType type() { @JsonProperty @Nullable - public T content() { + public Object content() { return content; } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/WatchResultDto.java b/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/WatchResultDto.java index 575861015c..500cc12461 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/WatchResultDto.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/api/v1/WatchResultDto.java @@ -31,9 +31,9 @@ public class WatchResultDto { private final Revision revision; - private final EntryDto entry; + private final EntryDto entry; - public WatchResultDto(Revision revision, @Nullable EntryDto entry) { + public WatchResultDto(Revision revision, @Nullable EntryDto entry) { this.revision = requireNonNull(revision, "revision"); this.entry = entry; } @@ -44,7 +44,7 @@ public Revision revision() { } @JsonProperty("entry") - public EntryDto entry() { + public EntryDto entry() { return entry; } diff --git a/it/src/test/java/com/linecorp/centraldogma/it/GetFileTest.java b/it/src/test/java/com/linecorp/centraldogma/it/GetFileTest.java index 877f0d06df..b14322c399 100644 --- a/it/src/test/java/com/linecorp/centraldogma/it/GetFileTest.java +++ b/it/src/test/java/com/linecorp/centraldogma/it/GetFileTest.java @@ -23,11 +23,15 @@ import java.util.concurrent.CompletionException; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; +import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.collect.ImmutableList; import com.linecorp.centraldogma.client.CentralDogma; import com.linecorp.centraldogma.common.Change; @@ -38,6 +42,7 @@ import com.linecorp.centraldogma.common.QueryExecutionException; import com.linecorp.centraldogma.common.RepositoryNotFoundException; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.internal.Json5; class GetFileTest { @@ -46,7 +51,7 @@ class GetFileTest { @ParameterizedTest @EnumSource(ClientType.class) - void getJsonAsText(ClientType clientType) throws Exception { + void getJsonAsText(ClientType clientType) { final CentralDogma client = clientType.client(dogma); client.forRepo(dogma.project(), dogma.repo1()) .commit("Add a file", Change.ofJsonUpsert("/test/foo.json", "{ \"a\": \"b\" }")) @@ -102,4 +107,36 @@ void invalidProject(ClientType clientType) throws Exception { Query.ofJsonPath("/test/test2.json", "$.non_exist_path")).join()) .isInstanceOf(CompletionException.class).hasCauseInstanceOf(ProjectNotFoundException.class); } + + @Nested + class GetFileJson5Test { + + @Test + void getJson5() throws JsonParseException { + final CentralDogma client = dogma.client(); + client.forRepo(dogma.project(), dogma.repo1()) + .commit("Add a file", Change.ofJsonUpsert("/test/foo.json5", "{ a: 'b' }")) + .push().join(); + + final Entry json = client.getFile(dogma.project(), dogma.repo1(), Revision.HEAD, + Query.ofJson("/test/foo.json5")).join(); + assertThatJson(json.content()).isEqualTo(Json5.readTree("{ a: 'b' }")); + assertThat(json.contentAsText()).isEqualTo("{ a: 'b' }\n"); + } + + @Test + void jsonPath() { + final CentralDogma client = dogma.client(); + + final Entry json1 = client.getFile( + dogma.project(), dogma.repo1(), Revision.HEAD, + Query.ofJsonPath("/test/test1.json5", "$.a")).join(); + assertThat(json1.content().asText()).isEqualTo("apple"); + + final Entry json2 = client.getFile( + dogma.project(), dogma.repo1(), Revision.HEAD, + Query.ofJsonPath("/test/test1.json5", ImmutableList.of("$.numbers", "$.[0]"))).join(); + assertThat(json2.content().asInt()).isEqualTo(1); + } + } } diff --git a/it/src/test/java/com/linecorp/centraldogma/it/MergeFileTest.java b/it/src/test/java/com/linecorp/centraldogma/it/MergeFileTest.java index 07ba968826..f29266a2d3 100644 --- a/it/src/test/java/com/linecorp/centraldogma/it/MergeFileTest.java +++ b/it/src/test/java/com/linecorp/centraldogma/it/MergeFileTest.java @@ -21,6 +21,8 @@ import java.util.concurrent.CompletionException; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; @@ -51,7 +53,9 @@ protected void scaffold(CentralDogma client) { .commit("Initial files", Change.ofJsonUpsert("/foo.json", "{ \"a\": \"bar\" }"), Change.ofJsonUpsert("/foo1.json", "{ \"b\": \"baz\" }"), - Change.ofJsonUpsert("/foo2.json", "{ \"a\": \"new_bar\" }")) + Change.ofJsonUpsert("/foo2.json", "{ \"a\": \"new_bar\" }"), + Change.ofJsonUpsert("/foo.json5", "{ a: 'qux' }"), + Change.ofJsonUpsert("/foo1.json5", "{ a: 'new_qux', b: 'quux' }")) .push().join(); } @@ -157,4 +161,31 @@ void mergeJsonPaths(ClientType clientType) { .isInstanceOf(CompletionException.class) .hasCauseInstanceOf(QueryExecutionException.class); } + + @Nested + class MergeFileJson5Test { + + @Test + void mergeJson5Files() { + final MergedEntry merged = dogma.client().mergeFiles( + "myPro", "myRepo", Revision.HEAD, + MergeSource.ofRequired("/foo.json5"), + MergeSource.ofRequired("/foo1.json5")).join(); + + assertThat(merged.paths()).containsExactly("/foo.json5", "/foo1.json5"); + assertThatJson(merged.content()).isEqualTo("{\"a\": \"new_qux\", \"b\": \"quux\"}"); + } + + @Test + void mergeWithOrdinaryJson() { + final MergedEntry merged = dogma.client().mergeFiles( + "myPro", "myRepo", Revision.HEAD, + MergeSource.ofRequired("/foo.json"), + MergeSource.ofRequired("/foo.json5"), + MergeSource.ofRequired("/foo1.json")).join(); + + assertThat(merged.paths()).containsExactly("/foo.json", "/foo.json5", "/foo1.json"); + assertThatJson(merged.content()).isEqualTo("{\"a\": \"qux\", \"b\": \"baz\"}"); + } + } } diff --git a/it/src/test/java/com/linecorp/centraldogma/it/MetaRepositoryPushTest.java b/it/src/test/java/com/linecorp/centraldogma/it/MetaRepositoryPushTest.java new file mode 100644 index 0000000000..4d0f242b3f --- /dev/null +++ b/it/src/test/java/com/linecorp/centraldogma/it/MetaRepositoryPushTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2021 LINE Corporation + * + * LINE Corporation 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: + * + * https://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 com.linecorp.centraldogma.it; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.net.UnknownHostException; +import java.util.concurrent.CompletionException; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import com.linecorp.centraldogma.client.CentralDogma; +import com.linecorp.centraldogma.client.armeria.ArmeriaCentralDogmaBuilder; +import com.linecorp.centraldogma.common.Change; +import com.linecorp.centraldogma.common.InvalidPushException; +import com.linecorp.centraldogma.common.PushResult; +import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; + +class MetaRepositoryPushTest { + + @RegisterExtension + static CentralDogmaExtension dogma = new CentralDogmaExtension() { + @Override + protected void scaffold(CentralDogma client) { + client.createProject("foo").join(); + } + }; + + @Test + void pushFileToMetaRepositoryShouldFail() throws UnknownHostException { + final CentralDogma client = new ArmeriaCentralDogmaBuilder() + .host(dogma.serverAddress().getHostString(), dogma.serverAddress().getPort()) + .build(); + + assertThatThrownBy(() -> client.push("foo", + "meta", + Revision.HEAD, + "summary", + Change.ofJsonUpsert("/bar.json", "{ \"a\": \"b\" }")) + .join()) + .isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(InvalidPushException.class); + } + + @Test + void pushMirrorsJsonFileToMetaRepository() throws UnknownHostException { + final CentralDogma client = new ArmeriaCentralDogmaBuilder() + .host(dogma.serverAddress().getHostString(), dogma.serverAddress().getPort()) + .build(); + + final PushResult result = client.push("foo", + "meta", + Revision.HEAD, + "summary", + Change.ofJsonUpsert("/mirrors.json", "[]")) + .join(); + assertThat(result.revision().major()).isPositive(); + } +} diff --git a/it/src/test/java/com/linecorp/centraldogma/it/WatchTest.java b/it/src/test/java/com/linecorp/centraldogma/it/WatchTest.java index cf9c1e636d..c04d726fee 100644 --- a/it/src/test/java/com/linecorp/centraldogma/it/WatchTest.java +++ b/it/src/test/java/com/linecorp/centraldogma/it/WatchTest.java @@ -34,6 +34,7 @@ import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; @@ -747,11 +748,19 @@ void repositoryWatcher_errorOnEntryNotFound_watchIsNotWorking() throws Exception } private static void revertTestFiles(ClientType clientType) { + revertTestFiles(clientType.client(dogma)); + } + + private static void revertTestFiles(CentralDogma client) { final Change change1 = Change.ofJsonUpsert("/test/test1.json", "[ 1, 2, 3 ]"); final Change change2 = Change.ofJsonUpsert("/test/test2.json", "{ \"a\": \"apple\" }"); + final Change change3 = Change.ofJsonUpsert("/test/test1.json5", "{\n" + + " // comments\n" + + " a: 'apple',\n" + + " numbers: [1, 2, 3],\n" + + "}\n"); - final List> changes = Arrays.asList(change1, change2); - final CentralDogma client = clientType.client(dogma); + final List> changes = Arrays.asList(change1, change2, change3); if (!client.getPreviewDiffs(dogma.project(), dogma.repo1(), Revision.HEAD, changes) .join().isEmpty()) { @@ -761,14 +770,74 @@ private static void revertTestFiles(ClientType clientType) { .join(); } - final Change change3 = Change.ofRemoval("/test_not_found/test.json"); + final Change change4 = Change.ofRemoval("/test_not_found/test.json"); final Map files = client.listFiles(dogma.project(), dogma.repo1(), Revision.HEAD, PathPattern.of("/test_not_found/**")).join(); - if (files.containsKey(change3.path())) { + if (files.containsKey(change4.path())) { client.forRepo(dogma.project(), dogma.repo1()) - .commit("Remove test files", change3) + .commit("Remove test files", change4) .push() .join(); } } + + @Nested + class WatchJson5Test { + + @Test + void watchJson5() throws Exception { + final CentralDogma client = dogma.client(); + revertTestFiles(client); + + final CompletableFuture> future = + client.forRepo(dogma.project(), dogma.repo1()) + .watch(Query.ofJson("/test/test1.json5")) + .start(Revision.HEAD); + + assertThatThrownBy(() -> future.get(500, TimeUnit.MILLISECONDS)) + .isInstanceOf(TimeoutException.class); + + // Make change to an irrelevant file. + client.forRepo(dogma.project(), dogma.repo1()) + .commit("Edit foo.json", Change.ofJsonUpsert("/test/foo.json", "{}")) + .push(Revision.HEAD) + .join(); + + assertThatThrownBy(() -> future.get(500, TimeUnit.MILLISECONDS)) + .isInstanceOf(TimeoutException.class); + + // Make change to a relevant file. + final PushResult result = + client.forRepo(dogma.project(), dogma.repo1()) + .commit("Edit test1.json5", Change.ofJsonUpsert("/test/test1.json5", "{a: 'foo'}")) + .push(Revision.HEAD) + .join(); + + assertThat(future.get(3, TimeUnit.SECONDS)).isEqualTo( + Entry.ofJson(result.revision(), "/test/test1.json5", "{a: 'foo'}\n")); + } + + @Test + void watchJson5_notNotifiedIfJsonContentNotChanged() { + final CentralDogma client = dogma.client(); + revertTestFiles(client); + + final CompletableFuture> future = + client.forRepo(dogma.project(), dogma.repo1()) + .watch(Query.ofJson("/test/test1.json5")) + .start(Revision.HEAD); + + // Edit file to the plain JSON, so it doesn't change the actual JSON content in it. + client.forRepo(dogma.project(), dogma.repo1()) + .commit("Edit test1.json5", + Change.ofJsonUpsert("/test/test1.json5", + "{\"a\": \"apple\", \"numbers\": [1,2,3]}")) + .push(Revision.HEAD) + .join(); + + // Watcher should not be notified since the JSON content is still the same. + assertThatThrownBy(() -> future.get(500, TimeUnit.MILLISECONDS)) + .isInstanceOf(TimeoutException.class); + } + } } diff --git a/it/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorTest.java b/it/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorTest.java index 00fda5c962..f7a815c7e6 100644 --- a/it/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorTest.java +++ b/it/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorTest.java @@ -177,6 +177,7 @@ void remoteToLocal() throws Exception { addToGitIndex(".gitkeep", ""); addToGitIndex("first/light.txt", "26-Aug-2014"); addToGitIndex("second/son.json", "{\"release_date\": \"21-Mar-2014\"}"); + addToGitIndex("third/daughter.json5", "{release_date: '21-Mar-2014'}"); git.commit().setMessage("Add the release dates of the 'Infamous' series").call(); mirroringService.mirror().join(); @@ -193,7 +194,10 @@ void remoteToLocal() throws Exception { Entry.ofText(rev3, "/first/light.txt", "26-Aug-2014\n"), Entry.ofDirectory(rev3, "/second"), Entry.ofJson(rev3, "/second/son.json", - "{\"release_date\": \"21-Mar-2014\"}")); + "{\"release_date\": \"21-Mar-2014\"}"), + Entry.ofDirectory(rev3, "/third"), + Entry.ofJson(rev3, "/third/daughter.json5", + "{release_date: '21-Mar-2014'}\n")); // Rewrite the history of the git repository and mirror. git.reset().setMode(ResetType.HARD).setRef("HEAD^").call(); diff --git a/it/src/test/resources/com/linecorp/centraldogma/it/import/test1.json5 b/it/src/test/resources/com/linecorp/centraldogma/it/import/test1.json5 new file mode 100644 index 0000000000..4195db3c90 --- /dev/null +++ b/it/src/test/resources/com/linecorp/centraldogma/it/import/test1.json5 @@ -0,0 +1,5 @@ +{ + // comments + a: 'apple', + numbers: [1, 2, 3], +} diff --git a/server/build.gradle b/server/build.gradle index dfaba9136a..0d8046eb47 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -57,7 +57,7 @@ clientDependencies { installDir = "${project.projectDir}/src/main/resources/webapp/vendor" copyExcludes = ['**/Gruntfile.js', 'gulpfile.js', 'source/**', 'test', 'karma.conf.js', 'index.js'] npm { - 'ace-builds'('1.2.5', from: 'src-min-noconflict') + 'ace-builds'('1.4.13', from: 'src-min-noconflict') 'angular'('1.5.8') 'angular-cache-buster'('0.4.3') 'angular-cookies'('1.5.8') diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java index 757194d008..1531110e9d 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java @@ -114,18 +114,18 @@ public ContentServiceV1(ProjectManager projectManager, CommandExecutor executor, *

Returns the list of files in the path. */ @Get("regex:/projects/(?[^/]+)/repos/(?[^/]+)/list(?(|/.*))$") - public CompletableFuture>> listFiles(@Param String path, - @Param @Default("-1") String revision, - Repository repository) { + public CompletableFuture> listFiles(@Param String path, + @Param @Default("-1") String revision, + Repository repository) { final String normalizedPath = normalizePath(path); final Revision normalizedRev = repository.normalizeNow(new Revision(revision)); - final CompletableFuture>> future = new CompletableFuture<>(); + final CompletableFuture> future = new CompletableFuture<>(); listFiles(repository, normalizedPath, normalizedRev, false, future); return future; } private static void listFiles(Repository repository, String pathPattern, Revision normalizedRev, - boolean withContent, CompletableFuture>> result) { + boolean withContent, CompletableFuture> result) { final Map, ?> options = withContent ? FindOptions.FIND_ALL_WITH_CONTENT : FindOptions.FIND_ALL_WITHOUT_CONTENT; @@ -266,11 +266,11 @@ public CompletableFuture getFiles( // get a file return repository.get(normalizedRev, query) .handle(returnOrThrow((Entry result) -> convert(repository, normalizedRev, - result, true))); + result, true, query.type()))); } // get files - final CompletableFuture>> future = new CompletableFuture<>(); + final CompletableFuture> future = new CompletableFuture<>(); listFiles(repository, normalizedPath, normalizedRev, true, future); return future; } @@ -287,7 +287,7 @@ private CompletableFuture watchFile(ServiceRequestContext ctx, return future.thenApply(entry -> { final Revision revision = entry.revision(); - final EntryDto entryDto = convert(repository, revision, entry, true); + final EntryDto entryDto = convert(repository, revision, entry, true, query.type()); return (Object) new WatchResultDto(revision, entryDto); }).exceptionally(ContentServiceV1::handleWatchFailure); } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/DtoConverter.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/DtoConverter.java index 67ae6d5d40..f3a03a9dee 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/DtoConverter.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/DtoConverter.java @@ -16,6 +16,7 @@ package com.linecorp.centraldogma.server.internal.api; +import static com.linecorp.centraldogma.internal.Util.isJson5; import static java.util.Objects.requireNonNull; import javax.annotation.Nullable; @@ -26,6 +27,7 @@ import com.linecorp.centraldogma.common.Entry; import com.linecorp.centraldogma.common.EntryType; import com.linecorp.centraldogma.common.MergedEntry; +import com.linecorp.centraldogma.common.QueryType; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.api.v1.ChangeDto; import com.linecorp.centraldogma.internal.api.v1.CommitDto; @@ -58,29 +60,38 @@ public static RepositoryDto convert(Repository repository) { repository.creationTimeMillis()); } - public static EntryDto convert(Repository repository, Revision revision, - Entry entry, boolean withContent) { + public static EntryDto convert(Repository repository, Revision revision, + Entry entry, boolean withContent) { + return convert(repository, revision, entry, withContent, null); + } + + public static EntryDto convert(Repository repository, Revision revision, + Entry entry, boolean withContent, @Nullable QueryType queryType) { requireNonNull(entry, "entry"); if (withContent && entry.hasContent()) { - return convert(repository, revision, entry.path(), entry.type(), entry.content()); + // TODO(ks-yim): Not sure of 'DtoConverter' having business logic and it looks hacky to receive + // queryType to handle JSON_PATH query for JSON5 files. + return convert(repository, revision, entry.path(), entry.type(), + isJson5(entry) && queryType != QueryType.JSON_PATH ? entry.contentAsText() + : entry.content()); } return convert(repository, revision, entry.path(), entry.type()); } - private static EntryDto convert(Repository repository, Revision revision, - String path, EntryType type) { + private static EntryDto convert(Repository repository, Revision revision, + String path, EntryType type) { return convert(repository, revision, path, type, null); } - private static EntryDto convert(Repository repository, Revision revision, String path, - EntryType type, @Nullable T content) { + private static EntryDto convert(Repository repository, Revision revision, String path, + EntryType type, @Nullable Object content) { requireNonNull(repository, "repository"); - return new EntryDto<>(requireNonNull(revision, "revision"), - requireNonNull(path, "path"), - requireNonNull(type, "type"), - repository.parent().name(), - repository.name(), - content); + return new EntryDto(requireNonNull(revision, "revision"), + requireNonNull(path, "path"), + requireNonNull(type, "type"), + repository.parent().name(), + repository.name(), + content); } public static PushResultDto convert(Revision revision, long commitTimeMillis) { 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 f8095da539..a5af96c508 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 @@ -17,6 +17,7 @@ package com.linecorp.centraldogma.server.internal.api.converter; import static com.google.common.base.Preconditions.checkArgument; +import static com.linecorp.centraldogma.internal.Util.isJson5; import java.lang.reflect.ParameterizedType; import java.util.List; @@ -89,6 +90,9 @@ private static Change readChange(JsonNode node) { return Change.ofTextUpsert(path, node.get("content").textValue()); } if (changeType == ChangeType.UPSERT_JSON) { + if (isJson5(path)) { + return Change.ofJsonUpsert(path, node.get("content").asText()); + } return Change.ofJsonUpsert(path, node.get("content")); } if (changeType == ChangeType.REMOVE) { diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/GitMirror.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/GitMirror.java index bf69926cbf..950dc3b0f5 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/GitMirror.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/GitMirror.java @@ -23,7 +23,6 @@ import java.io.File; import java.io.IOException; import java.net.URI; -import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; import java.util.regex.Pattern; @@ -52,7 +51,6 @@ import com.cronutils.model.Cron; import com.fasterxml.jackson.core.TreeNode; -import com.fasterxml.jackson.databind.JsonNode; import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.Entry; @@ -240,12 +238,10 @@ protected void mirrorRemoteToLocal(File workDir, CommandExecutor executor, final byte[] content = reader.open(objectId).getBytes(); switch (EntryType.guessFromPath(localPath)) { case JSON: - final JsonNode jsonNode = Jackson.readTree(content); - changes.putIfAbsent(localPath, Change.ofJsonUpsert(localPath, jsonNode)); + changes.putIfAbsent(localPath, Change.ofJsonUpsert(localPath, content)); break; case TEXT: - final String strVal = new String(content, StandardCharsets.UTF_8); - changes.putIfAbsent(localPath, Change.ofTextUpsert(localPath, strVal)); + changes.putIfAbsent(localPath, Change.ofTextUpsert(localPath, content)); break; } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepository.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepository.java index 3f9e5970d8..d970d6aaae 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepository.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepository.java @@ -18,6 +18,7 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkState; +import static com.linecorp.centraldogma.internal.Util.isJson5; import static com.linecorp.centraldogma.server.internal.storage.repository.git.FailFastUtil.context; import static com.linecorp.centraldogma.server.internal.storage.repository.git.FailFastUtil.failFastIfTimedOut; import static java.nio.charset.StandardCharsets.UTF_8; @@ -93,6 +94,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.JsonNodeFactory; import com.google.common.annotations.VisibleForTesting; @@ -115,6 +117,7 @@ import com.linecorp.centraldogma.common.RevisionNotFoundException; import com.linecorp.centraldogma.common.RevisionRange; import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Json5; import com.linecorp.centraldogma.internal.Util; import com.linecorp.centraldogma.internal.jsonpatch.JsonPatch; import com.linecorp.centraldogma.internal.jsonpatch.ReplaceMode; @@ -497,8 +500,7 @@ private Map> blockingFind( final byte[] content = reader.open(treeWalk.getObjectId(0)).getBytes(); switch (entryType) { case JSON: - final JsonNode jsonNode = Jackson.readTree(content); - entry = Entry.ofJson(normRevision, path, jsonNode); + entry = Entry.ofJson(normRevision, path, content); break; case TEXT: final String strVal = sanitizeText(new String(content, UTF_8)); @@ -751,7 +753,10 @@ private Map> toChangeMap(List diffEntryList) { switch (diffEntry.getChangeType()) { case MODIFY: - final EntryType oldEntryType = EntryType.guessFromPath(oldPath); + // Resolve JSON5 as EntryType.TEXT to modify it with text patch because + // applying json patch to JSON5 file makes it a plain JSON. + final EntryType oldEntryType = !isJson5(oldPath) ? EntryType.guessFromPath(oldPath) + : EntryType.TEXT; switch (oldEntryType) { case JSON: if (!oldPath.equals(newPath)) { @@ -793,13 +798,16 @@ private Map> toChangeMap(List diffEntryList) { } break; case ADD: - final EntryType newEntryType = EntryType.guessFromPath(newPath); + // Resolve JSON5 as EntryType.TEXT so that the JSON5 content can properly be included + // in the serialized ReplicationLog. + final EntryType newEntryType = !isJson5(newPath) ? EntryType.guessFromPath(newPath) + : EntryType.TEXT; switch (newEntryType) { case JSON: { - final JsonNode jsonNode = Jackson.readTree( - reader.open(diffEntry.getNewId().toObjectId()).getBytes()); + final String jsonText = new String( + reader.open(diffEntry.getNewId().toObjectId()).getBytes(), UTF_8); - putChange(changeMap, newPath, Change.ofJsonUpsert(newPath, jsonNode)); + putChange(changeMap, newPath, Change.ofJsonUpsert(newPath, jsonText)); break; } case TEXT: { @@ -1001,17 +1009,22 @@ private int applyChanges(@Nullable Revision baseRevision, @Nullable ObjectId bas switch (change.type()) { case UPSERT_JSON: { - final JsonNode oldJsonNode = oldContent != null ? Jackson.readTree(oldContent) : null; - final JsonNode newJsonNode = firstNonNull((JsonNode) change.content(), - JsonNodeFactory.instance.nullNode()); - - // Upsert only when the contents are really different. - if (!Objects.equals(newJsonNode, oldJsonNode)) { - applyPathEdit(dirCache, new InsertJson(changePath, inserter, newJsonNode)); - numEdits++; + if (!isJson5(changePath)) { + final JsonNode oldJsonNode = oldContent != null ? Jackson.readTree(oldContent) + : null; + final JsonNode newJsonNode = firstNonNull((JsonNode) change.content(), + JsonNodeFactory.instance.nullNode()); + + // Upsert only when the contents are really different. + if (!Objects.equals(newJsonNode, oldJsonNode)) { + applyPathEdit(dirCache, new InsertJson(changePath, inserter, newJsonNode)); + numEdits++; + } + break; } - break; } + // Fall-through to UPSERT_TEXT for JSON5 entries so that JSON5 format is preserved. + /* fall-thru */ case UPSERT_TEXT: { final String sanitizedOldText; if (oldContent != null) { @@ -1079,7 +1092,8 @@ private int applyChanges(@Nullable Revision baseRevision, @Nullable ObjectId bas case APPLY_JSON_PATCH: { final JsonNode oldJsonNode; if (oldContent != null) { - oldJsonNode = Jackson.readTree(oldContent); + oldJsonNode = isJson5(changePath) ? Json5.readTree(oldContent) + : Jackson.readTree(oldContent); } else { oldJsonNode = Jackson.nullNode; } @@ -1128,6 +1142,15 @@ private int applyChanges(@Nullable Revision baseRevision, @Nullable ObjectId bas throw new ChangeConflictException("failed to apply text patch: " + change, e); } + // JSON5 validation since JSON5 is eventually patched via text patch. + if (isJson5(change.path())) { + try { + Json5.readTree(newText); + } catch (JsonParseException e) { + throw new ChangeConflictException( + "failed to apply patch to JSON5 file: " + change, e); + } + } // Apply only when the contents are really different. if (!newText.equals(sanitizedOldText)) { applyPathEdit(dirCache, new InsertText(changePath, inserter, newText)); diff --git a/server/src/main/resources/webapp/scripts/app/entities/repositories/repository.file.controller.js b/server/src/main/resources/webapp/scripts/app/entities/repositories/repository.file.controller.js index 1f3e7a32de..403b342407 100644 --- a/server/src/main/resources/webapp/scripts/app/entities/repositories/repository.file.controller.js +++ b/server/src/main/resources/webapp/scripts/app/entities/repositories/repository.file.controller.js @@ -57,7 +57,7 @@ angular.module('CentralDogmaAdmin') RepositoryService.getFile($scope.project.name, $scope.repository.name, $scope.revision, {path: $scope.path}).then( function (file) { - if (file.type === 'JSON') { + if (file.type === 'JSON' && !StringUtil.endsWith(file.path, '.json5')) { file.content = JSON.stringify(JSON.parse(file.content), null, 2) + '\n'; } $scope.file = file; diff --git a/server/src/main/resources/webapp/scripts/app/entities/repositories/repository.file.new.controller.js b/server/src/main/resources/webapp/scripts/app/entities/repositories/repository.file.new.controller.js index 11981001bb..bff4eabc1e 100644 --- a/server/src/main/resources/webapp/scripts/app/entities/repositories/repository.file.new.controller.js +++ b/server/src/main/resources/webapp/scripts/app/entities/repositories/repository.file.new.controller.js @@ -129,11 +129,13 @@ angular.module('CentralDogmaAdmin') JSON.parse($scope.file.content); } catch (error) { NotificationUtil.error('entities.invalid_json'); - $timeout(function() { + $timeout(function () { $scope.editor.focus(); }); return; } + } else if (StringUtil.endsWith($scope.file.name.toLowerCase(), '.json5')) { + $scope.file.type = 'JSON'; } else { $scope.file.type = 'TEXT'; } @@ -144,6 +146,8 @@ angular.module('CentralDogmaAdmin') NotificationUtil.success('entities.saved_file', { path: $scope.file.path }); $scope.back(); + }, function (error) { + NotificationUtil.error(error); }); }; }); diff --git a/server/src/main/resources/webapp/scripts/components/util/ace-editor.directive.js b/server/src/main/resources/webapp/scripts/components/util/ace-editor.directive.js index b3a13ce3f4..c989af9358 100644 --- a/server/src/main/resources/webapp/scripts/components/util/ace-editor.directive.js +++ b/server/src/main/resources/webapp/scripts/components/util/ace-editor.directive.js @@ -131,7 +131,7 @@ angular.module('CentralDogmaAdmin') var modelist = window.ace.require('ace/ext/modelist'); var mode = modelist.getModeForPath(value).mode; session.setMode(mode); - if (mode === 'ace/mode/json') { + if (mode === 'ace/mode/json' || mode === 'ace/mode/json5') { session.setTabSize(2); } else { session.setTabSize(4); 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 05a1531415..57b9767e34 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 @@ -47,6 +47,7 @@ import com.linecorp.centraldogma.common.InvalidPushException; import com.linecorp.centraldogma.common.RedundantChangeException; import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Json5; import com.linecorp.centraldogma.server.CentralDogmaBuilder; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; @@ -304,6 +305,32 @@ void editFileWithPost() throws IOException { assertThat(content1.get(0).toString()).isEqualToIgnoringCase( "{\"op\":\"safeReplace\",\"path\":\"/a\",\"oldValue\":\"bar\",\"value\":\"baz\"}"); + addFooJson5(client); + final String editJson5Body = + '{' + + " \"path\": \"/foo.json5\"," + + " \"type\": \"UPSERT_JSON\"," + + " \"content\": \"{a: 'baz'}\"," + + " \"commitMessage\": {" + + " \"summary\": \"Edit foo.json5\"," + + " \"detail\": \"Edit because we need it.\"," + + " \"markup\": \"PLAINTEXT\"" + + " }" + + '}'; + client.execute(RequestHeaders.of(HttpMethod.POST, CONTENTS_PREFIX, + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON), + editJson5Body).aggregate().join(); + + // check whether the change is right + final AggregatedHttpResponse res2 = client + .get("/api/v1/projects/myPro/repos/myRepo/compare?from=4&to=5").aggregate().join(); + final JsonNode content2 = Jackson.readTree(res2.contentUtf8()).get(0).get("content"); + assertThat(content2.textValue()).isEqualToIgnoringCase("--- /foo.json5\n" + + "+++ /foo.json5\n" + + "@@ -1,1 +1,1 @@\n" + + "-{a: 'bar'}\n" + + "+{a: 'baz'}"); + addBarTxt(client); final String editTextBody = '{' + @@ -321,10 +348,10 @@ void editFileWithPost() throws IOException { editTextBody).aggregate().join(); // check whether the change is right - final AggregatedHttpResponse res2 = client - .get("/api/v1/projects/myPro/repos/myRepo/compare?from=4&to=5").aggregate().join(); - final JsonNode content2 = Jackson.readTree(res2.contentUtf8()).get(0).get("content"); - assertThat(content2.textValue()).isEqualToIgnoringCase("--- /a/bar.txt\n" + + final AggregatedHttpResponse res3 = client + .get("/api/v1/projects/myPro/repos/myRepo/compare?from=6&to=7").aggregate().join(); + final JsonNode content3 = Jackson.readTree(res3.contentUtf8()).get(0).get("content"); + assertThat(content3.textValue()).isEqualToIgnoringCase("--- /a/bar.txt\n" + "+++ /a/bar.txt\n" + "@@ -1,1 +1,1 @@\n" + "-text in the file.\n" + @@ -335,6 +362,7 @@ void editFileWithPost() throws IOException { void editFiles() { final WebClient client = dogma.httpClient(); addFooJson(client); + addFooJson5(client); addBarTxt(client); final String body = '{' + @@ -350,6 +378,11 @@ void editFiles() { " \"content\" : {\"b\": \"bar\"}" + " }," + " {" + + " \"path\" : \"/foo.json5\"," + + " \"type\" : \"UPSERT_JSON\"," + + " \"content\" : \"{b: 'bar'}\"" + + " }," + + " {" + " \"path\" : \"/a/bar.txt\"," + " \"type\" : \"UPSERT_TEXT\"," + " \"content\" : \"text in a file.\\n\"" + @@ -360,7 +393,7 @@ void editFiles() { HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); client.execute(headers, body).aggregate().join(); final AggregatedHttpResponse aRes = client - .get("/api/v1/projects/myPro/repos/myRepo/compare?from=3&to=4").aggregate().join(); + .get("/api/v1/projects/myPro/repos/myRepo/compare?from=4&to=5").aggregate().join(); final String expectedJson = '[' + " {" + @@ -383,6 +416,15 @@ void editFiles() { " \"path\": \"/b\"," + " \"value\": \"bar\"" + " }]" + + " }," + + " {" + + " \"path\": \"/foo.json5\"," + + " \"type\": \"APPLY_TEXT_PATCH\"," + + " \"content\": \"--- /foo.json5\\n" + + "+++ /foo.json5\\n" + + "@@ -1,1 +1,1 @@\\n" + + "-{a: 'bar'}\\n" + + "+{b: 'bar'}\"" + " }" + ']'; final String actualJson = aRes.contentUtf8(); @@ -393,9 +435,9 @@ void editFiles() { void getFile() { final WebClient client = dogma.httpClient(); addFooJson(client); - final AggregatedHttpResponse aRes = client.get(CONTENTS_PREFIX + "/foo.json").aggregate().join(); + final AggregatedHttpResponse aRes1 = client.get(CONTENTS_PREFIX + "/foo.json").aggregate().join(); - final String expectedJson = + final String expectedJson1 = '{' + " \"revision\": 2," + " \"path\": \"/foo.json\"," + @@ -403,19 +445,33 @@ void getFile() { " \"content\" : {\"a\":\"bar\"}," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json\"" + '}'; - final String actualJson = aRes.contentUtf8(); - assertThatJson(actualJson).isEqualTo(expectedJson); + final String actualJson1 = aRes1.contentUtf8(); + assertThatJson(actualJson1).isEqualTo(expectedJson1); + + addFooJson5(client); + final AggregatedHttpResponse aRes2 = client.get(CONTENTS_PREFIX + "/foo.json5").aggregate().join(); + + final String expectedJson2 = + '{' + + " \"revision\": 3," + + " \"path\": \"/foo.json5\"," + + " \"type\": \"JSON\"," + + " \"content\": \"{a: 'bar'}\\n\"," + + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json5\"" + + '}'; + final String actualJson2 = aRes2.contentUtf8(); + assertThatJson(actualJson2).isEqualTo(expectedJson2); } @Test void getFileWithJsonPath() { final WebClient client = dogma.httpClient(); addFooJson(client); - final AggregatedHttpResponse aRes = client + final AggregatedHttpResponse aRes1 = client .get(CONTENTS_PREFIX + "/foo.json?jsonpath=$[?(@.a == \"bar\")]&jsonpath=$[0].a") .aggregate().join(); - final String expectedJson = + final String expectedJson1 = '{' + " \"revision\": 2," + " \"path\": \"/foo.json\"," + @@ -423,14 +479,31 @@ void getFileWithJsonPath() { " \"content\" : \"bar\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json\"" + '}'; - final String actualJson = aRes.contentUtf8(); - assertThatJson(actualJson).isEqualTo(expectedJson); + final String actualJson1 = aRes1.contentUtf8(); + assertThatJson(actualJson1).isEqualTo(expectedJson1); + + addFooJson5(client); + final AggregatedHttpResponse aRes2 = client + .get(CONTENTS_PREFIX + "/foo.json5?jsonpath=$[?(@.a == \"bar\")]&jsonpath=$[0].a") + .aggregate().join(); + + final String expectedJson2 = + '{' + + " \"revision\": 3," + + " \"path\": \"/foo.json5\"," + + " \"type\": \"JSON\"," + + " \"content\": \"bar\"," + + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json5\"" + + '}'; + final String actualJson2 = aRes2.contentUtf8(); + assertThatJson(actualJson2).isEqualTo(expectedJson2); } @Test void listFiles() { final WebClient client = dogma.httpClient(); addFooJson(client); + addFooJson5(client); addBarTxt(client); // get the list of all files final AggregatedHttpResponse res1 = client @@ -438,22 +511,28 @@ void listFiles() { final String expectedJson1 = '[' + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/a\"," + " \"type\": \"DIRECTORY\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/a\"" + " }," + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/a/bar.txt\"," + " \"type\": \"TEXT\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/a/bar.txt\"" + " }," + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/foo.json\"," + " \"type\": \"JSON\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json\"" + + " }," + + " {" + + " \"revision\": 4," + + " \"path\": \"/foo.json5\"," + + " \"type\": \"JSON\"," + + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json5\"" + " }" + ']'; assertThatJson(res1.contentUtf8()).isEqualTo(expectedJson1); @@ -464,16 +543,22 @@ void listFiles() { final String expectedJson2 = '[' + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/a\"," + " \"type\": \"DIRECTORY\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/a\"" + " }," + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/foo.json\"," + " \"type\": \"JSON\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json\"" + + " }," + + " {" + + " \"revision\": 4," + + " \"path\": \"/foo.json5\"," + + " \"type\": \"JSON\"," + + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json5\"" + " }" + ']'; assertThatJson(res2.contentUtf8()).isEqualTo(expectedJson2); @@ -498,7 +583,7 @@ void listFiles() { final String expectedJson4 = '[' + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/foo.json\"," + " \"type\": \"JSON\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json\"" + @@ -511,6 +596,7 @@ void listFiles() { void listFilesWithContent() { final WebClient client = dogma.httpClient(); addFooJson(client); + addFooJson5(client); addBarTxt(client); // get the list of all files @@ -518,24 +604,31 @@ void listFilesWithContent() { final String expectedJson1 = '[' + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/a\"," + " \"type\": \"DIRECTORY\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/a\"" + " }," + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/a/bar.txt\"," + " \"type\": \"TEXT\"," + " \"content\" : \"text in the file.\\n\"," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/a/bar.txt\"" + " }," + " {" + - " \"revision\": 3," + + " \"revision\": 4," + " \"path\": \"/foo.json\"," + " \"type\": \"JSON\"," + " \"content\" : {\"a\":\"bar\"}," + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json\"" + + " }," + + " {" + + " \"revision\": 4," + + " \"path\": \"/foo.json5\"," + + " \"type\": \"JSON\"," + + " \"content\": \"{a: 'bar'}\\n\"," + + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json5\"" + " }" + ']'; assertThatJson(res1.contentUtf8()).isEqualTo(expectedJson1); @@ -621,6 +714,24 @@ void editFileWithJsonPatch() throws IOException { .isEqualToIgnoringCase("baz"); } + @Test + void editJson5FileWithJsonPatch() throws IOException { + final WebClient client = dogma.httpClient(); + addFooJson5(client); + final AggregatedHttpResponse res1 = editFooJson5WithJsonPatch(client); + final String expectedJson = + '{' + + " \"revision\": 3," + + " \"pushedAt\": \"${json-unit.ignore}\"" + + '}'; + final String actualJson = res1.contentUtf8(); + assertThatJson(actualJson).isEqualTo(expectedJson); + + final AggregatedHttpResponse res2 = client.get(CONTENTS_PREFIX + "/foo.json5").aggregate().join(); + final String json5Content = Jackson.readTree(res2.contentUtf8()).get("content").textValue(); + assertThat(Json5.readTree(json5Content).get("a").textValue()).isEqualToIgnoringCase("baz"); + } + @Test void editFileWithTextPatch() throws IOException { final WebClient client = dogma.httpClient(); @@ -657,6 +768,72 @@ void editFileWithTextPatch() throws IOException { .isEqualTo("text in some file.\n"); } + @Test + void editJson5FileWithTextPatch() throws IOException { + final WebClient client = dogma.httpClient(); + addFooJson5(client); + final String patch = + '{' + + " \"path\": \"/foo.json5\"," + + " \"type\": \"APPLY_TEXT_PATCH\"," + + " \"content\" : \"--- /foo.json5\\n" + + "+++ /foo.json5\\n" + + "@@ -1,1 +1,1 @@\\n" + + "-{a: 'bar'}\\n" + + "+{a: 'baz'}\\n\"," + + " \"commitMessage\" : {" + + " \"summary\" : \"Edit foo.json5\"," + + " \"detail\": \"Edit because we need it.\"," + + " \"markup\": \"PLAINTEXT\"" + + " }" + + '}'; + + final RequestHeaders headers = RequestHeaders.of(HttpMethod.POST, CONTENTS_PREFIX, + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + final AggregatedHttpResponse res1 = client.execute(headers, patch).aggregate().join(); + final String expectedJson = + '{' + + " \"revision\": 3," + + " \"pushedAt\": \"${json-unit.ignore}\"" + + '}'; + final String actualJson = res1.contentUtf8(); + assertThatJson(actualJson).isEqualTo(expectedJson); + + final AggregatedHttpResponse res2 = client.get(CONTENTS_PREFIX + "/foo.json5").aggregate().join(); + assertThat(Jackson.readTree(res2.contentUtf8()).get("content").textValue()) + .isEqualTo("{a: 'baz'}\n"); + } + + @Test + void editJson5FileWithTextPatch_invalidSyntax() { + final WebClient client = dogma.httpClient(); + addFooJson5(client); + final String patch = + '{' + + " \"path\": \"/foo.json5\"," + + " \"type\": \"APPLY_TEXT_PATCH\"," + + " \"content\": \"--- /foo.json5\\n" + + "+++ foo.json5\\n" + + "@@ -1,1 +1,1 @@\\n" + + "-{a: 'bar'}\\n" + + "+{a: new_bar}\\n\"," + // Invalid JSON5 + " \"commitMessage\": {" + + " \"summary\": \"Edit foo.json5\"," + + " \"detail\": \"Edit because we need it.\"," + + " \"markup\": \"PLAINTEXT\"" + + " }" + + '}'; + + final RequestHeaders headers = RequestHeaders.of(HttpMethod.POST, CONTENTS_PREFIX, + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + final AggregatedHttpResponse res = client.execute(headers, patch).aggregate().join(); + assertThatJson(res.contentUtf8()).isEqualTo( + '{' + + " \"exception\": \"" + ChangeConflictException.class.getName() + "\"," + + " \"message\": \"${json-unit.ignore}\"" + + '}'); + } + @Test void watchRepository() { final WebClient client = dogma.httpClient(); @@ -715,6 +892,39 @@ void watchFileWithIdentityQuery() { assertThatJson(actualJson).isEqualTo(expectedJson); } + @Test + void watchJson5FileWithIdentityQuery() { + final WebClient client = dogma.httpClient(); + addFooJson5(client); + final RequestHeaders headers = RequestHeaders.of(HttpMethod.GET, CONTENTS_PREFIX + "/foo.json5", + HttpHeaderNames.IF_NONE_MATCH, "-1"); + final CompletableFuture future = client.execute(headers).aggregate(); + + assertThatThrownBy(() -> future.get(500, TimeUnit.MILLISECONDS)) + .isExactlyInstanceOf(TimeoutException.class); + + // An irrelevant change should not trigger a notification. + addBarTxt(client); + assertThatThrownBy(() -> future.get(500, TimeUnit.MILLISECONDS)) + .isExactlyInstanceOf(TimeoutException.class); + + editFooJson5WithTextPatch(client); + final String expectedJson = + '{' + + " \"revision\": 4," + + " \"entry\": {" + + " \"revision\": 4," + + " \"path\": \"/foo.json5\"," + + " \"type\": \"JSON\"," + + " \"content\": \"{a: 'baz'}\\n\"," + + " \"url\": \"/api/v1/projects/myPro/repos/myRepo/contents/foo.json5\"" + + " }" + + '}'; + final AggregatedHttpResponse res = future.join(); + final String actualJson = res.contentUtf8(); + assertThatJson(actualJson).isEqualTo(expectedJson); + } + @Test void watchFileWithJsonPathQuery() { final WebClient client = dogma.httpClient(); @@ -819,6 +1029,23 @@ static AggregatedHttpResponse addFooJson(WebClient client) { return client.execute(headers, body).aggregate().join(); } + static AggregatedHttpResponse addFooJson5(WebClient client) { + final String body = + '{' + + " \"path\" : \"/foo.json5\"," + + " \"type\" : \"UPSERT_JSON\"," + + " \"content\" : \"{a: 'bar'}\"," + + " \"commitMessage\" : {" + + " \"summary\" : \"Add foo.json5\"," + + " \"detail\" : \" Add because we need it.\"," + + " \"markup\" : \"PLAINTEXT\"" + + " }" + + '}'; + final RequestHeaders headers = RequestHeaders.of(HttpMethod.POST, CONTENTS_PREFIX, + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + return client.execute(headers, body).aggregate().join(); + } + static AggregatedHttpResponse editFooJson(WebClient client) { final String body = '{' + @@ -842,6 +1069,51 @@ static AggregatedHttpResponse editFooJson(WebClient client) { return client.execute(headers, body).aggregate().join(); } + static AggregatedHttpResponse editFooJson5WithTextPatch(WebClient client) { + final String body = + '{' + + " \"path\": \"/foo.json5\"," + + " \"type\": \"APPLY_TEXT_PATCH\"," + + " \"content\": \"--- /foo.json5\\n" + + "+++ /foo.json5\\n" + + "@@ -1,1 +1,1 @@\\n" + + "-{a: 'bar'}\\n" + + "+{a: 'baz'}\"," + + " \"commitMessage\": {" + + " \"summary\": \"Edit foo.json5\"," + + " \"detail\": \"Edit because we need it.\"," + + " \"markup\": \"PLAINTEXT\"" + + " }" + + '}'; + + final RequestHeaders headers = RequestHeaders.of(HttpMethod.POST, CONTENTS_PREFIX, + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + return client.execute(headers, body).aggregate().join(); + } + + static AggregatedHttpResponse editFooJson5WithJsonPatch(WebClient client) { + final String body = + '{' + + " \"path\" : \"/foo.json5\"," + + " \"type\" : \"APPLY_JSON_PATCH\"," + + " \"content\" : [{" + + " \"op\" : \"safeReplace\"," + + " \"path\": \"/a\"," + + " \"oldValue\": \"bar\"," + + " \"value\": \"baz\"" + + " }]," + + " \"commitMessage\" : {" + + " \"summary\" : \"Edit foo.json5\"," + + " \"detail\": \"Edit because we need it.\"," + + " \"markup\": \"PLAINTEXT\"" + + " }" + + '}'; + + final RequestHeaders headers = RequestHeaders.of(HttpMethod.POST, CONTENTS_PREFIX, + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON); + return client.execute(headers, body).aggregate().join(); + } + private static AggregatedHttpResponse addBarTxt(WebClient client) { final String body = '{' + diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/MergeFileTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/MergeFileTest.java index a56a38cdf7..3e5466c62f 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/MergeFileTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/MergeFileTest.java @@ -56,8 +56,10 @@ void mergeJsonFiles() { addFilesForMergeJson(client); // The property "a" in "/foo.json" is overwritten by the property "a" in "/foo2.json" + //// and the property "b" in "/foo.json" is overwritten by the property "b" in "/foo1.json5". + //// and the property "c" in "/foo1.json5" is overwritten by the property "c" in "/foo2.json". String queryString = "path=/foo.json" + '&' + - "path=/foo1.json" + '&' + + "path=/foo1.json5" + '&' + "path=/foo2.json" + '&' + "optional_path=/foo3.json"; @@ -70,14 +72,15 @@ void mergeJsonFiles() { " \"type\" : \"JSON\"," + " \"content\" : {" + " \"a\" : \"new_bar\"," + - " \"b\" : \"baz\" " + + " \"b\" : \"new_baz\"," + + " \"c\" : \"new_qux\"" + " }," + - " \"paths\" : [\"/foo.json\", \"/foo1.json\", \"/foo2.json\"] " + + " \"paths\" : [\"/foo.json\", \"/foo1.json5\", \"/foo2.json\"] " + '}'; assertThatJson(aRes.contentUtf8()).isEqualTo(expectedJson); queryString = "path=/foo.json" + '&' + - "path=/foo1.json" + '&' + + "path=/foo1.json5" + '&' + "path=/foo2.json" + '&' + "path=/foo3.json"; aRes = client.get("/api/v1/projects/myPro/repos/myRepo/merge?" + queryString).aggregate().join(); @@ -113,9 +116,9 @@ void mergeJsonPaths() { final WebClient client = dogma.httpClient(); addFilesForMergeJson(client); String queryString = "path=/foo.json" + '&' + - "path=/foo1.json" + '&' + + "path=/foo1.json5" + '&' + "path=/foo2.json" + '&' + - "jsonpath=$[?(@.b == \"baz\")]&jsonpath=$[0].b"; + "jsonpath=$[?(@.b == \"new_baz\")]&jsonpath=$[0].b"; AggregatedHttpResponse aRes = client.get("/api/v1/projects/myPro/repos/myRepo/merge?" + queryString).aggregate().join(); @@ -123,21 +126,21 @@ void mergeJsonPaths() { '{' + " \"revision\" : 4," + " \"type\" : \"JSON\"," + - " \"content\" : \"baz\"," + - " \"paths\" : [\"/foo.json\", \"/foo1.json\", \"/foo2.json\"] " + + " \"content\" : \"new_baz\"," + + " \"paths\" : [\"/foo.json\", \"/foo1.json5\", \"/foo2.json\"] " + '}'; assertThatJson(aRes.contentUtf8()).isEqualTo(expectedJson); queryString = "path=/foo.json" + '&' + - "path=/foo1.json" + '&' + + "path=/foo1.json5" + '&' + "path=/foo2.json" + '&' + - "jsonpath=$.c"; + "jsonpath=$.d"; aRes = client.get("/api/v1/projects/myPro/repos/myRepo/merge?" + queryString).aggregate().join(); assertThat(aRes.status()).isEqualTo(HttpStatus.BAD_REQUEST); expectedJson = '{' + " \"exception\": \"com.linecorp.centraldogma.common.QueryExecutionException\"," + - " \"message\": \"JSON path evaluation failed: $.c\"" + + " \"message\": \"JSON path evaluation failed: $.d\"" + '}'; assertThatJson(aRes.contentUtf8()).isEqualTo(expectedJson); } @@ -181,7 +184,7 @@ void mismatchedValueWhileMerging() { client.execute(headers, body).aggregate().join(); final String queryString = "path=/foo.json" + '&' + - "path=/foo1.json" + '&' + + "path=/foo1.json5" + '&' + "path=/foo2.json" + '&' + "path=/foo10.json"; @@ -220,7 +223,7 @@ private static void addFilesForMergeJson(WebClient client) { '{' + " \"path\" : \"/foo.json\"," + " \"type\" : \"UPSERT_JSON\"," + - " \"content\" : {\"a\": \"bar\"}," + + " \"content\" : {\"a\": \"bar\", \"b\": \"baz\"}," + " \"commitMessage\" : {" + " \"summary\" : \"Add foo.json\"" + " }" + @@ -228,11 +231,11 @@ private static void addFilesForMergeJson(WebClient client) { client.execute(headers, body).aggregate().join(); body = '{' + - " \"path\" : \"/foo1.json\"," + + " \"path\" : \"/foo1.json5\"," + " \"type\" : \"UPSERT_JSON\"," + - " \"content\" : {\"b\": \"baz\"}," + + " \"content\" : \"{\\n // comments\\n b: 'new_baz',\\n c: 'qux',\\n}\\n\"," + " \"commitMessage\" : {" + - " \"summary\" : \"Add foo1.json\"" + + " \"summary\" : \"Add foo1.json5\"" + " }" + '}'; client.execute(headers, body).aggregate().join(); @@ -240,7 +243,7 @@ private static void addFilesForMergeJson(WebClient client) { '{' + " \"path\" : \"/foo2.json\"," + " \"type\" : \"UPSERT_JSON\"," + - " \"content\" : {\"a\": \"new_bar\"}," + + " \"content\" : {\"a\": \"new_bar\", \"c\": \"new_qux\"}," + " \"commitMessage\" : {" + " \"summary\" : \"Add foo3.json\"" + " }" + diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepositoryTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepositoryTest.java index 435d1d4336..3da62730b0 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepositoryTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepositoryTest.java @@ -18,6 +18,7 @@ import static com.linecorp.centraldogma.common.Revision.HEAD; import static com.linecorp.centraldogma.common.Revision.INIT; +import static com.linecorp.centraldogma.internal.Util.isJson5; import static java.util.concurrent.ForkJoinPool.commonPool; import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; @@ -59,6 +60,7 @@ import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.io.TempDir; +import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.TextNode; @@ -127,10 +129,13 @@ static void destroy() { private String prefix; private String allPattern; private final String[] jsonPaths = new String[NUM_ITERATIONS]; + private final String[] json5Paths = new String[NUM_ITERATIONS]; private final String[] textPaths = new String[NUM_ITERATIONS]; private final Change[] jsonUpserts = Util.unsafeCast(new Change[NUM_ITERATIONS]); + private final Change[] json5Upserts = Util.unsafeCast(new Change[NUM_ITERATIONS]); private final Change[] textUpserts = Util.unsafeCast(new Change[NUM_ITERATIONS]); private final Change[] jsonPatches = Util.unsafeCast(new Change[NUM_ITERATIONS]); + private final Change[] json5Patches = Util.unsafeCast(new Change[NUM_ITERATIONS]); private final Change[] textPatches = Util.unsafeCast(new Change[NUM_ITERATIONS]); @BeforeEach @@ -140,20 +145,27 @@ void setUp(TestInfo testInfo) { for (int i = 0; i < NUM_ITERATIONS; i++) { final String jsonPath = prefix + i + ".json"; + final String json5Path = prefix + i + ".json5"; final String textPath = prefix + i + ".txt"; jsonPaths[i] = jsonPath; + json5Paths[i] = json5Path; textPaths[i] = textPath; jsonUpserts[i] = Change.ofJsonUpsert(jsonPath, "{ \"" + i + "\": " + i + " }"); + json5Upserts[i] = Change.ofJsonUpsert(json5Path, "{ '" + i + "': '" + i + "' }"); textUpserts[i] = Change.ofTextUpsert(textPath, "value:\n" + i); } jsonPatches[0] = Change.ofJsonPatch(jsonPaths[0], null, jsonUpserts[0].content()); + // JSON5 patch is converted into text patch, internally. + json5Patches[0] = Change.ofTextPatch(json5Paths[0], null, json5Upserts[0].contentAsText()); textPatches[0] = Change.ofTextPatch(textPaths[0], null, textUpserts[0].content()); for (int i = 1; i < NUM_ITERATIONS; i++) { jsonPatches[i] = Change.ofJsonPatch(jsonPaths[0], jsonUpserts[i - 1].content(), jsonUpserts[i].content()); + json5Patches[i] = Change.ofTextPatch(json5Paths[0], json5Upserts[i - 1].contentAsText(), + json5Upserts[i].contentAsText()); textPatches[i] = Change.ofTextPatch(textPaths[0], textUpserts[i - 1].content(), textUpserts[i].content()); } @@ -162,16 +174,21 @@ void setUp(TestInfo testInfo) { } @Test - void testJsonUpsert() { + void testJsonUpsert() throws JsonParseException { testUpsert(jsonUpserts, EntryType.JSON); } @Test - void testTextUpsert() { + void testJson5Upsert() throws JsonParseException { + testUpsert(json5Upserts, EntryType.JSON); + } + + @Test + void testTextUpsert() throws JsonParseException { testUpsert(textUpserts, EntryType.TEXT); } - private void testUpsert(Change[] upserts, EntryType entryType) { + private void testUpsert(Change[] upserts, EntryType entryType) throws JsonParseException { final Revision oldHeadRev = repo.normalizeNow(HEAD); for (int i = 0; i < upserts.length; i++) { final Change change = upserts[i]; @@ -201,7 +218,9 @@ private void testUpsert(Change[] upserts, EntryType entryType) { assertThat(entries).containsEntry(path, Entry.of(headRev, path, EntryType.TEXT, c.content() + "\n")); } else { - assertThat(entries).containsEntry(path, Entry.of(headRev, path, entryType, c.content())); + // JSON5 text must also be sanitized so that the last line ends with \n. + // Ordinary JSON file ignores this internally in Entry#ofJson(...). + assertThat(entries).containsEntry(path, Entry.ofJson(headRev, path, c.contentAsText() + "\n")); } } } @@ -229,6 +248,21 @@ void testTextPatch() { testPatch(textPatches, textUpserts); } + @Test + void testJson5Patch() { + testPatch(json5Patches, json5Upserts); + + // JSON5 must be validated even though they are internally updated via text patch. + final String invalidJson5 = "{a: bar}"; // Invalid syntax + assertThatThrownBy( + () -> repo.commit(HEAD, 0L, Author.UNKNOWN, SUMMARY, + Change.ofTextPatch(json5Paths[0], + json5Upserts[json5Patches.length - 1].contentAsText(), + invalidJson5)).join()) + .isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(ChangeConflictException.class); + } + private static void testPatch(Change[] patches, Change[] upserts) { final String path = patches[0].path(); for (int i = 0; i < NUM_ITERATIONS; i++) { @@ -257,11 +291,16 @@ private static void testPatch(Change[] patches, Change[] upserts) { // Ensure the entry has been patched as expected. final Entry e = repo.get(HEAD, path).join(); - if (e.type() == EntryType.JSON) { - assertThatJson(e.content()).isEqualTo(upserts[i].content()); - } else { + if (e.type() == EntryType.TEXT) { // Text must be sanitized so that the last line ends with \n. assertThat(e.content()).isEqualTo(upserts[i].content() + "\n"); + } else { + if (isJson5(e)) { + // JSON5 is treated as text internally so must also be sanitized with \n. + assertThat(e.contentAsText()).isEqualTo(upserts[i].contentAsText() + "\n"); + } else { + assertThatJson(e.content()).isEqualTo(upserts[i].content()); + } } } }