Implement caching system to be used for page load functions

[Macludde]

This PR adds a naive in-memory caching system which works like this:

You have an async method you would like to cache, 99% a database request but does work for stuff like Github commit data etc.

You have to decide, is this data user-specific or global? (For example, alerts are global because we want every visitor to see the same alerts, most data is user-specific due to our access system)

Depending on the answer you either wrap your async call in a globallyCached(...) or userLevelCached(...) method. What this does is save the return value in an in-memory cache on the server, and next time it's called does a cache query first.

The cache system is purged every so often, and cache lifetime is set per-resource (with a default of 5 minutes IIRC)

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
web	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 9, 2024 2:23pm

[danieladugyan]

I'll paste this comment from #400 here as well:

I'm not a huge fan of caching Prisma calls like this unless strictly necessary.

A longer explanation. Leaving this as is kind of implies that this pattern is "correct" from an access control standpoint, and I will argues that it's not. Here's why:

1. AdminSetting has the following read policy: @@allow("read", has(auth().policies, "admin:settings:read"))
2. A user may or may not have this access policy. In this specific case, it doesn't matter because this function is currently only called with an authorized Prisma client (i.e no access control). This could change in the future though – there's nothing enforcing this.
3. In general, caching data that's protected by access control policies requires careful consideration since it completely bypasses our access policy system. A privileged user's permissions could be used to fill the cache, and subsequent requests will succeed no matter whether the user is authorized. Equally, a non-privileged user's permissions could be used to fill the cache (with empty data) and subsequent requests will fail no matter whether the user is authorized. None of these cases are good.

[Macludde]

I see your comment Daniel and guess it's disputing that perhaps ALL data is user-level, because even Alerts might some day not be the same for everyone. At some point I think we have to think about performance over potential future problems (which can be solved then), and right now our landing page takes almost a second to load, every time.

At least implementing user-level caching improves performance a lot for someone when you click around within the app, or go back and forth between pages.

[Isak-Kallini]

The /api/home endpoint already caches the commit data to avoid getting rate limited by Github, should probably extract the logic and remove the endpoint entirely

[danieladugyan]

I've given this a lot of thought over the past weeks and frankly I haven't really reached any sort of conclusion. But I'll put down some thoughts here while we await opinions from other DWWW-members.

I see your comment Daniel and guess it's disputing that perhaps ALL data is user-level, because even Alerts might some day not be the same for everyone.

This is a great point that really puts our entire model for access control into question. I pushed for the idea of database-level access control because I hoped it would make it easy to enforce access control everywhere. There's no risk of ever forgetting to enable access control on a specific server function, because it's handled at a lower layer.

However, that starts to break down when we have operations that should ignore access control. We can easily spot these operations since they occur wherever authorizedClient is used. In principle, that's equivalent to this caching PR – it sidesteps our access control code. Needing to sidestep the access control model is a sign of a flawed access control model and to me it's a typical case of a leaky abstraction.

At some point I think we have to think about performance over potential future problems (which can be solved then), and right now our landing page takes almost a second to load, every time.

This is a very reasonable take, but not one I care much about (but that's just a matter of priorities). I think a one second loading time is fine. Obviously it's not ideal, but I think it's unlikely to affect usage of our web page. Caching (as implemented in this PR) on the other hand is famously error-prone (see especially this, or this quote) so I don't think it's worth the trade-off

At least implementing user-level caching improves performance a lot for someone when you click around within the app, or go back and forth between pages.

It does, and I have always wanted user-level caching. However, my idea was to implement it using service workers on the client side. This seems less error-prone, but I haven't tried implementing it yet.

Summary:

1. Maybe we should rethink the way we handle access control? We could revert to defining access control rules manually in JavaScript for each operation. If we do then server-side caching could be done without overriding our conceptual model for access control.
2. I sometimes care more about DX than UX 😮
3. There are other approaches to caching - I think it's worth exploring some other options that might result in a way simpler developer experience: service workers, client side data fetching, prisma level caching, Redis? etc?