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

Error: "Cannot nest $ under $in" when querying Mongo IDs #231

Closed
zachsa opened this issue Aug 5, 2021 · 5 comments
Closed

Error: "Cannot nest $ under $in" when querying Mongo IDs #231

zachsa opened this issue Aug 5, 2021 · 5 comments
Labels

Comments

@zachsa
Copy link

zachsa commented Aug 5, 2021

Hi,

I use this library in conjunction with the DataLoader library (like many others I'm sure). I have a batching function that takes a number of filters, runs a single query on mongo, and then uses sift to 'unbatch' the results

filters = [
  {},
  {$in: [...]},
  etc
]

// Batch for Mongo
query = {$or: filters}
docs = mongo.find(query)

// 'Unbatch' using sift
filter1Results = docs.filter(sift(filter1))
filter2Results = docs.filter(sift(filter2))
etc

In other words I'm using the same filter to find Mongo documents as for filtering a JavaScript array using sift. This worked with versions prior 14 despite that sift needed to match objects to objects (auto-generated _ids in Mongo).

Since updating to v14 my batching functions fail.

v13.5.4
This works

docs.filter(sift({ _id: { '$in': [ new ObjectId("610b6bc9e29dbd1bb5f045bf") ] } }))

In v14.02, for the same expression sift throws an error:

cannot nest $ under $in
@crcn crcn added the bug label Aug 5, 2021
@crcn
Copy link
Owner

crcn commented Aug 6, 2021

Hmmm, your snippet works for me. Here's what I'm doing:

const test = sift({ _id: { '$in': [ new ObjectID("610b6bc9e29dbd1bb5f045bf") ] } })
assert.equal(test({ _id: new ObjectID("610b6bc9e29dbd1bb5f045bf") }), true);  

I'm guessing that the ObjectId object that you're using contains properties that are triggering this exception. What library are you using?

@zachsa
Copy link
Author

zachsa commented Aug 6, 2021

Hi,

The snippet I added above:

docs.filter(sift({ _id: { '$in': [ new ObjectId("610b6bc9e29dbd1bb5f045bf") ] } }))
  • I'm not actually calling new ObjectId(...). This is the output to the console when logging a MongoID. i.e. the _id returned from collection.find({}) (sorry, I could have made this clearer)
  • I'm using the Node.js Mongo client (https://www.npmjs.com/package/mongodb) v4.1
  • MongoDB 4.4.3

@eric-swann-q2
Copy link

eric-swann-q2 commented Sep 19, 2021

Ran into the same issue.... What I"m seeing is that this fails because it's looking at object "proto" properties in the following comparison function, thus finding a match on "toString" which exists in both.

var containsOperation = function (query, options) {
      for (var key in query) { //This isn't just cycling the $operations, it's also including _proto_ props, thus finding a match.
          if (options.operations[key])
              return true;
      }
      return false;
  };

Have you considered using the following ? I think that would eliminate the issue I'm seeing at least.

var containsOperation = function (query, options) {
      for (var key in query) {
          if (options.operations.hasOwnProperty(key)) // Test for hasOwnProperty here instead of just a regular key comparison
              return true;
      }
      return false;
  };

I can see in version 13.5.4, that this issue was mitigated by the following code, which explicitly looked for fields whose name started with a "$". That logic was removed in 14.x, which is why this issue surfaced.

var containsOperation = function (query) {
        for (var key in query) {
            if (key.charAt(0) === "$") // This prior comparison prevented this issue.
                return true;
        }
        return false;
    };

@crcn crcn closed this as completed in 11a33e0 Sep 24, 2021
@crcn crcn reopened this Sep 24, 2021
@crcn
Copy link
Owner

crcn commented Sep 24, 2021

Thanks for the suggestion! This should be up on NPM as sift@14.0.3. Let me know if that fixes the issue.

@crcn
Copy link
Owner

crcn commented Nov 18, 2021

Closing since this should be fixed now.

@crcn crcn closed this as completed Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants