From fe9b7e866ea5626f58985a4e745fffef7de1beac Mon Sep 17 00:00:00 2001 From: Jule Marcoueille Date: Fri, 23 Nov 2018 15:23:50 +0100 Subject: [PATCH 1/8] Improve target-dir restriction for detecting new android project structure used in plugin.xml. (#575) --- bin/templates/cordova/lib/pluginHandlers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js index 25890ba235..0021e0fdd6 100644 --- a/bin/templates/cordova/lib/pluginHandlers.js +++ b/bin/templates/cordova/lib/pluginHandlers.js @@ -295,7 +295,7 @@ function generateAttributeError (attribute, element, id) { function getInstallDestination (obj) { var APP_MAIN_PREFIX = 'app/src/main'; - if (obj.targetDir.startsWith('app')) { + if (obj.targetDir.startsWith('app/')) { // If any source file is using the new app directory structure, // don't penalize it return path.join(obj.targetDir, path.basename(obj.src)); From fd19d6c21d94f574458b37fec012d89718cb2385 Mon Sep 17 00:00:00 2001 From: Jule Marcoueille Date: Mon, 26 Nov 2018 19:56:36 +0100 Subject: [PATCH 2/8] Clarify old source-file declaration way from the new one and improve ambiguous code. --- bin/templates/cordova/lib/pluginHandlers.js | 33 +++++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js index 0021e0fdd6..318eab2fe8 100644 --- a/bin/templates/cordova/lib/pluginHandlers.js +++ b/bin/templates/cordova/lib/pluginHandlers.js @@ -295,26 +295,33 @@ function generateAttributeError (attribute, element, id) { function getInstallDestination (obj) { var APP_MAIN_PREFIX = 'app/src/main'; - if (obj.targetDir.startsWith('app/')) { + var targetDirArray = obj.targetDir.split('/'); + + if (targetDirArray[0] === 'app') { // If any source file is using the new app directory structure, // don't penalize it return path.join(obj.targetDir, path.basename(obj.src)); - } else if (obj.src.endsWith('.java')) { - return path.join(APP_MAIN_PREFIX, 'java', obj.targetDir.substring(4), path.basename(obj.src)); - } else if (obj.src.endsWith('.aidl')) { - return path.join(APP_MAIN_PREFIX, 'aidl', obj.targetDir.substring(4), path.basename(obj.src)); - } else if (obj.targetDir.includes('libs')) { - if (obj.src.endsWith('.so')) { - return path.join(APP_MAIN_PREFIX, 'jniLibs', obj.targetDir.substring(5), path.basename(obj.src)); - } else { + } else { + // plugin ignores the new app directory structure (DEPRECATED) + if (obj.src.endsWith('.java')) { + return path.join(APP_MAIN_PREFIX, 'java', obj.targetDir.replace(/^src\/?/, ''), + path.basename(obj.src)); + } else if (obj.src.endsWith('.aidl')) { + return path.join(APP_MAIN_PREFIX, 'aidl', obj.targetDir.replace(/^src\/?/, ''), + path.basename(obj.src)); + } else if (targetDirArray[0] === 'libs') { + if (obj.src.endsWith('.so')) { + return path.join(APP_MAIN_PREFIX, 'jniLibs', obj.targetDir.replace(/^libs\/?/, ''), + path.basename(obj.src)); + } else { + return path.join('app', obj.targetDir, path.basename(obj.src)); + } + } else if (obj.targetDir.startsWith('src/main/')) { return path.join('app', obj.targetDir, path.basename(obj.src)); } - } else if (obj.targetDir.includes('src/main')) { - return path.join('app', obj.targetDir, path.basename(obj.src)); - } else { + // For all other source files not using the new app directory structure, // add 'app/src/main' to the targetDir return path.join(APP_MAIN_PREFIX, obj.targetDir, path.basename(obj.src)); } - } From f109d06164918596c20a85ddf00636e7e0dd8763 Mon Sep 17 00:00:00 2001 From: Jule Marcoueille Date: Mon, 26 Nov 2018 20:19:17 +0100 Subject: [PATCH 3/8] Better check `src/main` forms. --- bin/templates/cordova/lib/pluginHandlers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js index 318eab2fe8..0fd34873ec 100644 --- a/bin/templates/cordova/lib/pluginHandlers.js +++ b/bin/templates/cordova/lib/pluginHandlers.js @@ -316,7 +316,7 @@ function getInstallDestination (obj) { } else { return path.join('app', obj.targetDir, path.basename(obj.src)); } - } else if (obj.targetDir.startsWith('src/main/')) { + } else if (targetDirArray[0] === 'src' && targetDirArray[1] === 'main') { return path.join('app', obj.targetDir, path.basename(obj.src)); } From 53caadeae5c01471cba9f4bdf7d3e6d68b080970 Mon Sep 17 00:00:00 2001 From: Jule Marcoueille Date: Tue, 27 Nov 2018 03:20:39 +0100 Subject: [PATCH 4/8] Replace path search by RegExps. --- bin/templates/cordova/lib/pluginHandlers.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js index 0fd34873ec..fc90fb571b 100644 --- a/bin/templates/cordova/lib/pluginHandlers.js +++ b/bin/templates/cordova/lib/pluginHandlers.js @@ -293,30 +293,34 @@ function generateAttributeError (attribute, element, id) { } function getInstallDestination (obj) { - var APP_MAIN_PREFIX = 'app/src/main'; + const APP_MAIN_PREFIX = 'app/src/main'; + const PATH_SEPARATOR = '/'; - var targetDirArray = obj.targetDir.split('/'); + const appReg = new RegExp(`^app\\${PATH_SEPARATOR}?`); + const libsReg = new RegExp(`^libs\\${PATH_SEPARATOR}?`); + const srcReg = new RegExp(`^src\\${PATH_SEPARATOR}?`); + const srcMainReg = new RegExp(`^src\\${PATH_SEPARATOR}main\\${PATH_SEPARATOR}?`); - if (targetDirArray[0] === 'app') { + if (appReg.test(obj.targetDir)) { // If any source file is using the new app directory structure, // don't penalize it return path.join(obj.targetDir, path.basename(obj.src)); } else { // plugin ignores the new app directory structure (DEPRECATED) if (obj.src.endsWith('.java')) { - return path.join(APP_MAIN_PREFIX, 'java', obj.targetDir.replace(/^src\/?/, ''), + return path.join(APP_MAIN_PREFIX, 'java', obj.targetDir.replace(srcReg, ''), path.basename(obj.src)); } else if (obj.src.endsWith('.aidl')) { - return path.join(APP_MAIN_PREFIX, 'aidl', obj.targetDir.replace(/^src\/?/, ''), + return path.join(APP_MAIN_PREFIX, 'aidl', obj.targetDir.replace(srcReg, ''), path.basename(obj.src)); - } else if (targetDirArray[0] === 'libs') { + } else if (libsReg.test(obj.targetDir)) { if (obj.src.endsWith('.so')) { - return path.join(APP_MAIN_PREFIX, 'jniLibs', obj.targetDir.replace(/^libs\/?/, ''), + return path.join(APP_MAIN_PREFIX, 'jniLibs', obj.targetDir.replace(libsReg, ''), path.basename(obj.src)); } else { return path.join('app', obj.targetDir, path.basename(obj.src)); } - } else if (targetDirArray[0] === 'src' && targetDirArray[1] === 'main') { + } else if (srcMainReg.test(obj.targetDir)) { return path.join('app', obj.targetDir, path.basename(obj.src)); } From 28dd7929b85c8a3dbd7b97da5c22aa79151a3e9b Mon Sep 17 00:00:00 2001 From: Jule Marcoueille Date: Tue, 27 Nov 2018 04:03:48 +0100 Subject: [PATCH 5/8] Fix RegExp in order to match `/` or `EOL`. --- bin/templates/cordova/lib/pluginHandlers.js | 8 ++++---- spec/fixtures/org.test.plugins.dummyplugin/plugin.xml | 2 ++ spec/unit/pluginHandlers/handlers.spec.js | 6 ++++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js index fc90fb571b..fca2090291 100644 --- a/bin/templates/cordova/lib/pluginHandlers.js +++ b/bin/templates/cordova/lib/pluginHandlers.js @@ -296,10 +296,10 @@ function getInstallDestination (obj) { const APP_MAIN_PREFIX = 'app/src/main'; const PATH_SEPARATOR = '/'; - const appReg = new RegExp(`^app\\${PATH_SEPARATOR}?`); - const libsReg = new RegExp(`^libs\\${PATH_SEPARATOR}?`); - const srcReg = new RegExp(`^src\\${PATH_SEPARATOR}?`); - const srcMainReg = new RegExp(`^src\\${PATH_SEPARATOR}main\\${PATH_SEPARATOR}?`); + var appReg = new RegExp(`^app(\\${PATH_SEPARATOR}|$)`); + var libsReg = new RegExp(`^libs(\\${PATH_SEPARATOR}|$)`); + var srcReg = new RegExp(`^src(\\${PATH_SEPARATOR}|$)`); + var srcMainReg = new RegExp(`^src\\${PATH_SEPARATOR}main(\\${PATH_SEPARATOR}|$)`); if (appReg.test(obj.targetDir)) { // If any source file is using the new app directory structure, diff --git a/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml b/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml index 9bc99d21dd..f7a82ba563 100644 --- a/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml +++ b/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml @@ -84,6 +84,8 @@ + diff --git a/spec/unit/pluginHandlers/handlers.spec.js b/spec/unit/pluginHandlers/handlers.spec.js index 31e7b453b9..a44b8cf0ec 100644 --- a/spec/unit/pluginHandlers/handlers.spec.js +++ b/spec/unit/pluginHandlers/handlers.spec.js @@ -169,6 +169,12 @@ describe('android project handler', function () { expect(copyFileSpy) .toHaveBeenCalledWith(dummyplugin, 'src/android/DummyPlugin2.java', temp, path.join('app/src/main/java/com/appco/DummyPlugin2.java'), false); }); + + it('Test#006k : should allow installing sources with target-dir that includes "app" in its first directory', function () { + android['source-file'].install(valid_source[11], dummyPluginInfo, dummyProject, {android_studio: true}); + expect(copyFileSpy) + .toHaveBeenCalledWith(dummyplugin, 'src/android/DummyPlugin2.java', temp, path.join('app/src/main/java/appco/src/DummyPlugin2.java'), false); + }); }); describe('of elements', function () { From 06da64b815ca38e78cbeb11760bced3d52dd2c57 Mon Sep 17 00:00:00 2001 From: Jule Marcoueille Date: Tue, 27 Nov 2018 04:09:53 +0100 Subject: [PATCH 6/8] Remove template strings for NodeJS 4 support. --- bin/templates/cordova/lib/pluginHandlers.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js index fca2090291..944291b839 100644 --- a/bin/templates/cordova/lib/pluginHandlers.js +++ b/bin/templates/cordova/lib/pluginHandlers.js @@ -293,13 +293,13 @@ function generateAttributeError (attribute, element, id) { } function getInstallDestination (obj) { - const APP_MAIN_PREFIX = 'app/src/main'; - const PATH_SEPARATOR = '/'; + var APP_MAIN_PREFIX = 'app/src/main'; + var PATH_SEPARATOR = '/'; - var appReg = new RegExp(`^app(\\${PATH_SEPARATOR}|$)`); - var libsReg = new RegExp(`^libs(\\${PATH_SEPARATOR}|$)`); - var srcReg = new RegExp(`^src(\\${PATH_SEPARATOR}|$)`); - var srcMainReg = new RegExp(`^src\\${PATH_SEPARATOR}main(\\${PATH_SEPARATOR}|$)`); + var appReg = new RegExp('^app(\\' + PATH_SEPARATOR + '|$)'); + var libsReg = new RegExp('^libs(\\' + PATH_SEPARATOR + '|$)'); + var srcReg = new RegExp('^src(\\' + PATH_SEPARATOR + '|$)'); + var srcMainReg = new RegExp('^src\\' + PATH_SEPARATOR + 'main(\\' + PATH_SEPARATOR + '|$)'); if (appReg.test(obj.targetDir)) { // If any source file is using the new app directory structure, From 947f850ddf00fb8b453df38f83108bd3fd89e720 Mon Sep 17 00:00:00 2001 From: Jule Marcoueille Date: Tue, 27 Nov 2018 04:16:53 +0100 Subject: [PATCH 7/8] Add pointer on deprecation plan. --- bin/templates/cordova/lib/pluginHandlers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js index 944291b839..586a4b016e 100644 --- a/bin/templates/cordova/lib/pluginHandlers.js +++ b/bin/templates/cordova/lib/pluginHandlers.js @@ -306,7 +306,7 @@ function getInstallDestination (obj) { // don't penalize it return path.join(obj.targetDir, path.basename(obj.src)); } else { - // plugin ignores the new app directory structure (DEPRECATED) + // Plugin using deprecated target directory structure (GH-580) if (obj.src.endsWith('.java')) { return path.join(APP_MAIN_PREFIX, 'java', obj.targetDir.replace(srcReg, ''), path.basename(obj.src)); From 40417634b4223ed4c99e9498a5a0841255a9f179 Mon Sep 17 00:00:00 2001 From: Jule Marcoueille Date: Wed, 28 Nov 2018 10:27:46 +0100 Subject: [PATCH 8/8] Make RegExp more readable. --- bin/templates/cordova/lib/pluginHandlers.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js index 586a4b016e..6d1a7336e3 100644 --- a/bin/templates/cordova/lib/pluginHandlers.js +++ b/bin/templates/cordova/lib/pluginHandlers.js @@ -296,10 +296,13 @@ function getInstallDestination (obj) { var APP_MAIN_PREFIX = 'app/src/main'; var PATH_SEPARATOR = '/'; - var appReg = new RegExp('^app(\\' + PATH_SEPARATOR + '|$)'); - var libsReg = new RegExp('^libs(\\' + PATH_SEPARATOR + '|$)'); - var srcReg = new RegExp('^src(\\' + PATH_SEPARATOR + '|$)'); - var srcMainReg = new RegExp('^src\\' + PATH_SEPARATOR + 'main(\\' + PATH_SEPARATOR + '|$)'); + var PATH_SEP_MATCH = '\\' + PATH_SEPARATOR; + var PATH_SEP_OR_EOL_MATCH = '(\\' + PATH_SEPARATOR + '|$)'; + + var appReg = new RegExp('^app' + PATH_SEP_OR_EOL_MATCH); + var libsReg = new RegExp('^libs' + PATH_SEP_OR_EOL_MATCH); + var srcReg = new RegExp('^src' + PATH_SEP_OR_EOL_MATCH); + var srcMainReg = new RegExp('^src' + PATH_SEP_MATCH + 'main' + PATH_SEP_OR_EOL_MATCH); if (appReg.test(obj.targetDir)) { // If any source file is using the new app directory structure,