Skip to content

refactor: Isolate Parse.Query SDK usage behind a QueryAdapter#10486

Open
dblythy wants to merge 3 commits into
parse-community:alphafrom
dblythy:chore/src-remove-internal-parse-query
Open

refactor: Isolate Parse.Query SDK usage behind a QueryAdapter#10486
dblythy wants to merge 3 commits into
parse-community:alphafrom
dblythy:chore/src-remove-internal-parse-query

Conversation

@dblythy
Copy link
Copy Markdown
Member

@dblythy dblythy commented May 30, 2026

Towards #8787.

Removes parse-server's internal use of the Parse JS SDK's Parse.Query and isolates the remaining trigger-facing usage behind a single adapter module.

Changes

  • src/cloud-code/QueryAdapter.js (new) — the single boundary between parse-server's internal REST/JSON query format and the SDK's Parse.Query. Exposes inflateQuery / deflateQuery / isQuery / applyQueryToRest. This is the only place in src/ that constructs or inspects a Parse.Query.
  • Auth/LiveQuery internal queries removed — deleted the vestigial !config SDK fallback branches in src/Auth.js (getAuthForSessionToken, getRolesForUser, getRolesByIds), threaded a real config into the two LiveQuery getAuthForSessionToken calls, and converted LiveQuery role-cache invalidation from new Parse.Query(...) to RestQuery. Deleted the dead, never-instantiated SessionTokenCache.
  • Core now calls the adaptertriggers.js, rest.js, RestQuery.js, and ParseLiveQueryServer.ts (beforeSubscribe) go through the adapter instead of touching Parse.Query directly.

No behavior change: cloud-code triggers (beforeFind / afterFind / beforeSubscribe) still receive and may return a real Parse.Query. The Parse.Object side of the trigger contract is intentionally out of scope and will follow in a separate PR.

Test plan

  • CloudCode, ParseLiveQuery, ParseLiveQueryServer, QueryTools — 387 specs, 0 failures
  • ParseQuery — 226 specs, 0 failures
  • Auth, ParseRole regression

Summary by CodeRabbit

  • Bug Fixes

    • More consistent role- and session-based access checks across live queries and auth flows; ACL evaluations behave reliably.
    • beforeFind hook now preserves explicit query overrides (e.g., limit(0)).
  • Refactor

    • Unified query handling for cloud-code and triggers, improving consistency when queries are passed between triggers and REST layers.
  • Tests

    • Updated and added tests covering role resolution, live query ACLs, and beforeFind behavior.

Review Change Stack

dblythy added 2 commits May 30, 2026 13:47
Towards parse-community#8787. Auth's session/role resolution and LiveQuery's
_clearCachedRoles now always go through RestQuery instead of the Parse
JS SDK's Parse.Query. The !config SDK fallback branches in Auth
(getAuthForSessionToken, getRolesForUser, getRolesByIds) are removed —
all real callers already pass a config, so a config is now required.
Threads config into the LiveQuery getAuthForSessionToken calls. Deletes
the dead SessionTokenCache (exported but never used in src). Drops the
obsolete no-config Auth/Role specs and rewrites the role-based ACL
LiveQuery specs to mock the auth seam instead of the removed Parse.Query.
Introduce src/cloud-code/QueryAdapter.js as the single boundary between
parse-server's internal REST/JSON query format and the Parse JS SDK's
Parse.Query. Core code (triggers, rest, RestQuery, LiveQuery) now calls
inflateQuery/deflateQuery/isQuery/applyQueryToRest instead of constructing
or inspecting Parse.Query directly, so the SDK query type lives in one place.

Towards parse-community#8787.
@parse-github-assistant
Copy link
Copy Markdown

🚀 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

  • Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
  • Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
  • Group code into logical blocks. Add a short comment before each block to explain its purpose.
  • We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
  • Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
  • Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

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. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 67d599e4-0342-46c4-bb30-f1947a57452d

📥 Commits

Reviewing files that changed from the base of the PR and between bb2d8ae and 0638023.

📒 Files selected for processing (2)
  • spec/CloudCode.spec.js
  • src/cloud-code/QueryAdapter.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/cloud-code/QueryAdapter.js

📝 Walkthrough

Walkthrough

Adds a QueryAdapter to convert between REST query JSON and Parse.Query, replaces config-dependent query/role/session branches with RestQuery-based flows in Auth and LiveQuery, integrates the adapter into triggers and REST, and updates tests to the new behavior.

Changes

Query Adapter and Unified Query Resolution

Layer / File(s) Summary
Query Adapter foundation module
src/cloud-code/QueryAdapter.js
New module exports inflateQuery, deflateQuery, isQuery, and applyQueryToRest and a whitelist of REST option keys.
Auth simplification to unified RestQuery paths
src/Auth.js
getAuthForSessionToken, getRolesForUser, and getRolesByIds now always use RestQuery (no config-dependent Parse.Query branches).
Trigger system QueryAdapter integration
src/triggers.js
Trigger handlers use isQuery/inflateQuery/applyQueryToRest to normalize REST inputs and return values, replacing manual Parse.Query construction and toJSON() extraction.
REST and RestQuery inflation usage
src/rest.js, src/RestQuery.js
Use inflateQuery to build Parse.Query objects for afterFind/runAfterFindTrigger paths instead of new Parse.Query(...).withJSON(...).
LiveQuery system updates with QueryAdapter and RestQuery
src/LiveQuery/ParseLiveQueryServer.ts, src/LiveQuery/QueryTools.js
BeforeSubscribe now inflates/deflates queries with the adapter; QueryTools uses isQuery() for detection; _clearCachedRoles uses RestQuery and getAuthForSessionToken calls include explicit config.
Test updates for config-independent paths
spec/Auth.spec.js, spec/ParseRole.spec.js, spec/ParseLiveQueryServer.spec.js, spec/CloudCode.spec.js
Removed tests asserting behavior without config, updated role-loading tests to use Config.get('test'), mocked getAuthForSessionToken in LiveQuery ACL tests, and added a beforeFind test ensuring falsy query overrides (e.g., limit(0)) are preserved.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • Moumouls
🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Engage In Review Feedback ❓ Inconclusive Cannot verify engagement in review feedback discussion; local repository lacks access to GitHub PR comments/discussion threads needed to confirm prior engagement per check requirements. Access GitHub PR #10486 discussion to verify the user engaged in feedback discussion and either implemented feedback or convinced reviewers to retract it.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title begins with 'refactor:' prefix and accurately describes the main change: isolating Parse.Query SDK usage behind a new QueryAdapter boundary.
Description check ✅ Passed The pull request description provides comprehensive details about the changes, approach, and test plan, though it deviates from the template structure by not explicitly using template section headings.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Check ✅ Passed Comprehensive security review found no vulnerabilities. QueryAdapter enforces strict whitelist, role validation prevents privilege escalation, and config/auth handling is secure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.22.0)

OpenGrep fatal error (exit code 2): [00.10][ERROR]: Error: exception Unix_error: No such file or directory stat spec/CloudCode.spec.js
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17-20
Called from Scan_


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cloud-code/QueryAdapter.js`:
- Around line 59-62: The loop in applyQueryToRest that assigns restOptions only
when jsonQuery[key] is truthy drops intentional falsy overrides (e.g., limit(0),
skip(0), explain(false)); change the condition to test for the presence of the
key instead (e.g., Object.prototype.hasOwnProperty.call(jsonQuery, key) or key
in jsonQuery) so you still initialize restOptions when needed and assign
restOptions[key] = jsonQuery[key], preserving falsy values; update the code that
iterates REST_OPTION_KEYS and uses jsonQuery and restOptions accordingly.
🪄 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: cb02caa6-ffec-4d8a-87c7-0cd783972b51

📥 Commits

Reviewing files that changed from the base of the PR and between 1e0d6ce and bb2d8ae.

📒 Files selected for processing (12)
  • spec/Auth.spec.js
  • spec/ParseLiveQueryServer.spec.js
  • spec/ParseRole.spec.js
  • spec/SessionTokenCache.spec.js
  • src/Auth.js
  • src/LiveQuery/ParseLiveQueryServer.ts
  • src/LiveQuery/QueryTools.js
  • src/LiveQuery/SessionTokenCache.js
  • src/RestQuery.js
  • src/cloud-code/QueryAdapter.js
  • src/rest.js
  • src/triggers.js
💤 Files with no reviewable changes (4)
  • src/LiveQuery/SessionTokenCache.js
  • spec/ParseRole.spec.js
  • spec/SessionTokenCache.spec.js
  • spec/Auth.spec.js

Comment thread src/cloud-code/QueryAdapter.js
@codecov
Copy link
Copy Markdown

codecov Bot commented May 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.62%. Comparing base (552c6dd) to head (0638023).
⚠️ Report is 1 commits behind head on alpha.

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha   #10486      +/-   ##
==========================================
+ Coverage   92.60%   92.62%   +0.02%     
==========================================
  Files         193      193              
  Lines       16893    16845      -48     
  Branches      234      234              
==========================================
- Hits        15643    15602      -41     
+ Misses       1227     1220       -7     
  Partials       23       23              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

applyQueryToRest tested option truthiness, so an explicit falsy override
from a beforeFind trigger (e.g. query.limit(0)) was silently dropped. Test
presence with hasOwnProperty instead. Parse.Query#toJSON only emits keys that
were set, so no default values leak in. Adds a regression test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant