Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Comments

feat: Ignore missinginclude targets (on fetch & query) with optionalignoreIncludeErrors flag.#9872

Open
swittk wants to merge 6 commits intoparse-community:alphafrom
swittk:ignoreincludes
Open

feat: Ignore missinginclude targets (on fetch & query) with optionalignoreIncludeErrors flag.#9872
swittk wants to merge 6 commits intoparse-community:alphafrom
swittk:ignoreincludes

Conversation

@swittk
Copy link

@swittkswittk commentedOct 7, 2025•
edited by coderabbitaibot
Loading

Pull Request

Issue

Closes:#9869

Approach

  • Added an opt-inignoreIncludeErrors boolean query option that flows from the routers through RestQuery, defaulting to the existing strict behaviour (throws Parse.Error.OBJECT_OBJECT_NOT_FOUND when included fields on Query/FetchWithInclude are no longer accessible).
  • When theignoreIncludeErrors flag is set; Updateinclude hydration with so unresolved pointers are left in place instead of throwing (current behavior) when the flag is set; arrays keep their original ordering and any missing entries stay as raw pointer JSON.
    • Edited the database controller to bypass the 101 error for nested FETCH/GET operations only when the option is enabled, while preserving all ACL/CLP enforcement.
  • Added REST tests that cover both unreadable pointers and partially missing arrays to prove the relaxed behaviour.

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)

Summary by CodeRabbit

  • New Features

    • Added an optional ignoreIncludeErrors flag for GET/FIND requests to preserve unreadable or unresolved included pointers (including inside arrays) and return 200 instead of failing.

  • Behavior

    • Default behavior unchanged: without the flag, requests may return 404/OBJECT_NOT_FOUND when includes can’t be hydrated.

  • Tests

    • Added tests covering unreadable/unresolved include pointers and array scenarios with and without the flag.

@parse-github-assistant
Copy link

parse-github-assistantbot commentedOct 7, 2025•
edited
Loading

🚀 Thanks for opening this pull request!

@coderabbitai
Copy link

coderabbitaibot commentedOct 7, 2025•
edited
Loading

📝 Walkthrough

Walkthrough

Adds a per-request ignoreIncludeErrors flag propagated from routers into RestQuery and DatabaseController.find; threads a preserveMissing option through pointer replacement to preserve unresolved/unreadable pointers during include resolution; tests added to validate behavior. No public API removals.

Changes

Cohort / File(s)Summary of Changes
Tests: include error tolerance
spec/rest.spec.js		Adds tests forignoreIncludeErrors: unreadable pointers preserved as Pointer objects when flag is true; arrays preserve unresolved pointers; 404 returned when include hydration fails without the flag.
Routing / request parsing
src/Routers/ClassesRouter.js		AcceptsignoreIncludeErrors in GET query params and FIND bodies, coerces/validates to boolean, and includes it in the options passed to REST handling; updates allowed GET query keys.
Query options surface
src/Adapters/Storage/StorageAdapter.js		AddsignoreIncludeErrors?: boolean to theQueryOptions type.
Database get/find behavior
src/Controllers/DatabaseController.js		AddsignoreIncludeErrors tofind options; when op is'get' and permission/include filtering yields no query, returns[] instead of throwingOBJECT_NOT_FOUND if the flag is true.
Include resolution and pointer replacement
src/RestQuery.js		PropagatesignoreIncludeErrors viapreserveMissing;replacePointers now accepts an options param and preserves unresolved/unreadable pointers (including array elements) when requested; threads options through recursive calls.

Sequence Diagram(s)

sequenceDiagram  autonumber  participant C as Client  participant R as ClassesRouter  participant Q as RestQuery  participant D as DatabaseController  participant S as StorageAdapter  Note over C,R: GET /classes/:Class/:id?include=...&ignoreIncludeErrors=true  C->>R: HTTP GET (include + ignoreIncludeErrors=true)  R->>Q: build RestQuery (ignoreIncludeErrors=true)  Q->>D: find(op='get', options.ignoreIncludeErrors=true)  D->>S: query for include targets  alt include targets unreadable/missing    S-->>D: no readable matches    D-->>Q: return [] (instead of OBJECT_NOT_FOUND due to flag)  else include targets found    S-->>D: matched records    D-->>Q: include records  end  Q->>Q: replacePointers(preserveMissing=true)  Note over Q: unresolved/unreadable pointers preserved as Pointer objects  Q-->>R: result with preserved pointers  R-->>C: 200 OK
Loading
sequenceDiagram  autonumber  participant C as Client  participant R as ClassesRouter  participant Q as RestQuery  participant D as DatabaseController  Note over C,R: GET with include, ignoreIncludeErrors not set  C->>R: HTTP GET (include)  R->>Q: build RestQuery (no ignoreIncludeErrors)  Q->>D: find(op='get', options.ignoreIncludeErrors=false)  alt include permission/filtering removes target    D-->>Q: throws OBJECT_NOT_FOUND    Q-->>R: propagate error    R-->>C: 404 OBJECT_NOT_FOUND  else include targets resolved    D-->>Q: include records    Q-->>R: hydrated object    R-->>C: 200 OK  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to:
    • Correct propagation ofignoreIncludeErrors through REST/Router -> RestQuery -> DatabaseController -> StorageAdapter.
    • Behavior changes in DatabaseController.find when returning[] vs throwing OBJECT_NOT_FOUND for op='get'.
    • replacePointers recursion and array handling whenpreserveMissing is enabled.
    • Type change in QueryOptions and any TypeScript/flow impacts.

Suggested reviewers

  • Moumouls

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check nameStatusExplanation
Title Check✅ PassedThe PR title "feat: Ignore missinginclude targets (on fetch & query) with optionalignoreIncludeErrors flag" clearly and concisely summarizes the primary change: introducing an optional flag to gracefully handle missing include targets. The title is specific enough to convey the feature's purpose and directly reflects the main objective of the changeset across all modified files (StorageAdapter, DatabaseController, RestQuery, ClassesRouter, and tests).
Linked Issues Check✅ PassedThe PR addresses the core requirements from issue#9869. The linked issue requests a backwards-compatible option to prevent include queries from failing when included pointers are deleted or unreadable, with the proposal suggesting modes including 'pointer' (return as raw pointer JSON). The implementation provides an opt-inignoreIncludeErrors boolean flag that, when enabled, implements the 'pointer' mode by preserving unresolved pointers as raw pointer JSON instead of throwing errors, preserves array ordering with missing entries as pointers, maintains ACL/CLP enforcement, and defaults to the strict behavior (throwing errors), satisfying the primary objective of solving the root query failure problem while surfacing missing includes in a tolerant way.
Out of Scope Changes Check✅ PassedAll code changes are directly related to implementing theignoreIncludeErrors feature as described in issue#9869 and the PR objectives. The modifications span from storage adapters (QueryOptions type), database controller (error handling logic), REST query handling (preserveMissing pointer replacement), routing layer (parameter propagation), and corresponding test coverage. No extraneous changes such as refactoring, unrelated bug fixes, or feature modifications outside the scope of this feature are present in the changeset.
Description Check✅ PassedThe PR description follows the repository template structure and includes all required sections: Issue (linked to#9869), Approach (explaining the feature implementation including the preserveMissing behavior and ACL/CLP enforcement), and Tasks (marking tests and documentation as completed). The description provides sufficient context about the implementation, including how unresolved pointers are preserved as raw pointer JSON and how arrays maintain their ordering. While some template sections like security checks are not explicitly addressed, the core required information is present and complete.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for usingCodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@parseplatformorg
Copy link
Contributor

parseplatformorg commentedOct 7, 2025•
edited
Loading

✅Snyk checks have passed. No issues have been found so far.

StatusScanner Critical High Medium LowTotal (0)
✅Open Source Security0000 0 issues

💻 Catch issues earlier using the plugins forVS Code,JetBrains IDEs,Visual Studio, andEclipse.

@swittkswittk changed the titleFeature: Ignore missinginclude targets (on fetch & query) with optionalignoreIncludeErrors flag.feature: Ignore missinginclude targets (on fetch & query) with optionalignoreIncludeErrors flag.Oct 7, 2025
Copy link

@coderabbitaicoderabbitaibot 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: 0

🧹 Nitpick comments (1)
src/RestQuery.js (1)

1116-1119:Clean up the lingering TODO

We’re shipping with a TODO that questions whether the new behavior is correct; please replace it with a concrete explanation (or remove it) so the intent is clear to future readers.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweenbe362fe and7e4e987.

📒 Files selected for processing (5)
  • spec/rest.spec.js (1 hunks)
  • src/Adapters/Storage/StorageAdapter.js (1 hunks)
  • src/Controllers/DatabaseController.js (2 hunks)
  • src/RestQuery.js (7 hunks)
  • src/Routers/ClassesRouter.js (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/Routers/ClassesRouter.js (2)
src/PromiseRouter.js (1)
  • body (141-141)
src/Controllers/DatabaseController.js (1)
  • options (1790-1790)
spec/rest.spec.js (2)
src/RestQuery.js (1)
  • schemaController (789-789)
src/rest.js (1)
  • schemaController (71-71)

@swittkswittk changed the titlefeature: Ignore missinginclude targets (on fetch & query) with optionalignoreIncludeErrors flag.feat: Ignore missinginclude targets (on fetch & query) with optionalignoreIncludeErrors flag.Oct 7, 2025
Copy link

@coderabbitaicoderabbitaibot 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: 0

🧹 Nitpick comments (1)
src/RestQuery.js (1)

1116-1119:Clean up the lingering TODO.

This TODO leaves an unresolved question in production code. WithpreserveMissing we keep the original pointer object, so arrays shouldn’t accrueundefined entries. Please drop the TODO (or replace it with a concrete assertion/test) before merging.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between7e4e987 and1c2b0e2.

📒 Files selected for processing (2)
  • spec/rest.spec.js (1 hunks)
  • src/RestQuery.js (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • spec/rest.spec.js

Copy link

@coderabbitaicoderabbitaibot 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweene287f10 andccc9cad.

📒 Files selected for processing (3)
  • spec/rest.spec.js (1 hunks)
  • src/Controllers/DatabaseController.js (2 hunks)
  • src/RestQuery.js (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • spec/rest.spec.js
  • src/Controllers/DatabaseController.js
🔇 Additional comments (4)
src/RestQuery.js (4)

212-222:LGTM! Clean option propagation.

TheignoreIncludeErrors option is correctly added to the list of options propagated fromrestOptions tofindOptions, following the established pattern for other query options.

1023-1029:LGTM! Correct flag derivation and propagation.

ThepreserveMissing flag is properly derived fromrestOptions.ignoreIncludeErrors with appropriate defaults, and the option is correctly propagated toincludeRestOptions for nested include operations.

1071-1073:LGTM! Options correctly passed to replacePointers.

ThepreserveMissing flag is properly passed through toreplacePointers via the options object.

1118-1154:LGTM! Solid implementation of preserveMissing logic.

The implementation correctly handles thepreserveMissing option:

  • Defaults safely withoptions = {} and!!options.preserveMissing
  • Arrays preserve unresolved pointers whenpreserveMissing is true, or filter them when false
  • Leaf pointer replacement returns the original pointer object when replacement is missing andpreserveMissing is true
  • Options are properly threaded through recursive calls

This maintains backward compatibility (default strict behavior) while enabling the new tolerant include resolution mode.

Comment on lines +1114 to 1116
// `options` is an optional options object; options currently include
// `preserveMissing?: boolean` where if it is true
// Returns something analogous to object, but with the appropriate

Choose a reason for hiding this comment

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

⚠️ Potential issue |🟡 Minor

Minor: Incomplete comment.

The comment has an incomplete sentence: "where if it is true" ends abruptly.

Consider completing the sentence:

-// `options` is an optional options object; options currently include-// `preserveMissing?: boolean` where if it is true+// `options` is an optional options object; options currently include:+// `preserveMissing?: boolean` - if true, missing pointer replacements are kept as-is
📝 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.

Suggested change
// `options` is an optional options object; options currently include
// `preserveMissing?: boolean`where ifit is true
// Returns something analogous to object, but with the appropriate
// `options` is an optional options object; options currently include:
// `preserveMissing?: boolean`- iftrue, missing pointer replacements are kept as-is
// Returns something analogous to object, but with the appropriate
🤖 Prompt for AI Agents
In src/RestQuery.js around lines 1114 to 1116, the comment block ends with anincomplete sentence ("where if it is true"). Update the comment to complete thatsentence and make the behavior clear (for example: explain what preserveMissingtrue does — e.g., "where if it is true, missing properties on the source objectshould be preserved in the result instead of being omitted"), and ensure thefull comment reads as a complete description of the options and the returnedvalue.

@codecov
Copy link

codecovbot commentedOct 25, 2025•
edited
Loading

Codecov Report

❌ Patch coverage is94.44444% with1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.01%. Comparing base (b298ccc) to head (ccc9cad).
⚠️ Report is 1 commits behind head on alpha.

Files with missing linesPatch %Lines
src/Routers/ClassesRouter.js75.00%1 Missing⚠️
Additional details and impacted files
@@            Coverage Diff             @@##            alpha    #9872      +/-   ##==========================================- Coverage   93.01%   93.01%   -0.01%==========================================  Files         187      187                Lines       15163    15176      +13       Branches      177      177              ==========================================+ Hits        14104    14116      +12- Misses       1047     1048       +1  Partials       12       12

☔ 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.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@coderabbitaicoderabbitai[bot]coderabbitai[bot] requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@swittk@parseplatformorg@mtrezza
[8]ページ先頭
©2009-2026 Movatter.jp