fix aggregate geoNear with date query by mtrezza · Pull Request #6540 · parse-community/parse-server

mtrezza · 2020-03-27T01:36:11Z

added failing test case

this is the test case only without fix

- geoNear stages were not parsed for date fields, but mongodb nodejs adapter requires date object

codecov · 2020-03-27T01:48:14Z

Codecov Report

Merging #6540 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #6540      +/-   ##
==========================================
- Coverage   94.02%   94.01%   -0.01%     
==========================================
  Files         169      169              
  Lines       11848    11850       +2     
==========================================
+ Hits        11140    11141       +1     
- Misses        708      709       +1

Impacted Files	Coverage Δ
src/Adapters/Storage/Mongo/MongoStorageAdapter.js	`93.49% <100.00%> (+0.02%)`	⬆️
src/RestWrite.js	`93.64% <0.00%> (-0.17%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13bda61...3499dd9. Read the comment docs.

mtrezza · 2020-03-27T02:27:16Z

@dplewis Could you take a look at this?

Test before the fix failed as expected.
Test after the fix fails with:

Unhandled promise rejection: TypeError: Cannot read property 'split' of undefined

I cannot find out where that split is.
Interestingly, the fix works locally with a query from cloud code, geoNear returns the correct results.

dplewis · 2020-03-27T02:28:57Z

What version of Mongo are you running locally?

mtrezza · 2020-03-27T02:30:41Z

4.2.3

- the geoNear object contains parameter keys which could be identical to field names in the collection, which should not be parsed and changed, therefore restricting parsing only to query parameter key

mtrezza · 2020-03-27T03:23:57Z

I am using VS Code to debug, but I didn't manage to set breakpoints when testing, or even run only a single test using the Test Explorer instead of all tests of a file with npm test x.spec.js. Therefore I am having a hard time to find the issue.

mtrezza · 2020-03-28T19:43:56Z

@dplewis Any help appreciated, I am stuck here unfortunately :/

mtrezza · 2020-03-28T22:41:42Z

@dplewis I should clarify that the test fails also locally with he same error.
I tried it with mongodb 4.0.4 mmapv1 and mongodb 4.2.3 wiredTiger.

However, the code fix itself works and returns the correct results when running a query in cloud code.
So maybe the test only fails because of some mock up for the test environment.

- required to create geo index for geoNear test

- test case likey failed due to date rounding

- temporary, to find out why test fails on mongodb 3.6.9

- mongodb <4.0 does not allow nultiple geo indices on a class when using geoNear - see https://docs.mongodb.com/v3.6/reference/operator/aggregation/geoNear/#behavior

- results were not ordered properly, so test validation failed sometimes

This reverts commit da81c51.

mtrezza · 2020-03-29T04:40:06Z

The geo near query failed due to missing geo index, which is required for geoNear.

Ready to merge.

mtrezza · 2020-03-29T20:11:18Z

@acinader Can we get this into the next release?

dplewis

LGTM!

matheusdavidson · 2020-05-20T01:48:04Z

Hey Guys, i think this broke geoNear.

I had parse-server@4.2.0 installed here when i tried to use geoNear aggregation with a very basic query and got this error: MongoError: query must be an object

I tried the same query directly on mongo and it worked, the query was very basic just to test, after searching i saw that this PR changed geoNear so i tried the version before(4.1.0) and it worked with the same query, went back again to 4.2.0 to confirm and got the same error.

This is the pipeline i was trying:

const query: any[] = [
    {
        geoNear: {
            near: [-73.99279, 40.719296],
            distanceField: 'distance',
            maxDistance: 1000,
            spherical: true
        }
    }
];

@mtrezza @dplewis

matheusdavidson · 2020-05-20T01:49:25Z

I'm running mongo v4.2.3 and node v12.16.1 by the way.

matheusdavidson · 2020-05-20T03:32:21Z

I think maybe you forced the query option(which is optional) to be passed to the geoNear object so as i'm not using it, its value is not an object, maybe thats why the error says query must be an object.

mtrezza · 2020-05-20T09:33:17Z

@matheusdavidson Thanks for reporting, can you please open a new issue regarding this and reference this PR? I will look into this in the meanwhile.

mtrezza · 2020-05-20T10:20:30Z

Interesting, I added test cases without the query parameter and tested against mongodb 4.0.4 and 4.2.3. and tests passed. #6696

matheusdavidson · 2020-05-28T03:40:45Z

Strange, had another team member with that same problem with a version of our app without downgrading to 4.1.0. geoNear won't work anymore in the latest version

mtrezza · 2020-05-28T09:53:48Z

@matheusdavidson Are you or your team member able to replicate or observe this currently? It could also depend on the version of nodejs or mongodb driver.

_{Sent with GitHawk}

mtrezza · 2020-05-28T09:54:33Z

@matheusdavidson Can you please open an issue for this and post your findings there? If we discuss it here is only has limited visibility.

_{Sent with GitHawk}

matheusdavidson · 2020-05-30T05:33:46Z

i'm gonna do that. Just had the same problem now on another project using geoNear, will have to lock the version on 4.1.0 on all projects here.

mtrezza · 2020-07-27T00:45:02Z

@matheusdavidson Do you have any update on this issue? If not, I will go ahead and close the related PR.

matheusdavidson · 2020-07-27T01:24:09Z

Hey @mtrezza, i didn't had time to see why this is happening but i'm sure the PR broke geoNear as i stated, for now what i did was downgrade all the projects to 4.1.0. Had lots of late projects and couldn't spend more time on this. Will open an issue in the future.

…efined see dicussion in parse-community#6540

mtrezza · 2020-07-27T01:54:41Z

@matheusdavidson Since I was not able to reproduce this issue with the test cases in #6696, I added a potential fix for this potential issue by changing:

if (stage.$geoNear) {
   stage.$geoNear.query = this._parseAggregateArgs(schema, stage.$geoNear.query);
}

to

if (stage.$geoNear && stage.$geoNear.query) {
   stage.$geoNear.query = this._parseAggregateArgs(schema, stage.$geoNear.query);
}

So stage.$geoNear.query is not set to undefined anymore.

It would be great if you could upgrade a project to Parse Server 4.2.0 to confirm that it fails and then deploy the Parse Server master branch with PR #6696 merged. Then we'd know for sure whether the issue is solved.

matheusdavidson · 2020-07-27T15:48:29Z

@mtrezza thank you for adding the fix, i will upgrade one of the projects and test today.

mtrezza · 2020-07-27T16:18:33Z

@matheusdavidson That would be great. Note that the PR is not merged into master yet, but you can use the PR branch.

matheusdavidson · 2020-09-08T20:10:10Z

Hey @mtrezza, sorry it took so long for me to test. That solves my problem, thank you for your efforts!!

mtrezza · 2020-09-08T20:13:19Z

@matheusdavidson Thanks for your feedback, glad it worked.

* add test cases for geoNear aggregation Test cases do not have the `query` parameter set in $geoNear aggregation stage. this is to test for a reported potential issue when the parameter is not set. * fixed potential issue when setting the geoNear.query parameter to undefined see dicussion in #6540 * fixed duplicate index name in test

added failing test case

60382a6

mtrezza mentioned this pull request Mar 27, 2020

aggregate geoNear with filter on date field not working #6539

Closed

mtrezza changed the title ~~fix bug for aggregate geoNear with date query~~ fix aggregate geoNear with date query Mar 27, 2020

add date conversion for geoNear query

39b3d39

- geoNear stages were not parsed for date fields, but mongodb nodejs adapter requires date object

reverted unnecessary code auto-formatting

fac1d02

mtrezza added 2 commits March 27, 2020 03:45

limited parsing to query property of geoNear stage

7f1bf02

- the geoNear object contains parameter keys which could be identical to field names in the collection, which should not be parsed and changed, therefore restricting parsing only to query parameter key

reverted unnecessary code auto-formatting

0cc52e5

mtrezza added 7 commits March 29, 2020 04:30

added index type parameter to ensureIndex

979b25e

- required to create geo index for geoNear test

added geo index creation to test case

124826a

fixed dates in test case

affc307

- test case likey failed due to date rounding

added error output to console

da81c51

- temporary, to find out why test fails on mongodb 3.6.9

create seperate class to avoid multiple geo indices on TestObject class

2654b20

- mongodb <4.0 does not allow nultiple geo indices on a class when using geoNear - see https://docs.mongodb.com/v3.6/reference/operator/aggregation/geoNear/#behavior

fixed incorrect result validation

f3de545

- results were not ordered properly, so test validation failed sometimes

removed error output to console

3499dd9

This reverts commit da81c51.

dplewis approved these changes Mar 29, 2020

View reviewed changes

dplewis merged commit 19dea5b into parse-community:master Mar 29, 2020

mtrezza mentioned this pull request May 20, 2020

fix potential issue with setting geoNear.query to undefined #6696

Merged

mtrezza added a commit to mtrezza/parse-server that referenced this pull request Jul 27, 2020

fixed potential issue when setting the geoNear.query parameter to und…

6b56ca1

…efined see dicussion in parse-community#6540

snyk-bot mentioned this pull request Jul 27, 2022

[Snyk] Upgrade apollo-server-express from 3.0.0 to 3.9.0 pradyum07/parse-server#3

Open

Uh oh!

Conversation

mtrezza commented Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mtrezza commented Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dplewis commented Mar 27, 2020

Uh oh!

mtrezza commented Mar 27, 2020 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrezza commented Mar 27, 2020

Uh oh!

mtrezza commented Mar 28, 2020

Uh oh!

mtrezza commented Mar 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrezza commented Mar 29, 2020

Uh oh!

mtrezza commented Mar 29, 2020

Uh oh!

dplewis left a comment

Choose a reason for hiding this comment

Uh oh!

matheusdavidson commented May 20, 2020

Uh oh!

matheusdavidson commented May 20, 2020

Uh oh!

matheusdavidson commented May 20, 2020

Uh oh!

mtrezza commented May 20, 2020

Uh oh!

mtrezza commented May 20, 2020

Uh oh!

matheusdavidson commented May 28, 2020

Uh oh!

mtrezza commented May 28, 2020

Uh oh!

mtrezza commented May 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matheusdavidson commented May 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrezza commented Jul 27, 2020

Uh oh!

matheusdavidson commented Jul 27, 2020

Uh oh!

mtrezza commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matheusdavidson commented Jul 27, 2020

Uh oh!

mtrezza commented Jul 27, 2020

Uh oh!

matheusdavidson commented Sep 8, 2020

Uh oh!

mtrezza commented Sep 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mtrezza commented Mar 27, 2020 •

edited

Loading

codecov Bot commented Mar 27, 2020 •

edited

Loading

mtrezza commented Mar 27, 2020 •

edited

Loading

mtrezza commented Mar 27, 2020 via email •

edited

Loading

mtrezza commented Mar 28, 2020 •

edited

Loading

mtrezza commented May 28, 2020 •

edited

Loading

matheusdavidson commented May 30, 2020 •

edited

Loading

mtrezza commented Jul 27, 2020 •

edited

Loading