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

Optimized handling of scheduled collections for update and removal. #1882

Closed
wants to merge 3 commits into from

Conversation

watari
Copy link
Contributor

@watari watari commented Oct 23, 2018

Q A
Type feature/improvement
BC Break no
Fixed issues

Summary

Was updated handling of scheduled collections for update and delete. Added to CollectionPersister methods deleteAll and updateAll. In added methods query "per-collection" replaced on query "per-parent" where it is was possible.

I added tests only for deleteAll method of CollectionPersister. Functional tests is covers updateAll method. Should I also add separate functional tests for deleteAll method?

…letion of collections that is belong to one document.
…eduled for update - now nested collections for one parent is updated inside one query where it was possible.
@alcaeus alcaeus self-requested a review October 23, 2018 18:52
@jmikola
Copy link
Member

jmikola commented Oct 23, 2018

@watari: This looks like it incorporates all of #1880 and adds updateAll. We would like to target any new functionality to the master branch, and then manually backport it to 1.3 after the fact. This isn't a candidate for 1.2 because it's introducing new public API to CollectionPersister.

Can you please revise/rebase this PR so it targets the master branch? And then consider closing #1880 if it is indeed redundant.

@watari
Copy link
Contributor Author

watari commented Oct 23, 2018

@jmikola, there is discussion about this PR in Gitter.
In short, I implemented it in 1.2.x branch since I have project that use this version of ODM and can perform tests in it. So it is faster for me to make changes. Also my project will use my fork with branch 1.x or your 1.3 (when it will be released) before 2.x will become stable.
When @alcaeus take a look at this PR and confirm that it can be used also for 2.x I will update #1880 to reflect same changes (DocumentPersister and CollectionPersister is same in both branches).

I can revise/rebase PR for 1.3.x branch. this PR and #1880 was intended provide same updates for different branches.

@watari
Copy link
Contributor Author

watari commented Oct 24, 2018

Close this PR. Proposed changes will be moved to PR #1882 (for master).

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.

2 participants