$resource can decorate arrays and errors

[kmccullough]

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Adds ability to use 'arrayDecorate' and 'errorDecorate' options to decorate a resource with additional properties in the response. The arrayDecorate option allows copying non-array indice object properties from the response to the result resource array, for resource collection. This is helpful when the response contains an object containing the collection as well as meta-data related to the collection. The 'errorDecorate' options allows copying object properties from the response to the resource, even where the request was rejected. This is helpful when the request fails, but the API returns useful error messages.

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

• The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:

[gkalpak]

Thx for the PR. This has been discussed before (e.g. #6138) and is generally somewhat problematic (i.e. there doesn't seem to be a clean/non-magical way to do it). Note that as the complexity gets higher, it is often simpler and more maintainable to use the low-level $http or write your app-specific wrapper using $http.

More specifically about this PR:

• I find the errorDecorate quite odd (since you are getting the error response back already).
• For arrayDecorate, I wonder how are you going to put metadata on a JSON array 😕

[kmccullough]

I find the errorDecorate quite odd (since you are getting the error response back already).

The 'errorDecorate' options allows copying object properties from the response to the resource, even where the request was rejected. You can see the response body in the catch() handler response, but you cannot copy properties from the response to the resource, from a transformResponse. This change makes it so those properties are copied over to the resource object.

For arrayDecorate, I wonder how are you going to put metadata on a JSON array 😕

The arrayDecorate option allows copying non-array indice object properties from the response to the result resource array, for resource collection. This is helpful when the response contains an object containing the collection as well as meta-data related to the collection. For example:

{
  "messages": [ ... ],
  "meta": { ... }
}

Which through transformResponse gets converted to an array with a 'meta' property. This change makes it so that these properties are copied to the resource, where normally only array indices are pushed and properties on the array ignored.

[kmccullough]

I may be able to avoid this change, by copying response.resource = value from the $http(httpConfig).then handler into catch handler for the same call. After that, I should then be able to access the original resource object (response.resource) from my interceptor.responseError, for attaching error metadata to the resource. Would that be a preferable PR, @gkalpak?

[gkalpak]

The 'errorDecorate' options allows copying object properties from the response to the resource, even where the request was rejected.

I don't think this a good practice that we should encourage. If a requests fails, it is very confusing to still have a useful resource instance (or array). Again, if you need something special for your usecase, it is preferrable to create your own wrapper around $http or $resource, but this doesn't sound like something that would be more useful than confusing to most people.

The arrayDecorate option allows copying non-array indice object properties from the response to the result resource array, for resource collection.

Yeah, but when returning JSON data it is not possible to have both array indices and metadata properties.

This is helpful when the response contains an object containing the collection as well as meta-data related to the collection.

In that case there will be no items in the array and the collection items will not be Resource instances (which beats the purpose of using $resource in the first place. If this what you are after, then something like #6139 sounds more to the point (but yet again, a custom wrapper would be better imo).

After that, I should then be able to access the original resource object (response.resource) from my interceptor.responseError, for attaching error metadata to the resource. Would that be a preferable PR?

As mentioned above, I don't like the idea of attaching error metadata to a resource instance (or array) 😁

[kmccullough]

I don't think this a good practice that we should encourage. If a requests fails, it is very confusing to still have a useful resource instance (or array). Again, if you need somethng special for your usecase, it is preferrable to create your own wrapper around $http or $resource, but this doesn't sound like something that would be more ueful than confusing to most people.

What makes it useful? You always get back a resource instance from a request. How is it confusing, when you always have a resource object returned from the request, in the current implementation? I can't just add a wrapper around $http or $resource, because the resource created is a private variable until it is added to response.resource in the then clause.

Yeah, but when returning JSON data it is not possible to have both array indices and metadata properties.

Hence why my example doesn't show that:

{
  "messages": [ ... ],
  "meta": { ... }
}

Which requires transformation, obviously.

In that case there will be no items in the array and the collection items will not be Resource instances (which beats the purpose of using $resource in the first place. If this what you are after, then something like #6139 sounds more to the point (but yet again, a custom wrapper would be better imo).

Nonsense. Do you not see the part of the code that copies over the array indices and then separately the non-numeric object properties? They are added as Resources, nothing changes there. The meta properties wouldn't be Resources, but then why would you expect them to be? You don't iterate the object properties when you iterate the array!

As mentioned above, I don't like the idea of attaching error metadata to a resource instance (or array)

My suggestion at the end was not to attach any meta data, but only to give the interceptors.responseError access to the response.resource, which would make this change moot and give me everything I need to implement this locally.

Basically just add this to the $http request:

.catch(function(response) {
  response.resource = value;
  $q.reject(response);
})

The only problem I have with the above is that the explicit rejection fails a bunch of resourceSpec tests that expected a failure, but not the explicit rejection, and errors "possibly unhandled rejection".

[gkalpak]

We obviously see things different, so there is no point in keep arguing 😃
It seems that we do agree in one things though: Setting response.resource = value for failed requests makes perfect sense, so feel free to send a PR for that (or update the current one).

I tried adding it and no test failed (as expected, since there is already a rejection):

promise = $http(config).then(function(response) {
  // Handle success
  // ...
}, function(response) {
  // Handle failure
  response.resource = value;
  return $q.reject(response);
});

[kmccullough]

Closing this in favor of #16109