Skip to content

Expose Parse Server config in Cloud Code #7869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 tasks done
mtrezza opened this issue Mar 20, 2022 · 18 comments · Fixed by #8244
Closed
3 tasks done

Expose Parse Server config in Cloud Code #7869

mtrezza opened this issue Mar 20, 2022 · 18 comments · Fixed by #8244
Labels
state:released Released as stable version state:released-5.x.x Released as LTS version state:released-alpha Released as alpha version state:released-beta Released as beta version type:feature New feature or improvement of existing feature

Comments

@mtrezza
Copy link
Member

mtrezza commented Mar 20, 2022

New Feature / Enhancement Checklist

Current Limitation

Customization is a key characteristic of Parse Server. There have been scenarios in the past in which developers need to access adapters / controllers of Parse Server, which is done via the server config. The suggested way to access the config is:

const Config = require('../node_modules/parse-server/lib/Config');

That is not a stable reference and may break if server files are refactored.

See:

Feature / Enhancement Description

Expose the server config via an official method, for example:

const Parse = require('parse/node');
const Config = Parse.config;

// or considering multitenency:
const Config = Parse.getConfig(Parse.applicationId);

Example Use Case

const config = Parse.config;
const url = config.filesController.adapter.getFileLocation(config, filename);

Alternatives / Workarounds

n/a

3rd Party References

n/a

@parse-github-assistant
Copy link

parse-github-assistant bot commented Mar 20, 2022

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@mtrezza mtrezza added the type:feature New feature or improvement of existing feature label Mar 20, 2022
@Moumouls
Copy link
Member

@mtrezza i can suggest another implementation. What about just adding a config key under the request object used in all triggers? A lot easier to implement and follow the internal architecture of the parse server. Currently, the config is transferred through function args, not by a method from the Parse singleton :)

@Moumouls
Copy link
Member

magic will happen here:

export function getRequestObject(

@mtrezza
Copy link
Member Author

mtrezza commented Jun 24, 2022

That means config would only be accessible in a request. If config is static per Parse instance and doesn't change per request, why would you tie it to a request? And how would you access config outside of a request?

@Moumouls
Copy link
Member

Moumouls commented Jun 24, 2022

If config is static per Parse instance and doesn't change per request

For DX purposes. Parse Server internally heavily use dependency injection by passing config during request execution. This is smart, easier for testing purposes and there is no reason to not expose the config through the request object. Also in a multi-tenancy config config/request makes sense.

NOTE: for example on the graphql API the full Parse Server context, with config and auth is available in the GraphQL resolver context arg

And how would you access config outside of a request?

Not supported in my PR, it could be an additional implementation. But not currently motivated to look into it. I've not seen this use case on my side since I use Parse Server.

So it sounds that const Config = Parse.getConfig(Parse.applicationId); is still up for grabs.

@mtrezza
Copy link
Member Author

mtrezza commented Jun 25, 2022

What's the point of adding code to every request route and add tests for each individual route, if exposing the config via Parse is a solution that requires less code, less tests and covers even more use cases? This seems to be blowing up the code unnecessarily?

Not supported in my PR, it could be an additional implementation.

Why would that be an additional implementation, wouldn't that make your PR obsolete?

@Moumouls
Copy link
Member

@mtrezza I suggested a quick implementation without any tradeoff. This implementation follow dependency injection strategy well know to facilitate tests (like unit test on cloud function etc...) and also flexibility.

Feel free to send a PR for the Parse.Config strategy.

My PR will not be outdated or useless. Some properties of the config are already used in each request. There is no reason to not extend the request and allow developers to access to the full config through the request interface.

@mtrezza
Copy link
Member Author

mtrezza commented Jun 26, 2022

My PR will not be outdated or useless.

How so? If config is accessible via Parse, why would it be useful to provide the config additionally in a request?

@Moumouls
Copy link
Member

why would it be useful to provide the config additionally in a request?

Because dependency injection is another way to consume data. I agree that Parse.Config is also useful but it's also another way to consume the config through a singleton. These 2 solutions can easily coexist and make sense.

Dependency injection strategy is easier also to use in a multi-tenant app. User will not need to care about which config to get in his cloud function/hooks.

@mtrezza
Copy link
Member Author

mtrezza commented Jun 26, 2022

These 2 solutions can easily coexist and make sense.

Why does it make sense for them to co-exist?

in a multi-tenant app

Does Parse Server currently support multi-tenancy?

@Moumouls
Copy link
Member

Why does it make sense for them to co-exist?

Sure, one use dependency injection strategy with it's own benefits and the other use the singleton/import strategy with also it's own benefits

Does Parse Server currently support multi-tenancy?

It seems that some users started to succeed to use Parse in multi tenancy config, so it make sense to still implement code "multi tenancy friendly" (ie: #7951)

@mtrezza
Copy link
Member Author

mtrezza commented Jun 27, 2022

It seems that some users started to succeed to use Parse in multi tenancy

We don't consider unofficial / hacked-in features. For Parse Server to support multi-tenancy more internal restructuring is needed. Just like there is Parse.applicationId, there can be Parse.config. If multi-tenancy ever becomes an official feature, both (and more) need to be reconsidered. Multi-tenancy is an issue that is discussed since Parse Server was open sourced and never made it so far. There have also been contrary efforts to remove the app ID in order to simplify the codebase. Both is on the table, neither can reasonably influence our current decision making.

So far I don't see any benefits for config as part of a request; but I do see downsides, as it requires more code, more maintenance and more tests. We want to reduce code. In every request, the Parse object is accessible and can provide a single path and logic to obtain the config.

@dblythy
Copy link
Member

dblythy commented Oct 17, 2022

I think there is a naming clash between this and Parse.Config, which could lead to some confusion:

https://docs.parseplatform.org/js/guide/#config

Perhaps we could have Parse.Server (available in cloud code only) which is an alias for Config.get(appId)

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.3.0-alpha.32

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Oct 29, 2022
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.4.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Oct 29, 2022
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.4.0-alpha.1

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.4.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Nov 19, 2022
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.4.0

@parseplatformorg parseplatformorg added the state:released-5.x.x Released as LTS version label Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-5.x.x Released as LTS version state:released-alpha Released as alpha version state:released-beta Released as beta version type:feature New feature or improvement of existing feature
Projects
None yet
4 participants