Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
feat($resource): expose promise based api via $then and $resolved
Browse files Browse the repository at this point in the history 
Expose $then and $resolved properties on resource action return values which
allow checking if a promise has been resolved already as well as registering
listeners at any time of the resource object life-cycle.

This commit replaces unreleased commit f3bff27
which exposed unintuitive $q api instead and didn't expose important stuff
like http headers.
  • Loading branch information
@IgorMinar
IgorMinar committed Feb 12, 2013
1 parent c0a0781 commit dba6bc7
Show file tree
Hide file tree
Showing 2 changed files with 238 additions and 64 deletions.
65 changes: 41 additions & 24 deletions src/ngResource/resource.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* the need to interact with the low level {@link ng.$http $http} service.
*
* @param {string} url A parameterized URL template with parameters prefixed by `:` as in
* `/user/:username`. If you are using a URL with a port number (e.g.
* `/user/:username`. If you are using a URL with a port number (e.g.
* `http://example.com:8080/api`), you'll need to escape the colon character before the port
* number, like this: `$resource('http://example.com\\:8080/api')`.
*
Expand Down Expand Up @@ -109,10 +109,23 @@
* - non-GET "class" actions: `Resource.action([parameters], postData, [success], [error])`
* - non-GET instance actions: `instance.$action([parameters], [success], [error])`
*
* The Resource also has these properties:
*
* - '$q': the promise from the underlying {@link ng.$http} call.
* - '$resolved': true if the promise has been resolved (either with success or rejection);
* The Resource instances and collection have these additional properties:
*
* - `$then`: the `then` method of a {@link ng.$q promise} derived from the underlying
* {@link ng.$http $http} call.
*
* The success callback for the `$then` method will be resolved if the underlying `$http` requests
* succeeds.
*
* The success callback is called with a single object which is the {@link ng.$http http response}
* object extended with a new property `resource`. This `resource` property is a reference to the
* result of the resource action — resource object or array of resources.
*
* The error callback is called with the {@link ng.$http http response} object when an http
* error occurs.
*
* - `$resolved`: true if the promise has been resolved (either with success or rejection);
* Knowing if the Resource has been resolved is useful in data-binding.
*
* @example
Expand Down Expand Up @@ -415,32 +428,36 @@ angular.module('ngResource', ['ng']).
httpConfig.data = data;
httpConfig.url = route.url(extend({}, extractParams(data, action.params || {}), params))

function markResolved() { value.$resolved = true; };
function markResolved() { value.$resolved = true; }

promise = $http(httpConfig);
value.$q = promise;
value.$resolved = false;
promise.then(markResolved, markResolved)
promise.then(function(response) {
var data = response.data;
var q = value.$q, resolved = value.$resolved;
if (data) {
if (action.isArray) {
value.length = 0;
forEach(data, function(item) {
value.push(new Resource(item));
});
} else {
copy(data, value);
value.$q = q;
value.$resolved = resolved;
}

promise.then(markResolved, markResolved);
value.$then = promise.then(function(response) {
var data = response.data;
var then = value.$then, resolved = value.$resolved;

if (data) {
if (action.isArray) {
value.length = 0;
forEach(data, function(item) {
value.push(new Resource(item));
});
} else {
copy(data, value);
value.$then = then;
value.$resolved = resolved;
}
(success||noop)(value, response.headers);
}, error);
}

return value;
(success||noop)(value, response.headers);

response.resource = value;
return response;
}, error).then;

return value;
};


Expand Down
237 changes: 197 additions & 40 deletions test/ngResource/resourceSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,51 +419,208 @@ describe("resource", function() {
expect(person.name).toEqual('misko');
});

it("should have $q and $resolved properties for get", function () {
$httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
var cc = CreditCard.get({id: 123}, callback);
expect(cc.$q).toBeDefined();
expect(typeof cc.$q).toBe('object');
expect(cc.$resolved).toBeFalsy();
$httpBackend.flush();
expect(cc.$q).toBeDefined();
expect(cc.$resolved).toBeTruthy();
});

it("should have $q and $resolved properties for query", function() {
$httpBackend.expect('GET', '/CreditCard?key=value').respond([{id: 1}, {id: 2}]);
describe('promise api', function() {

var ccs = CreditCard.query({key: 'value'}, callback);
expect(ccs.$q).toBeDefined();
expect(typeof ccs.$q).toBe('object');
expect(ccs.$resolved).toBeFalsy();
$httpBackend.flush();
expect(ccs.$q).toBeDefined();
expect(ccs.$resolved).toBeTruthy();
});
var $rootScope;

it("should have $q and $resolved properties for save", function() {
$httpBackend.expect('POST', '/CreditCard/123', '{"id":{"key":123},"name":"misko"}').
respond({id: {key: 123}, name: 'rama'});

var cc = CreditCard.save({id: {key: 123}, name: 'misko'}, callback);
expect(cc.$q).toBeDefined();
expect(typeof cc.$q).toBe('object');
expect(cc.$resolved).toBeFalsy();
$httpBackend.flush();
expect(cc.$q).toBeDefined();
expect(cc.$resolved).toBeTruthy();
});
beforeEach(inject(function(_$rootScope_) {
$rootScope = _$rootScope_;
}));

it('should should have $q and $resolved properties for delete', function() {
$httpBackend.expect('DELETE', '/CreditCard/123').respond({});
var removed = CreditCard.remove({id:123}, callback);
expect(removed.$q).toBeDefined();
expect(typeof removed.$q).toBe('object');
expect(removed.$resolved).toBeFalsy();
$httpBackend.flush();
expect(removed.$q).toBeDefined();
expect(removed.$resolved).toBeTruthy();

describe('single resource', function() {

it('should add promise $then method to the result object', function() {
$httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
var cc = CreditCard.get({id: 123});

cc.$then(callback);
expect(callback).not.toHaveBeenCalled();

$httpBackend.flush();

var response = callback.mostRecentCall.args[0];

expect(response).toEqualData({
data: {id: 123, number: '9876'},
status: 200,
config: {method: 'GET', data: undefined, url: '/CreditCard/123'},
resource: {id: 123, number: '9876', $resolved: true}
});
expect(typeof response.resource.$save).toBe('function');
});


it('should keep $then around after promise resolution', function() {
$httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
var cc = CreditCard.get({id: 123});

cc.$then(callback);
$httpBackend.flush();

var response = callback.mostRecentCall.args[0];

callback.reset();

cc.$then(callback);
$rootScope.$apply(); //flush async queue

expect(callback).toHaveBeenCalledOnceWith(response);
});


it('should allow promise chaining via $then method', function() {
$httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
var cc = CreditCard.get({id: 123});

cc.$then(function(response) { return 'new value'; }).then(callback);
$httpBackend.flush();

expect(callback).toHaveBeenCalledOnceWith('new value');
});


it('should allow error callback registration via $then method', function() {
$httpBackend.expect('GET', '/CreditCard/123').respond(404, 'resource not found');
var cc = CreditCard.get({id: 123});

cc.$then(null, callback);
$httpBackend.flush();

var response = callback.mostRecentCall.args[0];

expect(response).toEqualData({
data : 'resource not found',
status : 404,
config : { method : 'GET', data : undefined, url : '/CreditCard/123' }
});
});


it('should add $resolved boolean field to the result object', function() {
$httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
var cc = CreditCard.get({id: 123});

expect(cc.$resolved).toBe(false);

cc.$then(callback);
expect(cc.$resolved).toBe(false);

$httpBackend.flush();

expect(cc.$resolved).toBe(true);
});


it('should set $resolved field to true when an error occurs', function() {
$httpBackend.expect('GET', '/CreditCard/123').respond(404, 'resource not found');
var cc = CreditCard.get({id: 123});

cc.$then(null, callback);
$httpBackend.flush();
expect(callback).toHaveBeenCalledOnce();
expect(cc.$resolved).toBe(true);
});
});


describe('resource collection', function() {

it('should add promise $then method to the result object', function() {
$httpBackend.expect('GET', '/CreditCard?key=value').respond([{id: 1}, {id: 2}]);
var ccs = CreditCard.query({key: 'value'});

ccs.$then(callback);
expect(callback).not.toHaveBeenCalled();

$httpBackend.flush();

var response = callback.mostRecentCall.args[0];

expect(response).toEqualData({
data: [{id: 1}, {id :2}],
status: 200,
config: {method: 'GET', data: undefined, url: '/CreditCard?key=value'},
resource: [ { id : 1 }, { id : 2 } ]
});
expect(typeof response.resource[0].$save).toBe('function');
expect(typeof response.resource[1].$save).toBe('function');
});


it('should keep $then around after promise resolution', function() {
$httpBackend.expect('GET', '/CreditCard?key=value').respond([{id: 1}, {id: 2}]);
var ccs = CreditCard.query({key: 'value'});

ccs.$then(callback);
$httpBackend.flush();

var response = callback.mostRecentCall.args[0];

callback.reset();

ccs.$then(callback);
$rootScope.$apply(); //flush async queue

expect(callback).toHaveBeenCalledOnceWith(response);
});


it('should allow promise chaining via $then method', function() {
$httpBackend.expect('GET', '/CreditCard?key=value').respond([{id: 1}, {id: 2}]);
var ccs = CreditCard.query({key: 'value'});

ccs.$then(function(response) { return 'new value'; }).then(callback);
$httpBackend.flush();

expect(callback).toHaveBeenCalledOnceWith('new value');
});


it('should allow error callback registration via $then method', function() {
$httpBackend.expect('GET', '/CreditCard?key=value').respond(404, 'resource not found');
var ccs = CreditCard.query({key: 'value'});

ccs.$then(null, callback);
$httpBackend.flush();

var response = callback.mostRecentCall.args[0];

expect(response).toEqualData({
data : 'resource not found',
status : 404,
config : { method : 'GET', data : undefined, url : '/CreditCard?key=value' }
});
});


it('should add $resolved boolean field to the result object', function() {
$httpBackend.expect('GET', '/CreditCard?key=value').respond([{id: 1}, {id: 2}]);
var ccs = CreditCard.query({key: 'value'}, callback);

expect(ccs.$resolved).toBe(false);

ccs.$then(callback);
expect(ccs.$resolved).toBe(false);

$httpBackend.flush();

expect(ccs.$resolved).toBe(true);
});


it('should set $resolved field to true when an error occurs', function() {
$httpBackend.expect('GET', '/CreditCard?key=value').respond(404, 'resource not found');
var ccs = CreditCard.query({key: 'value'});

ccs.$then(null, callback);
$httpBackend.flush();
expect(callback).toHaveBeenCalledOnce();
expect(ccs.$resolved).toBe(true);
});
});
});


Expand Down

3 comments on commit dba6bc7

@ashtuchkin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice feature, thank you! That said, why not expose a real (derived) promise? It would be much easier to use in $routeProvider.when({resolve:{..}) clauses for example. Currently I need to wrap the provided $then function, but it's not as great as it could be:

$routeProvider.when("products", {
  resolve: {
    products: function(Products) {
      return Products.get().$promise;
    }
  },
  controller: function($scope, products) {
    $scope.products = products.products;
  },
  templateUrl: "products.html", // Now, the template will be rendered only when the request is finished and data is ready.
});

The $promise would then return data object on success, augmented with $http - original result from $http with http status, headers, etc.
Also, I would suggest moving $resolved property to the generic promise interface in $q, along with the then - isn't it general enough for it?

@IgorMinar
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with most of the suggestions. the reason why we ended up with $then was to allow easy chaining of resource requests resource1.$then(function(response) {}).

It totally makes sense to expose the whole promise though especially for the use with $route. Would you like to send a PR which would expose $promise in addition to the $then?

with regards to exposing $resolved or just resolved on the promise api - I'm not sure if that buys you that much. what is the use case? to access the value you'd still make an async call, so you can't really do much with that info. but maybe I'm missing something.

@ashtuchkin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created #2060 to continue discussion)

Please sign in to comment.