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

Jb/638 cache descendants #53

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Jb/638 cache descendants #53

wants to merge 3 commits into from

Conversation

jbx26
Copy link
Collaborator

@jbx26 jbx26 commented Jul 23, 2014

Das hier muss ich nochmal komplett neu angehen, sobald acts_as_dag und cached(:method) fertig sind.

Beim Löschen eines DagLink sollte auch der Cache von ancestor und descendant gelöscht werden.

Die Methode #descendants liefert erst nach einem #reload das korrekte Ergebnis.

Alle Verwendungsstellen von #descendants wurden durch #cached_descendants ersetzt.

Die Methode #cached_descendants ist besser, weil sie intern ein descendant_pages(true) und descendant_groups(true) aufruft. Jede Klasse, die is_structureable ist, hat mindestens Group oder Page als ancestor_class_name.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f3696bb on jb/638_cache_descendants into be7b84c on master.

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)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hier wäre es vermutlich schöner, wenn Du Structureable#descendants überschreibst, sodass Du dann in dieser Methode nur noch descendants(true) aufrufen musst.

def descendants(force_reload = false)
  descendant_pages(true) if force_reload && self.respond_to?(:descendant_pages)
  # ...
end

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 ist übrigens tatsächlich lediglich ein Join auf die Methode #descendant_tablenames von jeder Tabelle. Die einzelnen Methoden #descendant_tablenames sind übrigens mit Parameter force_reload versehen.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.18%) when pulling e0e8342 on jb/638_cache_descendants into fceb907 on master.

@fiedl fiedl force-pushed the master branch 2 times, most recently from b4150a4 to 1be2070 Compare September 26, 2014 20:07
@fiedl fiedl force-pushed the master branch 2 times, most recently from 3e70e6d to af086ae Compare September 30, 2014 10:25
@fiedl fiedl force-pushed the master branch 5 times, most recently from 4478ccc to 39728aa Compare October 25, 2014 19:08
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

Successfully merging this pull request may close these issues.

3 participants