fix: invalidate template cache and use live template when sending notifications [DHIS2-21836] - #24732
fix: invalidate template cache and use live template when sending notifications [DHIS2-21836]#24732enricocolasante wants to merge 1 commit into
Conversation
…ifications [DHIS2-21836]
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #24732 +/- ##
=============================================
+ Coverage 35.47% 47.58% +12.10%
+ Complexity 533 388 -145
=============================================
Files 3713 3705 -8
Lines 143929 144159 +230
Branches 16776 16823 +47
=============================================
+ Hits 51064 68595 +17531
+ Misses 88660 68988 -19672
- Partials 4205 6576 +2371
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2629 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
| // The metadata import pipeline persists directly through the store and bypasses | ||
| // ProgramNotificationTemplateService, so the getByUidCached cache would otherwise keep serving | ||
| // the pre-edit template (e.g. still delivering email after the channel was deselected). | ||
| programNotificationTemplateService.invalidateCache(template.getUid()); |
There was a problem hiding this comment.
invalidateCache is called from postCreate/postUpdate/preDelete, all of which run inside ObjectBundleService.commit()'s @Transactional boundary before commit. I think this is the same issue as the evict-at-flush in #24810 (review). This will remove a cached entry causing a concurrent reader to miss and fetch the template. PGs default isolation read-committed means the reader could still read a stale template (like the one that was just evicted). Only the TTL or another edit to the template would then evict that stale template.
The difference to the linked PR is that Hibernate also evicts after commit, which clears the stale entry unless the reader's put lands after it (see PR comment for the race it faces).
We could try to use TransactionSynchronization and only evict in afterCommit. That fixes the above issue. A race is still possible: cold cache, and the reader's SELECT must precede commit-visibility while its put lands after afterCommit. The difference is that evicting inside the transaction creates the miss, so any reader during an edit gets a stale entry, while with afterCommit the reader has to arrive already-missing.
4 participants
Summary
Fixes DHIS2-21836: after editing a program notification template to deselect the email delivery channel, users still received emails (even with no channel or only SMS selected), and clearing the cache did not help. The template was being read from an in-memory cache that the metadata edit path never invalidated, and scheduled program-rule notifications were reading a frozen template snapshot instead of the current template. Both are now fixed so channel edits take effect immediately.
Changes
ProgramNotificationTemplateObjectBundleHook.java): template edits go through the metadata/ObjectBundle pipeline, which bypassesProgramNotificationTemplateService.save/update/delete— the only place thegetByUidCachedcache was invalidated. The hook now evicts the cache inpostCreate,postUpdate, andpreDelete, so a deselected channel is honored on the next notification instead of serving the stale pre-edit template.ProgramNotificationTemplateService.java,DefaultProgramNotificationTemplateService.java): addsinvalidateCache(String uid)to the interface and implementation, and routes the existing save/update/delete invalidations through it.DefaultProgramNotificationService.java):getApplicableTemplatenow uses the live database template and only falls back to the frozen JSONB snapshot when the template has been deleted. This makes edits take effect on already-scheduled program-rule notifications while preserving the snapshot's purpose of surviving template deletion. The misleadinglog.warnnow fires only when no template can be resolved at all.NotificationTemplateMapper.java): correctssetSendRepeatable(t.isSendRepeatable()), which read from the snapshot being built instead of the source template, sosendRepeatableis now captured.ProgramNotificationTemplateObjectBundleHookTestverifies the cache is invalidated on create/update/delete;ProgramNotificationServiceTestadds cases proving the live template wins over a stale snapshot and that the snapshot is still used when the template was deleted.