From ff5f74d5d0d5ef8f7c07ca8762b7ad197aea156f Mon Sep 17 00:00:00 2001 From: mareklycka Date: Fri, 1 Mar 2019 13:00:07 +0100 Subject: [PATCH] Fixes double loading in Image source loading This patch removes a redundant image API call, which caused images to be loaded twice in the UI. Image filtering for snapshot/'normal' images has been merged into a single iteration for the sake of optimization. Additionally, tests have been changed to more comprehensively check which data is loaded into which data structures during model population. Code redundancy has also been removed in tests. Change-Id: Ie78900dae54ff63d41b7ddca3fa83f627d4260c4 Closes-Bug: #1818508 --- .../launch-instance-model.service.js | 66 ++++++------- .../launch-instance-model.service.spec.js | 97 +++++++++---------- 2 files changed, 77 insertions(+), 86 deletions(-) diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/workflow/launch-instance/launch-instance-model.service.js b/openstack_dashboard/dashboards/project/static/dashboard/project/workflow/launch-instance/launch-instance-model.service.js index b16fd10709..f3e3a9f7d6 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/workflow/launch-instance/launch-instance-model.service.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/workflow/launch-instance/launch-instance-model.service.js @@ -565,21 +565,11 @@ if (enabledImage || enabledSnapshot) { var filter = {status: 'active', sort_key: 'name', sort_dir: 'asc'}; - var filterCommunity = angular.merge({}, filter, {visibility: 'community'}); - - var imagePromises = [ - glanceAPI.getImages(filter), - glanceAPI.getImages(filterCommunity) - ]; - - $q.all(imagePromises).then(function getEnabledImages(data) { - if (enabledImage) { - onGetImages(data); + glanceAPI.getImages(filter).then( + function(data) { + onGetImageSources(data, enabledImage, enabledSnapshot); } - if (enabledSnapshot) { - onGetSnapshots(data); - } - }); + ); } } @@ -673,29 +663,39 @@ 'image'; } - function onGetImages(data) { - model.images.length = 0; - angular.forEach(data, function addData(data) { - push.apply(model.images, data.data.items.filter(function (image) { - return isBootableImageType(image) && - (!image.properties || image.properties.image_type !== 'snapshot'); - })); - }); - addAllowedBootSource(model.images, bootSourceTypes.IMAGE, gettext('Image')); + function isValidImage(image) { + return isBootableImageType(image) && + (!image.properties || image.properties.image_type !== 'snapshot'); } - function onGetSnapshots(data) { + function isValidSnapshot(image) { + return getImageType(image) === 'snapshot' && isBootableImageType(image); + } + + function onGetImageSources(data, enabledImage, enabledSnapshot) { model.imageSnapshots.length = 0; - angular.forEach(data, function addData(data) { - push.apply(model.imageSnapshots, data.data.items.filter(function (image) { - return isBootableImageType(image) && getImageType(image) === 'snapshot'; - })); + model.images.length = 0; + + angular.forEach(data.data.items, function(image) { + if (isValidSnapshot(image) && enabledSnapshot) { + model.imageSnapshots.push(image); + } else if (isValidImage(image) && enabledImage) { + model.images.push(image); + } }); - addAllowedBootSource( - model.imageSnapshots, - bootSourceTypes.INSTANCE_SNAPSHOT, - gettext('Instance Snapshot') - ); + + if (enabledImage) { + addAllowedBootSource( + model.images, bootSourceTypes.IMAGE, gettext('Image') + ); + } + + if (enabledSnapshot) { + addAllowedBootSource( + model.imageSnapshots, bootSourceTypes.INSTANCE_SNAPSHOT, + gettext('Instance Snapshot') + ); + } } function onGetVolumes(data) { diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/workflow/launch-instance/launch-instance-model.service.spec.js b/openstack_dashboard/dashboards/project/static/dashboard/project/workflow/launch-instance/launch-instance-model.service.spec.js index 8b426a6bc0..ac99cc0585 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/workflow/launch-instance/launch-instance-model.service.spec.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/workflow/launch-instance/launch-instance-model.service.spec.js @@ -177,51 +177,6 @@ VOLUME_SNAPSHOT = {type: 'volume_snapshot', label: 'Volume Snapshot'}; INSTANCE_SNAPSHOT = {type: 'snapshot', label: 'Instance Snapshot'}; }); - - $provide.value('horizon.app.core.openstack-service-api.nova', novaApi); - })); - - beforeEach(module('horizon.dashboard.project.workflow.launch-instance')); - - beforeEach(module(function($provide) { - $provide.value('horizon.app.core.openstack-service-api.glance', { - getImages: function() { - var images = [ - {container_format: 'aki', properties: {} }, - {container_format: 'ari', properties: {} }, - {container_format: 'ami', properties: {} }, - {container_format: 'raw', properties: {} }, - {container_format: 'ami', properties: {image_type: 'image'}}, - {container_format: 'raw', properties: {image_type: 'image'}}, - {container_format: 'ami', properties: { - block_device_mapping: '[{"source_type": "snapshot"}]'}}, - {container_format: 'raw', properties: { - block_device_mapping: '[{"source_type": "snapshot"}]'}} - ]; - - var deferred = $q.defer(); - deferred.resolve({ data: { items: images } }); - - return deferred.promise; - }, - getNamespaces: function() { - var namespaces = [ 'ns-1', 'ns-2' ]; - - var deferred = $q.defer(); - deferred.resolve({ data: { items: namespaces } }); - - return deferred.promise; - } - }); - - beforeEach(function() { - settings = { - LAUNCH_INSTANCE_DEFAULTS: { - config_drive: false - } - }; - }); - $provide.value('horizon.app.core.openstack-service-api.nova', novaApi); $provide.value('horizon.app.core.openstack-service-api.security-group', securityGroupApi); @@ -430,14 +385,50 @@ expect(model.initialized).toBe(true); expect(model.newInstanceSpec).toBeDefined(); - expect(model.images.length).toBe(12); - expect(model.imageSnapshots.length).toBe(4); - expect(model.availabilityZones.length).toBe(3); // 2 + 1 for 'nova pick' - expect(model.flavors.length).toBe(2); - expect(model.keypairs.length).toBe(2); - expect(model.securityGroups.length).toBe(2); - expect(model.novaLimits.maxTotalInstances).toBe(10); - expect(model.novaLimits.totalInstancesUsed).toBe(0); + var expectedImages = [ + {container_format: 'ami', properties: {}}, + {container_format: 'raw', properties: {}}, + {container_format: 'ami', properties: {image_type: 'image'}}, + {container_format: 'raw', properties: {image_type: 'image'}} + ]; + expect(model.images).toEqual(expectedImages); + + var expectedSnapshots = [ + { + container_format: 'ami', + properties: {block_device_mapping: '[{"source_type": "snapshot"}]'} + }, + { + container_format: 'raw', + properties: {block_device_mapping: '[{"source_type": "snapshot"}]'} + } + ]; + expect(model.imageSnapshots).toEqual(expectedSnapshots); + + var expectedZones = [ + {'label': 'Any Availability Zone', 'value': ''}, + {'label': 'zone-1', 'value': 'zone-1'}, + {'label': 'zone-2', 'value': 'zone-2'} + ]; + expect(model.availabilityZones).toEqual(expectedZones); + + var expectedFlavors = ['flavor-1', 'flavor-2']; + expect(model.flavors).toEqual(expectedFlavors); + + var expectedKeypairs = [ + {'name': 'key-1', id: 'li_keypair:key-1'}, + {'name': 'key-2', id: 'li_keypair:key-2'} + ]; + expect(model.keypairs).toEqual(expectedKeypairs); + + var expectedSecurityGroups = [ + {name: 'security-group-1'}, + {name: 'security-group-2'} + ]; + expect(model.securityGroups).toEqual(expectedSecurityGroups); + + var expectedLimits = {maxTotalInstances: 10, totalInstancesUsed: 0}; + expect(model.novaLimits).toEqual(expectedLimits); }); it('should have networks & no volumes if neutron enabled & cinder disabled', function() {