Skip to content

Commit

Permalink
polish
Browse files Browse the repository at this point in the history
  • Loading branch information
ikhoon committed Jan 17, 2025
1 parent fa878be commit 820316f
Show file tree
Hide file tree
Showing 19 changed files with 126 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 + ')');
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -341,7 +341,7 @@ public List<JsonPatchOperation> 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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\"}]");
}
Expand Down Expand Up @@ -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}]}");
Expand All @@ -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}]}");
Expand All @@ -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\"}]}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 820316f

Please sign in to comment.