findManyByIds is broken in last version

[vasily-polonsky]

Updated from 0.2.8 to 0.4.0 (also to 0.3.2) and mentioned that method findManyByIds is broken in new versions.
it returns different values compared to the native request

[lorensr]

Oh no, thanks for reporting! I'm not sure where exactly the bug is. There's nothing in the changelog between 0.2.8 and 0.3.2 that stands out as a culprit.

Is there a pattern to the different results? In your screenshots, it looks like the results might be the same except for the third result being undefined? Is there anything peculiar about that doc?

I'd like to update this test case to cover the bug:

https://github.com/GraphQLGuide/apollo-datasource-mongodb/blob/master/src/__tests__/cache.test.js#L126

And then fix it and publish a 0.4.1.

[jonasmeier1212]

I have found the reason for this error:
In https://github.com/GraphQLGuide/apollo-datasource-mongodb/blob/master/src/cache.js#L8 the following check is used:

if (ObjectId.isValid(str)) {
    return new ObjectId(str)
}

But the problem is, that isValid returns true for all strings of length=12 even if they aren't ObjectIDs.
https://www.geeksforgeeks.org/how-to-check-if-a-string-is-valid-mongodb-objectid-in-nodejs/

In Vasilys case the Ids are strings, but sometimes are converted to ObjectIDs and therefore can not be found, although they exist.

I will open a PR for fixing this issue later.

[lorensr]

Great, thanks!

…

On Thu, May 27, 2021 at 5:46 AM Jonas Meier ***@***.***> wrote: I have found the reason for this error: In https://github.com/GraphQLGuide/apollo-datasource-mongodb/blob/master/src/cache.js#L8 the following check is used: if (ObjectId.isValid(str)) { return new ObjectId(str)} But the problem is, that isValid returns true for all strings of length=12. https://www.geeksforgeeks.org/how-to-check-if-a-string-is-valid-mongodb-objectid-in-nodejs/ In Vasilys case the Ids are strings, but sometimes are converted to ObjectIDs and therefore can not be found, although they exist. I will open a PR for fixing this issue later. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#60 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB5LGFSYAVLMLX2PD3YOXTTPYIG7ANCNFSM45SJZ2OQ> .