This is an automated email from the ASF dual-hosted git repository.
raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-common.git
The following commit(s) were added to refs/heads/master by this push:
new 0a589b9 refactor(PlatformMunger): DRY & simplify config munging (#162)
0a589b9 is described below
commit 0a589b967e0739b94b2900eafbb40fa4f97e7368
Author: Raphael von der Grün <raphine...@gmail.com>
AuthorDate: Mon Oct 18 23:23:04 2021 +0200
refactor(PlatformMunger): DRY & simplify config munging (#162)
* factor out private helper _getChanges
* factor out private helper _generateMunge
* DRY config_munge generation
* change _munge_helper signature
* use _munge_helper for removal too
* remove superfluous if-guards
* is_conflicting gets global_munge itself
* destructure & remove more superfluous guards
* simplify info collected & returned by _is_conflicting
* rename conflict props
* nicer _is_conflicting
---
spec/ConfigChanges/ConfigChanges.spec.js | 2 +-
src/ConfigChanges/ConfigChanges.js | 288 +++++++++++--------------------
2 files changed, 100 insertions(+), 190 deletions(-)
diff --git a/spec/ConfigChanges/ConfigChanges.spec.js
b/spec/ConfigChanges/ConfigChanges.spec.js
index ac6f5ca..8076cd3 100644
--- a/spec/ConfigChanges/ConfigChanges.spec.js
+++ b/spec/ConfigChanges/ConfigChanges.spec.js
@@ -232,7 +232,7 @@ describe('config-changes module', function () {
const munger = new configChanges.PlatformMunger('android', temp,
platformJson, pluginInfoProvider);
const spy = spyOn(munger,
'generate_plugin_config_munge').and.returnValue({});
munger.process(plugins_dir);
- expect(spy).toHaveBeenCalledWith(jasmine.any(PluginInfo), {});
+ expect(spy).toHaveBeenCalledWith(jasmine.any(PluginInfo), {}, []);
});
describe(': installation', function () {
diff --git a/src/ConfigChanges/ConfigChanges.js
b/src/ConfigChanges/ConfigChanges.js
index de62856..b4851cf 100644
--- a/src/ConfigChanges/ConfigChanges.js
+++ b/src/ConfigChanges/ConfigChanges.js
@@ -86,21 +86,13 @@ class PlatformMunger {
const plugin_vars = is_top_level
? platform_config.installed_plugins[pluginInfo.id]
: platform_config.dependent_plugins[pluginInfo.id];
- let edit_config_changes = null;
- if (pluginInfo.getEditConfigs) {
- edit_config_changes = pluginInfo.getEditConfigs(this.platform);
- }
+ const edit_config_changes = this._getChanges(pluginInfo, 'EditConfig');
// get config munge, aka how did this plugin change various config
files
const config_munge = this.generate_plugin_config_munge(pluginInfo,
plugin_vars, edit_config_changes);
- // global munge looks at all plugins' changes to config files
- const global_munge = platform_config.config_munge;
- const munge = mungeutil.decrement_munge(global_munge, config_munge);
- for (const file in munge.files) {
- this.apply_file_munge(file, munge.files[file], /* remove = */
true);
- }
+ this._munge_helper(config_munge, { remove: true });
// Remove from installed_plugins
this.platformJson.removePlugin(pluginInfo.id, is_top_level);
@@ -109,43 +101,28 @@ class PlatformMunger {
}
add_plugin_changes (pluginInfo, plugin_vars, is_top_level,
should_increment, plugin_force) {
- const platform_config = this.platformJson.root;
- let config_munge;
- let edit_config_changes = null;
+ const edit_config_changes = this._getChanges(pluginInfo, 'EditConfig');
- if (pluginInfo.getEditConfigs) {
- edit_config_changes = pluginInfo.getEditConfigs(this.platform);
+ const { configConflicts, pluginConflicts } =
this._is_conflicting(edit_config_changes);
+ if (Object.keys(configConflicts.files).length > 0) {
+ // plugin.xml cannot overwrite config.xml changes even if --force
is used
+ throw new Error(`${pluginInfo.id} cannot be added. <edit-config>
changes in this plugin conflicts with <edit-config> changes in config.xml.
Conflicts must be resolved before plugin can be added.`);
}
-
- if (!edit_config_changes || edit_config_changes.length === 0) {
- // get config munge, aka how should this plugin change various
config files
- config_munge = this.generate_plugin_config_munge(pluginInfo,
plugin_vars);
- } else {
- const isConflictingInfo =
this._is_conflicting(edit_config_changes, platform_config.config_munge,
plugin_force);
-
- if (isConflictingInfo.conflictWithConfigxml) {
- throw new Error(`${pluginInfo.id} cannot be added.
<edit-config> changes in this plugin conflicts with <edit-config> changes in
config.xml. Conflicts must be resolved before plugin can be added.`);
- }
- if (plugin_force) {
- events.emit('warn', '--force is used. edit-config will
overwrite conflicts if any. Conflicting plugins may not work as expected.');
-
- // remove conflicting munges
- const conflict_munge =
mungeutil.decrement_munge(platform_config.config_munge,
isConflictingInfo.conflictingMunge);
- for (const conflict_file in conflict_munge.files) {
- this.apply_file_munge(conflict_file,
conflict_munge.files[conflict_file], /* remove = */ true);
- }
-
- // force add new munges
- config_munge = this.generate_plugin_config_munge(pluginInfo,
plugin_vars, edit_config_changes);
- } else if (isConflictingInfo.conflictFound) {
- throw new Error(`There was a conflict trying to modify
attributes with <edit-config> in plugin ${pluginInfo.id}. The conflicting
plugin, ${isConflictingInfo.conflictingPlugin}, already modified the same
attributes. The conflict must be resolved before ${pluginInfo.id} can be added.
You may use --force to add the plugin and overwrite the conflicting
attributes.`);
- } else {
- // no conflicts, will handle edit-config
- config_munge = this.generate_plugin_config_munge(pluginInfo,
plugin_vars, edit_config_changes);
- }
+ if (plugin_force) {
+ events.emit('warn', '--force is used. edit-config will overwrite
conflicts if any. Conflicting plugins may not work as expected.');
+
+ // remove conflicting munges, if any
+ this._munge_helper(pluginConflicts, { remove: true });
+ } else if (Object.keys(pluginConflicts.files).length > 0) {
+ // plugin cannot overwrite other plugin changes without --force
+ const witness =
Object.values(Object.values(pluginConflicts.files)[0].parents)[0][0];
+ const conflictingPlugin = witness.plugin;
+ throw new Error(`There was a conflict trying to modify attributes
with <edit-config> in plugin ${pluginInfo.id}. The conflicting plugin,
${conflictingPlugin}, already modified the same attributes. The conflict must
be resolved before ${pluginInfo.id} can be added. You may use --force to add
the plugin and overwrite the conflicting attributes.`);
}
- this._munge_helper(should_increment, platform_config, config_munge);
+ // get config munge, aka how should this plugin change various config
files
+ const config_munge = this.generate_plugin_config_munge(pluginInfo,
plugin_vars, edit_config_changes);
+ this._munge_helper(config_munge, { should_increment });
// Move to installed/dependent_plugins
this.platformJson.addPlugin(pluginInfo.id, plugin_vars || {},
is_top_level);
@@ -155,68 +132,45 @@ class PlatformMunger {
// Handle edit-config changes from config.xml
add_config_changes (config, should_increment) {
- const platform_config = this.platformJson.root;
- let changes = [];
-
- if (config.getEditConfigs) {
- const edit_config_changes = config.getEditConfigs(this.platform);
- if (edit_config_changes) {
- changes = changes.concat(edit_config_changes);
- }
- }
-
- if (config.getConfigFiles) {
- const config_files_changes = config.getConfigFiles(this.platform);
- if (config_files_changes) {
- changes = changes.concat(config_files_changes);
- }
+ const changes = [
+ ...this._getChanges(config, 'EditConfig'),
+ ...this._getChanges(config, 'ConfigFile')
+ ];
+
+ const { configConflicts, pluginConflicts } =
this._is_conflicting(changes);
+ if (Object.keys(pluginConflicts.files).length !== 0) {
+ events.emit('warn', 'Conflict found, edit-config changes from
config.xml will overwrite plugin.xml changes');
}
-
- if (changes && changes.length > 0) {
- const isConflictingInfo = this._is_conflicting(changes,
platform_config.config_munge, true /* always force overwrite other edit-config
*/);
- if (isConflictingInfo.conflictFound) {
- if (Object.keys(isConflictingInfo.configxmlMunge.files).length
!== 0) {
- // silently remove conflicting config.xml munges so new
munges can be added
- const conflict_munge =
mungeutil.decrement_munge(platform_config.config_munge,
isConflictingInfo.configxmlMunge);
- for (const conflict_file in conflict_munge.files) {
- this.apply_file_munge(conflict_file,
conflict_munge.files[conflict_file], /* remove = */ true);
- }
- }
-
- if
(Object.keys(isConflictingInfo.conflictingMunge.files).length !== 0) {
- events.emit('warn', 'Conflict found, edit-config changes
from config.xml will overwrite plugin.xml changes');
-
- // remove conflicting plugin.xml munges
- const conflict_munge =
mungeutil.decrement_munge(platform_config.config_munge,
isConflictingInfo.conflictingMunge);
- for (const conflict_file in conflict_munge.files) {
- this.apply_file_munge(conflict_file,
conflict_munge.files[conflict_file], /* remove = */ true);
- }
- }
- }
+ // remove conflicting config.xml & plugin.xml munges, if any
+ for (const conflict_munge of [configConflicts, pluginConflicts]) {
+ this._munge_helper(conflict_munge, { remove: true });
}
// Add config.xml edit-config and config-file munges
const config_munge = this.generate_config_xml_munge(config, changes,
'config.xml');
- this._munge_helper(should_increment, platform_config, config_munge);
+ this._munge_helper(config_munge, { should_increment });
// Move to installed/dependent_plugins
return this;
}
/** @private */
- _munge_helper (should_increment, platform_config, config_munge) {
+ _munge_helper (config_munge, { should_increment = true, remove = false } =
{}) {
// global munge looks at all changes to config files
// TODO: The should_increment param is only used by cordova-cli and is
going away soon.
// If should_increment is set to false, avoid modifying the
global_munge (use clone)
// and apply the entire config_munge because it's already a proper
subset of the global_munge.
+ const platform_config = this.platformJson.root;
const global_munge = platform_config.config_munge;
+
+ const method = remove ? 'decrement_munge' : 'increment_munge';
const munge = should_increment
- ? mungeutil.increment_munge(global_munge, config_munge)
+ ? mungeutil[method](global_munge, config_munge)
: config_munge;
for (const file in munge.files) {
- this.apply_file_munge(file, munge.files[file]);
+ this.apply_file_munge(file, munge.files[file], remove);
}
return this;
@@ -235,59 +189,45 @@ class PlatformMunger {
return this;
}
- // generate_config_xml_munge
// Generate the munge object from config.xml
generate_config_xml_munge (config, config_changes, type) {
- const munge = { files: {} };
-
- if (!config_changes) return munge;
-
- const id = type === 'config.xml' ? type : config.id;
-
- config_changes.forEach(change => {
- change.xmls.forEach(xml => {
- // 1. stringify each xml
- const stringified = (new et.ElementTree(xml)).write({
xml_declaration: false });
- // 2. add into munge
- if (change.mode) {
- mungeutil.deep_add(munge, change.file, change.target, {
xml: stringified, count: 1, mode: change.mode, id });
- } else {
- mungeutil.deep_add(munge, change.target, change.parent, {
xml: stringified, count: 1, after: change.after });
- }
- });
- });
-
- return munge;
+ const originInfo = { id: type === 'config.xml' ? type : config.id };
+ return this._generateMunge(config_changes, originInfo);
}
- // generate_plugin_config_munge
// Generate the munge object from plugin.xml + vars
generate_plugin_config_munge (pluginInfo, vars, edit_config_changes) {
- vars = vars || {};
- const munge = { files: {} };
const changes = pluginInfo.getConfigFiles(this.platform);
-
if (edit_config_changes) {
Array.prototype.push.apply(changes, edit_config_changes);
}
+ const filteredChanges = changes.filter(({ mode }) => mode !==
'remove');
+
+ const originInfo = { plugin: pluginInfo.id };
+ return this._generateMunge(filteredChanges, originInfo, vars || {});
+ }
+
+ /** @private */
+ _generateMunge (changes, originInfo, vars = {}) {
+ const munge = { files: {} };
changes.forEach(change => {
+ const [file, selector, rest] = change.mode
+ ? [change.file, change.target, { mode: change.mode,
...originInfo }]
+ : [change.target, change.parent, { after: change.after }];
+
change.xmls.forEach(xml => {
// 1. stringify each xml
let stringified = (new et.ElementTree(xml)).write({
xml_declaration: false });
- // interp vars
+
+ // interpolate vars, if any
Object.keys(vars).forEach(key => {
const regExp = new RegExp(`\\$${key}`, 'g');
stringified = stringified.replace(regExp, vars[key]);
});
+
// 2. add into munge
- if (change.mode) {
- if (change.mode !== 'remove') {
- mungeutil.deep_add(munge, change.file, change.target,
{ xml: stringified, count: 1, mode: change.mode, plugin: pluginInfo.id });
- }
- } else {
- mungeutil.deep_add(munge, change.target, change.parent, {
xml: stringified, count: 1, after: change.after });
- }
+ mungeutil.deep_add(munge, file, selector, { xml: stringified,
count: 1, ...rest });
});
});
@@ -295,82 +235,52 @@ class PlatformMunger {
}
/** @private */
- _is_conflicting (editchanges, config_munge, force) {
- const files = config_munge.files;
- let conflictFound = false;
- let conflictWithConfigxml = false;
- const conflictingMunge = { files: {} };
- const configxmlMunge = { files: {} };
- let conflictingParent;
- let conflictingPlugin;
-
- editchanges.forEach(editchange => {
- if (files[editchange.file]) {
- const parents = files[editchange.file].parents;
- let target = parents[editchange.target];
-
- // Check if the edit target will resolve to an existing target
- if (!target || target.length === 0) {
- const targetFile =
this.config_keeper.get(this.project_dir, this.platform, editchange.file);
-
- // For non-XML files (e.g. plist), the selector in
editchange.target uniquely identifies its target.
- // Thus we already know that we have no conflict if we are
not dealing with an XML file here.
- if (targetFile.type !== 'xml') return;
-
- // For XML files, the selector does NOT uniquely identify
its target. So we resolve editchange.target
- // and any existing selectors to their matched elements
and compare those for equality.
- const resolveEditTarget =
xml_helpers.resolveParent(targetFile.data, editchange.target);
- if (resolveEditTarget) {
- for (const parent in parents) {
- const resolveTarget =
xml_helpers.resolveParent(targetFile.data, parent);
- if (resolveEditTarget === resolveTarget) {
- conflictingParent = parent;
- target = parents[parent];
- break;
- }
- }
- }
- } else {
- conflictingParent = editchange.target;
- }
+ _getChanges (cfg, changeType) {
+ const method = `get${changeType}s`;
+ return (cfg[method] && cfg[method](this.platform)) || [];
+ }
- if (target && target.length !== 0) {
- // conflict has been found
- conflictFound = true;
-
- if (editchange.id === 'config.xml') {
- if (target[0].id === 'config.xml') {
- // Keep track of config.xml/config.xml edit-config
conflicts
- mungeutil.deep_add(configxmlMunge,
editchange.file, conflictingParent, target[0]);
- } else {
- // Keep track of config.xml x plugin.xml
edit-config conflicts
- mungeutil.deep_add(conflictingMunge,
editchange.file, conflictingParent, target[0]);
- }
- } else {
- if (target[0].id === 'config.xml') {
- // plugin.xml cannot overwrite config.xml changes
even if --force is used
- conflictWithConfigxml = true;
- return;
- }
-
- if (force) {
- // Need to find all conflicts when --force is
used, track conflicting munges
- mungeutil.deep_add(conflictingMunge,
editchange.file, conflictingParent, target[0]);
- } else {
- // plugin cannot overwrite other plugin changes
without --force
- conflictingPlugin = target[0].plugin;
- }
- }
- }
- }
+ /** @private */
+ _is_conflicting (editchanges) {
+ const platform_config = this.platformJson.root;
+ const { files } = platform_config.config_munge;
+
+ const configConflicts = { files: {} }; // config.xml edit-config
conflicts
+ const pluginConflicts = { files: {} }; // plugin.xml edit-config
conflicts
+
+ const registerConflict = (file, selector) => {
+ const witness = files[file].parents[selector][0];
+ const conflictMunge = witness.id === 'config.xml' ?
configConflicts : pluginConflicts;
+ mungeutil.deep_add(conflictMunge, file, selector, witness);
+ };
+
+ editchanges.forEach(({ file, target }) => {
+ if (!files[file]) return;
+ const { parents: changesBySelector } = files[file];
+
+ const conflicts = changesBySelector[target] || [];
+ if (conflicts.length > 0) return registerConflict(file, target);
+
+ const targetFile = this.config_keeper.get(this.project_dir,
this.platform, file);
+
+ // For non-XML files (e.g. plist), the selector in
editchange.target uniquely identifies its target.
+ // Thus we already know that we have no conflict if we are not
dealing with an XML file here.
+ if (targetFile.type !== 'xml') return;
+
+ // For XML files, the selector does NOT uniquely identify its
target. So we resolve editchange.target
+ // and any existing selectors to their matched elements and
compare those for equality.
+ const resolveEditTarget =
xml_helpers.resolveParent(targetFile.data, target);
+ if (!resolveEditTarget) return;
+
+ const selector = Object.keys(changesBySelector).find(parent =>
+ resolveEditTarget ===
xml_helpers.resolveParent(targetFile.data, parent)
+ );
+ if (selector) return registerConflict(file, selector);
});
return {
- conflictFound,
- conflictingPlugin,
- conflictingMunge,
- configxmlMunge,
- conflictWithConfigxml
+ configConflicts,
+ pluginConflicts
};
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cordova.apache.org
For additional commands, e-mail: commits-h...@cordova.apache.org
