getkirby / kql

Kirby's Query Language API combines the flexibility of Kirby's data structures, the power of GraphQL and the simplicity of REST.
https://getkirby.com
MIT License
143 stars 5 forks source link

add an optional cache per query #13

Closed bnomei closed 1 year ago

bnomei commented 4 years ago

implemented via usual plugin cache. config via each query...

{
    "query": "page('photography').children",
    "cache": {
       "key": "myquery-{{ page.id }}",
       "expire": 60
    },
    "select": {
...
bastianallgeier commented 4 years ago

Uhhh, that's a nice idea!

lukasIO commented 3 years ago

Just chiming in here, that I've been excited about kql for a while and super happy to see it reached 1.0.0. A caching layer would be the last missing puzzle piece for me to consider it for bigger projects.

Additionally I'd like to put an automatic cache option up for discussion, that would allow all incoming queries to be auto-cached. In my (rather naive) way of thinking I imagine that that kind of cache could be (also automagically) invalidated on any (relevant or not) file changes.

bastianallgeier commented 3 years ago

@lukasIO with such a cache layer I don't see any problems to introduce a global option for it. Yes, such a cache would be automatically erased whenever something changes in the panel. But be aware that this could not automatically listen to changes in the file system. You would still need to purge the cache manually in such cases.

lukasbestle commented 2 years ago

Two details we need to be careful about:

Possible solution:

<?php

return [
  'kql' => [
    'cache' => [
      'a6c2ef...' => 60
    ]
  ]
];

For queries that return dynamic data, there could be a callback to control the cache key extension, which would be appended to the hash (if not provided, just the hash would be used):

<?php

return [
  'kql' => [
    'cache' => [
      'a6c2ef...' => [
        'duration' => 60,
        'key' => fn($query) => 'my-custom-key'
      ]
    ]
  ]
];

For scenarios where the request can be fully trusted, the kql.cache option could be set to true to allow caching by the request like Bruno suggested in the first post.

distantnative commented 1 year ago

Reading this, automatic caches seem to be rather complex (actually very much the same as automatically normal Kirby queries in the core). Should we maybe first just tackle manual caching?

lukasbestle commented 1 year ago

Even with manual caching we need to make sure that the cache cannot be controlled from the request for the mentioned reasons.

distantnative commented 1 year ago

Lukas and I talked a lot about the potential implementation of this. In the end, I think this is getting too complex and complicated and too niche the it would justify the benefit for most users. I'd vote to close this as not planned from the core team.