From 3735b50ba8ecdd60325bb9b1d65ff13ddc4836e4 Mon Sep 17 00:00:00 2001 From: randgalt Date: Mon, 3 Oct 2016 18:34:53 +0200 Subject: [PATCH] Better exceptions messaging when there is a Schema violation. This is much more useful for debugging purposes --- .../imps/CuratorMultiTransactionImpl.java | 4 +- .../framework/imps/DeleteBuilderImpl.java | 2 +- .../framework/imps/ExistsBuilderImpl.java | 2 +- .../imps/GetChildrenBuilderImpl.java | 2 +- .../framework/imps/GetDataBuilderImpl.java | 2 +- .../curator/framework/schema/Schema.java | 22 ++-- .../curator/framework/schema/SchemaSet.java | 2 +- .../framework/schema/SchemaViolation.java | 101 ++++++++++++++++++ 8 files changed, 121 insertions(+), 16 deletions(-) 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 40eb4a2b7c..d0a8e7f30d 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.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 List forOperations(List operations) } 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 ) { 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 cbd12dd4f5..78eafd82b0 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 @@ public void performBackgroundOperation(OperationAndData dummy) throws Ex @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); 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 960b5778d7..97e81470e4 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 @@ public Stat forPath(String path) throws Exception { 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() ) 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 000c911f4f..d26f270cdd 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 @@ public void processResult(int rc, String path, Object o, List strings, S @Override public List 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); 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 bae126c7ff..8d92f8d3f7 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 @@ public void processResult(int rc, String path, Object ctx, byte[] data, Stat sta @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); 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 ce128e92e7..e9f4f181cf 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 @@ private String fixPath(String path) /** * 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 void validateCreate(CreateMode mode, String path, byte[] data, List { 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 void validateGeneral(String path, byte[] data, List acl) { 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"); } } 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 95a6ef9466..98eeefcab5 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 Schema getSchema(String path) { 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); } /** 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 47661d17af..3f8f1d974c 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 validation 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; + + public ViolatorData(String path, byte[] data, List 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 null + * + * @return path or null + */ + public String getPath() + { + return path; + } + + /** + * The data used in the API or null + * + * @return data or null + */ + public byte[] getData() + { + return data; + } + /** + * The ACLs used in the API or null + * + * @return ACLs or null + */ + public List 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 String getViolation() { 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; + } }