Committed by
Gerrit Code Review
Allow null values for DocumentTree nodes
Change-Id: I88a12727751c6d82843a7b6a9a2e753da1500c99
Showing
6 changed files
with
48 additions
and
24 deletions
| ... | @@ -49,8 +49,8 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { | ... | @@ -49,8 +49,8 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { |
| 49 | * Returns the value of the tree node at specified path. | 49 | * Returns the value of the tree node at specified path. |
| 50 | * | 50 | * |
| 51 | * @param path path to the node | 51 | * @param path path to the node |
| 52 | - * @return future that will be either be completed with node value or {@code null} if path | 52 | + * @return future that will be either be completed with node's {@link Versioned versioned} value |
| 53 | - * does not point to a valid node | 53 | + * or {@code null} if path does not point to a valid node |
| 54 | */ | 54 | */ |
| 55 | CompletableFuture<Versioned<V>> get(DocumentPath path); | 55 | CompletableFuture<Versioned<V>> get(DocumentPath path); |
| 56 | 56 | ||
| ... | @@ -58,9 +58,10 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { | ... | @@ -58,9 +58,10 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { |
| 58 | * Creates or updates a document tree node. | 58 | * Creates or updates a document tree node. |
| 59 | * | 59 | * |
| 60 | * @param path path to the node | 60 | * @param path path to the node |
| 61 | - * @param value the non-null value to be associated with the node | 61 | + * @param value value to be associated with the node ({@code null} is a valid value) |
| 62 | - * @return future that will either be completed with the previous node value or {@code null} if there was no | 62 | + * @return future that will either be completed with the previous {@link Versioned versioned} |
| 63 | - * node previously at that path. Future will be completed with a {@code IllegalDocumentModificationException} | 63 | + * value or {@code null} if there was no node previously at that path. |
| 64 | + * Future will be completed with a {@code IllegalDocumentModificationException} | ||
| 64 | * if the parent node (for the node to create/update) does not exist | 65 | * if the parent node (for the node to create/update) does not exist |
| 65 | */ | 66 | */ |
| 66 | CompletableFuture<Versioned<V>> set(DocumentPath path, V value); | 67 | CompletableFuture<Versioned<V>> set(DocumentPath path, V value); |
| ... | @@ -81,7 +82,7 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { | ... | @@ -81,7 +82,7 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { |
| 81 | * Creates a document tree node recursively by creating all missing intermediate nodes in the path. | 82 | * Creates a document tree node recursively by creating all missing intermediate nodes in the path. |
| 82 | * | 83 | * |
| 83 | * @param path path to the node | 84 | * @param path path to the node |
| 84 | - * @param value the non-null value to be associated with the key | 85 | + * @param value value to be associated with the node ({@code null} is a valid value) |
| 85 | * @return future that is completed with {@code true} if the new node was successfully | 86 | * @return future that is completed with {@code true} if the new node was successfully |
| 86 | * created. Future will be completed with {@code false} if a node already exists at the specified path | 87 | * created. Future will be completed with {@code false} if a node already exists at the specified path |
| 87 | */ | 88 | */ |
| ... | @@ -91,10 +92,10 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { | ... | @@ -91,10 +92,10 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { |
| 91 | * Conditionally updates a tree node if the current version matches a specified version. | 92 | * Conditionally updates a tree node if the current version matches a specified version. |
| 92 | * | 93 | * |
| 93 | * @param path path to the node | 94 | * @param path path to the node |
| 94 | - * @param newValue the non-null value to be associated with the node | 95 | + * @param newValue value to associate with the node ({@code null} is a valid value) |
| 95 | * @param version current version of the node for update to occur | 96 | * @param version current version of the node for update to occur |
| 96 | - * @return future that is either completed with {@code true} if the update was made and the tree was | 97 | + * @return future that is either completed with {@code true} if the update was made |
| 97 | - * modified or {@code false} if update did not happen | 98 | + * or {@code false} if update did not happen |
| 98 | */ | 99 | */ |
| 99 | CompletableFuture<Boolean> replace(DocumentPath path, V newValue, long version); | 100 | CompletableFuture<Boolean> replace(DocumentPath path, V newValue, long version); |
| 100 | 101 | ||
| ... | @@ -102,10 +103,10 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { | ... | @@ -102,10 +103,10 @@ public interface AsyncDocumentTree<V> extends DistributedPrimitive { |
| 102 | * Conditionally updates a tree node if the current node value matches a specified version. | 103 | * Conditionally updates a tree node if the current node value matches a specified version. |
| 103 | * | 104 | * |
| 104 | * @param path path to the node | 105 | * @param path path to the node |
| 105 | - * @param newValue the non-null value to be associated with the node | 106 | + * @param newValue value to associate with the node ({@code null} is a valid value) |
| 106 | * @param currentValue current value of the node for update to occur | 107 | * @param currentValue current value of the node for update to occur |
| 107 | - * @return future that is either completed with {@code true} if the update was made and the tree was | 108 | + * @return future that is either completed with {@code true} if the update was made |
| 108 | - * modified or {@code false} if update did not happen | 109 | + * or {@code false} if update did not happen |
| 109 | */ | 110 | */ |
| 110 | CompletableFuture<Boolean> replace(DocumentPath path, V newValue, V currentValue); | 111 | CompletableFuture<Boolean> replace(DocumentPath path, V newValue, V currentValue); |
| 111 | 112 | ... | ... |
| ... | @@ -21,6 +21,7 @@ import io.atomix.manager.util.ResourceManagerTypeResolver; | ... | @@ -21,6 +21,7 @@ import io.atomix.manager.util.ResourceManagerTypeResolver; |
| 21 | import io.atomix.variables.internal.LongCommands; | 21 | import io.atomix.variables.internal.LongCommands; |
| 22 | 22 | ||
| 23 | import java.util.Arrays; | 23 | import java.util.Arrays; |
| 24 | +import java.util.Optional; | ||
| 24 | 25 | ||
| 25 | import org.onlab.util.Match; | 26 | import org.onlab.util.Match; |
| 26 | import org.onosproject.cluster.Leader; | 27 | import org.onosproject.cluster.Leader; |
| ... | @@ -110,6 +111,7 @@ public final class CatalystSerializers { | ... | @@ -110,6 +111,7 @@ public final class CatalystSerializers { |
| 110 | serializer.register(ImmutableList.of().getClass(), factory); | 111 | serializer.register(ImmutableList.of().getClass(), factory); |
| 111 | serializer.register(ImmutableList.of("a").getClass(), factory); | 112 | serializer.register(ImmutableList.of("a").getClass(), factory); |
| 112 | serializer.register(Arrays.asList().getClass(), factory); | 113 | serializer.register(Arrays.asList().getClass(), factory); |
| 114 | + serializer.register(Optional.class, factory); | ||
| 113 | 115 | ||
| 114 | serializer.resolve(new LongCommands.TypeResolver()); | 116 | serializer.resolve(new LongCommands.TypeResolver()); |
| 115 | serializer.resolve(new AtomixConsistentMapCommands.TypeResolver()); | 117 | serializer.resolve(new AtomixConsistentMapCommands.TypeResolver()); | ... | ... |
| ... | @@ -27,6 +27,7 @@ import io.atomix.resource.ResourceTypeInfo; | ... | @@ -27,6 +27,7 @@ import io.atomix.resource.ResourceTypeInfo; |
| 27 | import java.util.HashMap; | 27 | import java.util.HashMap; |
| 28 | import java.util.List; | 28 | import java.util.List; |
| 29 | import java.util.Map; | 29 | import java.util.Map; |
| 30 | +import java.util.Optional; | ||
| 30 | import java.util.Properties; | 31 | import java.util.Properties; |
| 31 | import java.util.concurrent.CompletableFuture; | 32 | import java.util.concurrent.CompletableFuture; |
| 32 | import java.util.concurrent.Executor; | 33 | import java.util.concurrent.Executor; |
| ... | @@ -108,7 +109,7 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> | ... | @@ -108,7 +109,7 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> |
| 108 | 109 | ||
| 109 | @Override | 110 | @Override |
| 110 | public CompletableFuture<Versioned<byte[]>> set(DocumentPath path, byte[] value) { | 111 | public CompletableFuture<Versioned<byte[]>> set(DocumentPath path, byte[] value) { |
| 111 | - return client.submit(new Update(checkNotNull(path), checkNotNull(value), Match.any(), Match.any())) | 112 | + return client.submit(new Update(checkNotNull(path), Optional.ofNullable(value), Match.any(), Match.any())) |
| 112 | .thenCompose(result -> { | 113 | .thenCompose(result -> { |
| 113 | if (result.status() == INVALID_PATH) { | 114 | if (result.status() == INVALID_PATH) { |
| 114 | return Tools.exceptionalFuture(new NoSuchDocumentPathException()); | 115 | return Tools.exceptionalFuture(new NoSuchDocumentPathException()); |
| ... | @@ -136,7 +137,7 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> | ... | @@ -136,7 +137,7 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> |
| 136 | return createInternal(path, value) | 137 | return createInternal(path, value) |
| 137 | .thenCompose(status -> { | 138 | .thenCompose(status -> { |
| 138 | if (status == ILLEGAL_MODIFICATION) { | 139 | if (status == ILLEGAL_MODIFICATION) { |
| 139 | - return createRecursive(path.parent(), new byte[0]) | 140 | + return createRecursive(path.parent(), null) |
| 140 | .thenCompose(r -> createInternal(path, value).thenApply(v -> true)); | 141 | .thenCompose(r -> createInternal(path, value).thenApply(v -> true)); |
| 141 | } | 142 | } |
| 142 | return CompletableFuture.completedFuture(status == OK); | 143 | return CompletableFuture.completedFuture(status == OK); |
| ... | @@ -145,13 +146,19 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> | ... | @@ -145,13 +146,19 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> |
| 145 | 146 | ||
| 146 | @Override | 147 | @Override |
| 147 | public CompletableFuture<Boolean> replace(DocumentPath path, byte[] newValue, long version) { | 148 | public CompletableFuture<Boolean> replace(DocumentPath path, byte[] newValue, long version) { |
| 148 | - return client.submit(new Update(checkNotNull(path), newValue, Match.any(), Match.ifValue(version))) | 149 | + return client.submit(new Update(checkNotNull(path), |
| 150 | + Optional.ofNullable(newValue), | ||
| 151 | + Match.any(), | ||
| 152 | + Match.ifValue(version))) | ||
| 149 | .thenApply(result -> result.updated()); | 153 | .thenApply(result -> result.updated()); |
| 150 | } | 154 | } |
| 151 | 155 | ||
| 152 | @Override | 156 | @Override |
| 153 | public CompletableFuture<Boolean> replace(DocumentPath path, byte[] newValue, byte[] currentValue) { | 157 | public CompletableFuture<Boolean> replace(DocumentPath path, byte[] newValue, byte[] currentValue) { |
| 154 | - return client.submit(new Update(checkNotNull(path), newValue, Match.ifValue(currentValue), Match.any())) | 158 | + return client.submit(new Update(checkNotNull(path), |
| 159 | + Optional.ofNullable(newValue), | ||
| 160 | + Match.ifValue(currentValue), | ||
| 161 | + Match.any())) | ||
| 155 | .thenCompose(result -> { | 162 | .thenCompose(result -> { |
| 156 | if (result.status() == INVALID_PATH) { | 163 | if (result.status() == INVALID_PATH) { |
| 157 | return Tools.exceptionalFuture(new NoSuchDocumentPathException()); | 164 | return Tools.exceptionalFuture(new NoSuchDocumentPathException()); |
| ... | @@ -168,7 +175,7 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> | ... | @@ -168,7 +175,7 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> |
| 168 | if (path.equals(DocumentPath.from("root"))) { | 175 | if (path.equals(DocumentPath.from("root"))) { |
| 169 | return Tools.exceptionalFuture(new IllegalDocumentModificationException()); | 176 | return Tools.exceptionalFuture(new IllegalDocumentModificationException()); |
| 170 | } | 177 | } |
| 171 | - return client.submit(new Update(checkNotNull(path), null, Match.ifNotNull(), Match.any())) | 178 | + return client.submit(new Update(checkNotNull(path), null, Match.any(), Match.ifNotNull())) |
| 172 | .thenCompose(result -> { | 179 | .thenCompose(result -> { |
| 173 | if (result.status() == INVALID_PATH) { | 180 | if (result.status() == INVALID_PATH) { |
| 174 | return Tools.exceptionalFuture(new NoSuchDocumentPathException()); | 181 | return Tools.exceptionalFuture(new NoSuchDocumentPathException()); |
| ... | @@ -204,7 +211,7 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> | ... | @@ -204,7 +211,7 @@ public class AtomixDocumentTree extends AbstractResource<AtomixDocumentTree> |
| 204 | } | 211 | } |
| 205 | 212 | ||
| 206 | private CompletableFuture<DocumentTreeUpdateResult.Status> createInternal(DocumentPath path, byte[] value) { | 213 | private CompletableFuture<DocumentTreeUpdateResult.Status> createInternal(DocumentPath path, byte[] value) { |
| 207 | - return client.submit(new Update(checkNotNull(path), checkNotNull(value), Match.ifNull(), Match.any())) | 214 | + return client.submit(new Update(checkNotNull(path), Optional.ofNullable(value), Match.any(), Match.ifNull())) |
| 208 | .thenApply(result -> result.status()); | 215 | .thenApply(result -> result.status()); |
| 209 | } | 216 | } |
| 210 | 217 | ... | ... |
| ... | @@ -26,6 +26,7 @@ import io.atomix.copycat.Command; | ... | @@ -26,6 +26,7 @@ import io.atomix.copycat.Command; |
| 26 | import io.atomix.copycat.Query; | 26 | import io.atomix.copycat.Query; |
| 27 | 27 | ||
| 28 | import java.util.Map; | 28 | import java.util.Map; |
| 29 | +import java.util.Optional; | ||
| 29 | 30 | ||
| 30 | import org.onlab.util.Match; | 31 | import org.onlab.util.Match; |
| 31 | import org.onosproject.store.service.DocumentPath; | 32 | import org.onosproject.store.service.DocumentPath; |
| ... | @@ -139,7 +140,7 @@ public class AtomixDocumentTreeCommands { | ... | @@ -139,7 +140,7 @@ public class AtomixDocumentTreeCommands { |
| 139 | @SuppressWarnings("serial") | 140 | @SuppressWarnings("serial") |
| 140 | public static class Update extends DocumentTreeCommand<DocumentTreeUpdateResult<byte[]>> { | 141 | public static class Update extends DocumentTreeCommand<DocumentTreeUpdateResult<byte[]>> { |
| 141 | 142 | ||
| 142 | - private byte[] value; | 143 | + private Optional<byte[]> value; |
| 143 | private Match<byte[]> valueMatch; | 144 | private Match<byte[]> valueMatch; |
| 144 | private Match<Long> versionMatch; | 145 | private Match<Long> versionMatch; |
| 145 | 146 | ||
| ... | @@ -150,14 +151,14 @@ public class AtomixDocumentTreeCommands { | ... | @@ -150,14 +151,14 @@ public class AtomixDocumentTreeCommands { |
| 150 | this.versionMatch = null; | 151 | this.versionMatch = null; |
| 151 | } | 152 | } |
| 152 | 153 | ||
| 153 | - public Update(DocumentPath path, byte[] value, Match<byte[]> valueMatch, Match<Long> versionMatch) { | 154 | + public Update(DocumentPath path, Optional<byte[]> value, Match<byte[]> valueMatch, Match<Long> versionMatch) { |
| 154 | super(path); | 155 | super(path); |
| 155 | this.value = value; | 156 | this.value = value; |
| 156 | this.valueMatch = valueMatch; | 157 | this.valueMatch = valueMatch; |
| 157 | this.versionMatch = versionMatch; | 158 | this.versionMatch = versionMatch; |
| 158 | } | 159 | } |
| 159 | 160 | ||
| 160 | - public byte[] value() { | 161 | + public Optional<byte[]> value() { |
| 161 | return value; | 162 | return value; |
| 162 | } | 163 | } |
| 163 | 164 | ... | ... |
| ... | @@ -244,7 +244,7 @@ public class AtomixDocumentTreeState | ... | @@ -244,7 +244,7 @@ public class AtomixDocumentTreeState |
| 244 | 244 | ||
| 245 | @Override | 245 | @Override |
| 246 | public byte[] value() { | 246 | public byte[] value() { |
| 247 | - return commit.operation().value(); | 247 | + return commit.operation().value().orElse(null); |
| 248 | } | 248 | } |
| 249 | 249 | ||
| 250 | @Override | 250 | @Override | ... | ... |
| ... | @@ -89,6 +89,10 @@ public class AtomixDocumentTreeTest extends AtomixTestBase { | ... | @@ -89,6 +89,10 @@ public class AtomixDocumentTreeTest extends AtomixTestBase { |
| 89 | 89 | ||
| 90 | Versioned<byte[]> ac = tree.get(DocumentPath.from("root.a.c")).join(); | 90 | Versioned<byte[]> ac = tree.get(DocumentPath.from("root.a.c")).join(); |
| 91 | assertArrayEquals("ac".getBytes(), ac.value()); | 91 | assertArrayEquals("ac".getBytes(), ac.value()); |
| 92 | + | ||
| 93 | + tree.create(DocumentPath.from("root.x"), null).join(); | ||
| 94 | + Versioned<byte[]> x = tree.get(DocumentPath.from("root.x")).join(); | ||
| 95 | + assertNull(x.value()); | ||
| 92 | } | 96 | } |
| 93 | 97 | ||
| 94 | /** | 98 | /** |
| ... | @@ -100,10 +104,10 @@ public class AtomixDocumentTreeTest extends AtomixTestBase { | ... | @@ -100,10 +104,10 @@ public class AtomixDocumentTreeTest extends AtomixTestBase { |
| 100 | AtomixDocumentTree.class).join(); | 104 | AtomixDocumentTree.class).join(); |
| 101 | tree.createRecursive(DocumentPath.from("root.a.b.c"), "abc".getBytes()).join(); | 105 | tree.createRecursive(DocumentPath.from("root.a.b.c"), "abc".getBytes()).join(); |
| 102 | Versioned<byte[]> a = tree.get(DocumentPath.from("root.a")).join(); | 106 | Versioned<byte[]> a = tree.get(DocumentPath.from("root.a")).join(); |
| 103 | - assertArrayEquals(new byte[0], a.value()); | 107 | + assertArrayEquals(null, a.value()); |
| 104 | 108 | ||
| 105 | Versioned<byte[]> ab = tree.get(DocumentPath.from("root.a.b")).join(); | 109 | Versioned<byte[]> ab = tree.get(DocumentPath.from("root.a.b")).join(); |
| 106 | - assertArrayEquals(new byte[0], ab.value()); | 110 | + assertArrayEquals(null, ab.value()); |
| 107 | 111 | ||
| 108 | Versioned<byte[]> abc = tree.get(DocumentPath.from("root.a.b.c")).join(); | 112 | Versioned<byte[]> abc = tree.get(DocumentPath.from("root.a.b.c")).join(); |
| 109 | assertArrayEquals("abc".getBytes(), abc.value()); | 113 | assertArrayEquals("abc".getBytes(), abc.value()); |
| ... | @@ -131,6 +135,10 @@ public class AtomixDocumentTreeTest extends AtomixTestBase { | ... | @@ -131,6 +135,10 @@ public class AtomixDocumentTreeTest extends AtomixTestBase { |
| 131 | tree.set(DocumentPath.from("root.a.b"), "newAB".getBytes()).join(); | 135 | tree.set(DocumentPath.from("root.a.b"), "newAB".getBytes()).join(); |
| 132 | Versioned<byte[]> newAB = tree.get(DocumentPath.from("root.a.b")).join(); | 136 | Versioned<byte[]> newAB = tree.get(DocumentPath.from("root.a.b")).join(); |
| 133 | assertArrayEquals("newAB".getBytes(), newAB.value()); | 137 | assertArrayEquals("newAB".getBytes(), newAB.value()); |
| 138 | + | ||
| 139 | + tree.set(DocumentPath.from("root.x"), null).join(); | ||
| 140 | + Versioned<byte[]> x = tree.get(DocumentPath.from("root.x")).join(); | ||
| 141 | + assertNull(x.value()); | ||
| 134 | } | 142 | } |
| 135 | 143 | ||
| 136 | /** | 144 | /** |
| ... | @@ -199,6 +207,11 @@ public class AtomixDocumentTreeTest extends AtomixTestBase { | ... | @@ -199,6 +207,11 @@ public class AtomixDocumentTreeTest extends AtomixTestBase { |
| 199 | Versioned<byte[]> a = tree.removeNode(DocumentPath.from("root.a")).join(); | 207 | Versioned<byte[]> a = tree.removeNode(DocumentPath.from("root.a")).join(); |
| 200 | assertArrayEquals("a".getBytes(), a.value()); | 208 | assertArrayEquals("a".getBytes(), a.value()); |
| 201 | assertNull(tree.get(DocumentPath.from("root.a")).join()); | 209 | assertNull(tree.get(DocumentPath.from("root.a")).join()); |
| 210 | + | ||
| 211 | + tree.create(DocumentPath.from("root.x"), null).join(); | ||
| 212 | + Versioned<byte[]> x = tree.removeNode(DocumentPath.from("root.x")).join(); | ||
| 213 | + assertNull(x.value()); | ||
| 214 | + assertNull(tree.get(DocumentPath.from("root.a.x")).join()); | ||
| 202 | } | 215 | } |
| 203 | 216 | ||
| 204 | /** | 217 | /** | ... | ... |
-
Please register or login to post a comment