-
Notifications
You must be signed in to change notification settings - Fork 16
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
Fix _id field returned for findOneAndReplace and findOneAndUpdate with upsert option #421
Conversation
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.
Don't have much insight into this part of code in order to review.. Maybe PR title should be improved before merging, so we get meaningful messages in the git history..
shredder().shred(documentUpdaterResponse.document(), readDocument.txnId()); | ||
|
||
// Have to do this because shredder adds _id field to the document if it doesn't exist | ||
JsonNode updatedDocument = writableShreddedDocument.docJsonNode(); |
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.
Maybe we should improve the shredded document impl and avoid this side effect call?
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.
That sounds like a good improvement idea; something that handles insertion and reordering of _id
before Shredder gets to deal with it.
I'll create a follow-up issue.
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.
LGTM.
What this PR does:
Fix for Issue #416 and #417, replacement PR for #419
Which issue(s) this PR fixes:
Fixes #416 and #417
Checklist