De-confuse InvocationHandler and target methods 14/86214/4
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 1 Dec 2019 10:04:32 +0000 (11:04 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 5 Dec 2019 16:55:53 +0000 (17:55 +0100)
Having hashCode()/equals()/toString() implemented in InvocationHandler
could lead to confusing the handler with the object being proxied. It
furthermore violates equality contract requirement of being reflexive
-- the handler ends up being equal to other implementations of proxied
interface, but any other implementation would either have to handle
it specifically, or will determined it being non-equal.

Clean up the confusion by renaming the methods and have default
implementations take over. This is explicitly fine, as we do not have
any code accessing the handler directly, only through the proxy.
This also eliminates the need to suppress warnings.

Change-Id: Iac42d09b5a44716e8293c6b1e9003a780efae6f8
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 2cf87f876c50a276f6e5ddafc2fb44d5b6c26fbd)

library/impl/src/main/java/org/opendaylight/ovsdb/lib/schema/typed/TyperUtils.java

index 1d14de9af90a9d41ff8bfe8beb019b4c7d130c53..33316f8b54b3cea2925b815f6360739bd0118a86 100644 (file)
@@ -13,7 +13,6 @@ import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Range;
 import com.google.common.reflect.Reflection;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.util.HashMap;
@@ -33,7 +32,6 @@ import org.opendaylight.ovsdb.lib.notation.Version;
 import org.opendaylight.ovsdb.lib.schema.ColumnSchema;
 import org.opendaylight.ovsdb.lib.schema.DatabaseSchema;
 import org.opendaylight.ovsdb.lib.schema.GenericTableSchema;
-import org.opendaylight.ovsdb.lib.schema.TableSchema;
 
 /**
  * Utility methods for typed OVSDB schema data.
@@ -293,11 +291,8 @@ public final class TyperUtils {
                 return proxy;
             }
 
-            private Object processGetTableSchema() {
-                if (dbSchema == null) {
-                    return null;
-                }
-                return getTableSchema(dbSchema, klazz);
+            private GenericTableSchema processGetTableSchema() {
+                return dbSchema == null ? null : getTableSchema(dbSchema, klazz);
             }
 
             private Boolean isHashCodeMethod(final Method method, final Object[] args) {
@@ -328,49 +323,29 @@ public final class TyperUtils {
                 } else if (isGetColumn(method)) {
                     return processGetColumn(method);
                 } else if (isHashCodeMethod(method, args)) {
-                    return hashCode();
+                    return processHashCode();
                 } else if (isEqualsMethod(method, args)) {
-                    return proxy.getClass().isInstance(args[0]) && this.equals(args[0]);
+                    return proxy.getClass().isInstance(args[0]) && processEquals(args[0]);
                 } else if (isToStringMethod(method, args)) {
-                    return this.toString();
+                    return processToString();
                 }
                 throw new UnsupportedMethodException("Method not supported " + method.toString());
             }
 
-            @Override
-            @SuppressFBWarnings({"EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS", "EQ_UNUSUAL"})
-            public boolean equals(final Object obj) {
-                if (!(obj instanceof TypedBaseTable)) {
-                    return false;
-                }
-                TypedBaseTable<?> typedRowObj = (TypedBaseTable<?>)obj;
-                return Objects.equal(row, typedRowObj.getRow());
+            private boolean processEquals(final Object obj) {
+                return obj instanceof TypedBaseTable && Objects.equal(row, ((TypedBaseTable<?>)obj).getRow());
             }
 
-            @Override
-            public int hashCode() {
-                if (row == null) {
-                    return 0;
-                }
-                return row.hashCode();
+            private int processHashCode() {
+                return row == null ? 0 : row.hashCode();
             }
 
-            @Override
-            public String toString() {
-                String tableName;
-                TableSchema<?> schema = (TableSchema<?>)processGetTableSchema();
-                if (schema != null) {
-                    tableName = schema.getName();
-                } else {
-                    tableName = "";
-                }
-                if (row == null) {
-                    return tableName;
-                }
-                return tableName + " : " + row.toString();
+            private String processToString() {
+                final GenericTableSchema schema = processGetTableSchema();
+                final String tableName = schema != null ? schema.getName() : "";
+                return row == null ? tableName : tableName + " : " + row.toString();
             }
-        }
-        );
+        });
     }
 
     /**