Merge pull request #542 from brodybits/gh-540-hotfix

Resolve GH-539 & GH-540 on master
apache · Nov 12, 2018 · 576ad18 · 576ad18
2 parents e31596f + ef493b4
commit 576ad18
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 4 deletions.
diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js
@@ -26,7 +26,7 @@ var handlers = {
             if (!obj.src) throw new CordovaError(generateAttributeError('src', 'source-file', plugin.id));
             if (!obj.targetDir) throw new CordovaError(generateAttributeError('target-dir', 'source-file', plugin.id));
 
-            var dest = studioPathRemap(obj);
+            var dest = getInstallDestination(obj);
 
             if (options && options.force) {
                 copyFile(plugin.dir, obj.src, project.projectDir, dest, !!(options && options.link));
@@ -35,7 +35,7 @@ var handlers = {
             }
         },
         uninstall: function (obj, plugin, project, options) {
-            var dest = studioPathRemap(obj);
+            var dest = getInstallDestination(obj);
 
             // TODO: Add Koltin extension to uninstall, since they are handled like Java files
             if (obj.src.endsWith('java')) {
@@ -292,9 +292,15 @@ function generateAttributeError (attribute, element, id) {
     return 'Required attribute "' + attribute + '" not specified in <' + element + '> element from plugin: ' + id;
 }
 
+function getInstallDestination (obj) {
+    return studioPathRemap(obj) ||
+        path.join(obj.targetDir, path.basename(obj.src));
+}
+
 function studioPathRemap (obj) {
-    // If a Java file is using the new directory structure, don't penalize it
-    if (!obj.targetDir.includes('app/src/main')) {
+    // If any source file is using the app new directory structure,
+    // don't penalize it
+    if (!obj.targetDir.includes('app')) {
         if (obj.src.endsWith('.java')) {
             return path.join('app/src/main/java', obj.targetDir.substring(4), path.basename(obj.src));
         } else {

diff --git a/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml b/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml
@@ -70,6 +70,12 @@
 
         <source-file src="src/android/DummyPlugin.java"
                 target-dir="src/com/phonegap/plugins/dummyplugin" />
+        <source-file src="src/android/DummyPlugin2.java"
+                target-dir="app/src/main/src/com/phonegap/plugins/dummyplugin" />
+        <source-file src="src/android/TestLib.jar"
+                target-dir="app/libs" />
+        <source-file src="src/android/TestAar.aar"
+                target-dir="app/libs" />
         <lib-file src="src/android/TestLib.jar" />
     </platform>
 </plugin>
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/src/android/DummyPlugin2.java b/spec/fixtures/org.test.plugins.dummyplugin/src/android/DummyPlugin2.java
@@ -0,0 +1 @@
+./org.test.plugins.dummyplugin/src/android/DummyPlugin2.java
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/src/android/TestAar.aar b/spec/fixtures/org.test.plugins.dummyplugin/src/android/TestAar.aar
@@ -0,0 +1 @@
+./org.test.plugins.dummyplugin/src/android/TestAar.aar
diff --git a/spec/unit/pluginHandlers/handlers.spec.js b/spec/unit/pluginHandlers/handlers.spec.js
@@ -102,6 +102,25 @@ describe('android project handler', function () {
                     android['source-file'].install(valid_source[0], dummyPluginInfo, dummyProject);
                 }).toThrow(new Error('"' + target + '" already exists!'));
             });
+
+            it('Test#007 : should allow installing sources using proper path', function () {
+                android['source-file'].install(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy)
+                    .toHaveBeenCalledWith(dummyplugin, 'src/android/DummyPlugin2.java', temp, path.join('app/src/main/src/com/phonegap/plugins/dummyplugin/DummyPlugin2.java'), false);
+            });
+
+            // TODO: renumber these tests and other tests below
+            it('Test#007a : should allow installing lib file from sources using proper path', function () {
+                android['source-file'].install(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy)
+                    .toHaveBeenCalledWith(dummyplugin, 'src/android/TestLib.jar', temp, path.join('app/libs/TestLib.jar'), false);
+            });
+
+            it('Test#007b : should allow installing aar file from sources using proper path', function () {
+                android['source-file'].install(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy)
+                    .toHaveBeenCalledWith(dummyplugin, 'src/android/TestAar.aar', temp, path.join('app/libs/TestAar.aar'), false);
+            });
         });
 
         describe('of <framework> elements', function () {
@@ -251,6 +270,10 @@ describe('android project handler', function () {
             });
         });
 
+        // TODO:
+        // - merge tests of <source-file> elements into single describe block
+        //   (with proper beforeEach/afterEach)
+        // - renumber the tests after Test#019
         describe('of <source-file> elements', function () {
             it('Test#019 : should remove stuff by calling common.deleteJava for Android Studio projects', function () {
                 android['source-file'].install(valid_source[0], dummyPluginInfo, dummyProject);
@@ -259,6 +282,30 @@ describe('android project handler', function () {
             });
         });
 
+        describe('of <source-file> element, with specific app target-dir', function () {
+            it('Test#019a : should remove stuff by calling common.deleteJava for Android Studio projects, with specific app target-dir', function () {
+                android['source-file'].install(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(deleteJavaSpy).toHaveBeenCalledWith(temp, path.join('app/src/main/src/com/phonegap/plugins/dummyplugin/DummyPlugin2.java'));
+            });
+        });
+
+        describe('of <source-file> element, with JAR in specific app target-dir', function () {
+            it('Test#019b : should remove stuff by calling common.deleteJava for Android Studio projects, with JAR into specific app target-dir', function () {
+                android['source-file'].install(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/libs/TestLib.jar'));
+            });
+        });
+
+        describe('of <source-file> element, with AAR in specific app target-dir', function () {
+            it('Test#019c : should remove stuff by calling common.deleteJava for Android Studio projects, with JAR into specific app target-dir', function () {
+                android['source-file'].install(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/libs/TestAar.aar'));
+            });
+        });
+
         describe('of <framework> elements', function () {
 
             var someString = jasmine.any(String);