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
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 @@
             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 @@
         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 @@
     }
 
     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 (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.`);
         }
 
-        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);
-            }
-        }
-
-        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 @@
 
     // Handle edit-config changes from config.xml
     add_config_changes (config, should_increment) {
-        const platform_config = this.platformJson.root;
-        let changes = [];
+        const changes = [
+            ...this._getChanges(config, 'EditConfig'),
+            ...this._getChanges(config, 'ConfigFile')
+        ];
 
-        if (config.getEditConfigs) {
-            const edit_config_changes = config.getEditConfigs(this.platform);
-            if (edit_config_changes) {
-                changes = changes.concat(edit_config_changes);
-            }
+        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 (config.getConfigFiles) {
-            const config_files_changes = config.getConfigFiles(this.platform);
-            if (config_files_changes) {
-                changes = changes.concat(config_files_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 @@
         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 @@
     }
 
     /** @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;
+    _getChanges (cfg, changeType) {
+        const method = `get${changeType}s`;
+        return (cfg[method] && cfg[method](this.platform)) || [];
+    }
 
-        editchanges.forEach(editchange => {
-            if (files[editchange.file]) {
-                const parents = files[editchange.file].parents;
-                let target = parents[editchange.target];
+    /** @private */
+    _is_conflicting (editchanges) {
+        const platform_config = this.platformJson.root;
+        const { files } = platform_config.config_munge;
 
-                // 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);
+        const configConflicts = { files: {} }; // config.xml edit-config conflicts
+        const pluginConflicts = { files: {} }; // plugin.xml edit-config conflicts
 
-                    // 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;
+        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);
+        };
 
-                    // 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;
-                }
+        editchanges.forEach(({ file, target }) => {
+            if (!files[file]) return;
+            const { parents: changesBySelector } = files[file];
 
-                if (target && target.length !== 0) {
-                    // conflict has been found
-                    conflictFound = true;
+            const conflicts = changesBySelector[target] || [];
+            if (conflicts.length > 0) return registerConflict(file, target);
 
-                    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;
-                        }
+            const targetFile = this.config_keeper.get(this.project_dir, this.platform, file);
 
-                        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;
-                        }
-                    }
-                }
-            }
+            // 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
         };
     }