search

parse-community / parse-server

Parse Server for Node.js / Express
https://parseplatform.org
Apache License 2.0
20.92k stars 4.78k forks source link

Parse.Query.and Problem with nested "equalTo relational fields" condition #7579

Closed Kingtous closed 3 years ago

Kingtous commented 3 years ago

New Issue Checklist

Issue Description

I'm doing a data collection and publication application. It has two roles, data uploader, and reader. When readers retrieve data, the server need to remove data that is not reviewed by the administrator and only show data which had been reviewed. But for data uploader, he/she can see data uploaded by him/herself although the data has not been reviewed by the administrator. So we defined beforeFind trigger in a class named Topic(data class), it has a nested "equalTo relational fields" in Parse.Query.and (Parse.Query.or is ok). But the server sdk will treat nested "equalTo relational fields" as "equalTo pointer fields", other than relation.

Steps to reproduce

  1. define a class 'A', class 'B'
  2. add some test data
  3. set a relational field 'rel' in A, which is the relation between 'A' and 'B'
  4. set an arbitrary field(String, Boolean, etc) named 'C'
  5. define a beforeFind trigger in 'A' 
    ...
const q1 = request.query; // class A
q1.equalTo('rel',<Parse.Object for 'B'>);
const q2 = new Parse.Query('A');
q2.equalTo('C',true);
const finalReturnQuery = Parse.Query.and(q1,q2);
return finalReturnQuery;

Actual Outcome

zero results found. explain: 

{
    "results":{
    "queryPlanner": {
    "parsedQuery":{
    "$and":[
   {
    "rel" : {
    { "$eq" : "B$<id>"}
...
}}]}}}}

the 'rel' not be parsed to containedIn but be treated as a pointer.

Expected Outcome

multiple results found. expected explain: 

{
    "results":{
    "queryPlanner": {
    "parsedQuery":{
    "$and":[
   {
    "rel" : {
    "_id" : { "$in" : [<A objectId 1>,<A objectId 2>]}
...
}}]}}}}

Environment

^4.10.3

Server

Database

Client

test using Postman and restful-api for parse server too.

Logs

Topic is the target class, and 'cls' is a relational field to the class named 'Class'.

verbose: REQUEST for [GET] /parse/classes/Topic: {} 
verbose: RESPONSE from [GET] /parse/classes/Topic: {
  "response": {
    "results": {
      "queryPlanner": {
        "plannerVersion": 1,
        "namespace": "parse.Topic",
        "indexFilterSet": false,
        "parsedQuery": {
          "$and": [
            {
              "cls": {
                "$eq": "Class$5E1lG8FaQo"
              }
            },
            {
              "_rperm": {
                "$in": [
                  null,
                  "*",
                  "role:schoolAdmin",
                  "role:teacher",
                  "role:topAdmin",
                  "sLBwuv5J6B"
                ]
              }
            }
          ]
        },
        "winningPlan": {
          "stage": "LIMIT",
          "limitAmount": 100,
          "inputStage": {
            "stage": "COLLSCAN",
            "filter": {
              "$and": [
                {
                  "cls": {
                    "$eq": "Class$5E1lG8FaQo"
                  }
                },
                {
                  "_rperm": {
                    "$in": [
                      null,
                      "*",
                      "role:schoolAdmin",
                      "role:teacher",
                      "role:topAdmin",
                      "sLBwuv5J6B"
                    ]
                  }
                }
              ]
            },
            "direction": "forward"
          }
        },
        "rejectedPlans": []
      },
      "executionStats": {
        "executionSuccess": true,
        "nReturned": 0,
        "executionTimeMillis": 0,
        "totalKeysExamined": 0,
        "totalDocsExamined": 22,
        "executionStages": {
          "stage": "LIMIT",
          "nReturned": 0,
          "executionTimeMillisEstimate": 0,
          "works": 24,
          "advanced": 0,
          "needTime": 23,
          "needYield": 0,
          "saveState": 0,
          "restoreState": 0,
          "isEOF": 1,
          "limitAmount": 100,
          "inputStage": {
            "stage": "COLLSCAN",
            "filter": {
              "$and": [
                {
                  "cls": {
                    "$eq": "Class$5E1lG8FaQo"
                  }
                },
                {
                  "_rperm": {
                    "$in": [
                      null,
                      "*",
                      "role:schoolAdmin",
                      "role:teacher",
                      "role:topAdmin",
                      "sLBwuv5J6B"
                    ]
                  }
                }
              ]
            },
            "nReturned": 0,
            "executionTimeMillisEstimate": 0,
            "works": 24,
            "advanced": 0,
            "needTime": 23,
            "needYield": 0,
            "saveState": 0,
            "restoreState": 0,
            "isEOF": 1,
            "direction": "forward",
            "docsExamined": 22
          }
        },
        "allPlansExecution": []
      },
      "serverInfo": {
        "host": "kingtousdeMBP.lan",
        "port": 27017,
        "version": "4.4.6",
        "gitVersion": "72e66213c2c3eab37d9358d5e78ad7f5c1d0d0d7"
      },
      "ok": 1
    }
  }
} {"result":{"response":{"results":{"queryPlanner":{"plannerVersion":1,"namespace":"parse.Topic","indexFilterSet":false,"parsedQuery":{"$and":[{"cls":{"$eq":"Class$5E1lG8FaQo"}},{"_rperm":{"$in":[null,"*","role:schoolAdmin","role:teacher","role:topAdmin","sLBwuv5J6B"]}}]},"winningPlan":{"stage":"LIMIT","limitAmount":100,"inputStage":{"stage":"COLLSCAN","filter":{"$and":[{"cls":{"$eq":"Class$5E1lG8FaQo"}},{"_rperm":{"$in":[null,"*","role:schoolAdmin","role:teacher","role:topAdmin","sLBwuv5J6B"]}}]},"direction":"forward"}},"rejectedPlans":[]},"executionStats":{"executionSuccess":true,"nReturned":0,"executionTimeMillis":0,"totalKeysExamined":0,"totalDocsExamined":22,"executionStages":{"stage":"LIMIT","nReturned":0,"executionTimeMillisEstimate":0,"works":24,"advanced":0,"needTime":23,"needYield":0,"saveState":0,"restoreState":0,"isEOF":1,"limitAmount":100,"inputStage":{"stage":"COLLSCAN","filter":{"$and":[{"cls":{"$eq":"Class$5E1lG8FaQo"}},{"_rperm":{"$in":[null,"*","role:schoolAdmin","role:teacher","role:topAdmin","sLBwuv5J6B"]}}]},"nReturned":0,"executionTimeMillisEstimate":0,"works":24,"advanced":0,"needTime":23,"needYield":0,"saveState":0,"restoreState":0,"isEOF":1,"direction":"forward","docsExamined":22}},"allPlansExecution":[]},"serverInfo":{"host":"kingtousdeMBP.lan","port":27017,"version":"4.4.6","gitVersion":"72e66213c2c3eab37d9358d5e78ad7f5c1d0d0d7"},"ok":1}}}}
parse-github-assistant[bot] commented 3 years ago

Thanks for opening this issue!

mtrezza commented 3 years ago

Can you please change the code in "Steps to reproduce" to a specific example that one can copy/paste to reproduce the issue? Instead of using placeholders in const q1 = request.query; and q1.equalTo('rel',<Parse.Object for 'B'>); please use specific code. The code should be as simple and short as possible, just enough to reproduce the issue. It would be best if you could open a PR with a failing test that uses such a simple example and shows exactly where it fails.

Kingtous commented 3 years ago

here is the pr with a new test case

Kingtous commented 3 years ago

I don't know which branch should be chosen to be the target branch, so I opened 2 PRs. one is 'release-4.x.x', the other is 'master' branch. I just want to prove that this issue can be reproduced on both branches. If there is no need to open PR for both of them, I will close one of them. :)

mtrezza commented 3 years ago

We usually work and complete a fix for the master branch, and after merging backport it to 4.x branch. But you did well thinking about both branches! Maybe we can close the 4.x PR for now, until we have completed the master PR.

Kingtous commented 3 years ago

done.

Kingtous commented 3 years ago

I've pushed some commits to #7593, and please check it out. :)

Kingtous commented 3 years ago

I think It's necessary to backport this bugfix to release-4.x.x because it totally affects the process of $and identifier. Lacking this commit, Parse.Query.and will not be processed by parse server when querying with relational keys.

mtrezza commented 3 years ago

Thanks for pointing this out! Let's first merge this into the master branch, then try to backport it into 4.x.

Kingtous commented 3 years ago

any plans to merge this into alpha branch? #7593

mtrezza commented 3 years ago

Sure, I'll take a look at the PR, apologies for the wait.

Kingtous commented 3 years ago

Thanks for merging the pr into alpha, I'll try using the newest pre-release for my production. Should I close this issue now? These days I've noticed that the issue was labeled by bounty:$20, is it a bonus for resolving this issue? :)

mtrezza commented 3 years ago

Amazing, we can't have enough alpha testers, so please let us know in case you find any issues with the alpha release. There is indeed a bounty on this issue, the Parse Bounty Program explains how you can request the payout.

This issue can be closed as we assume the merged PR has solved it. Thanks for your contribution!

cbaker6 commented 2 years ago

@mtrezza there's a PR that broke the parse-server and I've tracked it to this PR. 5.0.0-alpha.1 works for me, 5.0.0-alpha.2 I haven't tested yet, but it only adds a GraphQL change which I'm not using. Leaving this one in which my servers don't work anymore for 5.0.0-alpha.3. Below is is my error logs:

parse-hipaa-parse-1      | info: afterSave triggered for _User for user undefined:
parse-hipaa-parse-1      |   Input: {"authData":{"anonymous":{"id":"9601869f-2eef-4fff-8519-690a422310fa"}},"ACL":{"*":{"read":true,"write":false},"aAnt8JX15Q9vjix6kGdZgjRQWm5QGrPY":{"read":true,"write":true}},"createdAt":"2021-11-22T22:21:46.478Z","username":"YBBMbganzC1KGlFhhKXWFRjNn","sessionToken":"r:97d78ad4fac78246134dc96ed8c08cac","updatedAt":"2021-11-22T22:21:46.478Z","objectId":"aAnt8JX15Q9vjix6kGdZgjRQWm5QGrPY"}
parse-hipaa-parse-1      |   Result: undefined {"className":"_User","triggerType":"afterSave"}
parse-hipaa-parse-1      | /parse-server/lib/ParseServer.js:258
parse-hipaa-parse-1      |           throw err;
parse-hipaa-parse-1      |           ^
parse-hipaa-parse-1      | 
parse-hipaa-parse-1      | TypeError: Cannot read properties of undefined (reading 'type')
parse-hipaa-parse-1      |     at /parse-server/lib/Adapters/Storage/Postgres/PostgresStorageAdapter.js:1344:40
parse-hipaa-parse-1      |     at Array.forEach (<anonymous>)
parse-hipaa-parse-1      |     at PostgresStorageAdapter.createObject (/parse-server/lib/Adapters/Storage/Postgres/PostgresStorageAdapter.js:1303:25)
parse-hipaa-parse-1      |     at /parse-server/lib/Controllers/DatabaseController.js:782:29
parse-hipaa-parse-1      |     at processTicksAndRejections (node:internal/process/task_queues:96:5)
parse-hipaa-parse-1 exited with code 7

Note, this completely crashes the server. I'm using the Swift SDK, but Query.and is implemented like the rest of the Client SDKs, so this a major breaking change. In addition, skimming the additions in this PR, I highly doubt the server wasn't handling $and before this PR. If I remove the code that uses triggers, none of my client side queries work correctly because of this PR.

I'll open an issue soon.

mtrezza commented 2 years ago

I highly doubt the server wasn't handling $and before this PR

The PR https://github.com/parse-community/parse-server/pull/7593 demoed the failing test, so the issue was apparently there before the PR. See this test: https://github.com/parse-community/parse-server/pull/7593/commits/c262c77540112f43715491f04227193cd25efe25