Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Caching promises doesn't work as expected #251

Open
ahmadalfy opened this issue Nov 23, 2016 · 9 comments
Open

Caching promises doesn't work as expected #251

ahmadalfy opened this issue Nov 23, 2016 · 9 comments

Comments

@ahmadalfy
Copy link

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 #115 and here #143 but nothing fix it

@jmdobry
Copy link
Owner

jmdobry commented Dec 12, 2016

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
Copy link
Author

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

@merqlove
Copy link

merqlove commented Jan 1, 2017

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

@ahmadalfy
Copy link
Author

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

@merqlove
Copy link

merqlove commented Jan 2, 2017

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
Copy link

MatheusArleson commented Jun 14, 2018

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
Copy link

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

@MatheusArleson
Copy link

MatheusArleson commented Jun 14, 2018

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

TL;DR

  • Angular will always cache a Promise Object. So internally it uses an cached value as an Promise Object.
  • This library maybe not cache a value as an Promise Object. When the cached data is used by angular (since it is not an Promise Object), $http service will generate an invalid Promise Object.

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

  1. $http creates a Promise of the http call.
  2. $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:

  • If the cached value is a Promise, chain the Promise (resolved or not) with the method resolvePromiseWithResult()
  • If the cached value is an Array, $http will (arbitrarily) use the cached value to resolve an Promise object with the cached array value. (ERROR HERE : This is the case that causes error in this library )
  • If the cached value is anything else, $http will resolve an Promise pass the cached value as the response content.
//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
Copy link

MatheusArleson commented Jun 14, 2018

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
       );

    }
  });
});
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants