feat: Allow updating Parse Server options dynamically

[dblythy]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

Currently, Parse Server config can be updated via Parse.Server = newConfig

Closes: #8448

Approach

Allow new Parse.Server hooks with the prefix of set, which allow for dynamic change of config. Such as:

Parse.Server.setSilent(false);
Parse.Server.set('silent', false);

Tasks

• Add tests

[parse-github-assistant]

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

[parse-github-assistant]

Thanks for opening this pull request!

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (65e5879) 94.33% compared to head (85c44cd) 94.33%.

❗ Current head 85c44cd differs from pull request most recent head 1ddb981. Consider uploading reports for the commit 1ddb981 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha    #8449   +/-   ##
=======================================
  Coverage   94.33%   94.33%           
=======================================
  Files         183      183           
  Lines       14515    14519    +4     
=======================================
+ Hits        13692    13697    +5     
+ Misses        823      822    -1     

Impacted Files	Coverage Δ
src/Config.js	91.15% <100.00%> (+0.13%)	⬆️
src/ParseServer.js	92.60% <100.00%> (-0.04%)	⬇️

... and 1 file 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.

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

[mtrezza]

I'm not sure about dynamically creating getters and setters Parse.Server.setSilent(false);, it seems difficult to review and adds a level of complexity without adding any significant benefit over Parse.Server.set('silent', false);. A reviewer needs to keep in mind that these properties are dynamically created when new options are introduced. How would a setter look like for an option setFlag? Would that become setSetFlag?

[dblythy]

A reviewer needs to keep in mind that these properties are dynamically created when new options are introduced

The proxy handler only allows existing methods defined in Parse Server options to be called using set(option), which is a more restrictive than current reassignments of Parse.Server. This allows for a more intuitive way to assign Parse Server options, with similarities to how Parse.Cloud operates.

If there is a future Parse server option that has the prefix 'set', this feature would have to be modified to support it. But, at the moment, we don't have such options so it seems unnecessary to code it in now.

The PR works by:

• Returning an ES6 Proxy for Parse.Server
• If the key being 'get' has a prefix of 'set' (such as Parse.Server.setSilent) return a function, which when invoked, update the Parse Server config with key silent to the value returned by the function
• Else, return the actual Config value for the key, such as Parse.Server.silent (returns a bool)

[mtrezza]

This may be intuitive for a Java developer by the concept of it, but we are creating new method names that are nowhere documented and we expect developers to infer them from option names by guessing (as soon as there is a "setFlag" option one can only speculate what the setter / getter naming would be).

What is the benefit of adding these setters / getters vs. the simple Parse.Server.set();?
Let's make a simple scorecard and decide.

Cons:

• Creates future debt because a setFlag option requires recoding or introduction of new option naming rules
• Almost certainly will introduce limitations on option names to avoid confusion getSet, setGet, setSet, getGet
• Adds review complexity, these prefixes need to be considered when reviewing option names in PRs
• Creates method names that are nowhere documented (while Parse.Server.set(...) can easily be documented)
• Adds complexity and code to maintain to the codebase for a feature that already exists with Parse.Server.set(...)

Pros:

• This allows for a more intuitive way to assign Parse Server options, with similarities to how Parse.Cloud operates

What do we have in Parse.Cloud that is similar?
Are there any other pros?

[dblythy]

I think it's mostly a preference of styling (and I think Javascript Proxies are cool), but I understand your reservations as the syntax Parse.Server.set is easier to maintain

[mtrezza]

Yes, I understand what you mean, and JS proxies are cool 😀 If we find more pros in the future let's add them. For now I think the cons outnumber.

[mtrezza]

This PR allows to update server options, but whether a new option value will be used by the server depends on how the option is implemented. For example, if an option is read from the config at start-up and then cached somewhere in a controller or adapter, only updating the option won't reinitialize the controller.

That would mean that each option needs to be considered separately and decided whether to allow it to be updated or not. Some options may need code changes, other may never be changeable without restart.

Could that be an issue?

[dblythy]

Yes, you are correct in your assumption. We would have to asses / rework at a per Parse Server option basis. For example, changing the databaseURI would probably have no impact on a running Parse Server

[mtrezza]

We could add a parameter to the option parser / composer to flag whether an option can be changed on-the-fly or requires restart. Like the :DEFAULT: key we could add a :DYNAMIC: that defaults to false which means the option requires the server to restart, but can be set to true which means it can be changed on-the-fly.

Based on that flag Parse Server can deny Parse.Server.set() for options that are not dynamic and log an error.
We could add to the contribution guide that options should preferably be implemented dynamic.
Over time existing options can be reworked from being static to dynamic in individual PRs.