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

feat: add caching infrastructure #424

Open
braaar opened this issue Feb 6, 2023 · 2 comments
Open

feat: add caching infrastructure #424

braaar opened this issue Feb 6, 2023 · 2 comments

Comments

@braaar
Copy link
Collaborator

braaar commented Feb 6, 2023

Currently, bank transactions are tied to custom invoices or subscriptions through the foreign keys payment_transaction on CustomInvoice and last_payment on ServiceSubscription, which are populated when running the BusinessLogic.updateuser function. The latter field, last_payment, is expected to change every so often.

While I understand that it is useful to have this field for performance reasons (so we don't have to go through all transactions every time we load the member page), I think the current implementation is a bit problematic.

What we are doing is essentially caching some data by storing it in the database. This is fine as long as there is a performance justification for doing so (which I think there is since going through all the transactions on demand could be quite performance heavy).

However, what I find problematic is that this is not clearly communicated as a cache. I think it would be appropriate to make some changes to make it clear that the foreign keys payment_transaction on CustomInvoice and last_payment on ServiceSubscription are cached data that has been computed based on the actual data basis which is the bank transaction reference number and the subscription/invoice reference number.

They contain the value that was calculated when BusinessLogic.updateuser was last called for this user, not any meaningful and original information.

It could even make sense to move this data to a separate cache service that takes in the user id and looks for a recent cache entry. The service should invalidate cache entries that are too old and recalculate them as needed.

I think the cache services should recalculate a cache entry if:

  1. The requested entry is missing, or
  2. Bank transactions have been added since the cache entry was created, or
  3. The cache entry is more than a week old (or maybe some user configurable number)

I think the current approach is confusing for developers and prone to user mistakes (I think the re-calculate button is a bit hacky).

While it may seem superfluous at this time, I think this kind of caching can become more and more useful in the future, and we at Hacklab Jyväsklyä are working on some features that would benefit greatly from this architecture (#425).

Dealing with caching properly can also be useful for addressing #410, #409, #214.

@braaar
Copy link
Collaborator Author

braaar commented Feb 6, 2023

The user field on BankTransaction is in the same boat as the other fields I mentioned. In practice it probably won't change a whole lot, but this information derives from other data and is set by updateuser as well.

@braaar braaar changed the title feat: Caching infrastructure feat: add caching infrastructure Feb 6, 2023
@braaar
Copy link
Collaborator Author

braaar commented Feb 6, 2023

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

1 participant