parse-community / parse-php-sdk

The PHP SDK for Parse Platform
https://parseplatform.org/
Other
812 stars 346 forks source link

`ParseQuery::equalTo` with strange behavior #476

Closed igor-mobapps closed 1 year ago

igor-mobapps commented 3 years ago

Issue Description

Hello, I currently have a use case that requires making 2 conditions in the same field but with different values.

However, when using equalTo(field, value) and then notEqualTo(field, value) it returns a warning

Message: Warning: Illegal string offset '$ne'

This happens because when trying to add the $ne condition to the $this->where in the ParseQuery::addCondition($key, $condition, $value) method, the $this->where[$key] is a string, due to the ParseQuery::equalTo($key, $value) method adding the value directly to the $this->where array

equalto01

Steps to reproduce

$query = new ParseQuery('SomeClass');
$query->equalTo('docs.situation', 'pending');
$query->notEqualTo('docs.situation', 'rejected'); -> throw an warning and query break

Note: If the equalTo query is the last one to be called, it overrides the previous condition

$query = new ParseQuery('SomeClass');
$query->notEqualTo('docs.situation', 'rejected'); // is not added to the where stack
$query->lessThan('docs.situation', 'rejected'); // is not added to the where stack
$query->greaterThan('docs.situation', 'rejected'); // is not added to the where stack
$query->equalTo('docs.situation', 'pending');

Note: the code above is just a use case, of course the query itself doesn’t make much sense

Result of the query structure above

Parse\ParseQuery Object
(
    [className:Parse\ParseQuery:private] => Taxista
    [where:Parse\ParseQuery:private] => Array
        (
            [docs.situation] => pending
        )

    [orderBy:Parse\ParseQuery:private] => Array
        (
        )

    [includes:Parse\ParseQuery:private] => Array
        (
        )

    [excludes:Parse\ParseQuery:private] => Array
        (
        )

    [selectedKeys:Parse\ParseQuery:private] => Array
        (
        )

    [skip:Parse\ParseQuery:private] => 0
    [count:Parse\ParseQuery:private] => 
    [limit:Parse\ParseQuery:private] => -1
    [readPreference:Parse\ParseQuery:private] => 
    [includeReadPreference:Parse\ParseQuery:private] => 
    [subqueryReadPreference:Parse\ParseQuery:private] => 
)

Solution

Instead of using $this->where[$key] inside the ParseQuery::equalTo() method, use the $this->addCondition($key, '$eq', $value) method.

equalto02

I don't know if there is any special reason for using $this->where directly in the ParseQuery::equalTo() method instead of using $this->addCondition($key, '$eq', $value), but with aggregation the query listed above works normally.

With aggregate

// it works
$query = new ParseQuery('SomeClass');
$query->aggregate([
    'match' => [
        'docs.situation' => [
            '$eq' => 'pending',
            '$ne' => 'rejected'
        ]
    ]
]);

Environment Details

dplewis commented 3 years ago

Can you write a failing test case?

The javascript SDK has similar behavior https://github.com/parse-community/Parse-SDK-JS/blob/master/src/ParseQuery.js#L1216 (my Apple M1 chip is preventing me from running the tests, im looking into it)

igor-mobapps commented 3 years ago

Hello @dplewis , I have no experience with open source contribution, in fact this is the first I do a PR for an open source project, anything just to report.

Below is the PR with the tests I did.

477

I did a test that queries the database and is returning all 3, but it should have returned only 2.

And the other test is about what I related about equalTo overwriting the previous queries that use the same key.

Note: I can't do the test where the equalTo() is called before the notEqualTo, because it launches a Warning: Illegal string offset '$ne' and I can't find a solution for expect in phpunit 7

Note: I made the same query on the mongodb compass and the result is expected to be only 2

testObjectQuery01

dplewis commented 3 years ago

A developer just posted a similar issue with the JS SDK. Does this exist there as well?

https://github.com/parse-community/Parse-SDK-JS/pull/1373

igor-mobapps commented 3 years ago

yes, my friend from work when trying to do the same query I did in parse-php-sdk the query didn't work correctly, I don't know exactly the error due to lack of js expertise, but I know it didn't work properly.

JS SDK https://github.com/parse-community/Parse-SDK-JS/blob/master/src/ParseQuery.js#L1225

PHP SDK https://github.com/parse-community/parse-php-sdk/blob/master/src/Parse/ParseQuery.php#L143

When looking at this piece of code, it looks a lot like the php SDK.

In PHP I know the error is because equalTo is assigning the value directly to the $where[$key] array instead of adding it to a $where[$key][$condition].

The main question is to understand why the equalTo method is not using the addCondition($key, $condition, $value) method, because when looking in other analytic SDKs, they are implementing equalTo in da likewise, without considering the use of the addCondition method.

sadakchap commented 2 years ago

The main question is to understand why the equalTo method is not using the addCondition($key, $condition, $value) method, because when looking in other analytic SDKs, they are implementing equalTo in da likewise, without considering the use of the addCondition method.

Maybe because we want equalTo method to override previous. See this testcase.