From 2131f06002cd8f29c812c00de792ecdf127da8bd Mon Sep 17 00:00:00 2001 From: Michael Krotscheck Date: Fri, 6 Nov 2015 16:00:16 -0800 Subject: [PATCH 1/2] feat($resource): add support for `request` and `requestError` interceptors This commit adds `request` and `requestError` interceptors for `$resource`, as per the documentation found for `$http` interceptors. It is important to note that returning an error at this stage of the request - before the call to `$http` - will completely bypass any global interceptors and/or recovery handlers, as those are added to a separate context. This is intentional; intercepting a request before it is passed to `$http` indicates that the resource itself has made a decision, and that it accepts the responsibility for recovery. Closes #5146 BREAKING CHANGE: Previously, calling a `$resource` method would synchronously call `$http`. Now, it will be called asynchronously (regardless if a `request`/`requestError` interceptor has been defined. This is not expected to affect applications at runtime, since the overall operation is asynchronous already, but may affect assertions in tests. For example, if you want to assert that `$http` has been called with specific arguments as a result of a `$resource` call, you now need to run a `$digest` first, to ensure the (possibly empty) request interceptor promise has been resolved. Before: ```js it('...', function() { $httpBackend.expectGET('/api/things').respond(...); var Things = $resource('/api/things'); Things.query(); expect($http).toHaveBeenCalledWith(...); }); ``` After: ```js it('...', function() { $httpBackend.expectGET('/api/things').respond(...); var Things = $resource('/api/things'); Things.query(); $rootScope.$digest(); expect($http).toHaveBeenCalledWith(...); }); ``` --- src/ngResource/resource.js | 20 ++- test/ngResource/resourceSpec.js | 270 +++++++++++++++++++++++++++++++- 2 files changed, 281 insertions(+), 9 deletions(-) diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index 55760d2f77e9..f3bda5460873 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -185,11 +185,11 @@ function shallowClearAndCopy(src, dst) { * for more information. * - **`responseType`** - `{string}` - see * [requestType](https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest#responseType). - * - **`interceptor`** - `{Object=}` - The interceptor object has two optional methods - - * `response` and `responseError`. Both `response` and `responseError` interceptors get called - * with `http response` object. See {@link ng.$http $http interceptors}. In addition, the + * - **`interceptor`** - `{Object=}` - The interceptor object has four optional methods - + * `request`, `requestError`, `response`, and `responseError`. See + * {@link ng.$http $http interceptors} for details. In addition, the * resource instance or array object is accessible by the `resource` property of the - * `http response` object. + * `http response` object passed to response interceptors. * Keep in mind that the associated promise will be resolved with the value returned by the * response interceptor, if one is specified. The default response interceptor returns * `response.resource` (i.e. the resource instance or array). @@ -707,6 +707,9 @@ angular.module('ngResource', ['ng']). var isInstanceCall = this instanceof Resource; var value = isInstanceCall ? data : (action.isArray ? [] : new Resource(data)); var httpConfig = {}; + var requestInterceptor = action.interceptor && action.interceptor.request || undefined; + var requestErrorInterceptor = action.interceptor && action.interceptor.requestError || + undefined; var responseInterceptor = action.interceptor && action.interceptor.response || defaultResponseInterceptor; var responseErrorInterceptor = action.interceptor && action.interceptor.responseError || @@ -743,7 +746,14 @@ angular.module('ngResource', ['ng']). extend({}, extractParams(data, action.params || {}), params), action.url); - var promise = $http(httpConfig).then(function(response) { + // Start the promise chain + var promise = $q. + resolve(httpConfig). + then(requestInterceptor). + catch(requestErrorInterceptor). + then($http); + + promise = promise.then(function(response) { var data = response.data; if (data) { diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index c472ad63f9f4..bdac70abe5bf 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -3,7 +3,7 @@ describe('resource', function() { describe('basic usage', function() { - var $resource, CreditCard, callback, $httpBackend, resourceProvider; + var $resource, CreditCard, callback, $httpBackend, resourceProvider, $q; beforeEach(module('ngResource')); @@ -14,6 +14,7 @@ describe('basic usage', function() { beforeEach(inject(function($injector) { $httpBackend = $injector.get('$httpBackend'); $resource = $injector.get('$resource'); + $q = $injector.get('$q'); CreditCard = $resource('/CreditCard/:id:verb', {id:'@id.key'}, { charge:{ method:'post', @@ -1129,6 +1130,187 @@ describe('basic usage', function() { }); + describe('requestInterceptor', function() { + var rejectReason = {'lol':'cat'}; + var successSpy, failureSpy; + + beforeEach(function() { + successSpy = jasmine.createSpy('successSpy'); + failureSpy = jasmine.createSpy('failureSpy'); + }); + + it('should allow per action request interceptor that gets full configuration', function() { + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function(httpConfig) { + callback(httpConfig); + return httpConfig; + } + } + } + }); + + $httpBackend.expect('GET', '/CreditCard').respond([{id: 1}]); + + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + + $httpBackend.flush(); + expect(callback).toHaveBeenCalledOnce(); + expect(successSpy).toHaveBeenCalledOnce(); + expect(failureSpy).not.toHaveBeenCalled(); + + expect(callback).toHaveBeenCalledWith({ + 'method': 'get', + 'url': '/CreditCard' + }); + }); + + it('should call $http with the value returned from requestInterceptor', function() { + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function(httpConfig) { + httpConfig.url = '/DebitCard'; + return httpConfig; + } + } + } + }); + + $httpBackend.expect('GET', '/DebitCard').respond([{id: 1}]); + + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + + $httpBackend.flush(); + expect(successSpy).toHaveBeenCalledOnce(); + expect(failureSpy).not.toHaveBeenCalled(); + }); + + it('should abort the operation if the requestInterceptor rejects the operation', function() { + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function() { + return $q.reject(rejectReason); + } + } + } + }); + + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + + // Make sure all promises resolve. + $rootScope.$apply(); + + // Ensure the resource promise was rejected + expect(resource.$resolved).toBeTruthy(); + expect(successSpy).not.toHaveBeenCalled(); + expect(failureSpy).toHaveBeenCalledOnce(); + expect(failureSpy).toHaveBeenCalledWith(rejectReason); + + // Ensure that no requests were made. + $httpBackend.verifyNoOutstandingRequest(); + }); + + it('should call requestErrorInterceptor if requestInterceptor rejects the operation', function() { + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function() { + return $q.reject(rejectReason); + }, + requestError: function(rejection) { + callback(rejection); + return $q.reject(rejection); + } + } + } + }); + + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + $rootScope.$digest(); + + expect(callback).toHaveBeenCalledOnce(); + expect(callback).toHaveBeenCalledWith(rejectReason); + expect(successSpy).not.toHaveBeenCalled(); + expect(failureSpy).toHaveBeenCalledOnce(); + expect(failureSpy).toHaveBeenCalledWith(rejectReason); + + // Ensure that no requests were made. + $httpBackend.verifyNoOutstandingRequest(); + }); + + it('should abort the operation if a requestErrorInterceptor rejects the operation', function() { + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function() { + return $q.reject(rejectReason); + }, + requestError: function(rejection) { + return $q.reject(rejection); + } + } + } + }); + + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + $rootScope.$apply(); + + expect(resource.$resolved).toBeTruthy(); + expect(successSpy).not.toHaveBeenCalled(); + expect(failureSpy).toHaveBeenCalledOnce(); + expect(failureSpy).toHaveBeenCalledWith(rejectReason); + + // Ensure that no requests were made. + $httpBackend.verifyNoOutstandingRequest(); + }); + + it('should continue the operation if a requestErrorInterceptor rescues it', function() { + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function(httpConfig) { + return $q.reject(httpConfig); + }, + requestError: function(httpConfig) { + return $q.resolve(httpConfig); + } + } + } + }); + + $httpBackend.expect('GET', '/CreditCard').respond([{id: 1}]); + + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + $httpBackend.flush(); + + expect(resource.$resolved).toBeTruthy(); + expect(successSpy).toHaveBeenCalledOnce(); + expect(failureSpy).not.toHaveBeenCalled(); + $httpBackend.verifyNoOutstandingRequest(); + }); + }); + it('should allow per action response interceptor that gets full response', function() { CreditCard = $resource('/CreditCard', {}, { query: { @@ -1584,6 +1766,7 @@ describe('extra params', function() { var $http; var $httpBackend; var $resource; + var $rootScope; beforeEach(module('ngResource')); @@ -1593,10 +1776,11 @@ describe('extra params', function() { }); })); - beforeEach(inject(function(_$http_, _$httpBackend_, _$resource_) { + beforeEach(inject(function(_$http_, _$httpBackend_, _$resource_, _$rootScope_) { $http = _$http_; $httpBackend = _$httpBackend_; $resource = _$resource_; + $rootScope = _$rootScope_; })); afterEach(function() { @@ -1610,6 +1794,7 @@ describe('extra params', function() { var R = $resource('/:foo'); R.get({foo: 'bar', baz: 'qux'}); + $rootScope.$digest(); expect($http).toHaveBeenCalledWith(jasmine.objectContaining({params: {baz: 'qux'}})); }); @@ -1624,7 +1809,7 @@ describe('extra params', function() { }); describe('errors', function() { - var $httpBackend, $resource, $q; + var $httpBackend, $resource, $q, $rootScope; beforeEach(module(function($exceptionHandlerProvider) { $exceptionHandlerProvider.mode('log'); @@ -1636,6 +1821,7 @@ describe('errors', function() { $httpBackend = $injector.get('$httpBackend'); $resource = $injector.get('$resource'); $q = $injector.get('$q'); + $rootScope = $injector.get('$rootScope'); })); @@ -1838,6 +2024,81 @@ describe('handling rejections', function() { expect($exceptionHandler.errors[0]).toMatch(/^Error: should be caught/); } ); + + describe('requestInterceptor', function() { + var rejectReason = {'lol':'cat'}; + var $q, $rootScope; + var successSpy, failureSpy, callback; + + beforeEach(inject(function(_$q_, _$rootScope_) { + $q = _$q_; + $rootScope = _$rootScope_; + + successSpy = jasmine.createSpy('successSpy'); + failureSpy = jasmine.createSpy('failureSpy'); + callback = jasmine.createSpy(); + })); + + it('should call requestErrorInterceptor if requestInterceptor throws an error', function() { + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function() { + throw rejectReason; + }, + requestError: function(rejection) { + callback(rejection); + return $q.reject(rejection); + } + } + } + }); + + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + $rootScope.$apply(); + + expect(callback).toHaveBeenCalledOnce(); + expect(callback).toHaveBeenCalledWith(rejectReason); + expect(successSpy).not.toHaveBeenCalled(); + expect(failureSpy).toHaveBeenCalledOnce(); + expect(failureSpy).toHaveBeenCalledWith(rejectReason); + + // Ensure that no requests were made. + $httpBackend.verifyNoOutstandingRequest(); + }); + + it('should abort the operation if a requestErrorInterceptor throws an exception', function() { + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function() { + return $q.reject(); + }, + requestError: function() { + throw rejectReason; + } + } + } + }); + + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + $rootScope.$apply(); + + expect(resource.$resolved).toBeTruthy(); + expect(successSpy).not.toHaveBeenCalled(); + expect(failureSpy).toHaveBeenCalledOnce(); + expect(failureSpy).toHaveBeenCalledWith(rejectReason); + + // Ensure that no requests were made. + $httpBackend.verifyNoOutstandingRequest(); + }); + }); }); describe('cancelling requests', function() { @@ -1902,7 +2163,7 @@ describe('cancelling requests', function() { ); it('should use `cancellable` value if passed a non-numeric `timeout` in an action', - inject(function($log, $q) { + inject(function($log, $q, $rootScope) { spyOn($log, 'debug'); $httpBackend.whenGET('/CreditCard').respond({}); @@ -1915,6 +2176,7 @@ describe('cancelling requests', function() { }); var creditCard = CreditCard.get(); + $rootScope.$digest(); expect(creditCard.$cancelRequest).toBeDefined(); expect(httpSpy.calls.argsFor(0)[0].timeout).toEqual(jasmine.any($q)); expect(httpSpy.calls.argsFor(0)[0].timeout.then).toBeDefined(); From 173db93383cb66496ad787e268ce714819d485f4 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 7 Dec 2017 11:56:29 +0200 Subject: [PATCH 2/2] fixup! feat($resource): add support for `request` and `requestError` interceptors --- src/ngResource/resource.js | 5 +-- test/ngResource/resourceSpec.js | 63 +++++++++++++++++---------------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index f3bda5460873..c8a79274ca2b 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -187,8 +187,9 @@ function shallowClearAndCopy(src, dst) { * [requestType](https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest#responseType). * - **`interceptor`** - `{Object=}` - The interceptor object has four optional methods - * `request`, `requestError`, `response`, and `responseError`. See - * {@link ng.$http $http interceptors} for details. In addition, the - * resource instance or array object is accessible by the `resource` property of the + * {@link ng.$http $http interceptors} for details. Note that `request`/`requestError` + * interceptors are applied before calling `$http`, thus before any global `$http` interceptors. + * The resource instance or array object is accessible by the `resource` property of the * `http response` object passed to response interceptors. * Keep in mind that the associated promise will be resolved with the value returned by the * response interceptor, if one is specified. The default response interceptor returns diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index bdac70abe5bf..00fce4b662a8 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -1189,7 +1189,9 @@ describe('basic usage', function() { resource.$promise.then(successSpy, failureSpy); $httpBackend.flush(); - expect(successSpy).toHaveBeenCalledOnce(); + expect(successSpy).toHaveBeenCalledOnceWith(jasmine.arrayContaining([ + jasmine.objectContaining({id: 1}) + ])); expect(failureSpy).not.toHaveBeenCalled(); }); @@ -1215,8 +1217,7 @@ describe('basic usage', function() { // Ensure the resource promise was rejected expect(resource.$resolved).toBeTruthy(); expect(successSpy).not.toHaveBeenCalled(); - expect(failureSpy).toHaveBeenCalledOnce(); - expect(failureSpy).toHaveBeenCalledWith(rejectReason); + expect(failureSpy).toHaveBeenCalledOnceWith(rejectReason); // Ensure that no requests were made. $httpBackend.verifyNoOutstandingRequest(); @@ -1243,44 +1244,41 @@ describe('basic usage', function() { resource.$promise.then(successSpy, failureSpy); $rootScope.$digest(); - expect(callback).toHaveBeenCalledOnce(); - expect(callback).toHaveBeenCalledWith(rejectReason); + expect(callback).toHaveBeenCalledOnceWith(rejectReason); expect(successSpy).not.toHaveBeenCalled(); - expect(failureSpy).toHaveBeenCalledOnce(); - expect(failureSpy).toHaveBeenCalledWith(rejectReason); + expect(failureSpy).toHaveBeenCalledOnceWith(rejectReason); // Ensure that no requests were made. $httpBackend.verifyNoOutstandingRequest(); }); it('should abort the operation if a requestErrorInterceptor rejects the operation', function() { - var CreditCard = $resource('/CreditCard', {}, { - query: { - method: 'get', - isArray: true, - interceptor: { - request: function() { - return $q.reject(rejectReason); - }, - requestError: function(rejection) { - return $q.reject(rejection); - } + var CreditCard = $resource('/CreditCard', {}, { + query: { + method: 'get', + isArray: true, + interceptor: { + request: function() { + return $q.reject(rejectReason); + }, + requestError: function(rejection) { + return $q.reject(rejection); } } - }); + } + }); - var resource = CreditCard.query(); - resource.$promise.then(successSpy, failureSpy); - $rootScope.$apply(); + var resource = CreditCard.query(); + resource.$promise.then(successSpy, failureSpy); + $rootScope.$apply(); - expect(resource.$resolved).toBeTruthy(); - expect(successSpy).not.toHaveBeenCalled(); - expect(failureSpy).toHaveBeenCalledOnce(); - expect(failureSpy).toHaveBeenCalledWith(rejectReason); + expect(resource.$resolved).toBeTruthy(); + expect(successSpy).not.toHaveBeenCalled(); + expect(failureSpy).toHaveBeenCalledOnceWith(rejectReason); - // Ensure that no requests were made. - $httpBackend.verifyNoOutstandingRequest(); - }); + // Ensure that no requests were made. + $httpBackend.verifyNoOutstandingRequest(); + }); it('should continue the operation if a requestErrorInterceptor rescues it', function() { var CreditCard = $resource('/CreditCard', {}, { @@ -1305,8 +1303,11 @@ describe('basic usage', function() { $httpBackend.flush(); expect(resource.$resolved).toBeTruthy(); - expect(successSpy).toHaveBeenCalledOnce(); + expect(successSpy).toHaveBeenCalledOnceWith(jasmine.arrayContaining([ + jasmine.objectContaining({id: 1}) + ])); expect(failureSpy).not.toHaveBeenCalled(); + $httpBackend.verifyNoOutstandingRequest(); }); }); @@ -2038,7 +2039,7 @@ describe('handling rejections', function() { failureSpy = jasmine.createSpy('failureSpy'); callback = jasmine.createSpy(); })); - + it('should call requestErrorInterceptor if requestInterceptor throws an error', function() { var CreditCard = $resource('/CreditCard', {}, { query: {