From fa878be144a07cc0414cf395725d8341c7bf8014 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 15 Jan 2025 00:17:19 +0900 Subject: [PATCH 1/7] Expose JSON Patch operations as public API --- .../client/armeria/ArmeriaCentralDogma.java | 5 +- .../client/CentralDogmaRepository.java | 10 +- .../linecorp/centraldogma/common/Change.java | 38 +++ .../jsonpatch/AddOperation.java | 35 +-- .../jsonpatch/CopyOperation.java | 32 +- .../jsonpatch/DualPathOperation.java | 60 ++-- .../jsonpatch/JsonPatchException.java | 35 +-- .../common/jsonpatch/JsonPatchOperation.java | 293 ++++++++++++++++++ .../jsonpatch/MoveOperation.java | 32 +- .../jsonpatch/PathValueOperation.java | 57 ++-- .../jsonpatch/RemoveIfExistsOperation.java | 22 +- .../jsonpatch/RemoveOperation.java | 38 +-- .../jsonpatch/ReplaceOperation.java | 32 +- .../jsonpatch/SafeReplaceOperation.java | 62 +++- .../jsonpatch/TestAbsenceOperation.java | 29 +- .../jsonpatch/TestOperation.java | 37 +-- .../common/jsonpatch/package-info.java | 34 ++ .../internal/jsonpatch/DiffProcessor.java | 14 +- .../internal/jsonpatch/JsonNumEquals.java | 2 +- .../internal/jsonpatch/JsonPatch.java | 2 + .../jsonpatch/JsonPatchOperation.java | 163 ---------- .../internal/jsonpatch/package-info.java | 14 +- .../JsonPatchOperationIntegrationTest.java | 287 +++++++++++++++++ .../JsonPatchOperationSerializationTest.java | 10 + .../jsonpatch/JsonPatchOperationTest.java | 3 + .../internal/jsonpatch/JsonPatchTest.java | 3 + .../jsonpatch/JsonPatchTestSuite.java | 2 + .../RemoveIfExistsOperationTest.java | 4 +- .../jsonpatch/RemoveOperationTest.java | 4 +- .../internal/api/HttpApiExceptionHandler.java | 3 + .../internal/api/MetadataApiService.java | 4 +- .../repository/git/DefaultChangesApplier.java | 3 +- .../storage/repository/git/GitRepository.java | 10 +- .../server/metadata/MetadataService.java | 105 ++++--- .../server/metadata/TokenTest.java | 13 +- 35 files changed, 1017 insertions(+), 480 deletions(-) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/AddOperation.java (79%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/CopyOperation.java (70%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/DualPathOperation.java (67%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/JsonPatchException.java (52%) create mode 100644 common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/MoveOperation.java (79%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/PathValueOperation.java (72%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/RemoveIfExistsOperation.java (81%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/RemoveOperation.java (72%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/ReplaceOperation.java (66%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/SafeReplaceOperation.java (66%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/TestAbsenceOperation.java (69%) rename common/src/main/java/com/linecorp/centraldogma/{internal => common}/jsonpatch/TestOperation.java (57%) create mode 100644 common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java delete mode 100644 common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperation.java create mode 100644 common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java diff --git a/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java b/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java index 4e0888651d..49a6d02944 100644 --- a/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java +++ b/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java @@ -105,6 +105,7 @@ import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.common.RevisionNotFoundException; import com.linecorp.centraldogma.common.ShuttingDownException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; import com.linecorp.centraldogma.internal.HistoryConstants; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.internal.Util; @@ -137,6 +138,7 @@ public final class ArmeriaCentralDogma extends AbstractCentralDogma { .put(ReadOnlyException.class.getName(), ReadOnlyException::new) .put(MirrorException.class.getName(), MirrorException::new) .put(PermissionException.class.getName(), PermissionException::new) + .put(JsonPatchException.class.getName(), JsonPatchException::new) .build(); private final WebClient client; @@ -153,7 +155,8 @@ public ArmeriaCentralDogma(ScheduledExecutorService blockingTaskExecutor, @Override public CompletableFuture whenEndpointReady() { - return client.endpointGroup().whenReady().thenRun(() -> {}); + return client.endpointGroup().whenReady().thenRun(() -> { + }); } @Override diff --git a/client/java/src/main/java/com/linecorp/centraldogma/client/CentralDogmaRepository.java b/client/java/src/main/java/com/linecorp/centraldogma/client/CentralDogmaRepository.java index 2fd4683b14..b891541c54 100644 --- a/client/java/src/main/java/com/linecorp/centraldogma/client/CentralDogmaRepository.java +++ b/client/java/src/main/java/com/linecorp/centraldogma/client/CentralDogmaRepository.java @@ -55,11 +55,17 @@ CentralDogma centralDogma() { return centralDogma; } - String projectName() { + /** + * Returns the name of the project. + */ + public String projectName() { return projectName; } - String repositoryName() { + /** + * Returns the name of the repository. + */ + public String repositoryName() { return repositoryName; } 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..30aceaad5c 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/Change.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/Change.java @@ -36,7 +36,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.google.common.collect.ImmutableList; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.internal.Util; import com.linecorp.centraldogma.internal.jsonpatch.JsonPatch; @@ -223,6 +225,42 @@ static Change ofJsonPatch(String path, @Nullable JsonNode oldJsonNode, JsonPatch.generate(oldJsonNode, newJsonNode, ReplaceMode.SAFE).toJson()); } + /** + * Returns a newly-created {@link Change} whose type is {@link ChangeType#APPLY_JSON_PATCH}. + * + * @param path the path of the file + * @param jsonPatch the patch in JSON patch format + */ + static Change ofJsonPatch(String path, JsonPatchOperation jsonPatch) { + requireNonNull(path, "path"); + requireNonNull(jsonPatch, "jsonPatch"); + return new DefaultChange<>(path, ChangeType.APPLY_JSON_PATCH, jsonPatch.toJsonNode()); + } + + /** + * Returns a newly-created {@link Change} whose type is {@link ChangeType#APPLY_JSON_PATCH}. + * + * @param path the path of the file + * @param jsonPatches the list of patches in JSON patch format + */ + static Change ofJsonPatch(String path, JsonPatchOperation... jsonPatches) { + requireNonNull(jsonPatches, "jsonPatches"); + return ofJsonPatch(path, ImmutableList.copyOf(jsonPatches)); + } + + /** + * Returns a newly-created {@link Change} whose type is {@link ChangeType#APPLY_JSON_PATCH}. + * + * @param path the path of the file + * @param jsonPatches the list of patches in JSON patch format + */ + static Change ofJsonPatch(String path, Iterable jsonPatches) { + requireNonNull(path, "path"); + requireNonNull(jsonPatches, "jsonPatches"); + return new DefaultChange<>(path, ChangeType.APPLY_JSON_PATCH, + JsonPatchOperation.asJsonArray(jsonPatches)); + } + /** * Returns a newly-created {@link Change} whose type is {@link ChangeType#APPLY_JSON_PATCH}. * diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/AddOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java similarity index 79% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/AddOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java index e70b37496f..f6cf5a0a15 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/AddOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,26 +13,10 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -79,14 +63,19 @@ public final class AddOperation extends PathValueOperation { private static final String LAST_ARRAY_ELEMENT = "-"; + /** + * Creates a new instance with the specified {@code path} and {@code value}. + */ @JsonCreator - public AddOperation(@JsonProperty("path") final JsonPointer path, - @JsonProperty("value") final JsonNode value) { + AddOperation(@JsonProperty("path") final JsonPointer path, + @JsonProperty("value") final JsonNode value) { super("add", path, value); } @Override - JsonNode apply(final JsonNode node) { + public JsonNode apply(final JsonNode node) { + requireNonNull(node, "node"); + final JsonPointer path = path(); if (path.toString().isEmpty()) { return valueCopy(); } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/CopyOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java similarity index 70% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/CopyOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java index 5923b671d6..736d7aa901 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/CopyOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,26 +13,10 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -49,6 +33,9 @@ */ public final class CopyOperation extends DualPathOperation { + /** + * Creates a new instance. + */ @JsonCreator CopyOperation(@JsonProperty("from") final JsonPointer from, @JsonProperty("path") final JsonPointer path) { @@ -56,12 +43,15 @@ public final class CopyOperation extends DualPathOperation { } @Override - JsonNode apply(final JsonNode node) { + public JsonNode apply(final JsonNode node) { + requireNonNull(node, "node"); + final JsonPointer from = from(); JsonNode source = node.at(from); if (source.isMissingNode()) { throw new JsonPatchException("non-existent source path: " + from); } + final JsonPointer path = path(); if (path.toString().isEmpty()) { return source; } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/DualPathOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/DualPathOperation.java similarity index 67% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/DualPathOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/DualPathOperation.java index eaa1efd71b..174cba02b9 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/DualPathOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/DualPathOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,28 +13,13 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import java.io.IOException; +import java.util.Objects; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonPointer; @@ -49,7 +34,7 @@ abstract class DualPathOperation extends JsonPatchOperation { @JsonSerialize(using = ToStringSerializer.class) - final JsonPointer from; + private final JsonPointer from; /** * Creates a new instance. @@ -60,15 +45,23 @@ abstract class DualPathOperation extends JsonPatchOperation { */ DualPathOperation(final String op, final JsonPointer from, final JsonPointer path) { super(op, path); - this.from = from; + this.from = requireNonNull(from, "from"); + } + + /** + * Returns the source path. + */ + public JsonPointer from() { + return from; } @Override public final void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException { + requireNonNull(jgen, "jgen"); jgen.writeStartObject(); - jgen.writeStringField("op", op); - jgen.writeStringField("path", path.toString()); + jgen.writeStringField("op", op()); + jgen.writeStringField("path", path().toString()); jgen.writeStringField("from", from.toString()); jgen.writeEndObject(); } @@ -80,8 +73,25 @@ public final void serializeWithType(final JsonGenerator jgen, serialize(jgen, provider); } + @Override + public boolean equals(Object o) { + if (!(o instanceof DualPathOperation)) { + return false; + } + if (!super.equals(o)) { + return false; + } + final DualPathOperation that = (DualPathOperation) o; + return from.equals(that.from); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), from); + } + @Override public final String toString() { - return "op: " + op + "; from: \"" + from + "\"; path: \"" + path + '"'; + return "op: " + op() + "; from: \"" + from + "\"; path: \"" + path() + '"'; } } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchException.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchException.java similarity index 52% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchException.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchException.java index aeb102d06f..4423249cfb 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchException.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchException.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,35 +13,28 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; -public final class JsonPatchException extends RuntimeException { +import com.linecorp.centraldogma.common.CentralDogmaException; + +/** + * An exception raised when a JSON Patch operation fails. + */ +public final class JsonPatchException extends CentralDogmaException { private static final long serialVersionUID = 4746173383862473527L; + /** + * Creates a new instance. + */ public JsonPatchException(final String message) { super(message); } + /** + * Creates a new instance with the specified {@code cause}. + */ public JsonPatchException(final String message, final Throwable cause) { super(message, cause); } diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java new file mode 100644 index 0000000000..1e35ad677e --- /dev/null +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java @@ -0,0 +1,293 @@ +/* + * Copyright 2025 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.common.jsonpatch; + +import static com.fasterxml.jackson.annotation.JsonSubTypes.Type; +import static com.fasterxml.jackson.annotation.JsonTypeInfo.Id; +import static java.util.Objects.requireNonNull; + +import java.util.Objects; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.core.JsonPointer; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.JsonSerializable; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; + +import com.linecorp.centraldogma.internal.Jackson; + +/** + * Base abstract class for one JSON + * Patch operation. A {@link JsonPatchOperation} can be converted into a JSON patch by calling + * {@link #toJsonNode()}. + * + *

JSON + * Patch, as its name implies, is an IETF draft describing a mechanism to + * apply a patch to any JSON value. This implementation covers all operations + * according to the specification; however, there are some subtle differences + * with regards to some operations which are covered in these operations' + * respective documentation.

+ * + *

An example of a JSON Patch is as follows:

+ * + * 
+ *     [
+ *         {
+ *             "op": "add",
+ *             "path": "/-",
+ *             "value": {
+ *                 "productId": 19,
+ *                 "name": "Duvel",
+ *                 "type": "beer"
+ *             }
+ *         }
+ *     ]
+ *
+ * + *

This patch contains a single operation which adds an item at the end of + * an array. A JSON Patch can contain more than one operation; in this case, all + * operations are applied to the input JSON value in their order of appearance, + * until all operations are applied or an error condition is encountered.

+ */ +@JsonTypeInfo(use = Id.NAME, property = "op") +@JsonSubTypes({ + @Type(name = "add", value = AddOperation.class), + @Type(name = "copy", value = CopyOperation.class), + @Type(name = "move", value = MoveOperation.class), + @Type(name = "remove", value = RemoveOperation.class), + @Type(name = "removeIfExists", value = RemoveIfExistsOperation.class), + @Type(name = "replace", value = ReplaceOperation.class), + @Type(name = "safeReplace", value = SafeReplaceOperation.class), + @Type(name = "test", value = TestOperation.class), + @Type(name = "testAbsence", value = TestAbsenceOperation.class) +}) +@JsonIgnoreProperties(ignoreUnknown = true) +public abstract class JsonPatchOperation implements JsonSerializable { + + /** + * Creates a new JSON Patch {@code add} operation. + * + * @param path the JSON Pointer for this operation + * @param value the value to add + */ + public static AddOperation add(JsonPointer path, JsonNode value) { + return new AddOperation(path, value); + } + + /** + * Creates a new JSON Patch {@code copy} operation. + * + * @param from the source JSON Pointer + * @param path the destination JSON Pointer + */ + public static CopyOperation copy(JsonPointer from, JsonPointer path) { + return new CopyOperation(from, path); + } + + /** + * Creates a new JSON Patch {@code move} operation. + * + * @param from the source JSON Pointer + * @param path the destination JSON Pointer + */ + public static MoveOperation move(JsonPointer from, JsonPointer path) { + return new MoveOperation(from, path); + } + + /** + * Creates a new JSON Patch {@code remove} operation. + * + *

Note that this operation will throw an exception if the path does not exist. + * + * @param path the JSON Pointer to remove + */ + public static RemoveOperation remove(JsonPointer path) { + return new RemoveOperation(path); + } + + /** + * Creates a new JSON Patch {@code removeIfExists} operation. + * + * @param path the JSON Pointer to remove if it exists + */ + public static RemoveIfExistsOperation removeIfExists(JsonPointer path) { + return new RemoveIfExistsOperation(path); + } + + /** + * Creates a new JSON Patch {@code replace} operation. + * + * @param path the JSON Pointer for this operation + * @param value the new value to replace the existing value + */ + public static ReplaceOperation replace(JsonPointer path, JsonNode value) { + return new ReplaceOperation(path, value); + } + + /** + * Creates a new JSON Patch {@code safeReplace} operation. + * + * @param path the JSON Pointer for this operation + * @param oldValue the old value to replace + * @param newValue the new value to replace the old value + */ + public static SafeReplaceOperation safeReplace(JsonPointer path, JsonNode oldValue, JsonNode newValue) { + return new SafeReplaceOperation(path, oldValue, newValue); + } + + /** + * Creates a new JSON Patch {@code test} operation. + * + *

This operation will throw an exception if the value at the path does not match the expected value. + * + * @param path the JSON Pointer for this operation + * @param value the value to test + */ + public static TestOperation test(JsonPointer path, JsonNode value) { + return new TestOperation(path, value); + } + + /** + * Creates a new JSON Patch {@code testAbsent} operation. + * + *

This operation will throw an exception if the value at the path exists. + * + * @param path the JSON Pointer for this operation + */ + public static TestAbsenceOperation testAbsence(JsonPointer path) { + return new TestAbsenceOperation(path); + } + + /** + * Converts {@link JsonPatchOperation}s to an array of {@link JsonNode}. + */ + public static JsonNode asJsonArray(JsonPatchOperation... jsonPatchOperations) { + requireNonNull(jsonPatchOperations, "jsonPatchOperations"); + return Jackson.valueToTree(jsonPatchOperations); + } + + /** + * Converts {@link JsonPatchOperation}s to an array of {@link JsonNode}. + */ + public static JsonNode asJsonArray(Iterable jsonPatchOperations) { + requireNonNull(jsonPatchOperations, "jsonPatchOperations"); + return Jackson.valueToTree(jsonPatchOperations); + } + + private final String op; + + /* + * Note: no need for a custom deserializer, Jackson will try and find a + * constructor with a single string argument and use it. + * + * However, we need to serialize using .toString(). + */ + private final JsonPointer path; + + /** + * Creates a new instance. + * + * @param op the operation name + * @param path the JSON Pointer for this operation + */ + JsonPatchOperation(final String op, final JsonPointer path) { + this.op = requireNonNull(op, "op"); + this.path = requireNonNull(path, "path"); + } + + /** + * Returns the operation name. + */ + public final String op() { + return op; + } + + /** + * Returns the JSON Pointer for this operation. + */ + public final JsonPointer path() { + return path; + } + + /** + * Applies this operation to a JSON value. + * + * @param node the value to patch + * @return the patched value + * @throws JsonPatchException operation failed to apply to this value + */ + public abstract JsonNode apply(JsonNode node); + + /** + * Converts this {@link JsonPatchOperation} to a {@link JsonNode}. + */ + public JsonNode toJsonNode() { + return JsonNodeFactory.instance.arrayNode().add(Jackson.valueToTree(this)); + } + + JsonNode ensureExistence(JsonNode node) { + final JsonNode found = node.at(path); + if (found.isMissingNode()) { + throw new JsonPatchException("non-existent path: " + path); + } + return found; + } + + static JsonNode ensureSourceParent(JsonNode node, JsonPointer path) { + return ensureParent(node, path, "source"); + } + + static JsonNode ensureTargetParent(JsonNode node, JsonPointer path) { + return ensureParent(node, path, "target"); + } + + private static JsonNode ensureParent(JsonNode node, JsonPointer path, String typeName) { + /* + * Check the parent node: it must exist and be a container (ie an array + * or an object) for the add operation to work. + */ + final JsonPointer parentPath = path.head(); + final JsonNode parentNode = node.at(parentPath); + if (parentNode.isMissingNode()) { + throw new JsonPatchException("non-existent " + typeName + " parent: " + parentPath); + } + if (!parentNode.isContainerNode()) { + throw new JsonPatchException(typeName + " parent is not a container: " + parentPath + + " (" + parentNode.getNodeType() + ')'); + } + return parentNode; + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof JsonPatchOperation)) { + return false; + } + final JsonPatchOperation that = (JsonPatchOperation) o; + return op.equals(that.op) && path.equals(that.path); + } + + @Override + public int hashCode() { + return Objects.hash(op, path); + } + + @Override + public abstract String toString(); +} diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/MoveOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java similarity index 79% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/MoveOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java index ce4670265d..615ccc3715 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/MoveOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,26 +13,10 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -73,6 +57,9 @@ */ public final class MoveOperation extends DualPathOperation { + /** + * Creates a new instance. + */ @JsonCreator MoveOperation(@JsonProperty("from") final JsonPointer from, @JsonProperty("path") final JsonPointer path) { @@ -80,7 +67,10 @@ public final class MoveOperation extends DualPathOperation { } @Override - JsonNode apply(final JsonNode node) { + public JsonNode apply(final JsonNode node) { + requireNonNull(node, "node"); + final JsonPointer from = from(); + final JsonPointer path = path(); if (from.equals(path)) { return node; } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/PathValueOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java similarity index 72% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/PathValueOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java index fbb1a16bf7..70fed8c043 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/PathValueOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,28 +13,13 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import java.io.IOException; +import java.util.Objects; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonPointer; @@ -44,6 +29,8 @@ import com.fasterxml.jackson.databind.jsontype.TypeSerializer; import com.google.common.base.Equivalence; +import com.linecorp.centraldogma.internal.jsonpatch.JsonNumEquals; + /** * Base class for patch operations taking a value in addition to a path. */ @@ -63,9 +50,13 @@ abstract class PathValueOperation extends JsonPatchOperation { */ PathValueOperation(final String op, final JsonPointer path, final JsonNode value) { super(op, path); + requireNonNull(value, "value"); this.value = value.deepCopy(); } + /** + * Returns the JSON value. + */ public JsonNode value() { return value; } @@ -76,7 +67,7 @@ JsonNode valueCopy() { void ensureEquivalence(JsonNode actual) { if (!EQUIVALENCE.equivalent(actual, value)) { - throw new JsonPatchException("mismatching value at '" + path + "': " + + throw new JsonPatchException("mismatching value at '" + path() + "': " + actual + " (expected: " + value + ')'); } } @@ -84,9 +75,10 @@ void ensureEquivalence(JsonNode actual) { @Override public final void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException { + requireNonNull(jgen, "jgen"); jgen.writeStartObject(); - jgen.writeStringField("op", op); - jgen.writeStringField("path", path.toString()); + jgen.writeStringField("op", op()); + jgen.writeStringField("path", path().toString()); jgen.writeFieldName("value"); jgen.writeTree(value); jgen.writeEndObject(); @@ -99,8 +91,25 @@ public final void serializeWithType(final JsonGenerator jgen, serialize(jgen, provider); } + @Override + public boolean equals(Object o) { + if (!(o instanceof PathValueOperation)) { + return false; + } + if (!super.equals(o)) { + return false; + } + final PathValueOperation that = (PathValueOperation) o; + return value.equals(that.value); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), value); + } + @Override public final String toString() { - return "op: " + op + "; path: \"" + path + "\"; value: " + value; + return "op: " + op() + "; path: \"" + path() + "\"; value: " + value; } } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveIfExistsOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveIfExistsOperation.java similarity index 81% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveIfExistsOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveIfExistsOperation.java index 7ef9189c70..125cc3b7f7 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveIfExistsOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveIfExistsOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2018 LINE Corporation + * Copyright 2025 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 @@ -14,7 +14,9 @@ * under the License. */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import java.io.IOException; @@ -37,13 +39,18 @@ */ public final class RemoveIfExistsOperation extends JsonPatchOperation { + /** + * Creates a new instance. + */ @JsonCreator - public RemoveIfExistsOperation(@JsonProperty("path") final JsonPointer path) { + RemoveIfExistsOperation(@JsonProperty("path") final JsonPointer path) { super("removeIfExists", path); } @Override - JsonNode apply(final JsonNode node) { + public JsonNode apply(final JsonNode node) { + requireNonNull(node, "node"); + final JsonPointer path = path(); if (path.toString().isEmpty()) { return MissingNode.getInstance(); } @@ -66,9 +73,10 @@ JsonNode apply(final JsonNode node) { @Override public void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException { + requireNonNull(jgen, "jgen"); jgen.writeStartObject(); - jgen.writeStringField("op", "removeIfExists"); - jgen.writeStringField("path", path.toString()); + jgen.writeStringField("op", op()); + jgen.writeStringField("path", path().toString()); jgen.writeEndObject(); } @@ -81,6 +89,6 @@ public void serializeWithType(final JsonGenerator jgen, @Override public String toString() { - return "op: " + op + "; path: \"" + path + '"'; + return "op: " + op() + "; path: \"" + path() + '"'; } } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveOperation.java similarity index 72% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveOperation.java index 6ba0e3142b..8a4189c11c 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,26 +13,10 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import java.io.IOException; @@ -55,13 +39,18 @@ */ public final class RemoveOperation extends JsonPatchOperation { + /** + * Creates a new instance. + */ @JsonCreator - public RemoveOperation(@JsonProperty("path") final JsonPointer path) { + RemoveOperation(@JsonProperty("path") final JsonPointer path) { super("remove", path); } @Override - JsonNode apply(final JsonNode node) { + public JsonNode apply(final JsonNode node) { + requireNonNull(node, "node"); + final JsonPointer path = path(); if (path.toString().isEmpty()) { return MissingNode.getInstance(); } @@ -80,9 +69,10 @@ JsonNode apply(final JsonNode node) { @Override public void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException { + requireNonNull(jgen, "jgen"); jgen.writeStartObject(); jgen.writeStringField("op", "remove"); - jgen.writeStringField("path", path.toString()); + jgen.writeStringField("path", path().toString()); jgen.writeEndObject(); } @@ -95,6 +85,6 @@ public void serializeWithType(final JsonGenerator jgen, @Override public String toString() { - return "op: " + op + "; path: \"" + path + '"'; + return "op: " + op() + "; path: \"" + path() + '"'; } } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/ReplaceOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/ReplaceOperation.java similarity index 66% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/ReplaceOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/ReplaceOperation.java index 0c77940c3d..13b8c92f60 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/ReplaceOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/ReplaceOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,26 +13,10 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -53,15 +37,17 @@ public final class ReplaceOperation extends PathValueOperation { @JsonCreator - public ReplaceOperation(@JsonProperty("path") final JsonPointer path, - @JsonProperty("value") final JsonNode value) { + ReplaceOperation(@JsonProperty("path") final JsonPointer path, + @JsonProperty("value") final JsonNode value) { super("replace", path, value); } @Override - JsonNode apply(final JsonNode node) { + public JsonNode apply(final JsonNode node) { + requireNonNull(node, "node"); ensureExistence(node); + final JsonPointer path = path(); final JsonNode replacement = valueCopy(); if (path.toString().isEmpty()) { return replacement; diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/SafeReplaceOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java similarity index 66% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/SafeReplaceOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java index 56ed5d3f50..2a5f3bf6e2 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/SafeReplaceOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -14,9 +14,12 @@ * under the License. */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import java.io.IOException; +import java.util.Objects; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -30,6 +33,14 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.base.Equivalence; +import com.linecorp.centraldogma.internal.jsonpatch.JsonNumEquals; + +/** + * JSON Patch {@code safeReplace} operation. + * + *

This operation is similar to {@link ReplaceOperation}, but it throws an error if the path does not have + * the expected value.

+ */ public final class SafeReplaceOperation extends JsonPatchOperation { private static final Equivalence EQUIVALENCE = JsonNumEquals.getInstance(); @@ -39,18 +50,40 @@ public final class SafeReplaceOperation extends JsonPatchOperation { @JsonSerialize private final JsonNode newValue; + /** + * Creates a new instance. + */ @JsonCreator SafeReplaceOperation(@JsonProperty("path") final JsonPointer path, @JsonProperty("oldValue") JsonNode oldValue, @JsonProperty("value") JsonNode newValue) { super("safeReplace", path); + requireNonNull(oldValue, "oldValue"); + requireNonNull(newValue, "newValue"); this.oldValue = oldValue.deepCopy(); this.newValue = newValue.deepCopy(); } + /** + * Returns the old value to be replaced. + */ + public JsonNode oldValue() { + return oldValue; + } + + /** + * Returns the new value to replace the old value. + */ + public JsonNode newValue() { + return newValue; + } + @Override - JsonNode apply(JsonNode node) { + public JsonNode apply(JsonNode node) { + requireNonNull(node, "node"); final JsonNode actual = ensureExistence(node); + + final JsonPointer path = path(); if (!EQUIVALENCE.equivalent(actual, oldValue)) { throw new JsonPatchException("mismatching value at '" + path + "': " + actual + " (expected: " + oldValue + ')'); @@ -72,8 +105,8 @@ JsonNode apply(JsonNode node) { @Override public void serialize(JsonGenerator gen, SerializerProvider serializers) throws IOException { gen.writeStartObject(); - gen.writeStringField("op", op); - gen.writeStringField("path", path.toString()); + gen.writeStringField("op", op()); + gen.writeStringField("path", path().toString()); gen.writeFieldName("oldValue"); gen.writeTree(oldValue); gen.writeFieldName("value"); @@ -87,8 +120,25 @@ public void serializeWithType(JsonGenerator gen, SerializerProvider serializers, serialize(gen, serializers); } + @Override + public boolean equals(Object o) { + if (!(o instanceof SafeReplaceOperation)) { + return false; + } + if (!super.equals(o)) { + return false; + } + final SafeReplaceOperation that = (SafeReplaceOperation) o; + return oldValue.equals(that.oldValue) && newValue.equals(that.newValue); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), oldValue, newValue); + } + @Override public String toString() { - return "op: " + op + "; path: \"" + path + "\"; oldValue: " + oldValue + "; value: " + newValue; + return "op: " + op() + "; path: \"" + path() + "\"; oldValue: " + oldValue + "; value: " + newValue; } } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/TestAbsenceOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java similarity index 69% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/TestAbsenceOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java index 646b48daf6..0e275f691a 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/TestAbsenceOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2018 LINE Corporation + * Copyright 2025 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 @@ -14,7 +14,9 @@ * under the License. */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import java.io.IOException; @@ -26,14 +28,27 @@ import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.jsontype.TypeSerializer; +/** + * JSON Patch {@code testAbsence} operation. + * + *

For this operation, {@code path} points to the value to test absence.

+ * + *

It is an error condition if {@code path} points to an actual JSON value.

+ */ public final class TestAbsenceOperation extends JsonPatchOperation { + + /** + * Creates a new instance. + */ @JsonCreator - public TestAbsenceOperation(@JsonProperty("path") final JsonPointer path) { + TestAbsenceOperation(@JsonProperty("path") final JsonPointer path) { super("testAbsence", path); } @Override - JsonNode apply(JsonNode node) { + public JsonNode apply(JsonNode node) { + requireNonNull(node, "node"); + final JsonPointer path = path(); final JsonNode found = node.at(path); if (!found.isMissingNode()) { throw new JsonPatchException("existent path: " + path); @@ -44,8 +59,8 @@ JsonNode apply(JsonNode node) { @Override public void serialize(JsonGenerator gen, SerializerProvider serializers) throws IOException { gen.writeStartObject(); - gen.writeStringField("op", op); - gen.writeStringField("path", path.toString()); + gen.writeStringField("op", op()); + gen.writeStringField("path", path().toString()); gen.writeEndObject(); } @@ -57,6 +72,6 @@ public void serializeWithType(JsonGenerator gen, SerializerProvider serializers, @Override public String toString() { - return "op: " + op + "; path: \"" + path + '"'; + return "op: " + op() + "; path: \"" + path() + '"'; } } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/TestOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestOperation.java similarity index 57% rename from common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/TestOperation.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestOperation.java index 10fb8f0752..b61edb2534 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/TestOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 LINE Corporation + * Copyright 2025 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 @@ -13,26 +13,10 @@ * License for the specific language governing permissions and limitations * under the License. */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ -package com.linecorp.centraldogma.internal.jsonpatch; +package com.linecorp.centraldogma.common.jsonpatch; + +import static java.util.Objects.requireNonNull; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -49,19 +33,22 @@ *

It is an error if no value exists at the given path.

* *

Also note that equality as defined by JSON Patch is exactly the same as it - * is defined by JSON Schema itself. As such, this operation reuses {@link - * JsonNumEquals} for testing equality.

+ * is defined by JSON Schema itself. */ public final class TestOperation extends PathValueOperation { + /** + * Creates a new instance. + */ @JsonCreator - public TestOperation(@JsonProperty("path") final JsonPointer path, - @JsonProperty("value") final JsonNode value) { + TestOperation(@JsonProperty("path") final JsonPointer path, + @JsonProperty("value") final JsonNode value) { super("test", path, value); } @Override - JsonNode apply(final JsonNode node) { + public JsonNode apply(final JsonNode node) { + requireNonNull(node, "node"); ensureEquivalence(ensureExistence(node)); return node; } diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java new file mode 100644 index 0000000000..4de3a53296 --- /dev/null +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java @@ -0,0 +1,34 @@ +/* + * Copyright 2025 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. + */ + +/** + * Implementation of JSON Patch. + * + *

As its name implies, JSON Patch is a mechanism designed to modify JSON + * documents. It consists of a series of operations to apply in order to the + * source JSON document until all operations are applied or an error has been + * encountered.

+ * + *

The main class is {@link com.linecorp.centraldogma.internal.jsonpatch.JsonPatch}.

+ * + *

Note that at this moment, the only way to build a patch is from a JSON + * representation (as a {@link com.fasterxml.jackson.databind.JsonNode}).

+ * + */ +@NonNullByDefault +package com.linecorp.centraldogma.common.jsonpatch; + +import com.linecorp.centraldogma.common.util.NonNullByDefault; diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/DiffProcessor.java b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/DiffProcessor.java index 55af1fb877..52133fc81d 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/DiffProcessor.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/DiffProcessor.java @@ -46,6 +46,8 @@ import com.google.common.base.Equivalence; import com.google.common.base.Predicate; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; + // TODO: cleanup final class DiffProcessor { @@ -75,16 +77,16 @@ public Map get() { void valueReplaced(final JsonPointer pointer, final JsonNode oldValue, final JsonNode newValue) { switch (replaceMode) { case RFC6902: - diffs.add(new ReplaceOperation(pointer, newValue)); + diffs.add(JsonPatchOperation.replace(pointer, newValue)); break; case SAFE: - diffs.add(new SafeReplaceOperation(pointer, oldValue, newValue)); + diffs.add(JsonPatchOperation.safeReplace(pointer, oldValue, newValue)); break; } } void valueRemoved(final JsonPointer pointer) { - diffs.add(new RemoveOperation(pointer)); + diffs.add(JsonPatchOperation.remove(pointer)); } void valueAdded(final JsonPointer pointer, final JsonNode value) { @@ -92,10 +94,10 @@ void valueAdded(final JsonPointer pointer, final JsonNode value) { if (value.isContainerNode()) { // Use copy operation only for container nodes. final JsonPointer ptr = findUnchangedValue(value); - op = ptr != null ? new CopyOperation(ptr, pointer) - : new AddOperation(pointer, value); + op = ptr != null ? JsonPatchOperation.copy(ptr, pointer) + : JsonPatchOperation.add(pointer, value); } else { - op = new AddOperation(pointer, value); + op = JsonPatchOperation.add(pointer, value); } diffs.add(op); diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonNumEquals.java b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonNumEquals.java index 0bde00dd95..3342969a32 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonNumEquals.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonNumEquals.java @@ -56,7 +56,7 @@ * equal if their mathematical value is the same. This class implements this * kind of equality.

*/ -final class JsonNumEquals extends Equivalence { +public final class JsonNumEquals extends Equivalence { private static final Equivalence INSTANCE = new JsonNumEquals(); diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java index b05946ccd9..9a12244f53 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java @@ -62,6 +62,8 @@ import com.google.common.collect.Iterators; import com.google.common.collect.Sets; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; import com.linecorp.centraldogma.internal.Jackson; /** diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperation.java b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperation.java deleted file mode 100644 index 50e2236d59..0000000000 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperation.java +++ /dev/null @@ -1,163 +0,0 @@ -/* - * Copyright 2017 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. - */ -/* - * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) - * - * This software is dual-licensed under: - * - * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any - * later version; - * - the Apache Software License (ASL) version 2.0. - * - * The text of this file and of both licenses is available at the root of this - * project or, if you have the jar distribution, in directory META-INF/, under - * the names LGPL-3.0.txt and ASL-2.0.txt respectively. - * - * Direct link to the sources: - * - * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt - * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt - */ - -package com.linecorp.centraldogma.internal.jsonpatch; - -import static com.fasterxml.jackson.annotation.JsonSubTypes.Type; -import static com.fasterxml.jackson.annotation.JsonTypeInfo.Id; -import static java.util.Objects.requireNonNull; - -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonSubTypes; -import com.fasterxml.jackson.annotation.JsonTypeInfo; -import com.fasterxml.jackson.core.JsonPointer; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.JsonSerializable; -import com.fasterxml.jackson.databind.node.JsonNodeFactory; - -import com.linecorp.centraldogma.internal.Jackson; - -/** - * Base abstract class for one patch operation. - * - *

Two more abstract classes extend this one according to the arguments of - * the operation:

- * - *
    - *
  • {@link DualPathOperation} for operations taking a second pointer as - * an argument ({@code copy} and {@code move});
    • - *
  • {@link PathValueOperation} for operations taking a value as an - * argument ({@code add}, {@code replace} and {@code test}).
    • - *
- */ -@JsonTypeInfo(use = Id.NAME, property = "op") -@JsonSubTypes({ - @Type(name = "add", value = AddOperation.class), - @Type(name = "copy", value = CopyOperation.class), - @Type(name = "move", value = MoveOperation.class), - @Type(name = "remove", value = RemoveOperation.class), - @Type(name = "removeIfExists", value = RemoveIfExistsOperation.class), - @Type(name = "replace", value = ReplaceOperation.class), - @Type(name = "safeReplace", value = SafeReplaceOperation.class), - @Type(name = "test", value = TestOperation.class), - @Type(name = "testAbsence", value = TestAbsenceOperation.class) -}) -@JsonIgnoreProperties(ignoreUnknown = true) -public abstract class JsonPatchOperation implements JsonSerializable { - - /** - * Converts {@link JsonPatchOperation}s to an array of {@link JsonNode}. - */ - public static JsonNode asJsonArray(JsonPatchOperation... jsonPatchOperations) { - requireNonNull(jsonPatchOperations, "jsonPatchOperations"); - return Jackson.valueToTree(jsonPatchOperations); - } - - final String op; - - /* - * Note: no need for a custom deserializer, Jackson will try and find a - * constructor with a single string argument and use it. - * - * However, we need to serialize using .toString(). - */ - final JsonPointer path; - - /** - * Creates a new instance. - * - * @param op the operation name - * @param path the JSON Pointer for this operation - */ - JsonPatchOperation(final String op, final JsonPointer path) { - this.op = op; - this.path = path; - } - - public JsonPointer path() { - return path; - } - - /** - * Applies this operation to a JSON value. - * - * @param node the value to patch - * @return the patched value - * @throws JsonPatchException operation failed to apply to this value - */ - abstract JsonNode apply(JsonNode node); - - @Override - public abstract String toString(); - - /** - * Converts this {@link JsonPatchOperation} to a {@link JsonNode}. - */ - public JsonNode toJsonNode() { - return JsonNodeFactory.instance.arrayNode().add(Jackson.valueToTree(this)); - } - - JsonNode ensureExistence(JsonNode node) { - final JsonNode found = node.at(path); - if (found.isMissingNode()) { - throw new JsonPatchException("non-existent path: " + path); - } - return found; - } - - static JsonNode ensureSourceParent(JsonNode node, JsonPointer path) { - return ensureParent(node, path, "source"); - } - - static JsonNode ensureTargetParent(JsonNode node, JsonPointer path) { - return ensureParent(node, path, "target"); - } - - private static JsonNode ensureParent(JsonNode node, JsonPointer path, String typeName) { - /* - * Check the parent node: it must exist and be a container (ie an array - * or an object) for the add operation to work. - */ - final JsonPointer parentPath = path.head(); - final JsonNode parentNode = node.at(parentPath); - if (parentNode.isMissingNode()) { - throw new JsonPatchException("non-existent " + typeName + " parent: " + parentPath); - } - if (!parentNode.isContainerNode()) { - throw new JsonPatchException(typeName + " parent is not a container: " + parentPath + - " (" + parentNode.getNodeType() + ')'); - } - return parentNode; - } -} diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/package-info.java b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/package-info.java index 3a440b3ae5..d06019d29c 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/package-info.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/package-info.java @@ -31,20 +31,8 @@ * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt */ - /** - * Implementation of JSON Patch. - * - *

As its name implies, JSON Patch is a mechanism designed to modify JSON - * documents. It consists of a series of operations to apply in order to the - * source JSON document until all operations are applied or an error has been - * encountered.

- * - *

The main class is {@link com.linecorp.centraldogma.internal.jsonpatch.JsonPatch}.

- * - *

Note that at this moment, the only way to build a patch is from a JSON - * representation (as a {@link com.fasterxml.jackson.databind.JsonNode}).

- * + * Internal utility classes. */ @NonNullByDefault package com.linecorp.centraldogma.internal.jsonpatch; diff --git a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java new file mode 100644 index 0000000000..5665247601 --- /dev/null +++ b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java @@ -0,0 +1,287 @@ +/* + * Copyright 2025 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.common.jsonpatch; + +import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.List; +import java.util.concurrent.CompletionException; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.JsonPointer; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.IntNode; +import com.google.common.collect.ImmutableList; + +import com.linecorp.centraldogma.client.CentralDogma; +import com.linecorp.centraldogma.client.CentralDogmaRepository; +import com.linecorp.centraldogma.common.Change; +import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; + +class JsonPatchOperationIntegrationTest { + + @RegisterExtension + final CentralDogmaExtension dogma = new CentralDogmaExtension() { + @Override + protected void scaffold(CentralDogma client) { + client.createProject("foo").join(); + client.createRepository("foo", "bar").join(); + } + + @Override + protected boolean runForEachTest() { + return true; + } + }; + + CentralDogmaRepository repository; + + @BeforeEach + void setUp() { + repository = dogma.client().forRepo("foo", "bar"); + } + + @Test + void testAdd() throws JsonParseException { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) + .push() + .join(); + final AddOperation add = JsonPatchOperation.add(JsonPointer.compile("/b"), new IntNode(2)); + final Change change = Change.ofJsonPatch("/a.json", add); + repository.commit("add b", change) + .push() + .join(); + + final JsonNode jsonNode = repository.file("/a.json").get().join().contentAsJson(); + assertThatJson(jsonNode).isEqualTo("{ \"a\": 1, \"b\": 2 }"); + } + + @Test + void testCopy() throws JsonParseException { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) + .push() + .join(); + final CopyOperation copy = JsonPatchOperation.copy(JsonPointer.compile("/a"), + JsonPointer.compile("/b")); + final Change change = Change.ofJsonPatch("/a.json", copy); + repository.commit("copy a", change) + .push() + .join(); + + final JsonNode jsonNode = repository.file("/a.json").get().join().contentAsJson(); + assertThatJson(jsonNode).isEqualTo("{ \"a\": 1, \"b\": 1 }"); + } + + @Test + void testMove() throws JsonParseException { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) + .push() + .join(); + final MoveOperation move = JsonPatchOperation.move(JsonPointer.compile("/a"), + JsonPointer.compile("/b")); + final Change change = Change.ofJsonPatch("/a.json", move); + repository.commit("move a", change) + .push() + .join(); + + final JsonNode jsonNode = repository.file("/a.json").get().join().contentAsJson(); + assertThatJson(jsonNode).isEqualTo("{ \"b\": 1 }"); + } + + @Test + void testRemove() throws JsonParseException { + repository.commit("add ab", Change.ofJsonUpsert("/a.json", "{ \"a\": 1, \"b\": 2 }")) + .push() + .join(); + final RemoveOperation remove = JsonPatchOperation.remove(JsonPointer.compile("/a")); + final Change change = Change.ofJsonPatch("/a.json", remove); + repository.commit("remove a", change) + .push() + .join(); + + final JsonNode jsonNode = repository.file("/a.json").get().join().contentAsJson(); + assertThatJson(jsonNode).isEqualTo("{ \"b\": 2 }"); + + assertThatThrownBy(() -> repository.commit("remove a again", change) + .push() + .join()) + .isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(JsonPatchException.class) + .hasMessageContaining("failed to apply JSON patch:") + .hasMessageContaining("path=/a.json, content=[{\"op\":\"remove\",\"path\":\"/a\"}]"); + } + + @Test + void removeIfExists() throws JsonParseException { + repository.commit("add ab", Change.ofJsonUpsert("/a.json", "{ \"a\": 1, \"b\": 2 }")) + .push() + .join(); + final RemoveIfExistsOperation removeIfExists = + JsonPatchOperation.removeIfExists(JsonPointer.compile("/a")); + final Change change = Change.ofJsonPatch("/a.json", removeIfExists); + repository.commit("remove a", change) + .push() + .join(); + + JsonNode jsonNode = repository.file("/a.json").get().join().contentAsJson(); + assertThatJson(jsonNode).isEqualTo("{ \"b\": 2 }"); + + repository.commit("remove a again", change) + .push() + .join(); + + jsonNode = repository.file("/a.json").get().join().contentAsJson(); + assertThatJson(jsonNode).isEqualTo("{ \"b\": 2 }"); + } + + @Test + void replace() throws JsonParseException { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) + .push() + .join(); + final ReplaceOperation replace = JsonPatchOperation.replace(JsonPointer.compile("/a"), new IntNode(2)); + final Change change = Change.ofJsonPatch("/a.json", replace); + repository.commit("replace a", change) + .push() + .join(); + + final JsonNode jsonNode = repository.file("/a.json").get().join().contentAsJson(); + assertThatJson(jsonNode).isEqualTo("{ \"a\": 2 }"); + } + + @Test + void safeReplace() throws JsonParseException { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) + .push() + .join(); + SafeReplaceOperation safeReplace = + JsonPatchOperation.safeReplace(JsonPointer.compile("/a"), new IntNode(1), new IntNode(2)); + final Change change = Change.ofJsonPatch("/a.json", safeReplace); + repository.commit("safe replace a", change) + .push() + .join(); + + final JsonNode jsonNode = repository.file("/a.json").get().join().contentAsJson(); + assertThatJson(jsonNode).isEqualTo("{ \"a\": 2 }"); + + safeReplace = JsonPatchOperation.safeReplace(JsonPointer.compile("/a"), new IntNode(3), new IntNode(4)); + final Change change1 = Change.ofJsonPatch("/a.json", safeReplace); + assertThatThrownBy(() -> { + repository.commit("invalid safe replace a", change1) + .push() + .join(); + }).isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(JsonPatchException.class) + .hasMessageContaining( + "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, path=/a.json, " + + "content=[{\"op\":\"safeReplace\",\"path\":\"/a\",\"oldValue\":3,\"value\":4}]}"); + } + + @Test + void test() { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) + .push() + .join(); + TestOperation test = JsonPatchOperation.test(JsonPointer.compile("/a"), new IntNode(1)); + final Change change = Change.ofJsonPatch("/a.json", test); + repository.commit("test a", change) + .push() + .join(); + + test = JsonPatchOperation.test(JsonPointer.compile("/a"), new IntNode(2)); + final Change change1 = Change.ofJsonPatch("/a.json", test); + assertThatThrownBy(() -> { + repository.commit("invalid test a", change1) + .push() + .join(); + }).isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(JsonPatchException.class) + .hasMessageContaining( + "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, " + + "path=/a.json, content=[{\"op\":\"test\",\"path\":\"/a\",\"value\":2}]}"); + } + + @Test + void testAbsence() { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) + .push() + .join(); + TestAbsenceOperation testAbsence = JsonPatchOperation.testAbsence(JsonPointer.compile("/b")); + final Change change = Change.ofJsonPatch("/a.json", testAbsence); + repository.commit("test absence", change) + .push() + .join(); + + testAbsence = JsonPatchOperation.testAbsence(JsonPointer.compile("/a")); + final Change change1 = Change.ofJsonPatch("/a.json", testAbsence); + assertThatThrownBy(() -> { + repository.commit("invalid test absence", change1) + .push() + .join(); + }).isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(JsonPatchException.class) + .hasMessageContaining( + "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, " + + "path=/a.json, content=[{\"op\":\"testAbsence\",\"path\":\"/a\"}]}"); + } + + @Test + void testMultipleOperations() throws JsonParseException { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1, \"b\": 2 }")) + .push() + .join(); + // { "a": 1, "b": 2 } -> { "a": 1, "b": 2, "c": 3 } + final AddOperation add = JsonPatchOperation.add(JsonPointer.compile("/c"), new IntNode(3)); + // { "a": 1, "b": 2, "c": 3 } -> { "b": 2, "c": 3, "d": 1 } + final MoveOperation move = JsonPatchOperation.move(JsonPointer.compile("/a"), + JsonPointer.compile("/d")); + // { "b": 2, "c": 3, "d": 1 } -> { "c": 3, "d": 1 } + final RemoveOperation remove = JsonPatchOperation.remove(JsonPointer.compile("/b")); + // { "c": 3, "d": 1 } -> { "c": 4, "d": 1 } + final SafeReplaceOperation safeReplace = JsonPatchOperation.safeReplace(JsonPointer.compile("/c"), + new IntNode(3), new IntNode(4)); + + final Change change = Change.ofJsonPatch("/a.json", + ImmutableList.of(add, move, remove, safeReplace)); + + repository.commit("json patch operations", change) + .push() + .join(); + + assertThatJson(repository.file("/a.json").get().join().contentAsJson()) + .isEqualTo("{ \"c\": 4, \"d\": 1 }"); + } + + @Test + void testEquality() throws JsonProcessingException { + final RemoveOperation remove = JsonPatchOperation.remove(JsonPointer.compile("/a")); + final JsonNode jsonNode = remove.toJsonNode(); + final ObjectMapper mapper = new ObjectMapper(); + final List jsonPatchOperations = + mapper.treeToValue(jsonNode, new TypeReference>() {}); + assertThat(jsonPatchOperations.get(0)).isEqualTo(remove); + } +} diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationSerializationTest.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationSerializationTest.java index 668081cc2b..aa4fc3ce76 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationSerializationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationSerializationTest.java @@ -52,6 +52,16 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.linecorp.centraldogma.common.jsonpatch.AddOperation; +import com.linecorp.centraldogma.common.jsonpatch.CopyOperation; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; +import com.linecorp.centraldogma.common.jsonpatch.MoveOperation; +import com.linecorp.centraldogma.common.jsonpatch.RemoveIfExistsOperation; +import com.linecorp.centraldogma.common.jsonpatch.RemoveOperation; +import com.linecorp.centraldogma.common.jsonpatch.ReplaceOperation; +import com.linecorp.centraldogma.common.jsonpatch.TestAbsenceOperation; +import com.linecorp.centraldogma.common.jsonpatch.TestOperation; + class JsonPatchOperationSerializationTest { private static final ObjectMapper MAPPER = new ObjectMapper(); diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationTest.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationTest.java index 8bffaea9cb..d105f69af5 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationTest.java @@ -51,6 +51,9 @@ import com.google.common.base.Equivalence; import com.google.common.collect.ImmutableList; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; + class JsonPatchOperationTest { private static final ObjectMapper MAPPER = new ObjectMapper(); diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTest.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTest.java index 261af8de7b..0feb8752a2 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTest.java @@ -52,6 +52,9 @@ import com.fasterxml.jackson.databind.node.JsonNodeFactory; import com.google.common.collect.ImmutableList; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; + class JsonPatchTest { private static final JsonNodeFactory FACTORY = JsonNodeFactory.instance; diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTestSuite.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTestSuite.java index 64ed246761..70c16972e9 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTestSuite.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTestSuite.java @@ -49,6 +49,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; + class JsonPatchTestSuite { private static final ObjectMapper MAPPER = new ObjectMapper(); diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveIfExistsOperationTest.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveIfExistsOperationTest.java index be7a8512d9..eb17f6c6a1 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveIfExistsOperationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveIfExistsOperationTest.java @@ -42,6 +42,8 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; + class RemoveIfExistsOperationTest { private static final JsonPointer EMPTY_JSON_POINTER = JsonPointer.compile(""); @@ -49,7 +51,7 @@ class RemoveIfExistsOperationTest { @Test void removingRootReturnsMissingNode() { final JsonNode node = JsonNodeFactory.instance.nullNode(); - final JsonPatchOperation op = new RemoveIfExistsOperation(EMPTY_JSON_POINTER); + final JsonPatchOperation op = JsonPatchOperation.removeIfExists(EMPTY_JSON_POINTER); final JsonNode ret = op.apply(node); assertThat(ret.isMissingNode()).isTrue(); diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveOperationTest.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveOperationTest.java index b718825b4b..8c68c7152f 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveOperationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/RemoveOperationTest.java @@ -42,6 +42,8 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; + class RemoveOperationTest { private static final JsonPointer EMPTY_JSON_POINTER = JsonPointer.compile(""); @@ -49,7 +51,7 @@ class RemoveOperationTest { @Test void removingRootReturnsMissingNode() { final JsonNode node = JsonNodeFactory.instance.nullNode(); - final JsonPatchOperation op = new RemoveOperation(EMPTY_JSON_POINTER); + final JsonPatchOperation op = JsonPatchOperation.remove(EMPTY_JSON_POINTER); final JsonNode ret = op.apply(node); assertThat(ret.isMissingNode()).isTrue(); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java index c388f2c034..9a9c2af0a4 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java @@ -49,6 +49,7 @@ import com.linecorp.centraldogma.common.RepositoryNotFoundException; import com.linecorp.centraldogma.common.RevisionNotFoundException; import com.linecorp.centraldogma.common.TooManyRequestsException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; import com.linecorp.centraldogma.server.internal.storage.RequestAlreadyTimedOutException; import com.linecorp.centraldogma.server.internal.storage.repository.RepositoryMetadataException; import com.linecorp.centraldogma.server.metadata.MemberNotFoundException; @@ -89,6 +90,8 @@ public final class HttpApiExceptionHandler implements ServerErrorHandler { .put(RepositoryExistsException.class, (ctx, cause) -> newResponse(ctx, HttpStatus.CONFLICT, cause, "Repository '%s' exists already.", cause.getMessage())) + .put(JsonPatchException.class, + (ctx, cause) -> newResponse(ctx, HttpStatus.CONFLICT, cause)) .put(RepositoryMetadataException.class, (ctx, cause) -> newResponse(ctx, HttpStatus.INTERNAL_SERVER_ERROR, cause)) .put(RepositoryNotFoundException.class, diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataApiService.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataApiService.java index 74003c8d6b..9a748ce7a3 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataApiService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataApiService.java @@ -41,9 +41,9 @@ import com.linecorp.centraldogma.common.ProjectRole; import com.linecorp.centraldogma.common.RepositoryRole; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; +import com.linecorp.centraldogma.common.jsonpatch.ReplaceOperation; import com.linecorp.centraldogma.internal.jsonpatch.JsonPatch; -import com.linecorp.centraldogma.internal.jsonpatch.JsonPatchOperation; -import com.linecorp.centraldogma.internal.jsonpatch.ReplaceOperation; import com.linecorp.centraldogma.server.QuotaConfig; import com.linecorp.centraldogma.server.command.CommandExecutor; import com.linecorp.centraldogma.server.internal.api.auth.RequiresProjectRole; diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java index 86c232801e..75fde09009 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java @@ -43,6 +43,7 @@ import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.ChangeConflictException; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.internal.Util; import com.linecorp.centraldogma.internal.jsonpatch.JsonPatch; @@ -158,7 +159,7 @@ int doApply(Revision unused, DirCache dirCache, try { newJsonNode = JsonPatch.fromJson((JsonNode) change.content()).apply(oldJsonNode); } catch (Exception e) { - throw new ChangeConflictException("failed to apply JSON patch: " + change, e); + throw new JsonPatchException("failed to apply JSON patch: " + change, e); } // Apply only when the contents are really different. 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 ec2334e510..3ff5dc3a41 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 @@ -81,6 +81,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Streams; import com.linecorp.armeria.common.CommonPools; import com.linecorp.armeria.common.util.Exceptions; @@ -88,6 +89,7 @@ import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.CentralDogmaException; import com.linecorp.centraldogma.common.Change; +import com.linecorp.centraldogma.common.ChangeType; import com.linecorp.centraldogma.common.Commit; import com.linecorp.centraldogma.common.Entry; import com.linecorp.centraldogma.common.EntryNotFoundException; @@ -854,8 +856,12 @@ public CompletableFuture commit( requireNonNull(detail, "detail"); requireNonNull(markup, "markup"); requireNonNull(changes, "changes"); + + // JsonPatch operations its own validation for the changes so we don't need to validate them here. + final boolean allowEmptyCommit = Streams.stream(changes).allMatch( + change -> change.type() == ChangeType.APPLY_JSON_PATCH); final CommitExecutor commitExecutor = - new CommitExecutor(this, commitTimeMillis, author, summary, detail, markup, false); + new CommitExecutor(this, commitTimeMillis, author, summary, detail, markup, allowEmptyCommit); return commit(baseRevision, commitExecutor, normBaseRevision -> { if (!directExecution) { return changes; @@ -1323,7 +1329,7 @@ public void cloneTo(File newRepoDir, BiConsumer progressListen // NB: We allow an empty commit here because an old version of Central Dogma had a bug // which allowed the creation of an empty commit. new CommitExecutor(newRepo, c.when(), c.author(), c.summary(), - c.detail(), c.markup(), true) + c.detail(), c.markup(), true) .execute(baseRevision, normBaseRevision -> blockingPreviewDiff( normBaseRevision, new DefaultChangesApplier(changes)).values()); } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java index 2ab51c74f3..7b08ec4a75 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java @@ -18,7 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableMap.toImmutableMap; -import static com.linecorp.centraldogma.internal.jsonpatch.JsonPatchOperation.asJsonArray; +import static com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation.asJsonArray; import static com.linecorp.centraldogma.internal.jsonpatch.JsonPatchUtil.encodeSegment; import static com.linecorp.centraldogma.server.internal.storage.project.ProjectApiManager.listProjectsWithoutInternal; import static com.linecorp.centraldogma.server.metadata.RepositoryMetadata.DEFAULT_PROJECT_ROLES; @@ -55,11 +55,8 @@ import com.linecorp.centraldogma.common.RepositoryExistsException; import com.linecorp.centraldogma.common.RepositoryRole; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; import com.linecorp.centraldogma.internal.Jackson; -import com.linecorp.centraldogma.internal.jsonpatch.AddOperation; -import com.linecorp.centraldogma.internal.jsonpatch.RemoveOperation; -import com.linecorp.centraldogma.internal.jsonpatch.ReplaceOperation; -import com.linecorp.centraldogma.internal.jsonpatch.TestAbsenceOperation; import com.linecorp.centraldogma.server.QuotaConfig; import com.linecorp.centraldogma.server.command.CommandExecutor; import com.linecorp.centraldogma.server.storage.project.Project; @@ -188,9 +185,9 @@ public CompletableFuture removeProject(Author author, String projectNa final Change change = Change.ofJsonPatch( METADATA_JSON, - asJsonArray(new TestAbsenceOperation(PROJECT_REMOVAL), - new AddOperation(PROJECT_REMOVAL, - Jackson.valueToTree(UserAndTimestamp.of(author))))); + asJsonArray(JsonPatchOperation.testAbsence(PROJECT_REMOVAL), + JsonPatchOperation.add(PROJECT_REMOVAL, + Jackson.valueToTree(UserAndTimestamp.of(author))))); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, "Remove the project: " + projectName, change); } @@ -203,7 +200,7 @@ public CompletableFuture restoreProject(Author author, String projectN requireNonNull(projectName, "projectName"); final Change change = - Change.ofJsonPatch(METADATA_JSON, new RemoveOperation(PROJECT_REMOVAL).toJsonNode()); + Change.ofJsonPatch(METADATA_JSON, JsonPatchOperation.remove(PROJECT_REMOVAL).toJsonNode()); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, "Restore the project: " + projectName, change); } @@ -234,8 +231,8 @@ public CompletableFuture addMember(Author author, String projectName, final JsonPointer path = JsonPointer.compile("/members" + encodeSegment(newMember.id())); final Change change = Change.ofJsonPatch(METADATA_JSON, - asJsonArray(new TestAbsenceOperation(path), - new AddOperation(path, Jackson.valueToTree(newMember)))); + asJsonArray(JsonPatchOperation.testAbsence(path), + JsonPatchOperation.add(path, Jackson.valueToTree(newMember)))); final String commitSummary = "Add a member '" + newMember.id() + "' to the project '" + projectName + '\''; return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, change); @@ -307,8 +304,9 @@ public CompletableFuture updateMemberRole(Author author, String projec final Change change = Change.ofJsonPatch( METADATA_JSON, - new ReplaceOperation(JsonPointer.compile("/members" + encodeSegment(member.id()) + "/role"), - Jackson.valueToTree(projectRole)).toJsonNode()); + JsonPatchOperation.replace( + JsonPointer.compile("/members" + encodeSegment(member.id()) + "/role"), + Jackson.valueToTree(projectRole)).toJsonNode()); final String commitSummary = "Updates the role of the member '" + member.id() + "' as '" + projectRole + "' for the project '" + projectName + '\''; return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, change); @@ -349,9 +347,9 @@ public CompletableFuture addRepo(Author author, String projectName, St RepositoryMetadata.of(repoName, UserAndTimestamp.of(author), projectRoles); final Change change = Change.ofJsonPatch(METADATA_JSON, - asJsonArray(new TestAbsenceOperation(path), - new AddOperation(path, - Jackson.valueToTree(newRepositoryMetadata)))); + asJsonArray(JsonPatchOperation.testAbsence(path), + JsonPatchOperation.add( + path, Jackson.valueToTree(newRepositoryMetadata)))); final String commitSummary = "Add a repo '" + newRepositoryMetadata.id() + "' to the project '" + projectName + '\''; return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, change) @@ -379,8 +377,8 @@ public CompletableFuture removeRepo(Author author, String projectName, final JsonPointer path = JsonPointer.compile("/repos" + encodeSegment(repoName) + "/removal"); final Change change = Change.ofJsonPatch(METADATA_JSON, - asJsonArray(new TestAbsenceOperation(path), - new AddOperation(path, Jackson.valueToTree( + asJsonArray(JsonPatchOperation.testAbsence(path), + JsonPatchOperation.add(path, Jackson.valueToTree( UserAndTimestamp.of(author))))); final String commitSummary = "Remove the repo '" + repoName + "' from the project '" + projectName + '\''; @@ -398,7 +396,7 @@ public CompletableFuture purgeRepo(Author author, String projectName, final JsonPointer path = JsonPointer.compile("/repos" + encodeSegment(repoName)); final Change change = Change.ofJsonPatch(METADATA_JSON, - new RemoveOperation(path).toJsonNode()); + JsonPatchOperation.remove(path).toJsonNode()); final String commitSummary = "Purge the repo '" + repoName + "' from the project '" + projectName + '\''; return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, change); @@ -415,7 +413,7 @@ public CompletableFuture restoreRepo(Author author, String projectName final Change change = Change.ofJsonPatch(METADATA_JSON, - new RemoveOperation(JsonPointer.compile( + JsonPatchOperation.remove(JsonPointer.compile( "/repos" + encodeSegment(repoName) + "/removal")).toJsonNode()); final String commitSummary = "Restore the repo '" + repoName + "' from the project '" + projectName + '\''; @@ -490,8 +488,9 @@ public CompletableFuture addToken(Author author, String projectName, final JsonPointer path = JsonPointer.compile("/tokens" + encodeSegment(registration.id())); final Change change = Change.ofJsonPatch(METADATA_JSON, - asJsonArray(new TestAbsenceOperation(path), - new AddOperation(path, Jackson.valueToTree(registration)))); + asJsonArray(JsonPatchOperation.testAbsence(path), + JsonPatchOperation.add(path, + Jackson.valueToTree(registration)))); final String commitSummary = "Add a token '" + registration.id() + "' to the project '" + projectName + "' with a role '" + role + '\''; return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, change); @@ -524,30 +523,30 @@ private CompletableFuture removeToken(String projectName, Author autho final String commitSummary = "Remove the token '" + appId + "' from the project '" + projectName + '\''; final ProjectMetadataTransformer transformer = new ProjectMetadataTransformer((headRevision, projectMetadata) -> { - final Map tokens = projectMetadata.tokens(); - final Map newTokens; - if (tokens.get(appId) == null) { - if (!quiet) { - throw new TokenNotFoundException( - "failed to find the token " + appId + " in project " + projectName); - } - newTokens = tokens; - } else { - newTokens = tokens.entrySet() - .stream() - .filter(entry -> !entry.getKey().equals(appId)) - .collect(toImmutableMap(Entry::getKey, Entry::getValue)); - } + final Map tokens = projectMetadata.tokens(); + final Map newTokens; + if (tokens.get(appId) == null) { + if (!quiet) { + throw new TokenNotFoundException( + "failed to find the token " + appId + " in project " + projectName); + } + newTokens = tokens; + } else { + newTokens = tokens.entrySet() + .stream() + .filter(entry -> !entry.getKey().equals(appId)) + .collect(toImmutableMap(Entry::getKey, Entry::getValue)); + } - final ImmutableMap newRepos = - removeTokenFromRepositories(appId, projectMetadata); - return new ProjectMetadata(projectMetadata.name(), - newRepos, - projectMetadata.members(), - newTokens, - projectMetadata.creation(), - projectMetadata.removal()); - }); + final ImmutableMap newRepos = + removeTokenFromRepositories(appId, projectMetadata); + return new ProjectMetadata(projectMetadata.name(), + newRepos, + projectMetadata.members(), + newTokens, + projectMetadata.creation(), + projectMetadata.removal()); + }); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); } @@ -587,8 +586,8 @@ public CompletableFuture updateTokenRole(Author author, String project final JsonPointer path = JsonPointer.compile("/tokens" + encodeSegment(registration.id())); final Change change = Change.ofJsonPatch(METADATA_JSON, - new ReplaceOperation(path, Jackson.valueToTree(registration)) - .toJsonNode()); + JsonPatchOperation.replace( + path, Jackson.valueToTree(registration)).toJsonNode()); final String commitSummary = "Update the role of a token '" + token.appId() + "' as '" + role + "' for the project '" + projectName + '\''; return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, change); @@ -835,7 +834,7 @@ public CompletableFuture updateWriteQuota( final JsonPointer path = JsonPointer.compile("/repos" + encodeSegment(repoName) + "/writeQuota"); final Change change = Change.ofJsonPatch(METADATA_JSON, - new AddOperation(path, Jackson.valueToTree(writeQuota)).toJsonNode()); + JsonPatchOperation.add(path, Jackson.valueToTree(writeQuota)).toJsonNode()); final String commitSummary = "Update a write quota for the repository '" + repoName + '\''; executor.setWriteQuota(projectName, repoName, writeQuota); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, change); @@ -1001,11 +1000,11 @@ public CompletableFuture createToken(Author author, String appId, Stri final JsonPointer secretPath = JsonPointer.compile("/secrets" + encodeSegment(newTokenSecret)); final Change change = Change.ofJsonPatch(TOKEN_JSON, - asJsonArray(new TestAbsenceOperation(appIdPath), - new TestAbsenceOperation(secretPath), - new AddOperation(appIdPath, Jackson.valueToTree(newToken)), - new AddOperation(secretPath, - Jackson.valueToTree(newToken.id())))); + asJsonArray(JsonPatchOperation.testAbsence(appIdPath), + JsonPatchOperation.testAbsence(secretPath), + JsonPatchOperation.add(appIdPath, Jackson.valueToTree(newToken)), + JsonPatchOperation.add(secretPath, + Jackson.valueToTree(newToken.id())))); return tokenRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, "Add a token: " + newToken.id(), change); } diff --git a/server/src/test/java/com/linecorp/centraldogma/server/metadata/TokenTest.java b/server/src/test/java/com/linecorp/centraldogma/server/metadata/TokenTest.java index b4fed9864b..8068a8f71e 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/metadata/TokenTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/metadata/TokenTest.java @@ -15,7 +15,7 @@ */ package com.linecorp.centraldogma.server.metadata; -import static com.linecorp.centraldogma.internal.jsonpatch.JsonPatchOperation.asJsonArray; +import static com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation.asJsonArray; import static com.linecorp.centraldogma.server.metadata.MetadataService.TOKEN_JSON; import static org.assertj.core.api.Assertions.assertThat; @@ -38,9 +38,8 @@ import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; import com.linecorp.centraldogma.internal.Jackson; -import com.linecorp.centraldogma.internal.jsonpatch.AddOperation; -import com.linecorp.centraldogma.internal.jsonpatch.TestAbsenceOperation; import com.linecorp.centraldogma.server.internal.api.sysadmin.TokenLevelRequest; import com.linecorp.centraldogma.server.internal.api.sysadmin.TokenService; import com.linecorp.centraldogma.server.storage.project.InternalProjectInitializer; @@ -77,10 +76,10 @@ static void setUp() throws JsonParseException { final JsonPointer secretPath = JsonPointer.compile("/secrets/" + APP_SECRET); final Change change = Change.ofJsonPatch( TOKEN_JSON, - asJsonArray(new TestAbsenceOperation(appIdPath), - new TestAbsenceOperation(secretPath), - new AddOperation(appIdPath, Jackson.readTree(tokenJson(true))), - new AddOperation(secretPath, Jackson.valueToTree(APP_ID)))); + asJsonArray(JsonPatchOperation.testAbsence(appIdPath), + JsonPatchOperation.testAbsence(secretPath), + JsonPatchOperation.add(appIdPath, Jackson.readTree(tokenJson(true))), + JsonPatchOperation.add(secretPath, Jackson.valueToTree(APP_ID)))); dogmaRepository.commit(Revision.HEAD, System.currentTimeMillis(), AUTHOR, "Add the legacy token", change).join(); From 820316f53497f4f9a83256be721eeb0e77ad13fd Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Fri, 17 Jan 2025 16:41:32 +0900 Subject: [PATCH 2/7] polish --- .../client/armeria/ArmeriaCentralDogma.java | 6 ++- .../common/TextPatchConflictException.java | 40 +++++++++++++++++++ .../common/jsonpatch/AddOperation.java | 6 +-- .../common/jsonpatch/CopyOperation.java | 2 +- ...n.java => JsonPatchConflictException.java} | 9 +++-- .../common/jsonpatch/JsonPatchOperation.java | 10 ++--- .../common/jsonpatch/MoveOperation.java | 2 +- .../common/jsonpatch/PathValueOperation.java | 4 +- .../jsonpatch/SafeReplaceOperation.java | 4 +- .../jsonpatch/TestAbsenceOperation.java | 2 +- .../internal/jsonpatch/JsonPatch.java | 6 +-- .../JsonPatchOperationIntegrationTest.java | 8 ++-- .../jsonpatch/JsonPatchOperationTest.java | 4 +- .../internal/jsonpatch/JsonPatchTest.java | 6 +-- .../jsonpatch/JsonPatchTestSuite.java | 4 +- .../internal/api/HttpApiExceptionHandler.java | 7 +++- .../repository/git/DefaultChangesApplier.java | 15 +++++-- .../storage/repository/git/GitRepository.java | 15 +++++-- .../repository/git/GitRepositoryTest.java | 29 +++++++++----- 19 files changed, 126 insertions(+), 53 deletions(-) create mode 100644 common/src/main/java/com/linecorp/centraldogma/common/TextPatchConflictException.java rename common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/{JsonPatchException.java => JsonPatchConflictException.java} (74%) diff --git a/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java b/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java index 49a6d02944..022661ead7 100644 --- a/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java +++ b/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java @@ -105,7 +105,8 @@ import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.common.RevisionNotFoundException; import com.linecorp.centraldogma.common.ShuttingDownException; -import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.TextPatchConflictException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchConflictException; import com.linecorp.centraldogma.internal.HistoryConstants; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.internal.Util; @@ -138,7 +139,8 @@ public final class ArmeriaCentralDogma extends AbstractCentralDogma { .put(ReadOnlyException.class.getName(), ReadOnlyException::new) .put(MirrorException.class.getName(), MirrorException::new) .put(PermissionException.class.getName(), PermissionException::new) - .put(JsonPatchException.class.getName(), JsonPatchException::new) + .put(JsonPatchConflictException.class.getName(), JsonPatchConflictException::new) + .put(TextPatchConflictException.class.getName(), TextPatchConflictException::new) .build(); private final WebClient client; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/TextPatchConflictException.java b/common/src/main/java/com/linecorp/centraldogma/common/TextPatchConflictException.java new file mode 100644 index 0000000000..cca896e2e5 --- /dev/null +++ b/common/src/main/java/com/linecorp/centraldogma/common/TextPatchConflictException.java @@ -0,0 +1,40 @@ +/* + * Copyright 2025 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.common; + +/** + * A {@link CentralDogmaException} that is raised when attempted to apply a text patch which cannot be applied + * without a conflict. + */ +public final class TextPatchConflictException extends ChangeConflictException { + private static final long serialVersionUID = -6150468151945332532L; + + /** + * Creates a new instance. + */ + public TextPatchConflictException(String message) { + super(message); + } + + /** + * Creates a new instance with the specified {@code cause}. + */ + public TextPatchConflictException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java index f6cf5a0a15..6338940070 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java @@ -99,12 +99,12 @@ static JsonNode addToArray(final JsonPointer path, final JsonNode node, final Js try { index = Integer.parseInt(rawToken); } catch (NumberFormatException ignored) { - throw new JsonPatchException("not an index: " + rawToken + " (expected: a non-negative integer)"); + throw new JsonPatchConflictException("not an index: " + rawToken + " (expected: a non-negative integer)"); } if (index < 0 || index > size) { - throw new JsonPatchException("index out of bounds: " + index + - " (expected: >= 0 && <= " + size + ')'); + throw new JsonPatchConflictException("index out of bounds: " + index + + " (expected: >= 0 && <= " + size + ')'); } target.insert(index, value); diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java index 736d7aa901..d9ea5a5b03 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java @@ -48,7 +48,7 @@ public JsonNode apply(final JsonNode node) { final JsonPointer from = from(); JsonNode source = node.at(from); if (source.isMissingNode()) { - throw new JsonPatchException("non-existent source path: " + from); + throw new JsonPatchConflictException("non-existent source path: " + from); } final JsonPointer path = path(); diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchException.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchConflictException.java similarity index 74% rename from common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchException.java rename to common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchConflictException.java index 4423249cfb..1ae4a5faa0 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchException.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchConflictException.java @@ -17,25 +17,26 @@ package com.linecorp.centraldogma.common.jsonpatch; import com.linecorp.centraldogma.common.CentralDogmaException; +import com.linecorp.centraldogma.common.ChangeConflictException; /** - * An exception raised when a JSON Patch operation fails. + * A {@link CentralDogmaException} raised when a JSON Patch operation fails. */ -public final class JsonPatchException extends CentralDogmaException { +public final class JsonPatchConflictException extends ChangeConflictException { private static final long serialVersionUID = 4746173383862473527L; /** * Creates a new instance. */ - public JsonPatchException(final String message) { + public JsonPatchConflictException(String message) { super(message); } /** * Creates a new instance with the specified {@code cause}. */ - public JsonPatchException(final String message, final Throwable cause) { + public JsonPatchConflictException(String message, Throwable cause) { super(message, cause); } } diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java index 1e35ad677e..d8ee15c32a 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java @@ -230,7 +230,7 @@ public final JsonPointer path() { * * @param node the value to patch * @return the patched value - * @throws JsonPatchException operation failed to apply to this value + * @throws JsonPatchConflictException operation failed to apply to this value */ public abstract JsonNode apply(JsonNode node); @@ -244,7 +244,7 @@ public JsonNode toJsonNode() { JsonNode ensureExistence(JsonNode node) { final JsonNode found = node.at(path); if (found.isMissingNode()) { - throw new JsonPatchException("non-existent path: " + path); + throw new JsonPatchConflictException("non-existent path: " + path); } return found; } @@ -265,11 +265,11 @@ private static JsonNode ensureParent(JsonNode node, JsonPointer path, String typ final JsonPointer parentPath = path.head(); final JsonNode parentNode = node.at(parentPath); if (parentNode.isMissingNode()) { - throw new JsonPatchException("non-existent " + typeName + " parent: " + parentPath); + throw new JsonPatchConflictException("non-existent " + typeName + " parent: " + parentPath); } if (!parentNode.isContainerNode()) { - throw new JsonPatchException(typeName + " parent is not a container: " + parentPath + - " (" + parentNode.getNodeType() + ')'); + throw new JsonPatchConflictException(typeName + " parent is not a container: " + parentPath + + " (" + parentNode.getNodeType() + ')'); } return parentNode; } diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java index 615ccc3715..421770c0b9 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java @@ -75,7 +75,7 @@ public JsonNode apply(final JsonNode node) { return node; } if (node.at(from).isMissingNode()) { - throw new JsonPatchException("non-existent source path: " + from); + throw new JsonPatchConflictException("non-existent source path: " + from); } final JsonNode sourceParent = ensureSourceParent(node, from); diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java index 70fed8c043..cb3ab39f6e 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java @@ -67,8 +67,8 @@ JsonNode valueCopy() { void ensureEquivalence(JsonNode actual) { if (!EQUIVALENCE.equivalent(actual, value)) { - throw new JsonPatchException("mismatching value at '" + path() + "': " + - actual + " (expected: " + value + ')'); + throw new JsonPatchConflictException("mismatching value at '" + path() + "': " + + actual + " (expected: " + value + ')'); } } diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java index 2a5f3bf6e2..4641968534 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java @@ -85,8 +85,8 @@ public JsonNode apply(JsonNode node) { final JsonPointer path = path(); if (!EQUIVALENCE.equivalent(actual, oldValue)) { - throw new JsonPatchException("mismatching value at '" + path + "': " + - actual + " (expected: " + oldValue + ')'); + throw new JsonPatchConflictException("mismatching value at '" + path + "': " + + actual + " (expected: " + oldValue + ')'); } final JsonNode replacement = newValue.deepCopy(); if (path.toString().isEmpty()) { diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java index 0e275f691a..2c199c7ae8 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java @@ -51,7 +51,7 @@ public JsonNode apply(JsonNode node) { final JsonPointer path = path(); final JsonNode found = node.at(path); if (!found.isMissingNode()) { - throw new JsonPatchException("existent path: " + path); + throw new JsonPatchConflictException("existent path: " + path); } return node; } diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java index 9a12244f53..fb7ec3a0c2 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java @@ -62,7 +62,7 @@ import com.google.common.collect.Iterators; import com.google.common.collect.Sets; -import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchConflictException; import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; import com.linecorp.centraldogma.internal.Jackson; @@ -141,7 +141,7 @@ public static JsonPatch fromJson(final JsonNode node) throws IOException { try { return Jackson.treeToValue(node, JsonPatch.class); } catch (JsonMappingException e) { - throw new JsonPatchException("invalid JSON patch", e); + throw new JsonPatchConflictException("invalid JSON patch", e); } } @@ -341,7 +341,7 @@ public List operations() { * * @param node the value to apply the patch to * @return the patched JSON value - * @throws JsonPatchException failed to apply patch + * @throws JsonPatchConflictException failed to apply patch * @throws NullPointerException input is null */ public JsonNode apply(final JsonNode node) { diff --git a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java index 5665247601..bbd7d235fb 100644 --- a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java @@ -129,7 +129,7 @@ void testRemove() throws JsonParseException { .push() .join()) .isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(JsonPatchException.class) + .hasCauseInstanceOf(JsonPatchConflictException.class) .hasMessageContaining("failed to apply JSON patch:") .hasMessageContaining("path=/a.json, content=[{\"op\":\"remove\",\"path\":\"/a\"}]"); } @@ -194,7 +194,7 @@ void safeReplace() throws JsonParseException { .push() .join(); }).isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(JsonPatchException.class) + .hasCauseInstanceOf(JsonPatchConflictException.class) .hasMessageContaining( "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, path=/a.json, " + "content=[{\"op\":\"safeReplace\",\"path\":\"/a\",\"oldValue\":3,\"value\":4}]}"); @@ -218,7 +218,7 @@ void test() { .push() .join(); }).isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(JsonPatchException.class) + .hasCauseInstanceOf(JsonPatchConflictException.class) .hasMessageContaining( "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, " + "path=/a.json, content=[{\"op\":\"test\",\"path\":\"/a\",\"value\":2}]}"); @@ -242,7 +242,7 @@ void testAbsence() { .push() .join(); }).isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(JsonPatchException.class) + .hasCauseInstanceOf(JsonPatchConflictException.class) .hasMessageContaining( "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, " + "path=/a.json, content=[{\"op\":\"testAbsence\",\"path\":\"/a\"}]}"); diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationTest.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationTest.java index d105f69af5..fbb6c35624 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchOperationTest.java @@ -51,7 +51,7 @@ import com.google.common.base.Equivalence; import com.google.common.collect.ImmutableList; -import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchConflictException; import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; class JsonPatchOperationTest { @@ -79,7 +79,7 @@ void errorsAreCorrectlyReported(JsonNode patch, JsonNode node, String message) t final JsonPatchOperation op = READER.readValue(patch); assertThatThrownBy(() -> op.apply(node)) - .isInstanceOf(JsonPatchException.class) + .isInstanceOf(JsonPatchConflictException.class) .hasMessage(message); } diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTest.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTest.java index 0feb8752a2..552b2a4f04 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTest.java @@ -52,7 +52,7 @@ import com.fasterxml.jackson.databind.node.JsonNodeFactory; import com.google.common.collect.ImmutableList; -import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchConflictException; import com.linecorp.centraldogma.common.jsonpatch.JsonPatchOperation; class JsonPatchTest { @@ -99,12 +99,12 @@ void operationsAreCalledInOrder() { void whenOneOperationFailsNextOperationIsNotCalled() { final String message = "foo"; when(op1.apply(any(JsonNode.class))) - .thenThrow(new JsonPatchException(message)); + .thenThrow(new JsonPatchConflictException(message)); final JsonPatch patch = new JsonPatch(ImmutableList.of(op1, op2)); assertThatThrownBy(() -> patch.apply(FACTORY.nullNode())) - .isInstanceOf(JsonPatchException.class) + .isInstanceOf(JsonPatchConflictException.class) .hasMessage(message); verifyNoMoreInteractions(op2); diff --git a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTestSuite.java b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTestSuite.java index 70c16972e9..92d4741f11 100644 --- a/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTestSuite.java +++ b/common/src/test/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatchTestSuite.java @@ -49,7 +49,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; -import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchConflictException; class JsonPatchTestSuite { @@ -64,7 +64,7 @@ void testSuite(JsonNode source, JsonPatch patch, JsonNode expected, boolean vali assertThat((Object) actual).isEqualTo(expected); } else { assertThatThrownBy(() -> patch.apply(source)) - .isInstanceOf(JsonPatchException.class); + .isInstanceOf(JsonPatchConflictException.class); } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java index 9a9c2af0a4..31fcde1ef3 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java @@ -48,8 +48,9 @@ import com.linecorp.centraldogma.common.RepositoryExistsException; import com.linecorp.centraldogma.common.RepositoryNotFoundException; import com.linecorp.centraldogma.common.RevisionNotFoundException; +import com.linecorp.centraldogma.common.TextPatchConflictException; import com.linecorp.centraldogma.common.TooManyRequestsException; -import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchConflictException; import com.linecorp.centraldogma.server.internal.storage.RequestAlreadyTimedOutException; import com.linecorp.centraldogma.server.internal.storage.repository.RepositoryMetadataException; import com.linecorp.centraldogma.server.metadata.MemberNotFoundException; @@ -90,7 +91,9 @@ public final class HttpApiExceptionHandler implements ServerErrorHandler { .put(RepositoryExistsException.class, (ctx, cause) -> newResponse(ctx, HttpStatus.CONFLICT, cause, "Repository '%s' exists already.", cause.getMessage())) - .put(JsonPatchException.class, + .put(JsonPatchConflictException.class, + (ctx, cause) -> newResponse(ctx, HttpStatus.CONFLICT, cause)) + .put(TextPatchConflictException.class, (ctx, cause) -> newResponse(ctx, HttpStatus.CONFLICT, cause)) .put(RepositoryMetadataException.class, (ctx, cause) -> newResponse(ctx, HttpStatus.INTERNAL_SERVER_ERROR, cause)) diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java index 75fde09009..af04a7add1 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java @@ -43,7 +43,8 @@ import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.ChangeConflictException; import com.linecorp.centraldogma.common.Revision; -import com.linecorp.centraldogma.common.jsonpatch.JsonPatchException; +import com.linecorp.centraldogma.common.TextPatchConflictException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchConflictException; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.internal.Util; import com.linecorp.centraldogma.internal.jsonpatch.JsonPatch; @@ -159,7 +160,11 @@ int doApply(Revision unused, DirCache dirCache, try { newJsonNode = JsonPatch.fromJson((JsonNode) change.content()).apply(oldJsonNode); } catch (Exception e) { - throw new JsonPatchException("failed to apply JSON patch: " + change, e); + if (e instanceof JsonPatchConflictException) { + throw (JsonPatchConflictException) e; + } else { + throw new JsonPatchConflictException("failed to apply JSON patch: " + change, e); + } } // Apply only when the contents are really different. @@ -196,7 +201,11 @@ int doApply(Revision unused, DirCache dirCache, newText = joiner.toString(); } } catch (Exception e) { - throw new ChangeConflictException("failed to apply text patch: " + change, e); + String message = "failed to apply text patch: " + change; + if (e.getMessage() != null) { + message += " (reason: " + e.getMessage() + ')'; + } + throw new TextPatchConflictException(message, e); } // Apply only when the contents are really different. 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 3ff5dc3a41..87d2b6a59e 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 @@ -81,6 +81,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import com.google.common.collect.Streams; import com.linecorp.armeria.common.CommonPools; @@ -857,9 +858,7 @@ public CompletableFuture commit( requireNonNull(markup, "markup"); requireNonNull(changes, "changes"); - // JsonPatch operations its own validation for the changes so we don't need to validate them here. - final boolean allowEmptyCommit = Streams.stream(changes).allMatch( - change -> change.type() == ChangeType.APPLY_JSON_PATCH); + final boolean allowEmptyCommit = shouldAllowEmptyCommit(changes); final CommitExecutor commitExecutor = new CommitExecutor(this, commitTimeMillis, author, summary, detail, markup, allowEmptyCommit); return commit(baseRevision, commitExecutor, normBaseRevision -> { @@ -870,6 +869,16 @@ public CompletableFuture commit( }); } + private boolean shouldAllowEmptyCommit(Iterable> changes) { + // allMatch returns true if the stream is empty. + if (Iterables.isEmpty(changes)) { + return false; + } + // JsonPatch operations its own validation for the changes so we don't need to validate them here. + return Streams.stream(changes) + .allMatch(change -> change.type() == ChangeType.APPLY_JSON_PATCH); + } + @Override public CompletableFuture commit(Revision baseRevision, long commitTimeMillis, Author author, String summary, String detail, Markup markup, 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 b2c49c8ca8..6767296c33 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 @@ -76,6 +76,8 @@ import com.linecorp.centraldogma.common.RedundantChangeException; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.common.RevisionNotFoundException; +import com.linecorp.centraldogma.common.TextPatchConflictException; +import com.linecorp.centraldogma.common.jsonpatch.JsonPatchConflictException; import com.linecorp.centraldogma.internal.Util; import com.linecorp.centraldogma.server.internal.JGitUtil; import com.linecorp.centraldogma.server.storage.StorageException; @@ -241,20 +243,20 @@ void testJsonPatch_safeReplace(TestInfo testInfo) { assertThatThrownBy( () -> repo.commit(HEAD, 0L, Author.UNKNOWN, SUMMARY, nextChange).join()) .isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(ChangeConflictException.class); + .hasCauseInstanceOf(JsonPatchConflictException.class); } @Test void testJsonPatch() { - testPatch(jsonPatches, jsonUpserts); + testPatch(jsonPatches, jsonUpserts, true); } @Test void testTextPatch() { - testPatch(textPatches, textUpserts); + testPatch(textPatches, textUpserts, false); } - private static void testPatch(Change[] patches, Change[] upserts) { + private static void testPatch(Change[] patches, Change[] upserts, boolean jsonPatch) { final String path = patches[0].path(); for (int i = 0; i < NUM_ITERATIONS; i++) { assert path.equals(patches[i].path()); @@ -267,7 +269,8 @@ private static void testPatch(Change[] patches, Change[] upserts) { assertThatThrownBy( () -> repo.commit(HEAD, 0L, Author.UNKNOWN, SUMMARY, patches[finalJ]).join()) .isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(ChangeConflictException.class); + .hasCauseExactlyInstanceOf(jsonPatch ? JsonPatchConflictException.class : + TextPatchConflictException.class); } // Ensure that the failed commit does not change the revision. @@ -558,7 +561,7 @@ void testMultipleChangesWithConflict() { assertThatThrownBy(() -> repo .commit(HEAD, 0L, Author.UNKNOWN, SUMMARY, jsonUpserts[0], jsonPatches[2]).join()) .isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(ChangeConflictException.class); + .hasCauseInstanceOf(JsonPatchConflictException.class); } /** @@ -599,7 +602,7 @@ void testPreviewDiff() { // Invalid patch assertThatThrownBy(() -> repo.previewDiff(HEAD, jsonPatches[1]).join()) .isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(ChangeConflictException.class); + .hasCauseInstanceOf(JsonPatchConflictException.class); // Invalid removal assertThatThrownBy(() -> repo.previewDiff(HEAD, Change.ofRemoval(jsonPaths[0])).join()) @@ -662,7 +665,9 @@ void testDiff_add() { // PATCH_TO_UPSERT diff option. final Map> diffUpsert = repo.diff( - prevRevision, currRevision, Repository.ALL_PATH, DiffResultType.PATCH_TO_UPSERT) + prevRevision, currRevision, + Repository.ALL_PATH, + DiffResultType.PATCH_TO_UPSERT) .join(); assertThat(diffUpsert).hasSize(2) .containsEntry(jsonPath, jsonUpsert) @@ -766,13 +771,17 @@ void testDiff_modify() { // PATCH_TO_UPSERT diff option. final Map> diffUpsert = repo.diff( - prevRevision, currRevision, Repository.ALL_PATH, DiffResultType.PATCH_TO_UPSERT) + prevRevision, currRevision, + Repository.ALL_PATH, + DiffResultType.PATCH_TO_UPSERT) .join(); assertThat(diffUpsert).hasSize(2) .containsEntry(jsonNodePath, expectedJsonUpsert) .containsEntry(textNodePath, expectedTextUpsert); final Change jsonQueryUpsert = repo.diff( - prevRevision, currRevision, Query.ofJson(jsonNodePath), DiffResultType.PATCH_TO_UPSERT) + prevRevision, currRevision, + Query.ofJson(jsonNodePath), + DiffResultType.PATCH_TO_UPSERT) .join(); assertThat(jsonQueryUpsert).isEqualTo(expectedJsonUpsert); final Change textQueryUpsert = repo.diff(prevRevision, currRevision, Query.ofText(textNodePath), From c709af4856b4bb478dd0a4e80f713786439b0463 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Fri, 17 Jan 2025 18:41:47 +0900 Subject: [PATCH 3/7] fix tests --- .../common/jsonpatch/package-info.java | 8 +-- .../JsonPatchOperationIntegrationTest.java | 51 ++++++++++--------- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java index 4de3a53296..89dbb24f73 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java @@ -15,18 +15,12 @@ */ /** - * Implementation of JSON Patch. + * Implementation of JSON Patch. * *

As its name implies, JSON Patch is a mechanism designed to modify JSON * documents. It consists of a series of operations to apply in order to the * source JSON document until all operations are applied or an error has been * encountered.

- * - *

The main class is {@link com.linecorp.centraldogma.internal.jsonpatch.JsonPatch}.

- * - *

Note that at this moment, the only way to build a patch is from a JSON - * representation (as a {@link com.fasterxml.jackson.databind.JsonNode}).

- * */ @NonNullByDefault package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java index bbd7d235fb..7f1b9f2339 100644 --- a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java @@ -20,7 +20,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import java.util.List; import java.util.concurrent.CompletionException; import org.junit.jupiter.api.BeforeEach; @@ -30,15 +29,14 @@ import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonPointer; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.IntNode; import com.google.common.collect.ImmutableList; import com.linecorp.centraldogma.client.CentralDogma; import com.linecorp.centraldogma.client.CentralDogmaRepository; import com.linecorp.centraldogma.common.Change; +import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; class JsonPatchOperationIntegrationTest { @@ -65,7 +63,7 @@ void setUp() { } @Test - void testAdd() throws JsonParseException { + void add() throws JsonParseException { repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) .push() .join(); @@ -80,7 +78,7 @@ void testAdd() throws JsonParseException { } @Test - void testCopy() throws JsonParseException { + void copy() throws JsonParseException { repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1 }")) .push() .join(); @@ -112,7 +110,7 @@ void testMove() throws JsonParseException { } @Test - void testRemove() throws JsonParseException { + void remove() throws JsonParseException { repository.commit("add ab", Change.ofJsonUpsert("/a.json", "{ \"a\": 1, \"b\": 2 }")) .push() .join(); @@ -130,8 +128,7 @@ void testRemove() throws JsonParseException { .join()) .isInstanceOf(CompletionException.class) .hasCauseInstanceOf(JsonPatchConflictException.class) - .hasMessageContaining("failed to apply JSON patch:") - .hasMessageContaining("path=/a.json, content=[{\"op\":\"remove\",\"path\":\"/a\"}]"); + .hasMessageContaining("non-existent path: /a"); } @Test @@ -195,9 +192,7 @@ void safeReplace() throws JsonParseException { .join(); }).isInstanceOf(CompletionException.class) .hasCauseInstanceOf(JsonPatchConflictException.class) - .hasMessageContaining( - "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, path=/a.json, " + - "content=[{\"op\":\"safeReplace\",\"path\":\"/a\",\"oldValue\":3,\"value\":4}]}"); + .hasMessageContaining("mismatching value at '/a': 2 (expected: 3)"); } @Test @@ -219,9 +214,7 @@ void test() { .join(); }).isInstanceOf(CompletionException.class) .hasCauseInstanceOf(JsonPatchConflictException.class) - .hasMessageContaining( - "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, " + - "path=/a.json, content=[{\"op\":\"test\",\"path\":\"/a\",\"value\":2}]}"); + .hasMessageContaining("mismatching value at '/a': 1 (expected: 2)"); } @Test @@ -243,9 +236,7 @@ void testAbsence() { .join(); }).isInstanceOf(CompletionException.class) .hasCauseInstanceOf(JsonPatchConflictException.class) - .hasMessageContaining( - "failed to apply JSON patch: Change{type=APPLY_JSON_PATCH, " + - "path=/a.json, content=[{\"op\":\"testAbsence\",\"path\":\"/a\"}]}"); + .hasMessageContaining("existent path: /a"); } @Test @@ -277,11 +268,25 @@ void testMultipleOperations() throws JsonParseException { @Test void testEquality() throws JsonProcessingException { - final RemoveOperation remove = JsonPatchOperation.remove(JsonPointer.compile("/a")); - final JsonNode jsonNode = remove.toJsonNode(); - final ObjectMapper mapper = new ObjectMapper(); - final List jsonPatchOperations = - mapper.treeToValue(jsonNode, new TypeReference>() {}); - assertThat(jsonPatchOperations.get(0)).isEqualTo(remove); + ensureSerdesEquality(JsonPatchOperation.add(JsonPointer.compile("/a"), new IntNode(1)), AddOperation.class); + ensureSerdesEquality(JsonPatchOperation.copy(JsonPointer.compile("/a"), JsonPointer.compile("/b")), + CopyOperation.class); + ensureSerdesEquality(JsonPatchOperation.move(JsonPointer.compile("/a"), JsonPointer.compile("/b")), + MoveOperation.class); + ensureSerdesEquality(JsonPatchOperation.remove(JsonPointer.compile("/a")), RemoveOperation.class); + ensureSerdesEquality(JsonPatchOperation.removeIfExists(JsonPointer.compile("/a")), RemoveIfExistsOperation.class); + ensureSerdesEquality(JsonPatchOperation.replace(JsonPointer.compile("/a"), new IntNode(1)), ReplaceOperation.class); + ensureSerdesEquality(JsonPatchOperation.safeReplace(JsonPointer.compile("/a"), new IntNode(1), new IntNode(2)), + SafeReplaceOperation.class); + ensureSerdesEquality(JsonPatchOperation.test(JsonPointer.compile("/a"), new IntNode(1)), TestOperation.class); + ensureSerdesEquality(JsonPatchOperation.testAbsence(JsonPointer.compile("/a")), TestAbsenceOperation.class); + } + + private static void ensureSerdesEquality(T operation, Class clazz) + throws JsonProcessingException { + final String json = Jackson.writeValueAsString(operation); + final JsonNode jsonNode = Jackson.readTree(json); + final T deserialized = Jackson.convertValue(jsonNode, clazz); + assertThat(deserialized).isEqualTo(operation); } } From 7c28e785012e61ddc47bc27a3298f41905399d7b Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Fri, 17 Jan 2025 18:46:54 +0900 Subject: [PATCH 4/7] lint --- .../client/armeria/ArmeriaCentralDogma.java | 3 +-- .../common/jsonpatch/AddOperation.java | 3 ++- .../JsonPatchOperationIntegrationTest.java | 20 ++++++++++++------- .../storage/repository/git/GitRepository.java | 20 +++++++++---------- .../repository/git/GitRepositoryTest.java | 4 ++-- 5 files changed, 28 insertions(+), 22 deletions(-) diff --git a/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java b/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java index 022661ead7..d7743bb6d5 100644 --- a/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java +++ b/client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java @@ -157,8 +157,7 @@ public ArmeriaCentralDogma(ScheduledExecutorService blockingTaskExecutor, @Override public CompletableFuture whenEndpointReady() { - return client.endpointGroup().whenReady().thenRun(() -> { - }); + return client.endpointGroup().whenReady().thenRun(() -> {}); } @Override diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java index 6338940070..6ab2eba06e 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java @@ -99,7 +99,8 @@ static JsonNode addToArray(final JsonPointer path, final JsonNode node, final Js try { index = Integer.parseInt(rawToken); } catch (NumberFormatException ignored) { - throw new JsonPatchConflictException("not an index: " + rawToken + " (expected: a non-negative integer)"); + throw new JsonPatchConflictException( + "not an index: " + rawToken + " (expected: a non-negative integer)"); } if (index < 0 || index > size) { diff --git a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java index 7f1b9f2339..f6ae064ef5 100644 --- a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java @@ -268,18 +268,24 @@ void testMultipleOperations() throws JsonParseException { @Test void testEquality() throws JsonProcessingException { - ensureSerdesEquality(JsonPatchOperation.add(JsonPointer.compile("/a"), new IntNode(1)), AddOperation.class); + ensureSerdesEquality(JsonPatchOperation.add(JsonPointer.compile("/a"), new IntNode(1)), + AddOperation.class); ensureSerdesEquality(JsonPatchOperation.copy(JsonPointer.compile("/a"), JsonPointer.compile("/b")), CopyOperation.class); ensureSerdesEquality(JsonPatchOperation.move(JsonPointer.compile("/a"), JsonPointer.compile("/b")), MoveOperation.class); ensureSerdesEquality(JsonPatchOperation.remove(JsonPointer.compile("/a")), RemoveOperation.class); - ensureSerdesEquality(JsonPatchOperation.removeIfExists(JsonPointer.compile("/a")), RemoveIfExistsOperation.class); - ensureSerdesEquality(JsonPatchOperation.replace(JsonPointer.compile("/a"), new IntNode(1)), ReplaceOperation.class); - ensureSerdesEquality(JsonPatchOperation.safeReplace(JsonPointer.compile("/a"), new IntNode(1), new IntNode(2)), - SafeReplaceOperation.class); - ensureSerdesEquality(JsonPatchOperation.test(JsonPointer.compile("/a"), new IntNode(1)), TestOperation.class); - ensureSerdesEquality(JsonPatchOperation.testAbsence(JsonPointer.compile("/a")), TestAbsenceOperation.class); + ensureSerdesEquality(JsonPatchOperation.removeIfExists(JsonPointer.compile("/a")), + RemoveIfExistsOperation.class); + ensureSerdesEquality(JsonPatchOperation.replace(JsonPointer.compile("/a"), new IntNode(1)), + ReplaceOperation.class); + ensureSerdesEquality( + JsonPatchOperation.safeReplace(JsonPointer.compile("/a"), new IntNode(1), new IntNode(2)), + SafeReplaceOperation.class); + ensureSerdesEquality(JsonPatchOperation.test(JsonPointer.compile("/a"), new IntNode(1)), + TestOperation.class); + ensureSerdesEquality(JsonPatchOperation.testAbsence(JsonPointer.compile("/a")), + TestAbsenceOperation.class); } private static void ensureSerdesEquality(T operation, Class clazz) 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 87d2b6a59e..426fd80b91 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 @@ -847,6 +847,16 @@ private static void putChange(Map> changeMap, String path, Cha assert oldChange == null; } + private static boolean shouldAllowEmptyCommit(Iterable> changes) { + // allMatch returns true if the stream is empty. + if (Iterables.isEmpty(changes)) { + return false; + } + // JsonPatch operations its own validation for the changes so we don't need to validate them here. + return Streams.stream(changes) + .allMatch(change -> change.type() == ChangeType.APPLY_JSON_PATCH); + } + @Override public CompletableFuture commit( Revision baseRevision, long commitTimeMillis, Author author, String summary, @@ -869,16 +879,6 @@ public CompletableFuture commit( }); } - private boolean shouldAllowEmptyCommit(Iterable> changes) { - // allMatch returns true if the stream is empty. - if (Iterables.isEmpty(changes)) { - return false; - } - // JsonPatch operations its own validation for the changes so we don't need to validate them here. - return Streams.stream(changes) - .allMatch(change -> change.type() == ChangeType.APPLY_JSON_PATCH); - } - @Override public CompletableFuture commit(Revision baseRevision, long commitTimeMillis, Author author, String summary, String detail, Markup markup, 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 6767296c33..39fd3bf549 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 @@ -269,8 +269,8 @@ private static void testPatch(Change[] patches, Change[] upserts, boolean assertThatThrownBy( () -> repo.commit(HEAD, 0L, Author.UNKNOWN, SUMMARY, patches[finalJ]).join()) .isInstanceOf(CompletionException.class) - .hasCauseExactlyInstanceOf(jsonPatch ? JsonPatchConflictException.class : - TextPatchConflictException.class); + .hasCauseExactlyInstanceOf(jsonPatch ? JsonPatchConflictException.class + : TextPatchConflictException.class); } // Ensure that the failed commit does not change the revision. From f50f4920b447ed69eabf67350728cfad8a1c02f0 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Fri, 17 Jan 2025 18:54:17 +0900 Subject: [PATCH 5/7] copyright --- .../common/jsonpatch/AddOperation.java | 20 ++++++++++++++++++- .../common/jsonpatch/CopyOperation.java | 20 ++++++++++++++++++- .../common/jsonpatch/DualPathOperation.java | 20 ++++++++++++++++++- .../common/jsonpatch/JsonPatchOperation.java | 20 ++++++++++++++++++- .../common/jsonpatch/MoveOperation.java | 20 ++++++++++++++++++- .../common/jsonpatch/PathValueOperation.java | 20 ++++++++++++++++++- .../jsonpatch/RemoveIfExistsOperation.java | 2 +- .../common/jsonpatch/RemoveOperation.java | 20 ++++++++++++++++++- .../common/jsonpatch/ReplaceOperation.java | 20 ++++++++++++++++++- .../jsonpatch/SafeReplaceOperation.java | 2 +- .../jsonpatch/TestAbsenceOperation.java | 2 +- .../common/jsonpatch/TestOperation.java | 20 ++++++++++++++++++- 12 files changed, 174 insertions(+), 12 deletions(-) diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java index 6ab2eba06e..a47b51f06f 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/AddOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java index d9ea5a5b03..e1d7c3cbf8 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/CopyOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/DualPathOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/DualPathOperation.java index 174cba02b9..f1779a5121 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/DualPathOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/DualPathOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java index d8ee15c32a..829bc4a907 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java index 421770c0b9..4bdc6f0a98 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/MoveOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java index cb3ab39f6e..b7fccf5c37 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/PathValueOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveIfExistsOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveIfExistsOperation.java index 125cc3b7f7..0e6a33046c 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveIfExistsOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveIfExistsOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2018 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 diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveOperation.java index 8a4189c11c..8eb98e1c9a 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/RemoveOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/ReplaceOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/ReplaceOperation.java index 13b8c92f60..b75ac120ba 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/ReplaceOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/ReplaceOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java index 4641968534..f9f7020798 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/SafeReplaceOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java index 2c199c7ae8..1286549502 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestAbsenceOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestOperation.java index b61edb2534..d9e9d16191 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/TestOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 LINE Corporation + * Copyright 2017 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 @@ -13,6 +13,24 @@ * License for the specific language governing permissions and limitations * under the License. */ +/* + * Copyright (c) 2014, Francis Galiegue (fgaliegue@gmail.com) + * + * This software is dual-licensed under: + * + * - the Lesser General Public License (LGPL) version 3.0 or, at your option, any + * later version; + * - the Apache Software License (ASL) version 2.0. + * + * The text of this file and of both licenses is available at the root of this + * project or, if you have the jar distribution, in directory META-INF/, under + * the names LGPL-3.0.txt and ASL-2.0.txt respectively. + * + * Direct link to the sources: + * + * - LGPL 3.0: https://www.gnu.org/licenses/lgpl-3.0.txt + * - ASL 2.0: https://www.apache.org/licenses/LICENSE-2.0.txt + */ package com.linecorp.centraldogma.common.jsonpatch; From 1d2c74e0ee4dc66ba65f09c293662fc3c5d6989e Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Tue, 21 Jan 2025 16:19:35 +0900 Subject: [PATCH 6/7] add shortcut factory methods --- .../common/jsonpatch/JsonPatchOperation.java | 112 +++++++++++++++++- .../JsonPatchOperationIntegrationTest.java | 26 ++++ 2 files changed, 132 insertions(+), 6 deletions(-) diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java index 829bc4a907..ed73f6f898 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java @@ -108,24 +108,54 @@ public static AddOperation add(JsonPointer path, JsonNode value) { return new AddOperation(path, value); } + /** + * Creates a new JSON Patch {@code add} operation. + * + * @param path the JSON Pointer for this operation + * @param value the value to add + */ + public static AddOperation add(String path, JsonNode value) { + requireNonNull(path, "path"); + return add(JsonPointer.compile(path), value); + } + + /** + * Creates a new JSON Patch {@code copy} operation. + * + * @param from the source JSON Pointer + * @param to the destination JSON Pointer + */ + public static CopyOperation copy(JsonPointer from, JsonPointer to) { + return new CopyOperation(from, to); + } + /** * Creates a new JSON Patch {@code copy} operation. * * @param from the source JSON Pointer - * @param path the destination JSON Pointer + * @param to the destination JSON Pointer */ - public static CopyOperation copy(JsonPointer from, JsonPointer path) { - return new CopyOperation(from, path); + public static CopyOperation copy(String from, String to) { + requireNonNull(from, "from"); + requireNonNull(to, "to"); + return copy(JsonPointer.compile(from), JsonPointer.compile(to)); } + /** * Creates a new JSON Patch {@code move} operation. * * @param from the source JSON Pointer - * @param path the destination JSON Pointer + * @param to the destination JSON Pointer */ - public static MoveOperation move(JsonPointer from, JsonPointer path) { - return new MoveOperation(from, path); + public static MoveOperation move(JsonPointer from, JsonPointer to) { + return new MoveOperation(from, to); + } + + public static MoveOperation move(String from, String to) { + requireNonNull(from, "from"); + requireNonNull(to, "to"); + return move(JsonPointer.compile(from), JsonPointer.compile(to)); } /** @@ -139,6 +169,18 @@ public static RemoveOperation remove(JsonPointer path) { return new RemoveOperation(path); } + /** + * Creates a new JSON Patch {@code remove} operation. + * + *

Note that this operation will throw an exception if the path does not exist. + * + * @param path the JSON Pointer to remove + */ + public static RemoveOperation remove(String path) { + requireNonNull(path, "path"); + return remove(JsonPointer.compile(path)); + } + /** * Creates a new JSON Patch {@code removeIfExists} operation. * @@ -148,6 +190,16 @@ public static RemoveIfExistsOperation removeIfExists(JsonPointer path) { return new RemoveIfExistsOperation(path); } + /** + * Creates a new JSON Patch {@code removeIfExists} operation. + * + * @param path the JSON Pointer to remove if it exists + */ + public static RemoveIfExistsOperation removeIfExists(String path) { + requireNonNull(path, "path"); + return removeIfExists(JsonPointer.compile(path)); + } + /** * Creates a new JSON Patch {@code replace} operation. * @@ -158,6 +210,17 @@ public static ReplaceOperation replace(JsonPointer path, JsonNode value) { return new ReplaceOperation(path, value); } + /** + * Creates a new JSON Patch {@code replace} operation. + * + * @param path the JSON Pointer for this operation + * @param value the new value to replace the existing value + */ + public static ReplaceOperation replace(String path, JsonNode value) { + requireNonNull(path, "path"); + return replace(JsonPointer.compile(path), value); + } + /** * Creates a new JSON Patch {@code safeReplace} operation. * @@ -169,6 +232,18 @@ public static SafeReplaceOperation safeReplace(JsonPointer path, JsonNode oldVal return new SafeReplaceOperation(path, oldValue, newValue); } + /** + * Creates a new JSON Patch {@code safeReplace} operation. + * + * @param path the JSON Pointer for this operation + * @param oldValue the old value to replace + * @param newValue the new value to replace the old value + */ + public static SafeReplaceOperation safeReplace(String path, JsonNode oldValue, JsonNode newValue) { + requireNonNull(path, "path"); + return safeReplace(JsonPointer.compile(path), oldValue, newValue); + } + /** * Creates a new JSON Patch {@code test} operation. * @@ -181,6 +256,19 @@ public static TestOperation test(JsonPointer path, JsonNode value) { return new TestOperation(path, value); } + /** + * Creates a new JSON Patch {@code test} operation. + * + *

This operation will throw an exception if the value at the path does not match the expected value. + * + * @param path the JSON Pointer for this operation + * @param value the value to test + */ + public static TestOperation test(String path, JsonNode value) { + requireNonNull(path, "path"); + return test(JsonPointer.compile(path), value); + } + /** * Creates a new JSON Patch {@code testAbsent} operation. * @@ -192,6 +280,18 @@ public static TestAbsenceOperation testAbsence(JsonPointer path) { return new TestAbsenceOperation(path); } + /** + * Creates a new JSON Patch {@code testAbsent} operation. + * + *

This operation will throw an exception if the value at the path exists. + * + * @param path the JSON Pointer for this operation + */ + public static TestAbsenceOperation testAbsence(String path) { + requireNonNull(path, "path"); + return testAbsence(JsonPointer.compile(path)); + } + /** * Converts {@link JsonPatchOperation}s to an array of {@link JsonNode}. */ diff --git a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java index f6ae064ef5..45a62d830e 100644 --- a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java @@ -266,6 +266,32 @@ void testMultipleOperations() throws JsonParseException { .isEqualTo("{ \"c\": 4, \"d\": 1 }"); } + @Test + void testMultipleOperationsWithStringPath() throws JsonParseException { + repository.commit("add a", Change.ofJsonUpsert("/a.json", "{ \"a\": 1, \"b\": 2 }")) + .push() + .join(); + // { "a": 1, "b": 2 } -> { "a": 1, "b": 2, "c": 3 } + final AddOperation add = JsonPatchOperation.add("/c", new IntNode(3)); + // { "a": 1, "b": 2, "c": 3 } -> { "b": 2, "c": 3, "d": 1 } + final MoveOperation move = JsonPatchOperation.move("/a", "/d"); + // { "b": 2, "c": 3, "d": 1 } -> { "c": 3, "d": 1 } + final RemoveOperation remove = JsonPatchOperation.remove("/b"); + // { "c": 3, "d": 1 } -> { "c": 4, "d": 1 } + final SafeReplaceOperation safeReplace = + JsonPatchOperation.safeReplace("/c", new IntNode(3), new IntNode(4)); + + final Change change = Change.ofJsonPatch("/a.json", + ImmutableList.of(add, move, remove, safeReplace)); + + repository.commit("json patch operations", change) + .push() + .join(); + + assertThatJson(repository.file("/a.json").get().join().contentAsJson()) + .isEqualTo("{ \"c\": 4, \"d\": 1 }"); + } + @Test void testEquality() throws JsonProcessingException { ensureSerdesEquality(JsonPatchOperation.add(JsonPointer.compile("/a"), new IntNode(1)), From 5630ad252fbf809e477975bb598f163bc8bb58ae Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Thu, 23 Jan 2025 15:43:57 +0900 Subject: [PATCH 7/7] Address comments --- .../common/jsonpatch/JsonPatchOperation.java | 10 ++++-- .../common/jsonpatch/package-info.java | 2 +- .../internal/jsonpatch/JsonPatch.java | 2 +- .../JsonPatchOperationIntegrationTest.java | 21 ++++++++----- .../repository/git/CommitExecutor.java | 29 ++++++++++++----- .../repository/git/DefaultChangesApplier.java | 8 ++--- .../storage/repository/git/GitRepository.java | 31 ++++++++++++------- 7 files changed, 67 insertions(+), 36 deletions(-) diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java index ed73f6f898..0113acb744 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperation.java @@ -51,11 +51,11 @@ import com.linecorp.centraldogma.internal.Jackson; /** - * Base abstract class for one JSON + * Base abstract class for one JSON * Patch operation. A {@link JsonPatchOperation} can be converted into a JSON patch by calling * {@link #toJsonNode()}. * - *

JSON + *

JSON * Patch, as its name implies, is an IETF draft describing a mechanism to * apply a patch to any JSON value. This implementation covers all operations * according to the specification; however, there are some subtle differences @@ -152,6 +152,12 @@ public static MoveOperation move(JsonPointer from, JsonPointer to) { return new MoveOperation(from, to); } + /** + * Creates a new JSON Patch {@code move} operation. + * + * @param from the source JSON Pointer + * @param to the destination JSON Pointer + */ public static MoveOperation move(String from, String to) { requireNonNull(from, "from"); requireNonNull(to, "to"); diff --git a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java index 89dbb24f73..13359235d4 100644 --- a/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java +++ b/common/src/main/java/com/linecorp/centraldogma/common/jsonpatch/package-info.java @@ -15,7 +15,7 @@ */ /** - * Implementation of JSON Patch. + * Implementation of JSON Patch. * *

As its name implies, JSON Patch is a mechanism designed to modify JSON * documents. It consists of a series of operations to apply in order to the diff --git a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java index fb7ec3a0c2..3035e15e8d 100644 --- a/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java +++ b/common/src/main/java/com/linecorp/centraldogma/internal/jsonpatch/JsonPatch.java @@ -69,7 +69,7 @@ /** * Implementation of JSON Patch. * - *

JSON + *

JSON * Patch, as its name implies, is an IETF draft describing a mechanism to * apply a patch to any JSON value. This implementation covers all operations * according to the specification; however, there are some subtle differences diff --git a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java index 45a62d830e..e70b7d44a3 100644 --- a/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java +++ b/common/src/test/java/com/linecorp/centraldogma/common/jsonpatch/JsonPatchOperationIntegrationTest.java @@ -36,6 +36,8 @@ import com.linecorp.centraldogma.client.CentralDogma; import com.linecorp.centraldogma.client.CentralDogmaRepository; import com.linecorp.centraldogma.common.Change; +import com.linecorp.centraldogma.common.Entry; +import com.linecorp.centraldogma.common.PushResult; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; @@ -139,19 +141,22 @@ void removeIfExists() throws JsonParseException { final RemoveIfExistsOperation removeIfExists = JsonPatchOperation.removeIfExists(JsonPointer.compile("/a")); final Change change = Change.ofJsonPatch("/a.json", removeIfExists); - repository.commit("remove a", change) - .push() - .join(); + final PushResult result0 = repository.commit("remove a", change) + .push() + .join(); JsonNode jsonNode = repository.file("/a.json").get().join().contentAsJson(); assertThatJson(jsonNode).isEqualTo("{ \"b\": 2 }"); - repository.commit("remove a again", change) - .push() - .join(); + final PushResult result1 = repository.commit("remove a again", change) + .push() + .join(); + // Should not increase the revision if the path is absent and the history must be the same. + assertThat(result1.revision()).isEqualTo(result0.revision()); - jsonNode = repository.file("/a.json").get().join().contentAsJson(); - assertThatJson(jsonNode).isEqualTo("{ \"b\": 2 }"); + final Entry data = repository.file("/a.json").get().join(); + assertThat(data.revision()).isEqualTo(result0.revision()); + assertThatJson(data.contentAsJson()).isEqualTo("{ \"b\": 2 }"); } @Test diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/CommitExecutor.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/CommitExecutor.java index b3bccb8833..c1390810db 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/CommitExecutor.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/CommitExecutor.java @@ -61,17 +61,17 @@ final class CommitExecutor { private final String summary; private final String detail; private final Markup markup; - private final boolean allowEmptyCommit; + private final EmptyCommitPolicy emptyCommitPolicy; CommitExecutor(GitRepository gitRepository, long commitTimeMillis, Author author, - String summary, String detail, Markup markup, boolean allowEmptyCommit) { + String summary, String detail, Markup markup, EmptyCommitPolicy emptyCommitPolicy) { this.gitRepository = gitRepository; this.commitTimeMillis = commitTimeMillis; this.author = author; this.summary = summary; this.detail = detail; this.markup = markup; - this.allowEmptyCommit = allowEmptyCommit; + this.emptyCommitPolicy = emptyCommitPolicy; } Author author() { @@ -153,11 +153,18 @@ RevisionAndEntries commit(@Nullable Revision headRevision, Revision nextRevision } else { diffEntries = ImmutableList.of(); } - if (!allowEmptyCommit && isEmpty) { - throw new RedundantChangeException( - headRevision, - "changes did not change anything in " + gitRepository.parent().name() + '/' + - gitRepository.name() + " at revision " + headRevision.major() + ": " + changes); + if (isEmpty) { + switch (emptyCommitPolicy) { + case ALLOW: + break; + case DISALLOW: + throw new RedundantChangeException( + headRevision, + "changes did not change anything in " + gitRepository.parent().name() + '/' + + gitRepository.name() + " at revision " + headRevision.major() + ": " + changes); + case IGNORE: + return new RevisionAndEntries(headRevision, diffEntries); + } } } else { // initial commit. @@ -211,4 +218,10 @@ static final class RevisionAndEntries { this.diffEntries = diffEntries; } } + + enum EmptyCommitPolicy { + ALLOW, + DISALLOW, + IGNORE + } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java index af04a7add1..3931a7ce1b 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java @@ -159,12 +159,10 @@ int doApply(Revision unused, DirCache dirCache, final JsonNode newJsonNode; try { newJsonNode = JsonPatch.fromJson((JsonNode) change.content()).apply(oldJsonNode); + } catch (JsonPatchConflictException e) { + throw e; } catch (Exception e) { - if (e instanceof JsonPatchConflictException) { - throw (JsonPatchConflictException) e; - } else { - throw new JsonPatchConflictException("failed to apply JSON patch: " + change, e); - } + throw new JsonPatchConflictException("failed to apply JSON patch: " + change, e); } // Apply only when the contents are really different. 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 426fd80b91..b66dd88125 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 @@ -111,6 +111,7 @@ import com.linecorp.centraldogma.server.internal.IsolatedSystemReader; import com.linecorp.centraldogma.server.internal.JGitUtil; import com.linecorp.centraldogma.server.internal.storage.repository.RepositoryCache; +import com.linecorp.centraldogma.server.internal.storage.repository.git.CommitExecutor.EmptyCommitPolicy; import com.linecorp.centraldogma.server.storage.StorageException; import com.linecorp.centraldogma.server.storage.project.Project; import com.linecorp.centraldogma.server.storage.repository.DiffResultType; @@ -251,7 +252,7 @@ class GitRepository implements Repository { commitIdDatabase = new CommitIdDatabase(jGitRepository); new CommitExecutor(this, creationTimeMillis, author, "Create a new repository", "", - Markup.PLAINTEXT, true) + Markup.PLAINTEXT, EmptyCommitPolicy.ALLOW) .executeInitialCommit(); headRevision = Revision.INIT; @@ -847,14 +848,22 @@ private static void putChange(Map> changeMap, String path, Cha assert oldChange == null; } - private static boolean shouldAllowEmptyCommit(Iterable> changes) { + private static EmptyCommitPolicy getEmptyCommitPolicy(Iterable> changes) { // allMatch returns true if the stream is empty. if (Iterables.isEmpty(changes)) { - return false; + // Disallowed by default + return EmptyCommitPolicy.DISALLOW; + } + final boolean allJsonPatches = + Streams.stream(changes) + .allMatch(change -> change.type() == ChangeType.APPLY_JSON_PATCH); + if (allJsonPatches) { + // JsonPatch operations may have an empty change that should be ignored. + // For example, the removeIfExists operation does nothing if the target path is absent. + return EmptyCommitPolicy.IGNORE; + } else { + return EmptyCommitPolicy.DISALLOW; } - // JsonPatch operations its own validation for the changes so we don't need to validate them here. - return Streams.stream(changes) - .allMatch(change -> change.type() == ChangeType.APPLY_JSON_PATCH); } @Override @@ -868,9 +877,9 @@ public CompletableFuture commit( requireNonNull(markup, "markup"); requireNonNull(changes, "changes"); - final boolean allowEmptyCommit = shouldAllowEmptyCommit(changes); + final EmptyCommitPolicy emptyCommitPolicy = getEmptyCommitPolicy(changes); final CommitExecutor commitExecutor = - new CommitExecutor(this, commitTimeMillis, author, summary, detail, markup, allowEmptyCommit); + new CommitExecutor(this, commitTimeMillis, author, summary, detail, markup, emptyCommitPolicy); return commit(baseRevision, commitExecutor, normBaseRevision -> { if (!directExecution) { return changes; @@ -890,7 +899,7 @@ public CompletableFuture commit(Revision baseRevision, long commit requireNonNull(markup, "markup"); requireNonNull(transformer, "transformer"); final CommitExecutor commitExecutor = - new CommitExecutor(this, commitTimeMillis, author, summary, detail, markup, false); + new CommitExecutor(this, commitTimeMillis, author, summary, detail, markup, EmptyCommitPolicy.DISALLOW); return commit(baseRevision, commitExecutor, normBaseRevision -> blockingPreviewDiff( normBaseRevision, new TransformingChangesApplier(transformer)).values()); @@ -1330,7 +1339,7 @@ public void cloneTo(File newRepoDir, BiConsumer progressListen try { new CommitExecutor(newRepo, c.when(), c.author(), c.summary(), - c.detail(), c.markup(), false) + c.detail(), c.markup(), EmptyCommitPolicy.DISALLOW) .execute(baseRevision, normBaseRevision -> blockingPreviewDiff( normBaseRevision, new DefaultChangesApplier(changes)).values()); previousNonEmptyRevision = revision; @@ -1338,7 +1347,7 @@ public void cloneTo(File newRepoDir, BiConsumer progressListen // NB: We allow an empty commit here because an old version of Central Dogma had a bug // which allowed the creation of an empty commit. new CommitExecutor(newRepo, c.when(), c.author(), c.summary(), - c.detail(), c.markup(), true) + c.detail(), c.markup(), EmptyCommitPolicy.ALLOW) .execute(baseRevision, normBaseRevision -> blockingPreviewDiff( normBaseRevision, new DefaultChangesApplier(changes)).values()); }