ngx-rocket / generator-ngx-rocket

:rocket: Extensible Angular 14+ enterprise-grade project generator
https://ngx-rocket.github.io/
MIT License
1.53k stars 217 forks source link

The HttpClient cache doesn't store and returns http headers response #440

Closed manzapanza closed 4 years ago

manzapanza commented 5 years ago

I'm submitting a...

Current behavior

The HttpClient cache doesn't store and returns http headers response. When the request cache is empty the headers are returned, when the response come from cache no one header is returned.

Expected behavior

All headers should be stored in the cache and returned

Minimal reproduction of the problem with instructions

getApartments() {
  this.http.get<any>(`/apartments`, { observe: 'response' }).subscribe(response => {
    console.log('apartments:');
    console.log(response.body);
    console.log('all headers:');
    response.headers.keys().forEach(key => console.log(`${key}: ${response.headers.get(key)}`));
  });
}

console.log('First time with empty cache:');
this.getApartments();

console.log('Second time when get response from cache:');
this.getApartments();

// 'First time with empty cache:'
// apartments:
// (15) [{…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}]
// all headers:
// date: Thu, 21 Mar 2019 12:55:59 GMT
// server: Apache/2.4.25 (Debian)
// x-powered-by: PHP/5.6.36
// vary: User-Agent
// content-type: application/json; charset=UTF-8
// access-control-allow-origin: *
// connection: close
// content-length: 105838
// x-total-count: 115
// Second time when get response from cache:
// ERROR TypeError: data.httpResponse.headers.keys is not a function

Environment

- generator version: 5.3.0
- node version: 10.4.1
- npm version: 6.1.0
- OS:  Mac

Others:

creal73 commented 5 years ago

Hi @manzapanza !

I just checked, HttpCacheService stores the whole HttpResponse object and returns it when cache is hit (have a look at http-cache.service.ts)

// ERROR TypeError: data.httpResponse.headers.keys is not a function

seems to not be the same code as in your method 'getApartments()'

manzapanza commented 5 years ago

Yes but it don't works because the response headers are not an object but are inside a function. To read an header you need to call response.headers.get(key) and not simply by response.headers[key]. If you look at the cache stored (in my case is localStorage) ther's not headers stored there:

image

So, in my opinion we should store headers inside the cache as as a plain key/value object and when restore the HttpResponse, restore also the HttpHeaders and pass here as second parameter:

https://github.com/ngx-rocket/generator-ngx-rocket/blob/08e68d6f96d8172db10f9e2e074a7b449cf183ff/generators/app/templates/src/app/core/http/cache.interceptor.ts#L40

sinedied commented 5 years ago

This seems indeed a bug, thanks also for providing a PR to fix it. Though, I have to be honest on this one and we were discussing recently about removing the cache service entirely, in favor of integration of state management libraries.

Still, I'm willing to merge the fix in the meantime, and we are still open to feedbacks whether what we should do ;)

sinedied commented 4 years ago

Dropping this one as cache service will be removed in next version (#534)