curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From randg...@apache.org
Subject curator git commit: Better exceptions messaging when there is a Schema violation. This is much more useful for debugging purposes
Date Mon, 10 Oct 2016 09:52:58 GMT
Repository: curator
Updated Branches:
  refs/heads/CURATOR-3.0 199849161 -> 3735b50ba


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-3.0
Commit: 3735b50ba8ecdd60325bb9b1d65ff13ddc4836e4
Parents: 1998491
Author: randgalt <randgalt@apache.org>
Authored: Mon Oct 3 18:34:53 2016 +0200
Committer: randgalt <randgalt@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;
+    }
 }


Mime
View raw message