🎄 Defined Schemas with auto migration #7091

Moumouls · 2020-12-22T14:21:14Z

New Pull Request Checklist

I am not disclosing a vulnerability.
I am creating this PR in reference to an issue.

Issue Description

Allow to start parse server with predefined and locked schemas.
This feature seems to be wanted since the 2 community discussions on this subject have gathered 263 views 👍

Related issue: #7063

Approach

As discussed on the Parse Community forum it's a first simple implementation out of the schema controller
https://community.parseplatform.org/t/possibility-to-set-class-level-permissions-via-file/1061/23

This a simple implementation that run after initialization of parse server. The the script perform operation depending of fields found into the DB.

Important note: Pre defined will block all Schema route except the delete one to avoid dual source of truth (and a huge mess). Delete route will be conserved to avoid unwanted data loss, the developer will have the choice to delete the class through the Parse Dashboard on DB Shell.

Usage example:

const server =  ParseServer.start({
      schemas: [{ className: '_User', fields: { aField: { type: 'String'}} }, { className: 'Test' }],
      beforeSchemasMigration: async () => {
        // Some code if you want to execute something before migration ops
      },
    });

Schema follow the Parse Schema REST JSON structure:
Full example here: https://github.com/Moumouls/next-atomic-gql-server/blob/master/src/schema/schemas/User.ts

TODOs before merging

Add test cases
Add entry to changelog
Add retry system (Defined Schema at server start and simple migrations #7063 (comment))
Add changes to documentation (guides, repository pages, in-code descriptions)
Add security check

Moumouls · 2020-12-22T15:25:58Z

Ok implementation concepts are done, just missing the testing part (and docs).

codecov · 2020-12-23T16:26:49Z

Codecov Report

Merging #7091 (d610bb9) into master (bdf73a0) will decrease coverage by 9.12%.
The diff coverage is 96.57%.

@@            Coverage Diff             @@
##           master    #7091      +/-   ##
==========================================
- Coverage   93.92%   84.79%   -9.13%     
==========================================
  Files         181      182       +1     
  Lines       13209    13365     +156     
==========================================
- Hits        12406    11333    -1073     
- Misses        803     2032    +1229

Impacted Files	Coverage Δ
src/Adapters/Storage/Postgres/PostgresClient.js	`5.00% <0.00%> (-65.00%)`	⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	`2.46% <0.00%> (-92.90%)`	⬇️
src/Deprecator/Deprecator.js	`100.00% <ø> (ø)`
src/Options/index.js	`100.00% <ø> (ø)`
src/Routers/PagesRouter.js	`97.70% <ø> (ø)`
src/Security/Check.js	`100.00% <ø> (ø)`
src/Security/CheckGroups/CheckGroupDatabase.js	`94.73% <ø> (ø)`
src/Security/CheckGroups/CheckGroupServerConfig.js	`95.23% <ø> (ø)`
src/batch.js	`91.37% <ø> (ø)`
src/Security/CheckRunner.js	`94.82% <83.33%> (ø)`
... and 18 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 bdf73a0...d610bb9. Read the comment docs.

Moumouls · 2020-12-24T09:19:35Z

Feature finished, hope we can merge this one ASAP, then i will write some docs :)

azlekov · 2020-12-27T18:30:10Z

@Moumouls I played with your Gist snippet and the schema generations works pretty well, congrats for the nice job!
One note for the documentation because I faced problem in production with Redis cache - It looks like you should enable single schema cache else the generation failed for random reasons each time

PARSE_SERVER_ENABLE_SINGLE_SCHEMA_CACHE=true

azlekov · 2020-12-27T18:54:23Z

@Moumouls I played with your Gist snippet and the schema generations works pretty well, congrats for the nice job!
One note for the documentation because I faced problem in production with Redis cache - It looks like you should enable single schema cache else the generation failed for random reasons each time
PARSE_SERVER_ENABLE_SINGLE_SCHEMA_CACHE=true

It seems that enabling single schema cache does not fixes the issue :(

2020-12-27 18:51:18.271 [debug]: RedisCacheAdapter
2020-12-27T18:51:18.278948+00:00 app[web.1]: 2020-12-27 18:51:18.278 [error]: Field address exists, cannot update.
2020-12-27T18:51:18.289883+00:00 app[web.1]: 2020-12-27 18:51:18.282 [debug]: RedisCacheAdapter
2020-12-27T18:51:18.290228+00:00 app[web.1]: 2020-12-27 18:51:18.290 [debug]: RedisCacheAdapter

Any ideas?

Moumouls · 2020-12-27T18:59:51Z

Thanks @L3K0V during this implementation; i discovered that a field option change (like adding/modifying defaultValue or required) on my Gist script trigger a field delete and then a field create. So be careful with the gist script !
(Note: this behavior cannot be corrected on the gist since the current version of parser server has a limitation on field option updates (required/defaultValue)).

The parse server onboarded implementation will have a better stability and have many little improvements !

Thanks for you report about the singleSchemaCache i will check why this options may cause some random failed generation. @L3K0V do you use Mongo or Postgres ?

i hope we can get reviews from @mtrezza @dplewis @davimacedo to merge this one asap :) (as several contributors are on holiday, I think we will be able to merge this one at the beginning of January. )

Moumouls · 2020-12-27T19:01:31Z

2020-12-27 18:51:18.271 [debug]: RedisCacheAdapter
2020-12-27T18:51:18.278948+00:00 app[web.1]: 2020-12-27 18:51:18.278 [error]: Field address exists, cannot update.
2020-12-27T18:51:18.289883+00:00 app[web.1]: 2020-12-27 18:51:18.282 [debug]: RedisCacheAdapter
2020-12-27T18:51:18.290228+00:00 app[web.1]: 2020-12-27 18:51:18.290 [debug]: RedisCacheAdapter

Any ideas?

Did you try to start Parse server without redis adapter ?

Moumouls · 2020-12-27T19:04:04Z

@L3K0V could you post your issue with field exists on #7063 to keep the PR discussion clean for future reviews ?

davimacedo

Great job, @Moumouls ! I have some questions.

davimacedo · 2021-01-05T07:56:55Z

spec/schemas.spec.js

@@ -763,7 +763,7 @@ describe('schemas', () => {
    });
  });

-  it('refuses to put to existing fields, even if it would not be a change', done => {
+  it('refuses to put to existing fields with different type, even if it would not be a change', done => {


I am wondering if it can cause some side effect (probably in dashboard?)

No side effect it's just to support field options

davimacedo · 2021-01-05T08:04:05Z

src/DefinedSchemas.js

+    this.resetSchemaOps(schema);
+  }
+
+  async execute() {


Why can't we just clean up the _Session collection and populate it with the startup data?

not sure to understand, what do you mean by clean up the _Session ?

Sorry. Clean up the _SCHEMA collection.

I'm not sure it will work since clean up schema will not perform field deletion, index deletion on the database.

I'm not sure if we should delete field/indexes. Deleting a column using the dashboard currently does not perform these operations. Also it could take a long time for a huge class. Why don't we just leave the data over there?

I share the concerns regarding auto-deletion of field data and indexes. How would a field of a large collection be deleted without impacting DB performance? See #6823.

Hummm, to be honest currently i don't know the answer and also the solution. I have checked your linked issue/pr.
I suppose that you have already encountered a performance issue on your projects on a field deletion ?

Deleting a field takes 100% of the database processor and also slows down other requests?
I don't know if mongo/postgres support field delete/unset in background without eating all the CPU or if we should manage this on our side ?

The PR describes that deleting a field in Parse Dashboard (as it works today) can be resource intensive for large collections. In the PR, the deleting of a field takes advantage of an index, if there is one, which has to be manually created and removed. The reason the index is not auto-created is because also index creation can be resource intensive for large collections.

So here it seems that we are blocked :/

I think in first step, we can add documentation on the issue of running field deletion on large collection. And also raise a warning on field deletion with a link to the docs.

Currently I do not see a solution to ensure data consistency and operation performance for this case :/

I'm totally open for an implementation suggestion.

Yes, I think a prominent warning in the docs would be helpful. This topics get further complicated when looking at the difference of index building processes between MongoDB <4.4 and >=4.4.

A pragmatic solution could be to define a step-by-step process for the developer to manually create additional required indices, then deploy with changed schema, then manually delete unused indices. Although, I believe a more sophisticated and automated solution is possible with a deeper look into the MongoDB index building process.

sadortun · 2021-04-09T10:31:44Z

@Moumouls I have very limited time (and about 6 PRs to complete for Parse JS + Server), but I'll try to implement the suggestions I made above.

@++

Moumouls · 2021-04-09T12:24:27Z

No problem @sadortun i've also some time issue to work properly on my parse server PRs, it's just to know if you want to contribute in the futur with me on the schema feature (since it's a huge one with many improvement planned ahhaha)

sadortun · 2021-04-11T22:06:33Z

@Moumouls Just to avoid duplicate work ...

Do you plan in rebasing your PR on master ? If so, when ?

Any other changes/updates planned in the short term ?

If you answer no, just let me know and I'll do the rebase and implement what I mentioned previously

I'll also try to add types definitions to the config structure.

@++

Moumouls · 2021-05-14T09:43:08Z

@sadortun hello there, so currently i've some hard time to work on my parse server PRs 🙁
So here may be some help is needed if you have some type on your side.

i'll try at least to sync again this Pr with master then only some little adjustments will be needed.

# Conflicts: # CHANGELOG.md # spec/ParseFile.spec.js # src/Config.js # src/Options/Definitions.js # src/Options/docs.js # src/ParseServer.js

Moumouls · 2021-05-14T10:52:13Z

@sadortun so it's synced with master, @mtrezza it seems that we have a prettier/eslint mismatch.
I tried to npm run lint-fix but at commit time lint staged raise an error because prettier cancel eslint fix and the commit is then empty.

mtrezza · 2021-05-14T10:54:47Z

I think the current process is run prettier, then lint. The result is what the CI expects.

sadortun · 2021-05-14T11:17:55Z

@Moumouls awesome.

I'm working on a different project for about another week, after that I'll be able to work on this.

mtrezza · 2021-06-02T09:55:18Z

Is anyone willing to pick up this PR, so we can get it merged?

sadortun · 2021-06-02T17:08:08Z

@mtrezza yes, I can get this PR ready. The only issue blocking me now is the new project I'll be integrating will be used in production soon, how stable is master currently? if it's not stable yet, what's would be the most recent stable commit ?

(Timeframe is roughly ~2 weeks to get my project in production, so I could work on this this week)

mtrezza · 2021-06-02T17:56:15Z

how stable is master currently

Ouch! We are still working on the alpha / beta release channels for Parse Server, so master is currently as stable as it gets without significant user feedback and testing. Big pain point. I think for now we just have to assume that it is stable (enough).

sadortun · 2021-06-02T19:49:54Z

Ok, thanks!

I'll complete the imementation in two steps.

I'll rebase this on master and test the integration with a project currently in development (that already use this PR)
I'll implement a more intuitive config to ease the learning curve and help users to transition to defining schemas.
With this, I'll put in place/test the process with a current project in production that needs it's schemas to be explicitly defined. And try to solve any pain points.

Moumouls · 2021-06-03T07:29:37Z

Hi there @mtrezza @sadortun , this PR is synced with my fork used in production for now more than 1 year, so i can say that it's pretty stable. Here may be the last missing point is to allow the user to choose the migration policy (auto at server start up) or manually at a given time.

I'll implement a more intuitive config to ease the learning curve and help users to transition to defining schemas.

What is your idea here ? 🙂

Here we need to be careful, the current config structure (class definition) is Parse Rest/ JS SDK ISO, we need to avoid a new interface here, it could lead to hard maintenance, since each change to Parse.Schema could lead to a specific adjustment on the pre defined schema. Currently any change/feat on Parse.Schema could be used directly in the predefined schema 🙂

sadortun · 2021-06-03T16:25:32Z

@Moumouls

What is your idea here ? 🙂

I'm not touching the actual schema structure.

I was thinking of a few things, mostly related when a field is missing in the schema, but defined in the server, having some logging that will tell the user what is missing from his schemas definitions.

Basically, if you have defined your object in the Parse Dashboard, then run the migration tool, it should tell you "hey, I found this class defined in Parse, but not in your schema and it contains X,Y,Z ...".

Since you're around, what do you think about moving the "buildSchema()" into Parse Server.

@++

sadortun · 2021-06-09T05:11:12Z

src/DefinedSchemas.js

+    const CLPKeys = ['find', 'count', 'get', 'create', 'update', 'delete', 'addField'];
+    CLPKeys.forEach(key => {
+      if (!clp[key]) {
+        clp[key] = cloudCLP[key] || { '*': true };


@Moumouls @mtrezza Please correct me if i'm wrong, but it seems from this that when no CLP is defined in the schema, the default is Read/Write/Add for anonymous ?

For a security perspective, i think this should have no permissions by default, or throw an error Missing CLP for "ObjectName".

I understand the behaviour from ParseDashboard but from the dashboard when the user add a Class, it's created by default public, BUT there is a clear indication at the top that the class is public ...

With the schema migration behaviour being accessible by default to anonymous users, the developer will probably have no idea his class is public. It seems dangerous, and not a "secure by default" approach.

I think it would be resonable to either of the following :

Set everything to private and display a warning CLP is not defined for "ObjectName", the CLP has been set to PRIVATE and the objects will only be accessible using a MASTER_KEY. Please see [link to docs] for details

Throw an error until any kind of CLP is set in the schema Missing CLP for "ObjectName"

@++
Samuel

mtrezza · 2021-08-05T09:22:43Z

Changing to draft; PR needs rebase and resolving open questions in previous review feedback.

@Moumouls I noticed that you assign your own PRs to yourself. It's not clear to me why, but I removed your assignments from this and your other PRs, because we don't use this feature and it is not clear what it implies for other contributors. Every PR can be picked up by anyone who wants to contribute. Especially in case of PRs that are inactive for months I am afraid it may cause others to think that the PR or issue is somehow "reserved" and may deter them from further working on the PR.

mtrezza · 2021-09-03T00:49:24Z

⚠️ Important change for merging PRs from Parse Server 5.0 onwards!

We are planning to release the first beta version of Parse Server 5.0 in October 2021.

If a PR contains a breaking change and is not merged before the beta release of Parse Server 5.0, it cannot be merged until the end of 2022. Instead it has to follow the Deprecation Policy and phase-in breaking changes to be merged during the course of 2022.

One of the most voiced community feedbacks was the demand for predictability in breaking changes to make it easy to upgrade Parse Server. We have made a first step towards this by introducing the Deprecation Policy in February 2021 that assists to phase-in breaking changes, giving developers time to adapt. We will follow-up with the introduction of Release Automation and a branch model that will allow breaking changes only with a new major release, scheduled for the beginning of each calendar year.

We understand that some PRs are a long time in the making and we very much appreciate your contribution. We want to make it easy for PRs that contain a breaking change and were created before the introduction of the Deprecation Policy. These PRs can be merged with a breaking change without being phased-in before the beta release of Parse Server 5.0. We are making this exception because we appreciate that this is a time of transition that requires additional effort from contributors to adapt. We encourage everyone to prepare their PRs until the end of September and account for review time and possible adaptions.

If a PR contains a breaking change and should be merged before the beta release, please mention @parse-community/server-maintenance and we will coordinate with you to merge the PR.

Thanks for your contribution and support during this transition to Parse Server release automation!

mtrezza · 2021-10-13T19:12:44Z

Closing, continued in #7418

wip: base implementation of defined schema

bd1498f

Moumouls marked this pull request as draft December 22, 2020 14:21

Moumouls linked an issue Dec 22, 2020 that may be closed by this pull request

Defined Schema at server start and simple migrations #7063

Closed

Moumouls self-assigned this Dec 22, 2020

Moumouls requested review from davimacedo, mtrezza and dplewis December 22, 2020 14:21

wip: block routes when defined schemas used

06ee331

Moumouls added 3 commits December 22, 2020 18:52

wip: tests

6cf6355

wip: testing

2907315

wip: tested indexes

0aaf531

Moumouls added 4 commits December 23, 2020 17:38

wip: tested CLP

cb07388

fix flow

acba945

remove fit

86b5125

coverage

69f19dc

Moumouls marked this pull request as ready for review December 24, 2020 09:19

Moumouls changed the title ~~Defined Schemas with auto migration~~ 🎄 Defined Schemas with auto migration Dec 24, 2020

Moumouls added 2 commits December 24, 2020 10:59

fix

ae1cc7a

add changelog

22e6815

wip: add retry, missing test

410bebe

davimacedo reviewed Jan 5, 2021

View reviewed changes

tested retry

3e0145e

This comment has been minimized.

Sign in to view

spread clp to avoid read only issue

0549d02

Moumouls added 4 commits May 14, 2021 12:33

Merge branch 'upstream/master' into moumouls/defined-schema

5f6628b

# Conflicts: # CHANGELOG.md # spec/ParseFile.spec.js # src/Config.js # src/Options/Definitions.js # src/Options/docs.js # src/ParseServer.js

restored lock

c79a32d

update workflow

46269ff

update node version

d610bb9

sadortun mentioned this pull request Jun 6, 2021

feat: add user-defined schema and migrations #7418

Merged

6 tasks

sadortun reviewed Jun 9, 2021

View reviewed changes

mtrezza marked this pull request as draft August 5, 2021 09:22

mtrezza removed the request for review from dplewis August 5, 2021 09:26

mtrezza unassigned Moumouls Aug 5, 2021

mtrezza closed this Oct 13, 2021

mtrezza mentioned this pull request Jul 20, 2022

feat: new auth adapter #7079

Closed

7 tasks

🎄 Defined Schemas with auto migration #7091

🎄 Defined Schemas with auto migration #7091

Conversation

Moumouls commented Dec 22, 2020 • edited Loading

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

Moumouls commented Dec 22, 2020

codecov bot commented Dec 23, 2020 • edited Loading

Codecov Report

Moumouls commented Dec 24, 2020

azlekov commented Dec 27, 2020

azlekov commented Dec 27, 2020

Moumouls commented Dec 27, 2020

Moumouls commented Dec 27, 2020

Moumouls commented Dec 27, 2020

davimacedo left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtrezza Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtrezza Jan 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtrezza Feb 3, 2021 • edited Loading

Choose a reason for hiding this comment

sadortun commented Apr 9, 2021

Moumouls commented Apr 9, 2021

This comment has been minimized.

This comment has been minimized.

sadortun commented Apr 11, 2021

Moumouls commented May 14, 2021

Moumouls commented May 14, 2021

mtrezza commented May 14, 2021

sadortun commented May 14, 2021

mtrezza commented Jun 2, 2021

sadortun commented Jun 2, 2021 • edited Loading

mtrezza commented Jun 2, 2021 • edited Loading

sadortun commented Jun 2, 2021

Moumouls commented Jun 3, 2021

sadortun commented Jun 3, 2021

sadortun Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

mtrezza commented Aug 5, 2021 • edited Loading

mtrezza commented Sep 3, 2021

⚠️ Important change for merging PRs from Parse Server 5.0 onwards!

mtrezza commented Oct 13, 2021

Moumouls commented Dec 22, 2020 •

edited

Loading

codecov bot commented Dec 23, 2020 •

edited

Loading

mtrezza Jan 5, 2021 •

edited

Loading

mtrezza Jan 6, 2021 •

edited

Loading

mtrezza Feb 3, 2021 •

edited

Loading

sadortun commented Jun 2, 2021 •

edited

Loading

mtrezza commented Jun 2, 2021 •

edited

Loading

sadortun Jun 9, 2021 •

edited

Loading

mtrezza commented Aug 5, 2021 •

edited

Loading