-
Notifications
You must be signed in to change notification settings - Fork 13
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
Jb/638 cache descendants #53
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,7 +121,20 @@ def destroy_links | |
end | ||
|
||
def delete_cache_structureable | ||
delete_cached_descendants | ||
delete_cache_roles | ||
end | ||
|
||
def delete_cached_descendants | ||
Rails.cache.delete([self, 'descendants']) | ||
end | ||
|
||
def cached_descendants | ||
Rails.cache.fetch([self, 'descendants'], expires_in: 1.week) do | ||
descendant_pages(true) if self.respond_to?(:descendant_pages) | ||
descendant_groups(true) if self.respond_to?(:descendant_groups) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hier wäre es vermutlich schöner, wenn Du def descendants(force_reload = false)
descendant_pages(true) if force_reload && self.respond_to?(:descendant_pages)
# ...
end There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ja, gute Idee. Leider wurde genau diese Methode in has_dag_links vergessen. Die Methode |
||
descendants | ||
end | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das gefällt mir eigentlich nicht so gut, da wir dann wieder mischen, was einen Cache verwendet und was nicht. Wenn Du die Methode
<<
verwendest, weißt Du nicht, dass intern bereits ein Cache verwendet wird.Ich mache mir nochmal Gedanken zu diesem Thema. Ein paar Ideen habe ich schon. Ich melde mich diesbezüglich per Trello.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meine Konvention war bisher, dass es transparent ist, ob eine Methode einen Cache verwendet oder nicht. Lediglich die Cache-Methode selbst hat ein Duplikat. Das aber nur zum Zwecke der Qualitätssicherung. Von mir aus können wir auch gerne auf das neue Caching-Konzept umsteigen. Es spart viel Schreibarbeit. An dieser Stelle muss auf jeden Fall eine Korrektur erfolgen, da die Methode
#descendants
ein falsches Ergebnis liefert.