jmdobry / angular-cache

angular-cache is a very useful replacement for the Angular 1 $cacheFactory.
http://jmdobry.github.io/angular-cache
MIT License
1.39k stars 156 forks source link

Angular 1.2.18 with $http/localStorage #116

Closed Delagen closed 10 years ago

Delagen commented 10 years ago

Does not work, store response object instead data. (use localStorage) On first request promise returns data because it is not in cache. On second full object as response with "config","status" and other fields where data is in "data" field.(because in cache stored response object)

ghost commented 10 years ago

Same here with angular 1.2.16 and storgage mode "memory" (using angular-cache in version 2.3.6)

vandres commented 10 years ago

Same here (angular 1.2.18, angular-cache 2.3.6.). Response object is saved instead of the data.

jmdobry commented 10 years ago

@Delagen @soeren-lubitz @vandres Code examples? Plnks that demonstrate the issue?

vandres commented 10 years ago
$angularCacheFactory('cache', {
                    maxAge: 86400000, // 1 day
                    storageMode: 'localStorage',
                    deleteOnExpire: 'aggressive',
                    verifyIntegrity: false
                });

$http({method: 'GET', timeout: Config.httpTimeout, cache: $angularCacheFactory.get('cache'), url: "..."})

First request delivers the JS objects. Second (cached) request delivers the response object, where the property "data" holds the JSON string

jmdobry commented 10 years ago

angular-cache can't just assume that the promise to be resolved is one created by an $http call. You could potentially put a promise from any library into angular-cache, and it will put the resolved value into the cache. How would you suggest that angular-cache detect that the resolved value is coming from $http?

vandres commented 10 years ago

I understand. But is it okay to put such a breaking change in a minor update?

jmdobry commented 10 years ago

@vandres No it's not okay. The change wasn't supposed to be breaking. This is a bug.

intellix commented 10 years ago

Personally, I don't see why anyone would want to inject a promise directly into angularCache. It's easy enough and more verbose to do it manually:

promise.then(function(something) {
    cache.put('blah', something);
});

It seems like the ability to store a promise and have it resolve and stored inside angularCache came from $http({ cache: cache }) not working and not by the demand to store a promise

I suppose if you really wanted that functionality, you could check for "success" or "error" as they only exist on $http promises: https://docs.angularjs.org/api/ng/service/$http

jmdobry commented 10 years ago

Fixed https://github.com/jmdobry/angular-cache/releases/tag/2.3.7

jmdobry commented 10 years ago

@intellix The ability to inject promises has been requested before. I had never implemented it because I had never noticed before that $http was actually injecting promise objects into the cache.

vandres commented 10 years ago

Thank you!