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

Avoid mutating arguments passed into various methods #39

Open
johnoliver opened this issue Aug 30, 2015 · 8 comments
Open

Avoid mutating arguments passed into various methods #39

johnoliver opened this issue Aug 30, 2015 · 8 comments

Comments

@johnoliver
Copy link
Contributor

Currently we will mutate JsonObjects that are passed in to some methods, most notably we will add _id fields to objects when calling a save. This will happen even if the save fails, I believe this could potentially lead to dangerous behaviour.

My previous logic was that if a save failed I would retry, however by this point an _id field had already been placed on the document, so the second call would have subtly different behaviour as now I was saving an object that had an _id field on it.

I would propose making a copy of all arguments passed in to avoid mutating them.

@karianna karianna added the bug label Aug 30, 2015
@karianna karianna added this to the 3.2.0 milestone Aug 30, 2015
@karianna
Copy link
Contributor

I'd agree that's breaking the prinicple of least surprise (and then some). The Id generation and save should effectively be an atomic operation (yeah, yeah, we're not in txn SQL now Dorothy)

@purplefox
Copy link
Contributor

Also bear in mind that the mongo client might be used remotely using a service proxy where any changes to the passed in JsonObject won't be visible.

@cescoffier cescoffier modified the milestones: 3.3.0, 3.2.0 Dec 11, 2015
@cescoffier
Copy link
Member

Postponed to 3.3.0

meshuga added a commit to meshuga/vertx-mongo-client that referenced this issue Dec 22, 2015
@meshuga
Copy link
Contributor

meshuga commented Dec 22, 2015

Today I discovered this side effect in my code during insert operation. I don't consider it as a bug and some people might use it as a feature, so copying the document might break compatibility. I've made a PR with updated Javadoc to inform other developers about this "feature" - #61

karianna added a commit that referenced this issue May 23, 2016
#39 Added information about document mutabi…
@karianna karianna modified the milestones: 3.3.0, 3.4.0 Jun 1, 2016
@karianna
Copy link
Contributor

We still believe it is a bug as this won't work correctly over the event bus. That said we will need to carefully note that a fix will be breaking backwards compatibility.

@karianna karianna modified the milestones: 3.4.0, Backlog Mar 15, 2017
@vietj vietj modified the milestones: Backlog, 4.0.0 Nov 29, 2018
@mszmurlo
Copy link

mszmurlo commented Jun 4, 2019

I've just spent half a Sunday on what I thought being a bug in my code because manipulated data by save() was mutated. I see it had been moved from one milestone to another... What is the status of this issue ? Thx.

@karianna
Copy link
Contributor

karianna commented Jun 5, 2019

@mszmurlo We don't have an active maintainer looking at this - so relying on community PR's at this stage.

@karianna
Copy link
Contributor

This would be a breaking change so if a fix was to occur it would need to be in the next major version

@karianna karianna removed this from the 4.0.0 milestone May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

7 participants