Cdn cache improvements 3#2860
Merged
Merged
Conversation
Section 9.3 honestly noted that purging is wired into the controller update/destroy actions and the bulk recalculation, but not a model callback, so a future write path that changes a displayed project field could leave the cached anonymous show page stale until Surrogate-Control max-age expires. Add Section 11 recording, as a deliberate (not-yet-implemented) proposal, an after_commit model callback that purges on any project change. Includes the exact callback, the implementation details that keep it correct (notably gating on skip_callbacks so the bulk recalc's single purge_all is not turned into thousands of per-project purges, after_commit vs after_save, enqueue vs synchronous purge, scope to update/destroy, removing the now-redundant controller purges, and purge-on-any-update over a fragile column allowlist), the pros and cons, the residual cross-model (user_display_name) limitation, and tests. Doc-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
projects#update previously scheduled the delayed re-purge only inside the "@project.save succeeded" branch. But update_additional_rights writes the AdditionalRight table (rendered on the anonymous /permissions page) in its own transaction, which commits independently of @project.save and is not rolled back if the save later fails. On a save-fails-after-rights-changed path, only the early immediate purge fired -- losing the delayed re-purge that closes the TCP-reorder race, so the CDN could hold obsolete collaborator data until max-age. Schedule the delayed re-purge unconditionally on entry to update (after the request is authorized), alongside the early immediate purge, and drop the now -redundant post-save delayed purge (keeping the post-save immediate purge for freshest-data eviction). Extra purges are harmless and the request is already authorized by that point. Apply the same immediate+delayed pair to destroy, which previously did only an immediate purge: a stale re-cache of a deleted project's page is exactly the case worth the extra re-purge. Also correct an outdated comment: ActiveJob is backed by solid_queue (a database queue) in production, so the delayed purge is durable and survives a restart mid-wait -- it is not "stored in RAM" as the old comment claimed. Add a regression test (CdnCachingTest): a permissions-only update (changing only additional_rights, project row unchanged) must enqueue PurgeCdnProjectJob with the project's record_key. Guards against re-gating the purge on @project.saved_changes?, which would silently serve stale rights. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2860 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 79 79
Lines 4331 4332 +1
=========================================
+ Hits 4331 4332 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.