]> source.dussan.org Git - vaadin-framework.git/commitdiff
Improve proxy comparison support (#14639)
authorJuuso Valli <juuso@vaadin.com>
Sat, 13 Sep 2014 12:19:55 +0000 (15:19 +0300)
committerVaadin Code Review <review@vaadin.com>
Mon, 15 Sep 2014 11:15:56 +0000 (11:15 +0000)
Change-Id: I114ea5bf9d55c78826c1163206caf585b96143ef

server/src/com/vaadin/server/AbstractClientConnector.java
server/tests/src/com/vaadin/server/AbstractClientConnectorProxyHandlingTest.java [new file with mode: 0644]

index 32277fcc3a95952e6eda1d38b8665cf26ec440c9..87b61c96231b8ffcc9842bc7a2ca4e07d92be55e 100644 (file)
@@ -999,11 +999,6 @@ public abstract class AbstractClientConnector implements ClientConnector,
         this.errorHandler = errorHandler;
     }
 
-    private AbstractClientConnector getInstance() {
-        // returns the underlying instance regardless of proxies
-        return this;
-    }
-
     /*
      * (non-Javadoc)
      * 
@@ -1011,12 +1006,36 @@ public abstract class AbstractClientConnector implements ClientConnector,
      */
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        /*
+         * This equals method must return true when we're comparing an object to
+         * its proxy. This happens a lot with CDI (and possibly Spring) when
+         * we're injecting Components. See #14639
+         */
         if (obj instanceof AbstractClientConnector) {
-            return super.equals(((AbstractClientConnector) obj).getInstance());
+            AbstractClientConnector connector = (AbstractClientConnector) obj;
+            return connector.isThis(this);
         }
         return false;
     }
 
+    /**
+     * For internal use only, may be changed or removed in future versions.
+     * <p>
+     * This method must be protected, because otherwise it will not be redefined
+     * by the proxy to actually be called on the underlying instance.
+     * <p>
+     * See #14639
+     * 
+     * @deprecated only defined for framework hacks, do not use.
+     */
+    @Deprecated
+    protected boolean isThis(Object that) {
+        return this == that;
+    }
+
     /*
      * (non-Javadoc)
      * 
diff --git a/server/tests/src/com/vaadin/server/AbstractClientConnectorProxyHandlingTest.java b/server/tests/src/com/vaadin/server/AbstractClientConnectorProxyHandlingTest.java
new file mode 100644 (file)
index 0000000..c15676c
--- /dev/null
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.server;
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * We test that AbstractClientConnector has a suitable isThis method which is
+ * needed to correctly perform an equals check between a proxy and it's
+ * underlying instance.
+ * 
+ * @author Vaadin Ltd
+ */
+public class AbstractClientConnectorProxyHandlingTest {
+
+    @Test
+    public void abstractClientConnectorTest() {
+        try {
+            Method method = AbstractClientConnector.class.getDeclaredMethod(
+                    "isThis", Object.class);
+            int modifiers = method.getModifiers();
+            if (Modifier.isFinal(modifiers) || !Modifier.isProtected(modifiers)
+                    || Modifier.isStatic(modifiers)) {
+                Assert.fail("isThis has invalid modifiers, CDI proxies will not work.");
+            }
+        } catch (SecurityException e) {
+            // Ignore, no can do
+        } catch (NoSuchMethodException e) {
+            Assert.fail("isThis is missing, CDI proxies will not work.");
+        }
+    }
+
+}