Skip to content

Comments

[FIX] Added fix to allow nested AutoDatasource instances to manage their own populateCache#791

Merged
LukeTowers merged 9 commits intowip/child-themefrom
wip/child-theme-autodatasource-caching
Dec 13, 2022
Merged

[FIX] Added fix to allow nested AutoDatasource instances to manage their own populateCache#791
LukeTowers merged 9 commits intowip/child-themefrom
wip/child-theme-autodatasource-caching

Conversation

@jaxwilko
Copy link
Member

@jaxwilko jaxwilko commented Dec 8, 2022

This PR adds a fix to support nested AutoDatasource instances. The issue with nesting an AutoDatasource inside an AutoDatasource is that the populateCache() method would not proxy the call to the nested instance and instead force the caching against the AutoDatasource instead of the child Datasource instances, this would mean that cache would not be correctly invalidated and after issuing insert, update or delete requests the parent cache would be updated but the instance cache would not be.

This change allows the populateCache() request to be passed into the nested AutoDatasource to allow for recursive behaviour.

@what-the-diff
Copy link

what-the-diff bot commented Dec 8, 2022

  • Added a new method to the AutoDatasource class called getPathCache()
  • Modified populateCache() so that it can handle other instances of itself (AutoDatasources) and merge their path caches together

robertalexa and others added 2 commits December 8, 2022 14:41
* develop:
  Fix issue with autoloading modules (#792)
  Add support for icon picker (#725)
  Classloader improvements (#779)
  Update Markdown content test
  Improved Ukrainian translation (#766)
  Add AJAX form validation to Snowboard (#777)
  Improve theme scaffolding (#778)
@LukeTowers
Copy link
Member

@jaxwilko the tests are more relevant now, can you fix it?

@jaxwilko
Copy link
Member Author

jaxwilko commented Dec 9, 2022

@jaxwilko the tests are more relevant now, can you fix it?

Yeah, so the issue is not caused by this PR, but just becomes apparent because of it. The fix for the issue is here: wintercms/storm#135

@LukeTowers LukeTowers added Status: Completed maintenance PRs that fix bugs, are translation changes or make only minor changes labels Dec 13, 2022
@LukeTowers LukeTowers merged commit cfb02c4 into wip/child-theme Dec 13, 2022
@LukeTowers LukeTowers deleted the wip/child-theme-autodatasource-caching branch December 13, 2022 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants