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

Commit

Permalink
fix(ngResource): don't convert literal values into Resource objects w…
Browse files Browse the repository at this point in the history
…hen isArray is true

Previously non-object literals would be thrown out of Resource responses with isArray===true, or
otherwise converted into Objects (in the case of string literals). The reason for this is because
shallowClearAndCopy iterates over keys, and copies keys into the destination. Iterating over String
keys results in integer keys, with a single-character value.

Not converting non-objects to Resources means that you lose the ability to perform Resource operations
on them. However, they become usable as strings, numbers, or booleans, which is important.

In the future, it would be useful to make these useful as Resources while still retaining their primitive
value usefulness.

Closes #6314
Closes #7741
  • Loading branch information
caitp authored and rodyhaddad committed Jun 13, 2014
1 parent 81b7e5a commit f0904cf
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 7 deletions.
23 changes: 16 additions & 7 deletions src/ngResource/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -522,23 +522,32 @@ angular.module('ngResource', ['ng']).
extend({}, extractParams(data, action.params || {}), params),
action.url);

var promise = $http(httpConfig).then(function(response) {
var promise = $http(httpConfig).then(function (response) {
var data = response.data,
promise = value.$promise;
promise = value.$promise;

if (data) {
// Need to convert action.isArray to boolean in case it is undefined
// jshint -W018
if (angular.isArray(data) !== (!!action.isArray)) {
throw $resourceMinErr('badcfg', 'Error in resource configuration. Expected ' +
'response to contain an {0} but got an {1}',
action.isArray?'array':'object', angular.isArray(data)?'array':'object');
throw $resourceMinErr('badcfg',
'Error in resource configuration. Expected ' +
'response to contain an {0} but got an {1}',
action.isArray ? 'array' : 'object',
angular.isArray(data) ? 'array' : 'object');
}
// jshint +W018
if (action.isArray) {
value.length = 0;
forEach(data, function(item) {
value.push(new Resource(item));
forEach(data, function (item) {
if (typeof item === "object") {
value.push(new Resource(item));
} else {
// Valid JSON values may be string literals, and these should not be converted
// into objects. These items will not have access to the Resource prototype
// methods, but unfortunately there
value.push(item);
}
});
} else {
shallowClearAndCopy(data, value);
Expand Down
21 changes: 21 additions & 0 deletions test/ngResource/resourceSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,27 @@ describe("resource", function() {
$httpBackend.flush();
expect(user).toEqualData([ {id: 1, name: 'user1'} ]);
});

it('should not convert string literals in array into Resource objects', function() {
$httpBackend.expect('GET', '/names.json').respond(["mary", "jane"]);
var strings = $resource('/names.json').query();
$httpBackend.flush();
expect(strings).toEqualData(["mary", "jane"]);
});

it('should not convert number literals in array into Resource objects', function() {
$httpBackend.expect('GET', '/names.json').respond([213, 456]);
var numbers = $resource('/names.json').query();
$httpBackend.flush();
expect(numbers).toEqualData([213, 456]);
});

it('should not convert boolean literals in array into Resource objects', function() {
$httpBackend.expect('GET', '/names.json').respond([true, false]);
var bools = $resource('/names.json').query();
$httpBackend.flush();
expect(bools).toEqualData([true, false]);
});
});

describe('get', function(){
Expand Down

3 comments on commit f0904cf

@caitp
Copy link
Contributor Author

@caitp caitp commented on f0904cf Jun 23, 2014

Choose a reason for hiding this comment

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

@owendismuke
Since this commit, my parameter is being converted from a string into an array for each character. e.g. 'EHACWF5HWC' becomes '0=E&1=H&2=A&3=C&4=W&5=F&6=5&7=H&8=W&9=C'. Also, the parameter is thrown at the end after a ? instead of following the url template like it was on Friday.

For example:
On Friday, my template of $resource('trip/:token', {token: '@token'}) resulted in trip/EHACWF5HWC
Today, the same template results in trip?0=E&1=H&2=A&3=C&4=W&5=F&6=5&7=H&8=W&9=C

Are you sure that was this commit? because this isn't related to URL serialization, this is literally only for creating Resource objects from responses --- nothing to do with requests

@owendismuke
Copy link

Choose a reason for hiding this comment

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

@caitp
Are you sure that was this commit? because this isn't related to URL serialization, this is literally only for creating Resource objects from responses --- nothing to do with requests

You're right. I don't see this transformation happening in ngResource. I thought I did yesterday, but realized that isArrayLike(obj) (angular.js:264) was considering my string literal to be array like. This in turn was causing angular.forEach (angular.js:308) to run iterator.call(context, obj[key], key) on my literal. Which then passes 0=E&1=H&2=A&3=C&4=W&5=F&6=5&7=H&8=W&9=C instead of token="EHACWF5HWC" in the dst object through angular.extend (angular.js:422) to my $resource param.

I initially thought this commit to be the culprit since it was the only one I could find between last Friday and this Monday (edit: I now notice this was committed 11 days ago). However, hardcoding the value of token in the $resource call worked when passing it as a value from the controller did not. Which is why I deleted my comment yesterday. I cannot find what change is making my parameter pass as an array instead of a key, value object, but it has completely shut down my development. I guess my next step is to create a test project that I can share with fresh files to see if I can recreate my issue.

Regards and apologies,
Owen

@owendismuke
Copy link

Choose a reason for hiding this comment

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

UPDATE: The problem was completely on my end. My controller's code calling the api changes and was passing just the literal instead of telling which parameter to assign the value.
i.e. api.Trip.get(data.token) - Incorrect
api.Trip.get({token: data.token}) - Correct.

Although, this presents an interesting problem (feature?). A string literal passed as an argument to $resource ends up at the end of the templated url in an array after a '?'.
Thus,
api/trip/EHACWF5HWC
becomes
api/trip?0=E&1=H&2=A&3=C&4=W&5=F&6=5&7=H&8=W&9=C

Regards,
Owen

Please sign in to comment.