You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by matrosov-nikita <gi...@git.apache.org> on 2016/10/19 11:26:25 UTC
[GitHub] cordova-windows pull request #203: CB-11933: Add uap prefixes for capabiliti...
GitHub user matrosov-nikita opened a pull request:
https://github.com/apache/cordova-windows/pull/203
CB-11933: Add uap prefixes for capabilities from package.windows10.appxmanifest when installing plugin
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist
is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### What does this PR do?
Windows has a special logic for adding appxmanifest's capabilities. This PR adds uap prefixes to capabilities from package.windows10.appxmanifest when installing plugin.
### What testing has been done on this change?
Auto test
### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
- [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
- [x] Added automated test coverage as appropriate for this change.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/matrosov-nikita/cordova-windows CB-11933-add
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-windows/pull/203.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #203
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows issue #203: CB-11933: Add uap prefixes for capabilities from...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the issue:
https://github.com/apache/cordova-windows/pull/203
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request #203: CB-11933: Add uap prefixes for capabiliti...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/203#discussion_r84070095
--- Diff: template/cordova/lib/ConfigChanges.js ---
@@ -34,54 +39,121 @@ util.inherits(PlatformMunger, CommonMunger);
* need to be removed or added to the file
*/
PlatformMunger.prototype.apply_file_munge = function (file, munge, remove) {
- // Call parent class' method
- PlatformMunger.super_.prototype.apply_file_munge.call(this, file, munge, remove);
-
- // CB-11066 If this is a windows10 manifest and we're removing the changes
- // then we also need to check if there are <Capability> elements were previously
- // added and schedule removal of corresponding <uap:Capability> elements
- if (remove && file === 'package.windows10.appxmanifest') {
- var uapCapabilitiesMunge = generateUapCapabilities(munge);
- // We do not check whether generated munge is empty or not before calling
- // 'apply_file_munge' since applying empty one is just a no-op
- PlatformMunger.super_.prototype.apply_file_munge.call(this, file, uapCapabilitiesMunge, remove);
- }
+
+ // Create a copy to avoid modification of original munge
+ var mungeCopy = cloneObject(munge);
+ var capabilities = mungeCopy.parents[CAPS_SELECTOR];
+
+ // Add 'uap' prefixes for windows 10 manifest
+ if (file === WINDOWS10_MANIFEST)
--- End diff --
Please add `{}` here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request #203: CB-11933: Add uap prefixes for capabiliti...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/203#discussion_r84070152
--- Diff: template/cordova/lib/ConfigChanges.js ---
@@ -34,54 +39,121 @@ util.inherits(PlatformMunger, CommonMunger);
* need to be removed or added to the file
*/
PlatformMunger.prototype.apply_file_munge = function (file, munge, remove) {
- // Call parent class' method
- PlatformMunger.super_.prototype.apply_file_munge.call(this, file, munge, remove);
-
- // CB-11066 If this is a windows10 manifest and we're removing the changes
- // then we also need to check if there are <Capability> elements were previously
- // added and schedule removal of corresponding <uap:Capability> elements
- if (remove && file === 'package.windows10.appxmanifest') {
- var uapCapabilitiesMunge = generateUapCapabilities(munge);
- // We do not check whether generated munge is empty or not before calling
- // 'apply_file_munge' since applying empty one is just a no-op
- PlatformMunger.super_.prototype.apply_file_munge.call(this, file, uapCapabilitiesMunge, remove);
- }
+
+ // Create a copy to avoid modification of original munge
+ var mungeCopy = cloneObject(munge);
+ var capabilities = mungeCopy.parents[CAPS_SELECTOR];
+
+ // Add 'uap' prefixes for windows 10 manifest
+ if (file === WINDOWS10_MANIFEST)
+ capabilities = generateUapCapabilities(capabilities);
+
+ // Remove duplicates and sort capabilities when installing plugin
+ if (!remove)
--- End diff --
Please add `{}` here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request #203: CB-11933: Add uap prefixes for capabiliti...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/203#discussion_r84077075
--- Diff: spec/unit/ConfigChanges.spec.js ---
@@ -57,76 +57,98 @@ describe('PlatformMunger', function () {
it('should call parent\'s method with the same parameters', function () {
munger.apply_file_munge(WINDOWS_MANIFEST, munge, false);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS_MANIFEST, munge, false);
--- End diff --
Why do we need to remove these checks?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request #203: CB-11933: Add uap prefixes for capabiliti...
Posted by matrosov-nikita <gi...@git.apache.org>.
Github user matrosov-nikita commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/203#discussion_r84247927
--- Diff: spec/unit/ConfigChanges.spec.js ---
@@ -57,24 +57,28 @@ describe('PlatformMunger', function () {
it('should call parent\'s method with the same parameters', function () {
munger.apply_file_munge(WINDOWS_MANIFEST, munge, false);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS_MANIFEST, munge, false);
});
it('should additionally call parent\'s method with another munge if removing changes from windows 10 appxmanifest', function () {
munger.apply_file_munge(WINDOWS10_MANIFEST, munge, /*remove=*/true);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS10_MANIFEST, munge, true);
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS10_MANIFEST, jasmine.any(Object), true);
--- End diff --
@daserge, I've removed this check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request #203: CB-11933: Add uap prefixes for capabiliti...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-windows/pull/203
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows issue #203: CB-11933: Add uap prefixes for capabilities from...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the issue:
https://github.com/apache/cordova-windows/pull/203
@daserge, can you take a look?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request #203: CB-11933: Add uap prefixes for capabiliti...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/203#discussion_r84127530
--- Diff: spec/unit/ConfigChanges.spec.js ---
@@ -57,24 +57,28 @@ describe('PlatformMunger', function () {
it('should call parent\'s method with the same parameters', function () {
munger.apply_file_munge(WINDOWS_MANIFEST, munge, false);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS_MANIFEST, munge, false);
});
it('should additionally call parent\'s method with another munge if removing changes from windows 10 appxmanifest', function () {
munger.apply_file_munge(WINDOWS10_MANIFEST, munge, /*remove=*/true);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS10_MANIFEST, munge, true);
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS10_MANIFEST, jasmine.any(Object), true);
--- End diff --
I know this is an original code but I wonder if the second check is actually checking something?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows issue #203: CB-11933: Add uap prefixes for capabilities from...
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/cordova-windows/pull/203
## [Current coverage](https://codecov.io/gh/apache/cordova-windows/pull/203?src=pr) is 74.66% (diff: 93.47%)
> Merging [#203](https://codecov.io/gh/apache/cordova-windows/pull/203?src=pr) into [master](https://codecov.io/gh/apache/cordova-windows/branch/master?src=pr) will increase coverage by **0.30%**
```diff
@@ master #203 diff @@
==========================================
Files 15 15
Lines 2032 2068 +36
Methods 384 390 +6
Messages 0 0
Branches 396 404 +8
==========================================
+ Hits 1511 1544 +33
- Misses 521 524 +3
Partials 0 0
```
> Powered by [Codecov](https://codecov.io?src=pr). Last update [610b0ad...487061a](https://codecov.io/gh/apache/cordova-windows/compare/610b0adeb65add1a033c63ebaa1b2b052ff770bd...487061a48a333747b090ae6a8ee044bb5bc42d7d?src=pr)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request #203: CB-11933: Add uap prefixes for capabiliti...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/203#discussion_r84069176
--- Diff: spec/unit/ConfigChanges.spec.js ---
@@ -57,76 +57,98 @@ describe('PlatformMunger', function () {
it('should call parent\'s method with the same parameters', function () {
munger.apply_file_munge(WINDOWS_MANIFEST, munge, false);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS_MANIFEST, munge, false);
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
});
it('should additionally call parent\'s method with another munge if removing changes from windows 10 appxmanifest', function () {
- munger.apply_file_munge('package.windows10.appxmanifest', munge, /*remove=*/true);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS10_MANIFEST, munge, true);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS10_MANIFEST, jasmine.any(Object), true);
+ munger.apply_file_munge(WINDOWS10_MANIFEST, munge, /*remove=*/true);
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
});
it('should remove uap: capabilities added by windows prepare step', function () {
// Generate a munge that contain non-prefixed capabilities changes
- var baseMunge = { parents: { WINDOWS10_MANIFEST: [
+ var baseMunge = { parents: { 'package.windows10.appxmanifest': [
// Emulate capability that was initially added with uap prefix
{ before: undefined, count: 1, xml: '<uap:Capability Name=\"privateNetworkClientServer\">'},
{ before: undefined, count: 1, xml: '<Capability Name=\"enterpriseAuthentication\">'}
]}};
-
- var capabilitiesMunge = { parents: { WINDOWS10_MANIFEST: [
- { before: undefined, count: 1, xml: '<uap:Capability Name=\"enterpriseAuthentication\">'}
- ]}};
-
- munger.apply_file_munge('package.windows10.appxmanifest', baseMunge, /*remove=*/true);
- expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalledWith(WINDOWS10_MANIFEST, capabilitiesMunge, true);
+ munger.apply_file_munge(WINDOWS10_MANIFEST, baseMunge, /*remove=*/true);
+ expect(BaseMunger.prototype.apply_file_munge).toHaveBeenCalled();
});
});
});
describe('Capabilities within package.windows.appxmanifest', function() {
- var testDir;
+
+ var testDir, windowsPlatform, windowsManifest, windowsManifest10, dummyPluginInfo, api;
beforeEach(function() {
testDir = path.join(__dirname, 'testDir');
shell.mkdir('-p', testDir);
shell.cp('-rf', windowsProject + '/*', testDir);
+ windowsPlatform = path.join(testDir, 'platforms/windows');
+ windowsManifest = path.join(windowsPlatform, WINDOWS_MANIFEST);
+ windowsManifest10 = path.join(windowsPlatform, WINDOWS10_MANIFEST);
+ dummyPluginInfo = new PluginInfo(dummyPlugin);
+ api = new Api();
+ api.root = windowsPlatform;
+ api.locations.root = windowsPlatform;
+ api.locations.www = path.join(windowsPlatform, 'www');
});
afterEach(function() {
shell.rm('-rf', testDir);
});
- it('should be removed using overriden PlatformMunger', function(done) {
- var windowsPlatform = path.join(testDir, 'platforms/windows');
- var windowsManifest = path.join(windowsPlatform, WINDOWS_MANIFEST);
- var api = new Api();
- api.root = windowsPlatform;
- api.locations.root = windowsPlatform;
- api.locations.www = path.join(windowsPlatform, 'www');
- var dummyPluginInfo = new PluginInfo(dummyPlugin);
+ function getPluginCapabilities(pluginInfo) {
+ return pluginInfo.getConfigFiles()[0].xmls;
+ }
- var fail = jasmine.createSpy('fail')
- .andCallFake(function (err) {
- console.error(err);
- });
+ function getManifestCapabilities(manifest) {
+ var appxmanifest = AppxManifest.get(manifest, true);
+ return appxmanifest.getCapabilities();
+ }
- function getPluginCapabilities() {
- return dummyPluginInfo.getConfigFiles()[0].xmls;
- }
+ var fail = jasmine.createSpy('fail')
+ .andCallFake(function (err) {
+ console.error(err);
+ });
+
+ it('should be removed using overriden PlatformMunger', function(done) {
+ api.addPlugin(dummyPluginInfo)
+ .then(function() {
+ // There is the one default capability in manifest with 'internetClient' name
+ expect(getManifestCapabilities(windowsManifest).length).toBe(getPluginCapabilities(dummyPluginInfo).length + 1);
+ api.removePlugin(dummyPluginInfo);
+ })
+ .then(function() {
+ expect(getManifestCapabilities(windowsManifest).length).toBe(1);
+ })
+ .catch(fail)
+ .finally(function() {
+ expect(fail).not.toHaveBeenCalled();
+ done();
+ });
+ });
- function getManifestCapabilities() {
- var appxmanifest = AppxManifest.get(windowsManifest, true);
- return appxmanifest.getCapabilities();
- }
+ it('should be added with uap prefixes when install plugin', function(done) {
api.addPlugin(dummyPluginInfo)
.then(function() {
// There is the one default capability in manifest with 'internetClient' name
- expect(getManifestCapabilities().length).toBe(getPluginCapabilities().length + 1);
+ var manifestCapabilities = getManifestCapabilities(windowsManifest10);
+ expect(manifestCapabilities.length).toBe(getPluginCapabilities(dummyPluginInfo).length + 1);
+
+ // Count 'uap' prefixed capabilities
+ var uapPrefixedCapsCount = manifestCapabilities.reduce(function(prefixedCount, currCap) {
--- End diff --
`filter` would be more readable here IMHO.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows issue #203: CB-11933: Add uap prefixes for capabilities from...
Posted by matrosov-nikita <gi...@git.apache.org>.
Github user matrosov-nikita commented on the issue:
https://github.com/apache/cordova-windows/pull/203
@daserge, thank you for your comments. I've returned back to specs with a few modifications and refactored some parts of code.
@daserge @vladimir-kotikov, can you review changes, pls?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org