Repository: ambari Updated Branches: refs/heads/branch-2.4 167c6c9d7 -> 2a472bb41
AMBARI-18308. Ambari UI: Memory leak while adding and removing property. (jaimin) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/2a472bb4 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/2a472bb4 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/2a472bb4 Branch: refs/heads/branch-2.4 Commit: 2a472bb41d6f1da3a55586621e9815e59c772c51 Parents: 167c6c9 Author: Jaimin Jetly <[email protected]> Authored: Thu Sep 29 12:08:23 2016 -0700 Committer: Jaimin Jetly <[email protected]> Committed: Thu Sep 29 12:08:23 2016 -0700 ---------------------------------------------------------------------- .../configs/service_configs_by_category_view.js | 532 +++++++++++-------- .../service_configs_by_category_view_test.js | 7 +- 2 files changed, 311 insertions(+), 228 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/2a472bb4/ambari-web/app/views/common/configs/service_configs_by_category_view.js ---------------------------------------------------------------------- diff --git a/ambari-web/app/views/common/configs/service_configs_by_category_view.js b/ambari-web/app/views/common/configs/service_configs_by_category_view.js index ad1a2f9..0874872 100644 --- a/ambari-web/app/views/common/configs/service_configs_by_category_view.js +++ b/ambari-web/app/views/common/configs/service_configs_by_category_view.js @@ -52,27 +52,102 @@ App.ServiceConfigsByCategoryView = Em.View.extend(App.UserPref, App.ConfigOverri * and sorted by <code>displayType</code> and <code>index</code> * @type {App.ServiceConfigProperty[]} */ - categoryConfigs: function () { - // sort content type configs, sort the rest of configs based on index and then add content array at the end (as intended) - var categoryConfigs = this.get('categoryConfigsAll'), - contentOrderedArray = this.orderContentAtLast(categoryConfigs.filterProperty('displayType','content')), - contentFreeConfigs = categoryConfigs.filter(function(config) {return config.get('displayType')!=='content';}), - indexOrdered = this.sortByIndex(contentFreeConfigs); - - return indexOrdered.concat(contentOrderedArray).filterProperty('isVisible', true); - }.property('[email protected]').cacheable(), + categoryConfigs: [], /** - * This method provides all the properties which apply + * This is array of all the properties which apply * to this category, irrespective of visibility. This * is helpful in Oozie/Hive database configuration, where * MySQL etc. database options don't show up, because * they were not visible initially. * @type {App.ServiceConfigProperty[]} */ - categoryConfigsAll: function () { - return this.get('serviceConfigs').filterProperty('category', this.get('category.name')); - }.property('serviceConfigs.@each').cacheable(), + categoryConfigsAll: [], + + didInsertElement: function () { + var self = this; + this.setCategoryConfigs(); + this.setVisibleCategoryConfigs(); + var isCollapsed = this.calcIsCollapsed(); + this.set('category.isCollapsed', isCollapsed); + if (isCollapsed) { + this.$('.accordion-body').hide(); + } + else { + this.$('.accordion-body').show(); + } + $('#serviceConfig').tooltip({ + selector: '[data-toggle=tooltip]', + placement: 'top' + }); + this.filteredCategoryConfigs(); + Em.run.next(function () { + self.updateReadOnlyFlags(); + }); + }, + + willDestroyElement: function () { + $('[data-toggle=tooltip]').tooltip('destroy'); + }, + + setVisibleCategoryConfigsOnce: function () { + Em.run.once(this, 'addConfigToCategoryConfigs'); + }.observes('[email protected]'), + + setCategoryConfigs: function () { + var categoryConfigsAll = this.get('serviceConfigs').filterProperty('category', this.get('category.name')); + if (categoryConfigsAll && categoryConfigsAll.length) { + this.set('categoryConfigsAll',categoryConfigsAll); + } + }, + + setVisibleCategoryConfigs: function () { + var orderedCategoryConfigs = this.getOrderedCategoryConfigs(); + this.set('categoryConfigs', orderedCategoryConfigs); + }, + + /** + * This method is invoked when any change in visibility of items of `categoryConfigsAll` array happens and accordingly sets `categoryConfigs` array + * Instead of completely resetting `categoryConfigs` array whenever this function is invoked , items are added/removed in `categoryConfigs` (which is binded in template) + * Doing so is observed to avoid memory leak + */ + addConfigToCategoryConfigs: function () { + var orderedCategoryConfigs = this.getOrderedCategoryConfigs(); + var categoryConfigs = this.get('categoryConfigs'); + var configsToAdd = []; + var configsToRemove = []; + // If property is added or made visible then add it to visible categoryconfigs + orderedCategoryConfigs.forEach(function(item){ + var isPresent = categoryConfigs.filterProperty('name', item.get('name')).findProperty('filename',item.get('filename')); + if (!isPresent) { + configsToAdd.pushObject(item); + } + }, this); + + // If property is removed or made invisible, then remove it from visible categoryconfigs + categoryConfigs.forEach(function(item){ + var orderedCategoryConfig = orderedCategoryConfigs.filterProperty('name', item.get('name')).findProperty('filename',item.get('filename')); + if (!orderedCategoryConfig) { + configsToRemove.pushObject(item); + } + }, this); + + categoryConfigs.pushObjects(configsToAdd); + categoryConfigs.removeObjects(configsToRemove); + }, + + getOrderedCategoryConfigs: function() { + var categoryConfigsAll = this.get('categoryConfigsAll'); + var orderedCategoryConfigs = []; + if (categoryConfigsAll) { + var contentOrderedArray = this.orderContentAtLast(categoryConfigsAll.filterProperty('displayType','content')), + contentFreeConfigs = categoryConfigsAll.filter(function(config) {return config.get('displayType')!=='content';}), + indexOrdered = this.sortByIndex(contentFreeConfigs); + orderedCategoryConfigs = indexOrdered.concat(contentOrderedArray).filterProperty('isVisible', true); + } + return orderedCategoryConfigs; + }, + /** * If added/removed a serverConfigObject, this property got updated. @@ -304,30 +379,6 @@ App.ServiceConfigsByCategoryView = Em.View.extend(App.UserPref, App.ConfigOverri return Em.isNone(this.get('category.isCollapsed')) ? (this.get('category.name').indexOf('Advanced') != -1 || this.get('category.name').indexOf('CapacityScheduler') != -1 || this.get('category.name').indexOf('Custom') != -1) : this.get('category.isCollapsed'); }, - didInsertElement: function () { - var isCollapsed = this.calcIsCollapsed(); - var self = this; - this.set('category.isCollapsed', isCollapsed); - if (isCollapsed) { - this.$('.accordion-body').hide(); - } - else { - this.$('.accordion-body').show(); - } - $('#serviceConfig').tooltip({ - selector: '[data-toggle=tooltip]', - placement: 'top' - }); - this.filteredCategoryConfigs(); - Em.run.next(function () { - self.updateReadOnlyFlags(); - }); - }, - - willDestroyElement: function () { - $('[data-toggle=tooltip]').tooltip('destroy'); - }, - /** * @returns {string} */ @@ -358,221 +409,243 @@ App.ServiceConfigsByCategoryView = Em.View.extend(App.UserPref, App.ConfigOverri isNotSaved: true }, selectedConfigGroup); } - this.get('serviceConfigs').pushObject(App.ServiceConfigProperty.create(config)); + var serviceConfigProperty = App.ServiceConfigProperty.create(config); + this.get('serviceConfigs').pushObject(serviceConfigProperty); + this.get('categoryConfigsAll').pushObject(serviceConfigProperty); }, /** - * Show popup for adding new config-properties - * @method showAddPropertyWindow - */ - showAddPropertyWindow: function () { - var persistController = this; - var modePersistKey = this.persistKey(); - var selectedConfigGroup = this.get('controller.selectedConfigGroup'); - - persistController.getUserPref(modePersistKey).pipe(function (data) { - return !!data; - },function () { - return false; - }).always((function (isBulkMode) { - - var category = this.get('category'); - var siteFileName = category.get('siteFileName'); - - var service = this.get('service'); - var serviceName = service.get('serviceName'); - - var configsOfFile = service.get('configs').filterProperty('filename', siteFileName); - - /** - * Find duplications within the same confType - * Result in error, as no duplicated property keys are allowed in the same configType - * */ - function isDuplicatedConfigKey(name) { - return configsOfFile.findProperty('name', name); - } + * Find duplications within the same confType + * Result in error, as no duplicated property keys are allowed in the same configType + * */ + isDuplicatedConfigKey: function(name) { + var category = this.get('category'); + var siteFileName = category.get('siteFileName'); + + var service = this.get('service'); + var serviceName = service.get('serviceName'); + + var configsOfFile = service.get('configs').filterProperty('filename', siteFileName); + return configsOfFile.findProperty('name', name); + }, - /** - * find duplications in all confTypes of the service - * Result in warning, to remind user the existence of a same-named property - * */ - function isDuplicatedConfigKeyinConfigs(name) { - var files = []; - var configFiles = service.get('configs').mapProperty('filename').uniq(); - configFiles.forEach(function(configFile){ - var configsOfFile = service.get('configs').filterProperty('filename', configFile); - if(configsOfFile.findProperty('name', name)){ - files.push(configFile); - } - }, this); - return files; - } + /** + * find duplications in all confTypes of the service + * Result in warning, to remind user the existence of a same-named property + * */ + isDuplicatedConfigKeyinConfigs: function(name) { + var files = []; + var service = this.get('service'); + var configFiles = service.get('configs').mapProperty('filename').uniq(); + configFiles.forEach(function (configFile) { + var configsOfFile = service.get('configs').filterProperty('filename', configFile); + if (configsOfFile.findProperty('name', name)) { + files.push(configFile); + } + }, this); + return files; + }, - var serviceConfigObj = Ember.Object.create({ - isBulkMode: isBulkMode, - bulkConfigValue: '', - bulkConfigError: false, - bulkConfigErrorMessage: '', - - name: '', - value: '', - isKeyError: false, - showFilterLink: false, - errorMessage: '', - observeAddPropertyValue: function () { - var name = this.get('name'); - if (name.trim() != '') { - if (validator.isValidConfigKey(name)) { - if (!isDuplicatedConfigKey(name)) { //no duplication within the same confType - var files = isDuplicatedConfigKeyinConfigs(name); - if (files.length > 0) { - //still check for a warning, if there are duplications across confTypes - this.set('showFilterLink', true); - this.set('isKeyWarning', true); - this.set('isKeyError', false); - this.set('warningMessage', Em.I18n.t('services.service.config.addPropertyWindow.error.derivedKey.location').format(files.join(" "))); - } else { - this.set('showFilterLink', false); - this.set('isKeyError', false); - this.set('isKeyWarning', false); - this.set('errorMessage', ''); - } - } else { - this.set('showFilterLink', true); - this.set('isKeyError', true); - this.set('isKeyWarning', false); - this.set('errorMessage', Em.I18n.t('services.service.config.addPropertyWindow.error.derivedKey.infile')); - } + processAddPropertyWindow: function(isBulkMode, modePersistKey) { + var self = this; + var category = this.get('category'); + var siteFileName = category.get('siteFileName'); + + var service = this.get('service'); + var serviceName = service.get('serviceName'); + + var serviceConfigObj = Em.Object.create({ + isBulkMode: isBulkMode, + bulkConfigValue: '', + bulkConfigError: false, + bulkConfigErrorMessage: '', + + name: '', + value: '', + isKeyError: false, + showFilterLink: false, + errorMessage: '', + observeAddPropertyValue: function () { + var name = this.get('name'); + if (name.trim() != '') { + if (validator.isValidConfigKey(name)) { + if (!self.isDuplicatedConfigKey(name)) { //no duplication within the same confType + var files = self.isDuplicatedConfigKeyinConfigs(name); + if (files.length > 0) { + //still check for a warning, if there are duplications across confTypes + this.set('showFilterLink', true); + this.set('isKeyWarning', true); + this.set('isKeyError', false); + this.set('warningMessage', Em.I18n.t('services.service.config.addPropertyWindow.error.derivedKey.location').format(files.join(" "))); } else { this.set('showFilterLink', false); - this.set('isKeyError', true); + this.set('isKeyError', false); this.set('isKeyWarning', false); - this.set('errorMessage', Em.I18n.t('form.validator.configKey')); + this.set('errorMessage', ''); } } else { - this.set('showFilterLink', false); + this.set('showFilterLink', true); this.set('isKeyError', true); this.set('isKeyWarning', false); - this.set('errorMessage', Em.I18n.t('services.service.config.addPropertyWindow.error.required')); + this.set('errorMessage', Em.I18n.t('services.service.config.addPropertyWindow.error.derivedKey.infile')); } - }.observes('name') - }); - - function processConfig(config, callback) { - var lines = config.split('\n'); - var errorMessages = []; - var parsedConfig = {}; - var propertyCount = 0; - - function lineNumber(index) { - return Em.I18n.t('services.service.config.addPropertyWindow.error.lineNumber').format(index + 1); + } else { + this.set('showFilterLink', false); + this.set('isKeyError', true); + this.set('isKeyWarning', false); + this.set('errorMessage', Em.I18n.t('form.validator.configKey')); } + } else { + this.set('showFilterLink', false); + this.set('isKeyError', true); + this.set('isKeyWarning', false); + this.set('errorMessage', Em.I18n.t('services.service.config.addPropertyWindow.error.required')); + } + } + }); - lines.forEach(function (line, index) { - if (line.trim() === '') { - return; - } - var delimiter = '='; - var delimiterPosition = line.indexOf(delimiter); - if (delimiterPosition === -1) { - errorMessages.push(lineNumber(index) + Em.I18n.t('services.service.config.addPropertyWindow.error.format')); - return; - } - var key = Em.Handlebars.Utils.escapeExpression(line.slice(0, delimiterPosition).trim()); - var value = line.slice(delimiterPosition + 1); - if (validator.isValidConfigKey(key)) { - if (!isDuplicatedConfigKey(key) && !(key in parsedConfig)) { - parsedConfig[key] = value; - propertyCount++; - } else { - errorMessages.push(lineNumber(index) + Em.I18n.t('services.service.config.addPropertyWindow.error.derivedKey.specific').format(key)); - } + App.ModalPopup.show({ + classNames: ['sixty-percent-width-modal'], + header: Em.I18n.t('installer.step7.config.addProperty'), + primary: Em.I18n.t('add'), + secondary: Em.I18n.t('common.cancel'), + onPrimary: function () { + var propertyObj = { + filename: siteFileName, + serviceName: serviceName, + categoryName: category.get('name') + }; + if (serviceConfigObj.isBulkMode) { + var popup = this; + this.processConfig(serviceConfigObj.bulkConfigValue, function (error, parsedConfig) { + if (error) { + serviceConfigObj.set('bulkConfigError', true); + serviceConfigObj.set('bulkConfigErrorMessage', error); } else { - errorMessages.push(lineNumber(index) + Em.I18n.t('form.validator.configKey.specific').format(key)); + for (var key in parsedConfig) { + if (parsedConfig.hasOwnProperty(key)) { + propertyObj.name = key; + propertyObj.value = parsedConfig[key]; + self.createProperty(propertyObj); + } + } + popup.hide(); } }); - - if (errorMessages.length > 0) { - callback(errorMessages.join('<br>'), parsedConfig); + } else { + serviceConfigObj.observeAddPropertyValue(); + /** + * For the first entrance use this if (serviceConfigObj.name.trim() != '') + */ + if (!serviceConfigObj.isKeyError) { + propertyObj.name = serviceConfigObj.get('name'); + propertyObj.value = serviceConfigObj.get('value'); + self.createProperty(propertyObj); + this.hide(); } - else if (propertyCount === 0) { - callback(Em.I18n.t('services.service.config.addPropertyWindow.propertiesPlaceholder', parsedConfig)); + } + }, + + lineNumber: function(index) { + return Em.I18n.t('services.service.config.addPropertyWindow.error.lineNumber').format(index + 1); + }, + + processConfig: function(config, callback) { + var lines = config.split('\n'); + var errorMessages = []; + var parsedConfig = {}; + var propertyCount = 0; + lines.forEach(function (line, index) { + if (line.trim() === '') { + return; } - else { - callback(null, parsedConfig); + var delimiter = '='; + var delimiterPosition = line.indexOf(delimiter); + if (delimiterPosition === -1) { + errorMessages.push(this.lineNumber(index) + Em.I18n.t('services.service.config.addPropertyWindow.error.format')); + return; } - } - - App.ModalPopup.show({ - classNames: ['sixty-percent-width-modal'], - header: Em.I18n.t('installer.step7.config.addProperty'), - primary: Em.I18n.t('add'), - secondary: Em.I18n.t('common.cancel'), - onPrimary: function () { - var propertyObj = { - filename: siteFileName, - serviceName: serviceName, - categoryName: category.get('name') - }; - if (serviceConfigObj.isBulkMode) { - var popup = this; - processConfig(serviceConfigObj.bulkConfigValue, function (error, parsedConfig) { - if (error) { - serviceConfigObj.set('bulkConfigError', true); - serviceConfigObj.set('bulkConfigErrorMessage', error); - } else { - for (var key in parsedConfig) { - if (parsedConfig.hasOwnProperty(key)) { - propertyObj.name = key; - propertyObj.value = parsedConfig[key]; - persistController.createProperty(propertyObj); - } - } - popup.hide(); - } - }); - } - else { - serviceConfigObj.observeAddPropertyValue(); - /** - * For the first entrance use this if (serviceConfigObj.name.trim() != '') - */ - if (!serviceConfigObj.isKeyError) { - propertyObj.name = serviceConfigObj.get('name'); - propertyObj.value = serviceConfigObj.get('value'); - persistController.createProperty(propertyObj); - this.hide(); - } - } - }, - bodyClass: Em.View.extend({ - fileName: siteFileName, - notMisc: serviceName !== 'MISC', - templateName: require('templates/common/configs/addPropertyWindow'), - controllerBinding: 'App.router.mainServiceInfoConfigsController', - serviceConfigObj: serviceConfigObj, - didInsertElement: function () { - App.tooltip(this.$("[data-toggle=tooltip]"), { - placement: "top" - }); - }, - toggleBulkMode: function () { - this.toggleProperty('serviceConfigObj.isBulkMode'); - persistController.postUserPref(modePersistKey, this.get('serviceConfigObj.isBulkMode')); - }, - filterByKey: function (event) { - var controller = (App.router.get('currentState.name') == 'configs') - ? App.router.get('mainServiceInfoConfigsController') - : App.router.get('wizardStep7Controller'); - this.get('parentView').onClose(); - controller.set('filter', event.view.get('serviceConfigObj.name')); + var key = Em.Handlebars.Utils.escapeExpression(line.slice(0, delimiterPosition).trim()); + var value = line.slice(delimiterPosition + 1); + if (validator.isValidConfigKey(key)) { + if (!self.isDuplicatedConfigKey(key) && !(key in parsedConfig)) { + parsedConfig[key] = value; + propertyCount++; + } else { + errorMessages.push(this.lineNumber(index) + Em.I18n.t('services.service.config.addPropertyWindow.error.derivedKey.specific').format(key)); } - }) + } else { + errorMessages.push(this.lineNumber(index) + Em.I18n.t('form.validator.configKey.specific').format(key)); + } }); - }).bind(this)); + if (errorMessages.length > 0) { + callback(errorMessages.join('<br>'), parsedConfig); + } + else if (propertyCount === 0) { + callback(Em.I18n.t('services.service.config.addPropertyWindow.propertiesPlaceholder', parsedConfig)); + } + else { + callback(null, parsedConfig); + } + }, + willDestroyElement: function () { + serviceConfigObj.destroy(); + this._super(); + }, + bodyClass: Em.View.extend({ + fileName: siteFileName, + notMisc: serviceName !== 'MISC', + templateName: require('templates/common/configs/addPropertyWindow'), + serviceConfigObj: serviceConfigObj, + didInsertElement: function () { + this._super(); + serviceConfigObj.addObserver('name', serviceConfigObj, 'observeAddPropertyValue'); + App.tooltip(this.$("[data-toggle=tooltip]"), { + placement: "top" + }); + }, + willDestroyElement: function () { + this.$().popover('destroy'); + serviceConfigObj.removeObserver('name', serviceConfigObj, 'observeAddPropertyValue'); + this.set('serviceConfigObj', null); + this._super(); + }, + toggleBulkMode: function () { + this.toggleProperty('serviceConfigObj.isBulkMode'); + self.postUserPref(modePersistKey, this.get('serviceConfigObj.isBulkMode')); + }, + filterByKey: function (event) { + var controller = (App.router.get('currentState.name') == 'configs') + ? App.router.get('mainServiceInfoConfigsController') + : App.router.get('wizardStep7Controller'); + this.get('parentView').onClose(); + controller.set('filter', event.view.get('serviceConfigObj.name')); + } + }) + }); + }, + + /** + * Show popup for adding new config-properties + * @method showAddPropertyWindow + */ + showAddPropertyWindow: function () { + var persistController = this; + var modePersistKey = this.persistKey(); + var selectedConfigGroup = this.get('controller.selectedConfigGroup'); + + persistController.getUserPref(modePersistKey).then(function (data) { + return !!data; + },function () { + return false; + }).always((function (isBulkMode) { + persistController.processAddPropertyWindow(isBulkMode, modePersistKey); + })); }, + + /** * Toggle <code>isFinal</code> for selected config-property if <code>isNotEditable</code> is false * @param {object} event @@ -584,6 +657,7 @@ App.ServiceConfigsByCategoryView = Em.View.extend(App.UserPref, App.ConfigOverri return; } serviceConfigProperty.toggleProperty('isFinal'); + serviceConfigProperty = null; }, /** @@ -593,7 +667,6 @@ App.ServiceConfigsByCategoryView = Em.View.extend(App.UserPref, App.ConfigOverri */ removeProperty: function (event) { var serviceConfigProperty = event.contexts[0]; - this.get('serviceConfigs').removeObject(serviceConfigProperty); // push config's file name if this config was stored on server if (!serviceConfigProperty.get('isNotSaved')) { var modifiedFileNames = this.get('controller.modifiedFileNames'), @@ -609,6 +682,9 @@ App.ServiceConfigsByCategoryView = Em.View.extend(App.UserPref, App.ConfigOverri } } } + this.get('serviceConfigs').removeObject(serviceConfigProperty); + this.get('categoryConfigsAll').removeObject(serviceConfigProperty); + serviceConfigProperty = null; Em.$('body>.tooltip').remove(); //some tooltips get frozen when their owner's DOM element is removed }, @@ -620,6 +696,7 @@ App.ServiceConfigsByCategoryView = Em.View.extend(App.UserPref, App.ConfigOverri setRecommendedValue: function (event) { var serviceConfigProperty = event.contexts[0]; serviceConfigProperty.set('value', serviceConfigProperty.get('recommendedValue')); + serviceConfigProperty = null; }, /** @@ -644,6 +721,7 @@ App.ServiceConfigsByCategoryView = Em.View.extend(App.UserPref, App.ConfigOverri serviceConfigProperty.set('isFinal', savedIsFinal); } this.configChangeObserver(serviceConfigProperty); + serviceConfigProperty = null; Em.$('body>.tooltip').remove(); //some tooltips get frozen when their owner's DOM element is removed } http://git-wip-us.apache.org/repos/asf/ambari/blob/2a472bb4/ambari-web/test/views/common/configs/service_configs_by_category_view_test.js ---------------------------------------------------------------------- diff --git a/ambari-web/test/views/common/configs/service_configs_by_category_view_test.js b/ambari-web/test/views/common/configs/service_configs_by_category_view_test.js index 7b6c451..9b49653 100644 --- a/ambari-web/test/views/common/configs/service_configs_by_category_view_test.js +++ b/ambari-web/test/views/common/configs/service_configs_by_category_view_test.js @@ -308,9 +308,14 @@ describe('App.ServiceConfigsByCategoryView', function () { category: { name: item.categoryNname }, - serviceConfigs: item.serviceConfigs + serviceConfigs: item.serviceConfigs, + filteredCategoryConfigs: Em.K, + collapseCategory: Em.K }); + view.setCategoryConfigs(); + view.setVisibleCategoryConfigs(); expect(view.get('categoryConfigs').mapProperty('resultId')).to.deep.equal(result); + view.destroy(); }); }); });
