GEODE-9851: Use InterestType and DataPolicy over ordinal int. (apache…

…#7103) * Make InterestType an enum and use strong type in method parameters. * Use strong type DataPolicy in method parameters. * Prepare for migration to enum.
marcoman · Dec 10, 2021 · 79475fa · 79475fa
1 parent d89fdf6
commit 79475fa
Show file tree

Hide file tree

Showing 152 changed files with 1,533 additions and 1,015 deletions.
diff --git a/geode-core/build.gradle b/geode-core/build.gradle
@@ -342,6 +342,8 @@ dependencies {
 
   testImplementation(files("${System.getProperty('java.home')}/../lib/tools.jar"))
 
+  testCompileOnly('org.jetbrains:annotations')
+
   testRuntimeOnly('commons-collections:commons-collections')
   testRuntimeOnly('commons-configuration:commons-configuration')
   testRuntimeOnly('commons-io:commons-io')

diff --git a/...distributedTest/java/org/apache/geode/internal/cache/tier/sockets/HAInterestTestCase.java b/...distributedTest/java/org/apache/geode/internal/cache/tier/sockets/HAInterestTestCase.java
@@ -627,7 +627,7 @@ public String description() {
     list.add(k1);
     list.add(k2);
     List serverKeys = srp.registerInterest(list, KEY, KEYS, false,
-        r.getAttributes().getDataPolicy().ordinal);
+        r.getAttributes().getDataPolicy());
     assertNotNull(serverKeys);
     List resultKeys = (List) serverKeys.get(0);
     assertEquals(2, resultKeys.size());
@@ -687,7 +687,7 @@ public String description() {
     list.add(k1);
     list.add(k2);
     List serverKeys = srp.registerInterest(list, KEY, KEYS, false,
-        r.getAttributes().getDataPolicy().ordinal);
+        r.getAttributes().getDataPolicy());
 
     assertNotNull(serverKeys);
     List resultKeys = (List) serverKeys.get(0);
@@ -724,7 +724,7 @@ public String description() {
     list.add(k1);
     list.add(k2);
     List serverKeys = srp.registerInterest(list, KEY, KEYS, false,
-        r.getAttributes().getDataPolicy().ordinal);
+        r.getAttributes().getDataPolicy());
 
     assertNotNull(serverKeys);
     List resultKeys = (List) serverKeys.get(0);
@@ -776,7 +776,7 @@ public static void registerK1AndK2OnPrimaryAndSecondaryAndVerifyResponse() {
 
     // Primary server
     List serverKeys1 = srp.registerInterestOn(primary, list, InterestType.KEY,
-        InterestResultPolicy.KEYS, false, r.getAttributes().getDataPolicy().ordinal);
+        InterestResultPolicy.KEYS, false, r.getAttributes().getDataPolicy());
     assertNotNull(serverKeys1);
     // expect serverKeys in response from primary
     List resultKeys = (List) serverKeys1.get(0);
@@ -786,7 +786,7 @@ public static void registerK1AndK2OnPrimaryAndSecondaryAndVerifyResponse() {
 
     // Secondary server
     List serverKeys2 = srp.registerInterestOn(secondary, list, InterestType.KEY,
-        InterestResultPolicy.KEYS, false, r.getAttributes().getDataPolicy().ordinal);
+        InterestResultPolicy.KEYS, false, r.getAttributes().getDataPolicy());
     // if the list is null then it is empty
     if (serverKeys2 != null) {
       // no serverKeys in response from secondary

diff --git a/geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt b/geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
@@ -104,3 +104,4 @@ org/apache/geode/cache/query/internal/xml/ElementType$2
 org/apache/geode/cache/query/internal/xml/ElementType$3
 org/apache/geode/internal/net/ByteBufferVendor$OpenAttemptTimedOut
 org/apache/geode/internal/cache/wan/GatewaySenderEventImpl$TransactionMetadataDisposition
+org/apache/geode/internal/cache/tier/InterestType
diff --git a/...c/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt b/...c/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
@@ -1447,8 +1447,8 @@ fromData,62
 toData,39
 
 org/apache/geode/internal/cache/partitioned/FetchKeysMessage,2
-fromData,48
-toData,48
+fromData,51
+toData,51
 
 org/apache/geode/internal/cache/partitioned/FetchKeysMessage$FetchKeysReplyMessage,2
 fromData,55
@@ -1714,8 +1714,8 @@ fromData,82
 toData,99
 
 org/apache/geode/internal/cache/tier/sockets/ClientInterestMessageImpl,2
-fromData,84
-toData,81
+fromData,87
+toData,84
 
 org/apache/geode/internal/cache/tier/sockets/ClientMarkerMessageImpl,2
 fromData,12

diff --git a/geode-core/src/main/java/org/apache/geode/cache/DataPolicy.java b/geode-core/src/main/java/org/apache/geode/cache/DataPolicy.java
@@ -16,6 +16,8 @@
 
 package org.apache.geode.cache;
 
+import static java.lang.String.format;
+
 import java.io.ObjectStreamException;
 
 import org.apache.geode.annotations.Immutable;
@@ -58,6 +60,9 @@ public class DataPolicy implements java.io.Serializable {
   @Immutable
   private static final DataPolicy[] VALUES = new DataPolicy[11];
 
+  @Immutable
+  private static final String[] NAMES = new String[VALUES.length];
+
   /**
    * Data is never stored in local memory. The region will always be empty locally. It can be used
    * to for zero footprint producers that only want to distribute their data to others and for zero
@@ -126,7 +131,12 @@ public class DataPolicy implements java.io.Serializable {
   /** The name of this mirror type. */
   private final transient String name;
 
-  /** used as ordinal to represent this DataPolicy */
+  /**
+   * Used as ordinal to represent this DataPolicy
+   *
+   * @deprecated use {@link #ordinal()}
+   */
+  @Deprecated
   public final byte ordinal;
 
   private Object readResolve() throws ObjectStreamException {
@@ -135,20 +145,49 @@ private Object readResolve() throws ObjectStreamException {
 
 
   /** Creates a new instance of DataPolicy. */
-  private DataPolicy(int ordinal, String name) {
+  private DataPolicy(final int ordinal, final String name) {
     if (ordinal >= VALUES.length) {
       throw new IllegalArgumentException(
-          String.format("Only %s DataPolicies may be defined",
-              Integer.valueOf(VALUES.length + 1)));
+          format("Only %s DataPolicies may be defined", VALUES.length + 1));
     }
     if (VALUES[ordinal] != null) {
       throw new IllegalArgumentException(
-          String.format("Ordinal %s is already defined by %s",
-              new Object[] {Integer.valueOf(ordinal), VALUES[ordinal]}));
+          format("Ordinal %s is already defined by %s", ordinal, VALUES[ordinal]));
     }
     this.name = name;
     this.ordinal = (byte) (ordinal & 0xff);
     VALUES[this.ordinal] = this;
+    NAMES[this.ordinal] = name;
+  }
+
+  /**
+   * @return ordinal value.
+   */
+  public int ordinal() {
+    return ordinal;
+  }
+
+  /**
+   * Get enum value by name.
+   *
+   * @param name of enum value.
+   * @return enum by name.
+   * @throws IllegalArgumentException if the specified enum type has no constant with the specified
+   *         name.
+   * @throws NullPointerException if name is null.
+   */
+  public static DataPolicy valueOf(final String name) throws IllegalArgumentException {
+    if (null == name) {
+      throw new NullPointerException();
+    }
+
+    for (int i = 0; i < NAMES.length; i++) {
+      if (NAMES[i].equals(name)) {
+        return VALUES[i];
+      }
+    }
+
+    throw new IllegalArgumentException(name);
   }
 
   /** Return the DataPolicy represented by specified ordinal */
@@ -299,25 +338,18 @@ public boolean isPartition() {
    */
   @Override
   public String toString() {
-    return this.name;
+    return name;
   }
 
-  public static DataPolicy fromString(String s) {
-    String[] allowedValues =
-        new String[] {"EMPTY", "NORMAL", "REPLICATE", "PERSISTENT_REPLICATE", "PARTITION",
-            "PRELOADED", "PERSISTENT_PARTITION"};
-    int valueIndex = -1;
-    for (int i = 0; i < allowedValues.length; i++) {
-      if (allowedValues[i].equals(s)) {
-        valueIndex = i;
-        break;
-      }
-    }
-
-    if (valueIndex != -1) {
-      return VALUES[valueIndex];
+  /**
+   * @deprecated use {@link #valueOf(String)}
+   */
+  @Deprecated
+  public static DataPolicy fromString(final String s) {
+    try {
+      return valueOf(s);
+    } catch (NullPointerException | IllegalArgumentException e) {
+      return null;
     }
-
-    return null;
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
@@ -21,6 +21,8 @@
 import java.net.SocketTimeoutException;
 
 import org.apache.logging.log4j.Logger;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import org.apache.geode.InternalGemFireError;
 import org.apache.geode.cache.client.ServerConnectivityException;
@@ -198,40 +200,37 @@ protected boolean needsUserId() {
    * @return the result of the operation or <code>null</code> if the operation has no result.
    * @throws Exception if the execute failed
    */
-  protected Object attemptReadResponse(Connection cnx) throws Exception {
-    Message msg = createResponseMessage();
-    if (msg != null) {
-      msg.setComms(cnx.getSocket(), cnx.getInputStream(), cnx.getOutputStream(),
-          cnx.getCommBuffer(), cnx.getStats());
-      if (msg instanceof ChunkedMessage) {
-        try {
-          return processResponse(msg, cnx);
-        } finally {
-          msg.unsetComms();
-          processSecureBytes(cnx, msg);
-        }
-      } else {
-        try {
-          msg.receive();
-        } finally {
-          msg.unsetComms();
-          processSecureBytes(cnx, msg);
-        }
+  protected Object attemptReadResponse(final @NotNull Connection cnx) throws Exception {
+    final Message msg = createResponseMessage();
+    msg.setComms(cnx.getSocket(), cnx.getInputStream(), cnx.getOutputStream(),
+        cnx.getCommBuffer(), cnx.getStats());
+    if (msg instanceof ChunkedMessage) {
+      try {
         return processResponse(msg, cnx);
+      } finally {
+        msg.unsetComms();
+        processSecureBytes(cnx, msg);
       }
     } else {
-      return null;
+      try {
+        msg.receive();
+      } finally {
+        msg.unsetComms();
+        processSecureBytes(cnx, msg);
+      }
+      return processResponse(msg, cnx);
     }
   }
 
   /**
    * By default just create a normal one part msg. Subclasses can override this.
    */
-  protected Message createResponseMessage() {
+  protected @NotNull Message createResponseMessage() {
     return new Message(1, KnownVersion.CURRENT);
   }
 
-  protected Object processResponse(Message m, Connection con) throws Exception {
+  protected @Nullable Object processResponse(final @NotNull Message m,
+      final @NotNull Connection con) throws Exception {
     return processResponse(m);
   }
 
@@ -242,7 +241,7 @@ protected Object processResponse(Message m, Connection con) throws Exception {
    * @throws Exception if response could not be processed or we received a response with a server
    *         exception.
    */
-  protected abstract Object processResponse(Message msg) throws Exception;
+  protected abstract @Nullable Object processResponse(final @NotNull Message msg) throws Exception;
 
   /**
    * Return true of <code>messageType</code> indicates the operation had an error on the server.

diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXEnumOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXEnumOp.java
@@ -14,6 +14,8 @@
  */
 package org.apache.geode.cache.client.internal;
 
+import org.jetbrains.annotations.NotNull;
+
 import org.apache.geode.internal.cache.tier.MessageType;
 import org.apache.geode.internal.cache.tier.sockets.Message;
 import org.apache.geode.pdx.internal.EnumInfo;
@@ -50,7 +52,7 @@ public AddPdxEnumOpImpl(int id, EnumInfo ei) {
     }
 
     @Override
-    protected Object processResponse(Message msg) throws Exception {
+    protected Object processResponse(final @NotNull Message msg) throws Exception {
       processAck(msg, "addPDXEnum");
       return null;
     }

diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXTypeOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXTypeOp.java
@@ -14,6 +14,8 @@
  */
 package org.apache.geode.cache.client.internal;
 
+import org.jetbrains.annotations.NotNull;
+
 import org.apache.geode.internal.cache.tier.MessageType;
 import org.apache.geode.internal.cache.tier.sockets.Message;
 import org.apache.geode.pdx.internal.PdxType;
@@ -50,7 +52,7 @@ public AddPDXTypeOpImpl(int id, PdxType type) {
     }
 
     @Override
-    protected Object processResponse(Message msg) throws Exception {
+    protected Object processResponse(final @NotNull Message msg) throws Exception {
       processAck(msg, "addPDXType");
       return null;
     }

diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AuthenticateUserOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AuthenticateUserOp.java
@@ -19,6 +19,8 @@
 
 import java.util.Properties;
 
+import org.jetbrains.annotations.NotNull;
+
 import org.apache.geode.DataSerializer;
 import org.apache.geode.InternalGemFireError;
 import org.apache.geode.annotations.VisibleForTesting;
@@ -205,7 +207,8 @@ Object parentAttempt(Connection connection) throws Exception {
 
 
     @Override
-    protected Object processResponse(Message msg, Connection connection) throws Exception {
+    protected Object processResponse(final @NotNull Message msg,
+        final @NotNull Connection connection) throws Exception {
       byte[] bytes;
       Part part = msg.getPart(0);
       final int msgType = msg.getMessageType();
@@ -275,7 +278,7 @@ protected void endAttempt(ConnectionStats stats, long start) {
     }
 
     @Override
-    protected Object processResponse(Message msg) throws Exception {
+    protected Object processResponse(final @NotNull Message msg) throws Exception {
       return null;
     }
 

diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClearOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClearOp.java
@@ -14,6 +14,8 @@
  */
 package org.apache.geode.cache.client.internal;
 
+import org.jetbrains.annotations.NotNull;
+
 import org.apache.geode.internal.cache.EventID;
 import org.apache.geode.internal.cache.tier.MessageType;
 import org.apache.geode.internal.cache.tier.sockets.Message;
@@ -73,7 +75,7 @@ public ClearOpImpl(String region, EventID eventId, Object callbackArg) {
     }
 
     @Override
-    protected Object processResponse(Message msg) throws Exception {
+    protected Object processResponse(final @NotNull Message msg) throws Exception {
       processAck(msg, "clear region");
       return null;
     }

diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/CloseConnectionOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/CloseConnectionOp.java
@@ -16,6 +16,8 @@
 
 import java.io.EOFException;
 
+import org.jetbrains.annotations.NotNull;
+
 import org.apache.geode.internal.cache.tier.MessageType;
 import org.apache.geode.internal.cache.tier.sockets.Message;
 
@@ -65,7 +67,7 @@ protected void sendMessage(Connection cnx) throws Exception {
     }
 
     @Override
-    protected Object processResponse(Message msg) throws Exception {
+    protected Object processResponse(final @NotNull Message msg) throws Exception {
       // CloseConnectionOp doesn't return anything - we wait for a response
       // so that we know that the server has processed the request before
       // we return from execute()