You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ra...@apache.org on 2016/10/28 15:06:17 UTC
[2/6] curator git commit: Better exceptions messaging when there is a
Schema violation. This is much more useful for debugging purposes
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 <ra...@apache.org>
Authored: Mon Oct 3 18:34:53 2016 +0200
Committer: randgalt <ra...@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;
+ }
}