-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: Ignore missing include
targets (on fetch & query) with optional ignoreIncludeErrors
flag.
#9872
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
base: alpha
Are you sure you want to change the base?
Conversation
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdds a per-request ignoreIncludeErrors flag across REST GET/FIND flows. Routers accept and propagate the flag; RestQuery threads a preserveMissing option into pointer replacement to preserve unreadable/unresolved pointers; DatabaseController.find may return [] for get when include filtering removes the target and the flag is true. Tests added. Changes
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 (Pointer objects remain)
Q-->>R: result with preserved pointers
R-->>C: 200 OK
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
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. |
include
targets (on fetch & query) with optional ignoreIncludeErrors
flag.include
targets (on fetch & query) with optional ignoreIncludeErrors
flag.
There was a problem hiding this 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 TODOWe’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
📒 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)
include
targets (on fetch & query) with optional ignoreIncludeErrors
flag.include
targets (on fetch & query) with optional ignoreIncludeErrors
flag.
There was a problem hiding this 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. With
preserveMissing
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
📒 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
Pull Request
Issue
Closes: #9869
Approach
ignoreIncludeErrors
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).ignoreIncludeErrors
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.Tasks
Summary by CodeRabbit
New Features
Behavior
Tests