freshcoders / discorki

MIT License
1 stars 0 forks source link

Add caching #48

Closed jessedezwart closed 1 year ago

jessedezwart commented 1 year ago

Closes #45 and #16

ikdekker commented 1 year ago

I think there's a condition in which it still would be nice to have caching for matches.

After looking at the execution flow, I guess the personal matches are a bigger motivator for caching reworking the logic.

We currently look for notable event on summoner level (source), then we check for match/team notifs AND personal notifs (for EACH summoner). This does not make much sense in hindsight. It would require us to fetch the match for each tracked summoner in the team. What I think would make more sense is to

  1. Loop tracked summoners;
  2. Get the match for the current summoner, if it was not locked (another summoner that was on the same team is being checked, only required in a concurrent check) else exit;
  3. Get the trackedSummoners in this match (we already do)
  4. Instead of calling the personal checkers once in a summer, call it for all tracked summoners

Side note:

If we were to implement a parallel stream for checking summoners (which is likely in the future), the checkForNotableEvents method has to be used to determine whether a match was notified for. We can not check in parallel, for people on the same team, if we check for notifications sent on match-level and we keep the old per-summoner-check.

We could do the check earlier (if choosing the proposed logic), if we have the previous match information (which the summoner is just out of). The team will be known and we can remove any users on the same team from the trackedSummoners. This way, only one summoner will cause the match to be checked, propagating to the summoners that were in this game too. Resulting in only 1 outofgame check (saves 1 API call per team member), one match history fetch and the actual match fetch (2 API calls).

All-in-all, I believe it is currently not worth saving match history in cache.

ikdekker commented 1 year ago

For the cache to work, I believe we need the @EnableCaching annotation read here.

If I add the annotation to the LeagueApiController, I get the error below

image