kql icon indicating copy to clipboard operation
kql copied to clipboard

add an optional cache per query

Open bnomei opened this issue 3 years ago • 4 comments

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

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

bnomei avatar Jul 14 '20 18:07 bnomei

Uhhh, that's a nice idea!

bastianallgeier avatar Jul 15 '20 07:07 bastianallgeier

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.

lukasIO avatar Feb 26 '21 20:02 lukasIO

@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.

bastianallgeier avatar Mar 01 '21 09:03 bastianallgeier

Two details we need to be careful about:

  • If the cache is controlled by the request, attackers could easily fill up the cache with loads of different automated requests. I think we should avoid that, even more with the new feature that allows unauthenticated KQL requests.
  • It could be possible to access "private caches" with data from other users by faking the cache key for retrieval. E.g. user A requests data that depends on the currently logged in user and user B retrieves that cached data by using the same cache key.

Possible solution:

  • Each KQL response could contain a new field hash. This hash would be calculated from the request data (the query and all the options, e.g. for select etc.). However the hash would need to be calculated deterministically from a JSON string with a normalized structure (normalized key order etc.) to ensure that two requests with the same data share the same hash.
  • Devs can now copy the hashes from the queries they want to cache and enter them in site/config/config.php. There would be a list of cacheable hashes, each with the cache duration:
<?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.

lukasbestle avatar May 26 '22 13:05 lukasbestle

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?

distantnative avatar Nov 21 '22 08:11 distantnative

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

lukasbestle avatar Nov 24 '22 20:11 lukasbestle

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.

distantnative avatar Dec 05 '22 18:12 distantnative