From 053d0a669ef6d339f1ab5a2cfcaf23fb5fdbb6f3 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Wed, 22 Jun 2016 14:22:14 +1000 Subject: [PATCH] Implement file update (edit) in Swift UI The ability to update (edit contents of) an object was never present in the previous Swift UI. It was explicitly blocked due to code in the swift_upload_object() function, which has been removed in this patch. To replace that "upload would replace existing contents" check, this patch implements a client-side check to warn the user if the upload would do so. Partially-Implements: blueprint swift-ui-functionality Change-Id: I9fb57dda59322907f0661372f9ee223551ff8a6e --- openstack_dashboard/api/rest/swift.py | 32 ++- openstack_dashboard/api/swift.py | 2 - .../containers/edit-object-controller.js | 48 +++++ .../containers/edit-object-controller.spec.js | 55 +++++ .../project/containers/edit-object-modal.html | 40 ++++ .../containers/file-change-directive.js | 25 +++ .../object-name-exists.directive.js | 74 +++++++ .../object-name-exists.directive.spec.js | 69 +++++++ .../objects-batch-actions.service.js | 2 +- .../containers/objects-row-actions.service.js | 189 +++++++++++++----- .../objects-row-actions.service.spec.js | 93 ++++++++- .../containers/objects.controller.spec.js | 39 ++++ .../containers/upload-object-controller.js | 20 +- .../upload-object-controller.spec.js | 27 +-- .../containers/upload-object-modal.html | 18 +- .../openstack-service-api/swift.service.js | 35 ++-- .../swift.service.spec.js | 13 -- .../test/api_tests/swift_tests.py | 30 +-- 18 files changed, 647 insertions(+), 164 deletions(-) create mode 100644 openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-controller.js create mode 100644 openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-controller.spec.js create mode 100644 openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-modal.html create mode 100644 openstack_dashboard/dashboards/project/static/dashboard/project/containers/object-name-exists.directive.js create mode 100644 openstack_dashboard/dashboards/project/static/dashboard/project/containers/object-name-exists.directive.spec.js diff --git a/openstack_dashboard/api/rest/swift.py b/openstack_dashboard/api/rest/swift.py index d0fbf571e2..1dd4ee1580 100644 --- a/openstack_dashboard/api/rest/swift.py +++ b/openstack_dashboard/api/rest/swift.py @@ -154,7 +154,7 @@ class Object(generic.View): # note: not an AJAX request - the body will be raw file content @csrf_exempt def post(self, request, container, object_name): - """Create a new object or pseudo-folder + """Create or replace an object or pseudo-folder :param request: :param container: @@ -176,23 +176,19 @@ class Object(generic.View): data = form.clean() - try: - if object_name[-1] == '/': - result = api.swift.swift_create_pseudo_folder( - request, - container, - object_name - ) - else: - result = api.swift.swift_upload_object( - request, - container, - object_name, - data['file'] - ) - except exceptions.AlreadyExists as e: - # 409 Conflict - return rest_utils.JSONResponse(str(e), 409) + if object_name[-1] == '/': + result = api.swift.swift_create_pseudo_folder( + request, + container, + object_name + ) + else: + result = api.swift.swift_upload_object( + request, + container, + object_name, + data['file'] + ) return rest_utils.CreatedResponse( u'/api/swift/containers/%s/object/%s' % (container, result.name) diff --git a/openstack_dashboard/api/swift.py b/openstack_dashboard/api/swift.py index 49dd46a84b..53c9c6ed9a 100644 --- a/openstack_dashboard/api/swift.py +++ b/openstack_dashboard/api/swift.py @@ -272,8 +272,6 @@ def swift_copy_object(request, orig_container_name, orig_object_name, def swift_upload_object(request, container_name, object_name, object_file=None): - if swift_object_exists(request, container_name, object_name): - raise exceptions.AlreadyExists(object_name, 'object') headers = {} size = 0 if object_file: diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-controller.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-controller.js new file mode 100644 index 0000000000..d71250ae17 --- /dev/null +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-controller.js @@ -0,0 +1,48 @@ +/* + * (c) Copyright 2015 Rackspace US, Inc + * + * Licensed under the Apache License, Version 2.0 (the 'License'); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an 'AS IS' BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +(function () { + 'use strict'; + + angular + .module('horizon.dashboard.project.containers') + .controller( + 'horizon.dashboard.project.containers.EditObjectModalController', + EditObjectModalController + ); + + EditObjectModalController.$inject = ['fileDetails']; + + function EditObjectModalController(fileDetails) { + var ctrl = this; + + ctrl.model = { + container: fileDetails.container, + path: fileDetails.path, + view_file: null, // file object managed by angular form ngModel + edit_file: null // file object from the DOM element with the actual upload + }; + ctrl.changeFile = changeFile; + + /////////// + + function changeFile(files) { + if (files.length) { + // record the file selected for upload for use in the action that invoked this modal + ctrl.model.edit_file = files[0]; + } + } + } +})(); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-controller.spec.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-controller.spec.js new file mode 100644 index 0000000000..0d0b031708 --- /dev/null +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-controller.spec.js @@ -0,0 +1,55 @@ +/** + * (c) Copyright 2016 Rackspace US, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. You may obtain + * a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +(function() { + 'use strict'; + + describe('horizon.dashboard.project.containers edit-object controller', function() { + var controller, ctrl; + + beforeEach(module('horizon.framework')); + beforeEach(module('horizon.dashboard.project.containers')); + + beforeEach(module(function ($provide) { + $provide.value('fileDetails', { + container: 'spam', + path: 'ham/eggs' + }); + })); + + beforeEach(inject(function ($injector) { + controller = $injector.get('$controller'); + ctrl = controller('horizon.dashboard.project.containers.EditObjectModalController'); + })); + + it('should initialise the controller model when created', function test() { + expect(ctrl.model.path).toEqual('ham/eggs'); + expect(ctrl.model.container).toEqual('spam'); + }); + + it('should respond to file changes correctly', function test() { + var file = {name: 'eggs'}; + ctrl.changeFile([file]); + expect(ctrl.model.edit_file).toEqual(file); + }); + + it('should not respond to file changes if no files are present', function test() { + ctrl.model.edit_file = 'spam'; + ctrl.changeFile([]); + expect(ctrl.model.edit_file).toEqual('spam'); + }); + }); +})(); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-modal.html b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-modal.html new file mode 100644 index 0000000000..1b8ee65a08 --- /dev/null +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/edit-object-modal.html @@ -0,0 +1,40 @@ +
+ +
+ Edit File: {$ ctrl.model.container $} : {$ ctrl.model.path $} +
+
+ +
+
+
+
+
+
+ +
+ +
+
+
+
+
+
+ +
+ + +
+
diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/file-change-directive.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/file-change-directive.js index 67318716a8..1e1f5f04a8 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/file-change-directive.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/file-change-directive.js @@ -16,6 +16,31 @@ (function () { 'use strict'; + /** + * @ngdoc directive + * @name on-file-change + * @element + * @description + * The `on-file-change` directive watches a file input and fires + * a callback when the file input is changed. + * + * The callback will be passed the "files" property from the + * browser event. + * + * @example + * ``` + * + * + * + * function changeFile(files) { + * if (files.length) { + * // update the upload file & its name + * ctrl.upload_file = files[0]; + * ctrl.file_name = files[0].name; + * } + * } + * ``` + */ angular .module('horizon.dashboard.project.containers') .directive('onFileChange', OnFileChange); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/object-name-exists.directive.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/object-name-exists.directive.js new file mode 100644 index 0000000000..af79d8db7a --- /dev/null +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/object-name-exists.directive.js @@ -0,0 +1,74 @@ +/* + * (c) Copyright 2015 Rackspace US, Inc + * + * Licensed under the Apache License, Version 2.0 (the 'License'); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an 'AS IS' BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +(function () { + 'use strict'; + + /** + * @ngdoc directive + * @name object-name-exists + * @element + * @description + * The `object-name-exists` directive is used on an angular form + * element to verify whether a Swift object name in the current context + * already exists or not. The current context (container name and + * folder) is taken from the container model service. + * + * If the name is taken, the ngModel will have $error.exists set + * (and all the other usual validation properties). + * + * Additionally since the check is asynchronous the ngModel + * will also have $pending.exists set while the lookup is being + * performed. + * + * @example + * ``` + * + * Checking if this name is used... + * This name already exists! + * ``` + */ + angular + .module('horizon.dashboard.project.containers') + .directive('objectNameExists', ObjectNameExists); + + ObjectNameExists.$inject = [ + 'horizon.app.core.openstack-service-api.swift', + 'horizon.dashboard.project.containers.containers-model', + '$q' + ]; + + function ObjectNameExists(swiftAPI, model, $q) { + return { + restrict: 'A', + require: 'ngModel', + link: function(scope, element, attrs, ngModel) { + ngModel.$asyncValidators.exists = function exists(modelValue) { + if (ngModel.$isEmpty(modelValue)) { + // consider empty model valid + return $q.when(); + } + + var def = $q.defer(); + // reverse the sense here - successful lookup == error + swiftAPI + .getObjectDetails(model.container.name, model.fullPath(modelValue), true) + .then(def.reject, def.resolve); + return def.promise; + }; + } + }; + } +})(); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/object-name-exists.directive.spec.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/object-name-exists.directive.spec.js new file mode 100644 index 0000000000..d2667aa27e --- /dev/null +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/object-name-exists.directive.spec.js @@ -0,0 +1,69 @@ +/* + * (c) Copyright 2016 Rackspace US, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +(function() { + 'use strict'; + + describe('horizon.dashboard.project.containers model', function() { + beforeEach(module('horizon.app.core.openstack-service-api')); + beforeEach(module('horizon.dashboard.project.containers')); + beforeEach(module('horizon.framework')); + + var $compile, $scope, model, element, swiftAPI, apiDeferred; + + beforeEach(inject(function inject($injector, _$q_, _$rootScope_) { + $scope = _$rootScope_.$new(); + $compile = $injector.get('$compile'); + model = $injector.get('horizon.dashboard.project.containers.containers-model'); + swiftAPI = $injector.get('horizon.app.core.openstack-service-api.swift'); + apiDeferred = _$q_.defer(); + spyOn(swiftAPI, 'getObjectDetails').and.returnValue(apiDeferred.promise); + model.container = {name: 'spam'}; + model.folder = 'ham'; + $scope.model = ''; + element = angular.element( + '
' + + '' + + 'EXISTS' + + '
' + ); + element = $compile(element)($scope); + $scope.$apply(); + })); + + it('should reject names that exist', function test() { + // edit the field + element.find('input').val('exists.txt').trigger('input'); + expect(swiftAPI.getObjectDetails).toHaveBeenCalledWith('spam', 'ham/exists.txt', true); + + // cause the lookup to complete successfully (file exists) + apiDeferred.resolve(); + $scope.$apply(); + + expect(element.find('span').hasClass('ng-hide')).toEqual(false); + }); + + it('should accept names that do not exist', function test() { + // edit the field + element.find('input').val('not-exists.txt').trigger('input'); + expect(swiftAPI.getObjectDetails).toHaveBeenCalledWith('spam', 'ham/not-exists.txt', true); + + // cause the lookup to complete successfully (file exists) + apiDeferred.resolve(); + $scope.$apply(); + expect(element.find('span').hasClass('ng-hide')).toEqual(false); + }); + }); +})(); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-batch-actions.service.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-batch-actions.service.js index eb6f55be38..008403c07c 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-batch-actions.service.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-batch-actions.service.js @@ -68,7 +68,7 @@ function uploadModal(html, $modal) { var localSpec = { backdrop: 'static', - controller: 'UploadObjectModalController as ctrl', + controller: 'horizon.dashboard.project.containers.UploadObjectModalController as ctrl', templateUrl: html }; return $modal.open(localSpec).result; diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-row-actions.service.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-row-actions.service.js index d127778889..5976c98da0 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-row-actions.service.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-row-actions.service.js @@ -22,25 +22,25 @@ .factory('horizon.dashboard.project.containers.objects-row-actions', rowActions) .factory('horizon.dashboard.project.containers.objects-actions.delete', deleteService) .factory('horizon.dashboard.project.containers.objects-actions.download', downloadService) + .factory('horizon.dashboard.project.containers.objects-actions.edit', editService) .factory('horizon.dashboard.project.containers.objects-actions.view', viewService); rowActions.$inject = [ - 'horizon.dashboard.project.containers.basePath', 'horizon.dashboard.project.containers.objects-actions.delete', 'horizon.dashboard.project.containers.objects-actions.download', + 'horizon.dashboard.project.containers.objects-actions.edit', 'horizon.dashboard.project.containers.objects-actions.view', 'horizon.framework.util.i18n.gettext' ]; - /** * @ngdoc factory * @name horizon.app.core.images.table.row-actions.service * @description A list of row actions. */ function rowActions( - basePath, deleteService, downloadService, + editService, viewService, gettext ) { @@ -56,6 +56,10 @@ service: downloadService, template: {text: gettext('Download')} }, + { + service: editService, + template: {text: gettext('Edit')} + }, { service: viewService, template: {text: gettext('View Details')} @@ -75,13 +79,19 @@ function downloadService($qExtensions, $window) { return { - allowed: function allowed(file) { return $qExtensions.booleanAsPromise(file.is_object); }, - // remove leading url slash to ensure uses relative link/base path - // thus using webroot. - perform: function perform(file) { - $window.location.href = file.url.replace(/^\//, ''); - } + allowed: allowed, + perform: perform }; + + function allowed(file) { + return $qExtensions.booleanAsPromise(file.is_object); + } + + // remove leading url slash to ensure uses relative link/base path + // thus using webroot. + function perform(file) { + $window.location.href = file.url.replace(/^\//, ''); + } } viewService.$inject = [ @@ -94,30 +104,99 @@ function viewService(swiftAPI, basePath, model, $qExtensions, $modal) { return { - allowed: function allowed(file) { - return $qExtensions.booleanAsPromise(file.is_object); - }, - perform: function perform(file) { - var objectPromise = swiftAPI.getObjectDetails( - model.container.name, - model.fullPath(file.name) - ).then( - function received(response) { - return response.data; - } - ); - var localSpec = { - backdrop: 'static', - controller: 'SimpleModalController as ctrl', - templateUrl: basePath + 'object-details-modal.html', - resolve: { - context: function context() { return objectPromise; } - } - }; - - $modal.open(localSpec); - } + allowed: allowed, + perform: perform }; + + function allowed(file) { + return $qExtensions.booleanAsPromise(file.is_object); + } + + function perform(file) { + var objectPromise = swiftAPI.getObjectDetails( + model.container.name, + model.fullPath(file.name) + ).then( + function received(response) { + return response.data; + } + ); + var localSpec = { + backdrop: 'static', + controller: 'SimpleModalController as ctrl', + templateUrl: basePath + 'object-details-modal.html', + resolve: { + context: function context() { return objectPromise; } + } + }; + + $modal.open(localSpec); + } + } + + editService.$inject = [ + 'horizon.app.core.openstack-service-api.swift', + 'horizon.dashboard.project.containers.basePath', + 'horizon.dashboard.project.containers.containers-model', + 'horizon.framework.util.q.extensions', + 'horizon.framework.widgets.modal-wait-spinner.service', + 'horizon.framework.widgets.toast.service', + '$modal' + ]; + + function editService(swiftAPI, basePath, model, $qExtensions, modalWaitSpinnerService, + toastService, $modal) { + return { + allowed: allowed, + perform: perform + }; + + function allowed(file) { + return $qExtensions.booleanAsPromise(file.is_object); + } + + function perform(file) { + var localSpec = { + backdrop: 'static', + controller: 'horizon.dashboard.project.containers.EditObjectModalController as ctrl', + templateUrl: basePath + 'edit-object-modal.html', + resolve: { + fileDetails: function fileDetails() { + return { + path: file.path, + container: model.container.name + }; + } + } + }; + return $modal.open(localSpec).result.then(editObjectCallback); + } + + function editObjectCallback(uploadInfo) { + modalWaitSpinnerService.showModalSpinner(gettext("Uploading")); + swiftAPI.uploadObject( + model.container.name, + uploadInfo.path, + uploadInfo.edit_file + ).then(success, error); + + function success() { + modalWaitSpinnerService.hideModalSpinner(); + toastService.add( + 'success', + interpolate(gettext('File %(path)s uploaded.'), uploadInfo, true) + ); + model.updateContainer(); + model.selectContainer( + model.container.name, + model.folder + ); + } + + function error() { + modalWaitSpinnerService.hideModalSpinner(); + } + } } deleteService.$inject = [ @@ -129,27 +208,31 @@ function deleteService(basePath, actionResultService, $qExtensions, $modal) { return { - allowed: function allowed() { - return $qExtensions.booleanAsPromise(true); - }, - perform: function perform(file) { - var localSpec = { - backdrop: 'static', - controller: 'DeleteObjectsModalController as ctrl', - templateUrl: basePath + 'delete-objects-modal.html', - resolve: { - selected: function () { - return [file]; - } - } - }; - - return $modal.open(localSpec).result.then(function finished() { - return actionResultService.getActionResult().deleted( - 'OS::Swift::Object', file.name - ).result; - }); - } + allowed: allowed, + perform: perform }; + + function allowed() { + return $qExtensions.booleanAsPromise(true); + } + + function perform(file) { + var localSpec = { + backdrop: 'static', + controller: 'DeleteObjectsModalController as ctrl', + templateUrl: basePath + 'delete-objects-modal.html', + resolve: { + selected: function () { + return [file]; + } + } + }; + + return $modal.open(localSpec).result.then(function finished() { + return actionResultService.getActionResult().deleted( + 'OS::Swift::Object', file.name + ).result; + }); + } } })(); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-row-actions.service.spec.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-row-actions.service.spec.js index 93509007f6..ad2591329c 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-row-actions.service.spec.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects-row-actions.service.spec.js @@ -41,7 +41,7 @@ it('should create an actions list', function test() { expect(rowActions.actions).toBeDefined(); var actions = rowActions.actions(); - expect(actions.length).toEqual(3); + expect(actions.length).toEqual(4); angular.forEach(actions, function check(action) { expect(action.service).toBeDefined(); expect(action.template).toBeDefined(); @@ -171,6 +171,97 @@ }); }); + describe('editService', function test() { + var swiftAPI, editService, modalWaitSpinnerService, toastService, $q; + + beforeEach(inject(function inject($injector, _$q_) { + swiftAPI = $injector.get('horizon.app.core.openstack-service-api.swift'); + editService = $injector.get('horizon.dashboard.project.containers.objects-actions.edit'); + modalWaitSpinnerService = $injector.get( + 'horizon.framework.widgets.modal-wait-spinner.service' + ); + toastService = $injector.get('horizon.framework.widgets.toast.service'); + $q = _$q_; + })); + + it('should have an allowed and perform', function test() { + expect(editService.allowed).toBeDefined(); + expect(editService.perform).toBeDefined(); + }); + + it('should only allow files', function test() { + expectAllowed(editService.allowed({is_object: true})); + }); + + it('should only now allow folders', function test() { + expectNotAllowed(editService.allowed({is_object: false})); + }); + + it('should handle upload success correctly', function() { + var modalDeferred = $q.defer(); + var apiDeferred = $q.defer(); + var result = { result: modalDeferred.promise }; + spyOn($modal, 'open').and.returnValue(result); + spyOn(modalWaitSpinnerService, 'showModalSpinner'); + spyOn(modalWaitSpinnerService, 'hideModalSpinner'); + spyOn(swiftAPI, 'uploadObject').and.returnValue(apiDeferred.promise); + spyOn(toastService, 'add').and.callThrough(); + spyOn(model,'updateContainer'); + spyOn(model,'selectContainer'); + + editService.perform(); + model.container = {name: 'spam'}; + $rootScope.$apply(); + + // Close the modal, make sure API call succeeds + modalDeferred.resolve({name: 'ham', path: '/folder/ham'}); + apiDeferred.resolve(); + $rootScope.$apply(); + + // Check the string of functions called by this code path succeed + expect($modal.open).toHaveBeenCalled(); + expect(modalWaitSpinnerService.showModalSpinner).toHaveBeenCalled(); + expect(swiftAPI.uploadObject).toHaveBeenCalled(); + expect(toastService.add).toHaveBeenCalledWith('success', 'File /folder/ham uploaded.'); + expect(modalWaitSpinnerService.hideModalSpinner).toHaveBeenCalled(); + expect(model.updateContainer).toHaveBeenCalled(); + expect(model.selectContainer).toHaveBeenCalled(); + }); + + it('should handle upload error correctly', function() { + var modalDeferred = $q.defer(); + var apiDeferred = $q.defer(); + var result = { result: modalDeferred.promise }; + spyOn($modal, 'open').and.returnValue(result); + spyOn(modalWaitSpinnerService, 'showModalSpinner'); + spyOn(modalWaitSpinnerService, 'hideModalSpinner'); + spyOn(swiftAPI, 'uploadObject').and.returnValue(apiDeferred.promise); + spyOn(toastService, 'add').and.callThrough(); + spyOn(model,'updateContainer'); + spyOn(model,'selectContainer'); + + editService.perform(); + model.container = {name: 'spam'}; + $rootScope.$apply(); + + // Close the modal, make sure API call is rejected + modalDeferred.resolve({name: 'ham', path: '/'}); + apiDeferred.reject(); + $rootScope.$apply(); + + // Check the string of functions called by this code path succeed + expect(modalWaitSpinnerService.showModalSpinner).toHaveBeenCalled(); + expect(swiftAPI.uploadObject).toHaveBeenCalled(); + expect(modalWaitSpinnerService.hideModalSpinner).toHaveBeenCalled(); + expect($modal.open).toHaveBeenCalled(); + + // Check the success branch is not called + expect(model.updateContainer).not.toHaveBeenCalled(); + expect(model.selectContainer).not.toHaveBeenCalled(); + expect(toastService.add).not.toHaveBeenCalledWith('success'); + }); + }); + function exerciseAllowedPromise(promise) { var handler = jasmine.createSpyObj('handler', ['success', 'error']); promise.then(handler.success, handler.error); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects.controller.spec.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects.controller.spec.js index 2e4c5ab357..f1a4bc8b0c 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects.controller.spec.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/objects.controller.spec.js @@ -91,5 +91,44 @@ expect(model.selectContainer).toHaveBeenCalledWith('spam', 'ham'); }); + + it('should handle action results when result is undefined', function test() { + var ctrl = createController(); + + spyOn(model, 'updateContainer'); + spyOn($scope, '$broadcast'); + ctrl.actionResultHandler(); + + expect($scope.$broadcast).not.toHaveBeenCalled(); + expect(model.updateContainer).not.toHaveBeenCalled(); + expect(model.selectContainer).not.toHaveBeenCalled(); + }); + + it('should handle action results with an empty deleted list', function test() { + var ctrl = createController(); + var result = { deleted: [] }; + + spyOn(model, 'updateContainer'); + spyOn($scope, '$broadcast'); + ctrl.actionResultHandler(result); + + expect($scope.$broadcast).not.toHaveBeenCalled(); + expect(model.updateContainer).not.toHaveBeenCalled(); + expect(model.selectContainer).not.toHaveBeenCalled(); + }); + + it('should handle action results', function test() { + var ctrl = createController(); + spyOn($scope, '$broadcast'); + spyOn(model, 'updateContainer'); + + var d = $q.defer(); + ctrl.actionResultHandler(d.promise); + d.resolve({deleted: [1]}); + $scope.$apply(); + + expect(model.updateContainer).toHaveBeenCalled(); + expect(model.selectContainer).toHaveBeenCalledWith('spam', undefined); + }); }); })(); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-controller.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-controller.js index 30be321012..62ff5c0512 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-controller.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-controller.js @@ -18,24 +18,27 @@ angular .module('horizon.dashboard.project.containers') - .controller('UploadObjectModalController', UploadObjectModalController); + .controller( + 'horizon.dashboard.project.containers.UploadObjectModalController', + UploadObjectModalController + ); UploadObjectModalController.$inject = [ - 'horizon.dashboard.project.containers.containers-model', - '$scope' + 'horizon.dashboard.project.containers.containers-model' ]; - function UploadObjectModalController(model, $scope) { + function UploadObjectModalController(model) { var ctrl = this; ctrl.model = { - name:'', + name: '', container: model.container, folder: model.folder, view_file: null, // file object managed by angular form ngModel upload_file: null, // file object from the DOM element with the actual upload DELIMETER: model.DELIMETER }; + ctrl.form = null; // set by the HTML ctrl.changeFile = changeFile; /////////// @@ -45,9 +48,12 @@ // update the upload file & its name ctrl.model.upload_file = files[0]; ctrl.model.name = files[0].name; + ctrl.form.name.$setDirty(); - // we're modifying the model value from a DOM event so we need to manually $digest - $scope.$digest(); + // Note that a $scope.$digest() is now needed for the change to the ngModel to be + // reflected in the page (since this callback is fired from inside a DOM event) + // but the on-file-changed directive currently does a digest after this callback + // is invoked. } } } diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-controller.spec.js b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-controller.spec.js index 27b4940ee7..2a4d0c1d29 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-controller.spec.js +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-controller.spec.js @@ -18,7 +18,7 @@ 'use strict'; describe('horizon.dashboard.project.containers upload-object controller', function() { - var controller, $scope; + var controller, ctrl; beforeEach(module('horizon.framework')); beforeEach(module('horizon.dashboard.project.containers')); @@ -30,42 +30,31 @@ }); })); - beforeEach(inject(function ($injector, _$rootScope_) { + beforeEach(inject(function ($injector) { controller = $injector.get('$controller'); - $scope = _$rootScope_.$new(true); + ctrl = controller('horizon.dashboard.project.containers.UploadObjectModalController'); + ctrl.form = {name: {$setDirty: angular.noop}}; + spyOn(ctrl.form.name, '$setDirty'); })); - function createController() { - return controller('UploadObjectModalController', {$scope: $scope}); - } - it('should initialise the controller model when created', function test() { - var ctrl = createController(); expect(ctrl.model.name).toEqual(''); expect(ctrl.model.container.name).toEqual('spam'); expect(ctrl.model.folder).toEqual('ham'); }); it('should respond to file changes correctly', function test() { - var ctrl = createController(); - spyOn($scope, '$digest'); var file = {name: 'eggs'}; - ctrl.changeFile([file]); - expect(ctrl.model.name).toEqual('eggs'); expect(ctrl.model.upload_file).toEqual(file); - expect($scope.$digest).toHaveBeenCalled(); + expect(ctrl.form.name.$setDirty).toHaveBeenCalled(); }); - it('should respond to file changes correctly if no files are present', function test() { - var ctrl = createController(); - spyOn($scope, '$digest'); - + it('should not respond to file changes if no files are present', function test() { ctrl.changeFile([]); - expect(ctrl.model.name).toEqual(''); - expect($scope.$digest).not.toHaveBeenCalled(); + expect(ctrl.form.name.$setDirty).not.toHaveBeenCalled(); }); }); })(); diff --git a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-modal.html b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-modal.html index b7c4556bab..e88a4f6655 100644 --- a/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-modal.html +++ b/openstack_dashboard/dashboards/project/static/dashboard/project/containers/upload-object-modal.html @@ -8,7 +8,7 @@ -
+
@@ -20,11 +20,18 @@ ng-model="ctrl.model.view_file" on-file-change="ctrl.changeFile">
-
+
- + + + Checking if this name is used... + + + This name already exists. +
@@ -38,7 +45,6 @@

-
@@ -47,7 +53,7 @@ Cancel diff --git a/openstack_dashboard/static/app/core/openstack-service-api/swift.service.js b/openstack_dashboard/static/app/core/openstack-service-api/swift.service.js index 1d19463433..25d249b5f4 100644 --- a/openstack_dashboard/static/app/core/openstack-service-api/swift.service.js +++ b/openstack_dashboard/static/app/core/openstack-service-api/swift.service.js @@ -60,7 +60,7 @@ /** * @name getObjectURL - * @param {Object} container - A container + * @param {Object} container - A container name * @param {Object} object - The object to be encoded * @param {string} type - String representation of the type * @description @@ -211,11 +211,11 @@ /** * @name uploadObject * @param {Object} container - The container - * @param {string} objectName - The new object's name + * @param {string} objectName - The object's name (and optional path) * @param {Object} file - File data * @description - * Add a file to the specified container with the given objectName (which - * may include pseudo-folder path), the mimetype and raw file data. + * Add or replace a file in the specified container with the given objectName + * (which may include pseudo-folder path), the mimetype and raw file data. * @returns {Object} The result of the API call * */ @@ -224,12 +224,8 @@ service.getObjectURL(container, objectName), {file: file} ) - .error(function (response, status) { - if (status === 409) { - toastService.add('error', response); - } else { - toastService.add('error', gettext('Unable to upload the object.')); - } + .error(function () { + toastService.add('error', gettext('Unable to upload the object.')); }); } @@ -264,16 +260,23 @@ * @param {string} objectName - The name of the object to get info about * @description * Get the metadata for an object. + * + * If you just wish to test for the existence of the object, set + * ignoreError so user-visible error isn't automatically displayed. * @returns {Object} The result of the API call * */ - function getObjectDetails(container, objectName) { - return apiService.get( + function getObjectDetails(container, objectName, ignoreError) { + var promise = apiService.get( service.getObjectURL(container, objectName, 'metadata') - ) - .error(function () { - toastService.add('error', gettext('Unable to get details of the object.')); - }); + ); + if (ignoreError) { + // provide a noop error handler so the error is ignored + return promise.error(angular.noop); + } + return promise.error(function () { + toastService.add('error', gettext('Unable to get details of the object.')); + }); } /** diff --git a/openstack_dashboard/static/app/core/openstack-service-api/swift.service.spec.js b/openstack_dashboard/static/app/core/openstack-service-api/swift.service.spec.js index 077d969d36..50a14abec5 100644 --- a/openstack_dashboard/static/app/core/openstack-service-api/swift.service.spec.js +++ b/openstack_dashboard/static/app/core/openstack-service-api/swift.service.spec.js @@ -178,19 +178,6 @@ expect(toastService.add).toHaveBeenCalledWith('error', message); }); - it('returns a relevant error message when uploadObject returns a 409 error', function test() { - var promise = {error: angular.noop}; - spyOn(apiService, 'post').and.returnValue(promise); - spyOn(promise, 'error'); - service.uploadObject('spam', 'ham'); - spyOn(toastService, 'add'); - var innerFunc = promise.error.calls.argsFor(0)[0]; - // In the case of 409 - var message = 'A object with the name "ham" already exists.'; - innerFunc(message, 409); - expect(toastService.add).toHaveBeenCalledWith('error', message); - }); - it('returns a relevant error message when deleteContainer returns a 409 error', function test() { var promise = {error: angular.noop}; diff --git a/openstack_dashboard/test/api_tests/swift_tests.py b/openstack_dashboard/test/api_tests/swift_tests.py index 7ea18b6132..36646a5418 100644 --- a/openstack_dashboard/test/api_tests/swift_tests.py +++ b/openstack_dashboard/test/api_tests/swift_tests.py @@ -211,9 +211,7 @@ class SwiftApiTests(test.APITestCase): headers = {'X-Object-Meta-Orig-Filename': fake_name} - swift_api = self.stub_swiftclient(2) - exc = self.exceptions.swift - swift_api.head_object(container.name, obj.name).AndRaise(exc) + swift_api = self.stub_swiftclient() test_file = FakeFile() swift_api.put_object(container.name, obj.name, @@ -227,35 +225,11 @@ class SwiftApiTests(test.APITestCase): obj.name, test_file) - def test_swift_upload_duplicate_object(self): - container = self.containers.first() - obj = self.objects.first() - fake_name = 'fake_object.jpg' - - class FakeFile(object): - def __init__(self): - self.name = fake_name - self.data = obj.data - self.size = len(obj.data) - - swift_api = self.stub_swiftclient() - swift_api.head_object(container.name, obj.name).AndReturn(obj) - test_file = FakeFile() - self.mox.ReplayAll() - - with self.assertRaises(exceptions.AlreadyExists): - api.swift.swift_upload_object(self.request, - container.name, - obj.name, - test_file) - def test_swift_upload_object_without_file(self): container = self.containers.first() obj = self.objects.first() - swift_api = self.stub_swiftclient(2) - exc = self.exceptions.swift - swift_api.head_object(container.name, obj.name).AndRaise(exc) + swift_api = self.stub_swiftclient() swift_api.put_object(container.name, obj.name, None,