caching?

[KieronWiltshire]

Any plans for caching mechanisms?

[spruce-bruce]

I've actually been thinking about caching, but I haven't really planned to submit a PR to implement into the library.

One of the things I like about virgen-acl is that it's dirt simple and can be conformed to almost any use case (that I have anyway). If virgen-acl implements caching then it will

1. Be one more configuration step
2. Be an opinionated configuration step

If I write caching into virgen-acl for example I'm going to write a redis cache layer, which will probably suit most people, but it seems like A Bad Thing to turn away memcached die hards, you know? Or there are people who would be perfectly suited using an in-memory cache that I'm not particularly interested in writing because I wouldn't use it in my projects (I want TTL and I don't want to write TTL).

My plan was to implement my own caching in my .allow() callbacks, but if you don't need .allow() callbacks then you won't be able to cache (and it won't cache the role -> resource -> permission lookup).

That being said, though, now that you've got me thinking about it I do like the idea of caching the permission lookup and not even executing the .allow() callback if there's a cached value. My own use case allows me to implement this cache before I call query, but maybe it's worthing talking about building into the lib?

If you have suggestions, though, this is likely something I'm going to be exploring in the near future and if @djvirgen is on board it might be something worth implementing directly into the library. In the mean time the simplest way to implement caching would be something like

var role = 'guest', resource = 'blog', action='edit';
var result = aclCacheQuery(role, resource, action);

if (typeof cacheResult === 'undefined') {
  result = acl.query(role, resource, action);
}

It's some extra boilerplate to your .query() calls, but it'll work

[djvirgen]

I like the idea of caching support, but I agree with @spruce-bruce that an opinionated solution might make this code less portable. One thing we can do to get around that is to provide a cache interface that you could use to wrap your own custom caching solution, for example (in pseudocode):

interface AclCache {
  @return this - fluent inteface
  public function set(key, value, ttl);

  @return boolean|null
  public function get(key);
}

The idea being you could implement this interface and provide the instance to VirgenACL:

class MyCache {
  constructor(backend) {
    this.backend = backend;
  }

  set(key, value, ttl) {
    this.backend.store(key, value, ttl);
    return this;
  }

  get(key) {
    let value = this.backend.load(key);
    return (typeof value !== 'undefined') ? value : null;
  }
}

let myCache = new MyCache(customCacheBackend);
assert(typeof myCache.set == 'function');
assert(typeof myCache.get == 'function');
$acl->setCache(myCache);

When a cache is provided, VirgenACL can automatically write to it after successfully determining access, and fetch from it prior to checking for access. If the AclCache#get function returns null, we can assume that it was a cache miss.

We may need to think through this a bit more but overall what do you guys think of this approach?

[spruce-bruce]

That suits me quite well. The other thing I hadn't thought about is the case where people are using this on the frontend and won't ever care about redis/db/whatever caching. The interface solution handles the frontend case perfectly in that you provide a different cache adapter for in your frontend acl instance than your backend.

👍 from me!

I can probably volunteer some time to implement as well, though not likely in the near term (and if not me then I might make one of my coworkers do it). Either way I think this is a good idea.

We would also end up writing a redis adapter that we could then publish for anybody to use

[djvirgen]

Awesome! If I get some free time soon, I'll work on it and update you all here before I get started. Thanks for the feedback!

[KieronWiltshire]

Instead of calling from cache, and if cache doesn't exist, then call virgen, perhaps implement a layer on top that does the caching behind the scenes, so:

virgen = require('virgen-acl)(someCacheLayer);

// continue to use virgen as normal.

I personally don't think there's a need for TTL, what I would personally do is, whenever something is added/removed from a role or whatever, invalidate the relevant cache keys

[spruce-bruce]

Instead of calling from cache, and if cache doesn't exist, then call virgen, perhaps implement a layer on top that does the caching behind the scenes, so:

This is essentially what @djvirgen proposed, and will work really nicely. I was just giving an example of how to implement caching on top of virgen-acl until caching gets implemented into the library, since there's not really a good way to know when anybody will be able to free up time for this addition.

I personally don't think there's a need for TTL, what I would personally do is, whenever something is added/removed from a role or whatever, invalidate the relevant cache keys

The use case for TTL (or some other manual invalidation) is for people (like myself) who are using callbacks in their .allow() calls to do extra logic on top of what virgen-acl provides.

I think TTL is important, but doesn't need to be implemented by virgen-acl, this could be left up to the individual cache adapters.