Better exceptions messaging when there is a Schema violation. This is much more useful for debugging purposes
Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/3735b50b Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/3735b50b Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/3735b50b Branch: refs/heads/CURATOR-351 Commit: 3735b50ba8ecdd60325bb9b1d65ff13ddc4836e4 Parents: 1998491 Author: randgalt <randg...@apache.org> Authored: Mon Oct 3 18:34:53 2016 +0200 Committer: randgalt <randg...@apache.org> Committed: Mon Oct 3 18:34:53 2016 +0200 ---------------------------------------------------------------------- .../imps/CuratorMultiTransactionImpl.java | 4 +- .../framework/imps/DeleteBuilderImpl.java | 2 +- .../framework/imps/ExistsBuilderImpl.java | 2 +- .../framework/imps/GetChildrenBuilderImpl.java | 2 +- .../framework/imps/GetDataBuilderImpl.java | 2 +- .../apache/curator/framework/schema/Schema.java | 22 ++-- .../curator/framework/schema/SchemaSet.java | 2 +- .../framework/schema/SchemaViolation.java | 101 +++++++++++++++++++ 8 files changed, 121 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/3735b50b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java index 40eb4a2..d0a8e7f 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java @@ -38,6 +38,7 @@ import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.OpResult; import org.apache.zookeeper.ZooDefs; import org.apache.zookeeper.proto.CreateRequest; +import org.apache.zookeeper.proto.DeleteRequest; import org.apache.zookeeper.proto.SetDataRequest; import java.util.Arrays; import java.util.List; @@ -133,7 +134,8 @@ public class CuratorMultiTransactionImpl implements } else if ( (curatorOp.get().getType() == ZooDefs.OpCode.delete) || (curatorOp.get().getType() == ZooDefs.OpCode.deleteContainer) ) { - schema.validateDelete(); + DeleteRequest deleteRequest = (DeleteRequest)curatorOp.get().toRequestRecord(); + schema.validateDelete(deleteRequest.getPath()); } else if ( curatorOp.get().getType() == ZooDefs.OpCode.setData ) { http://git-wip-us.apache.org/repos/asf/curator/blob/3735b50b/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java index cbd12dd..78eafd8 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java @@ -213,7 +213,7 @@ class DeleteBuilderImpl implements DeleteBuilder, BackgroundOperation<String>, E @Override public Void forPath(String path) throws Exception { - client.getSchemaSet().getSchema(path).validateDelete(); + client.getSchemaSet().getSchema(path).validateDelete(path); final String unfixedPath = path; path = client.fixForNamespace(path); http://git-wip-us.apache.org/repos/asf/curator/blob/3735b50b/curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java index 960b577..97e8147 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java @@ -158,7 +158,7 @@ class ExistsBuilderImpl implements ExistsBuilder, BackgroundOperation<String>, E { path = client.fixForNamespace(path); - client.getSchemaSet().getSchema(path).validateWatch(watching.isWatched() || watching.hasWatcher()); + client.getSchemaSet().getSchema(path).validateWatch(path, watching.isWatched() || watching.hasWatcher()); Stat returnStat = null; if ( backgrounding.inBackground() ) http://git-wip-us.apache.org/repos/asf/curator/blob/3735b50b/curator-framework/src/main/java/org/apache/curator/framework/imps/GetChildrenBuilderImpl.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/GetChildrenBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/GetChildrenBuilderImpl.java index 000c911..d26f270 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/GetChildrenBuilderImpl.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/GetChildrenBuilderImpl.java @@ -197,7 +197,7 @@ class GetChildrenBuilderImpl implements GetChildrenBuilder, BackgroundOperation< @Override public List<String> forPath(String path) throws Exception { - client.getSchemaSet().getSchema(path).validateWatch(watching.isWatched() || watching.hasWatcher()); + client.getSchemaSet().getSchema(path).validateWatch(path, watching.isWatched() || watching.hasWatcher()); path = client.fixForNamespace(path); http://git-wip-us.apache.org/repos/asf/curator/blob/3735b50b/curator-framework/src/main/java/org/apache/curator/framework/imps/GetDataBuilderImpl.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/GetDataBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/GetDataBuilderImpl.java index bae126c..8d92f8d 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/GetDataBuilderImpl.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/GetDataBuilderImpl.java @@ -276,7 +276,7 @@ class GetDataBuilderImpl implements GetDataBuilder, BackgroundOperation<String>, @Override public byte[] forPath(String path) throws Exception { - client.getSchemaSet().getSchema(path).validateWatch(watching.isWatched() || watching.hasWatcher()); + client.getSchemaSet().getSchema(path).validateWatch(path, watching.isWatched() || watching.hasWatcher()); path = client.fixForNamespace(path); http://git-wip-us.apache.org/repos/asf/curator/blob/3735b50b/curator-framework/src/main/java/org/apache/curator/framework/schema/Schema.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/schema/Schema.java b/curator-framework/src/main/java/org/apache/curator/framework/schema/Schema.java index ce128e9..e9f4f18 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/schema/Schema.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/schema/Schema.java @@ -134,32 +134,34 @@ public class Schema /** * Validate that this schema allows znode deletion * + * @param path the znode full path * @throws SchemaViolation if schema does not allow znode deletion */ - public void validateDelete() + public void validateDelete(String path) { if ( !canBeDeleted ) { - throw new SchemaViolation(this, "Cannot be deleted"); + throw new SchemaViolation(this, new SchemaViolation.ViolatorData(path, null, null), "Cannot be deleted"); } } /** * Validate that this schema's watching setting matches * + * @param path the znode full path * @param isWatching true if attempt is being made to watch node * @throws SchemaViolation if schema's watching setting does not match */ - public void validateWatch(boolean isWatching) + public void validateWatch(String path, boolean isWatching) { if ( isWatching && (watched == Allowance.CANNOT) ) { - throw new SchemaViolation(this, "Cannot be watched"); + throw new SchemaViolation(this, new SchemaViolation.ViolatorData(path, null, null), "Cannot be watched"); } if ( !isWatching && (watched == Allowance.MUST) ) { - throw new SchemaViolation(this, "Must be watched"); + throw new SchemaViolation(this, new SchemaViolation.ViolatorData(path, null, null), "Must be watched"); } } @@ -176,22 +178,22 @@ public class Schema { if ( mode.isEphemeral() && (ephemeral == Allowance.CANNOT) ) { - throw new SchemaViolation(this, "Cannot be ephemeral"); + throw new SchemaViolation(this, new SchemaViolation.ViolatorData(path, data, acl), "Cannot be ephemeral"); } if ( !mode.isEphemeral() && (ephemeral == Allowance.MUST) ) { - throw new SchemaViolation(this, "Must be ephemeral"); + throw new SchemaViolation(this, new SchemaViolation.ViolatorData(path, data, acl), "Must be ephemeral"); } if ( mode.isSequential() && (sequential == Allowance.CANNOT) ) { - throw new SchemaViolation(this, "Cannot be sequential"); + throw new SchemaViolation(this, new SchemaViolation.ViolatorData(path, data, acl), "Cannot be sequential"); } if ( !mode.isSequential() && (sequential == Allowance.MUST) ) { - throw new SchemaViolation(this, "Must be sequential"); + throw new SchemaViolation(this, new SchemaViolation.ViolatorData(path, data, acl), "Must be sequential"); } validateGeneral(path, data, acl); @@ -210,7 +212,7 @@ public class Schema { if ( !schemaValidator.isValid(this, path, data, acl) ) { - throw new SchemaViolation(this, "Data is not valid"); + throw new SchemaViolation(this, new SchemaViolation.ViolatorData(path, data, acl), "Data is not valid"); } } http://git-wip-us.apache.org/repos/asf/curator/blob/3735b50b/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaSet.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaSet.java b/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaSet.java index 95a6ef9..98eeefc 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaSet.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaSet.java @@ -171,7 +171,7 @@ public class SchemaSet { return defaultSchema; } - throw new SchemaViolation("No schema found for: " + path); + throw new SchemaViolation(null, new SchemaViolation.ViolatorData(path, null, null), "No schema found for: " + path); } /** http://git-wip-us.apache.org/repos/asf/curator/blob/3735b50b/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaViolation.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaViolation.java b/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaViolation.java index 47661d1..3f8f1d9 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaViolation.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/schema/SchemaViolation.java @@ -18,6 +18,12 @@ */ package org.apache.curator.framework.schema; +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; +import org.apache.zookeeper.data.ACL; +import java.util.Arrays; +import java.util.List; + /** * Thrown by the various <code>validation</code> methods in a Schema */ @@ -25,19 +31,98 @@ public class SchemaViolation extends RuntimeException { private final Schema schema; private final String violation; + private final ViolatorData violatorData; + + /** + * Data about the calling API that violated the schema + */ + public static class ViolatorData + { + private final String path; + private final byte[] data; + private final List<ACL> acl; + + public ViolatorData(String path, byte[] data, List<ACL> acl) + { + this.path = path; + this.data = (data != null) ? Arrays.copyOf(data, data.length) : null; + this.acl = (acl != null) ? ImmutableList.copyOf(acl) : null; + } + + /** + * The path used in the API or <code>null</code> + * + * @return path or null + */ + public String getPath() + { + return path; + } + + /** + * The data used in the API or <code>null</code> + * + * @return data or null + */ + public byte[] getData() + { + return data; + } + /** + * The ACLs used in the API or <code>null</code> + * + * @return ACLs or null + */ + public List<ACL> getAcl() + { + return acl; + } + + @Override + public String toString() + { + String dataString = (data != null) ? new String(data) : ""; + return "ViolatorData{" + "path='" + path + '\'' + ", data=" + dataString + ", acl=" + acl + '}'; + } + } + + /** + * @param violation the violation + * @deprecated use {@link #SchemaViolation(Schema, ViolatorData, String)} instance + */ public SchemaViolation(String violation) { super(String.format("Schema violation: %s", violation)); this.schema = null; this.violation = violation; + this.violatorData = new ViolatorData(null, null, null); } + /** + * @param schema the schema + * @param violation the violation + * @deprecated use {@link #SchemaViolation(Schema, ViolatorData, String)} instance + */ public SchemaViolation(Schema schema, String violation) { super(String.format("Schema violation: %s for schema: %s", violation, schema)); this.schema = schema; this.violation = violation; + this.violatorData = new ViolatorData(null, null, null); + } + + /** + * @param schema the schema + * @param violatorData data about the caller who violated the schema + * @param violation the violation + */ + public SchemaViolation(Schema schema, ViolatorData violatorData, String violation) + { + super(toString(schema, violation, violatorData)); + this.schema = schema; + this.violation = violation; + this.violatorData = violatorData; } public Schema getSchema() @@ -49,4 +134,20 @@ public class SchemaViolation extends RuntimeException { return violation; } + + public ViolatorData getViolatorData() + { + return violatorData; + } + + @Override + public String toString() + { + return toString(schema, violation, violatorData) + super.toString(); + } + + private static String toString(Schema schema, String violation, ViolatorData violatorData) + { + return Objects.firstNonNull(violation, "") + " " + schema + " " + violatorData; + } }
