ZOOKEEPER-291. regression for legacy code using KeeperException.Code constants (due to 246). (pat via mahadev)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/zookeeper/trunk@740847 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -21,6 +21,9 @@ BUGFIXES:
 
   ZOOKEEPER-255. zoo_set() api does not return stat datastructure.
   (avery ching via mahadev)
+  
+  ZOOKEEPER-246. review error code definition in both source and docs. 
+  (pat via mahadev)
 
 Backward compatibile changes:
 
@@ -100,6 +103,9 @@ and runping via mahadev)
   ZOOKEEPER-252. PurgeTxnLog is not handling the new dataDir directory
   structure (mahadev via phunt)
 
+  ZOOKEEPER-291. regression for legacy code using KeeperException.Code 
+  constants (due to 246). (pat via mahadev)
+
 IMPROVEMENTS:
    
   ZOOKEEPER-64. Log system env information when initializing server and
@@ -122,9 +128,6 @@ IMPROVEMENTS:
    ZOOKEEPER-222.  print C client log message timestamp in human readable
    form. (pat via mahadev) 
   
-   ZOOKEEPER-246. review error code definition in both source and docs.
-   (pat via mahadev)
-
    ZOOKEEPER-256. support use of JMX to manage log4j configuration at runtime.
    (pat via mahadev)
 

src/java/main/org/apache/zookeeper/KeeperException.java

@@ -136,198 +136,223 @@ public abstract class KeeperException extends Exception {
         this.code = Code.get(code);
     }
 
-    public static enum Code {
-        /** Everything is OK */
-        OK (0),
-
-        /** System and server-side errors.
-         * This is never thrown by the server, it shouldn't be used other than
-         * to indicate a range. Specifically error codes greater than this
-         * value, but lesser than {@link #APIERROR}, are system errors.
-         */
-        SYSTEMERROR (-1),
-
-        /** A runtime inconsistency was found */
-        RUNTIMEINCONSISTENCY (-2),
-        /** A data inconsistency was found */
-        DATAINCONSISTENCY (-3),
-        /** Connection to the server has been lost */
-        CONNECTIONLOSS (-4),
-        /** Error while marshalling or unmarshalling data */
-        MARSHALLINGERROR (-5),
-        /** Operation is unimplemented */
-        UNIMPLEMENTED (-6),
-        /** Operation timeout */
-        OPERATIONTIMEOUT (-7),
-        /** Invalid arguments */
-        BADARGUMENTS (-8),
-
-        /** API errors.
-         * This is never thrown by the server, it shouldn't be used other than
-         * to indicate a range. Specifically error codes greater than this
-         * value are API errors (while values less than this indicate a 
-         * {@link #SYSTEMERROR}).
-         */
-        APIERROR (-100),
-
-        /** Node does not exist */
-        NONODE (-101),
-        /** Not authenticated */
-        NOAUTH (-102),
-        /** Version conflict */
-        BADVERSION (-103),
-        /** Ephemeral nodes may not have children */
-        NOCHILDRENFOREPHEMERALS (-108),
-        /** The node already exists */
-        NODEEXISTS (-110),
-        /** The node has children */
-        NOTEMPTY (-111),
-        /** The session has been expired by the server */
-        SESSIONEXPIRED (-112),
-        /** Invalid callback specified */
-        INVALIDCALLBACK (-113),
-        /** Invalid ACL specified */
-        INVALIDACL (-114),
-        /** Client authentication failed */
-        AUTHFAILED (-115);
-
-        private static final Map<Integer,Code> lookup
-            = new HashMap<Integer,Code>();
-
-        static {
-            for(Code c : EnumSet.allOf(Code.class))
-                lookup.put(c.code, c);
-        }
-
-        private final int code;
-        Code(int code) {
-            this.code = code;
-        }
-
-        /**
-         * Get the int value for a particular Code.
-         * @return error code as integer
-         */
-        public int intValue() { return code; }
-
-        /**
-         * Get the Code value for a particular integer error code
-         * @param code int error code
-         * @return Code value corresponding to specified int code, or null
-         */
-        public static Code get(int code) {
-            return lookup.get(code);
-        }
-
+    /** This interface contains the original static final int constants
+     * which have now been replaced with an enumeration in Code. Do not
+     * reference this class directly, if necessary (legacy code) continue
+     * to access the constants through Code.
+     * Note: an interface is used here due to the fact that enums cannot
+     * reference constants defined within the same enum as said constants
+     * are considered initialized _after_ the enum itself. By using an
+     * interface as a super type this allows the deprecated constants to
+     * be initialized first and referenced when constructing the enums. I
+     * didn't want to have constants declared twice. This
+     * interface should be private, but it's declared public to enable
+     * javadoc to include in the user API spec.
+     */
+    @Deprecated
+    public interface CodeDeprecated {
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #OK} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#OK} instead
          */
         @Deprecated
-        public static final int Ok = OK.code;
+        public static final int Ok = 0;
 
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #SYSTEMERROR} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#SYSTEMERROR} instead
          */
         @Deprecated
-        public static final int SystemError = SYSTEMERROR.code;
+        public static final int SystemError = -1;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #RUNTIMEINCONSISTENCY} instead
+         * @deprecated deprecated in 3.1.0, use
+         * {@link Code#RUNTIMEINCONSISTENCY} instead
          */
         @Deprecated
-        public static final int RuntimeInconsistency = RUNTIMEINCONSISTENCY.code;
+        public static final int RuntimeInconsistency = -2;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #DATAINCONSISTENCY}
+         * @deprecated deprecated in 3.1.0, use {@link Code#DATAINCONSISTENCY}
          * instead
          */
         @Deprecated
-        public static final int DataInconsistency = DATAINCONSISTENCY.code;
+        public static final int DataInconsistency = -3;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #CONNECTIONLOSS}
+         * @deprecated deprecated in 3.1.0, use {@link Code#CONNECTIONLOSS}
          * instead
          */
         @Deprecated
-        public static final int ConnectionLoss = CONNECTIONLOSS.code;
+        public static final int ConnectionLoss = -4;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #MARSHALLINGERROR}
+         * @deprecated deprecated in 3.1.0, use {@link Code#MARSHALLINGERROR}
          * instead
          */
         @Deprecated
-        public static final int MarshallingError = MARSHALLINGERROR.code;
+        public static final int MarshallingError = -5;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #UNIMPLEMENTED} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#UNIMPLEMENTED}
+         * instead
          */
         @Deprecated
-        public static final int Unimplemented = UNIMPLEMENTED.code;
+        public static final int Unimplemented = -6;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #OPERATIONTIMEOUT}
+         * @deprecated deprecated in 3.1.0, use {@link Code#OPERATIONTIMEOUT}
          * instead
          */
         @Deprecated
-        public static final int OperationTimeout = OPERATIONTIMEOUT.code;
+        public static final int OperationTimeout = -7;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #BADARGUMENTS} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#BADARGUMENTS}
+         * instead
          */
         @Deprecated
-        public static final int BadArguments = BADARGUMENTS.code;
+        public static final int BadArguments = -8;
 
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #APIERROR} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#APIERROR} instead
          */
         @Deprecated
-        public static final int APIError = APIERROR.code;
+        public static final int APIError = -100;
 
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #NONODE} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#NONODE} instead
          */
         @Deprecated
-        public static final int NoNode = NONODE.code;
+        public static final int NoNode = -101;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #NOAUTH} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#NOAUTH} instead
          */
         @Deprecated
-        public static final int NoAuth = NOAUTH.code;
+        public static final int NoAuth = -102;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #BADVERSION} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#BADVERSION} instead
          */
         @Deprecated
-        public static final int BadVersion = BADVERSION.code;
+        public static final int BadVersion = -103;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #NOCHILDRENFOREPHEMERALS}
+         * @deprecated deprecated in 3.1.0, use
+         * {@link Code#NOCHILDRENFOREPHEMERALS}
          * instead
          */
         @Deprecated
-        public static final int
-            NoChildrenForEphemerals = NOCHILDRENFOREPHEMERALS.code;
+        public static final int NoChildrenForEphemerals = -108;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #NODEEXISTS} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#NODEEXISTS} instead
          */
         @Deprecated
-        public static final int NodeExists = NODEEXISTS.code;
+        public static final int NodeExists = -110;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #NOTEMPTY} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#NOTEMPTY} instead
          */
         @Deprecated
-        public static final int NotEmpty = NOTEMPTY.code;
+        public static final int NotEmpty = -111;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #SESSIONEXPIRED} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#SESSIONEXPIRED} instead
          */
         @Deprecated
-        public static final int SessionExpired = SESSIONEXPIRED.code;
+        public static final int SessionExpired = -112;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #INVALIDCALLBACK} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#INVALIDCALLBACK}
+         * instead
          */
         @Deprecated
-        public static final int InvalidCallback = INVALIDCALLBACK.code;
+        public static final int InvalidCallback = -113;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #INVALIDACL} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#INVALIDACL} instead
          */
         @Deprecated
-        public static final int InvalidACL = INVALIDACL.code;
+        public static final int InvalidACL = -114;
         /**
-         * @deprecated deprecated in 3.1.0, use {@link #AUTHFAILED} instead
+         * @deprecated deprecated in 3.1.0, use {@link Code#AUTHFAILED} instead
          */
         @Deprecated
-        public static final int AuthFailed = AUTHFAILED.code;
+        public static final int AuthFailed = -115;
+    }
+
+    /** Codes which represent the various KeeperException
+     * types. This enum replaces the deprecated earlier static final int
+     * constants. The old, deprecated, values are in "camel case" while the new
+     * enum values are in all CAPS.
+     */
+    public static enum Code implements CodeDeprecated {
+        /** Everything is OK */
+        OK (Ok),
+
+        /** System and server-side errors.
+         * This is never thrown by the server, it shouldn't be used other than
+         * to indicate a range. Specifically error codes greater than this
+         * value, but lesser than {@link #APIERROR}, are system errors.
+         */
+        SYSTEMERROR (SystemError),
+
+        /** A runtime inconsistency was found */
+        RUNTIMEINCONSISTENCY (RuntimeInconsistency),
+        /** A data inconsistency was found */
+        DATAINCONSISTENCY (DataInconsistency),
+        /** Connection to the server has been lost */
+        CONNECTIONLOSS (ConnectionLoss),
+        /** Error while marshalling or unmarshalling data */
+        MARSHALLINGERROR (MarshallingError),
+        /** Operation is unimplemented */
+        UNIMPLEMENTED (Unimplemented),
+        /** Operation timeout */
+        OPERATIONTIMEOUT (OperationTimeout),
+        /** Invalid arguments */
+        BADARGUMENTS (BadArguments),
+
+        /** API errors.
+         * This is never thrown by the server, it shouldn't be used other than
+         * to indicate a range. Specifically error codes greater than this
+         * value are API errors (while values less than this indicate a
+         * {@link #SYSTEMERROR}).
+         */
+        APIERROR (APIError),
+
+        /** Node does not exist */
+        NONODE (NoNode),
+        /** Not authenticated */
+        NOAUTH (NoAuth),
+        /** Version conflict */
+        BADVERSION (BadVersion),
+        /** Ephemeral nodes may not have children */
+        NOCHILDRENFOREPHEMERALS (NoChildrenForEphemerals),
+        /** The node already exists */
+        NODEEXISTS (NodeExists),
+        /** The node has children */
+        NOTEMPTY (NotEmpty),
+        /** The session has been expired by the server */
+        SESSIONEXPIRED (SessionExpired),
+        /** Invalid callback specified */
+        INVALIDCALLBACK (InvalidCallback),
+        /** Invalid ACL specified */
+        INVALIDACL (InvalidACL),
+        /** Client authentication failed */
+        AUTHFAILED (AuthFailed);
+
+        private static final Map<Integer,Code> lookup
+            = new HashMap<Integer,Code>();
+
+        static {
+            for(Code c : EnumSet.allOf(Code.class))
+                lookup.put(c.code, c);
+        }
+
+        private final int code;
+        Code(int code) {
+            this.code = code;
+        }
+
+        /**
+         * Get the int value for a particular Code.
+         * @return error code as integer
+         */
+        public int intValue() { return code; }
+
+        /**
+         * Get the Code value for a particular integer error code
+         * @param code int error code
+         * @return Code value corresponding to specified int code, or null
+         */
+        public static Code get(int code) {
+            return lookup.get(code);
+        }
     }
 
     static String getCodeMessage(Code code) {

src/java/test/org/apache/zookeeper/test/KeeperStateTest.java

@@ -21,9 +21,10 @@ package org.apache.zookeeper.test;
 import java.util.EnumSet;
 
 import junit.framework.TestCase;
-import org.junit.Test;
 
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.KeeperState;
+import org.junit.Test;
 
 public class KeeperStateTest extends TestCase {
     
@@ -47,4 +48,29 @@ public class KeeperStateTest extends TestCase {
         }
 
     }
+
+    /** Validate that the deprecated constant still works. There were issues
+     * found with switch statements - which need compile time constants.
+     */
+    @Test
+    @SuppressWarnings("deprecation")
+    public void testDeprecatedCodeOkInSwitch() {
+        int test = 1;
+        switch (test) {
+        case Code.Ok:
+            assertTrue(true);
+            break;
+        }
+    }
+
+    /** Verify the enum works (paranoid) */
+    @Test
+    public void testCodeOKInSwitch() {
+        Code test = Code.OK;
+        switch (test) {
+        case OK:
+            assertTrue(true);
+            break;
+        }
+    }
 }