From 04413150ac7ef28e50d9e207080f2a00db5a59be Mon Sep 17 00:00:00 2001
From: =?utf8?q?Leif=20=C3=85strand?= <leif@vaadin.com>
Date: Thu, 11 Oct 2012 10:31:11 +0300
Subject: [PATCH] Use class hierachy for resolving @Connect conflicts (#9826)

Change-Id: Ic268bfb5704a5c9113dee80017196c72b5236b93
---
 .../ConnectorBundleLoaderFactory.java         | 83 ++++++++++++++++++-
 .../metadata/ConnectorInitVisitor.java        | 17 ----
 .../client/IntermediateReplaceConnector.java  | 31 +++++++
 .../widgetset/client/ReplacedConnector.java   | 37 +++++++++
 .../widgetset/client/ReplacingConnector.java  | 30 +++++++
 .../widgetset/server/ReplaceComponent.html    | 26 ++++++
 .../widgetset/server/ReplaceComponent.java    | 23 +++++
 .../widgetset/server/ReplaceComponentUI.java  | 42 ++++++++++
 8 files changed, 268 insertions(+), 21 deletions(-)
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/client/IntermediateReplaceConnector.java
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/client/ReplacedConnector.java
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/client/ReplacingConnector.java
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponent.html
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponent.java
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponentUI.java

diff --git a/client-compiler/src/com/vaadin/server/widgetsetutils/ConnectorBundleLoaderFactory.java b/client-compiler/src/com/vaadin/server/widgetsetutils/ConnectorBundleLoaderFactory.java
index aa220225c9..0e29c07405 100644
--- a/client-compiler/src/com/vaadin/server/widgetsetutils/ConnectorBundleLoaderFactory.java
+++ b/client-compiler/src/com/vaadin/server/widgetsetutils/ConnectorBundleLoaderFactory.java
@@ -583,10 +583,12 @@ public class ConnectorBundleLoaderFactory extends Generator {
         JClassType connectorType = typeOracle.getType(ServerConnector.class
                 .getName());
         JClassType[] subtypes = connectorType.getSubtypes();
-        for (JClassType connectorSubtype : subtypes) {
-            if (!connectorSubtype.isAnnotationPresent(Connect.class)) {
-                continue;
-            }
+
+        // Find all types with a valid mapping
+        Collection<JClassType> selectedTypes = getMappedTypes(logger, subtypes);
+
+        // Group by load style
+        for (JClassType connectorSubtype : selectedTypes) {
             LoadStyle loadStyle = getLoadStyle(connectorSubtype);
             if (loadStyle != null) {
                 connectorsByLoadStyle.get(loadStyle).add(connectorSubtype);
@@ -637,6 +639,79 @@ public class ConnectorBundleLoaderFactory extends Generator {
         return bundles;
     }
 
+    private Collection<JClassType> getMappedTypes(TreeLogger logger,
+            JClassType[] types) throws UnableToCompleteException {
+        Map<String, JClassType> mappings = new HashMap<String, JClassType>();
+
+        // Keep track of what has happened to avoid logging intermediate state
+        Map<JClassType, List<JClassType>> replaced = new HashMap<JClassType, List<JClassType>>();
+
+        for (JClassType type : types) {
+            Connect connectAnnotation = type.getAnnotation(Connect.class);
+            if (connectAnnotation == null) {
+                continue;
+            }
+
+            String identifier = connectAnnotation.value().getCanonicalName();
+
+            JClassType previousMapping = mappings.put(identifier, type);
+            if (previousMapping != null) {
+                // There are multiple mappings, pick the subclass
+                JClassType subclass;
+                JClassType superclass;
+                if (previousMapping.isAssignableFrom(type)) {
+                    subclass = type;
+                    superclass = previousMapping;
+                } else if (type.isAssignableFrom(previousMapping)) {
+                    subclass = previousMapping;
+                    superclass = type;
+                } else {
+                    // Neither inherits from the other - this is a conflict
+                    logger.log(
+                            Type.ERROR,
+                            "Conflicting @Connect mappings detected for "
+                                    + identifier
+                                    + ": "
+                                    + type.getQualifiedSourceName()
+                                    + " and "
+                                    + previousMapping.getQualifiedSourceName()
+                                    + ". There can only be multiple @Connect mappings for the same server-side type if one is the subclass of the other.");
+                    throw new UnableToCompleteException();
+                }
+
+                mappings.put(identifier, subclass);
+
+                // Inherit any previous replacements
+                List<JClassType> previousReplacements = replaced
+                        .remove(superclass);
+                if (previousReplacements == null) {
+                    previousReplacements = new ArrayList<JClassType>();
+                }
+
+                previousReplacements.add(superclass);
+                replaced.put(subclass, previousReplacements);
+            }
+        }
+
+        // Log the final set of replacements
+        for (Entry<JClassType, List<JClassType>> entry : replaced.entrySet()) {
+            String msg = entry.getKey().getQualifiedSourceName() + " replaces ";
+
+            List<JClassType> list = entry.getValue();
+            for (int i = 0; i < list.size(); i++) {
+                if (i != 0) {
+                    msg += ", ";
+                }
+                msg += list.get(i).getQualifiedSourceName();
+            }
+
+            logger.log(Type.INFO, msg);
+        }
+
+        // Return the types of the final mapping
+        return mappings.values();
+    }
+
     private Collection<TypeVisitor> getVisitors(TypeOracle oracle)
             throws NotFoundException {
         List<TypeVisitor> visitors = Arrays.<TypeVisitor> asList(
diff --git a/client-compiler/src/com/vaadin/server/widgetsetutils/metadata/ConnectorInitVisitor.java b/client-compiler/src/com/vaadin/server/widgetsetutils/metadata/ConnectorInitVisitor.java
index 7500d65443..253d657af0 100644
--- a/client-compiler/src/com/vaadin/server/widgetsetutils/metadata/ConnectorInitVisitor.java
+++ b/client-compiler/src/com/vaadin/server/widgetsetutils/metadata/ConnectorInitVisitor.java
@@ -4,19 +4,14 @@
 
 package com.vaadin.server.widgetsetutils.metadata;
 
-import java.util.Map;
-
 import com.google.gwt.core.ext.TreeLogger;
 import com.google.gwt.core.ext.TreeLogger.Type;
 import com.google.gwt.core.ext.UnableToCompleteException;
 import com.google.gwt.core.ext.typeinfo.JClassType;
-import com.google.gwt.dev.util.collect.HashMap;
 import com.vaadin.shared.ui.Connect;
 
 public class ConnectorInitVisitor extends TypeVisitor {
 
-    private Map<String, JClassType> processedConnections = new HashMap<String, JClassType>();
-
     @Override
     public void visitConnector(TreeLogger logger, JClassType type,
             ConnectorBundle bundle) throws UnableToCompleteException {
@@ -26,18 +21,6 @@ public class ConnectorInitVisitor extends TypeVisitor {
                     + bundle.getName().replaceAll("^_*", "") + " bundle");
             String identifier = connectAnnotation.value().getCanonicalName();
 
-            JClassType previousMapping = processedConnections.put(identifier,
-                    type);
-            if (previousMapping != null) {
-                logger.log(
-                        Type.ERROR,
-                        "Multiple @Connect mappings detected for " + identifier
-                                + ": " + type.getQualifiedSourceName()
-                                + " and "
-                                + previousMapping.getQualifiedSourceName());
-                throw new UnableToCompleteException();
-            }
-
             bundle.setIdentifier(type, identifier);
             bundle.setNeedsGwtConstructor(type);
         }
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/IntermediateReplaceConnector.java b/uitest/src/com/vaadin/tests/widgetset/client/IntermediateReplaceConnector.java
new file mode 100644
index 0000000000..7f2b5a9a93
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/client/IntermediateReplaceConnector.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2012 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.tests.widgetset.client;
+
+import com.vaadin.shared.ui.Connect;
+import com.vaadin.tests.widgetset.server.ReplaceComponent;
+
+@Connect(value = ReplaceComponent.class)
+public class IntermediateReplaceConnector extends ReplacedConnector {
+    @Override
+    protected void init() {
+        super.init();
+        getWidget().setHTML(
+                IntermediateReplaceConnector.class.getName()
+                        + ", should not be used");
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/ReplacedConnector.java b/uitest/src/com/vaadin/tests/widgetset/client/ReplacedConnector.java
new file mode 100644
index 0000000000..7f116e4803
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/client/ReplacedConnector.java
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2012 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.tests.widgetset.client;
+
+import com.google.gwt.user.client.ui.HTML;
+import com.vaadin.client.ui.AbstractComponentConnector;
+import com.vaadin.shared.ui.Connect;
+import com.vaadin.tests.widgetset.server.ReplaceComponent;
+
+@Connect(ReplaceComponent.class)
+public class ReplacedConnector extends AbstractComponentConnector {
+
+    @Override
+    protected void init() {
+        getWidget().setHTML(
+                ReplacedConnector.class.getName() + ", should not be used");
+    }
+
+    @Override
+    public HTML getWidget() {
+        return (HTML) super.getWidget();
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/ReplacingConnector.java b/uitest/src/com/vaadin/tests/widgetset/client/ReplacingConnector.java
new file mode 100644
index 0000000000..03e50aab16
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/client/ReplacingConnector.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright 2012 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.tests.widgetset.client;
+
+import com.vaadin.shared.ui.Connect;
+import com.vaadin.tests.widgetset.server.ReplaceComponent;
+
+@Connect(value = ReplaceComponent.class)
+public class ReplacingConnector extends IntermediateReplaceConnector {
+    @Override
+    protected void init() {
+        super.init();
+        getWidget().setHTML(
+                ReplacingConnector.class.getName() + ", this is the right one");
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponent.html b/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponent.html
new file mode 100644
index 0000000000..79b231949c
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponent.html
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="" />
+<title>New Test</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">New Test</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.widgetset.server.ReplaceComponentUI?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestswidgetsetserverReplaceComponentUI::/VVerticalLayout[0]/VOrderedLayout$Slot[1]/VVerticalLayout[0]/VOrderedLayout$Slot[0]/HTML[0]</td>
+	<td>com.vaadin.tests.widgetset.client.ReplacingConnector, this is the right one</td>
+</tr>
+</tbody></table>
+</body>
+</html>
diff --git a/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponent.java b/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponent.java
new file mode 100644
index 0000000000..81c8af95a6
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponent.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2012 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.tests.widgetset.server;
+
+import com.vaadin.ui.AbstractComponent;
+
+public class ReplaceComponent extends AbstractComponent {
+
+}
diff --git a/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponentUI.java b/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponentUI.java
new file mode 100644
index 0000000000..1f83cb0d48
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/server/ReplaceComponentUI.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2012 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.tests.widgetset.server;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.widgetset.TestingWidgetSet;
+
+@Widgetset(TestingWidgetSet.NAME)
+public class ReplaceComponentUI extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        addComponent(new ReplaceComponent());
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Tests that the right client-side connector is used when there are multiple connectors with @Connect mappings to the same server-side component.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(9826);
+    }
+
+}
-- 
2.39.5