Ensure DocumentTree exceptions are properly returned to clients on getChildren calls
Change-Id: Ia374be077f89fd4ed6ad81aafa4d1b9d506bd0bd
diff --git a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTree.java b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTree.java
index e10fe43..99f8df6 100644
--- a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTree.java
+++ b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTree.java
@@ -51,9 +51,9 @@
import static org.onosproject.store.primitives.resources.impl.AtomixDocumentTreeOperations.GET_CHILDREN;
import static org.onosproject.store.primitives.resources.impl.AtomixDocumentTreeOperations.REMOVE_LISTENER;
import static org.onosproject.store.primitives.resources.impl.AtomixDocumentTreeOperations.UPDATE;
-import static org.onosproject.store.primitives.resources.impl.DocumentTreeUpdateResult.Status.ILLEGAL_MODIFICATION;
-import static org.onosproject.store.primitives.resources.impl.DocumentTreeUpdateResult.Status.INVALID_PATH;
-import static org.onosproject.store.primitives.resources.impl.DocumentTreeUpdateResult.Status.OK;
+import static org.onosproject.store.primitives.resources.impl.DocumentTreeResult.Status.ILLEGAL_MODIFICATION;
+import static org.onosproject.store.primitives.resources.impl.DocumentTreeResult.Status.INVALID_PATH;
+import static org.onosproject.store.primitives.resources.impl.DocumentTreeResult.Status.OK;
/**
* Distributed resource providing the {@link AsyncDocumentTree} primitive.
@@ -94,7 +94,20 @@
@Override
public CompletableFuture<Map<String, Versioned<byte[]>>> getChildren(DocumentPath path) {
- return proxy.invoke(GET_CHILDREN, SERIALIZER::encode, new GetChildren(checkNotNull(path)), SERIALIZER::decode);
+ return proxy.<GetChildren, DocumentTreeResult<Map<String, Versioned<byte[]>>>>invoke(
+ GET_CHILDREN,
+ SERIALIZER::encode,
+ new GetChildren(checkNotNull(path)),
+ SERIALIZER::decode)
+ .thenCompose(result -> {
+ if (result.status() == INVALID_PATH) {
+ return Tools.exceptionalFuture(new NoSuchDocumentPathException());
+ } else if (result.status() == ILLEGAL_MODIFICATION) {
+ return Tools.exceptionalFuture(new IllegalDocumentModificationException());
+ } else {
+ return CompletableFuture.completedFuture(result);
+ }
+ }).thenApply(result -> result.result());
}
@Override
@@ -104,7 +117,7 @@
@Override
public CompletableFuture<Versioned<byte[]>> set(DocumentPath path, byte[] value) {
- return proxy.<Update, DocumentTreeUpdateResult<byte[]>>invoke(UPDATE,
+ return proxy.<Update, DocumentTreeResult<Versioned<byte[]>>>invoke(UPDATE,
SERIALIZER::encode,
new Update(checkNotNull(path), Optional.ofNullable(value), Match.any(), Match.any()),
SERIALIZER::decode)
@@ -116,7 +129,7 @@
} else {
return CompletableFuture.completedFuture(result);
}
- }).thenApply(result -> result.oldValue());
+ }).thenApply(result -> result.result());
}
@Override
@@ -144,7 +157,7 @@
@Override
public CompletableFuture<Boolean> replace(DocumentPath path, byte[] newValue, long version) {
- return proxy.<Update, DocumentTreeUpdateResult<byte[]>>invoke(UPDATE,
+ return proxy.<Update, DocumentTreeResult<byte[]>>invoke(UPDATE,
SERIALIZER::encode,
new Update(checkNotNull(path),
Optional.ofNullable(newValue),
@@ -155,7 +168,7 @@
@Override
public CompletableFuture<Boolean> replace(DocumentPath path, byte[] newValue, byte[] currentValue) {
- return proxy.<Update, DocumentTreeUpdateResult<byte[]>>invoke(UPDATE,
+ return proxy.<Update, DocumentTreeResult<byte[]>>invoke(UPDATE,
SERIALIZER::encode,
new Update(checkNotNull(path),
Optional.ofNullable(newValue),
@@ -178,7 +191,7 @@
if (path.equals(DocumentPath.from("root"))) {
return Tools.exceptionalFuture(new IllegalDocumentModificationException());
}
- return proxy.<Update, DocumentTreeUpdateResult<byte[]>>invoke(UPDATE,
+ return proxy.<Update, DocumentTreeResult<Versioned<byte[]>>>invoke(UPDATE,
SERIALIZER::encode,
new Update(checkNotNull(path), null, Match.any(), Match.ifNotNull()),
SERIALIZER::decode)
@@ -190,7 +203,7 @@
} else {
return CompletableFuture.completedFuture(result);
}
- }).thenApply(result -> result.oldValue());
+ }).thenApply(result -> result.result());
}
@Override
@@ -217,8 +230,8 @@
return CompletableFuture.completedFuture(null);
}
- private CompletableFuture<DocumentTreeUpdateResult.Status> createInternal(DocumentPath path, byte[] value) {
- return proxy.<Update, DocumentTreeUpdateResult<byte[]>>invoke(UPDATE,
+ private CompletableFuture<DocumentTreeResult.Status> createInternal(DocumentPath path, byte[] value) {
+ return proxy.<Update, DocumentTreeResult<byte[]>>invoke(UPDATE,
SERIALIZER::encode,
new Update(checkNotNull(path), Optional.ofNullable(value), Match.any(), Match.ifNull()),
SERIALIZER::decode)
diff --git a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTreeOperations.java b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTreeOperations.java
index 1b61094..726e1f1 100644
--- a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTreeOperations.java
+++ b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTreeOperations.java
@@ -67,8 +67,8 @@
.register(DocumentPath.class)
.register(Match.class)
.register(Versioned.class)
- .register(DocumentTreeUpdateResult.class)
- .register(DocumentTreeUpdateResult.Status.class)
+ .register(DocumentTreeResult.class)
+ .register(DocumentTreeResult.Status.class)
.build("AtomixDocumentTreeOperations");
/**
diff --git a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTreeService.java b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTreeService.java
index 2995908..9ee352c 100644
--- a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTreeService.java
+++ b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/AtomixDocumentTreeService.java
@@ -47,7 +47,7 @@
import org.onosproject.store.primitives.resources.impl.AtomixDocumentTreeOperations.Listen;
import org.onosproject.store.primitives.resources.impl.AtomixDocumentTreeOperations.Unlisten;
import org.onosproject.store.primitives.resources.impl.AtomixDocumentTreeOperations.Update;
-import org.onosproject.store.primitives.resources.impl.DocumentTreeUpdateResult.Status;
+import org.onosproject.store.primitives.resources.impl.DocumentTreeResult.Status;
import org.onosproject.store.serializers.KryoNamespaces;
import org.onosproject.store.service.DocumentPath;
import org.onosproject.store.service.DocumentTree;
@@ -156,18 +156,22 @@
protected Versioned<byte[]> get(Commit<? extends Get> commit) {
try {
Versioned<byte[]> value = docTree.get(commit.value().path());
- return value == null ? null : value.map(node -> node == null ? null : node);
+ return value == null ? null : value.map(node -> node);
} catch (IllegalStateException e) {
return null;
}
}
- protected Map<String, Versioned<byte[]>> getChildren(Commit<? extends GetChildren> commit) {
- return docTree.getChildren(commit.value().path());
+ protected DocumentTreeResult<Map<String, Versioned<byte[]>>> getChildren(Commit<? extends GetChildren> commit) {
+ try {
+ return DocumentTreeResult.ok(docTree.getChildren(commit.value().path()));
+ } catch (NoSuchDocumentPathException e) {
+ return DocumentTreeResult.invalidPath();
+ }
}
- protected DocumentTreeUpdateResult<byte[]> update(Commit<? extends Update> commit) {
- DocumentTreeUpdateResult<byte[]> result = null;
+ protected DocumentTreeResult<Versioned<byte[]>> update(Commit<? extends Update> commit) {
+ DocumentTreeResult<Versioned<byte[]>> result = null;
DocumentPath path = commit.value().path();
boolean updated = false;
Versioned<byte[]> currentValue = docTree.get(path);
@@ -178,25 +182,46 @@
if (versionMatch.matches(currentValue == null ? null : currentValue.version())
&& valueMatch.matches(currentValue == null ? null : currentValue.value())) {
if (commit.value().value() == null) {
- docTree.removeNode(path);
+ Versioned<byte[]> oldValue = docTree.removeNode(path);
+ result = new DocumentTreeResult<>(Status.OK, oldValue);
+ if (oldValue != null) {
+ notifyListeners(new DocumentTreeEvent<>(
+ path,
+ Type.DELETED,
+ Optional.empty(),
+ Optional.of(oldValue)));
+ }
} else {
- docTree.set(path, commit.value().value().orElse(null));
+ Versioned<byte[]> oldValue = docTree.set(path, commit.value().value().orElse(null));
+ Versioned<byte[]> newValue = docTree.get(path);
+ result = new DocumentTreeResult<>(Status.OK, newValue);
+ if (oldValue == null) {
+ notifyListeners(new DocumentTreeEvent<>(
+ path,
+ Type.CREATED,
+ Optional.of(newValue),
+ Optional.empty()));
+ } else {
+ notifyListeners(new DocumentTreeEvent<>(
+ path,
+ Type.UPDATED,
+ Optional.of(newValue),
+ Optional.of(oldValue)));
+ }
}
- updated = true;
+ } else {
+ result = new DocumentTreeResult<>(
+ commit.value().value() == null ? Status.INVALID_PATH : Status.NOOP,
+ currentValue);
}
- Versioned<byte[]> newValue = updated ? docTree.get(path) : currentValue;
- Status updateStatus = updated
- ? Status.OK : commit.value().value() == null ? Status.INVALID_PATH : Status.NOOP;
- result = new DocumentTreeUpdateResult<>(path, updateStatus, newValue, currentValue);
} catch (IllegalDocumentModificationException e) {
- result = DocumentTreeUpdateResult.illegalModification(path);
+ result = DocumentTreeResult.illegalModification();
} catch (NoSuchDocumentPathException e) {
- result = DocumentTreeUpdateResult.invalidPath(path);
+ result = DocumentTreeResult.invalidPath();
} catch (Exception e) {
getLogger().error("Failed to apply {} to state machine", commit.value(), e);
throw Throwables.propagate(e);
}
- notifyListeners(path, result);
return result;
}
@@ -219,16 +244,7 @@
}
}
- private void notifyListeners(DocumentPath path, DocumentTreeUpdateResult<byte[]> result) {
- if (result.status() != Status.OK) {
- return;
- }
- DocumentTreeEvent<byte[]> event =
- new DocumentTreeEvent<>(path,
- result.created() ? Type.CREATED : result.newValue() == null ? Type.DELETED : Type.UPDATED,
- Optional.ofNullable(result.newValue()),
- Optional.ofNullable(result.oldValue()));
-
+ private void notifyListeners(DocumentTreeEvent<byte[]> event) {
listeners.values()
.stream()
.filter(l -> event.path().isDescendentOf(l.leastCommonAncestorPath()))
diff --git a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/DocumentTreeResult.java b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/DocumentTreeResult.java
new file mode 100644
index 0000000..3e30a00
--- /dev/null
+++ b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/DocumentTreeResult.java
@@ -0,0 +1,127 @@
+/*
+ * Copyright 2016-present Open Networking Foundation
+ *
+ * Licensed 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
+ *
+ * http://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 org.onosproject.store.primitives.resources.impl;
+
+import com.google.common.base.MoreObjects;
+
+/**
+ * Result of a document tree operation.
+ *
+ * @param <V> value type
+ */
+public class DocumentTreeResult<V> {
+
+ public enum Status {
+ /**
+ * Indicates a successful update.
+ */
+ OK,
+
+ /**
+ * Indicates a noop i.e. existing and new value are both same.
+ */
+ NOOP,
+
+ /**
+ * Indicates a failed update due to a write lock.
+ */
+ WRITE_LOCK,
+
+ /**
+ * Indicates a failed update due to a invalid path.
+ */
+ INVALID_PATH,
+
+ /**
+ * Indicates a failed update due to a illegal modification attempt.
+ */
+ ILLEGAL_MODIFICATION,
+ }
+
+ @SuppressWarnings("unchecked")
+ public static final DocumentTreeResult INVALID_PATH =
+ new DocumentTreeResult(Status.INVALID_PATH, null);
+
+ @SuppressWarnings("unchecked")
+ public static final DocumentTreeResult ILLEGAL_MODIFICATION =
+ new DocumentTreeResult(Status.ILLEGAL_MODIFICATION, null);
+
+ /**
+ * Returns a successful result.
+ *
+ * @param result the operation result
+ * @param <V> the result value type
+ * @return successful result
+ */
+ public static <V> DocumentTreeResult<V> ok(V result) {
+ return new DocumentTreeResult<V>(Status.OK, result);
+ }
+
+ /**
+ * Returns an {@code INVALID_PATH} result.
+ *
+ * @param <V> the result value type
+ * @return invalid path result
+ */
+ @SuppressWarnings("unchecked")
+ public static <V> DocumentTreeResult<V> invalidPath() {
+ return INVALID_PATH;
+ }
+
+ /**
+ * Returns an {@code ILLEGAL_MODIFICATION} result.
+ *
+ * @param <V> the result value type
+ * @return illegal modification result
+ */
+ @SuppressWarnings("unchecked")
+ public static <V> DocumentTreeResult<V> illegalModification() {
+ return ILLEGAL_MODIFICATION;
+ }
+
+ private final Status status;
+ private final V result;
+
+ public DocumentTreeResult(Status status, V result) {
+ this.status = status;
+ this.result = result;
+ }
+
+ public Status status() {
+ return status;
+ }
+
+ public V result() {
+ return result;
+ }
+
+ public boolean updated() {
+ return status == Status.OK;
+ }
+
+ public boolean created() {
+ return updated() && result == null;
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(getClass())
+ .add("status", status)
+ .add("value", result)
+ .toString();
+ }
+}
diff --git a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/DocumentTreeUpdateResult.java b/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/DocumentTreeUpdateResult.java
deleted file mode 100644
index 88f7371..0000000
--- a/core/store/primitives/src/main/java/org/onosproject/store/primitives/resources/impl/DocumentTreeUpdateResult.java
+++ /dev/null
@@ -1,116 +0,0 @@
-/*
- * Copyright 2016-present Open Networking Foundation
- *
- * Licensed 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
- *
- * http://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 org.onosproject.store.primitives.resources.impl;
-
-import org.onosproject.store.service.DocumentPath;
-import org.onosproject.store.service.Versioned;
-
-import com.google.common.base.MoreObjects;
-
-/**
- * Result of a document tree node update operation.
- * <p>
- * Both old and new values are accessible along with a status of update.
- *
- * @param <V> value type
- */
-public class DocumentTreeUpdateResult<V> {
-
- public enum Status {
- /**
- * Indicates a successful update.
- */
- OK,
-
- /**
- * Indicates a noop i.e. existing and new value are both same.
- */
- NOOP,
-
- /**
- * Indicates a failed update due to a write lock.
- */
- WRITE_LOCK,
-
- /**
- * Indicates a failed update due to a invalid path.
- */
- INVALID_PATH,
-
- /**
- * Indicates a failed update due to a illegal modification attempt.
- */
- ILLEGAL_MODIFICATION,
- }
-
- private final DocumentPath path;
- private final Status status;
- private final Versioned<V> oldValue;
- private final Versioned<V> newValue;
-
- public DocumentTreeUpdateResult(DocumentPath path,
- Status status,
- Versioned<V> newValue,
- Versioned<V> oldValue) {
- this.status = status;
- this.path = path;
- this.newValue = newValue;
- this.oldValue = oldValue;
- }
-
- public static <V> DocumentTreeUpdateResult<V> invalidPath(DocumentPath path) {
- return new DocumentTreeUpdateResult<>(path, Status.INVALID_PATH, null, null);
- }
-
- public static <V> DocumentTreeUpdateResult<V> illegalModification(DocumentPath path) {
- return new DocumentTreeUpdateResult<>(path, Status.ILLEGAL_MODIFICATION, null, null);
- }
-
- public Status status() {
- return status;
- }
-
- public DocumentPath path() {
- return path;
- }
-
- public Versioned<V> oldValue() {
- return oldValue;
- }
-
- public Versioned<V> newValue() {
- return this.newValue;
- }
-
- public boolean updated() {
- return status == Status.OK;
- }
-
- public boolean created() {
- return updated() && oldValue == null;
- }
-
- @Override
- public String toString() {
- return MoreObjects.toStringHelper(getClass())
- .add("path", path)
- .add("status", status)
- .add("newValue", newValue)
- .add("oldValue", oldValue)
- .toString();
- }
-}