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

Allow findAndModify to take an options parameter: BREAKING CHANGE #911

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@dmonagle
Contributor

dmonagle commented Dec 3, 2014

The third parameter to findAndModify was previously set as the returnFieldSelector. The problem with this is that the findAndModify function in mongo takes quite a large number of optional parameters, such as the very commonly used "new" parameter.

This fix replaces the third parameter with an options argument which should take anything that can be iterated as an associative array (such as a Json or Bson object)

Example:

auto result = collection.findAndModify(
    ["_id": Bson(_id), "count": Bson(count)],
    ["$inc": Bson(["count": Bson(1)],
    ["new": true]
);

To achieve this, I have strayed away from using the CMD struct, but the struct doesn't allow for optional parameters.

Allow findAndModify to take an options parameter: BREAKING CHANGE
The third parameter to findAndModify was previously set as the returnFieldSelector. The problem with this is that the findAndModify function in mongo takes quite a large number of optional parameters, such as the very commonly used "new" parameter.

This fix replaces the third parameter with an options argument which should take anything that can be iterated as an associative array (such as a Json or Bson object)

Example:

auto result = collection.findAndModify(
    ["_id": Bson(_id), "count": Bson(count)],
    ["$inc": Bson(["count": Bson(1)],
    ["new": true]
);

To achieve this, I have strayed away from using the CMD struct, but the struct doesn't allow for optional parameters.
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 21, 2015

Member

With find, there is always the possibility to define the query like this:

{
  "$query": {"_id": ..., "count": ...},
  "$inc": {"count": 1"},
  "$new": true
}

I don't know if this works for findAndModify, though, possibly not. The problem is that we can't make a silent breaking change like the one here. Alternatively, adding an overload with an additional options parameter would be a workable solution.

It would also be nice to assemble the command without doing dynamic memory allocations (which Bson does when appending fields), but I don't have a handy solution for this (might require some extensions to vibe.data.serialization).

Member

s-ludwig commented Feb 21, 2015

With find, there is always the possibility to define the query like this:

{
  "$query": {"_id": ..., "count": ...},
  "$inc": {"count": 1"},
  "$new": true
}

I don't know if this works for findAndModify, though, possibly not. The problem is that we can't make a silent breaking change like the one here. Alternatively, adding an overload with an additional options parameter would be a workable solution.

It would also be nice to assemble the command without doing dynamic memory allocations (which Bson does when appending fields), but I don't have a handy solution for this (might require some extensions to vibe.data.serialization).

@vuaru

This comment has been minimized.

Show comment
Hide comment
@vuaru

vuaru Mar 20, 2015

I was wanting to use findAndModify with "remove": true because findAndRemove didn't exist, but it seems neither option is possible at the moment.
+1

vuaru commented Mar 20, 2015

I was wanting to use findAndModify with "remove": true because findAndRemove didn't exist, but it seems neither option is possible at the moment.
+1

@sigod

This comment has been minimized.

Show comment
Hide comment
@sigod

sigod Jun 24, 2015

Contributor

@vuaru, I've studied code a little. It seems to me we can invoke findAndModify by calling find with

{
    "findAndModify": "<name of the collection",
    ...
}

Haven't tested it, yet.

Contributor

sigod commented Jun 24, 2015

@vuaru, I've studied code a little. It seems to me we can invoke findAndModify by calling find with

{
    "findAndModify": "<name of the collection",
    ...
}

Haven't tested it, yet.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Oct 10, 2015

Member

I've simply created a new method with a different name to make this a non-breaking change.

Member

s-ludwig commented Oct 10, 2015

I've simply created a new method with a different name to make this a non-breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment