parse-server icon indicating copy to clipboard operation
parse-server copied to clipboard

feat: add options to disable case insensitivity and force email and username to lower case

Open Moumouls opened this issue 3 years ago • 8 comments
trafficstars

New Pull Request Checklist

  • [x] I am not disclosing a vulnerability.
  • [x] I am creating this PR in reference to an issue.

Issue Description

Add options to be able to support MongoDB Serverless (no collation support)

Related issue: #7937

Approach

Add 2 abstract options disableCaseInsensitivity and forceEmailAndUsernameToLowerCase to allow a developer to disable default collations of Parse Server and forceEmailAndUsernameToLowerCase to keep clean usernames and emails in the database.

TODOs before merging

  • [x] Add tests
  • [ ] Add changes to documentation (guides, repository pages, in-code descriptions)
  • [x] Add security check
  • [x] A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

Moumouls avatar Jun 13 '22 13:06 Moumouls

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

Codecov Report

Patch coverage: 73.33% and project coverage change: -0.13 :warning:

Comparison is base (65e5879) 94.33% compared to head (15710ee) 94.20%.

:exclamation: Current head 15710ee differs from pull request most recent head 24ab89b. Consider uploading reports for the commit 24ab89b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #8042      +/-   ##
==========================================
- Coverage   94.33%   94.20%   -0.13%     
==========================================
  Files         183      182       -1     
  Lines       14515    13718     -797     
==========================================
- Hits        13692    12923     -769     
+ Misses        823      795      -28     
Impacted Files Coverage Δ
src/Options/Definitions.js 100.00% <ø> (ø)
src/Options/index.js 100.00% <ø> (ø)
src/Controllers/DatabaseController.js 94.13% <73.33%> (+0.19%) :arrow_up:

... and 132 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Jun 13 '22 13:06 codecov[bot]

Can we find something that reads easier than "disable case insensitivity" - avoiding the double negation. It could also be more descriptive - "case sensitivity for what"?

mtrezza avatar Jun 13 '22 20:06 mtrezza

Currently, case-insensitivity is only used for indexes/queries on email/username fields on User class.

I can suggest:

  • preventCollation
  • disableCaseInsensitiveIndexAndQuery
  • preventCollationOnIndexAndQuery

Collation is also the error name returned by the Mongo driver if you try to use "case insensitive" index/query on Mongo Serverless. Using the same wording could help developers to fix the error.

I do not have other ideas @mtrezza 🙂

Moumouls avatar Jun 13 '22 20:06 Moumouls

Collation is also the error name returned by the Mongo driver

Maybe lets call it enableCollation.... It may be useful to split up the collation for email / username into 2 separate params to make this more useful, also outside of a server-less usage. Email collation is also somewhat sensitive, because by specification [email protected] is not the same as [email protected], these are technically 2 different mailboxes.

mtrezza avatar Jun 13 '22 21:06 mtrezza

Yes enableCollation, transformEmailToLowerCase and transformUsernameToLowerCase makes sense.

Do you have a suggestion about "default injection" of enableCollation to true. Or i just use a === false check in the code. It means that if enableCollation does not exists in the config, it's enabled @mtrezza

Moumouls avatar Jun 14 '22 07:06 Moumouls

Let's simply use convert, it's a simple case conversion.

I don't understand the 2nd part.

mtrezza avatar Jun 14 '22 20:06 mtrezza

Real usage feedback: With this PR and options activated I currently succeed in running Parse Server in a fully Serverless env using Google Cloud Run (CPU only reserved during requests) and MongoDB Atlas Serverless and it works pretty well.

Moumouls avatar Jul 03 '22 10:07 Moumouls

I will reformat the title to use the proper commit message syntax.

okay @mtrezza @dblythy I'll refresh this one with your feedback

Moumouls avatar Nov 14 '22 23:11 Moumouls

To go ahead and merge this, could you address any open comments?

mtrezza avatar Apr 09 '23 20:04 mtrezza

Does this resovle issue #8492 ? I just tried to run this PR and still hitting the same error message.

lilonpro avatar May 19 '23 03:05 lilonpro

Real usage feedback: With the PR of @Moumouls and options activated I currently succeed in running Parse Server in a fully Serverless env using Azure Web App + Azure Cosmos DB for MongoDB and it works pretty well.

I think by now it is necessary to make Parse Server compatible with serverless environments.

mattia1208 avatar Aug 18 '23 13:08 mattia1208

@mattia1208 As you can see in the conversation this PR has open issues that need to be resolved to get merged. Maybe @Moumouls could review them to see which ones can be closed. And there is a merge conflict to resolve.

mtrezza avatar Aug 18 '23 18:08 mtrezza

Hi @mattia1208 , happy to here that adjustments that i made allow to run parse server with Azure cosmos DB.

It's because Azure Cosmos like Mongo Serverless doesn't support insensitive indexes ?

A special not @mattia1208 , Parse Server is quite slow to start form a cold start, what is your serverless setup on Azure to prevent cold start. On my side on GCP Cloud Run i've some minute cron task needed for special events, it helps to keep atleast one instance active wihtout paying for a full hot docker instance.

Moumouls avatar Aug 29 '23 16:08 Moumouls

Hi @Moumouls ,

Yes, you're correct. Azure Cosmos, like Mongo Serverless, does not support case insensitive indexes.

For my serverless setup on Azure, I use the following:

Azure Web App + Database + Redis Cache The Web App is hosted on Linux using a Premium V2 SKU, specifically a small size with a total of 210 ACU and 3.5GB of memory. I also utilize Azure Cosmos DB with the API for MongoDB.

With this setup, I haven't experienced any cold start issues.

I hope this provides clarity!

mattia1208 avatar Sep 15 '23 08:09 mattia1208

@Moumouls, @mtrezza,

I recently came across this PR while searching for a solution for a particular problem.

Long story short, AWS’ DocumentDB also suffers from the same issue that this PR is intended to solve.

To have this PR merged would be a GIGANTIC help for our team, if we could utilize AWS DocumentDB Elastic Clusters to scale our database horizontally. I tried to install @Moumouls version from this exact branch from this PR and it works absolutely beautifully.

Is there any chance that you gentlemen can make this happen in the foreseeable future? Is there anything I can help with to make this happen?

Thanks.

b10f avatar Sep 21 '23 14:09 b10f

@b10f see https://github.com/parse-community/parse-server/pull/8042#issuecomment-1684308292; if you want to help, you could either suggest the changes via PR review (use code suggestion), or use this PR as a start for a new PR and incorporate the changes.

mtrezza avatar Sep 21 '23 16:09 mtrezza

@Moumouls, any thoughts?

b10f avatar Sep 26 '23 13:09 b10f

Hi @mtrezza @b10f @mattia1208 @lilonpro , if someone want to recycle my code and finish last details feel free to continue this one.

My team is moving out of Mongo Serverless for some specific reason ( but Mongo Serverless works really well with this PR), if it helps your teams now feel free to finish last details of this PR 🚀

We are close, but i will dedicate some time to parse-server on other PRs like GraphQL.

Moumouls avatar Sep 26 '23 14:09 Moumouls

@Moumouls, since you are the original author of this PR, and considering that it only needs some merge conflicts to be sorted out, and addressing some review comments, I'm assuming it would not take too much effort.

Any chance that you could finish it? Even though you're moving out from Mongo Serverless, this PR would make it possible to use AWS DocumentDB and other providers that still have the same limitation.

b10f avatar Sep 26 '23 14:09 b10f

It only needs some merge conflicts to be sorted out and addressing some review comments.

Actually, it's not difficult to finish the PR. I'm a volunteer contributor to Parse Server, and most of the time, I try to send PRs to parse-server when I discover an improvement. However, there is no magic, unfortunately. I work and dedicate my time to parse-server on features that are really used by my team. Now we are moving away from Mongo Serverless, and we do not use DocumentDB, so we are not "motivated" to ship this ASAP 😉. I keep my open-source time for other PRs/projects.

Here, the job is almost done. Since we are in an open-source environment, you can easily fork, copy the branch, and provide suggested changes by @mtrezza, and it's done. If it's your first contribution to parse-server, it might take some additional time, but contributing to parse-server is feasible. Even better, you will be able to easily contribute in the future 🚀.

I'll be happy to help you start contributing 💯. @b10f

Moumouls avatar Sep 26 '23 14:09 Moumouls

I have also simplified the comments and removed some of the less important suggested changes (like naming), so it will hopefully be easier to get this PR finished.

mtrezza avatar Sep 26 '23 17:09 mtrezza

@mtrezza https://github.com/parse-community/parse-server/pull/8767

b10f avatar Sep 29 '23 17:09 b10f

Continued in https://github.com/parse-community/parse-server/pull/8805

mtrezza avatar Nov 14 '23 00:11 mtrezza