You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ib...@apache.org on 2021/12/03 07:27:02 UTC
[ignite-3] branch main updated: IGNITE-16042 Fail NamedListChange#createAfter if called on a deleted key (#487)
This is an automated email from the ASF dual-hosted git repository.
ibessonov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new eefc59a IGNITE-16042 Fail NamedListChange#createAfter if called on a deleted key (#487)
eefc59a is described below
commit eefc59a707384c3e3e32c6b541700667f289c65e
Author: Alexander Polovtcev <al...@gmail.com>
AuthorDate: Fri Dec 3 10:26:56 2021 +0300
IGNITE-16042 Fail NamedListChange#createAfter if called on a deleted key (#487)
---
.../internal/configuration/tree/NamedListNode.java | 40 +++++++++++++++-------
.../configuration/tree/NamedListNodeTest.java | 35 +++++++++++++------
2 files changed, 51 insertions(+), 24 deletions(-)
diff --git a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/NamedListNode.java b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/NamedListNode.java
index d254864..f6df993 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/NamedListNode.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/NamedListNode.java
@@ -191,8 +191,12 @@ public final class NamedListNode<N> implements NamedListChange<N, N>, Traversabl
Objects.requireNonNull(key, "key");
Objects.requireNonNull(valConsumer, "valConsumer");
- if (!map.containsKey(precedingKey)) {
- throw new IllegalArgumentException("Element with name " + precedingKey + " doesn't exist.");
+ ElementDescriptor precedingElement = map.get(precedingKey);
+
+ if (precedingElement == null) {
+ throw elementMissingException(precedingKey);
+ } else if (precedingElement.value == null) {
+ throw elementRemovedException(precedingKey);
}
checkNewKey(key);
@@ -217,7 +221,7 @@ public final class NamedListNode<N> implements NamedListChange<N, N>, Traversabl
ElementDescriptor element = map.get(key);
if (element != null && element.value == null) {
- throw new IllegalArgumentException("Can't create entity that has just been deleted [key=" + key + ']');
+ throw elementRemovedException(key);
}
if (element == null) {
@@ -244,9 +248,9 @@ public final class NamedListNode<N> implements NamedListChange<N, N>, Traversabl
ElementDescriptor element = map.get(key);
if (element == null) {
- throw new IllegalArgumentException("Element with name " + key + " does not exist.");
+ throw elementMissingException(key);
} else if (element.value == null) {
- throw new IllegalArgumentException("Can't update entity that has just been deleted [key=" + key + ']');
+ throw elementRemovedException(key);
}
element = element.copy();
@@ -267,11 +271,9 @@ public final class NamedListNode<N> implements NamedListChange<N, N>, Traversabl
ElementDescriptor element = map.get(oldKey);
if (element == null) {
- throw new IllegalArgumentException("Element with name " + oldKey + " does not exist.");
+ throw elementMissingException(oldKey);
} else if (element.value == null) {
- throw new IllegalArgumentException(
- "Can't rename entity that has just been deleted [key=" + oldKey + ']'
- );
+ throw elementRemovedException(oldKey);
}
checkNewKey(newKey);
@@ -294,9 +296,9 @@ public final class NamedListNode<N> implements NamedListChange<N, N>, Traversabl
if (element != null) {
if (element.value == null) {
- throw new IllegalArgumentException("You can't create entity that has just been deleted [key=" + key + ']');
+ throw elementRemovedException(key);
} else {
- throw new IllegalArgumentException("Element with name " + key + " already exists.");
+ throw elementExistsException(key);
}
}
}
@@ -355,7 +357,7 @@ public final class NamedListNode<N> implements NamedListChange<N, N>, Traversabl
ElementDescriptor element = map.get(key);
if (element == null) {
- throw new IllegalArgumentException("Element with name '" + key + "' does not exist.");
+ throw elementMissingException(key);
}
return element.internalId;
@@ -413,7 +415,7 @@ public final class NamedListNode<N> implements NamedListChange<N, N>, Traversabl
ElementDescriptor element = map.get(key);
if (element != null && element.value == null) {
- throw new IllegalArgumentException("You can't create entity that has just been deleted [key=" + key + ']');
+ throw elementRemovedException(key);
}
if (element == null) {
@@ -547,4 +549,16 @@ public final class NamedListNode<N> implements NamedListChange<N, N>, Traversabl
return value == null ? null : value.specificNode();
}
+
+ private static IllegalArgumentException elementMissingException(String key) {
+ return new IllegalArgumentException("Named List element with key \"" + key + "\" does not exist");
+ }
+
+ private static IllegalArgumentException elementExistsException(String key) {
+ return new IllegalArgumentException("Named List element with key \"" + key + "\" already exists");
+ }
+
+ private static IllegalArgumentException elementRemovedException(String key) {
+ return new IllegalArgumentException("Named List element with key \"" + key + "\" has been removed");
+ }
}
diff --git a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/tree/NamedListNodeTest.java b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/tree/NamedListNodeTest.java
index ac08af0..af893d1 100644
--- a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/tree/NamedListNodeTest.java
+++ b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/tree/NamedListNodeTest.java
@@ -299,10 +299,6 @@ public class NamedListNodeTest {
}));
assertThrows(NullPointerException.class, () -> b.create(0, null, z -> {
}));
- assertThrows(NullPointerException.class, () -> b.createAfter(null, "Z", z -> {
- }));
- assertThrows(NullPointerException.class, () -> b.createAfter("X", null, z -> {
- }));
assertThrows(NullPointerException.class, () -> b.rename(null, "Z"));
assertThrows(NullPointerException.class, () -> b.rename("X", null));
assertThrows(NullPointerException.class, () -> b.delete(null));
@@ -311,21 +307,14 @@ public class NamedListNodeTest {
assertThrows(NullPointerException.class, () -> b.create("Z", null));
assertThrows(NullPointerException.class, () -> b.createOrUpdate("Z", null));
assertThrows(NullPointerException.class, () -> b.create(0, "Z", null));
- assertThrows(NullPointerException.class, () -> b.createAfter("X", "Z", null));
// Already existing keys.
assertThrows(IllegalArgumentException.class, () -> b.create("X", x -> {
}));
assertThrows(IllegalArgumentException.class, () -> b.create(0, "X", x -> {
}));
- assertThrows(IllegalArgumentException.class, () -> b.createAfter("X", "Y", y -> {
- }));
assertThrows(IllegalArgumentException.class, () -> b.rename("X", "Y"));
- // Nonexistent preceding key.
- assertThrows(IllegalArgumentException.class, () -> b.createAfter("A", "Z", z -> {
- }));
-
// Wrong indexes.
assertThrows(IndexOutOfBoundsException.class, () -> b.create(-1, "Z", z -> {
}));
@@ -380,4 +369,28 @@ public class NamedListNodeTest {
// updating a non existent key should throw
assertThrows(IllegalArgumentException.class, () -> list.update("wrong", ch -> {}));
}
+
+ @Test
+ void testCreateAfterErrors() {
+ var list = new NamedListNode<>("name", () -> cgen.instantiateNode(SecondConfigurationSchema.class), null);
+
+ list
+ .create("X", x -> {})
+ .create("Y", y -> {});
+
+ assertThrows(NullPointerException.class, () -> list.createAfter(null, "Z", z -> {}));
+
+ assertThrows(NullPointerException.class, () -> list.createAfter("X", null, z -> {}));
+
+ assertThrows(NullPointerException.class, () -> list.createAfter("X", "Z", null));
+
+ // inserting an existing key should throw
+ assertThrows(IllegalArgumentException.class, () -> list.createAfter("X", "Y", y -> {}));
+
+ // inserting after a missing key should throw
+ assertThrows(IllegalArgumentException.class, () -> list.createAfter("A", "Z", z -> {}));
+
+ // inserting after a removed key should throw
+ assertThrows(IllegalArgumentException.class, () -> list.delete("X").createAfter("X", "foo", foo -> {}));
+ }
}