Add failure condition when extensions collide on package name

Taifun reported that importing many extensions of the same package
name into a single project made them unable to load. This change
detects this scenario and returns an error to the user preventing them
from loading the conflicting extension. There is also a test to ensure
that if one has two extensions in the same package and then loads a
bundle containing the individually FQCN extensions the old extensions
will be removed and placed with the bundled version.

Change-Id: If35663d650d3a24f6c5aaeceaf490a24ac008522

appinventor/appengine/src/com/google/appinventor/client/wizards/ComponentImportWizard.java

@@ -53,7 +53,7 @@ public class ComponentImportWizard extends Wizard {
     @Override
     public void onSuccess(ComponentImportResponse response) {
       if (response.getStatus() == ComponentImportResponse.Status.FAILED){
-        Window.alert(MESSAGES.componentImportError());
+        Window.alert(MESSAGES.componentImportError() + "\n" + response.getMessage());
         return;
       }
       else if (response.getStatus() != ComponentImportResponse.Status.IMPORTED &&
@@ -65,8 +65,12 @@ public class ComponentImportWizard extends Wizard {
         Window.alert(MESSAGES.componentImportUnknownURLError());
       }
       else if (response.getStatus() == ComponentImportResponse.Status.UPGRADED) {
-        String componentName = SimpleComponentDatabase.getInstance().getComponentName(response.getComponentType());
-        Window.alert(MESSAGES.componentUpgradedAlert() + componentName + " !");
+        StringBuilder sb = new StringBuilder(MESSAGES.componentUpgradedAlert());
+        for (String name : response.getComponentTypes().values()) {
+          sb.append("\n");
+          sb.append(name);
+        }
+        Window.alert(sb.toString());
       }
 
       List<ProjectNode> compNodes = response.getNodes();

appinventor/appengine/src/com/google/appinventor/server/ComponentServiceImpl.java

@@ -44,6 +44,8 @@ import org.json.JSONObject;
 public class ComponentServiceImpl extends OdeRemoteServiceServlet
     implements ComponentService {
 
+  private static final String CANNOT_UPGRADE_MESSAGE = "An extension containing %s already exists" +
+      " on the server but could not be upgraded. The new extension was not loaded.";
   private static final Logger LOG =
       Logger.getLogger(ComponentServiceImpl.class.getName());
 
@@ -186,12 +188,18 @@ public class ComponentServiceImpl extends OdeRemoteServiceServlet
       // upgrade of one or more existing extensions
       try {
         Map<String, JSONObject> componentMap = makeComponentMap(newComponents);
+        boolean willCollide = sourceFiles.contains(basepath + nameMap.get("classes.jar"));
         Iterator<String> i = oldTypes.iterator();
         while (i.hasNext()) {
           String extension = i.next();
           if (upgradeOldExtension(userId, projectId, basepath, extension,
               existingExtensions.get(extension), componentMap)) {
             status = Status.UPGRADED;
+          } else if (willCollide) {
+            // collision but we are not upgrading an existing extension? abort!
+            response.setStatus(Status.FAILED);
+            response.setMessage(String.format(CANNOT_UPGRADE_MESSAGE, extension));
+            return;
           } else {
             // no overlap between the old and new extensions, so don't delete!
             i.remove();

appinventor/appengine/tests/com/google/appinventor/server/ComponentServiceTest.java

@@ -7,7 +7,6 @@ package com.google.appinventor.server;
 
 import com.google.appinventor.common.testutils.TestUtils;
 import com.google.appinventor.server.encryption.KeyczarEncryptor;
-import com.google.appinventor.server.storage.StorageIo;
 import com.google.appinventor.server.storage.StorageIoInstanceHolder;
 import com.google.appinventor.shared.rpc.component.ComponentImportResponse;
 import com.google.appinventor.shared.rpc.component.ComponentImportResponse.Status;
@@ -24,7 +23,6 @@ import org.junit.Test;
 import java.io.File;
 import java.io.FileInputStream;
 import java.net.URI;
-import java.util.Arrays;
 import java.util.List;
 
 import static org.junit.Assert.*;
@@ -55,7 +53,6 @@ public class ComponentServiceTest {
   @Before
   public void setUp() throws Exception {
     helper.setUp();
-    StorageIo storageIo = StorageIoInstanceHolder.INSTANCE;
     LocalUser localUserMock = LocalUser.getInstance();
     localUserMock.set(new User("1", "NonSuch", "NoName", null, 0, false, false, 0, null));
     localUserMock.setSessionId("test-session");
@@ -219,6 +216,47 @@ public class ComponentServiceTest {
     assertAssetsOnServer("assets/external_comps/test/components.json");
   }
 
+  /**
+   * Tests whether two extensions compiled with the same package name will collide. This won't
+   * happen if one adheres to using a single package per set of extension. However, in practice
+   * there have been reports of older extensions using the same package name being upgraded to the
+   * new extensions model and then one extension overwrites another due to collisions at the
+   * package name level. For example, com.example.Bar overwrites com.example.Foo because both
+   * packages are com.example. Extension authors should use one package per extension bundle, so
+   * com.example.foo.Foo and com.example.bar.Bar would be acceptable, or generate a single package
+   * with both extensions, com.example containing com.example.Foo and com.example.Bar.
+   */
+  @Test
+  public void testFailIfPackageCollisionWithDifferingComponents() {
+    importTestExtension("test.Extension3.aix");
+    ComponentImportResponse result = importTestExtension("test.Extension4.aix");
+    assertEquals(Status.FAILED, result.getStatus());
+    assertEquals(0, result.getComponentTypes().size());
+    assertNotNull(result.getMessage());
+    System.err.println(result.getMessage());
+  }
+
+  /**
+   * Tests to ensure that if multiple (old) extensions share the same package name that after
+   * importing a new style extension bundle containing the union of the old extensions will result
+   * in the old extension files being removed and replaced with the contents of the single new
+   * extension.
+   */
+  @Test
+  public void testUpgradeManyOldExtensionsToOneBundle() {
+    importTestExtension("FooOld.aix");
+    importTestExtension("BarOld.aix");
+    ComponentImportResponse result = importTestExtension("FooBar.aix");
+    assertEquals(Status.UPGRADED, result.getStatus());
+    assertEquals(2, result.getComponentTypes().size());
+    assertNull(result.getMessage());
+    assertTrue(result.getComponentTypes().containsKey("com.example.Foo"));
+    assertTrue(result.getComponentTypes().containsKey("com.example.Bar"));
+    assertAssetsWithPrefixRemoved("assets/external_comps/com.example.Foo/");
+    assertAssetsWithPrefixRemoved("assets/external_comps/com.example.Bar/");
+    assertAssetsOnServer("assets/external_comps/com.example/components.json");
+  }
+
   @Test
   public void testEmptyComponentDescriptors() {
     ComponentImportResponse result = importTestExtension("test.EmptyComponentDescriptor.aix");