feat: create `await new ParseServer(config).startApp()` to resolve cloud code and schema before express app is exposed #7914

dblythy · 2022-03-31T07:22:48Z

New Pull Request Checklist

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

Issue Description

Currently, there is no way to use cloud code if you are using ES Modules due to the fact that import needs to be async.

Related issue: #7560
Closes: #7527, #7560

Approach

Adds await new ParseServer().startApp() that create a Parse Server asynchronously so that a developer using ES imports and schemas can be sure that their cloud triggers and schemas are registered before creating the express app.

TODOs before merging

Add tests
A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

…-community#7839)

## [5.2.1-alpha.1](parse-community/parse-server@5.2.0...5.2.1-alpha.1) (2022-03-26) ### Bug Fixes * return correct response when revert is used in beforeSave ([parse-community#7839](parse-community#7839)) ([f63fb2b](parse-community@f63fb2b))

… Cloud Function validation (parse-community#7892)

## [5.2.1-alpha.2](parse-community/parse-server@5.2.1-alpha.1...5.2.1-alpha.2) (2022-03-26) ### Performance Improvements * reduce database operations when using the constant parameter in Cloud Function validation ([parse-community#7892](parse-community#7892)) ([48bd512](parse-community@48bd512))

# [5.3.0-alpha.1](parse-community/parse-server@5.2.1-alpha.2...5.3.0-alpha.1) (2022-03-27) ### Features * add MongoDB 5.1 compatibility ([parse-community#7682](parse-community#7682)) ([90155cf](parse-community@90155cf))

…ommunity#7893)

# [5.3.0-alpha.2](parse-community/parse-server@5.3.0-alpha.1...5.3.0-alpha.2) (2022-03-27) ### Bug Fixes * security upgrade parse push adapter from 4.1.0 to 4.1.2 ([parse-community#7893](parse-community#7893)) ([ef56e98](parse-community@ef56e98))

# [5.3.0-alpha.3](parse-community/parse-server@5.3.0-alpha.2...5.3.0-alpha.3) (2022-03-27) ### Features * add MongoDB 5.2 support ([parse-community#7894](parse-community#7894)) ([6b4b358](parse-community@6b4b358))

parse-github-assistant · 2022-03-31T07:22:57Z

Thanks for opening this pull request!

🎉 We are excited about your hands-on contribution!

codecov · 2022-03-31T08:38:45Z

Codecov Report

Base: 94.21% // Head: 85.04% // Decreases project coverage by -9.17% ⚠️

Coverage data is based on head (c29ba4f) compared to base (3b775a1).
Patch coverage: 95.45% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha    #7914      +/-   ##
==========================================
- Coverage   94.21%   85.04%   -9.18%     
==========================================
  Files         182      182              
  Lines       13733    13762      +29     
==========================================
- Hits        12939    11704    -1235     
- Misses        794     2058    +1264

Impacted Files	Coverage Δ
src/Options/Definitions.js	`100.00% <ø> (ø)`
src/Options/index.js	`100.00% <ø> (ø)`
src/Config.js	`88.72% <50.00%> (-0.30%)`	⬇️
src/ParseServer.js	`91.74% <97.22%> (+1.69%)`	⬆️
src/SchemaMigrations/DefinedSchemas.js	`88.77% <100.00%> (+0.12%)`	⬆️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	`2.48% <0.00%> (-93.23%)`	⬇️
src/Adapters/Storage/Postgres/PostgresClient.js	`5.00% <0.00%> (-65.00%)`	⬇️
src/Controllers/UserController.js	`95.34% <0.00%> (-2.33%)`	⬇️
src/Controllers/FilesController.js	`92.00% <0.00%> (-2.00%)`	⬇️
src/batch.js	`92.98% <0.00%> (-1.76%)`	⬇️
... and 5 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

mtrezza · 2022-03-31T16:55:06Z

Restarted failed tests

src/ParseServer.js

…community#7911)

# [5.3.0-alpha.4](parse-community/parse-server@5.3.0-alpha.3...5.3.0-alpha.4) (2022-04-04) ### Bug Fixes * custom database options are not passed to MongoDB GridFS ([parse-community#7911](parse-community#7911)) ([a72b384](parse-community@a72b384))

dblythy · 2022-04-05T16:26:29Z

I think this might close #7527

…#7921)

)

dblythy · 2022-04-28T15:24:26Z

It’s a bit of everything really. And yep, I believe the race condition is fixed by this PR. @Moumouls might have more to say though

Moumouls · 2022-04-28T17:25:11Z

Hi there @mtrezza @dblythy,

This PR also resolves the issue of the server being exposed before schemas are loaded.

Perfect, it was also a point that I planned to check to support correctly serverless env.

Here I only have one question; if you expose the app port AFTER the migration/init/serverStartComplete, what happens if a developer uses the Parse JS SDK into these functions (before/after migration, startServerComplete). From my knowledge, if I remember correctly the Parse JS SDK onboarded on parse-server use the "serverUrl" to interact with parse-server. So a developer could be blocked (also if he use the rest/graphql api in these functions).

To avoid this issue on my side for serverless env, I found that the we should introduce a new "publicPort" option like the "publicServerUrl" to fix the issue, and only expose parse-server to external traffic once it's ready.

We should not use a "directAccess" strategy, since developers could use GraphQL API or the REST API in these initialization functions.

If you confirm with a test that Parse JS SDK (or rest call, or Graphql call) query into before/after migration /startServerComplete fails, your changes will be a breaking change. And we will need to add a new "publicPort" to expose the completely initialized parse-server to the external traffic.

Tell what do you think about this @dblythy 🙂

Moumouls · 2022-04-28T17:30:19Z

src/SchemaMigrations/DefinedSchemas.js

  async createDeleteSession() {
-    const session = new Parse.Session();
-    await session.save(null, { useMasterKey: true });
-    await session.destroy({ useMasterKey: true });
+    const { response } = await rest.create(this.config, Auth.master(this.config), '_Session', {});
+    await rest.del(this.config, Auth.master(this.config), '_Session', response.objectId);
  }


Owww I see here @dblythy that you encountered the error since the SDK uses the port.

the old code should not be changed, if the old code works, any Parse SDK/GraphQL/REST call during the init will work.

src/ParseServer.js

dblythy · 2022-04-28T17:57:56Z

The old code works fine because the core tests don’t care for race conditions / deploying schema before exposure and checking cloud code, and use the current function ParseServer.start. This results in Parse mounting on its express app as soon as it’s created so any endpoint is available, before schemas, cloud code etc, which is problematic in envs which re-scale often. If you have a beforeSave or beforeFind hook that ensures data security and Parse exposes that route before the trigger is registered, then it can lead to big leaks if new instances are regularly deployed.

This new feature isn’t a breaking change as existing functionality works fine, if you want Parse Server to wait to init before your express app you can await the new method, otherwise you can leave as is. No migration is required, only recommended for scaling deployments.

Any JS SDK methods will only work when .listen is called, but if someone is going to the effort to use await new ParseServer().startApp(), I would argue they would expect it to await and only be available when the mount it to their express app.

My thoughts is that there isn’t any downside to the new function, there will be some improvements to be made for sure but I don’t see why it can’t operate as is alongside the current implementation. Anyone who would implement it would be aware that the server won’t be available until they mount it with express.

Moumouls · 2022-04-29T07:30:40Z

The old code works fine because the core tests don’t care for race conditions

Yes but the new start-up strategy will not expose parse-server port until startServerComplete for example. In my case for example I've some Rest/GraphQL calls during serverStartComplete to seed/perform DB checks before exposing the app.

Also, users that want to use the before/afterMigration function and try to request some data will be blocked.

Parse Server historically has 2 serverUrl properties, one for public traffic, and another one for internal traffic. serverUrl and publicServerUrl. I think we can introduce the same strategy with "publicPort"

You can easily call listen 2 times on express app :)

before/afterMigration, serverStartComplete function without the possibility to call Parse REST/GraphQL/SDK API seems useless to me.

Also, you can test the breaking change by trying to call a Parse.Query into the serverStartComplete. it should fail currently.

mtrezza · 2022-04-29T09:47:41Z

In my case for example I've some Rest/GraphQL calls during serverStartComplete to seed/perform DB checks before exposing the app.

Could you give an example? Does this mean you want to be able to call the Parse Server API before the server has finished its start-up procedure?

dblythy · 2022-05-03T03:46:34Z

Does this mean you want to be able to call the Parse Server API before the server has finished its start-up procedure?

I'm not quite sure how this is at all possible in the current version, considering in order to call the Parse API the public routes have to be available, and app.listen is called. If any "startup" procedures currently require the JS API, then all API routes are publicly available (as Parse Server has been mounted).

To circumvent this, I have added a Parse Server option holdPublicRoutes. If this is set to true, the Parse API will require a valid masterKey until mountPublicRoutes() is called. This allows for:

const config = {
   ...
  holdPublicRoutes: true,
}
const parseServer = await new ParseServer(config).startApp();
const app = express();
app.use('/parse', parseServer);
const server = app.listen(12701);
await longStartupMethod();
parseServer.mountPublicRoutes(); // any client API calls before this will return unauthorized: master key is required

Moumouls · 2022-05-03T09:30:27Z

@dblythy @mtrezza I apologize. I think I found the misunderstanding here 🙂

From the beginning, on my side, I was talking about the static start() ParseServer method. The static start method calls the app.listen in the same NodeJS event loop as the ParseServer constructor. This behavior exposes the app before the server is initialized and allows a developer to perform queries on an initializing server.

The static start method should be maybe refactored/deprecated in favor of a static async start, to only listen once the server is initialized. The developers can await the start or use serverStartComplete.
In the static async start use case, the listen SHOULD occur, after before if (serverStartComplete) {. Then developers will be able to perform queries in "serverStartComplete".

I use the static start method on my projects combined with serverStartComplete: ex: https://github.com/Moumouls/parse-next-mono-starter/blob/65d2a9e6d485185c1164619e17d64d4d45034d88/packages/back/src/server.ts#L75

I was also wrong to want parse-server to be able to handle readiness by itself. I think we should not implement options like mountPublicRoutes(); or publicPort. After a quick check of some process app managers/orchestrators, like PM2, Kubernetes, Knative, GCP Cloud Run, and other hosting services, readiness probes are handled very differently. It's out of the scope of parse-server to be able to emit/handle correctly these readiness probes, public traffic routing availability.

Finally, sorry for the wasted time here. I can suggest removing the mountPublicRoutes options, and to add the static version of startAsync with the call to listen() BEFORE the startServerComplete in case of a startup initiated from the static call (this is the current behavior of parse-server). Then developers will be able to implement their readiness probes, code business logic, after the await startAsync() or in the serverStartComplete

From the static start and startAsync method the events should occur in this order from my point of view:

performInitialization
hooksController.load()
await new DefinedSchemas(schema, this.config).execute();
(in case of static start: call app.listen here)
await Promise.resolve(serverStartComplete())
startCallbackSuccess()

Then we should be good to go 🚀

mtrezza · 2022-05-03T21:05:23Z

It's out of the scope of parse-server to be able to emit/handle correctly these readiness probes, public traffic routing availability.

There are two sides to readiness:

the sender (eg. load balancer) should not send until the recipient (Parse Server) is ready.
the recipient (Parse Server) should not process any requests until it has finished its start-up procedure.

For (1) we already have the the /health endpoint, and it is definitely within Parse Server's (or the external express route's) scope that the endpoint handles requests that are sent before the server is ready, it should for example not crash or cause undefined behavior.

For (2) I don't think Parse Server should handle any type of request until is has started up, unless we want to implement a distinct "start-up" API, which would likely be a more complex discussion and I don't think is currently part of this discussion.

More generally, polling Parse Server for readiness is actually a somewhat outdated technique. The start-of-the-art mechanism would be to launch Parse Server with a configuration of hooks that are called once it's ready / failed / etc, and then the load balancer starts sending requests, restarts or replaces the instance. Also in this case, readiness management is very much within the scope of Parse Server.

Moumouls · 2022-05-08T03:25:05Z

Also in this case, readiness management is very much within the scope of Parse Server.

I think Parse Server should provide sufficient tools to handle a graceful start. But for example, PM2 can send traffic once it detects a server.listen(), K8 has a flexible readiness probe system, and in the case of parse-server, sending a query to /health will work perfectly. In Knative, Google cloud run, the traffic is also sent once the target port configured on GCP is listening.

I think the core problem is how to allow developers to use SDK/REST/GQL API in before/after migration and serverStartComplete function given that these functions should be called BEFORE listen to ensure a graceful start.

Or developers need to start a custom express app, with a dedicated port to handle on their side the graceful start with their related business logic ?

dblythy · 2022-09-18T07:19:04Z

I'm not sure what's pending here or what the general feedback was. This approach covers the following objectives:

ES imports for cloud code as server won't be mounted to /parse before await .startApp() is called
Allows developers optional parameter to run custom initialization logic (such as building schema, custom database ops, or anything else) before mounting routes
Converts schema initialization to use direct methods (e.g data.create) rather than unnecessarily routing through express via the Parse JS SDK

dblythy · 2022-10-12T01:57:10Z

Closing as this will be introduced as a breaking change in V6.

mtrezza and others added 12 commits March 25, 2022 19:47

ci: fix node engine check (parse-community#7891)

9ef1968

fix: return correct response when revert is used in beforeSave (parse…

f63fb2b

…-community#7839)

perf: reduce database operations when using the constant parameter in…

48bd512

… Cloud Function validation (parse-community#7892)

feat: add MongoDB 5.1 compatibility (parse-community#7682)

90155cf

chore(release): 5.3.0-alpha.1 [skip ci]

499cead

# [5.3.0-alpha.1](parse-community/parse-server@5.2.1-alpha.2...5.3.0-alpha.1) (2022-03-27) ### Features * add MongoDB 5.1 compatibility ([parse-community#7682](parse-community#7682)) ([90155cf](parse-community@90155cf))

fix: security upgrade parse push adapter from 4.1.0 to 4.1.2 (parse-c…

ef56e98

…ommunity#7893)

feat: add MongoDB 5.2 support (parse-community#7894)

6b4b358

chore(release): 5.3.0-alpha.3 [skip ci]

75eca2d

# [5.3.0-alpha.3](parse-community/parse-server@5.3.0-alpha.2...5.3.0-alpha.3) (2022-03-27) ### Features * add MongoDB 5.2 support ([parse-community#7894](parse-community#7894)) ([6b4b358](parse-community@6b4b358))

fix: allow ES import for cloud string if package type is module

0cec09f

dblythy marked this pull request as draft March 31, 2022 07:55

revert helper

4e14aa2

dblythy marked this pull request as ready for review March 31, 2022 09:28

dblythy closed this Mar 31, 2022

dblythy reopened this Mar 31, 2022

snyk-bot and others added 3 commits April 3, 2022 16:09

refactor: upgrade mongodb from 4.3.1 to 4.4.1 (parse-community#7906)

132a3e0

ci: bump environment (parse-community#7907)

d74b6a3

refactor: add missing schema definitions (parse-community#7917)

2b0957c

mtrezza reviewed Apr 3, 2022

View reviewed changes

src/ParseServer.js Outdated Show resolved Hide resolved

dblythy and others added 2 commits April 4, 2022 02:04

fix: custom database options are not passed to MongoDB GridFS (parse-…

a72b384

…community#7911)

dblythy and others added 3 commits April 6, 2022 02:27

Update ParseServer.js

cbdf7e8

refactor: upgrade @apollo/client from 3.5.8 to 3.5.9 (parse-community…

ef3680a

…#7921)

fix: security upgrade moment from 2.29.1 to 2.29.2 (parse-community#7931

6b68593

)

dblythy changed the title ~~fix: allow ES import for cloud string if package type is module, create await new ParseServer(config).startApp()~~ feat : allow ES import for cloud string if package type is module, create await new ParseServer(config).startApp() Apr 28, 2022

dblythy changed the title ~~feat : allow ES import for cloud string if package type is module, create await new ParseServer(config).startApp()~~ feat: create await new ParseServer(config).startApp() to resolve cloud code and schema before express app is exposed Apr 28, 2022

Moumouls suggested changes Apr 28, 2022

View reviewed changes

dblythy added 2 commits April 29, 2022 12:47

Update ParseServer.js

caa75f6

Update ParseServer.js

dec4082

mtrezza force-pushed the alpha branch 2 times, most recently from 59215e6 to e6d7d8f Compare May 1, 2022 02:29

dblythy added 3 commits May 3, 2022 13:35

add holdPublicRoutes

d65be73

Merge remote-tracking branch 'upstream/alpha' into alpha

61ef23f

Merge branch 'alpha' into await-cloud-code

60145a2

definitions

7cd5e38

Moumouls mentioned this pull request May 3, 2022

Parse Server compatibility with Serverless environment #7938

Open

3 tasks

Merge branch 'alpha' into await-cloud-code

871f13c

dblythy added 2 commits May 16, 2022 10:43

Merge branch 'alpha' into await-cloud-code

d378e72

Merge branch 'alpha' into await-cloud-code

c29ba4f

dblythy requested a review from a team September 18, 2022 07:15

dblythy closed this Oct 12, 2022

dblythy mentioned this pull request Oct 16, 2022

feat: Asynchronous initialization of Parse Server #8232

Merged

5 tasks

mtrezza mentioned this pull request Nov 12, 2023

Cannot specify cloud file string using ES Module #7559

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create `await new ParseServer(config).startApp()` to resolve cloud code and schema before express app is exposed #7914

feat: create `await new ParseServer(config).startApp()` to resolve cloud code and schema before express app is exposed #7914

dblythy commented Mar 31, 2022 •

edited

Loading

parse-github-assistant bot commented Mar 31, 2022 •

edited

Loading

codecov bot commented Mar 31, 2022 •

edited

Loading

mtrezza commented Mar 31, 2022

dblythy commented Apr 5, 2022

dblythy commented Apr 28, 2022

Moumouls commented Apr 28, 2022

Moumouls Apr 28, 2022

Moumouls Apr 28, 2022

dblythy commented Apr 28, 2022 •

edited

Loading

Moumouls commented Apr 29, 2022

mtrezza commented Apr 29, 2022

dblythy commented May 3, 2022 •

edited

Loading

Moumouls commented May 3, 2022

mtrezza commented May 3, 2022 •

edited

Loading

Moumouls commented May 8, 2022

dblythy commented Sep 18, 2022 •

edited

Loading

dblythy commented Oct 12, 2022

feat: create await new ParseServer(config).startApp() to resolve cloud code and schema before express app is exposed #7914

feat: create await new ParseServer(config).startApp() to resolve cloud code and schema before express app is exposed #7914

Conversation

dblythy commented Mar 31, 2022 • edited Loading

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

parse-github-assistant bot commented Mar 31, 2022 • edited Loading

Thanks for opening this pull request!

codecov bot commented Mar 31, 2022 • edited Loading

Codecov Report

mtrezza commented Mar 31, 2022

dblythy commented Apr 5, 2022

dblythy commented Apr 28, 2022

Moumouls commented Apr 28, 2022

Moumouls Apr 28, 2022

Choose a reason for hiding this comment

Moumouls Apr 28, 2022

Choose a reason for hiding this comment

dblythy commented Apr 28, 2022 • edited Loading

Moumouls commented Apr 29, 2022

mtrezza commented Apr 29, 2022

dblythy commented May 3, 2022 • edited Loading

Moumouls commented May 3, 2022

mtrezza commented May 3, 2022 • edited Loading

Moumouls commented May 8, 2022

dblythy commented Sep 18, 2022 • edited Loading

dblythy commented Oct 12, 2022

feat: create `await new ParseServer(config).startApp()` to resolve cloud code and schema before express app is exposed #7914

feat: create `await new ParseServer(config).startApp()` to resolve cloud code and schema before express app is exposed #7914

dblythy commented Mar 31, 2022 •

edited

Loading

parse-github-assistant bot commented Mar 31, 2022 •

edited

Loading

codecov bot commented Mar 31, 2022 •

edited

Loading

dblythy commented Apr 28, 2022 •

edited

Loading

dblythy commented May 3, 2022 •

edited

Loading

mtrezza commented May 3, 2022 •

edited

Loading

dblythy commented Sep 18, 2022 •

edited

Loading