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

Caching promises doesn't work as expected #251

Open ahmadalfy opened 7 years ago

ahmadalfy commented 7 years ago

Using Angular 1.5.8 and angular-cache 4.6

In my service:

    if (!CacheFactory.get('projectCache')) {
      CacheFactory.createCache('projectCache', {
        deleteOnExpire: 'aggressive',
        recycleFreq: 60000
      });
    }
    projectCache = CacheFactory.get('projectCache');

    return {
      getProject: function(id, slug) {
        return $q(function(resolve, reject) {
          $http.get(apiCall, { cache: projectCache })
          .success(function(data) {
            resolve(data);
          }).error(function(data) {
            reject("Server didn't send the correct data");
          });
        });
      }

In my localStorage the value is stored like this:

cachedResp: Array[4]
    0: 200
    1: "Stringified JSON object"
    2: Object
    3: "OK"

It's basically the same issue here https://github.com/jmdobry/angular-cache/issues/115 and here https://github.com/jmdobry/angular-cache/issues/143 but nothing fix it

jmdobry commented 7 years ago

I think it might be that Angular.js 1.x changed the format of the $http response data that it saves to the cache. Ugh, angular-cache would have to change its behavior depending on which version of Angular is being used.

ahmadalfy commented 7 years ago

I tried searching Angular docs for any documentation for that update but I didn't find any.

merqlove commented 7 years ago

Same here. Yep seems that response format was changed...

ahmadalfy commented 7 years ago

Note that starting from 1.6, $http deprecated callback methods: success()/error() were removed. I am not sure how that would affect promises

merqlove commented 7 years ago

For our needs i'm just added wrapper around CacheFactory, to drop token headers from the cache. Currently sometimes $http uses cache.put with Promise and sometimes with plain Array.

MatheusArleson commented 6 years ago

Facing the same issues here, using angular 1.6.7

1 - $http now does not have sucess() / error() methods anymore. 2 - If the value for an URL inside the cache is an array, angular will use the cached array value arbitrarily. (this is related to issues #264, #262, #251)

//extract from the $http service file of angular v1.6.7
   function sendReq(config, reqData) {
  ....
if (cache) {
        cachedResp = cache.get(url);
        if (isDefined(cachedResp)) {
          if (isPromiseLike(cachedResp)) {
            // cached request has already been sent, but there is no response yet
            cachedResp.then(resolvePromiseWithResult, resolvePromiseWithResult);
          } else {
            // serving from cache
            if (isArray(cachedResp)) { //<---- this will be executed case cached value is an array
            //notice the usage of fixed indexes here....
              resolvePromise(cachedResp[1], cachedResp[0], shallowCopy(cachedResp[2]), cachedResp[3], cachedResp[4]);
            } else {
              resolvePromise(cachedResp, 200, {}, 'OK', 'complete');
            }
          }
        } else {
          // put the promise for the non-transformed response into cache as a placeholder
          cache.put(url, promise);
        }
      }
  ...
}
MatheusArleson commented 6 years ago

If you replace the CacheFactory for the angular $cacheFactory, and cache an array, the same problem does not happen.

MatheusArleson commented 6 years ago

Investigating further, the reason it works with $cacheFactory for caching an array is simply the way angular uses the cached value.

TL;DR

How it works under the hood, (angular 1.5.X and later)

0) $http creates a Promise of the http call. 1) $http will checks the cache (if a cache is available).

If the cache do not have a cached value: 2) $http will put the promise object in the cache (resolved or not) 3) $http wil return the promise. Notice that eventually that promise will resolve, and the cache will have it resolved for the next call.

If the cache has a cached value:

//extract from Angular $http Service
    function sendReq(config, reqData) {
  ....
if (cache) {
        cachedResp = cache.get(url);
        if (isDefined(cachedResp)) {
          if (isPromiseLike(cachedResp)) {
            // cached request has already been sent, but there is no response yet
            cachedResp.then(resolvePromiseWithResult, resolvePromiseWithResult);
          } else {

           // serving from cache
            if (isArray(cachedResp)) {

                //this block uses any Array object coming from the cache
               // ---> EVEN IF IT IS NOT AN PROMISE OBJECT <----
               //notice the usage of fixed indexes here. 
              resolvePromise(cachedResp[1], cachedResp[0], shallowCopy(cachedResp[2]), cachedResp[3], cachedResp[4]);

            } else {
              resolvePromise(cachedResp, 200, {}, 'OK', 'complete');
            }
          }
        } else {
          // put the promise for the non-transformed response into cache as a placeholder
          cache.put(url, promise);
        }
      }
  ...
}

     function resolvePromiseWithResult(result) {
        //result will be the cached promise, so we can use Promise properties
        resolvePromise(result.data, result.status, shallowCopy(result.headers()), result.statusText, result.xhrStatus);
      }

      /**
       * Resolves the raw $http promise.
       */
      function resolvePromise(response, status, headers, statusText, xhrStatus) {
        //status: HTTP response status code, 0, -1 (aborted by timeout / promise)
        status = status >= -1 ? status : 0;

        (isSuccess(status) ? deferred.resolve : deferred.reject)({
          data: response,
          status: status,
          headers: headersGetter(headers),
          config: config,
          statusText: statusText,
          xhrStatus: xhrStatus
        });
      }
MatheusArleson commented 6 years ago

I've solved the issue passing the full Promise Object generated by the $http service to the cache.

Angular will check fot the success of the Promise when resolving it on the resolvePromise() method of the $http service. (check code in the my comment above)

With that approach your Promise content can be anything and angular will take care of properly using it (avoiding the isArray() check and using the cached value array as an Promise object).

//this is the code for configuration of the cache (available on the home page of this library)
angular.module('app', ['angular-cache']).config(function (CacheFactoryProvider) {
  angular.extend(CacheFactoryProvider.defaults, {
      maxAge: 3600000,
      deleteOnExpire: 'aggressive',
      onExpire: function (key, value) {
         var _this = this; // "this" is the cache object in which the item expired

         _this.put(
             key, //the cache key (URL)
             angular.injector(['ng']).get('$http'.get(key)) //get() returns an Promise object of the http call
         );

      }
    });
  });
}