fix: LiveQuery throws on create event using Parse.Query.containedIn with null value in array or using Parse.Query.equalTo for Parse.Object with key value null
#9744
Conversation
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
|
🚀 Thanks for opening this pull request! ❌ Please fill out all fields with a placeholder |
📝 WalkthroughWalkthroughTwo new focused test cases were added to the Changes
Sequence Diagram(s)sequenceDiagram
participant TestCase as Test Case
participant Server as Server
participant LiveQuery as Live Query Server
participant Client as Client
TestCase->>Server: Configure live query for TestObject
TestCase->>LiveQuery: Start live query server
Client->>LiveQuery: Subscribe to query (with pointer constraint)
Client->>Server: Save new TestObject (with pointer array or null)
LiveQuery->>Client: Emit 'create' event (if applicable)
Note right of LiveQuery: Ensure no server errors or crashes
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
spec/ParseLiveQuery.spec.js (5)
1328-1328: Remove trailing whitespaceThere's a trailing whitespace on this line.
- await child3.save(); - + await child3.save();🧰 Tools
🪛 ESLint
[error] 1328-1328: Trailing spaces not allowed.
(no-trailing-spaces)
1331-1331: Remove trailing whitespaceThere's a trailing whitespace on this line.
- query.containedIn('childs', [child1, child2, null]); - + query.containedIn('childs', [child1, child2, null]);🧰 Tools
🪛 ESLint
[error] 1331-1331: Trailing spaces not allowed.
(no-trailing-spaces)
1357-1357: Remove trailing whitespaceThere's a trailing whitespace on this line.
- query.equalTo('child', child); - + query.equalTo('child', child);🧰 Tools
🪛 ESLint
[error] 1357-1357: Trailing spaces not allowed.
(no-trailing-spaces)
1332-1339: Consider adding assertions to verify behaviorCurrently, the test just ensures the server doesn't crash, but it would be better to assert the expected behavior. Consider adding assertions to verify that objects with different child arrays than the query are correctly handled.
subscription.on('create', () => {}); // Do not need any expect block, just make sure that the server doesn't crash or throw error const object1 = new TestObject(); object1.set('childs', [child3]); await object1.save(); + + // Add assertion to verify that the subscription doesn't receive a notification + // Wait a short time to ensure any event would have been processed + await new Promise(resolve => setTimeout(resolve, 100)); + expect(subscription.subscribed).toBe(true); // Verify subscription is still active🧰 Tools
🪛 ESLint
[error] 1336-1336: 'TestObject' is not defined.
(no-undef)
1359-1365: Consider adding assertions to verify behaviorSimilar to the first test, this test just ensures the server doesn't crash, but it would be better to assert the expected behavior. Consider adding assertions to verify that objects with null pointer fields are correctly handled.
subscription.on('create', () => {}); // Do not need any expect block, just make sure that the server doesn't crash or throw error const object1 = new TestObject(); object1.set('child', null); await object1.save(); + + // Add assertion to verify that the subscription doesn't receive a notification + // Wait a short time to ensure any event would have been processed + await new Promise(resolve => setTimeout(resolve, 100)); + expect(subscription.subscribed).toBe(true); // Verify subscription is still active🧰 Tools
🪛 ESLint
[error] 1362-1362: 'TestObject' is not defined.
(no-undef)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/ParseLiveQuery.spec.js(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
spec/ParseLiveQuery.spec.js
[error] 1312-1312: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
[error] 1342-1342: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
🪛 ESLint
spec/ParseLiveQuery.spec.js
[error] 1312-1312: 'fit' is not defined.
(no-undef)
[error] 1313-1313: 'reconfigureServer' is not defined.
(no-undef)
[error] 1328-1328: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1329-1329: 'TestObject' is not defined.
(no-undef)
[error] 1331-1331: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1336-1336: 'TestObject' is not defined.
(no-undef)
[error] 1342-1342: 'fit' is not defined.
(no-undef)
[error] 1343-1343: 'reconfigureServer' is not defined.
(no-undef)
[error] 1355-1355: 'TestObject' is not defined.
(no-undef)
[error] 1357-1357: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1362-1362: 'TestObject' is not defined.
(no-undef)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Docker Build
RahulLanjewar93
changed the title
null value in db or new object is created with value as null
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/ParseLiveQuery.spec.js (1)
1328-1328: Minor: Remove trailing whitespace.There are a few lines with trailing whitespace that should be removed to follow coding style guidelines and avoid linting errors.
- +Also applies to: 1331-1331, 1357-1357
🧰 Tools
🪛 ESLint
[error] 1328-1328: Trailing spaces not allowed.
(no-trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
spec/ParseLiveQuery.spec.js(1 hunks)src/LiveQuery/QueryTools.js(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
spec/ParseLiveQuery.spec.js (5)
spec/helper.js (3)
reconfigureServer(171-205)Parse(4-4)TestObject(257-259)spec/ParseQuery.spec.js (2)
Parse(7-7)TestObject(2937-2937)spec/ParseLiveQueryServer.spec.js (7)
Parse(1-1)subscription(354-354)subscription(413-413)subscription(468-473)subscription(1192-1194)subscription(1205-1207)subscription(1894-1896)spec/ParseGeoPoint.spec.js (1)
TestObject(5-5)spec/ParsePolygon.spec.js (1)
TestObject(1-1)
🪛 ESLint
spec/ParseLiveQuery.spec.js
[error] 1312-1312: 'it' is not defined.
(no-undef)
[error] 1313-1313: 'reconfigureServer' is not defined.
(no-undef)
[error] 1328-1328: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1329-1329: 'TestObject' is not defined.
(no-undef)
[error] 1331-1331: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1336-1336: 'TestObject' is not defined.
(no-undef)
[error] 1342-1342: 'it' is not defined.
(no-undef)
[error] 1343-1343: 'reconfigureServer' is not defined.
(no-undef)
[error] 1355-1355: 'TestObject' is not defined.
(no-undef)
[error] 1357-1357: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1362-1362: 'TestObject' is not defined.
(no-undef)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Redis Cache
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: Docker Build
🔇 Additional comments (4)
src/LiveQuery/QueryTools.js (2)
102-102: Good addition of null check to prevent runtime errors.This is a robust defensive check that ensures
ptris not null before attempting to access its properties. It prevents potential runtime errors when handling pointer fields that could be null, which directly addresses the first issue mentioned in the PR regardingcontainedInqueries with null values.
215-215: Well-placed null check for pointer comparison.Adding this null check prevents runtime errors when comparing objects where one might be null. This directly addresses the second issue in the PR regarding
equalToconstraints where the field value could be null.spec/ParseLiveQuery.spec.js (2)
1312-1339: Good test case for the containedIn null pointer issue.This test effectively validates the fix for the first scenario described in the PR where live queries would fail when using
containedInwith an array containing null. The test creates a subscription with a mixed array of pointers and null, then verifies the server doesn't crash when a new object is saved with a different pointer array.Note that this test doesn't make assertions, as it's focused on stability testing (ensuring no errors are thrown).
🧰 Tools
🪛 ESLint
[error] 1312-1312: 'it' is not defined.
(no-undef)
[error] 1313-1313: 'reconfigureServer' is not defined.
(no-undef)
[error] 1328-1328: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1329-1329: 'TestObject' is not defined.
(no-undef)
[error] 1331-1331: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1336-1336: 'TestObject' is not defined.
(no-undef)
1342-1365: Good test case for the equalTo null pointer issue.This test validates the fix for the second scenario described in the PR, ensuring live queries don't crash when using
equalTowith a pointer and handling objects where that field is set to null. The test properly sets up a subscription with anequalToconstraint, then creates an object with the pointer field set to null.As with the previous test, it focuses on stability rather than making assertions, which is appropriate for this type of error-prevention test.
🧰 Tools
🪛 ESLint
[error] 1342-1342: 'it' is not defined.
(no-undef)
[error] 1343-1343: 'reconfigureServer' is not defined.
(no-undef)
[error] 1355-1355: 'TestObject' is not defined.
(no-undef)
[error] 1357-1357: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 1362-1362: 'TestObject' is not defined.
(no-undef)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## alpha #9744 +/- ##
=======================================
Coverage 93.00% 93.00%
=======================================
Files 187 187
Lines 15082 15082
Branches 174 174
=======================================
Hits 14027 14027
Misses 1043 1043
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The title could have been better but I'm out of ideas |
…eQuery-contains-and-matchesQuery
|
@mtrezza This is also a bug. added a fix |
| const subscription = await query.subscribe(); | ||
| subscription.on('create', () => {}); | ||
|
|
||
| // Do not need any expect block, just make sure that the server doesn't crash or throw error |
There was a problem hiding this comment.
Add expect, presumably await expectAsync(...).toBeResolved();?
| // Do not need any expect block, just make sure that the server doesn't crash or throw error | ||
| const object1 = new TestObject(); | ||
| object1.set('child', null); | ||
| await object1.save(); |
| }); | ||
|
|
||
|
|
||
| it('Live query should work if we set equalTo(someKey,someParseObject) and new Parse object is created but someKey = null', async () => { |
There was a problem hiding this comment.
Test descriptions should be more concise, see others for examples. If there is anything important that requires a longer explanation you could add a comment block inside the test.
| it('Live query should work if we set equalTo(someKey,someParseObject) and new Parse object is created but someKey = null', async () => { | |
| it('triggers create event with equalTo for object with key value null', async () => { |
| @@ -1308,4 +1308,59 @@ describe('ParseLiveQuery', function () { | |||
| await new Promise(resolve => setTimeout(resolve, 100)); | |||
| expect(createSpy).toHaveBeenCalledTimes(1); | |||
| }); | |||
|
|
|||
| it('Live query should work if needle is ParsePointer and haystack is any[], checking QueryTools.js>contains', async () => { | |||
mtrezza
changed the title
null value in db or new object is created with value as nullcreate event using Parse.Query.containedIn with null value or Parse.Object is saved with key value as null
mtrezza
changed the title
create event using Parse.Query.containedIn with null value or Parse.Object is saved with key value as nullcreate event using Parse.Query.containedIn with null value or Parse.Query.equalTo for Parse.Object key value null
mtrezza
changed the title
create event using Parse.Query.containedIn with null value or Parse.Query.equalTo for Parse.Object key value nullcreate event using Parse.Query.containedIn with null value in array or using Parse.Query.equalTo for Parse.Object with key value null
Pull Request
Issue
Live query throw error if we create subsciption for
ParentObjectQuery.containedIn('childs', [...childs,null])and a ParentObject is created withParentObject.set('childs',[newChild]ParentObjectQuery.equalTo('child', child)and a ParentObject is created withParentObject.set('child',null)Closes: FILL_THIS_OUT
Approach
Add not null and not object check before doing the actual checks
Tasks
Summary by CodeRabbit