From 772945e01bc309db8f13422f219307c9e9a9e643 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Fri, 20 Oct 2017 17:55:44 +1100 Subject: [PATCH 01/14] change scope behaviour to consider scopes to be part of the package name --- .../cordova/plugin/plugin_spec_parser.spec.js | 38 +++++++++---------- spec/cordova/plugin/save.spec.js | 4 +- src/cordova/plugin/add.js | 10 +---- src/cordova/plugin/plugin_spec_parser.js | 10 ++--- src/cordova/plugin/save.js | 9 ----- src/plugman/registry/registry.js | 3 +- 6 files changed, 26 insertions(+), 48 deletions(-) diff --git a/spec/cordova/plugin/plugin_spec_parser.spec.js b/spec/cordova/plugin/plugin_spec_parser.spec.js index d55bbacb9..4a8ad53be 100644 --- a/spec/cordova/plugin/plugin_spec_parser.spec.js +++ b/spec/cordova/plugin/plugin_spec_parser.spec.js @@ -21,39 +21,37 @@ var pluginSpec = require('../../../src/cordova/plugin/plugin_spec_parser'); describe('methods for parsing npm plugin packages', function () { - function checkPluginSpecParsing (testString, scope, id, version) { + function checkPluginSpecParsing (testString, id, version) { var parsedSpec = pluginSpec.parse(testString); - expect(parsedSpec.scope).toEqual(scope); expect(parsedSpec.id).toEqual(id || testString); expect(parsedSpec.version).toEqual(version); - expect(parsedSpec.package).toEqual(scope ? scope + id : id); } it('Test 001 : should handle package names with no scope or version', function () { - checkPluginSpecParsing('test-plugin', null, 'test-plugin', null); + checkPluginSpecParsing('test-plugin', 'test-plugin', null); }); it('Test 002 : should handle package names with a version', function () { - checkPluginSpecParsing('test-plugin@1.0.0', null, 'test-plugin', '1.0.0'); - checkPluginSpecParsing('test-plugin@latest', null, 'test-plugin', 'latest'); + checkPluginSpecParsing('test-plugin@1.0.0', 'test-plugin', '1.0.0'); + checkPluginSpecParsing('test-plugin@latest', 'test-plugin', 'latest'); }); it('Test 003 : should handle package names with a scope', function () { - checkPluginSpecParsing('@test/test-plugin', '@test/', 'test-plugin', null); + checkPluginSpecParsing('@test/test-plugin', '@test/test-plugin', null); }); it('Test 004 : should handle package names with a scope and a version', function () { - checkPluginSpecParsing('@test/test-plugin@1.0.0', '@test/', 'test-plugin', '1.0.0'); - checkPluginSpecParsing('@test/test-plugin@latest', '@test/', 'test-plugin', 'latest'); + checkPluginSpecParsing('@test/test-plugin@1.0.0', '@test/test-plugin', '1.0.0'); + checkPluginSpecParsing('@test/test-plugin@latest', '@test/test-plugin', 'latest'); }); it('Test 005 : should handle invalid package specs', function () { - checkPluginSpecParsing('@nonsense', null, null, null); - checkPluginSpecParsing('@/nonsense', null, null, null); - checkPluginSpecParsing('@', null, null, null); - checkPluginSpecParsing('@nonsense@latest', null, null, null); - checkPluginSpecParsing('@/@', null, null, null); - checkPluginSpecParsing('/', null, null, null); - checkPluginSpecParsing('../../@directory', null, null, null); - checkPluginSpecParsing('@directory/../@directory', null, null, null); - checkPluginSpecParsing('./directory', null, null, null); - checkPluginSpecParsing('directory/directory', null, null, null); - checkPluginSpecParsing('http://cordova.apache.org', null, null, null); + checkPluginSpecParsing('@nonsense', null, null); + checkPluginSpecParsing('@/nonsense', null, null); + checkPluginSpecParsing('@', null, null); + checkPluginSpecParsing('@nonsense@latest', null, null); + checkPluginSpecParsing('@/@', null, null); + checkPluginSpecParsing('/', null, null); + checkPluginSpecParsing('../../@directory', null, null); + checkPluginSpecParsing('@directory/../@directory', null, null); + checkPluginSpecParsing('./directory', null, null); + checkPluginSpecParsing('directory/directory', null, null); + checkPluginSpecParsing('http://cordova.apache.org', null, null); }); }); diff --git a/spec/cordova/plugin/save.spec.js b/spec/cordova/plugin/save.spec.js index d51cf1149..82acbcb9a 100644 --- a/spec/cordova/plugin/save.spec.js +++ b/spec/cordova/plugin/save.spec.js @@ -150,9 +150,9 @@ describe('cordova/plugin/save', function () { expect(save.getSpec({id: 'cordova-plugin-camera@^1.1.0'}, '/some/path', 'cordova-plugin-camera')).toEqual('^1.1.0'); }); - it('should return a version that includes scope if scope was part of plugin id', function () { + it('should return a version if the package name includes a scope', function () { save.versionString.and.callThrough(); - expect(save.getSpec({ type: 'registry', id: '@scoped/package@^1.0.0' }, '/some/path', 'cordova-plugin-camera')).toEqual('@scoped/package@^1.0.0'); + expect(save.getSpec({ type: 'registry', id: '@scoped/package@^1.0.0' }, '/some/path', 'cordova-plugin-camera')).toEqual('^1.0.0'); }); it('should fall back to using PluginInfoProvider to retrieve a version as last resort', function () { diff --git a/src/cordova/plugin/add.js b/src/cordova/plugin/add.js index 3b9b78627..ca17843e7 100644 --- a/src/cordova/plugin/add.js +++ b/src/cordova/plugin/add.js @@ -171,16 +171,10 @@ function add (projectRoot, hooksRunner, opts) { attributes.spec = src; } else { var ver = '~' + pluginInfo.version; - // Scoped packages need to have the package-spec along with the version - var parsedSpec = pluginSpec.parse(target); if (pkgJson && pkgJson.dependencies && pkgJson.dependencies[pluginInfo.id]) { attributes.spec = pkgJson.dependencies[pluginInfo.id]; } else { - if (parsedSpec.scope) { - attributes.spec = parsedSpec.package + '@' + ver; - } else { - attributes.spec = ver; - } + attributes.spec = ver; } } xml = cordova_util.projectConfig(projectRoot); @@ -263,7 +257,7 @@ function determinePluginTarget (projectRoot, cfg, target, fetchOptions) { } } */ - if (cordova_util.isUrl(parsedSpec.version) || cordova_util.isDirectory(parsedSpec.version) || pluginSpec.parse(parsedSpec.version).scope) { + if (cordova_util.isUrl(parsedSpec.version) || cordova_util.isDirectory(parsedSpec.version)) { return Q(parsedSpec.version); } diff --git a/src/cordova/plugin/plugin_spec_parser.js b/src/cordova/plugin/plugin_spec_parser.js index bfbe332ed..31fa179ec 100644 --- a/src/cordova/plugin/plugin_spec_parser.js +++ b/src/cordova/plugin/plugin_spec_parser.js @@ -26,14 +26,10 @@ module.exports.parse = parse; * Represents a parsed specification for a plugin * @class * @param {String} raw The raw specification (i.e. provided by the user) - * @param {String} scope The scope of the package if this is an npm package * @param {String} id The id of the package if this is an npm package * @param {String} version The version specified for the package if this is an npm package */ -function PluginSpec (raw, scope, id, version) { - /** @member {String|null} The npm scope of the plugin spec or null if it does not have one */ - this.scope = scope || null; - +function PluginSpec (raw, id, version) { /** @member {String|null} The id of the plugin or the raw plugin spec if it is not an npm package */ this.id = id || raw; @@ -41,7 +37,7 @@ function PluginSpec (raw, scope, id, version) { this.version = version || null; /** @member {String|null} The npm package of the plugin (with scope) or null if this is not a spec for an npm package */ - this.package = (scope ? scope + id : id) || null; + this.package = id; } /** @@ -54,7 +50,7 @@ function PluginSpec (raw, scope, id, version) { function parse (raw) { var split = NPM_SPEC_REGEX.exec(raw); if (split) { - return new PluginSpec(raw, split[1], split[2], split[3]); + return new PluginSpec(raw, (split[1] || '') + split[2], split[3]); } return new PluginSpec(raw); diff --git a/src/cordova/plugin/save.js b/src/cordova/plugin/save.js index d24ffb5f9..c46b62714 100644 --- a/src/cordova/plugin/save.js +++ b/src/cordova/plugin/save.js @@ -81,7 +81,6 @@ function getSpec (pluginSource, projectRoot, pluginName) { } var version = null; - var scopedPackage = null; if (pluginSource.hasOwnProperty('id')) { // Note that currently version is only saved here if it was explicitly specified when the plugin was added. var parsedSpec = pluginSpec.parse(pluginSource.id); @@ -89,10 +88,6 @@ function getSpec (pluginSource, projectRoot, pluginName) { if (version) { version = module.exports.versionString(version); } - - if (parsedSpec.scope) { - scopedPackage = parsedSpec.package; - } } if (!version) { @@ -110,10 +105,6 @@ function getSpec (pluginSource, projectRoot, pluginName) { } } - if (scopedPackage) { - version = scopedPackage + '@' + version; - } - return version; } diff --git a/src/plugman/registry/registry.js b/src/plugman/registry/registry.js index 723c0546d..4ed0a1640 100644 --- a/src/plugman/registry/registry.js +++ b/src/plugman/registry/registry.js @@ -118,6 +118,5 @@ function initThenLoadSettingsWithRestore (promises) { function fetchPlugin (plugin) { events.emit('log', 'Fetching plugin "' + plugin + '" via npm'); var parsedSpec = pluginSpec.parse(plugin); - var scope = parsedSpec.scope || ''; - return npmhelper.fetchPackage(scope + parsedSpec.id, parsedSpec.version); + return npmhelper.fetchPackage(parsedSpec.id, parsedSpec.version); } From 456b73cb7b600866b6a596818fc103845af82f55 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Fri, 20 Oct 2017 18:07:56 +1100 Subject: [PATCH 02/14] remove extraneous done callback from scope plugin tests --- integration-tests/plugin.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/plugin.spec.js b/integration-tests/plugin.spec.js index 815d4182b..d8a613220 100644 --- a/integration-tests/plugin.spec.js +++ b/integration-tests/plugin.spec.js @@ -263,7 +263,7 @@ describe('plugin end-to-end', function () { mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); spyOn(registry, 'info').and.returnValue(Q({})); - addPlugin(scopedPackage, npmInfoTestPlugin, {}, done) + addPlugin(scopedPackage, npmInfoTestPlugin, {}) .then(function () { // Check to make sure that we are at least trying to get the correct package. // This package is not published to npm, so we can't truly do end-to-end tests @@ -284,7 +284,7 @@ describe('plugin end-to-end', function () { mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); spyOn(registry, 'info'); - addPlugin(scopedPackage, npmInfoTestPlugin, {}, done) + addPlugin(scopedPackage, npmInfoTestPlugin, {}) .then(function () { expect(registry.info).not.toHaveBeenCalled(); From 987aaa5b5d098cc433b0c32c9aa42226272b71b4 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Fri, 20 Oct 2017 19:40:08 +1100 Subject: [PATCH 03/14] add scoped plugin testcase --- integration-tests/plugin.spec.js | 3 + .../cordova-lib-test-plugin-scoped/LICENSE | 202 ++++++++++++++++++ .../cordova-lib-test-plugin-scoped/NOTICE | 5 + .../package.json | 28 +++ .../cordova-lib-test-plugin-scoped/plugin.xml | 4 + 5 files changed, 242 insertions(+) create mode 100644 spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/LICENSE create mode 100644 spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/NOTICE create mode 100644 spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/package.json create mode 100644 spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/plugin.xml diff --git a/integration-tests/plugin.spec.js b/integration-tests/plugin.spec.js index d8a613220..cb39ee4c0 100644 --- a/integration-tests/plugin.spec.js +++ b/integration-tests/plugin.spec.js @@ -46,6 +46,9 @@ var org_test_defaultvariables = 'org.test.defaultvariables'; var npmInfoTestPlugin = 'cordova-lib-test-plugin'; var npmInfoTestPluginVersion = '1.1.2'; +var npmScopedTestPlugin = '@testscope/cordova-lib-test-plugin-scoped'; +var npmScopedTestPluginDir = 'cordova-lib-test-plugin-scoped'; + var testGitPluginRepository = 'https://github.com/apache/cordova-plugin-device.git'; var testGitPluginId = 'cordova-plugin-device'; diff --git a/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/LICENSE b/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/LICENSE new file mode 100644 index 000000000..9b5e4019d --- /dev/null +++ b/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/LICENSE @@ -0,0 +1,202 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + 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. \ No newline at end of file diff --git a/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/NOTICE b/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/NOTICE new file mode 100644 index 000000000..27bc4a085 --- /dev/null +++ b/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/NOTICE @@ -0,0 +1,5 @@ +Apache Cordova +Copyright 2012 The Apache Software Foundation + +This product includes software developed at +The Apache Software Foundation (http://www.apache.org/). diff --git a/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/package.json b/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/package.json new file mode 100644 index 000000000..154dde04a --- /dev/null +++ b/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/package.json @@ -0,0 +1,28 @@ +{ + "name": "@testscope/cordova-lib-test-plugin-scoped", + "version": "1.3.1", + "description": "Empty plugin used as part of the tests in cordova-lib", + "cordova": { + "id": "@testscope/cordova-lib-test-plugin-scoped", + "platforms": [] + }, + "repository": { + "type": "git", + "url": "git://git-wip-us.apache.org/repos/asf/cordova-lib.git" + }, + "author": "Apache Software Foundation", + "license": "Apache-2.0", + "engines": { + "cordovaDependencies": { + "0.0.0": { + "cordova-android": "<2.1.0" + }, + "1.1.2": { + "cordova-android": ">=2.1.0 <7.0.0" + }, + "1.3.0": { + "cordova-android": "7.0.0" + } + } + } +} diff --git a/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/plugin.xml b/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/plugin.xml new file mode 100644 index 000000000..e4b47f4ad --- /dev/null +++ b/spec/cordova/fixtures/plugins/cordova-lib-test-plugin-scoped/plugin.xml @@ -0,0 +1,4 @@ + + + @testscope/cordova-lib-test-plugin-scoped + From a195d554705ea9f6ce8a2a0c2cb839f9723f195f Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Fri, 20 Oct 2017 19:41:46 +1100 Subject: [PATCH 04/14] allow scoped plugins to exist in a dir structure reflecting there name, c.f. npm --- integration-tests/plugin.spec.js | 18 ++++++++---------- src/cordova/plugin/add.js | 2 +- src/cordova/util.js | 19 ++++++++++++++----- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/integration-tests/plugin.spec.js b/integration-tests/plugin.spec.js index cb39ee4c0..527ce58c3 100644 --- a/integration-tests/plugin.spec.js +++ b/integration-tests/plugin.spec.js @@ -98,8 +98,7 @@ function mockPluginFetch (id, dir) { spyOn(plugman, 'fetch').and.callFake(function (target, pluginPath, fetchOptions) { var dest = path.join(project, 'plugins', id); var src = path.join(dir, 'plugin.xml'); - - shell.mkdir(dest); + shell.mkdir('-p', dest); shell.cp(src, dest); return Q(dest); }); @@ -262,19 +261,18 @@ describe('plugin end-to-end', function () { }, 30000); it('Test 011 : should handle scoped npm packages', function (done) { - var scopedPackage = '@testscope/' + npmInfoTestPlugin; - mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); + mockPluginFetch(npmScopedTestPlugin, path.join(pluginsDir, npmScopedTestPluginDir)); spyOn(registry, 'info').and.returnValue(Q({})); - addPlugin(scopedPackage, npmInfoTestPlugin, {}) + addPlugin(npmScopedTestPlugin, npmScopedTestPlugin, {}) .then(function () { // Check to make sure that we are at least trying to get the correct package. // This package is not published to npm, so we can't truly do end-to-end tests - expect(registry.info).toHaveBeenCalledWith([scopedPackage]); + expect(registry.info).toHaveBeenCalledWith([npmScopedTestPlugin]); var fetchTarget = plugman.fetch.calls.mostRecent().args[0]; - expect(fetchTarget).toEqual(scopedPackage); + expect(fetchTarget).toEqual(npmScopedTestPlugin); }) .fail(function (err) { expect(err).toBeUndefined(); @@ -283,11 +281,11 @@ describe('plugin end-to-end', function () { }, 30000); it('Test 012 : should handle scoped npm packages with given version tags', function (done) { - var scopedPackage = '@testscope/' + npmInfoTestPlugin + '@latest'; - mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); + var scopedPackage = npmScopedTestPlugin + '@latest'; + mockPluginFetch(npmScopedTestPlugin, path.join(pluginsDir, npmScopedTestPluginDir)); spyOn(registry, 'info'); - addPlugin(scopedPackage, npmInfoTestPlugin, {}) + addPlugin(scopedPackage, npmScopedTestPlugin, {}) .then(function () { expect(registry.info).not.toHaveBeenCalled(); diff --git a/src/cordova/plugin/add.js b/src/cordova/plugin/add.js index ca17843e7..2759a18d2 100644 --- a/src/cordova/plugin/add.js +++ b/src/cordova/plugin/add.js @@ -129,7 +129,7 @@ function add (projectRoot, hooksRunner, opts) { }; events.emit('verbose', 'Calling plugman.install on plugin "' + pluginInfo.dir + '" for platform "' + platform); - return plugman.install(platform, platformRoot, path.basename(pluginInfo.dir), pluginPath, options) + return plugman.install(platform, platformRoot, pluginInfo.id, pluginPath, options) .then(function (didPrepare) { // If platform does not returned anything we'll need // to trigger a prepare after all plugins installed diff --git a/src/cordova/util.js b/src/cordova/util.js index 972ca8994..ae9219269 100644 --- a/src/cordova/util.js +++ b/src/cordova/util.js @@ -255,11 +255,20 @@ function findPlugins (pluginDir) { var plugins = []; if (fs.existsSync(pluginDir)) { - plugins = fs.readdirSync(pluginDir).filter(function (fileName) { - var pluginPath = path.join(pluginDir, fileName); - var isPlugin = isDirectory(pluginPath) || isSymbolicLink(pluginPath); - return fileName !== '.svn' && fileName !== 'CVS' && isPlugin; - }); + plugins = fs.readdirSync(pluginDir) + .reduce(function (plugins, pluginOrScope) { + if (pluginOrScope[0] === '@') { + plugins.push(...fs.readdirSync(path.join(pluginDir, pluginOrScope)).map(s => path.join(pluginOrScope, s))); + } else { + plugins.push(pluginOrScope); + } + return plugins; + }, []) + .filter(function (fileName) { + var pluginPath = path.join(pluginDir, fileName); + var isPlugin = isDirectory(pluginPath) || isSymbolicLink(pluginPath); + return fileName !== '.svn' && fileName !== 'CVS' && isPlugin; + }); } return plugins; From d1a227a5aea283e0746b478ffb8f63acfa3db2d0 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Fri, 20 Oct 2017 19:42:07 +1100 Subject: [PATCH 05/14] add an integration test to check scoped plugin add+remove --- integration-tests/plugin.spec.js | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/integration-tests/plugin.spec.js b/integration-tests/plugin.spec.js index 527ce58c3..f03f2e9f1 100644 --- a/integration-tests/plugin.spec.js +++ b/integration-tests/plugin.spec.js @@ -297,4 +297,37 @@ describe('plugin end-to-end', function () { }) .fin(done); }, 30000); + + it('Test 013 : should be able to add and remove scoped npm packages without screwing up everything', function (done) { + mockPluginFetch(npmScopedTestPlugin, path.join(pluginsDir, npmScopedTestPluginDir)); + + spyOn(registry, 'info').and.returnValue(Q({})); + addPlugin(npmScopedTestPlugin, npmScopedTestPlugin, {}) + .then(function () { + expect(registry.info).toHaveBeenCalledWith([npmScopedTestPlugin]); + + var fetchTarget = plugman.fetch.calls.mostRecent().args[0]; + expect(fetchTarget).toEqual(npmScopedTestPlugin); + }) + .then(function () { + return cordova.plugin('ls'); + }) + .then(function (list) { + console.dir(list); + }) + .then(function () { + return removePlugin(npmScopedTestPlugin); + }) + .then(function () { + return cordova.plugin('ls'); + }) + .then(function (list) { + console.dir(list); + }) + .fail(function (err) { + console.error(err); + expect(err).toBeUndefined(); + }) + .fin(done); + }, 30000); }); From ff2d0369a8587356976364f27333b7481370043d Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Fri, 20 Oct 2017 19:43:14 +1100 Subject: [PATCH 06/14] TEMP lint with typescript --- package.json | 1 + tsconfig.json | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 tsconfig.json diff --git a/package.json b/package.json index 07879c3d1..20e108725 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "xcode": "^0.9.0" }, "devDependencies": { + "@types/node": "^8.0.46", "codecov": "^2.1.0", "eslint": "^4.2.0", "eslint-config-semistandard": "^11.0.0", diff --git a/tsconfig.json b/tsconfig.json new file mode 100644 index 000000000..65cf7307b --- /dev/null +++ b/tsconfig.json @@ -0,0 +1,55 @@ +{ + "compilerOptions": { + /* Basic Options */ + "target": "es5", /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', or 'ESNEXT'. */ + "module": "commonjs", /* Specify module code generation: 'none', commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */ + // "lib": [], /* Specify library files to be included in the compilation: */ + "allowJs": true, /* Allow javascript files to be compiled. */ + "checkJs": true, /* Report errors in .js files. */ + // "jsx": "preserve", /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */ + // "declaration": true, /* Generates corresponding '.d.ts' file. */ + // "sourceMap": true, /* Generates corresponding '.map' file. */ + // "outFile": "./", /* Concatenate and emit output to single file. */ + // "outDir": "./", /* Redirect output structure to the directory. */ + // "rootDir": "./", /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */ + // "removeComments": true, /* Do not emit comments to output. */ + // "noEmit": true, /* Do not emit outputs. */ + // "importHelpers": true, /* Import emit helpers from 'tslib'. */ + // "downlevelIteration": true, /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */ + // "isolatedModules": true, /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */ + + /* Strict Type-Checking Options */ + "strict": false /* Enable all strict type-checking options. */ + // "noImplicitAny": true, /* Raise error on expressions and declarations with an implied 'any' type. */ + // "strictNullChecks": true, /* Enable strict null checks. */ + // "noImplicitThis": true, /* Raise error on 'this' expressions with an implied 'any' type. */ + // "alwaysStrict": true, /* Parse in strict mode and emit "use strict" for each source file. */ + + /* Additional Checks */ + // "noUnusedLocals": true, /* Report errors on unused locals. */ + // "noUnusedParameters": true, /* Report errors on unused parameters. */ + // "noImplicitReturns": true, /* Report error when not all code paths in function return a value. */ + // "noFallthroughCasesInSwitch": true, /* Report errors for fallthrough cases in switch statement. */ + + /* Module Resolution Options */ + // "moduleResolution": "node", /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */ + // "baseUrl": "./", /* Base directory to resolve non-absolute module names. */ + // "paths": {}, /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */ + // "rootDirs": [], /* List of root folders whose combined content represents the structure of the project at runtime. */ + // "typeRoots": [], /* List of folders to include type definitions from. */ + // "types": [], /* Type declaration files to be included in compilation. */ + // "allowSyntheticDefaultImports": true, /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */ + // "preserveSymlinks": true, /* Do not resolve the real path of symlinks. */ + + /* Source Map Options */ + // "sourceRoot": "./", /* Specify the location where debugger should locate TypeScript files instead of source locations. */ + // "mapRoot": "./", /* Specify the location where debugger should locate map files instead of generated locations. */ + // "inlineSourceMap": true, /* Emit a single file with source maps instead of having a separate file. */ + // "inlineSources": true, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */ + + /* Experimental Options */ + // "experimentalDecorators": true, /* Enables experimental support for ES7 decorators. */ + // "emitDecoratorMetadata": true, /* Enables experimental support for emitting type metadata for decorators. */ + }, + "include": ["src/"] +} \ No newline at end of file From 6acff9309641c080dfd9227161384f027a013496 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Fri, 20 Oct 2017 19:42:27 +1100 Subject: [PATCH 07/14] TEMP override plugin discovery in cordova-common --- src/cordova/plugin/util.js | 43 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/cordova/plugin/util.js b/src/cordova/plugin/util.js index 6dfced9da..46c85f5b4 100644 --- a/src/cordova/plugin/util.js +++ b/src/cordova/plugin/util.js @@ -27,6 +27,49 @@ module.exports.saveToConfigXmlOn = saveToConfigXmlOn; module.exports.getInstalledPlugins = getInstalledPlugins; module.exports.mergeVariables = mergeVariables; +/** TODO JARRAD REMOVE THIS */ +var fs = require('fs'); +PluginInfoProvider.prototype.getAllWithinSearchPath = function (dirName) { + var absPath = path.resolve(dirName); + if (!this._getAllCache[absPath]) { + this._getAllCache[absPath] = getAllHelper(absPath, this); + } + return this._getAllCache[absPath]; +}; + +function getAllHelper (absPath, provider) { + if (!fs.existsSync(absPath)) { + return []; + } + // If dir itself is a plugin, return it in an array with one element. + if (fs.existsSync(path.join(absPath, 'plugin.xml'))) { + return [provider.get(absPath)]; + } + var subdirs = fs.readdirSync(absPath); + subdirs.forEach(function (subdir) { + // if subdir is an npm @scope/ + if (subdir[0] === '@') { + // add the real plugins inside it + var scopePluginDirs = fs.readdirSync(path.join(absPath, subdir)) + .map((scopedPluginDir) => path.join(subdir, scopedPluginDir)); + subdirs.push.apply(subdirs, scopePluginDirs); + } + }); + var plugins = []; + subdirs.forEach(function (subdir) { + var d = path.join(absPath, subdir); + if (fs.existsSync(path.join(d, 'plugin.xml'))) { + try { + plugins.push(provider.get(d)); + } catch (e) { + events.emit('warn', 'Error parsing ' + path.join(d, 'plugin.xml.\n' + e.stack)); + } + } + }); + return plugins; +} +/* END TODO JARRAD */ + function getInstalledPlugins (projectRoot) { var pluginsDir = path.join(projectRoot, 'plugins'); // TODO: This should list based off of platform.json, not directories within plugins/ From 026af9e5f8a9a409e5e6025a322c4d5b29d962a7 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Mon, 23 Oct 2017 17:23:14 +1100 Subject: [PATCH 08/14] fix get_fetch_metadata to not guess plugins_dir anymore --- src/cordova/platform/addHelper.js | 3 +-- src/plugman/install.js | 2 +- src/plugman/uninstall.js | 2 +- src/plugman/util/metadata.js | 5 ++--- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/cordova/platform/addHelper.js b/src/cordova/platform/addHelper.js index 1c0b39fc3..3c432cde2 100644 --- a/src/cordova/platform/addHelper.js +++ b/src/cordova/platform/addHelper.js @@ -354,10 +354,9 @@ function installPluginsForNewPlatform (platform, projectRoot, opts) { return plugins.reduce(function (soFar, plugin) { return soFar.then(function () { events.emit('verbose', 'Installing plugin "' + plugin + '" following successful platform add of ' + platform); - plugin = path.basename(plugin); // Get plugin variables from fetch.json if have any and pass them as cli_variables to plugman - var pluginMetadata = fetchMetadata.get_fetch_metadata(path.join(plugins_dir, plugin)); + var pluginMetadata = fetchMetadata.get_fetch_metadata(plugins_dir, plugin); var options = { searchpath: opts.searchpath, diff --git a/src/plugman/install.js b/src/plugman/install.js index 51c6baf74..d2fda25c2 100644 --- a/src/plugman/install.js +++ b/src/plugman/install.js @@ -427,7 +427,7 @@ function tryFetchDependency (dep, install, options) { if (dep.url === '.') { // Look up the parent plugin's fetch metadata and determine the correct URL. - var fetchdata = require('./util/metadata').get_fetch_metadata(install.top_plugin_dir); + var fetchdata = require('./util/metadata').get_fetch_metadata(install.plugins_dir, install.top_plugin_id); if (!fetchdata || !(fetchdata.source && fetchdata.source.type)) { relativePath = dep.subdir || dep.id; diff --git a/src/plugman/uninstall.js b/src/plugman/uninstall.js index 562636e49..58811e213 100644 --- a/src/plugman/uninstall.js +++ b/src/plugman/uninstall.js @@ -149,7 +149,7 @@ module.exports.uninstallPlugin = function (id, plugins_dir, options) { // Recursively remove plugins which were installed as dependents (that are not top-level) var toDelete = []; function findDependencies (pluginId) { - var depPluginDir = path.join(plugin_dir, '..', pluginId); + var depPluginDir = path.join(plugins_dir, pluginId); // Skip plugin check for dependencies if it does not exist (CB-7846). if (!fs.existsSync(depPluginDir)) { events.emit('verbose', 'Plugin "' + pluginId + '" does not exist (' + depPluginDir + ')'); diff --git a/src/plugman/util/metadata.js b/src/plugman/util/metadata.js index e306bfbd9..16a629711 100644 --- a/src/plugman/util/metadata.js +++ b/src/plugman/util/metadata.js @@ -34,9 +34,8 @@ function getJson (pluginsDir) { return cachedJson; } -exports.get_fetch_metadata = function (plugin_dir) { - var pluginsDir = path.dirname(plugin_dir); - var pluginId = path.basename(plugin_dir); +exports.get_fetch_metadata = function (pluginsDir, pluginId) { + var plugin_dir = path.join(pluginsDir, pluginId); var metadataJson = getJson(pluginsDir); if (metadataJson[pluginId]) { From a61c1824a54b2252c88a690892b02769ed503a7d Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Mon, 23 Oct 2017 17:53:59 +1100 Subject: [PATCH 09/14] remove top_plugins override --- src/plugman/install.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/plugman/install.js b/src/plugman/install.js index d2fda25c2..9d4a6e1c7 100644 --- a/src/plugman/install.js +++ b/src/plugman/install.js @@ -380,11 +380,7 @@ function runInstall (actions, platform, project_dir, plugin_dir, plugins_dir, op function installDependencies (install, dependencies, options) { events.emit('verbose', 'Dependencies detected, iterating through them...'); - var top_plugins = path.join(options.plugin_src_dir || install.top_plugin_dir, '..'); - - // Add directory of top-level plugin to search path options.searchpath = options.searchpath || []; - if (top_plugins !== install.plugins_dir && options.searchpath.indexOf(top_plugins) === -1) { options.searchpath.push(top_plugins); } // Search for dependency by Id is: // a) Look for {$top_plugins}/{$depId} directory From dcaedb846394b97bc993a6c7315bde39f840715e Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Tue, 24 Oct 2017 10:38:16 +1100 Subject: [PATCH 10/14] unit test scopes --- spec/cordova/platform/addHelper.spec.js | 28 ++++++++++++++++++++----- spec/cordova/plugin/add.spec.js | 15 +++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/spec/cordova/platform/addHelper.spec.js b/spec/cordova/platform/addHelper.spec.js index 72f017401..100c44e38 100644 --- a/spec/cordova/platform/addHelper.spec.js +++ b/spec/cordova/platform/addHelper.spec.js @@ -444,15 +444,33 @@ describe('cordova/platform/addHelper', function () { }).done(done); }); - it('should invoke plugman.install, giving correct platform, plugin and other arguments', function (done) { - spyOn(cordova_util, 'findPlugins').and.returnValue(['cordova-plugin-whitelist']); - fetch_metadata.get_fetch_metadata.and.returnValue({ }); + function testInstallPluginsForNewPlatform (pluginName, getFetchMetadataResponse, expectedInstallOptions, done) { + spyOn(cordova_util, 'findPlugins').and.returnValue([pluginName]); + fetch_metadata.get_fetch_metadata.and.returnValue(getFetchMetadataResponse); platform_addHelper.installPluginsForNewPlatform('browser', projectRoot, {save: true, fetch: true}).then(function () { - expect(plugman.install).toHaveBeenCalled(); - expect(events.emit).toHaveBeenCalledWith('verbose', 'Installing plugin "cordova-plugin-whitelist" following successful platform add of browser'); + expect(plugman.install).toHaveBeenCalledWith( + 'browser', + '/some/path/platforms/browser', + pluginName, + '/some/path/plugins', + jasmine.objectContaining(expectedInstallOptions) + ); + expect(events.emit).toHaveBeenCalledWith('verbose', `Installing plugin "${pluginName}" following successful platform add of browser`); }).fail(function (e) { fail('fail handler unexpectedly invoked'); }).done(done); + } + + it('should invoke plugman.install, giving correct platform, plugin and other arguments with a dependent plugin', function (done) { + testInstallPluginsForNewPlatform('cordova-plugin-whitelist', {}, {is_top_level: undefined, fetch: true, save: true}, done); + }); + + it('should invoke plugman.install, giving correct platform, plugin and other arguments with a dependent plugin', function (done) { + testInstallPluginsForNewPlatform('cordova-plugin-top-level', {is_top_level: true}, {is_top_level: true, fetch: true, save: true}, done); + }); + + it('should invoke plugman.install, giving correct platform, plugin and other arguments with a dependent plugin', function (done) { + testInstallPluginsForNewPlatform('@cordova/cordova-plugin-scoped', {is_top_level: true}, {is_top_level: true, fetch: true, save: true}, done); }); it('should include any plugin variables as options when invoking plugman install', function (done) { diff --git a/spec/cordova/plugin/add.spec.js b/spec/cordova/plugin/add.spec.js index 2059ad2a7..bd44400b7 100644 --- a/spec/cordova/plugin/add.spec.js +++ b/spec/cordova/plugin/add.spec.js @@ -458,6 +458,21 @@ describe('cordova/plugin/add', function () { console.log(e); }).done(done); }); + it('should retrieve installed plugins and installed platforms version with a scoped page, and feed that information into determinePluginVersionToFetch', function (done) { + plugin_util.getInstalledPlugins.and.returnValue([{'id': '@cordova/cordova-plugin-camera', 'version': '2.0.0'}]); + cordova_util.getInstalledPlatformsWithVersions.and.returnValue(Q({'android': '6.0.0'})); + pluginInfo.engines = {}; + pluginInfo.engines.cordovaDependencies = {'^1.0.0': {'cordova': '>7.0.0'}}; + add.getFetchVersion(projectRoot, pluginInfo, '7.0.0') + .then(function () { + expect(plugin_util.getInstalledPlugins).toHaveBeenCalledWith(projectRoot); + expect(cordova_util.getInstalledPlatformsWithVersions).toHaveBeenCalledWith(projectRoot); + expect(add.determinePluginVersionToFetch).toHaveBeenCalledWith(pluginInfo, {'@cordova/cordova-plugin-camera': '2.0.0'}, {'android': '6.0.0'}, '7.0.0'); + }).fail(function (e) { + fail('fail handler unexpectedly invoked'); + console.log(e); + }).done(done); + }); }); // TODO More work to be done here to replace plugin_fetch.spec.js describe('determinePluginVersionToFetch helper method', function () { From 707fd7397b89243aed182becb093420b1fd283e6 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Tue, 24 Oct 2017 15:36:53 +1100 Subject: [PATCH 11/14] add metadata unit tests --- spec/plugman/util/metadata.spec.js | 210 +++++++++++++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 spec/plugman/util/metadata.spec.js diff --git a/spec/plugman/util/metadata.spec.js b/spec/plugman/util/metadata.spec.js new file mode 100644 index 000000000..294260113 --- /dev/null +++ b/spec/plugman/util/metadata.spec.js @@ -0,0 +1,210 @@ + +var rewire = require('rewire'); +var metadata = rewire('../../../src/plugman/util/metadata'); + +var fileMocks; +var fsMock = { + readFileSync: function (path, encoding) { + if (fileMocks[path]) { + var buffer = Buffer.from(fileMocks[path]); + return (encoding) ? buffer.toString(encoding) : buffer; + } else { + throw new Error('fs mock: readFileSync: no such file.'); + } + }, + existsSync: function (path) { + return Boolean(fileMocks[path]); + }, + writeFileSync: function (path, data, encoding) { + fileMocks[path] = data.toString(encoding); + }, + unlinkSync: function (path) { + if (fileMocks[path]) { + delete fileMocks[path]; + } else { + throw new Error('fs mock: unlinkSync: no such file.'); + } + } +}; +metadata.__set__('fs', fsMock); + +function getFileMocksJson () { + var fileJson = {}; + for (var filename in fileMocks) { + fileJson[filename] = JSON.parse(fileMocks[filename]); + } + return fileJson; +} + +describe('plugman.metadata', function () { + beforeEach(function () { + fileMocks = {}; + metadata.__set__('cachedJson', null); + }); + + describe('get_fetch_metadata', function () { + + var get_fetch_metadata = metadata.get_fetch_metadata; + + describe('with no record', function () { + it('should return an empty object if there is no record', function () { + expect(get_fetch_metadata('/plugins_dir/', 'cordova-plugin-thinger')).toEqual({}); + }); + }); + + describe('cache behaviour', function () { + + beforeEach(function () { + spyOn(fsMock, 'readFileSync').and.callThrough(); + spyOn(fsMock, 'existsSync').and.callThrough(); + }); + + it('with no cache, it should read from the filesystem', function () { + + metadata.__set__('cachedJson', null); + fileMocks['/plugins_dir/fetch.json'] = JSON.stringify({ + 'cordova-plugin-thinger': { + 'metadata': 'matches' + } + }); + + var meta = get_fetch_metadata('/plugins_dir/', 'cordova-plugin-thinger'); + + expect(meta).toEqual({metadata: 'matches'}); + expect(fsMock.existsSync).toHaveBeenCalledWith('/plugins_dir/fetch.json'); + expect(fsMock.readFileSync).toHaveBeenCalledWith('/plugins_dir/fetch.json', 'utf-8'); + }); + + it('with a cache, it should read from the cache', function () { + metadata.__set__('cachedJson', { + 'cordova-plugin-thinger': { + metadata: 'cached' + } + }); + + fileMocks['/plugins_dir/fetch.json'] = JSON.stringify({ + 'cordova-plugin-thinger': { + 'metadata': 'matches' + } + }); + + var meta = get_fetch_metadata('/plugins_dir/', 'cordova-plugin-thinger'); + + expect(meta).toEqual({metadata: 'cached'}); + expect(fsMock.existsSync).not.toHaveBeenCalled(); + expect(fsMock.readFileSync).not.toHaveBeenCalled(); + }); + + }); + + it('should return the fetch metadata in plugins_dir/fetch.json if it is there', function () { + fileMocks['/plugins_dir/fetch.json'] = JSON.stringify({ + 'cordova-plugin-thinger': { + 'metadata': 'matches' + } + }); + + var meta = get_fetch_metadata('/plugins_dir/', 'cordova-plugin-thinger'); + + expect(meta).toEqual({metadata: 'matches'}); + }); + + it('should migrate legacy fetch metadata if it is there', function () { + fileMocks['/plugins_dir/cordova-plugin-thinger/.fetch.json'] = JSON.stringify({ + metadata: 'matches' + }); + fileMocks['/plugins_dir/@cordova/cordova-plugin-thinger/.fetch.json'] = JSON.stringify({ + metadata: 'matches' + }); + + var meta = get_fetch_metadata('/plugins_dir', '@cordova/cordova-plugin-thinger'); + + expect(meta).toEqual({metadata: 'matches'}); + expect(getFileMocksJson()).toEqual({ + '/plugins_dir/cordova-plugin-thinger/.fetch.json': {metadata: 'matches'}, + '/plugins_dir/fetch.json': { + '@cordova/cordova-plugin-thinger': { + metadata: 'matches' + } + } + }); + }); + + it('should return the fetch metadata in plugins_dir/fetch.json if it is there with a scoped plugin', function () { + fileMocks['/plugins_dir/fetch.json'] = JSON.stringify({ + '@cordova/cordova-plugin-thinger': { + 'metadata': 'matches' + } + }); + spyOn(fsMock, 'readFileSync').and.callThrough(); + spyOn(fsMock, 'existsSync').and.callThrough(); + + var meta = get_fetch_metadata('/plugins_dir/', '@cordova/cordova-plugin-thinger'); + + expect(meta).toEqual({metadata: 'matches'}); + expect(fsMock.existsSync).toHaveBeenCalledWith('/plugins_dir/fetch.json'); + expect(fsMock.readFileSync).toHaveBeenCalledWith('/plugins_dir/fetch.json', 'utf-8'); + }); + + }); + + describe('save_fetch_metadata', function () { + it('should save plugin metadata to a new fetch.json', function () { + var meta = {metadata: 'saved'}; + + metadata.save_fetch_metadata('/plugins_dir', '@cordova/cordova-plugin-thinger', meta); + + expect(getFileMocksJson()).toEqual({ + '/plugins_dir/fetch.json': { + '@cordova/cordova-plugin-thinger': { + metadata: 'saved' + } + } + }); + }); + + it('should save plugin metadata to an existing fetch.json', function () { + var meta = {metadata: 'saved'}; + + fileMocks = { + '/plugins_dir/fetch.json': JSON.stringify({ + 'some-other-plugin': { + metadata: 'not-touched' + } + }) + }; + + metadata.save_fetch_metadata('/plugins_dir', '@cordova/cordova-plugin-thinger', meta); + + expect(getFileMocksJson()).toEqual({ + '/plugins_dir/fetch.json': { + '@cordova/cordova-plugin-thinger': { + metadata: 'saved' + }, + 'some-other-plugin': { + metadata: 'not-touched' + } + } + }); + }); + }); + + describe('remove_fetch_metadata', function () { + it('should remove metadata', function () { + fileMocks = { + '/plugins_dir/fetch.json': JSON.stringify({ + 'some-plugin': { + metadata: 'existing' + } + }) + }; + + metadata.remove_fetch_metadata('/plugins_dir', 'some-plugin'); + + expect(getFileMocksJson()).toEqual({ + '/plugins_dir/fetch.json': { } + }); + }); + }); + +}); From c4c04158ad4b483e5b8838ff95dcae497816d6db Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Thu, 2 Nov 2017 10:21:45 +1100 Subject: [PATCH 12/14] fix cordova-lib/util.js for node 4 --- src/cordova/util.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cordova/util.js b/src/cordova/util.js index ae9219269..c24558d22 100644 --- a/src/cordova/util.js +++ b/src/cordova/util.js @@ -258,7 +258,9 @@ function findPlugins (pluginDir) { plugins = fs.readdirSync(pluginDir) .reduce(function (plugins, pluginOrScope) { if (pluginOrScope[0] === '@') { - plugins.push(...fs.readdirSync(path.join(pluginDir, pluginOrScope)).map(s => path.join(pluginOrScope, s))); + var scopedPlugins = fs.readdirSync(path.join(pluginDir, pluginOrScope)) + .map(scopedPluginDir => path.join(pluginOrScope, scopedPluginDir)); + plugins.push.apply(plugins, scopedPlugins); } else { plugins.push(pluginOrScope); } From 365cbb71c99612f2d7008cb30d58b403fd2951c5 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Thu, 2 Nov 2017 11:00:42 +1100 Subject: [PATCH 13/14] fix addHelper tests on Windows --- spec/cordova/platform/addHelper.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/cordova/platform/addHelper.spec.js b/spec/cordova/platform/addHelper.spec.js index 100c44e38..3d8702bfe 100644 --- a/spec/cordova/platform/addHelper.spec.js +++ b/spec/cordova/platform/addHelper.spec.js @@ -450,9 +450,9 @@ describe('cordova/platform/addHelper', function () { platform_addHelper.installPluginsForNewPlatform('browser', projectRoot, {save: true, fetch: true}).then(function () { expect(plugman.install).toHaveBeenCalledWith( 'browser', - '/some/path/platforms/browser', + path.normalize('/some/path/platforms/browser'), pluginName, - '/some/path/plugins', + path.normalize('/some/path/plugins'), jasmine.objectContaining(expectedInstallOptions) ); expect(events.emit).toHaveBeenCalledWith('verbose', `Installing plugin "${pluginName}" following successful platform add of browser`); From a2feee09f2c4a9f348a65dd7e5fde36463c96016 Mon Sep 17 00:00:00 2001 From: Jarrad Whitaker Date: Thu, 2 Nov 2017 11:00:53 +1100 Subject: [PATCH 14/14] fix get_fetch_metadata tests on windows --- spec/plugman/util/metadata.spec.js | 53 ++++++++++++++++-------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/spec/plugman/util/metadata.spec.js b/spec/plugman/util/metadata.spec.js index 294260113..e0c5e389e 100644 --- a/spec/plugman/util/metadata.spec.js +++ b/spec/plugman/util/metadata.spec.js @@ -1,6 +1,7 @@ var rewire = require('rewire'); var metadata = rewire('../../../src/plugman/util/metadata'); +var path = require('path'); var fileMocks; var fsMock = { @@ -42,13 +43,15 @@ describe('plugman.metadata', function () { metadata.__set__('cachedJson', null); }); + var pluginsDir = path.normalize('/plugins_dir/'); + describe('get_fetch_metadata', function () { var get_fetch_metadata = metadata.get_fetch_metadata; describe('with no record', function () { it('should return an empty object if there is no record', function () { - expect(get_fetch_metadata('/plugins_dir/', 'cordova-plugin-thinger')).toEqual({}); + expect(get_fetch_metadata(pluginsDir, 'cordova-plugin-thinger')).toEqual({}); }); }); @@ -62,17 +65,17 @@ describe('plugman.metadata', function () { it('with no cache, it should read from the filesystem', function () { metadata.__set__('cachedJson', null); - fileMocks['/plugins_dir/fetch.json'] = JSON.stringify({ + fileMocks[path.normalize('/plugins_dir/fetch.json')] = JSON.stringify({ 'cordova-plugin-thinger': { 'metadata': 'matches' } }); - var meta = get_fetch_metadata('/plugins_dir/', 'cordova-plugin-thinger'); + var meta = get_fetch_metadata(pluginsDir, 'cordova-plugin-thinger'); expect(meta).toEqual({metadata: 'matches'}); - expect(fsMock.existsSync).toHaveBeenCalledWith('/plugins_dir/fetch.json'); - expect(fsMock.readFileSync).toHaveBeenCalledWith('/plugins_dir/fetch.json', 'utf-8'); + expect(fsMock.existsSync).toHaveBeenCalledWith(path.normalize('/plugins_dir/fetch.json')); + expect(fsMock.readFileSync).toHaveBeenCalledWith(path.normalize('/plugins_dir/fetch.json'), 'utf-8'); }); it('with a cache, it should read from the cache', function () { @@ -82,13 +85,13 @@ describe('plugman.metadata', function () { } }); - fileMocks['/plugins_dir/fetch.json'] = JSON.stringify({ + fileMocks[path.normalize('/plugins_dir/fetch.json')] = JSON.stringify({ 'cordova-plugin-thinger': { 'metadata': 'matches' } }); - var meta = get_fetch_metadata('/plugins_dir/', 'cordova-plugin-thinger'); + var meta = get_fetch_metadata(pluginsDir, 'cordova-plugin-thinger'); expect(meta).toEqual({metadata: 'cached'}); expect(fsMock.existsSync).not.toHaveBeenCalled(); @@ -98,31 +101,31 @@ describe('plugman.metadata', function () { }); it('should return the fetch metadata in plugins_dir/fetch.json if it is there', function () { - fileMocks['/plugins_dir/fetch.json'] = JSON.stringify({ + fileMocks[path.normalize('/plugins_dir/fetch.json')] = JSON.stringify({ 'cordova-plugin-thinger': { 'metadata': 'matches' } }); - var meta = get_fetch_metadata('/plugins_dir/', 'cordova-plugin-thinger'); + var meta = get_fetch_metadata(pluginsDir, 'cordova-plugin-thinger'); expect(meta).toEqual({metadata: 'matches'}); }); it('should migrate legacy fetch metadata if it is there', function () { - fileMocks['/plugins_dir/cordova-plugin-thinger/.fetch.json'] = JSON.stringify({ + fileMocks[path.normalize('/plugins_dir/cordova-plugin-thinger/.fetch.json')] = JSON.stringify({ metadata: 'matches' }); - fileMocks['/plugins_dir/@cordova/cordova-plugin-thinger/.fetch.json'] = JSON.stringify({ + fileMocks[path.normalize('/plugins_dir/@cordova/cordova-plugin-thinger/.fetch.json')] = JSON.stringify({ metadata: 'matches' }); - var meta = get_fetch_metadata('/plugins_dir', '@cordova/cordova-plugin-thinger'); + var meta = get_fetch_metadata(pluginsDir, '@cordova/cordova-plugin-thinger'); expect(meta).toEqual({metadata: 'matches'}); expect(getFileMocksJson()).toEqual({ - '/plugins_dir/cordova-plugin-thinger/.fetch.json': {metadata: 'matches'}, - '/plugins_dir/fetch.json': { + [path.normalize('/plugins_dir/cordova-plugin-thinger/.fetch.json')]: {metadata: 'matches'}, + [path.normalize('/plugins_dir/fetch.json')]: { '@cordova/cordova-plugin-thinger': { metadata: 'matches' } @@ -131,7 +134,7 @@ describe('plugman.metadata', function () { }); it('should return the fetch metadata in plugins_dir/fetch.json if it is there with a scoped plugin', function () { - fileMocks['/plugins_dir/fetch.json'] = JSON.stringify({ + fileMocks[path.normalize('/plugins_dir/fetch.json')] = JSON.stringify({ '@cordova/cordova-plugin-thinger': { 'metadata': 'matches' } @@ -139,11 +142,11 @@ describe('plugman.metadata', function () { spyOn(fsMock, 'readFileSync').and.callThrough(); spyOn(fsMock, 'existsSync').and.callThrough(); - var meta = get_fetch_metadata('/plugins_dir/', '@cordova/cordova-plugin-thinger'); + var meta = get_fetch_metadata(pluginsDir, '@cordova/cordova-plugin-thinger'); expect(meta).toEqual({metadata: 'matches'}); - expect(fsMock.existsSync).toHaveBeenCalledWith('/plugins_dir/fetch.json'); - expect(fsMock.readFileSync).toHaveBeenCalledWith('/plugins_dir/fetch.json', 'utf-8'); + expect(fsMock.existsSync).toHaveBeenCalledWith(path.normalize('/plugins_dir/fetch.json')); + expect(fsMock.readFileSync).toHaveBeenCalledWith(path.normalize('/plugins_dir/fetch.json'), 'utf-8'); }); }); @@ -152,10 +155,10 @@ describe('plugman.metadata', function () { it('should save plugin metadata to a new fetch.json', function () { var meta = {metadata: 'saved'}; - metadata.save_fetch_metadata('/plugins_dir', '@cordova/cordova-plugin-thinger', meta); + metadata.save_fetch_metadata(pluginsDir, '@cordova/cordova-plugin-thinger', meta); expect(getFileMocksJson()).toEqual({ - '/plugins_dir/fetch.json': { + [path.normalize('/plugins_dir/fetch.json')]: { '@cordova/cordova-plugin-thinger': { metadata: 'saved' } @@ -167,7 +170,7 @@ describe('plugman.metadata', function () { var meta = {metadata: 'saved'}; fileMocks = { - '/plugins_dir/fetch.json': JSON.stringify({ + [path.normalize('/plugins_dir/fetch.json')]: JSON.stringify({ 'some-other-plugin': { metadata: 'not-touched' } @@ -177,7 +180,7 @@ describe('plugman.metadata', function () { metadata.save_fetch_metadata('/plugins_dir', '@cordova/cordova-plugin-thinger', meta); expect(getFileMocksJson()).toEqual({ - '/plugins_dir/fetch.json': { + [path.normalize('/plugins_dir/fetch.json')]: { '@cordova/cordova-plugin-thinger': { metadata: 'saved' }, @@ -192,17 +195,17 @@ describe('plugman.metadata', function () { describe('remove_fetch_metadata', function () { it('should remove metadata', function () { fileMocks = { - '/plugins_dir/fetch.json': JSON.stringify({ + [path.normalize('/plugins_dir/fetch.json')]: JSON.stringify({ 'some-plugin': { metadata: 'existing' } }) }; - metadata.remove_fetch_metadata('/plugins_dir', 'some-plugin'); + metadata.remove_fetch_metadata(pluginsDir, 'some-plugin'); expect(getFileMocksJson()).toEqual({ - '/plugins_dir/fetch.json': { } + [path.normalize('/plugins_dir/fetch.json')]: { } }); }); });