New query condition support to match all strings that starts with some other given strings #3864

eduardbosch · 2017-05-26T07:46:40Z

Hi to all! 🎉

This changes gives support to this contributions:
parse-community/Parse-SDK-Android#673
and
parse-community/Parse-SDK-JS#437

It allows to send regular expressions with $all query.

Please, read this link to know why I need this changes in the server:

parse-community/Parse-SDK-Android#673

Thanks a lot 😀

This reverts commit c9dfbcb.

flovilmart · 2017-05-28T16:45:17Z

Thanks for the PR, this looks great but could you add proper edge to edge test with it_only_db('mongo') so we don't run it on Postgres. @vitaly-t do you think this could work on PG as well?

flovilmart

Edge to edge tests would be nice, postgres support would be awesome.

vitaly-t · 2017-05-28T16:51:25Z

@flovilmart I can't really say. I haven't actually used parse-server for anything yet. Just doing support as far as pg-promise is concerned.

codecov · 2017-05-28T20:01:04Z

Codecov Report

Merging #3864 into master will increase coverage by 0.32%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3864      +/-   ##
==========================================
+ Coverage   90.14%   90.47%   +0.32%     
==========================================
  Files         114      114              
  Lines        7550     7684     +134     
==========================================
+ Hits         6806     6952     +146     
+ Misses        744      732      -12

Impacted Files	Coverage Δ
src/Adapters/Storage/Mongo/MongoTransform.js	`83.56% <100%> (+0.59%)`	⬆️
src/Controllers/SchemaController.js	`97.05% <0%> (ø)`	⬆️
src/Controllers/DatabaseController.js	`94.35% <0%> (+0.01%)`	⬆️
src/RestQuery.js	`94.94% <0%> (+0.02%)`	⬆️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js	`92.98% <0%> (+0.08%)`	⬆️
src/Routers/ClassesRouter.js	`93.05% <0%> (+0.09%)`	⬆️
src/rest.js	`97.18% <0%> (+0.16%)`	⬆️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	`95.83% <0%> (+0.37%)`	⬆️
src/Adapters/Storage/Mongo/MongoCollection.js	`95.12% <0%> (+0.38%)`	⬆️
src/RestWrite.js	`93.29% <0%> (+0.42%)`	⬆️
... and 7 more

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 1f11ad5...c3921f3. Read the comment docs.

flovilmart · 2017-05-28T20:03:21Z

spec/MongoTransform.spec.js

@@ -320,6 +320,9 @@ describe('parseObjectToMongoObjectForCreate', () => {
    done();
  });

+});
+
+describe_only_db('mongo')('parseObjectToMongoObjectForCreate', () => {


I was thinking about an edge to edge test case, with proper data (create multiple objects and run the query). Do you think that's possible?

I was writing you to ask if this was what you where asking for. I don't know how to make this tests. Is there any edge to edge test case in the repository? Sorry, I'm not really familiar with Jasmine.

If you tell me how to make this tests or where to take an idea, I'll take a look for sure.

Do you think I should rollback this change??

Do you think I should rollback this change??

You can roll it back, but that doesn't really change anything.

The edge to edge test would look like:

it('should match all strings that starts with string', (done) => { let object = new Parse.Object('Object'); object.set('strings', ['the', 'brown', 'fox', 'jumps']); let object2 = new Parse.Object('Object'); object2.set('strings', ['over', 'the', 'lazy', 'dog']); Parse.Object.saveAll([object, object2]).then(() => { return request.get({ url: Parse.serverURL + 'classes/Object', json: true, query: ... // build the JSON query }); }).then((results) => { expect(....) // TODO: validate the results }); });

thanks @flovilmart

I'll make this tests when I have some time!

This reverts commit d7194c7.

eduardbosch · 2017-06-15T08:19:44Z

I've added the tests with containsAllStartingWith function. To simplify the tests, I updated the parse dependency to the one I've implemented the containsAllStartingWith for Parse JS SDK.

Sorry for the delay, but I'm so busy this days.

Let me know the next steps ☺️

flovilmart · 2017-06-18T01:22:38Z

@eduardbosch any chance you'd get it to work with postgres? I know that regexp support is not 💯 in PG, so perhaps you should just replace it by it_exclude_dbs(['postgres'])

dplewis · 2017-06-27T02:09:02Z

@eduardbosch @flovilmart I have this working for postgres. You can find a solution on my branch.

dplewis@bc449e9

You may have to add some validation on postgres transform and a few more edge to edge tests to be sure.

codecov-io · 2017-07-07T14:43:03Z

Codecov Report

Merging #3864 into master will increase coverage by <.01%.
The diff coverage is 95.83%.

@@            Coverage Diff             @@
##           master    #3864      +/-   ##
==========================================
+ Coverage   92.66%   92.66%   +<.01%     
==========================================
  Files         119      119              
  Lines        8585     8632      +47     
==========================================
+ Hits         7955     7999      +44     
- Misses        630      633       +3

Impacted Files	Coverage Δ
src/Adapters/Storage/Postgres/sql/index.js	`87.5% <ø> (ø)`	⬆️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	`97.2% <95.65%> (-0.04%)`	⬇️
src/Adapters/Storage/Mongo/MongoTransform.js	`86.54% <96%> (+0.38%)`	⬆️
src/Routers/PushRouter.js	`92.85% <0%> (-3.58%)`	⬇️
src/RestWrite.js	`93.41% <0%> (ø)`	⬆️

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 2c357df...917ec15. Read the comment docs.

@dplewis

Thanks to @dplewis

eduardbosch · 2017-07-07T15:29:14Z

@dplewis Thanks for the changes. I've added to the PR.

@flovilmart Do you know why Travis is not checking the integration? I'm new to Travis, so I'm not really sure what's happening. I think postgres tests should work with @dplewis changes now.

In the other hand, I would like to know how to add tests to check progress for regex transform.

Thanks to all and sorry for the delay. I'll like to fix all this 😛

dplewis · 2017-07-07T17:37:00Z

@eduardbosch Can you make a test for REST similar to ParseQuery.spec
https://github.com/parse-community/parse-server/blob/master/spec/ParseQuery.spec.js#L2914

The following should throw an error. Every object of $all should check for $regex for containsAllStartingWith to work

where: {
  strings: {
    $all: [
      {$regex: ...},
      {$regex: ...},
      {$unknown: ...}
    ]
  }
}

eduardbosch · 2017-07-07T23:33:38Z

@dplewis Now parse server checks that all values in $all must be of type {$regex: '^\Qstring\E'} or none.

I've added also a REST test for containsAllStartingWith.

dplewis · 2017-07-08T02:12:36Z

package.json

@@ -29,7 +29,7 @@
    "mime": "1.3.6",
    "mongodb": "2.2.26",
    "multer": "1.3.0",
-    "parse": "1.9.2",
+    "parse": "eduardbosch/Parse-SDK-JS#contains-all-starting-with",


Since you are using REST the JavaScript branch isn't needed. Changing it back will remove the conflict also.

dplewis · 2017-07-08T02:14:36Z

spec/ParseQuery.spec.js

+        .then(function (results) {
+          equal(results.results.length, 1);
+
+          return require('request-promise').get({


Move this to a separate test for errors

dplewis · 2017-07-08T02:15:49Z

spec/ParseQuery.spec.js

+      equal(objectList.length, results.length);
+
+      new Parse.Query('Object')
+        .containsAllStartingWith('strings', ['the', 'fox', 'lazy'])


REST covers this you can remove the .containsAllStartingWith

dplewis · 2017-07-08T02:16:42Z

src/Controllers/DatabaseController.js

@@ -44,6 +44,30 @@ const transformObjectACL = ({ ACL, ...result }) => {

 const specialQuerykeys = ['$and', '$or', '_rperm', '_wperm', '_perishable_token', '_email_verify_token', '_email_verify_token_expires_at', '_account_lockout_expires_at', '_failed_login_count'];

+const isStartsWith = value => {


Can you move the checks from the controller to the adapters?

dplewis · 2017-07-08T02:19:07Z

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

@@ -307,7 +307,15 @@ const buildWhereClause = ({ schema, query, index }) => {
    }

    if (Array.isArray(fieldValue.$all) && isArrayField) {
-      patterns.push(`array_contains_all($${index}:name, $${index + 1}::jsonb)`);
+      if (fieldValue.$all[0].$regex) {


Here I just checked the first index but you can loop through and check all fields instead of checking on the controller

The same can be done for mongo

It's alredy done for Mongo too 🙂

eduardbosch · 2018-02-16T18:50:21Z

@dplewis tests added

I've added some tests to spec/MongoTransform.spec.js also.

I had to setup different tests for Postgres and Mongo as containsAll works differently in both databases.

Maybe would be better to try to unify the same functionality for both databases?

dplewis · 2018-02-16T19:05:52Z

spec/ParseQuery.spec.js

+        }
+      });
+    })
+      .then(function () {


@eduardbosch I figured they would return different values for both databases. Some of your tests are missing .then() return values. If you add the return values for PG I can look at the script I wrote to see if I can get similar functionality to mongo

@dplewis I don't understand. Some miss .then() return values because they throw an error, so that function is never called.

Sorry missed that. Looking at your test, it should be easy to make PG throw an error instead of returning zero results for invalid $all.

…tains-all-starting-with # Conflicts: # spec/ParseQuery.spec.js

…tains-all-starting-with

$regex in $all list must be { $regex: "string" }

eduardbosch · 2018-05-05T12:37:59Z

Sorry for the long delay!

I've unified all functionality from MongoDB and PostgreSQL except when the $all array is empty.

In MongoDB returns an empty result but in PostgreSQL returns all results.

Maybe contains-all-regex.sql should be changed to return empty results on empty $all.

dplewis · 2018-05-16T00:38:50Z

@eduardbosch @flovilmart Sorry for the late reply. The empty $all has another issue here.

query.containAll('field', []) returns all values for PG and have nothing to do with this PR.

Basically contains-all-regex.sql doesn't get called here because there aren't any regex present but contains-all.sql does.

This should be good to merge. I'll create another PR for the empty $all

flovilmart · 2018-05-16T00:40:31Z

As it stands because there’s no edge to edge tests, it’s passing but adding a proper test would let it fail. Do you believe $nor and $elemMatch would be portable to PG?

dplewis · 2018-05-16T00:42:18Z

@flovilmart Wrong thread 😜. I can look at it if there are test cases.

flovilmart · 2018-05-16T00:43:01Z

Oops! Sorry!!

dplewis · 2018-05-16T01:36:01Z

@eduardbosch Sorry this took almost a year. 🎉. We learned a lot about PG and found a bug. Thanks for the contribution. 🙏

dplewis · 2018-05-16T01:39:30Z

@flovilmart How does this look?

flovilmart

This is looking good if it’s looking good for you @dplewis!

flovilmart · 2018-05-16T01:43:51Z

Thanks @eduardbosch for your commitment and patience!

eduardbosch · 2018-05-16T08:50:58Z

Thanks to you @flovilmart @dplewis for your patience too!! I've learned a lot too 🎉

So happy to see this merged 😍I hope this could be a useful feature for someone.

I think we should start taking a look at the SDKs integrations now:

New query condition to match all strings that starts with some other given strings Parse-SDK-JS#437
New query condition to match all strings that starts with some other given strings Parse-SDK-Android#673

I'm currently porting my app to iOS, so I think I'll add those changes to the iOS SDK too in a future.

@dplewis

…e other given strings (parse-community#3864) * feat: Convert $regex value to RegExp object * feat: Add lib folder * Revert "feat: Add lib folder" This reverts commit c9dfbcb. * feat: Add $regex test in $all array * test: Test regex with $all only in MongoDB * Revert "test: Test regex with $all only in MongoDB" This reverts commit d7194c7. * feat: Add tests for containsAllStartingWith * feat: Add postgres support Thanks to @dplewis * feat: Check that all values in $all must be regex or none * test: Check that $all vaules must be regex or none * feat: Update tests to use only REST API * refactor: Move $all regex check to adapter * feat: Check for valid $all values in progres * refactor: Update function name * fix: Postgres $all values regex checking * fix: Check starts with as string * fix: Define contains all regex sql function * fix: Wrong value check * fix: Check valid data * fix: Check regex when there is only one value * fix: Constains all starting with string returns empty with bad params * fix: Pass correct regex value * feat: Add missing tests * feat: Add missing tests * feat: Add more tests * fix: Unify MongoDB and PostgreSQL functionality * fix: Lint checks * fix: Test broken $regex in $all list must be { $regex: "string" } * test for empty $all

Eduard Bosch Bertran added 4 commits May 26, 2017 09:27

feat: Convert $regex value to RegExp object

0379019

feat: Add lib folder

810b7ec

Revert "feat: Add lib folder"

77580a0

This reverts commit c9dfbcb.

feat: Add $regex test in $all array

e209087

eduardbosch mentioned this pull request May 26, 2017

New query condition to match all strings that starts with some other given strings parse-community/Parse-SDK-Android#673

Closed

flovilmart self-requested a review May 28, 2017 16:50

flovilmart suggested changes May 28, 2017

View reviewed changes

test: Test regex with $all only in MongoDB

d7194c7

flovilmart suggested changes May 28, 2017

View reviewed changes

Eduard Bosch Bertran added 2 commits May 28, 2017 22:53

Revert "test: Test regex with $all only in MongoDB"

59a57f7

This reverts commit d7194c7.

feat: Add tests for containsAllStartingWith

132b0d6

flovilmart mentioned this pull request Jun 18, 2017

New query condition to match all strings that starts with some other given strings parse-community/Parse-SDK-JS#437

Merged

flovilmart added this to the 2.6.0 milestone Jun 25, 2017

eduardbosch force-pushed the feat/contains-all-starting-with branch 2 times, most recently from 7e36fb1 to 132b0d6 Compare July 7, 2017 15:23

feat: Add postgres support

ee84d53

Thanks to @dplewis

Eduard Bosch Bertran added 2 commits July 8, 2017 01:30

feat: Check that all values in $all must be regex or none

52a394e

test: Check that $all vaules must be regex or none

3432fc8

dplewis suggested changes Jul 8, 2017

View reviewed changes

feat: Update tests to use only REST API

81ecf4d

Eduard Bosch Bertran and others added 3 commits February 16, 2018 18:27

feat: Add missing tests

80772cf

feat: Add missing tests

4a75023

feat: Add more tests

efc43ed

dplewis reviewed Feb 16, 2018

View reviewed changes

eduardbosch added 5 commits May 5, 2018 13:36

fix: Unify MongoDB and PostgreSQL functionality

cc0ed3f

Merge commit '3acb3e7a9b19a7b4e43eae1fd8ad84d0a1b3c53e' into feat/con…

6381fdd

…tains-all-starting-with # Conflicts: # spec/ParseQuery.spec.js

fix: Lint checks

79f1f32

Merge commit 'bb1641419fb3708ac975ad7959dacb7318c65717' into feat/con…

c14ffd0

…tains-all-starting-with

fix: Test broken

da76c18

$regex in $all list must be { $regex: "string" }

dplewis added 2 commits May 15, 2018 20:21

test for empty $all

07a4d69

Merge branch 'master' into feat/contains-all-starting-with

917ec15

dplewis approved these changes May 16, 2018

View reviewed changes

flovilmart approved these changes May 16, 2018

View reviewed changes

dplewis merged commit c0e3672 into parse-community:master May 16, 2018

dplewis mentioned this pull request May 16, 2018

Adding Mongodb element to add arrayMatches the #4762 #4766

Merged

dplewis mentioned this pull request May 24, 2018

Roadmap parse-community/parse-php-sdk#401

Closed

8 tasks

This was referenced Aug 24, 2018

Roadmap parse-community/Parse-SDK-iOS-OSX#1332

Closed

[Feature Request] containsStartingWith Query parse-community/Parse-SDK-iOS-OSX#1336

Closed

dplewis mentioned this pull request Oct 4, 2018

ContainsAllStartingWith Query parse-community/parse-php-sdk#420

Merged

		@@ -44,6 +44,30 @@ const transformObjectACL = ({ ACL, ...result }) => {

		const specialQuerykeys = ['$and', '$or', '_rperm', '_wperm', '_perishable_token', '_email_verify_token', '_email_verify_token_expires_at', '_account_lockout_expires_at', '_failed_login_count'];

		const isStartsWith = value => {

New query condition support to match all strings that starts with some other given strings #3864

New query condition support to match all strings that starts with some other given strings #3864

Conversation

eduardbosch commented May 26, 2017

flovilmart commented May 28, 2017

flovilmart left a comment

Choose a reason for hiding this comment

vitaly-t commented May 28, 2017

codecov bot commented May 28, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardbosch commented Jun 15, 2017

flovilmart commented Jun 18, 2017

dplewis commented Jun 27, 2017 • edited Loading

codecov-io commented Jul 7, 2017 • edited by codecov bot Loading

Codecov Report

eduardbosch commented Jul 7, 2017

dplewis commented Jul 7, 2017 • edited Loading

eduardbosch commented Jul 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardbosch commented Feb 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardbosch commented May 5, 2018

dplewis commented May 16, 2018

flovilmart commented May 16, 2018

dplewis commented May 16, 2018

flovilmart commented May 16, 2018

dplewis commented May 16, 2018 • edited Loading

dplewis commented May 16, 2018

flovilmart left a comment

Choose a reason for hiding this comment

flovilmart commented May 16, 2018

eduardbosch commented May 16, 2018

codecov bot commented May 28, 2017 •

edited

Loading

dplewis commented Jun 27, 2017 •

edited

Loading

codecov-io commented Jul 7, 2017 •

edited by codecov bot

Loading

dplewis commented Jul 7, 2017 •

edited

Loading

eduardbosch commented Jul 7, 2017 •

edited

Loading

dplewis commented May 16, 2018 •

edited

Loading