Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
📝 WalkthroughWalkthroughRefactors cloud code from a global Parse mutation to per-ParseServer instances: adds Changes
Sequence Diagram(s)mermaid Dev->>PS: start(ParseServer with cloud option) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 6-9: The "./cloud" export in package.json is missing a types
mapping so TypeScript can't find declarations; update the package.json exports
entry for "./cloud" (the "./cloud": "./lib/cloud.js" mapping) to include a types
field pointing to the declaration file (for example "types":
"./types/cloud.d.ts"), or alternatively add a typesVersions mapping that maps
the "./cloud" subpath to the declaration file; ensure the declaration file path
matches your built .d.ts location and keep the existing "./cloud" value intact
while adding the types mapping.
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 120-159: The define and defineTrigger wrappers drop
validator.rateLimit because they forward the raw validator to triggers without
calling validateValidator() or applyRateLimit(); update define (around the
FunctionHandler wrapping and triggers.addFunction) and defineTrigger (around
wrappedHandler and triggers.addTrigger/addConnectTrigger) to first call
validateValidator(validator) then call applyRateLimit(validated,
nameOrClassIdentifier, this._appId) (or the equivalent helper used by
cloudDefine/cloudTrigger) and pass the validated/resulting validator (or ensure
applyRateLimit is applied) into
triggers.addFunction/addTrigger/addConnectTrigger so rate limits are enforced
for SDK-registered handlers.
- Around line 264-272: The setter for Parse.Server currently overwrites the
cached config with only newVal and appId, causing loss of required fields;
update the setter to first load the existing config via
Config.get(Parse.applicationId), merge that existing config with newVal
(ensuring appId is preserved/assigned), and then call Config.put(mergedConfig)
so partial assignments (e.g., Parse.Server = { enableSingleSchemaCache: true })
only modify specified keys and retain controllers and other required fields.
- Around line 319-339: The cloudAuthTrigger function parses an optional
Validator into the local validator variable but never registers it with
triggers.addTrigger, so validators for auth triggers (e.g., beforeLogin /
beforePasswordResetRequest) never run; update the call to triggers.addTrigger in
cloudAuthTrigger to pass the parsed validator as the validation handler (use the
validator variable) alongside type, className, and this._appId, and keep
applyRateLimit using that same validator—ensure you use the same
handler/validator ordering expected by triggers.addTrigger so existing overload
logic (string/object handler resolution) still works.
- Around line 22-33: getRoute currently maps system classes via ROUTE_MAP but
still appends the class name, causing e.g. `_User` -> `/users/_User...`; change
the logic in getRoute (and keep ROUTE_MAP) so that when parseClass is in
ROUTE_MAP you return the base segment `/${route}` (except the special '@File'
case which should remain `/${route}{/*id}` and keep '@Config' as `/${route}`),
and only for unmapped classes return `/${route}/${parseClass}{/*id}`; update
getRoute to check ROUTE_MAP[parseClass] first and return the appropriate base
route to ensure rate-limit paths match real REST routes.
In `@src/cloud-code/types.ts`:
- Around line 126-128: ObjectTriggerHandler currently allows Partial<T> | void
which incorrectly permits non-beforeSave handlers (afterSave, beforeDelete,
afterDelete) to return patches; create a distinct handler type for beforeSave
(e.g., BeforeSaveObjectTriggerHandler<T> = (request: ObjectTriggerRequest<T>) =>
BeforeSaveResult<T>) and change ObjectTriggerHandler<T> to be the void-only type
for afterSave/beforeDelete/afterDelete (i.e., (request: ObjectTriggerRequest<T>)
=> AfterTriggerResult). Update TriggerHandlerMap to use
BeforeSaveObjectTriggerHandler for the beforeSave key and ObjectTriggerHandler
for the other keys, referencing the existing types ObjectTriggerRequest,
BeforeSaveResult, AfterTriggerResult, ObjectTriggerHandler, and
TriggerHandlerMap in your changes.
In `@src/ParseServer.ts`:
- Around line 129-133: ParseServer currently constructs a concrete LegacyCloud
and uses its .Parse and .bindToParseCloud(), which bypasses the
CloudCodeRegistrar contract and prevents custom registrars from being used;
change ParseServer to retrieve the registrar instance via
CloudCodeRegistrar.getInstance() (or the registrar accessor used in this
codebase) instead of new LegacyCloud(), and call only the registrar's contract
methods (e.g., expose Parse via the registrar API and invoke bindToParseCloud()
through the registrar interface) rather than downcasting to LegacyCloud so
custom CloudCodeRegistrar implementations can plug in without being overwritten.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15231df1-a0ac-4c07-88e1-05844979e470
📒 Files selected for processing (8)
package.jsonsrc/ParseServer.tssrc/cloud-code/CloudCodeRegistrar.tssrc/cloud-code/LegacyCloud.tssrc/cloud-code/Parse.Cloud.jssrc/cloud-code/README.mdsrc/cloud-code/types.tssrc/cloud.ts
💤 Files with no reviewable changes (1)
- src/cloud-code/Parse.Cloud.js
src/ParseServer.ts
Outdated
| // Initialize the cloud SDK and register it | ||
| const cloudSDK = new LegacyCloud(); | ||
| cloudSDK.initialize({ appId, masterKey, javascriptKey, serverURL }); | ||
| CloudCodeRegistrar.setInstance(cloudSDK); | ||
| const Parse = cloudSDK.Parse; |
There was a problem hiding this comment.
ParseServer still depends on LegacyCloud, not the registrar contract.
These paths always install LegacyCloud and later downcast the registry entry back to LegacyCloud for .Parse and .bindToParseCloud(). That means a custom CloudCodeRegistrar implementation—the feature this PR is documenting—cannot actually plug in here without being overwritten or failing at startup.
Also applies to: 166-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ParseServer.ts` around lines 129 - 133, ParseServer currently constructs
a concrete LegacyCloud and uses its .Parse and .bindToParseCloud(), which
bypasses the CloudCodeRegistrar contract and prevents custom registrars from
being used; change ParseServer to retrieve the registrar instance via
CloudCodeRegistrar.getInstance() (or the registrar accessor used in this
codebase) instead of new LegacyCloud(), and call only the registrar's contract
methods (e.g., expose Parse via the registrar API and invoke bindToParseCloud()
through the registrar interface) rather than downcasting to LegacyCloud so
custom CloudCodeRegistrar implementations can plug in without being overwritten.
…letion The SDK adapter refactor replaced Parse.Cloud.js with new modular files (LegacyCloud.ts, TriggerStore.ts, etc.). Upstream modified the old file, so we keep our deletion.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/cloud-code/LegacyCloud.ts (3)
272-285:⚠️ Potential issue | 🟠 MajorAuth trigger validators are still dropped.
cloudAuthTrigger()parsesvalidatorand even uses it forrateLimit, but thetriggers.addTrigger()call omits the validation handler.beforeLogin,afterLogin,afterLogout, andbeforePasswordResetRequesttherefore never enforce their validator rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 272 - 285, The auth trigger registration currently drops the validator when calling triggers.addTrigger, so update the call to include the parsed validator (the same validator variable used for rateLimit) so beforeLogin/afterLogin/afterLogout/beforePasswordResetRequest enforce validation; ensure you pass the validator alongside type, className, handler and this._appId (and adjust the addTrigger argument order if necessary) and keep applyRateLimit using that same validator variable.
214-216:⚠️ Potential issue | 🟠 MajorPartial
Parse.Serverwrites still clobber the cached config.
Config.put(newVal)replaces the whole app config. A setter call likeParse.Server = { enableSingleSchemaCache: true }will drop controllers and the rest of the live configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 214 - 216, The setter in LegacyCloud.ts currently calls Config.put(newVal) which replaces the entire app config and clobbers live state; change it to load the existing config for Parse.applicationId (e.g., via Config.get or currentConfig), merge the incoming newVal into that existing config (shallow-merge only provided keys so controllers and other fields are preserved), ensure appId is set on the merged object, then call Config.put(mergedConfig) so only supplied properties (like enableSingleSchemaCache) are updated instead of overwriting the whole config.
27-31:⚠️ Potential issue | 🟠 MajorSystem-class rate-limit paths still don't match the real REST routes.
getRoute('_User')still resolves to/users/_User{/*id}and_Sessionto/sessions/_Session{/*id}here, sovalidator.rateLimiton those triggers never matches the actual/usersor/sessionsendpoints.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 27 - 31, getRoute currently appends the parseClass name to the path even when ROUTE_MAP already maps that class to a top-level route (so getRoute('_User') -> `/users/_User{/*id}`), breaking rate-limit matching; update getRoute (function name getRoute, symbol ROUTE_MAP and parameter parseClass) so that if ROUTE_MAP[parseClass] exists you return `/${route}{/*id}` (same as the `@File/`@Config branches) instead of `/${route}/${parseClass}{/*id}`, otherwise fall back to the `classes` behavior; keep the existing `@File` and `@Config` special cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 98-107: LegacyCloud.initialize currently assigns
require('parse/node').Parse to global.Parse which causes cross-server
interference; remove writing to global.Parse and instantiate/retain a
per-instance Parse reference in LegacyCloud instead. Update call sites to pass
explicit appId: change maybeRunValidator to accept and use an explicit
applicationId (call getValidator(functionName, applicationId) instead of relying
on Parse.applicationId), modify runLiveQueryEventHandlers to require an appId
parameter and use it rather than defaulting to Parse.applicationId, and update
ParseLiveQueryServer to call runLiveQueryEventHandlers(this.config.appId, ...)
so live-query handlers run against the correct server; ensure any other places
that depended on global.Parse use the LegacyCloud instance or explicit appId
instead.
In `@src/cloud-code/TriggerStore.ts`:
- Around line 163-167: The validator map (s.validators) can retain stale entries
because addFunction/addTrigger/addConnectTrigger only add validators when
provided and removeFunction/removeTrigger never delete them; update these
methods (addFunction, addTrigger, addConnectTrigger) to explicitly remove any
existing validator entry for the given name when no validator is passed (i.e.,
delete s.validators[name] if validator is undefined) and update the removal
methods (removeFunction, removeTrigger, and the connect removal equivalent) to
also delete the corresponding s.validators[name] when removing the handler so
validators can't persist after a hook is replaced or removed.
- Around line 253-260: The clearAll() implementation currently only clears
TriggerStore._stores and therefore skips unregistering cloud rate-limiters;
update clearAll() to iterate over all stored appIds (keys of
TriggerStore._stores) and call the same cleanup logic as removeAllHooks(appId) —
i.e., for each appId call Config.get(appId)?.unregisterRateLimiters() before
clearing the store — or alternatively call removeAllHooks(appId) for each appId
to ensure unregisterRateLimiters runs for every app; ensure the final step still
clears TriggerStore._stores.
In `@src/cloud-code/validateTrigger.ts`:
- Around line 4-5: The current check in validateTrigger.ts only forbids
beforeSave for className === '_PushStatus' but the error says only afterSave is
allowed; change the predicate to reject any trigger on '_PushStatus' whose type
is not TriggerType.afterSave (i.e., if (className === '_PushStatus' && type !==
TriggerType.afterSave) throw 'Only afterSave is allowed on _PushStatus'),
updating the condition that references type, className, and TriggerType (inside
the validateTrigger function) so all unsupported hook types (afterDelete,
beforeFind, etc.) are consistently blocked.
---
Duplicate comments:
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 272-285: The auth trigger registration currently drops the
validator when calling triggers.addTrigger, so update the call to include the
parsed validator (the same validator variable used for rateLimit) so
beforeLogin/afterLogin/afterLogout/beforePasswordResetRequest enforce
validation; ensure you pass the validator alongside type, className, handler and
this._appId (and adjust the addTrigger argument order if necessary) and keep
applyRateLimit using that same validator variable.
- Around line 214-216: The setter in LegacyCloud.ts currently calls
Config.put(newVal) which replaces the entire app config and clobbers live state;
change it to load the existing config for Parse.applicationId (e.g., via
Config.get or currentConfig), merge the incoming newVal into that existing
config (shallow-merge only provided keys so controllers and other fields are
preserved), ensure appId is set on the merged object, then call
Config.put(mergedConfig) so only supplied properties (like
enableSingleSchemaCache) are updated instead of overwriting the whole config.
- Around line 27-31: getRoute currently appends the parseClass name to the path
even when ROUTE_MAP already maps that class to a top-level route (so
getRoute('_User') -> `/users/_User{/*id}`), breaking rate-limit matching; update
getRoute (function name getRoute, symbol ROUTE_MAP and parameter parseClass) so
that if ROUTE_MAP[parseClass] exists you return `/${route}{/*id}` (same as the
`@File/`@Config branches) instead of `/${route}/${parseClass}{/*id}`, otherwise
fall back to the `classes` behavior; keep the existing `@File` and `@Config` special
cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3821d467-0a53-45a0-8fa0-6c339967b7f1
📒 Files selected for processing (8)
src/ParseServer.tssrc/cloud-code/LegacyCloud.tssrc/cloud-code/README.mdsrc/cloud-code/TriggerStore.tssrc/cloud-code/types.tssrc/cloud-code/validateTrigger.tssrc/cloud.tssrc/triggers.js
✅ Files skipped from review due to trivial changes (3)
- src/cloud-code/README.md
- src/cloud.ts
- src/cloud-code/types.ts
| initialize(config: { appId: string; masterKey: string; javascriptKey?: string; serverURL: string }): void { | ||
| this._appId = config.appId; | ||
| this._parse = require('parse/node').Parse; | ||
| this._parse.initialize( | ||
| config.appId, | ||
| config.javascriptKey || 'unused', | ||
| config.masterKey | ||
| ); | ||
| this._parse.serverURL = config.serverURL; | ||
| (global as any).Parse = this._parse; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the changed surface for remaining process-global Parse dependencies.
rg -n -C2 "require\\('parse/node'\\)\\.Parse|global\\.Parse|Parse\\.applicationId" \
src/cloud-code/LegacyCloud.ts src/ParseServer.ts src/triggers.jsRepository: parse-community/parse-server
Length of output: 2567
🏁 Script executed:
# Check how TriggerStore is initialized and used - does it properly isolate by appId?
rg -n "class TriggerStore|new TriggerStore|TriggerStore\." src/triggers.js -A 3Repository: parse-community/parse-server
Length of output: 2265
🏁 Script executed:
# Check the _appId function more closely and how it's used throughout triggers.js
rg -n "_appId|function _appId" src/triggers.js -B 2 -A 5Repository: parse-community/parse-server
Length of output: 2174
🏁 Script executed:
# Look at how triggers are registered and called - is appId properly passed?
rg -n "addTrigger|runTriggers|getTriggers" src/triggers.js -B 1 -A 3Repository: parse-community/parse-server
Length of output: 362
🏁 Script executed:
# Check if there are any safeguards or defensive code around Parse.applicationId usage
rg -n "applicationId.*Parse|Parse.*applicationId" src/cloud-code/LegacyCloud.ts src/triggers.js -B 1 -A 1Repository: parse-community/parse-server
Length of output: 1215
🏁 Script executed:
# Check who calls maybeRunValidator and how they pass applicationId
rg -n "maybeRunValidator" src/ -B 2 -A 2Repository: parse-community/parse-server
Length of output: 2426
🏁 Script executed:
# Check who calls runLiveQueryEventHandlers and how they pass applicationId
rg -n "runLiveQueryEventHandlers" src/ -B 2 -A 2Repository: parse-community/parse-server
Length of output: 3391
🏁 Script executed:
# Check how applicationId flows through RestWrite - is it always available?
rg -n "applicationId|auth\.app" src/RestWrite.js | head -30Repository: parse-community/parse-server
Length of output: 311
🏁 Script executed:
# Check what auth object contains and if it has applicationId
rg -n "interface.*Auth|class.*Auth|auth\\.app" src/Auth.js src/RestWrite.js -A 5 | head -50Repository: parse-community/parse-server
Length of output: 54
🏁 Script executed:
# Check ParseLiveQueryServer initialization - does it have access to appId?
rg -n "constructor|new ParseLiveQueryServer|this\.appId|this\.applicationId" src/LiveQuery/ParseLiveQueryServer.ts -B 2 -A 3 | head -60Repository: parse-community/parse-server
Length of output: 282
🏁 Script executed:
# Look for how applicationId should be passed to ParseLiveQueryServer handlers
rg -n "class ParseLiveQueryServer|constructor.*ParseLiveQueryServer" src/LiveQuery/ParseLiveQueryServer.ts -A 20Repository: parse-community/parse-server
Length of output: 785
🏁 Script executed:
# Check how auth object is structured - does it have applicationId/appId?
rg -n "auth\.appId|auth\.applicationId" src/ -B 2 -A 2 | head -40Repository: parse-community/parse-server
Length of output: 54
🏁 Script executed:
# Verify that maybeRunValidator has the same issue - check all its callers
rg -n "maybeRunValidator" src/triggers.js -B 5 | grep -E "maybeRunValidator|applicationId|auth"Repository: parse-community/parse-server
Length of output: 1101
🏁 Script executed:
# Check if request or auth parameter in maybeRunValidator could contain applicationId
rg -n "function getValidator|export function getValidator" src/triggers.js -A 10Repository: parse-community/parse-server
Length of output: 367
🏁 Script executed:
# Check if runTrigger has access to applicationId or config
rg -n "function runTrigger|export.*runTrigger" src/triggers.js -A 15Repository: parse-community/parse-server
Length of output: 594
🏁 Script executed:
# Check the file trigger and config trigger handlers - they explicitly use config.applicationId
rg -n "fileTrigger|configTrigger" src/triggers.js -B 5 -A 5Repository: parse-community/parse-server
Length of output: 1790
LegacyCloud is still backed by process-global Parse state, breaking multi-server isolation.
initialize() pulls in require('parse/node').Parse and writes it to global.Parse. A second ParseServer in the same process overwrites Parse.applicationId and Parse.serverURL for the first one.
The trigger system has architectural dependencies on this global state:
maybeRunValidator(line 584 insrc/triggers.js) callsgetValidator(functionName, Parse.applicationId)with noapplicationIdparameter, forcing fallback to global staterunLiveQueryEventHandlers(line 897) defaults toParse.applicationIdwhen called without explicitappIdParseLiveQueryServerknows its ownappId(stored inthis.config.appId) but callsrunLiveQueryEventHandlers()without passing it, relying on the global default
This causes the first server's validators and live query handlers to execute against the second server's app configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cloud-code/LegacyCloud.ts` around lines 98 - 107, LegacyCloud.initialize
currently assigns require('parse/node').Parse to global.Parse which causes
cross-server interference; remove writing to global.Parse and instantiate/retain
a per-instance Parse reference in LegacyCloud instead. Update call sites to pass
explicit appId: change maybeRunValidator to accept and use an explicit
applicationId (call getValidator(functionName, applicationId) instead of relying
on Parse.applicationId), modify runLiveQueryEventHandlers to require an appId
parameter and use it rather than defaulting to Parse.applicationId, and update
ParseLiveQueryServer to call runLiveQueryEventHandlers(this.config.appId, ...)
so live-query handlers run against the correct server; ensure any other places
that depended on global.Parse use the LegacyCloud instance or explicit appId
instead.
| static addFunction(appId: string, name: string, handler: Handler, validator?: Validator): void { | ||
| const s = TriggerStore._getOrCreate(appId); | ||
| addToStore(s.functions, name, handler); | ||
| if (validator) addToStore(s.validators, name, validator); | ||
| } |
There was a problem hiding this comment.
Clear validator entries when hooks are replaced or removed.
addFunction / addTrigger / addConnectTrigger only write s.validators when a new validator is provided, and removeFunction / removeTrigger never delete the old entry. Because src/triggers.js resolves validators by name independently of the handler, a re-registered hook can inherit stale auth/field rules from the previous definition.
🐛 Proposed fix
static addFunction(appId: string, name: string, handler: Handler, validator?: Validator): void {
const s = TriggerStore._getOrCreate(appId);
addToStore(s.functions, name, handler);
- if (validator) addToStore(s.validators, name, validator);
+ if (validator) addToStore(s.validators, name, validator);
+ else removeFromStore(s.validators, name);
}
@@
static addTrigger(appId: string, type: string, className: string, handler: Handler, validator?: Validator): void {
validateClassNameForTriggers(className, type);
+ const key = `${type}.${className}`;
const s = TriggerStore._getOrCreate(appId);
- addToStore(s.triggers, `${type}.${className}`, handler);
- if (validator) addToStore(s.validators, `${type}.${className}`, validator);
+ addToStore(s.triggers, key, handler);
+ if (validator) addToStore(s.validators, key, validator);
+ else removeFromStore(s.validators, key);
}
@@
static addConnectTrigger(appId: string, type: string, handler: Handler, validator?: Validator): void {
+ const key = `${type}.${ConnectClassName}`;
const s = TriggerStore._getOrCreate(appId);
- addToStore(s.triggers, `${type}.${ConnectClassName}`, handler);
- if (validator) addToStore(s.validators, `${type}.${ConnectClassName}`, validator);
+ addToStore(s.triggers, key, handler);
+ if (validator) addToStore(s.validators, key, validator);
+ else removeFromStore(s.validators, key);
}
@@
static removeFunction(appId: string, name: string): void {
const s = TriggerStore._stores.get(appId);
- if (s) removeFromStore(s.functions, name);
+ if (s) {
+ removeFromStore(s.functions, name);
+ removeFromStore(s.validators, name);
+ }
}
@@
static removeTrigger(appId: string, type: string, className: string): void {
const s = TriggerStore._stores.get(appId);
- if (s) removeFromStore(s.triggers, `${type}.${className}`);
+ if (s) {
+ const key = `${type}.${className}`;
+ removeFromStore(s.triggers, key);
+ removeFromStore(s.validators, key);
+ }
}Also applies to: 173-184, 243-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cloud-code/TriggerStore.ts` around lines 163 - 167, The validator map
(s.validators) can retain stale entries because
addFunction/addTrigger/addConnectTrigger only add validators when provided and
removeFunction/removeTrigger never delete them; update these methods
(addFunction, addTrigger, addConnectTrigger) to explicitly remove any existing
validator entry for the given name when no validator is passed (i.e., delete
s.validators[name] if validator is undefined) and update the removal methods
(removeFunction, removeTrigger, and the connect removal equivalent) to also
delete the corresponding s.validators[name] when removing the handler so
validators can't persist after a hook is replaced or removed.
| static removeAllHooks(appId: string): void { | ||
| TriggerStore._stores.delete(appId); | ||
| Config.get(appId)?.unregisterRateLimiters(); | ||
| } | ||
|
|
||
| static clearAll(): void { | ||
| TriggerStore._stores.clear(); | ||
| } |
There was a problem hiding this comment.
clearAll() skips cloud rate-limiter cleanup.
removeAllHooks(appId) unregisters cloud rate limiters, but clearAll() only wipes _stores. Since src/triggers.js::_unregisterAll() now routes through this method, teardown can leave stale cloud rate-limit state behind.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cloud-code/TriggerStore.ts` around lines 253 - 260, The clearAll()
implementation currently only clears TriggerStore._stores and therefore skips
unregistering cloud rate-limiters; update clearAll() to iterate over all stored
appIds (keys of TriggerStore._stores) and call the same cleanup logic as
removeAllHooks(appId) — i.e., for each appId call
Config.get(appId)?.unregisterRateLimiters() before clearing the store — or
alternatively call removeAllHooks(appId) for each appId to ensure
unregisterRateLimiters runs for every app; ensure the final step still clears
TriggerStore._stores.
| if (type == TriggerType.beforeSave && className === '_PushStatus') { | ||
| throw 'Only afterSave is allowed on _PushStatus'; |
There was a problem hiding this comment.
Reject unsupported _PushStatus hooks consistently.
The predicate and error text disagree here. As written, this only blocks beforeSave, so unsupported registrations like afterDelete or beforeFind on _PushStatus still get through even though the message says only afterSave is allowed.
🐛 Proposed fix
- if (type == TriggerType.beforeSave && className === '_PushStatus') {
+ if (className === '_PushStatus' && type !== TriggerType.afterSave) {
throw 'Only afterSave is allowed on _PushStatus';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (type == TriggerType.beforeSave && className === '_PushStatus') { | |
| throw 'Only afterSave is allowed on _PushStatus'; | |
| if (className === '_PushStatus' && type !== TriggerType.afterSave) { | |
| throw 'Only afterSave is allowed on _PushStatus'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cloud-code/validateTrigger.ts` around lines 4 - 5, The current check in
validateTrigger.ts only forbids beforeSave for className === '_PushStatus' but
the error says only afterSave is allowed; change the predicate to reject any
trigger on '_PushStatus' whose type is not TriggerType.afterSave (i.e., if
(className === '_PushStatus' && type !== TriggerType.afterSave) throw 'Only
afterSave is allowed on _PushStatus'), updating the condition that references
type, className, and TriggerType (inside the validateTrigger function) so all
unsupported hook types (afterDelete, beforeFind, etc.) are consistently blocked.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/cloud-code/LegacyCloud.ts (2)
100-109:⚠️ Potential issue | 🟠 Major
LegacyCloudis still backed by the shared Parse SDK singleton.
require('parse/node').Parsegives you the process-wide SDK object. Re-initializing it here and assigning it toglobal.Parsemeans the lastParseServerstarted in-process wins; laterParse.Cloud/Parse.Serverbindings then operate against that shared mutable state instead of a single app.Run this read-only check to confirm the remaining global Parse /
Parse.applicationIddependencies:#!/bin/bash rg -n -C2 "require\\('parse/node'\\)\\.Parse|global\\.Parse|bindToParseCloud|Parse\\.Cloud\\[" \ src/cloud-code/LegacyCloud.ts src/ParseServer.ts rg -n -C2 "Parse\\.applicationId|runLiveQueryEventHandlers\\(|maybeRunValidator\\(" \ src/triggers.js src/LiveQuery/ParseLiveQueryServer.tsExpected result: any
global.Parsewrite orParse.applicationIdfallback means the adapter is still process-global instead of app-scoped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 100 - 109, The initialize method currently reinitializes and writes the process-wide Parse singleton (require('parse/node').Parse) and assigns it to global.Parse; stop mutating the global SDK: keep the Parse SDK instance scoped to the LegacyCloud instance (use this._parse only), remove the global assignment (delete the line that sets (global as any).Parse), and ensure all other code paths use this._parse instead of relying on global.Parse or Parse.applicationId; run the suggested ripgrep checks from the review to find remaining globals like global.Parse, Parse.applicationId, Parse.Cloud, bindToParseCloud, runLiveQueryEventHandlers, and update those call sites to accept/forward the instance from LegacyCloud (or change them to instance methods) so the adapter is app-scoped rather than process-global.
27-33:⚠️ Potential issue | 🟠 MajorThe system-class route fix is still incomplete.
getRoute('_User')now returns/users, butcloudBeforeSave()registers bothPOSTandPUT. That covers signup, but it still misses object-level routes likePUT /users/:objectId, sovalidator.rateLimitwill not fire for user updates.@Configis the only mapped route that should stay exact here.🔧 Suggested fix
function getRoute(parseClass: string): string { const route = ROUTE_MAP[parseClass]; if (route) { - if (parseClass === '@File') return `/${route}{/*id}`; - return `/${route}`; + if (parseClass === '@Config') return `/${route}`; + return `/${route}{/*id}`; } return `/classes/${parseClass}{/*id}`; }Also applies to: 127-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloud-code/LegacyCloud.ts` around lines 27 - 33, getRoute currently returns collection-only routes for mapped system classes (e.g., returning "/users" for "_User") which prevents registering object-level handlers; update getRoute so that when ROUTE_MAP[parseClass] exists you return a collection+object route ("/{route}{/*id}") for all mapped classes except the special exact-only "@Config" (keep the existing special-case for "@File" too); adjust the logic in getRoute to check for parseClass === '@File' and parseClass === '@Config' and return the appropriate string so object-level routes (e.g., PUT /users/:objectId) are generated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 187-195: cloudSendEmail currently logs and returns undefined when
no mail adapter exists which causes callers to treat the call as successful;
update cloudSendEmail (in LegacyCloud) to instead reject when emailAdapter is
missing: keep the config.loggerController.error call for logging, then throw or
return Promise.reject(new Error('No mail adapter configured for Parse Server'))
so callers (e.g., await Parse.Cloud.sendEmail(...)) receive a rejected promise
and can handle the failure.
---
Duplicate comments:
In `@src/cloud-code/LegacyCloud.ts`:
- Around line 100-109: The initialize method currently reinitializes and writes
the process-wide Parse singleton (require('parse/node').Parse) and assigns it to
global.Parse; stop mutating the global SDK: keep the Parse SDK instance scoped
to the LegacyCloud instance (use this._parse only), remove the global assignment
(delete the line that sets (global as any).Parse), and ensure all other code
paths use this._parse instead of relying on global.Parse or Parse.applicationId;
run the suggested ripgrep checks from the review to find remaining globals like
global.Parse, Parse.applicationId, Parse.Cloud, bindToParseCloud,
runLiveQueryEventHandlers, and update those call sites to accept/forward the
instance from LegacyCloud (or change them to instance methods) so the adapter is
app-scoped rather than process-global.
- Around line 27-33: getRoute currently returns collection-only routes for
mapped system classes (e.g., returning "/users" for "_User") which prevents
registering object-level handlers; update getRoute so that when
ROUTE_MAP[parseClass] exists you return a collection+object route
("/{route}{/*id}") for all mapped classes except the special exact-only
"@Config" (keep the existing special-case for "@File" too); adjust the logic in
getRoute to check for parseClass === '@File' and parseClass === '@Config' and
return the appropriate string so object-level routes (e.g., PUT
/users/:objectId) are generated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 69ba9810-341e-47d2-ba56-106a3c13355d
📒 Files selected for processing (4)
package.jsonsrc/cloud-code/LegacyCloud.tssrc/cloud-code/types.tssrc/cloud.ts
✅ Files skipped from review due to trivial changes (2)
- src/cloud.ts
- src/cloud-code/types.ts
| cloudSendEmail(data: Record<string, unknown>): unknown { | ||
| const config = Config.get(this._appId); | ||
| const emailAdapter = config.userController.adapter; | ||
| if (!emailAdapter) { | ||
| config.loggerController.error('Failed to send email because no mail adapter is configured for Parse Server.'); | ||
| return; | ||
| } | ||
| return emailAdapter.sendMail(data); | ||
| } |
There was a problem hiding this comment.
cloudSendEmail() should reject when mail is unavailable.
When no mail adapter is configured, this logs and returns undefined. await Parse.Cloud.sendEmail(...) will therefore resolve as if the operation succeeded, which makes missing email configuration very hard to detect from Cloud Code.
💡 Suggested fix
if (!emailAdapter) {
- config.loggerController.error('Failed to send email because no mail adapter is configured for Parse Server.');
- return;
+ const message = 'Failed to send email because no mail adapter is configured for Parse Server.';
+ config.loggerController.error(message);
+ return Promise.reject(new Error(message));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cloud-code/LegacyCloud.ts` around lines 187 - 195, cloudSendEmail
currently logs and returns undefined when no mail adapter exists which causes
callers to treat the call as successful; update cloudSendEmail (in LegacyCloud)
to instead reject when emailAdapter is missing: keep the
config.loggerController.error call for logging, then throw or return
Promise.reject(new Error('No mail adapter configured for Parse Server')) so
callers (e.g., await Parse.Cloud.sendEmail(...)) receive a rejected promise and
can handle the failure.
2 participants
Summary
Test plan
Summary by CodeRabbit
New Features
Documentation