Skip to content

Conversation

theSlyest
Copy link

@theSlyest theSlyest commented Jun 25, 2025

Added:

  • ParseLiveQuery class
  • ParseLiveQueryController class
  • ParseLiveQuerySubscription class
  • API ParseQuery.GetLive() for creating a ParseLiveQuery
  • LiveQueryServerConnectionData as a ServerConnectionData field to ParseClient to save live query server info.
  • APIs ConnectLiveQueryServerAsync(...) and DisconnectLiveQueryServerAsync(...) to ObjectServiceExtensions for connecting to and disconnecting respectively from the given parse live query server.
  • Some other smaller classes and interfaces...

Summary by CodeRabbit

  • New Features

    • Full Live Query support: real‑time subscribe/update/unsubscribe with create/enter/update/leave/delete events over WebSocket.
    • Service surface and configuration for Live Queries exposed (connection data, timeouts, buffer sizes) and ParseQuery.GetLive to create live queries.
    • New text WebSocket transport and session‑aware message building/parsing; APIs to connect/disconnect Live Query servers; ParseClient accepts live‑query config.
  • Tests

    • Extensive unit tests covering messaging, parser/builder, WebSocket client, controller, subscriptions, event args, and related flows.

Copy link

parse-github-assistant bot commented Jun 25, 2025

🚀 Thanks for opening this pull request!

Copy link

coderabbitai bot commented Jun 25, 2025

📝 Walkthrough

Walkthrough

Adds Parse Live Query support: new live-query connection data, WebSocket client and event args, message parser/builder, controller and subscription types, service-hub wiring and ParseClient overloads, query-to-live conversion and connect/disconnect helpers, plus unit tests.

Changes

Cohort / File(s) Change Summary
Connection data
Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs, Parse/Infrastructure/LiveQueryServerConnectionData.cs
New ILiveQueryServerConnectionData interface and LiveQueryServerConnectionData struct with Timeout, MessageBufferSize, defaults and server/app keys/headers.
WebSocket client & events
Parse/Abstractions/Infrastructure/Execution/IWebSocketClient.cs, Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs, Parse/Infrastructure/Execution/TextWebSocketClient.cs
Added IWebSocketClient (events + Open/Close/Send), MessageReceivedEventArgs, and TextWebSocketClient implementation (async open/close/send, listener loop, multi-frame handling, events, cancellation, disposal).
LiveQuery controller
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryController.cs, Parse/Platform/LiveQueries/ParseLiveQueryController.cs
New IParseLiveQueryController and concrete ParseLiveQueryController with states, Connect/Subscribe/Update/Unsubscribe/Close, per-request signaling, error eventing, lifecycle and disposal.
Message parser & builder
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs, Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs, Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs, Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs
Added parser and builder interfaces and implementations to parse incoming messages and build connect/subscribe/update/unsubscribe payloads (includes LiveQueryError struct and session token injection).
Subscriptions & events
Parse/Abstractions/Platform/LiveQueries/IParseLiveQuerySubscription.cs, Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs, Parse/Platform/LiveQueries/ParseLiveQueryEventArgs.cs, Parse/Platform/LiveQueries/ParseLiveQueryDualEventArgs.cs, Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs
Added subscription contract and ParseLiveQuerySubscription<T>, event-arg types (single/dual-state and error), events, lifecycle methods and internal invokers.
LiveQuery API & query integration
Parse/Platform/LiveQueries/ParseLiveQuery.cs, Parse/Platform/Queries/ParseQuery.cs
New ParseLiveQuery<T> (filters, selected/watched keys, Watch, BuildParameters, SubscribeAsync) and ParseQuery<T>.GetLive() conversion.
Service hub integration
Parse/Abstractions/Infrastructure/...
Parse/Abstractions/Infrastructure/IServiceHub.cs, .../IMutableServiceHub.cs, .../CustomServiceHub.cs, Parse/Infrastructure/ServiceHub.cs, Parse/Infrastructure/MutableServiceHub.cs, Parse/Infrastructure/LateInitializedMutableServiceHub.cs, Parse/Infrastructure/OrchestrationServiceHub.cs
Extended hub interfaces and implementations to expose LiveQueryServerConnectionData, IWebSocketClient, IParseLiveQueryMessageParser, IParseLiveQueryMessageBuilder, and IParseLiveQueryController; added mutable setters and lazy initialization/delegation.
ParseClient wiring
Parse/Platform/ParseClient.cs
New constructor overload accepting ILiveQueryServerConnectionData with validation, test-mode handling, and application of service-hub configurators to wire live-query config.
Utilities / helpers
Parse/Utilities/ObjectServiceExtensions.cs
Added ConnectLiveQueryServerAsync and DisconnectLiveQueryServerAsync IServiceHub extension methods; tiny whitespace cleanup.
Tests — LiveQuery and related
Parse.Tests/*LiveQuery*.cs, Parse.Tests/MessageReceivedEventArgsTests.cs, Parse.Tests/TextWebSocketClientTests.cs
Added comprehensive unit tests for message builder/parser, controller/subscription behaviors, event args, TextWebSocketClient, and related components.
Tests — other adjustments
Parse.Tests/EncoderTests.cs, Parse.Tests/DecoderTests.cs, Parse.Tests/ObjectCoderTests.cs, Parse.Tests/SessionTests.cs, Parse.Tests/RelationTests.cs, Parse.Tests/UserTests.cs, Parse.Tests/Parse.Tests.csproj
Test setup changes (constructors call Client.Publicize()), moved some tests into Parse.Tests namespace, removed redundant signup calls, adjusted mocks/bindings, and updated test package versions.

Sequence Diagram(s)

Live Query Subscription Flow

sequenceDiagram
    autonumber
    participant App
    participant ParseQuery
    participant ParseLiveQuery
    participant LiveQueryController
    participant WebSocketClient
    participant Server

    App->>ParseQuery: GetLive()
    ParseQuery->>ParseLiveQuery: create
    App->>ParseLiveQuery: SubscribeAsync()
    ParseLiveQuery->>LiveQueryController: SubscribeAsync(liveQuery)
    LiveQueryController->>WebSocketClient: OpenAsync(serverUri)
    WebSocketClient->>Server: Open WebSocket
    LiveQueryController->>WebSocketClient: SendAsync(subscribe)
    Server-->>WebSocketClient: events (create/update/...)
    WebSocketClient-->>LiveQueryController: MessageReceived
    LiveQueryController-->>ParseLiveQuerySubscription: OnCreate/OnUpdate/...
    ParseLiveQuerySubscription-->>App: Raise event
Loading

Live Query Connect / Disconnect

sequenceDiagram
    participant App
    participant IServiceHub
    participant LiveQueryController
    participant WebSocketClient

    App->>IServiceHub: ConnectLiveQueryServerAsync(onError)
    IServiceHub->>LiveQueryController: ConnectAsync()
    LiveQueryController->>WebSocketClient: OpenAsync(serverUri)

    App->>IServiceHub: DisconnectLiveQueryServerAsync()
    IServiceHub->>LiveQueryController: CloseAsync()
    LiveQueryController->>WebSocketClient: CloseAsync()
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.42% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: Add livequery support" is concise and accurately summarizes the primary change (adding Live Query support, controller, subscription types, and related APIs) so it reflects the main changeset and follows conventional commit-style prefixing. It is specific enough for a teammate scanning history to understand the PR purpose without listing files or extraneous detail.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

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

Copy link

@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: 20

🧹 Nitpick comments (5)
Parse/Abstractions/Infrastructure/Execution/IWebSocketClient.cs (2)

19-19: Consider adding connection state visibility.

The interface lacks a way to query the current connection state, which could be useful for clients to determine if they need to reconnect or if operations are valid.

Consider adding a property like:

+ /// <summary>
+ /// Gets a value indicating whether the WebSocket connection is currently open.
+ /// </summary>
+ bool IsConnected { get; }

48-48: Missing cancellation token default parameter.

The SendAsync method lacks a default value for the cancellation token parameter, which is inconsistent with the other methods.

Apply this diff for consistency:

- public Task SendAsync(string message, CancellationToken cancellationToken);
+ public Task SendAsync(string message, CancellationToken cancellationToken = default);
Parse/Platform/Queries/ParseQuery.cs (1)

915-919: Add XML documentation for the new method.

The GetLive() method lacks XML documentation, which is inconsistent with the rest of the codebase that has comprehensive documentation.

Add XML documentation:

+/// <summary>
+/// Creates a live query from this query that can be used to receive real-time updates
+/// when objects matching the query are created, updated, or deleted.
+/// </summary>
+/// <returns>A new ParseLiveQuery instance configured with this query's parameters.</returns>
 public ParseLiveQuery<T> GetLive()
Parse/Platform/LiveQueries/ParseLiveQueryDualEventArgs.cs (1)

19-22: Documentation comment is redundant.

The constructor's XML documentation largely repeats information already available in the class-level documentation and property documentation.

Consider simplifying or removing the redundant constructor documentation:

- /// <summary>
- /// Represents the event arguments provided to Live Query event handlers in the Parse platform.
- /// This class provides information about the current and original state of the Parse object
- /// involved in the Live Query operation.
- /// </summary>
+ /// <summary>
+ /// Initializes a new instance with the current and original object states.
+ /// </summary>
Parse/Platform/LiveQueries/ParseLiveQueryEventArgs.cs (1)

20-25: Fix documentation inconsistency in constructor comment.

The constructor documentation mentions "current and original state of the Parse object" but the class only contains a single Object property representing the current state. This inconsistency should be resolved.

    /// <summary>
-    /// Represents the event arguments provided to Live Query event handlers in the Parse platform.
-    /// This class provides information about the current and original state of the Parse object
-    /// involved in the Live Query operation.
+    /// Initializes a new instance of the ParseLiveQueryEventArgs class with the specified Parse object.
    /// </summary>
+    /// <param name="current">The current state of the Parse object associated with the event.</param>
    internal ParseLiveQueryEventArgs(ParseObject current) => Object = current;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9c1855 and 4b83d23.

📒 Files selected for processing (21)
  • Parse.sln (2 hunks)
  • Parse/Abstractions/Infrastructure/CustomServiceHub.cs (4 hunks)
  • Parse/Abstractions/Infrastructure/Execution/IWebSocketClient.cs (1 hunks)
  • Parse/Abstractions/Infrastructure/IMutableServiceHub.cs (3 hunks)
  • Parse/Abstractions/Infrastructure/IServiceHub.cs (3 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryController.cs (1 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQuerySubscription.cs (1 hunks)
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs (1 hunks)
  • Parse/Infrastructure/LateInitializedMutableServiceHub.cs (5 hunks)
  • Parse/Infrastructure/MutableServiceHub.cs (6 hunks)
  • Parse/Infrastructure/OrchestrationServiceHub.cs (4 hunks)
  • Parse/Infrastructure/ServiceHub.cs (4 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQuery.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryDualEventArgs.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryEventArgs.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (1 hunks)
  • Parse/Platform/ParseClient.cs (1 hunks)
  • Parse/Platform/Queries/ParseQuery.cs (1 hunks)
  • Parse/Utilities/ObjectServiceExtensions.cs (5 hunks)
🔇 Additional comments (20)
Parse/Abstractions/Infrastructure/Execution/IWebSocketClient.cs (1)

29-38: Ensure proper error handling in implementations.

While the interface design is solid, implementations should handle common WebSocket scenarios like connection failures, network timeouts, and invalid URIs. Consider documenting expected exceptions in XML comments.

The interface methods should specify what exceptions can be thrown. Consider updating the documentation to include common exceptions like InvalidOperationException for operations on closed connections or ArgumentException for invalid URIs.

Parse.sln (1)

20-21: LGTM! Solution file properly configured.

The addition of the ParseApp project follows standard Visual Studio solution conventions with proper GUID assignment and build configurations for both Debug and Release modes.

Also applies to: 36-39

Parse/Abstractions/Infrastructure/CustomServiceHub.cs (1)

8-8: LGTM! Consistent implementation following established patterns.

The additions properly extend the service hub abstraction to include live query capabilities. The new properties follow the same delegation pattern as existing properties and maintain consistency with the class design.

Also applies to: 35-35, 47-47, 67-67

Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs (1)

8-51: Well-designed event args class with comprehensive documentation.

The class follows established EventArgs patterns with immutable properties and an internal constructor for controlled instantiation. The documentation is thorough and clearly explains each property's purpose.

Parse/Infrastructure/OrchestrationServiceHub.cs (2)

9-9: Good addition of required using statement.

The using statement for live query abstractions is correctly added to support the new properties.


37-37: Well-integrated live query service properties.

The new properties (WebSocketClient, LiveQueryController, LiveQueryServerConnectionData) follow the established orchestration pattern perfectly, using the same Custom ?? Default fallback approach as existing services.

Also applies to: 49-50, 69-70

Parse/Abstractions/Infrastructure/IServiceHub.cs (2)

10-10: Correct addition of live query abstractions using statement.

The using statement is properly added to support the new live query interface types.


30-30: Well-positioned live query service properties in interface.

The new properties are logically positioned within the interface structure and follow consistent naming conventions with existing services.

Also applies to: 43-43, 50-50

Parse/Infrastructure/ServiceHub.cs (2)

10-10: Correct addition of required using statements.

Both abstraction and platform using statements for live queries are properly added to support the new implementations.

Also applies to: 24-24


42-42: Excellent implementation of live query services with proper dependency injection.

The live query services are well-integrated following the established ServiceHub pattern:

  • LiveQueryServerConnectionData as a simple property
  • WebSocketClient with lazy initialization of TextWebSocketClient
  • LiveQueryController with proper dependency injection of WebSocketClient and Decoder

The dependency hierarchy is logical and the lazy initialization pattern is maintained consistently.

Also applies to: 55-55, 62-62

Parse/Infrastructure/LateInitializedMutableServiceHub.cs (1)

75-79: LGTM! Proper lazy initialization pattern implemented.

The WebSocketClient and LiveQueryController properties follow the established lazy initialization pattern correctly, with appropriate dependency ordering (WebSocketClient is initialized before LiveQueryController which depends on it).

Also applies to: 111-115, 178-178

Parse/Abstractions/Infrastructure/IMutableServiceHub.cs (1)

22-22: Clean interface extension for live query support.

The additions follow the established pattern of the mutable service hub interface, properly extending functionality without breaking existing contracts.

Also applies to: 35-35, 42-42

Parse/Utilities/ObjectServiceExtensions.cs (1)

291-317: Well-designed extension methods for live query management.

The ConnectLiveQueryServerAsync and DisconnectLiveQueryServerAsync methods provide a clean API for managing live query connections with appropriate optional parameters and proper async patterns.

Parse/Platform/ParseClient.cs (1)

100-159: Constructor implementation follows established patterns.

The new constructor overload properly handles separate configurations for the main server and live query server, maintaining consistency with the existing constructor design.

Parse/Infrastructure/MutableServiceHub.cs (1)

39-39: Proper integration of live query services.

The MutableServiceHub correctly extends the service initialization pattern to include live query functionality, with appropriate dependency ordering and consistent null-coalescing initialization.

Also applies to: 52-52, 59-59, 73-76, 93-93, 100-100

Parse/Abstractions/Platform/LiveQueries/IParseLiveQuerySubscription.cs (1)

9-77: Well-designed interface with comprehensive event coverage.

The interface provides a clean abstraction for live query subscriptions with proper event handling for all object lifecycle events and appropriate async method signatures with cancellation token support.

Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryController.cs (1)

8-127: Excellent interface design with comprehensive functionality.

The interface provides a complete contract for live query management with proper async patterns, exception documentation, and comprehensive method coverage for all live query operations.

Parse/Platform/LiveQueries/ParseLiveQuery.cs (1)

18-96: Well-structured live query implementation with modern C# patterns.

The class design is clean with proper encapsulation, immutable composition pattern, and modern C# features like collection expressions in MergeWatchers.

Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (1)

14-51: Well-implemented subscription class with proper event handling.

The class correctly implements the interface with proper event definitions and event handler methods that safely invoke events with null checks.

Also applies to: 98-142

Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

26-26: Make ClientId setter private to ensure immutability.

The ClientId is only set once during connection establishment and should not be modified externally.

-private string ClientId { get; set; }
+private string ClientId { get; private set; }

Likely an incorrect or invalid review comment.

Copy link

@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

♻️ Duplicate comments (1)
Parse/Platform/LiveQueries/ParseLiveQuery.cs (1)

108-109: Fix cancellation token usage in SubscribeAsync.

The method ignores the implicit cancellation token and uses CancellationToken.None, which prevents callers from cancelling the subscription operation.

The method should accept a cancellation token parameter:

-public async Task<IParseLiveQuerySubscription> SubscribeAsync() =>
-    await Services.LiveQueryController.SubscribeAsync(this, CancellationToken.None);
+public async Task<IParseLiveQuerySubscription> SubscribeAsync(CancellationToken cancellationToken = default) =>
+    await Services.LiveQueryController.SubscribeAsync(this, cancellationToken);
🧹 Nitpick comments (1)
Parse/Platform/LiveQueries/ParseLiveQuery.cs (1)

63-77: Simplify KeyWatchers assignment logic.

The assignment at line 71 is unnecessary since it's conditionally overwritten at line 75. This makes the code harder to follow.

 private ParseLiveQuery(ParseLiveQuery<T> source, IEnumerable<string> watchedKeys = null)
 {
     ArgumentNullException.ThrowIfNull(source);

     Services = source.Services;
     ClassName = source.ClassName;
     Filters = source.Filters;
     KeySelections = source.KeySelections;
-    KeyWatchers = source.KeyWatchers;

-    if (watchedKeys is not null)
-    {
-        KeyWatchers = new ReadOnlyCollection<string>(MergeWatchers(watchedKeys).ToList());
-    }
+    KeyWatchers = watchedKeys is not null 
+        ? new ReadOnlyCollection<string>(MergeWatchers(watchedKeys).ToList())
+        : source.KeyWatchers;
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b83d23 and 65c73e4.

📒 Files selected for processing (2)
  • Parse/Platform/LiveQueries/ParseLiveQuery.cs (1 hunks)
  • Parse/Platform/Queries/ParseQuery.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Parse/Platform/Queries/ParseQuery.cs
🔇 Additional comments (2)
Parse/Platform/LiveQueries/ParseLiveQuery.cs (2)

79-79: LGTM!

The method correctly uses modern C# collection expression syntax and handles null values appropriately.


90-98: LGTM!

The method correctly builds query parameters with appropriate null checks to avoid adding empty collections.

Copy link

@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: 3

♻️ Duplicate comments (6)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (2)

82-82: Make buffer size configurable.

The buffer size is hardcoded to 4KB, which limits flexibility for different use cases.

Add a configurable property:

+private int BufferSize { get; set; } = 1024 * 4;

 private async Task ListenForMessages(CancellationToken cancellationToken)
 {
-    byte[] buffer = new byte[1024 * 4];
+    byte[] buffer = new byte[BufferSize];

167-171: Add WebSocket state validation.

The method should verify the WebSocket is open before sending to avoid exceptions.

 public async Task SendAsync(string message, CancellationToken cancellationToken = default)
 {
     ArgumentNullException.ThrowIfNull(webSocket);
+    if (webSocket.State != WebSocketState.Open)
+    {
+        throw new InvalidOperationException("WebSocket is not connected or not in an open state.");
+    }
     await webSocket.SendAsync(Encoding.UTF8.GetBytes(message), WebSocketMessageType.Text, true, cancellationToken);
 }
Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (2)

78-79: Fix cancellation token usage.

The method ignores the cancellationToken parameter.

 public async Task UpdateAsync<T1>(ParseLiveQuery<T1> liveQuery, CancellationToken cancellationToken = default) where T1 : ParseObject =>
-    await Services.LiveQueryController.UpdateSubscriptionAsync(liveQuery, RequestId, cancellationToken);
+    await Services.LiveQueryController.UpdateSubscriptionAsync(liveQuery, RequestId, cancellationToken);

Also consider simplifying the generic parameter since the class is already generic:

-public async Task UpdateAsync<T1>(ParseLiveQuery<T1> liveQuery, CancellationToken cancellationToken = default) where T1 : ParseObject =>
+public async Task UpdateAsync(ParseLiveQuery<T> liveQuery, CancellationToken cancellationToken = default) =>

88-89: Fix cancellation token usage in CancelAsync.

The method ignores the cancellationToken parameter.

 public async Task CancelAsync(CancellationToken cancellationToken = default) =>
-    await Services.LiveQueryController.UnsubscribeAsync(RequestId, cancellationToken);
+    await Services.LiveQueryController.UnsubscribeAsync(RequestId, cancellationToken);
Parse/Platform/LiveQueries/ParseLiveQuery.cs (1)

39-42: Add null validation for critical constructor parameters.

The constructor should validate serviceHub and className parameters.

 internal ParseLiveQuery(IServiceHub serviceHub, string className, IDictionary<string, object> filters, IEnumerable<string> selectedKeys = null, IEnumerable<string> watchedKeys = null)
 {
+    ArgumentNullException.ThrowIfNull(serviceHub);
+    ArgumentException.ThrowIfNullOrWhiteSpace(className);
     ArgumentNullException.ThrowIfNull(filters);
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

262-262: Add safe casting to prevent InvalidCastException.

Direct casting to bool could throw if the value is null or not a boolean.

-            (bool) message["reconnect"]);
+            message.TryGetValue("reconnect", out object reconnectValue) && reconnectValue is bool reconnect ? reconnect : false);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 340f6fb and 7e66bb6.

📒 Files selected for processing (7)
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQuery.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryDualEventArgs.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryEventArgs.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (1 hunks)
  • Parse/Platform/ParseClient.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • Parse/Platform/LiveQueries/ParseLiveQueryDualEventArgs.cs
  • Parse/Platform/LiveQueries/ParseLiveQueryEventArgs.cs
  • Parse/Platform/ParseClient.cs
🧰 Additional context used
🧬 Code Graph Analysis (1)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (9)
  • Task (297-305)
  • Task (307-308)
  • Task (310-323)
  • Task (350-389)
  • Task (391-416)
  • Task (440-458)
  • Task (479-488)
  • Task (505-514)
  • Task (525-533)

Copy link

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

♻️ Duplicate comments (2)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (2)

183-187: Add WebSocket state validation to match documented exceptions.

The method correctly handles null checks but doesn't validate the WebSocket state before sending, which could result in exceptions that don't match the documented behavior.

public async Task SendAsync(string message, CancellationToken cancellationToken = default)
{
    ArgumentNullException.ThrowIfNull(webSocket);
+    
+    if (webSocket.State != WebSocketState.Open)
+    {
+        throw new InvalidOperationException($"WebSocket is not in Open state. Current state: {webSocket.State}");
+    }
+    
    await webSocket.SendAsync(Encoding.UTF8.GetBytes(message), WebSocketMessageType.Text, true, cancellationToken);
}

55-67: Address remaining race condition in connection logic.

While the WebSocket initialization is properly synchronized, the state check and connection attempt are still not atomic. Multiple threads could pass the state check simultaneously and attempt to connect concurrently.

Consider this approach to ensure atomic connection logic:

public async Task OpenAsync(string serverUri, CancellationToken cancellationToken = default)
{
+    ClientWebSocket webSocketToConnect = null;
+    
    lock (connectionLock)
    {
        webSocket ??= new ClientWebSocket();
+        
+        if (webSocket.State != WebSocketState.Open && webSocket.State != WebSocketState.Connecting)
+        {
+            webSocketToConnect = webSocket;
+        }
    }

-    if (webSocket.State != WebSocketState.Open && webSocket.State != WebSocketState.Connecting)
+    if (webSocketToConnect != null)
    {
-        await webSocket.ConnectAsync(new Uri(serverUri), cancellationToken);
+        await webSocketToConnect.ConnectAsync(new Uri(serverUri), cancellationToken);
        StartListening(cancellationToken);
    }
}
🧹 Nitpick comments (2)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (2)

77-78: Remove unnecessary null-forgiving operator.

The null-forgiving operator (!) is unnecessary and potentially misleading since the null-conditional operator (?.) can return null.

-public async Task CloseAsync(CancellationToken cancellationToken = default) =>
-    await webSocket?.CloseAsync(WebSocketCloseStatus.NormalClosure, String.Empty, cancellationToken)!;
+public async Task CloseAsync(CancellationToken cancellationToken = default)
+{
+    if (webSocket != null)
+    {
+        await webSocket.CloseAsync(WebSocketCloseStatus.NormalClosure, String.Empty, cancellationToken);
+    }
+}

80-137: Excellent exception handling and partial message support!

The method properly handles various exception types and correctly manages partial WebSocket messages. However, consider making the buffer size configurable for different use cases.

Consider adding a configurable buffer size:

+private const int DefaultBufferSize = 1024 * 4;
+private readonly int bufferSize;

+// Add to constructor or make it a property
+public int BufferSize { get; init; } = DefaultBufferSize;

private async Task ListenForMessages(CancellationToken cancellationToken)
{
-    byte[] buffer = new byte[1024 * 4];
+    byte[] buffer = new byte[BufferSize];
    // ... rest of the method
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e66bb6 and 84f7060.

📒 Files selected for processing (2)
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQuery.cs (1 hunks)
🔇 Additional comments (11)
Parse/Platform/LiveQueries/ParseLiveQuery.cs (9)

1-9: LGTM - Clean imports and namespace declaration.

The using statements are appropriate and well-organized, importing necessary System libraries and Parse-specific abstractions for live query functionality.


12-18: LGTM - Well-documented generic class declaration.

The XML documentation clearly explains the purpose and constraints of the ParseLiveQuery class. The generic constraint where T : ParseObject is appropriate for the Parse SDK context.


20-37: LGTM - Well-designed immutable properties.

The properties follow good immutability practices with get-only accessors and appropriate access levels. Using ReadOnlyCollection<string> for collections ensures the class remains immutable after construction.


39-58: LGTM - Robust constructor with proper validation.

The constructor properly validates all critical parameters and handles optional collections correctly. The use of ToList() prevents potential deferred execution issues with the input enumerables.


65-79: LGTM - Well-designed composition constructor.

The private constructor for query composition properly copies all properties from the source and allows extending watched keys. Good validation and immutability design.


83-90: LGTM - Clean fluent API method.

The Watch method provides a clean fluent interface for adding watched keys, maintaining immutability by returning a new instance.


92-100: LGTM - Correct parameter building logic.

The BuildParameters method correctly constructs the dictionary needed for live query operations, properly handling optional collections and converting them to arrays as expected by the Parse Server API.


102-111: LGTM - Proper async subscription with cancellation support.

The SubscribeAsync method correctly accepts a cancellation token parameter and passes it through to the controller. This addresses the previous review concern about cancellation token usage.


81-81: Verify C# 12 collection expression compatibility.

The code uses C# 12 collection expression syntax [..] which may not be compatible with older target frameworks. Ensure your project targets .NET 8+ or C# 12+.

#!/bin/bash
# Description: Check the target framework and C# language version in project files
# Expected: Confirm .NET 8+ or C# 12+ support for collection expressions

echo "Checking .csproj files for target framework and language version..."
fd -e csproj --exec cat {} \; | grep -E "(TargetFramework|LangVersion)"

echo -e "\nChecking Directory.Build.props for global settings..."
find . -name "Directory.Build.props" -exec cat {} \;
Parse/Infrastructure/Execution/TextWebSocketClient.cs (2)

11-44: LGTM! Good class structure with proper thread safety.

The class is well-structured with appropriate encapsulation, proper field declarations, and includes a connection lock for thread safety. The use of events for message notification follows good event-driven design patterns.


143-169: Well-implemented listener management with proper fault monitoring.

The method correctly prevents multiple concurrent listeners and properly monitors the listening task for faults without causing application crashes. The fault handling approach is much improved.

Copy link

@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: 3

🧹 Nitpick comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

564-564: Consider potential deadlock in Dispose method.

Using GetAwaiter().GetResult() in a disposal context could potentially cause deadlocks in certain synchronization contexts, though this is a common pattern for dispose methods.

Consider using ConfigureAwait(false) to reduce deadlock risk:

- CloseAsync().GetAwaiter().GetResult();
+ CloseAsync().ConfigureAwait(false).GetAwaiter().GetResult();
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84f7060 and 834ff89.

📒 Files selected for processing (15)
  • Parse/Abstractions/Infrastructure/CustomServiceHub.cs (4 hunks)
  • Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs (1 hunks)
  • Parse/Abstractions/Infrastructure/IMutableServiceHub.cs (3 hunks)
  • Parse/Abstractions/Infrastructure/IServiceHub.cs (3 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryController.cs (1 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQuerySubscription.cs (1 hunks)
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs (1 hunks)
  • Parse/Infrastructure/LateInitializedMutableServiceHub.cs (3 hunks)
  • Parse/Infrastructure/LiveQueryServerConnectionData.cs (1 hunks)
  • Parse/Infrastructure/MutableServiceHub.cs (6 hunks)
  • Parse/Infrastructure/OrchestrationServiceHub.cs (4 hunks)
  • Parse/Infrastructure/ServiceHub.cs (4 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
  • Parse/Platform/ParseClient.cs (1 hunks)
  • Parse/Utilities/ObjectServiceExtensions.cs (5 hunks)
✅ Files skipped from review due to trivial changes (1)
  • Parse/Infrastructure/LiveQueryServerConnectionData.cs
🚧 Files skipped from review as they are similar to previous changes (12)
  • Parse/Infrastructure/OrchestrationServiceHub.cs
  • Parse/Abstractions/Infrastructure/CustomServiceHub.cs
  • Parse/Utilities/ObjectServiceExtensions.cs
  • Parse/Abstractions/Infrastructure/IMutableServiceHub.cs
  • Parse/Abstractions/Infrastructure/IServiceHub.cs
  • Parse/Infrastructure/ServiceHub.cs
  • Parse/Platform/ParseClient.cs
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryController.cs
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQuerySubscription.cs
  • Parse/Infrastructure/LateInitializedMutableServiceHub.cs
  • Parse/Infrastructure/MutableServiceHub.cs
🔇 Additional comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

20-124: Excellent implementation of previous feedback.

The class now properly implements thread safety with volatile state field, ConcurrentDictionary collections, Interlocked.Increment for request IDs, and a proper IDisposable pattern. The async patterns have been correctly implemented with TaskCompletionSource instead of blocking calls.

Copy link

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

♻️ Duplicate comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

174-174: Use safe type conversion to prevent runtime exceptions.

The direct use of Convert.ToInt32() and Convert.ToBoolean() without null checking can throw exceptions if the message values are null or invalid.

Apply safe conversion patterns as previously suggested:

- int requestId = Convert.ToInt32(message["requestId"]);
+ if (!message.TryGetValue("requestId", out object requestIdObj) || !int.TryParse(requestIdObj?.ToString(), out int requestId))
+     return;

Also applies to lines 189, 209, 229, 249, 262, 264, 273, 285.

🧹 Nitpick comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

169-257: Extract repeated client ID validation into a helper method.

Multiple event processing methods duplicate the same client ID validation pattern, violating DRY principles.

Consider extracting this into a helper method:

private bool ValidateClientMessage(IDictionary<string, object> message, out int requestId)
{
    requestId = 0;
    
    if (!(message.TryGetValue("clientId", out object clientIdObj) && 
          clientIdObj is string clientId && clientId == ClientId))
        return false;
        
    return message.TryGetValue("requestId", out object requestIdObj) && 
           int.TryParse(requestIdObj?.ToString(), out requestId);
}

Then update each event processing method:

- string clientId = message["clientId"] as string;
- if (clientId != ClientId)
-     return;
- int requestId = Convert.ToInt32(message["requestId"]);
+ if (!ValidateClientMessage(message, out int requestId))
+     return;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 834ff89 and bd36b7d.

📒 Files selected for processing (2)
  • Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs
🧰 Additional context used
🧬 Code Graph Analysis (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (6)
Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs (2)
  • ParseLiveQueryErrorEventArgs (8-51)
  • ParseLiveQueryErrorEventArgs (45-50)
Parse/Platform/LiveQueries/ParseLiveQuery.cs (6)
  • IDictionary (92-100)
  • Task (110-111)
  • ParseLiveQuery (18-112)
  • ParseLiveQuery (39-58)
  • ParseLiveQuery (65-79)
  • ParseLiveQuery (90-90)
Parse/Platform/ParseClient.cs (7)
  • IDictionary (185-196)
  • IDictionary (198-201)
  • ParseClient (19-220)
  • ParseClient (58-58)
  • ParseClient (66-98)
  • ParseClient (107-159)
  • ParseClient (164-164)
Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (9)
  • OnDelete (141-142)
  • OnLeave (130-133)
  • OnUpdate (118-121)
  • OnEnter (107-110)
  • OnCreate (98-99)
  • Task (78-79)
  • Task (88-89)
  • ParseLiveQuerySubscription (14-143)
  • ParseLiveQuerySubscription (58-63)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryController.cs (5)
  • Task (41-41)
  • Task (64-64)
  • Task (84-84)
  • Task (101-101)
  • Task (114-114)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (4)
  • Task (57-74)
  • Task (84-90)
  • Task (92-149)
  • Task (195-203)
🔇 Additional comments (3)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (3)

20-124: Class structure and constructor implementation look excellent.

The implementation properly addresses thread safety concerns with ConcurrentDictionary collections, volatile state field, and uses Interlocked operations for request ID generation. The constructor is well-designed with clear parameter requirements.


352-534: Excellent async implementation with proper resource management.

The public API methods demonstrate best practices with TaskCompletionSource usage, proper cancellation token handling, and comprehensive resource cleanup. The refactored SendAndWaitForSignalAsync helper eliminates code duplication effectively.


544-567: Proper IDisposable pattern implementation.

The dispose pattern correctly implements standard .NET practices with appropriate resource cleanup and proper handling of the async CloseAsync call within the synchronous dispose context.

@mtrezza
Copy link
Member

mtrezza commented Jun 30, 2025

Is this work in progress?

Copy link

@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: 2

🧹 Nitpick comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

575-584: Consider potential deadlock risk in Dispose method.

The current implementation calls CloseAsync().ConfigureAwait(false).GetAwaiter().GetResult() which could potentially cause deadlocks in certain synchronization contexts, particularly in ASP.NET applications.

Consider one of these alternatives:

Option 1: Use async disposal pattern (if targeting .NET Standard 2.1+):

-public class ParseLiveQueryController : IParseLiveQueryController, IDisposable
+public class ParseLiveQueryController : IParseLiveQueryController, IDisposable, IAsyncDisposable

+public async ValueTask DisposeAsync()
+{
+    await CloseAsync();
+    GC.SuppressFinalize(this);
+}

 private void Dispose(bool disposing)
 {
     if (disposed)
         return;
     if (disposing)
     {
-        CloseAsync().ConfigureAwait(false).GetAwaiter().GetResult();
+        // For sync disposal, best effort cleanup without waiting
+        _ = Task.Run(async () => await CloseAsync());
     }
     disposed = true;
 }

Option 2: Keep current approach but document the risk:
Add XML documentation warning about potential deadlock when calling Dispose from UI thread or ASP.NET request context.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd36b7d and e9c6bcc.

📒 Files selected for processing (1)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
🔇 Additional comments (5)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (5)

20-104: LGTM! Thread safety improvements are well implemented.

The class properly implements IDisposable and uses thread-safe collections (ConcurrentDictionary) and volatile state field. The previous thread safety concerns have been well addressed.


358-408: LGTM! Async pattern implementation is excellent.

The ConnectAsync method properly uses TaskCompletionSource and async/await patterns, eliminating the previous blocking issues. The timeout handling and state management are well implemented.


410-435: LGTM! Excellent refactoring that eliminates code duplication.

The SendAndWaitForSignalAsync helper method is well-designed and eliminates the code duplication that existed in the subscription methods. The async pattern usage and resource cleanup are properly implemented.


437-533: LGTM! Subscription methods are well implemented.

The subscription methods properly use thread-safe operations (Interlocked.Increment) and the new helper method to eliminate code duplication. Error handling for closed connections is appropriate.


535-552: LGTM! Resource cleanup is properly implemented.

The CloseAsync method correctly unsubscribes from event handlers and clears all collections. The resource cleanup addresses the previous concerns about memory leaks.

Copy link

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

♻️ Duplicate comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

318-327: Add null check for ConnectionSignal to prevent potential race condition.

The ProcessConnectionMessage method calls ConnectionSignal.TrySetResult() without checking if ConnectionSignal is null. There's a potential race condition where this could be called after ConnectAsync has set ConnectionSignal to null in its finally block.

 void ProcessConnectionMessage(IDictionary<string, object> message)
 {
     if (!(message.TryGetValue("clientId", out object clientIdObj) &&
         clientIdObj is string clientId))
         return;

     ClientId = clientId;
     _state = ParseLiveQueryState.Connected;
-    ConnectionSignal.TrySetResult();
+    ConnectionSignal?.TrySetResult();
 }
🧹 Nitpick comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

587-596: Consider using fire-and-forget pattern to avoid potential deadlocks.

Using ConfigureAwait(false).GetAwaiter().GetResult() in the Dispose method could potentially cause deadlocks in certain synchronization contexts.

 private void Dispose(bool disposing)
 {
     if (disposed)
         return;
     if (disposing)
     {
-        CloseAsync().ConfigureAwait(false).GetAwaiter().GetResult();
+        try
+        {
+            CloseAsync().ConfigureAwait(false).GetAwaiter().GetResult();
+        }
+        catch
+        {
+            // Suppress exceptions during disposal
+        }
     }
     disposed = true;
 }

Alternatively, consider making the cleanup fire-and-forget:

-        CloseAsync().ConfigureAwait(false).GetAwaiter().GetResult();
+        _ = Task.Run(async () => await CloseAsync().ConfigureAwait(false));
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9c6bcc and 8938a4d.

📒 Files selected for processing (1)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
🔇 Additional comments (8)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (8)

118-124: LGTM! Constructor properly initializes all dependencies.

The constructor correctly initializes all required fields and sets the initial state appropriately.


422-447: LGTM! Excellent async helper method implementation.

The SendAndWaitForSignalAsync method provides a clean, reusable pattern for async operations with proper timeout and cancellation handling.


471-489: LGTM! Well-implemented subscription method with proper validation.

The SubscribeAsync method correctly validates the connection state, uses thread-safe ID generation, and properly manages the subscription lifecycle.


279-294: LGTM! Robust error message processing with safe parsing.

The ProcessErrorMessage method properly validates all required fields and uses safe parsing patterns to prevent exceptions.


169-192: LGTM! Excellent helper methods for safe message processing.

The ValidateClientMessage and GetDictEntry methods provide consistent, safe patterns for validating and extracting data from WebSocket messages.


357-368: LGTM! Safe WebSocket message handling with proper validation.

The WebSocketClientOnMessageReceived method properly validates the parsed JSON before processing, preventing null reference exceptions.


98-104: LGTM! Thread-safe state and collection management.

The implementation properly uses a volatile backing field for state and ConcurrentDictionary for all shared collections, ensuring thread safety.


16-21: Excellent overall implementation of the live query controller.

This implementation demonstrates good software engineering practices with:

  • Proper thread safety using concurrent collections and volatile fields
  • Clean async/await patterns without blocking calls
  • Comprehensive error handling and validation
  • Proper resource management with IDisposable implementation
  • Well-structured code with good separation of concerns

The refactoring based on previous review feedback has resulted in a robust, maintainable implementation.

@theSlyest theSlyest dismissed stale reviews from mtrezza and coderabbitai[bot] via 871f015 August 29, 2025 16:39
Copy link

@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: 4

🧹 Nitpick comments (5)
Parse.Tests/ObjectCoderTests.cs (1)

40-40: Remove debug output from tests.
Debug.WriteLine adds noise; rely on assertions.

-        Debug.WriteLine(resultACL is null);
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (4)

35-46: Docs: “in milliseconds” mismatches TimeSpan

TimeSpan-based Timeout isn’t “in milliseconds.” Tweak wording to avoid confusion.

-    /// Gets or sets the timeout duration, in milliseconds, used by the ParseLiveQueryController
+    /// Gets or sets the timeout duration used by the ParseLiveQueryController

168-169: Use the already-parsed op variable in default case

Avoid reindexing the dictionary.

-                Debug.WriteLine($"Unknown operation: {message["op"]}");
+                Debug.WriteLine($"Unknown operation: {op}");

460-462: TCS continuations should run asynchronously to avoid reentrancy/deadlocks

Use RunContinuationsAsynchronously for safer signaling from socket callbacks.

-        TaskCompletionSource tcs = new TaskCompletionSource();
+        TaskCompletionSource tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);

642-657: Avoid blocking sync dispose on async CloseAsync

Blocking with GetAwaiter().GetResult() can deadlock. Prefer IAsyncDisposable or fire-and-forget CloseAsync with logging.

-            try
-            {
-                CloseAsync().GetAwaiter().GetResult();
-            }
-            catch (Exception ex)
-            {
-                Debug.WriteLine($"Error during disposal: {ex}");
-            }
+            _ = Task.Run(async () =>
+            {
+                try { await CloseAsync().ConfigureAwait(false); }
+                catch (Exception ex) { Debug.WriteLine($"Error during disposal: {ex}"); }
+            });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e3b5df9 and 871f015.

📒 Files selected for processing (16)
  • Parse.Tests/DecoderTests.cs (1 hunks)
  • Parse.Tests/ObjectCoderTests.cs (1 hunks)
  • Parse.Tests/SessionTests.cs (1 hunks)
  • Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs (1 hunks)
  • Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs (1 hunks)
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs (1 hunks)
  • Parse/Infrastructure/LateInitializedMutableServiceHub.cs (3 hunks)
  • Parse/Infrastructure/LiveQueryServerConnectionData.cs (1 hunks)
  • Parse/Infrastructure/MutableServiceHub.cs (6 hunks)
  • Parse/Infrastructure/ServiceHub.cs (4 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQuery.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryEventArgs.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (1 hunks)
  • Parse/Platform/Queries/ParseQuery.cs (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • Parse.Tests/SessionTests.cs
🚧 Files skipped from review as they are similar to previous changes (10)
  • Parse/Platform/LiveQueries/ParseLiveQueryEventArgs.cs
  • Parse/Platform/Queries/ParseQuery.cs
  • Parse/Infrastructure/ServiceHub.cs
  • Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs
  • Parse/Infrastructure/MutableServiceHub.cs
  • Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs
  • Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs
  • Parse/Infrastructure/LiveQueryServerConnectionData.cs
  • Parse/Platform/LiveQueries/ParseLiveQuery.cs
  • Parse/Infrastructure/LateInitializedMutableServiceHub.cs
🧰 Additional context used
🧬 Code graph analysis (3)
Parse.Tests/DecoderTests.cs (2)
Parse.Tests/SessionTests.cs (1)
  • TestInitialize (31-35)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (7)
Parse/Infrastructure/Utilities/JsonUtilities.cs (2)
  • Parse (332-372)
  • JsonUtilities (16-508)
Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs (2)
  • ParseLiveQueryErrorEventArgs (8-65)
  • ParseLiveQueryErrorEventArgs (59-64)
Parse/Platform/LiveQueries/ParseLiveQuery.cs (6)
  • IDictionary (96-104)
  • Task (114-115)
  • ParseLiveQuery (18-116)
  • ParseLiveQuery (39-58)
  • ParseLiveQuery (65-79)
  • ParseLiveQuery (90-94)
Parse/Platform/ParseClient.cs (7)
  • IDictionary (185-196)
  • IDictionary (198-201)
  • ParseClient (19-220)
  • ParseClient (58-58)
  • ParseClient (66-98)
  • ParseClient (107-159)
  • ParseClient (164-164)
Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (9)
  • OnDelete (143-144)
  • OnLeave (132-135)
  • OnUpdate (120-123)
  • OnEnter (109-112)
  • OnCreate (100-101)
  • Task (80-81)
  • Task (90-91)
  • ParseLiveQuerySubscription (14-145)
  • ParseLiveQuerySubscription (58-65)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (4)
  • Task (60-77)
  • Task (87-90)
  • Task (92-150)
  • Task (196-204)
Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs (1)
  • MessageReceivedEventArgs (8-14)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (2)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (11)
  • Task (333-341)
  • Task (343-344)
  • Task (346-359)
  • Task (393-437)
  • Task (455-480)
  • Task (504-523)
  • Task (544-554)
  • Task (571-581)
  • Task (592-603)
  • Dispose (613-617)
  • Dispose (642-659)
Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs (1)
  • MessageReceivedEventArgs (8-14)
🔇 Additional comments (2)
Parse.Tests/ObjectCoderTests.cs (2)

9-10: LGTM: Move to file-scoped Parse.Tests namespace is consistent and non-functional.
Keeps tests grouped under the Parse.Tests namespace like the rest of the suite.


9-10: C# 10+ support confirmed; no update required
Parse.Tests targets net8.0;net9.0 with LangVersion latest, so file-scoped namespaces are supported.

Copy link

@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: 2

♻️ Duplicate comments (1)
Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs (1)

13-21: Prefer moving defaults out of the interface to avoid const-propagation and keep the contract clean

Constants in interfaces become compile-time inlined in downstream assemblies; changing them later won’t take effect without a recompile. Consider a dedicated static defaults holder.

  • Add a static class:
namespace Parse.Abstractions.Infrastructure
{
    public static class LiveQueryDefaults
    {
        public static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(5);
        public const int DefaultBufferSize = 4096; // 4KB
    }
}
  • Then remove the constant from the interface and reference LiveQueryDefaults.DefaultBufferSize where defaults are applied (e.g., in the implementation’s initializer).
🧹 Nitpick comments (1)
Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs (1)

7-11: Doc nit: Don’t hardcode “in seconds” for a TimeSpan

The type already conveys units; “in seconds” can mislead (consumers may pass any TimeSpan). Prefer neutral wording.

-    /// The timeout duration, in seconds, used for various operations, such as
+    /// The timeout duration used for various operations, such as
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 871f015 and 5444d5d.

📒 Files selected for processing (4)
  • Parse.Tests/EncoderTests.cs (1 hunks)
  • Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs (1 hunks)
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs (1 hunks)
  • Parse/Infrastructure/LiveQueryServerConnectionData.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • Parse.Tests/EncoderTests.cs
  • Parse/Infrastructure/LiveQueryServerConnectionData.cs
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs
🔇 Additional comments (1)
Parse/Abstractions/Infrastructure/ILiveQueryServerConnectionData.cs (1)

13-16: Docs are now consistent (4KB) — good fix

The earlier “4MB” mismatch is resolved and aligns with the 4096-byte value.

@YBTopaz8
Copy link
Member

YBTopaz8 commented Sep 1, 2025

@mtrezza this looks good. If merged, I'll pull here and rework on my intended changes as a natural progression 🙂
You may merge this, please.

Thanks a lot @theSlyest . This solution works plainly which is superb.
Now Parse Devs for .NET will have 2 options for Live Queries.
I think we are the first with this luxury!

@theSlyest
Copy link
Author

theSlyest commented Sep 1, 2025

LGTM, should this be merged now?

@mtrezza It's ok now, I wanted to correct some potential issues first. You can run the tests and then merge.

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 1, 2025
@theSlyest
Copy link
Author

@mtrezza Anytime, my dear.

@mtrezza
Copy link
Member

mtrezza commented Sep 5, 2025

Started the CI; @YBTopaz8 what do you mean by "Now Parse Devs for .NET will have 2 options for Live Queries."?

@YBTopaz8
Copy link
Member

YBTopaz8 commented Sep 5, 2025

Should this PR be merged, then the official SDK will have LQ.

There is another Parse LQ SDK that has LiveQueries but uses ReactiveX instead.

Done by myself. So it's unofficial

@theSlyest
Copy link
Author

theSlyest commented Sep 8, 2025

@mtrezza please retry, or update the check to use dotnet build (official .NET SDK) instead of msbuild (mono).

@mtrezza
Copy link
Member

mtrezza commented Sep 8, 2025

Retry started

@theSlyest
Copy link
Author

@mtrezza I have added and updated some tests, please try again.

@mtrezza
Copy link
Member

mtrezza commented Sep 9, 2025

Running again...

Copy link

@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: 13

♻️ Duplicate comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

616-633: Avoid blocking in Dispose; prefer non-blocking cleanup.

Blocking on GetResult() can deadlock. Use fire-and-forget or rely on DisposeAsync.

-            try
-            {
-                CloseAsync().GetAwaiter().GetResult();
-            }
-            catch (Exception ex)
-            {
-                Debug.WriteLine($"Error during disposal: {ex}");
-            }
+            _ = Task.Run(async () =>
+            {
+                try { await CloseAsync(); }
+                catch (Exception ex) { Debug.WriteLine($"Error during disposal: {ex}"); }
+            });
🧹 Nitpick comments (12)
Parse.Tests/LiveQuerySubscriptionTests.cs (1)

17-23: Redundant Publicize calls

You call Client.Publicize() in both the constructor and [TestInitialize]. One is sufficient; keep only in [TestInitialize] for clarity.

Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1)

39-46: Optional: prefer a dedicated error type over tuple in public API

A small record/class (e.g., LiveQueryError) improves discoverability and forward-compatibility.

Parse/Infrastructure/ServiceHub.cs (1)

74-77: Consider handling null LiveQueryServerConnectionData more gracefully

The null checks for LiveQueryServerConnectionData are only applied to WebSocketClient and LiveQueryController, but not to LiveQueryMessageParser and LiveQueryMessageBuilder. These components are always initialized even when Live Query is not configured, which may lead to unnecessary resource allocation.

Consider lazy initialization for all Live Query components only when needed:

-public IParseLiveQueryMessageParser LiveQueryMessageParser => LateInitializer.GetValue(() => new ParseLiveQueryMessageParser(Decoder));
-public IParseLiveQueryMessageBuilder LiveQueryMessageBuilder => LateInitializer.GetValue(() => new ParseLiveQueryMessageBuilder());
+public IParseLiveQueryMessageParser LiveQueryMessageParser => LateInitializer.GetValue(() => LiveQueryServerConnectionData is null ? null : new ParseLiveQueryMessageParser(Decoder));
+public IParseLiveQueryMessageBuilder LiveQueryMessageBuilder => LateInitializer.GetValue(() => LiveQueryServerConnectionData is null ? null : new ParseLiveQueryMessageBuilder());
Parse.Tests/LiveQueryMessageBuilderTests.cs (2)

36-45: Consider verifying session token handling in tests

The tests verify the message structure but don't validate session token inclusion when available. This is an important security feature that should be tested.

Consider adding test coverage for session token scenarios to ensure proper authentication handling.


72-73: Collection initialization can be simplified

The array initializations can use the more concise collection expression syntax available in newer C# versions.

-            ["foo"],
-            ["foo"]);
+            ["foo"],
+            ["foo"]);

Note: This syntax is already correct if using C# 12 or later collection expressions. Disregard if the project targets an appropriate version.

Also applies to: 122-124

Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (3)

41-50: Fix placeholder in exception message (string interpolation bug).

The placeholder %{key} won’t be interpolated. Use C# interpolation.

-        if (!(message.TryGetValue(key, out object obj) && obj is IDictionary<string, object> dict))
-            throw new ArgumentException(@"Message does not contain a valid %{key}.", nameof(message));
+        if (!(message.TryGetValue(key, out object obj) && obj is IDictionary<string, object> dict))
+            throw new ArgumentException($"Message does not contain a valid '{key}'.", nameof(message));

57-61: Remove unreachable null checks after GetDictionary.

GetDictionary throws on invalid input; subsequent is null checks are redundant.

-        if (current is null)
-            throw new ArgumentException("Message does not contain a valid object state.", nameof(message));
-        if (original is null)
-            throw new ArgumentException("Message does not contain a valid original object state.", nameof(message));

Also applies to: 69-73


61-62: Consider decoupling from global ParseClient for testability.

Passing ParseClient.Instance.Services here couples the parser to global state. Prefer injecting IServiceHub into the parser (or the decoder) via constructor.

Also applies to: 73-74

Parse/Infrastructure/LateInitializedMutableServiceHub.cs (1)

165-169: WebSocketClient may be null; consider nullable annotation or throwing.

Getter can return null; consider IWebSocketClient? or throw with guidance if connection data is missing.

Parse/Platform/LiveQueries/ParseLiveQueryController.cs (3)

175-176: Use local op variable in log to avoid redundant dictionary lookup.

Minor cleanup; avoids indexer re-access.

-                Debug.WriteLine($"Unknown operation: {message["op"]}");
+                Debug.WriteLine($"Unknown operation: {op}");

325-333: Remove unused AppendSessionToken method.

This private helper isn’t used; delete to reduce surface area.

-    private async Task<IDictionary<string, object>> AppendSessionToken(IDictionary<string, object> message)
-    {
-        string sessionToken = await ParseClient.Instance.Services.GetCurrentSessionToken();
-        return sessionToken is null
-            ? message
-            : message.Concat(new Dictionary<string, object> {
-                { "sessionToken", sessionToken }
-            }).ToDictionary();
-    }

279-294: Leverage the parser for error messages for consistency.

Use MessageParser.GetError instead of manual parsing.

-    void ProcessErrorMessage(IDictionary<string, object> message)
-    {
-        if (!(message.TryGetValue("code", out object codeObj) &&
-            Int32.TryParse(codeObj?.ToString(), out int code)))
-            return;
-
-        if (!(message.TryGetValue("error", out object errorObj) &&
-              errorObj is string error))
-            return;
-
-        if (!(message.TryGetValue("reconnect", out object reconnectObj) &&
-            Boolean.TryParse(reconnectObj?.ToString(), out bool reconnect)))
-            return;
-
-        Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(code, error, reconnect));
-    }
+    void ProcessErrorMessage(IDictionary<string, object> message)
+    {
+        var (code, error, reconnect) = MessageParser.GetError(message);
+        Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(code, error, reconnect));
+    }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 96b20f6 and a13c7ab.

📒 Files selected for processing (21)
  • Parse.Tests/LiveQueryDualEventArgsTests.cs (1 hunks)
  • Parse.Tests/LiveQueryErrorEventArgsTests.cs (1 hunks)
  • Parse.Tests/LiveQueryEventArgsTests.cs (1 hunks)
  • Parse.Tests/LiveQueryMessageBuilderTests.cs (1 hunks)
  • Parse.Tests/LiveQueryMessageParserTests.cs (1 hunks)
  • Parse.Tests/LiveQuerySubscriptionTests.cs (1 hunks)
  • Parse.Tests/LiveQueryTests.cs (1 hunks)
  • Parse.Tests/MessageReceivedEventArgsTests.cs (1 hunks)
  • Parse.Tests/Parse.Tests.csproj (1 hunks)
  • Parse/Abstractions/Infrastructure/CustomServiceHub.cs (4 hunks)
  • Parse/Abstractions/Infrastructure/IMutableServiceHub.cs (3 hunks)
  • Parse/Abstractions/Infrastructure/IServiceHub.cs (3 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (1 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1 hunks)
  • Parse/Infrastructure/LateInitializedMutableServiceHub.cs (3 hunks)
  • Parse/Infrastructure/MutableServiceHub.cs (6 hunks)
  • Parse/Infrastructure/OrchestrationServiceHub.cs (4 hunks)
  • Parse/Infrastructure/ServiceHub.cs (4 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • Parse/Abstractions/Infrastructure/CustomServiceHub.cs
  • Parse.Tests/LiveQueryEventArgsTests.cs
  • Parse.Tests/LiveQueryErrorEventArgsTests.cs
🧰 Additional context used
🧬 Code graph analysis (17)
Parse.Tests/LiveQuerySubscriptionTests.cs (3)
Parse/Infrastructure/LiveQueryServerConnectionData.cs (1)
  • LiveQueryServerConnectionData (12-12)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Infrastructure/ServiceHub.cs (1)
  • ServiceHub (37-83)
Parse.Tests/MessageReceivedEventArgsTests.cs (3)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Infrastructure/ServiceHub.cs (1)
  • ServiceHub (37-83)
Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs (1)
  • MessageReceivedEventArgs (8-14)
Parse/Infrastructure/OrchestrationServiceHub.cs (1)
Parse/Infrastructure/LiveQueryServerConnectionData.cs (1)
  • LiveQueryServerConnectionData (12-12)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (2)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (5)
  • GetClientId (16-16)
  • GetRequestId (23-23)
  • IObjectState (30-30)
  • IObjectState (37-37)
  • code (46-46)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (58-68)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (3)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (6)
  • GetClientId (19-28)
  • IDictionary (41-50)
  • GetRequestId (30-39)
  • IObjectState (52-62)
  • IObjectState (64-74)
  • code (76-91)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (34-34)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (58-68)
Parse.Tests/LiveQueryDualEventArgsTests.cs (2)
Parse.Tests/LiveQueryEventArgsTests.cs (3)
  • TestClass (11-46)
  • TestMethod (21-41)
  • TestMethod (43-45)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (2)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (4)
  • Task (14-14)
  • Task (21-21)
  • Task (28-28)
  • IDictionary (34-34)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (9)
  • Task (325-333)
  • Task (335-336)
  • Task (338-351)
  • Task (385-425)
  • Task (443-468)
  • Task (492-506)
  • Task (527-532)
  • Task (549-555)
  • Task (566-577)
Parse.Tests/LiveQueryMessageParserTests.cs (3)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (8)
  • ParseLiveQueryMessageParser (10-92)
  • ParseLiveQueryMessageParser (14-17)
  • IDictionary (41-50)
  • GetClientId (19-28)
  • GetRequestId (30-39)
  • IObjectState (52-62)
  • IObjectState (64-74)
  • code (76-91)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (5)
  • GetClientId (16-16)
  • GetRequestId (23-23)
  • IObjectState (30-30)
  • IObjectState (37-37)
  • code (46-46)
Parse/Abstractions/Infrastructure/IMutableServiceHub.cs (1)
Parse/Infrastructure/LiveQueryServerConnectionData.cs (1)
  • LiveQueryServerConnectionData (12-12)
Parse.Tests/LiveQueryMessageBuilderTests.cs (3)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (6)
  • ParseLiveQueryMessageBuilder (8-69)
  • Task (10-17)
  • Task (19-24)
  • Task (26-40)
  • Task (42-56)
  • IDictionary (58-68)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (4)
  • Task (14-14)
  • Task (21-21)
  • Task (28-28)
  • IDictionary (34-34)
Parse/Infrastructure/ServiceHub.cs (6)
Parse/Infrastructure/LiveQueryServerConnectionData.cs (1)
  • LiveQueryServerConnectionData (12-12)
Parse/Infrastructure/Utilities/LateInitializer.cs (1)
  • LateInitializer (14-90)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (1)
  • TextWebSocketClient (16-239)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (2)
  • ParseLiveQueryMessageParser (10-92)
  • ParseLiveQueryMessageParser (14-17)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • ParseLiveQueryMessageBuilder (8-69)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (2)
  • ParseLiveQueryController (22-636)
  • ParseLiveQueryController (127-134)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (1)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (58-68)
Parse/Infrastructure/LateInitializedMutableServiceHub.cs (6)
Parse/Infrastructure/Utilities/LateInitializer.cs (2)
  • LateInitializer (14-90)
  • SetValue (53-73)
Parse/Infrastructure/LiveQueryServerConnectionData.cs (1)
  • LiveQueryServerConnectionData (12-12)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (1)
  • TextWebSocketClient (16-239)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (2)
  • ParseLiveQueryMessageParser (10-92)
  • ParseLiveQueryMessageParser (14-17)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • ParseLiveQueryMessageBuilder (8-69)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (2)
  • ParseLiveQueryController (22-636)
  • ParseLiveQueryController (127-134)
Parse.Tests/LiveQueryTests.cs (3)
Parse/Infrastructure/LiveQueryServerConnectionData.cs (1)
  • LiveQueryServerConnectionData (12-12)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Platform/Queries/ParseQuery.cs (20)
  • ParseLiveQuery (920-927)
  • IDictionary (157-193)
  • IDictionary (835-855)
  • IDictionary (872-883)
  • ParseQuery (47-928)
  • ParseQuery (84-133)
  • ParseQuery (198-198)
  • ParseQuery (205-205)
  • ParseQuery (215-218)
  • ParseQuery (226-229)
  • ParseQuery (241-244)
  • ParseQuery (255-258)
  • ParseQuery (268-271)
  • ParseQuery (280-283)
  • ParseQuery (292-295)
  • ParseQuery (305-308)
  • ParseQuery (310-313)
  • ParseQuery (324-327)
  • ParseQuery (336-339)
  • ParseQuery (348-351)
Parse/Infrastructure/MutableServiceHub.cs (4)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (2)
  • ParseLiveQueryMessageParser (10-92)
  • ParseLiveQueryMessageParser (14-17)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • ParseLiveQueryMessageBuilder (8-69)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (1)
  • TextWebSocketClient (16-239)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (2)
  • ParseLiveQueryController (22-636)
  • ParseLiveQueryController (127-134)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (9)
Parse/Infrastructure/Utilities/JsonUtilities.cs (2)
  • Parse (332-372)
  • JsonUtilities (16-508)
Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs (2)
  • ParseLiveQueryErrorEventArgs (8-65)
  • ParseLiveQueryErrorEventArgs (59-64)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (34-34)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (58-68)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (6)
  • IDictionary (41-50)
  • GetClientId (19-28)
  • GetRequestId (30-39)
  • IObjectState (52-62)
  • IObjectState (64-74)
  • code (76-91)
Parse/Platform/LiveQueries/ParseLiveQuery.cs (6)
  • IDictionary (96-104)
  • Task (114-115)
  • ParseLiveQuery (18-116)
  • ParseLiveQuery (39-58)
  • ParseLiveQuery (65-79)
  • ParseLiveQuery (90-94)
Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (9)
  • OnDelete (143-144)
  • OnLeave (132-135)
  • OnUpdate (120-123)
  • OnEnter (109-112)
  • OnCreate (100-101)
  • Task (80-81)
  • Task (90-91)
  • ParseLiveQuerySubscription (14-145)
  • ParseLiveQuerySubscription (58-65)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (6)
  • Task (60-79)
  • Task (89-95)
  • Task (97-155)
  • Task (201-209)
  • Dispose (214-218)
  • Dispose (220-238)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryController.cs (5)
  • Task (41-41)
  • Task (64-64)
  • Task (84-84)
  • Task (101-101)
  • Task (114-114)
Parse/Abstractions/Infrastructure/IServiceHub.cs (1)
Parse/Infrastructure/LiveQueryServerConnectionData.cs (1)
  • LiveQueryServerConnectionData (12-12)
🔇 Additional comments (8)
Parse.Tests/Parse.Tests.csproj (1)

9-12: Test SDK + MSTest upgrades look good

No compatibility concerns spotted for net8.0/net9.0. Proceed.

Parse.Tests/MessageReceivedEventArgsTests.cs (1)

25-37: LGTM: constructor and null-guard coverage are clear

The tests accurately validate the event args behavior and lifecycle setup/teardown.

Parse.Tests/LiveQueryDualEventArgsTests.cs (1)

21-44: LGTM: dual-args object/original semantics validated

Constructor and null-argument cases are properly covered.

Parse/Infrastructure/OrchestrationServiceHub.cs (1)

37-37: LGTM: live-query/WebSocket accessors wired through orchestration hub

Pattern matches existing members and preserves Custom-over-Default precedence.

Also applies to: 49-49, 69-74

Parse/Abstractions/Infrastructure/IMutableServiceHub.cs (1)

22-22: LGTM: mutable hooks for live-query components

Setter additions are consistent with the existing service hub contract.

Also applies to: 35-35, 42-45

Parse/Abstractions/Infrastructure/IServiceHub.cs (1)

26-30: Well-structured Live Query integration

The addition of Live Query components to the service hub interface follows the existing pattern and provides a clean abstraction for the Live Query functionality.

Parse/Infrastructure/MutableServiceHub.cs (1)

114-121: LGTM - Proper null-safe initialization of Live Query components

The conditional initialization of WebSocketClient and LiveQueryController only when LiveQueryServerConnectionData is present is a good defensive programming practice that prevents unnecessary resource allocation.

Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)

19-24: Verify the use of "windowsKey" field name

Based on the PR comments, YBTopaz8 previously questioned the use of "windowsKey" and theSlyest confirmed it matches the LiveQuery guide and Parse Server RequestSchema.js. This is intentional and correct.

Copy link

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

♻️ Duplicate comments (2)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (2)

30-39: Harden requestId parsing (int/long/string; overflow-safe).

LiveQuery servers may send requestId as int or string; current strict long check will throw. Also use checked cast.

-        if (!(message.TryGetValue("requestId", out object requestIdObj) && requestIdObj is long requestIdLong && requestIdLong > 0))
-            throw new ArgumentException(@"Message does not contain a valid request ID.", nameof(message));
-
-        return (int)requestIdLong;
+        if (!message.TryGetValue("requestId", out object requestIdObj))
+            throw new ArgumentException("Message does not contain a request ID.", nameof(message));
+
+        long requestIdLong = requestIdObj switch
+        {
+            long l => l,
+            int i => i,
+            string s when long.TryParse(s, out var parsed) => parsed,
+            _ => 0
+        };
+        if (requestIdLong <= 0)
+            throw new ArgumentException("Message does not contain a valid request ID.", nameof(message));
+        return checked((int)requestIdLong);

71-86: Harden error parsing (code int/long/string; reconnect bool/string; overflow-safe).

Prevents brittle casts on real-world payloads.

-        if (!(message.TryGetValue("code", out object codeObj) && codeObj is long codeLong))
-            throw new ArgumentException("Message does not contain a valid code.", nameof(message));
+        if (!message.TryGetValue("code", out object codeObj))
+            throw new ArgumentException("Message does not contain a code.", nameof(message));
+        long codeLong = codeObj switch
+        {
+            long l => l,
+            int i => i,
+            string s when long.TryParse(s, out var parsed) => parsed,
+            _ => 0
+        };
 
         if (!(message.TryGetValue("error", out object errorObj) && errorObj is string error))
             throw new ArgumentException("Message does not contain a valid error description.", nameof(message));
 
-        if (!(message.TryGetValue("reconnect", out object reconnectObj) && reconnectObj is bool reconnect))
-            throw new ArgumentException("Message does not contain a valid reconnect flag.", nameof(message));
+        if (!message.TryGetValue("reconnect", out object reconnectObj))
+            throw new ArgumentException("Message does not contain a reconnect flag.", nameof(message));
+        bool reconnect = reconnectObj switch
+        {
+            bool b => b,
+            string s when bool.TryParse(s, out var parsed) => parsed,
+            _ => false
+        };
 
-        return ((int)codeLong, error, reconnect);
+        return (checked((int)codeLong), error, reconnect);
🧹 Nitpick comments (7)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (4)

10-10: Seal the parser and make visibility explicit.

Prevents unintended inheritance and clarifies intent.

-class ParseLiveQueryMessageParser : IParseLiveQueryMessageParser
+internal sealed class ParseLiveQueryMessageParser : IParseLiveQueryMessageParser

24-28: Style consistency: prefer string alias.

Minor C# style nit.

-        if (!(message.TryGetValue("clientId", out object clientIdObj) && clientIdObj is string clientId && !String.IsNullOrWhiteSpace(clientId)))
+        if (!(message.TryGetValue("clientId", out object clientIdObj) && clientIdObj is string clientId && !string.IsNullOrWhiteSpace(clientId)))

41-47: Fix exception message placeholder and validate inputs.

@"...%{key}..." won’t interpolate; add proper message and basic argument validation.

     private IDictionary<string, object> GetDictionary(IDictionary<string, object> message, string key)
     {
-        if (!(message.TryGetValue(key, out object obj) && obj is IDictionary<string, object> dict))
-            throw new ArgumentException(@"Message does not contain a valid %{key}.", nameof(message));
+        if (message is null)
+            throw new ArgumentNullException(nameof(message));
+        if (string.IsNullOrWhiteSpace(key))
+            throw new ArgumentException("Key must be provided.", nameof(key));
+        if (!message.TryGetValue(key, out object obj) || obj is not IDictionary<string, object> dict)
+            throw new ArgumentException($"Message does not contain a valid '{key}' dictionary.", nameof(message));
 
         return dict;
     }

49-58: Avoid global dependency on ParseClient.Instance.Services. Inject services.

Improves testability and removes hidden global coupling.

-    private IParseDataDecoder Decoder { get; }
+    private IParseDataDecoder Decoder { get; }
+    private IServiceHub Services { get; }
 
-    public ParseLiveQueryMessageParser(IParseDataDecoder decoder)
+    public ParseLiveQueryMessageParser(IParseDataDecoder decoder, IServiceHub services)
     {
-        Decoder = decoder ?? throw new ArgumentNullException(nameof(decoder));
+        Decoder = decoder ?? throw new ArgumentNullException(nameof(decoder));
+        Services = services ?? throw new ArgumentNullException(nameof(services));
     }
-        return ParseObjectCoder.Instance.Decode(current, Decoder, ParseClient.Instance.Services);
+        return ParseObjectCoder.Instance.Decode(current, Decoder, Services);
-        return ParseObjectCoder.Instance.Decode(original, Decoder, ParseClient.Instance.Services);
+        return ParseObjectCoder.Instance.Decode(original, Decoder, Services);

Add this using at the top of the file:

using Parse.Abstractions.Infrastructure;

Also applies to: 60-69

Parse.Tests/LiveQueryMessageBuilderTests.cs (3)

40-46: Consider removing or enhancing the constructor test.

This test only verifies that the constructor doesn't throw an exception, which provides minimal value. Consider either removing it or enhancing it to verify meaningful initialization state.

-    [TestMethod]
-    public void TestConstructor()
-    {
-        ParseLiveQueryMessageBuilder builder = new ParseLiveQueryMessageBuilder();
-
-        Assert.IsNotNull(builder, "Builder should not be null after construction.");
-    }

83-132: Eliminate code duplication in subscribe and update subscription tests.

The test logic for TestBuildSubscribeMessage and TestBuildUpdateSubscriptionMessage (lines 135-183) is nearly identical, differing only in the expected operation type ("subscribe" vs "update"). This creates maintenance overhead and increases the likelihood of inconsistencies.

Consider extracting the common validation logic into a helper method:

+    private async Task ValidateQueryMessage(IDictionary<string, object> message, string expectedOp, int requestId)
+    {
+        Assert.IsNotNull(message);
+        Assert.HasCount(4, message);
+
+        Assert.IsTrue(message.ContainsKey("op"));
+        Assert.AreEqual(expectedOp, message["op"]);
+
+        Assert.IsTrue(message.ContainsKey("requestId"));
+        Assert.AreEqual(requestId, message["requestId"]);
+
+        Assert.IsTrue(message.ContainsKey("query"));
+        Assert.IsInstanceOfType<IDictionary<string, object>>(message["query"], "The 'query' value should be a Dictionary<string, object>.");
+        Assert.HasCount(4, (IDictionary<string, object>) message["query"]);
+        IDictionary<string, object> query = message["query"] as IDictionary<string, object>;
+
+        Assert.IsTrue(query.ContainsKey("className"), "The 'query' dictionary should contain the 'className' key.");
+        Assert.AreEqual("DummyClass", query["className"], "The 'className' property should be 'DummyClass'.");
+
+        Assert.IsTrue(query.ContainsKey("where"), "The 'query' dictionary should contain the 'where' key.");
+        Assert.IsInstanceOfType<IDictionary<string, object>>(query["where"], "The 'where' property should be a Dictionary<string, object>.");
+        IDictionary<string, object> where = (IDictionary<string, object>) query["where"];
+        Assert.HasCount(1, where, "The 'where' dictionary should contain exactly one key-value pair.");
+        Assert.IsTrue(where.ContainsKey("foo"), "The 'where' dictionary should contain the 'foo' key.");
+        Assert.AreEqual("bar", where["foo"], "The 'foo' property in 'where' should be 'bar'.");
+
+        Assert.IsTrue(query.ContainsKey("keys"), "The 'query' dictionary should contain the 'keys' key.");
+        Assert.IsInstanceOfType<string[]>(query["keys"], "The 'keys' property should be a string array.");
+        Assert.HasCount(1, (string[]) query["keys"], "The 'keys' array should contain exactly one element.");
+        Assert.AreEqual("foo", ((string[]) query["keys"])[0], "The 'keys' parameter should contain 'foo'.");
+
+        Assert.IsTrue(query.ContainsKey("watch"), "The 'query' dictionary should contain the 'watch' key.");
+        Assert.IsInstanceOfType<string[]>(query["watch"], "The 'watch' property should be a string array.");
+        Assert.HasCount(1, (string[]) query["watch"], "The 'watch' array should contain exactly one element.");
+        Assert.AreEqual("foo", ((string[]) query["watch"])[0], "The 'watch' parameter should contain 'foo'.");
+    }

     [TestMethod]
     public async Task TestBuildSubscribeMessage()
     {
         int requestId = 2;
         ParseLiveQuery<ParseObject> liveQuery = new ParseLiveQuery<ParseObject>(
             Client.Services,
             "DummyClass",
             new Dictionary<string, object> { { "foo", "bar" } },
             ["foo"],
             ["foo"]);
         ParseLiveQueryMessageBuilder builder = new ParseLiveQueryMessageBuilder();
         IDictionary<string, object> message = await builder.BuildSubscribeMessage<ParseObject>(requestId, liveQuery);

-        Assert.IsNotNull(message);
-        Assert.HasCount(4, message);
-
-        Assert.IsTrue(message.ContainsKey("op"));
-        Assert.AreEqual("subscribe", message["op"]);
-
-        Assert.IsTrue(message.ContainsKey("requestId"));
-        Assert.AreEqual(requestId, message["requestId"]);
-
-        Assert.IsTrue(message.ContainsKey("query"));
-        Assert.IsInstanceOfType<IDictionary<string, object>>(message["query"], "The 'query' value should be a Dictionary<string, object>.");
-        Assert.HasCount(4, (IDictionary<string, object>) message["query"]);
-        IDictionary<string, object> query = message["query"] as IDictionary<string, object>;
-
-        Assert.IsTrue(query.ContainsKey("className"), "The 'query' dictionary should contain the 'className' key.");
-        Assert.AreEqual("DummyClass", query["className"], "The 'className' property should be 'DummyClass'.");
-
-        Assert.IsTrue(query.ContainsKey("where"), "The 'query' dictionary should contain the 'where' key.");
-        Assert.IsInstanceOfType<IDictionary<string, object>>(query["where"], "The 'where' property should be a Dictionary<string, object>.");
-        IDictionary<string, object> where = (IDictionary<string, object>) query["where"];
-        Assert.HasCount(1, where, "The 'where' dictionary should contain exactly one key-value pair.");
-        Assert.IsTrue(where.ContainsKey("foo"), "The 'where' dictionary should contain the 'foo' key.");
-        Assert.AreEqual("bar", where["foo"], "The 'foo' property in 'where' should be 'bar'.");
-
-        Assert.IsTrue(query.ContainsKey("keys"), "The 'query' dictionary should contain the 'keys' key.");
-        Assert.IsInstanceOfType<string[]>(query["keys"], "The 'keys' property should be a string array.");
-        Assert.HasCount(1, (string[]) query["keys"], "The 'keys' array should contain exactly one element.");
-        Assert.AreEqual("foo", ((string[]) query["keys"])[0], "The 'keys' parameter should contain 'foo'.");
-
-        Assert.IsTrue(query.ContainsKey("watch"), "The 'query' dictionary should contain the 'watch' key.");
-        Assert.IsInstanceOfType<string[]>(query["watch"], "The 'watch' property should be a string array.");
-        Assert.HasCount(1, (string[]) query["watch"], "The 'watch' array should contain exactly one element.");
-        Assert.AreEqual("foo", ((string[]) query["watch"])[0], "The 'watch' parameter should contain 'foo'.");
+        await ValidateQueryMessage(message, "subscribe", requestId);

         await Assert.ThrowsExactlyAsync<ArgumentOutOfRangeException>(async () => await builder.BuildSubscribeMessage<ParseObject>(0, liveQuery));
         await Assert.ThrowsExactlyAsync<ArgumentNullException>(async () => await builder.BuildSubscribeMessage<ParseObject>(requestId, null));
     }

134-183: Apply the same refactoring to update subscription test.

As mentioned in the previous comment, this test duplicates the validation logic from the subscribe test.

Apply similar changes to use the helper method:

     [TestMethod]
     public async Task TestBuildUpdateSubscriptionMessage()
     {
         int requestId = 2;
         ParseLiveQuery<ParseObject> liveQuery = new ParseLiveQuery<ParseObject>(
             Client.Services,
             "DummyClass",
             new Dictionary<string, object> { { "foo", "bar" } },
             ["foo"],
             ["foo"]);
         ParseLiveQueryMessageBuilder builder = new ParseLiveQueryMessageBuilder();
         IDictionary<string, object> message = await builder.BuildUpdateSubscriptionMessage<ParseObject>(requestId, liveQuery);

-        Assert.IsNotNull(message);
-        Assert.HasCount(4, message);
-
-        Assert.IsTrue(message.ContainsKey("op"));
-        Assert.AreEqual("update", message["op"]);
-
-        Assert.IsTrue(message.ContainsKey("requestId"));
-        Assert.AreEqual(requestId, message["requestId"]);
-
-        Assert.IsTrue(message.ContainsKey("query"));
-        Assert.IsInstanceOfType<IDictionary<string, object>>(message["query"], "The 'query' value should be a Dictionary<string, object>.");
-        Assert.HasCount(4, (IDictionary<string, object>) message["query"]);
-        IDictionary<string, object> query = message["query"] as IDictionary<string, object>;
-
-        Assert.IsTrue(query.ContainsKey("className"), "The 'query' dictionary should contain the 'className' key.");
-        Assert.AreEqual("DummyClass", query["className"], "The 'className' property should be 'DummyClass'.");
-
-        Assert.IsTrue(query.ContainsKey("where"), "The 'query' dictionary should contain the 'where' key.");
-        Assert.IsInstanceOfType<IDictionary<string, object>>(query["where"], "The 'where' property should be a Dictionary<string, object>.");
-        IDictionary<string, object> where = (IDictionary<string, object>) query["where"];
-        Assert.HasCount(1, where, "The 'where' dictionary should contain exactly one key-value pair.");
-        Assert.IsTrue(where.ContainsKey("foo"), "The 'where' dictionary should contain the 'foo' key.");
-        Assert.AreEqual("bar", where["foo"], "The 'foo' property in 'where' should be 'bar'.");
-
-        Assert.IsTrue(query.ContainsKey("keys"), "The 'query' dictionary should contain the 'keys' key.");
-        Assert.IsInstanceOfType<string[]>(query["keys"], "The 'keys' property should be a string array.");
-        Assert.HasCount(1, (string[]) query["keys"], "The 'keys' array should contain exactly one element.");
-        Assert.AreEqual("foo", ((string[]) query["keys"])[0], "The 'keys' parameter should contain 'foo'.");
-
-        Assert.IsTrue(query.ContainsKey("watch"), "The 'query' dictionary should contain the 'watch' key.");
-        Assert.IsInstanceOfType<string[]>(query["watch"], "The 'watch' property should be a string array.");
-        Assert.HasCount(1, (string[]) query["watch"], "The 'watch' array should contain exactly one element.");
-        Assert.AreEqual("foo", ((string[]) query["watch"])[0], "The 'watch' parameter should contain 'foo'.");
+        await ValidateQueryMessage(message, "update", requestId);

         await Assert.ThrowsExactlyAsync<ArgumentOutOfRangeException>(async () => await builder.BuildUpdateSubscriptionMessage<ParseObject>(0, liveQuery));
         await Assert.ThrowsExactlyAsync<ArgumentNullException>(async () => await builder.BuildUpdateSubscriptionMessage<ParseObject>(requestId, null));
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a13c7ab and b86a45a.

📒 Files selected for processing (4)
  • Parse.Tests/LiveQueryMessageBuilderTests.cs (1 hunks)
  • Parse.Tests/LiveQueryMessageParserTests.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • Parse.Tests/LiveQueryMessageParserTests.cs
  • Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs
🧰 Additional context used
🧬 Code graph analysis (2)
Parse.Tests/LiveQueryMessageBuilderTests.cs (4)
Parse/Infrastructure/MutableServiceHub.cs (2)
  • MutableServiceHub (36-124)
  • MutableServiceHub (75-123)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (6)
  • ParseLiveQueryMessageBuilder (8-69)
  • Task (10-17)
  • Task (19-24)
  • Task (26-40)
  • Task (42-56)
  • IDictionary (58-68)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (4)
  • Task (14-14)
  • Task (21-21)
  • Task (28-28)
  • IDictionary (34-34)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (2)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (5)
  • GetClientId (16-16)
  • GetRequestId (23-23)
  • IObjectState (30-30)
  • IObjectState (37-37)
  • code (46-46)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (58-68)
🔇 Additional comments (4)
Parse.Tests/LiveQueryMessageBuilderTests.cs (4)

1-11: LGTM! Clean imports and namespace setup.

All necessary imports are included and the namespace follows the established pattern for the test project.


14-38: Well-structured test setup with proper mocking.

The test class properly sets up the required dependencies with a mocked IParseCurrentUserController and creates a ParseClient instance for testing. The use of Client.Publicize() correctly makes the test client available as the singleton instance.


48-64: Comprehensive test for connect message building.

The test thoroughly validates all expected fields in the connect message and verifies they contain the correct values from the mock setup.


66-81: Good coverage of unsubscribe message functionality.

The test validates both the successful case and the error handling for invalid request IDs. The use of ThrowsExactly is appropriate for testing specific exception types.

Copy link

@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: 2

♻️ Duplicate comments (2)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (2)

408-410: Improve exception handling for connection timeout.

On connection timeout, the controller remains in the Connecting state with handlers attached. Consider rolling back to a clean state.

 catch (OperationCanceledException)
 {
+    // Rollback state on failure
+    _state = ParseLiveQueryState.Closed;
+    try
+    {
+        WebSocketClient.MessageReceived -= WebSocketClientOnMessageReceived;
+        WebSocketClient.WebsocketError -= WebSocketClientOnWebsocketError;
+        WebSocketClient.UnknownError -= WebSocketClientOnUnknownError;
+        await WebSocketClient.CloseAsync(CancellationToken.None);
+    }
+    catch { /* Ignore cleanup errors */ }
     throw new TimeoutException("Live query server connection request has reached timeout");
 }

573-577: Cancel pending waiters before clearing dictionaries to prevent hangs.

When closing the connection, pending TaskCompletionSource waiters in the signal dictionaries should be canceled before clearing to avoid leaving tasks hanging indefinitely.

 await WebSocketClient.CloseAsync(cancellationToken);
 _state = ParseLiveQueryState.Closed;
+
+// Cancel all pending signals to avoid hangs
+foreach (var kv in SubscriptionSignals) 
+    kv.Value?.TrySetCanceled();
+foreach (var kv in UnsubscriptionSignals) 
+    kv.Value?.TrySetCanceled();
+ConnectionSignal?.TrySetCanceled();
+
 SubscriptionSignals.Clear();
 UnsubscriptionSignals.Clear();
 Subscriptions.Clear();
🧹 Nitpick comments (6)
Parse.Tests/TextWebSocketClientTests.cs (4)

28-30: Use RunContinuationsAsynchronously and dispose CTS.

Construct TCS with RunContinuationsAsynchronously to avoid inline-continuation surprises, and prefer using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); over manual CancelAfter + no dispose.


31-39: Avoid assertions inside async event handlers.

If the assertion throws on a background thread, runners may not attribute the failure correctly. Capture the value in the handler, signal TCS, and assert on the test thread after awaiting.


33-36: Provider‑specific filter is brittle.

Hard‑coding “Request served by” ties the test to one echo provider. Either remove the filter (with a controlled echo server) or generalize via a predicate/config.


13-20: Use using var for disposal in constructor test.

Keeps cleanup exception‑safe and concise.

Apply:

-        TextWebSocketClient client = new TextWebSocketClient(4096);
-        Assert.IsNotNull(client);
-
-        client.Dispose();
+        using var client = new TextWebSocketClient(4096);
+        Assert.IsNotNull(client);
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (2)

180-189: Simplify method: Remove unnecessary initialization and return statement.

The method initializes requestId to 0 and later checks if it's non-zero, which is redundant. Also consider inlining null check.

 private bool ValidateClientMessage(IDictionary<string, object> message, out int requestId)
 {
-    requestId = 0;
-
     ClientId = MessageParser.GetClientId(message);
     if (ClientId is null)
+    {
+        requestId = 0;
         return false;
+    }
 
     requestId = MessageParser.GetRequestId(message);
     return requestId != 0;
 }

388-425: Consider extracting the connection logic into a separate method.

The ConnectAsync method has multiple responsibilities and complex branching logic. Consider extracting the actual connection logic for better readability.

Consider refactoring by extracting the connection establishment logic:

private async Task EstablishConnectionAsync(CancellationToken cancellationToken)
{
    _state = ParseLiveQueryState.Connecting;
    await OpenAsync(cancellationToken);
    
    WebSocketClient.MessageReceived += WebSocketClientOnMessageReceived;
    WebSocketClient.WebsocketError += WebSocketClientOnWebsocketError;
    WebSocketClient.UnknownError += WebSocketClientOnUnknownError;
    
    IDictionary<string, object> message = await MessageBuilder.BuildConnectMessage();
    ConnectionSignal = new TaskCompletionSource();
    
    try
    {
        await SendMessage(message, cancellationToken);
        using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
        cts.CancelAfter(Timeout);
        await ConnectionSignal.Task.WaitAsync(cts.Token);
    }
    catch (OperationCanceledException)
    {
        // Cleanup on failure...
        throw new TimeoutException("Live query server connection request has reached timeout");
    }
    finally
    {
        ConnectionSignal = null;
    }
}

Then simplify ConnectAsync:

public async Task ConnectAsync(CancellationToken cancellationToken = default)
{
    ThrowIfDisposed();
    
    if (_state == ParseLiveQueryState.Closed)
    {
        await EstablishConnectionAsync(cancellationToken);
    }
    else if (_state == ParseLiveQueryState.Connecting)
    {
        var signal = ConnectionSignal;
        if (signal is not null)
        {
            await signal.Task.WaitAsync(cancellationToken);
        }
    }
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b86a45a and ea60936.

📒 Files selected for processing (3)
  • Parse.Tests/TextWebSocketClientTests.cs (1 hunks)
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Parse/Infrastructure/Execution/TextWebSocketClient.cs
🧰 Additional context used
🧬 Code graph analysis (2)
Parse.Tests/TextWebSocketClientTests.cs (1)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (7)
  • TextWebSocketClient (16-239)
  • Dispose (214-218)
  • Dispose (220-238)
  • Task (60-79)
  • Task (89-95)
  • Task (97-155)
  • Task (201-209)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (6)
Parse/Infrastructure/Utilities/JsonUtilities.cs (2)
  • Parse (332-372)
  • JsonUtilities (16-508)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (6)
  • IDictionary (41-47)
  • GetClientId (19-28)
  • GetRequestId (30-39)
  • IObjectState (49-58)
  • IObjectState (60-69)
  • code (71-86)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (58-68)
Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (9)
  • OnDelete (143-144)
  • OnLeave (132-135)
  • OnUpdate (120-123)
  • OnEnter (109-112)
  • OnCreate (100-101)
  • Task (80-81)
  • Task (90-91)
  • ParseLiveQuerySubscription (14-145)
  • ParseLiveQuerySubscription (58-65)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (6)
  • Task (60-79)
  • Task (89-95)
  • Task (97-155)
  • Task (201-209)
  • Dispose (214-218)
  • Dispose (220-238)
Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs (1)
  • MessageReceivedEventArgs (8-14)
🔇 Additional comments (3)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (3)

625-625: LGTM! Proper handling of disposal with error catching.

The blocking call to CloseAsync().GetAwaiter().GetResult() is acceptable in the Dispose pattern since it's a best-effort cleanup, and you're correctly catching and logging any exceptions.


353-372: LGTM! Robust message parsing with proper error handling.

The implementation correctly handles parsing failures and invalid message formats, providing appropriate debug logging and error events.


443-468: LGTM! Clean abstraction for send-and-wait pattern.

The helper method effectively eliminates code duplication from the subscription methods and properly handles timeouts and cleanup.

Comment on lines 22 to 47
[TestMethod]
public async Task TestSendAndReceive()
{
TextWebSocketClient client = new TextWebSocketClient(32);
await client.OpenAsync("wss://echo.websocket.org", CancellationToken.None);

TaskCompletionSource ConnectionSignal = new TaskCompletionSource();
CancellationTokenSource cts = new CancellationTokenSource();

client.MessageReceived += (_, e) =>
{
if (e.Message.StartsWith("Request served by"))
{
return;
}
Assert.AreEqual("Hello world, WebSocket listening!", e.Message);
ConnectionSignal?.TrySetResult();
};

await client.SendAsync("Hello world, WebSocket listening!", cts.Token);
cts.CancelAfter(5000);
await ConnectionSignal.Task.WaitAsync(cts.Token);
ConnectionSignal = null;
await client.CloseAsync();
client.Dispose();
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Don’t hit public echo servers in unit tests; gate as Integration and add timeouts/cancellation.

External endpoints are flaky and can hang CI. Treat this as an integration test, require a configurable URL, and pass a timeout token to every async op (including OpenAsync/CloseAsync). Also prefer disposing via using var.

Apply:

-    [TestMethod]
-    public async Task TestSendAndReceive()
+    [TestMethod]
+    [TestCategory("Integration")]
+    [Timeout(15000)]
+    public async Task TestSendAndReceive()
     {
-        TextWebSocketClient client = new TextWebSocketClient(32);
-        await client.OpenAsync("wss://echo.websocket.org", CancellationToken.None);
-
-        TaskCompletionSource ConnectionSignal = new TaskCompletionSource();
-        CancellationTokenSource cts = new CancellationTokenSource();
+        using var client = new TextWebSocketClient(32);
+        using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));
+
+        var url = Environment.GetEnvironmentVariable("ECHO_WS_URL");
+        if (string.IsNullOrWhiteSpace(url))
+        {
+            Assert.Inconclusive("No echo server configured (set ECHO_WS_URL) for integration runs.");
+        }
+        await client.OpenAsync(url!, cts.Token);
 
-        client.MessageReceived += (_, e) =>
-        {
-            if (e.Message.StartsWith("Request served by"))
-            {
-                return;
-            }
-            Assert.AreEqual("Hello world, WebSocket listening!", e.Message);
-            ConnectionSignal?.TrySetResult();
-        };
+        string received = null;
+        EventHandler<MessageReceivedEventArgs> handler = (_, e) =>
+        {
+            if (e.Message.StartsWith("Request served by")) return; // provider noise
+            received = e.Message;
+            tcs.TrySetResult(true);
+        };
+        var tcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
+        client.MessageReceived += handler;
 
-        await client.SendAsync("Hello world, WebSocket listening!", cts.Token);
-        cts.CancelAfter(5000);
-        await ConnectionSignal.Task.WaitAsync(cts.Token);
-        ConnectionSignal = null;
-        await client.CloseAsync();
-        client.Dispose();
+        await client.SendAsync("Hello world, WebSocket listening!", cts.Token);
+        await tcs.Task.WaitAsync(cts.Token);
+        Assert.AreEqual("Hello world, WebSocket listening!", received);
+
+        client.MessageReceived -= handler;
+        await client.CloseAsync(cts.Token);
     }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In Parse.Tests/TextWebSocketClientTests.cs around lines 22 to 47, this unit test
hits a public echo server and lacks cancellation for all async calls; convert it
to an integration test (e.g., add [TestCategory("Integration")] or move to
integration suite), make the echo server URL configurable via a test
setting/environment variable (with the test skipped/failing fast if not set),
create a CancellationTokenSource with a reasonable timeout and pass its Token
into OpenAsync, SendAsync and CloseAsync, use using var (or try/finally) to
dispose the client, and ensure the test aborts/returns promptly when the token
is canceled instead of hanging.

Copy link

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

♻️ Duplicate comments (1)
Parse.Tests/TextWebSocketClientTests.cs (1)

25-31: Don’t hard‑code a public echo server; make it configurable and skip when unset.

Public endpoints are flaky and can hang CI. Read from an env var (e.g., ECHO_WS_URL) and mark the test inconclusive if not configured.

Apply:

-        await client.OpenAsync("wss://echo.websocket.org", cts.Token);
+        var url = Environment.GetEnvironmentVariable("ECHO_WS_URL");
+        if (string.IsNullOrWhiteSpace(url))
+        {
+            Assert.Inconclusive("No echo server configured (set ECHO_WS_URL) for integration runs.");
+        }
+        await client.OpenAsync(url!, cts.Token);
🧹 Nitpick comments (15)
Parse.Tests/TextWebSocketClientTests.cs (7)

25-27: Dispose resources with using to avoid leaks in tests.

Both client and CTS are IDisposable. Prefer using var to guarantee cleanup.

Apply:

-        TextWebSocketClient client = new TextWebSocketClient(32);
-        CancellationTokenSource cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));
+        using var client = new TextWebSocketClient(32);
+        using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));

41-50: Always detach handler and close in finally.

If the test times out/throws, the current code may skip cleanup.

Apply:

-        client.MessageReceived += handler;
-
-        await client.SendAsync("Hello world, WebSocket listening!", cts.Token);
-        cts.CancelAfter(5000);
-        await receiveTcs.Task.WaitAsync(cts.Token);
-        Assert.AreEqual("Hello world, WebSocket listening!", receivedMessage);
-
-        client.MessageReceived -= handler;
-        await client.CloseAsync(cts.Token);
+        client.MessageReceived += handler;
+        try
+        {
+            await client.SendAsync("Hello world, WebSocket listening!", cts.Token);
+            await receiveTcs.Task.WaitAsync(cts.Token);
+            Assert.AreEqual("Hello world, WebSocket listening!", receivedMessage);
+        }
+        finally
+        {
+            client.MessageReceived -= handler;
+            await client.CloseAsync(cts.Token);
+        }

44-44: Redundant CancelAfter.

CTS already has a 5s timeout; this second CancelAfter is unnecessary and can mask timing.

Apply:

-        cts.CancelAfter(5000);

34-40: Avoid provider‑specific filtering; wait for the exact echo.

Match the expected payload instead of filtering “Request served by…”.

Apply:

-            if (e.Message.StartsWith("Request served by"))
-            {
-                return;
-            }
-            receivedMessage = e.Message;
-            receiveTcs?.TrySetResult();
+            if (e.Message == "Hello world, WebSocket listening!")
+            {
+                receivedMessage = e.Message;
+                receiveTcs.TrySetResult();
+            }

31-31: Honor nullable reference types.

Annotate as string? to avoid warnings when NRT is enabled.

Apply:

-        string receivedMessage = null;
+        string? receivedMessage = null;

16-18: Dispose the client in TestConstructor.

Use using var to enforce disposal even in simple ctor tests.

Apply:

-        TextWebSocketClient client = new TextWebSocketClient(4096);
-        Assert.IsNotNull(client);
+        using var client = new TextWebSocketClient(4096);
+        Assert.IsNotNull(client);

2-2: Remove unused using.

System.Diagnostics isn’t used in this test file.

Apply:

-using System.Diagnostics;
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (2)

12-24: Make LiveQueryError a readonly struct

This is immutable; marking it readonly avoids defensive copies and communicates intent.

-    struct LiveQueryError
+    readonly struct LiveQueryError
     {
         public int Code { get; }
         public string Message { get; }
         public bool Reconnect { get; }

         public LiveQueryError(int code, string message, bool reconnect)
         {
             Code = code;
             Message = message;
             Reconnect = reconnect;
         }
     }

54-61: Fix XML doc: return type is LiveQueryError, not a tuple

The summary says this returns a tuple, but the signature returns LiveQueryError.

-    /// <returns>
-    /// A tuple containing the error code, error message, and a boolean indicating whether to reconnect.
-    /// </returns>
+    /// <returns>
+    /// Error details (code, message, reconnect) as a LiveQueryError.
+    /// </returns>
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (2)

12-20: Avoid mutating the input dictionary

AppendSessionToken currently mutates the passed dictionary; returning a copy is safer and consistent with the controller’s append helper.

     private async Task<IDictionary<string, object>> AppendSessionToken(IDictionary<string, object> message)
     {
         if (message is null)
             throw new ArgumentNullException(nameof(message));

         string sessionToken = await ParseClient.Instance.Services.GetCurrentSessionToken();
-        if (sessionToken is not null)
-            message.Add("sessionToken", sessionToken);
-
-        return message;
+        return sessionToken is null
+            ? message
+            : new Dictionary<string, object>(message)
+            {
+                ["sessionToken"] = sessionToken
+            };
     }

22-33: Harden config validation for connect payload

Guard against uninitialized Services and empty credentials; fail fast with clear errors.

-    public async Task<IDictionary<string, object>> BuildConnectMessage() => await AppendSessionToken(new Dictionary<string, object>
+    public async Task<IDictionary<string, object>> BuildConnectMessage() => await AppendSessionToken(new Dictionary<string, object>
         {
             { "op", "connect" },
-            {
-                "applicationId",
-                ParseClient.Instance.Services.LiveQueryServerConnectionData?.ApplicationID ?? throw new InvalidOperationException("LiveQueryServerConnectionData is not configured")
-            },
-            {
-                "windowsKey",
-                ParseClient.Instance.Services.LiveQueryServerConnectionData?.Key ?? throw new InvalidOperationException("LiveQueryServerConnectionData is not configured")
-            }
+            { "applicationId", GetConnectionData().ApplicationID },
+            { "windowsKey", GetConnectionData().Key }
         });
+
+    private static ILiveQueryServerConnectionData GetConnectionData()
+    {
+        var services = ParseClient.Instance.Services ?? throw new InvalidOperationException("ParseClient.Services is not configured");
+        var data = services.LiveQueryServerConnectionData ?? throw new InvalidOperationException("LiveQueryServerConnectionData is not configured");
+        if (string.IsNullOrWhiteSpace(data.ApplicationID))
+            throw new InvalidOperationException("LiveQueryServerConnectionData.ApplicationID is missing.");
+        if (string.IsNullOrWhiteSpace(data.Key))
+            throw new InvalidOperationException("LiveQueryServerConnectionData.Key is missing.");
+        return data;
+    }
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (4)

37-46: Correct docs: Timeout is a TimeSpan, not “milliseconds”

Doc says milliseconds; the type and usage are TimeSpan.

-    /// Gets or sets the timeout duration, in milliseconds, used by the ParseLiveQueryController
+    /// Gets or sets the timeout duration, as a TimeSpan, used by the ParseLiveQueryController

579-598: Cancel pending connection waiter on CloseAsync

Prevent dangling awaits by canceling ConnectionSignal as well.

     public async Task CloseAsync(CancellationToken cancellationToken = default)
     {
         ThrowIfDisposed();
         _state = ParseLiveQueryState.Closed;
         WebSocketClient.MessageReceived -= WebSocketClientOnMessageReceived;
         WebSocketClient.WebsocketError -= WebSocketClientOnWebsocketError;
         WebSocketClient.UnknownError -= WebSocketClientOnUnknownError;
         await WebSocketClient.CloseAsync(cancellationToken);
+        ConnectionSignal?.TrySetCanceled();
         foreach (var signal in SubscriptionSignals.Values)
         {
             signal.TrySetCanceled();
         }
         SubscriptionSignals.Clear();
         foreach (var signal in UnsubscriptionSignals.Values)
         {
             signal.TrySetCanceled();
         }
         UnsubscriptionSignals.Clear();
         Subscriptions.Clear();
     }

280-297: Simplify error processing; avoid double-parse and silent returns

Delegate validation to the parser; surface parse errors via Error to avoid swallowing server error messages.

-    void ProcessErrorMessage(IDictionary<string, object> message)
-    {
-        if (!(message.TryGetValue("code", out object codeObj) &&
-            Int32.TryParse(codeObj?.ToString(), out int code)))
-            return;
-
-        if (!(message.TryGetValue("error", out object errorObj) &&
-              errorObj is string error))
-            return;
-
-        if (!(message.TryGetValue("reconnect", out object reconnectObj) &&
-            Boolean.TryParse(reconnectObj?.ToString(), out bool reconnect)))
-            return;
-
-        var liveQueryError = MessageParser.GetError(message);
-
-        Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(liveQueryError.Code, liveQueryError.Message, liveQueryError.Reconnect));
-    }
+    void ProcessErrorMessage(IDictionary<string, object> message)
+    {
+        try
+        {
+            var e = MessageParser.GetError(message);
+            Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(e.Code, e.Message, e.Reconnect));
+        }
+        catch (ArgumentException ex)
+        {
+            Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(31, $"Invalid error message: {ex.Message}", true, ex));
+        }
+    }

109-109: Make Subscriptions read-only

Setter is unnecessary; prevents accidental reassignment.

-    private ConcurrentDictionary<int, IParseLiveQuerySubscription> Subscriptions { get; set; } = new ConcurrentDictionary<int, IParseLiveQuerySubscription>();
+    private ConcurrentDictionary<int, IParseLiveQuerySubscription> Subscriptions { get; } = new();
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea60936 and 8341e12.

📒 Files selected for processing (10)
  • Parse.Tests/LiveQueryMessageBuilderTests.cs (1 hunks)
  • Parse.Tests/LiveQueryMessageParserTests.cs (1 hunks)
  • Parse.Tests/LiveQuerySubscriptionTests.cs (1 hunks)
  • Parse.Tests/LiveQueryTests.cs (1 hunks)
  • Parse.Tests/TextWebSocketClientTests.cs (1 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1 hunks)
  • Parse/Infrastructure/ServiceHub.cs (4 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • Parse.Tests/LiveQueryMessageBuilderTests.cs
  • Parse/Infrastructure/ServiceHub.cs
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs
  • Parse.Tests/LiveQuerySubscriptionTests.cs
  • Parse.Tests/LiveQueryTests.cs
  • Parse.Tests/LiveQueryMessageParserTests.cs
🧰 Additional context used
🧬 Code graph analysis (4)
Parse.Tests/TextWebSocketClientTests.cs (3)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (5)
  • TextWebSocketClient (16-239)
  • Task (60-79)
  • Task (89-95)
  • Task (97-155)
  • Task (201-209)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (10)
  • Task (328-336)
  • Task (338-339)
  • Task (341-354)
  • Task (377-409)
  • Task (423-438)
  • Task (456-481)
  • Task (505-519)
  • Task (540-545)
  • Task (562-568)
  • Task (579-598)
Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs (1)
  • MessageReceivedEventArgs (8-14)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (2)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (10)
  • Task (328-336)
  • Task (338-339)
  • Task (341-354)
  • Task (377-409)
  • Task (423-438)
  • Task (456-481)
  • Task (505-519)
  • Task (540-545)
  • Task (562-568)
  • Task (579-598)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (4)
  • Task (14-14)
  • Task (21-21)
  • Task (28-28)
  • IDictionary (34-34)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (6)
Parse/Infrastructure/Utilities/JsonUtilities.cs (2)
  • Parse (332-372)
  • JsonUtilities (16-508)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (6)
  • IParseLiveQueryMessageParser (68-83)
  • IDictionary (41-50)
  • GetClientId (19-28)
  • GetRequestId (30-39)
  • IObjectState (52-58)
  • IObjectState (60-66)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (67-77)
Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (9)
  • OnDelete (143-144)
  • OnLeave (132-135)
  • OnUpdate (120-123)
  • OnEnter (109-112)
  • OnCreate (100-101)
  • Task (80-81)
  • Task (90-91)
  • ParseLiveQuerySubscription (14-145)
  • ParseLiveQuerySubscription (58-65)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (6)
  • Task (60-79)
  • Task (89-95)
  • Task (97-155)
  • Task (201-209)
  • Dispose (214-218)
  • Dispose (220-238)
Parse/Infrastructure/Execution/MessageReceivedEventArgs.cs (1)
  • MessageReceivedEventArgs (8-14)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (4)
  • IParseLiveQueryMessageParser (68-83)
  • GetClientId (19-28)
  • IDictionary (41-50)
  • GetRequestId (30-39)
🔇 Additional comments (2)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)

67-77: Unsubscribe message looks good

No token needed here; payload minimal and correct.

Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1)

106-109: Use TaskCompletionSource for clarity and TFM compatibility

  • Replace non-generic TaskCompletionSource with TaskCompletionSource and change TrySetResult() calls to TrySetResult(true); update ConcurrentDictionary<int, TaskCompletionSource> → ConcurrentDictionary<int, TaskCompletionSource> and TCS initializations (new TaskCompletionSource() or new()).
  • Affected file/locations: Parse/Platform/LiveQueries/ParseLiveQueryController.cs — lines 106–109, 304–319, 386–388, 432–436, 461–472.

Automated verification failed due to script errors; confirm target frameworks or run the verification scripts and tests to ensure no other uses remain.

Copy link

@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: 5

♻️ Duplicate comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)

10-20: Avoid mutating the input dictionary in AppendSessionToken (return a copy).

Mutating the caller‑owned dictionary is surprising and can cause shared‑state bugs. Return a new dictionary when appending the token (mirrors the controller’s non‑mutating approach).

 private async Task<IDictionary<string, object>> AppendSessionToken(IDictionary<string, object> message)
 {
     if (message is null)
         throw new ArgumentNullException(nameof(message));
 
     string sessionToken = await ParseClient.Instance.Services.GetCurrentSessionToken();
-    if (sessionToken is not null)
-        message.Add("sessionToken", sessionToken);
-
-    return message;
+    if (!string.IsNullOrEmpty(sessionToken))
+    {
+        var copy = new Dictionary<string, object>(message);
+        copy["sessionToken"] = sessionToken;
+        return copy;
+    }
+    return message;
 }
🧹 Nitpick comments (11)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (2)

22-33: Harden against null ParseClient.Instance/Services before accessing LiveQuery config.

Defensive checks produce a better error than a NullReferenceException if Instance/Services aren’t initialized.

-    public async Task<IDictionary<string, object>> BuildConnectMessage() => await AppendSessionToken(new Dictionary<string, object>
+    public async Task<IDictionary<string, object>> BuildConnectMessage() => await AppendSessionToken(new Dictionary<string, object>
         {
             { "op", "connect" },
-            {
-                "applicationId",
-                ParseClient.Instance.Services.LiveQueryServerConnectionData?.ApplicationID ?? throw new InvalidOperationException("LiveQueryServerConnectionData is not configured")
-            },
-            {
-                "windowsKey",
-                ParseClient.Instance.Services.LiveQueryServerConnectionData?.Key ?? throw new InvalidOperationException("LiveQueryServerConnectionData is not configured")
-            }
+            { "applicationId",
+                (ParseClient.Instance?.Services?.LiveQueryServerConnectionData?.ApplicationID)
+                ?? throw new InvalidOperationException("LiveQueryServerConnectionData is not configured") },
+            { "windowsKey",
+                (ParseClient.Instance?.Services?.LiveQueryServerConnectionData?.Key)
+                ?? throw new InvalidOperationException("LiveQueryServerConnectionData is not configured") }
         });

35-49: De‑duplicate Subscribe vs. Update message construction.

Both methods differ only by the op value. Factor into a single helper to reduce drift.

+    private async Task<IDictionary<string, object>> BuildSubscriptionMessageCore<T>(string op, int requestId, ParseLiveQuery<T> liveQuery) where T : ParseObject
+    {
+        if (requestId <= 0) throw new ArgumentOutOfRangeException(nameof(requestId), "Request ID must be greater than zero.");
+        if (liveQuery is null) throw new ArgumentNullException(nameof(liveQuery));
+        return await AppendSessionToken(new Dictionary<string, object>
+        {
+            { "op", op },
+            { "requestId", requestId },
+            { "query", liveQuery.BuildParameters() }
+        });
+    }
+
     public async Task<IDictionary<string, object>> BuildSubscribeMessage<T>(int requestId, ParseLiveQuery<T> liveQuery) where T : ParseObject
-    {
-        if (requestId <= 0)
-            throw new ArgumentOutOfRangeException(nameof(requestId), "Request ID must be greater than zero.");
-
-        if (liveQuery is null)
-            throw new ArgumentNullException(nameof(liveQuery));
-
-        return await AppendSessionToken(new Dictionary<string, object>
-        {
-            { "op", "subscribe" },
-            { "requestId", requestId },
-            { "query", liveQuery.BuildParameters() }
-        });
-    }
+        => await BuildSubscriptionMessageCore("subscribe", requestId, liveQuery);
 
     public async Task<IDictionary<string, object>> BuildUpdateSubscriptionMessage<T>(int requestId, ParseLiveQuery<T> liveQuery) where T : ParseObject
-    {
-        if (requestId <= 0)
-            throw new ArgumentOutOfRangeException(nameof(requestId), "Request ID must be greater than zero.");
-
-        if (liveQuery is null)
-            throw new ArgumentNullException(nameof(liveQuery));
-
-        return await AppendSessionToken(new Dictionary<string, object>
-        {
-            { "op", "update" },
-            { "requestId", requestId },
-            { "query", liveQuery.BuildParameters() }
-        });
-    }
+        => await BuildSubscriptionMessageCore("update", requestId, liveQuery);

Also applies to: 51-65

Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (2)

54-61: Fix XML doc: not a tuple, return LiveQueryError struct.

Docs should reflect the actual API to avoid confusion for consumers.

-    /// <returns>
-    /// A tuple containing the error code, error message, and a boolean indicating whether to reconnect.
-    /// </returns>
+    /// <returns>
+    /// A LiveQueryError containing the error code, message, and a boolean indicating whether to reconnect.
+    /// </returns>

12-24: Make LiveQueryError a readonly struct to enforce immutability and reduce defensive copies.

-    struct LiveQueryError
+    readonly struct LiveQueryError
     {
         public int Code { get; }
         public string Message { get; }
         public bool Reconnect { get; }
 
         public LiveQueryError(int code, string message, bool reconnect)
         {
             Code = code;
             Message = message;
             Reconnect = reconnect;
         }
     }
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (7)

328-336: Remove dead code: private AppendSessionToken is unused.

It diverges in semantics from the builder’s version and invites future drift.

-    private async Task<IDictionary<string, object>> AppendSessionToken(IDictionary<string, object> message)
-    {
-        string sessionToken = await ParseClient.Instance.Services.GetCurrentSessionToken();
-        return sessionToken is null
-            ? message
-            : message.Concat(new Dictionary<string, object> {
-                { "sessionToken", sessionToken }
-            }).ToDictionary();
-    }

341-354: Null‑safe services access in OpenAsync to avoid NRE when Instance is not set.

-        if (ParseClient.Instance.Services == null)
+        if (ParseClient.Instance?.Services == null)
         {
             throw new InvalidOperationException("ParseClient.Services must be initialized before connecting to the LiveQuery server.");
         }
 
-        if (ParseClient.Instance.Services.LiveQueryServerConnectionData == null)
+        if (ParseClient.Instance.Services.LiveQueryServerConnectionData == null)
         {
             throw new InvalidOperationException("ParseClient.Services.LiveQueryServerConnectionData must be initialized before connecting to the LiveQuery server.");
         }

456-471: Dispose linked CTS in SendAndWaitForSignalAsync.

Minor resource leak; mirror the pattern used during connect.

-            CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
-            cts.CancelAfter(Timeout);
-
-            await tcs.Task.WaitAsync(cts.Token);
+            using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
+            cts.CancelAfter(Timeout);
+            await tcs.Task.WaitAsync(cts.Token);

36-49: Update doc: Timeout is a TimeSpan, not “in milliseconds.”

-    /// Gets or sets the timeout duration, in milliseconds, used by the ParseLiveQueryController
+    /// Gets or sets the timeout duration used by the ParseLiveQueryController (as a TimeSpan)

109-109: Make Subscriptions read‑only (remove setter).

No external reassignment needed; reduces accidental overwrites.

-    private ConcurrentDictionary<int, IParseLiveQuerySubscription> Subscriptions { get; set; } = new ConcurrentDictionary<int, IParseLiveQuerySubscription>();
+    private ConcurrentDictionary<int, IParseLiveQuerySubscription> Subscriptions { get; } = new ConcurrentDictionary<int, IParseLiveQuerySubscription>();

387-388: Prefer RunContinuationsAsynchronously on TaskCompletionSource to avoid inline continuations on event thread.

Reduces risk of reentrancy and subtle deadlocks.

-        ConnectionSignal = new TaskCompletionSource();
+        ConnectionSignal = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
...
-        TaskCompletionSource tcs = new TaskCompletionSource();
+        TaskCompletionSource tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);

Also applies to: 461-463


280-297: Simplify error processing: rely on MessageParser.GetError and handle exceptions once.

Removes duplicated field extraction and single‑sources validation.

-        if (!(message.TryGetValue("code", out object codeObj) &&
-            Int32.TryParse(codeObj?.ToString(), out int code)))
-            return;
-
-        if (!(message.TryGetValue("error", out object errorObj) &&
-              errorObj is string error))
-            return;
-
-        if (!(message.TryGetValue("reconnect", out object reconnectObj) &&
-            Boolean.TryParse(reconnectObj?.ToString(), out bool reconnect)))
-            return;
-
-        var liveQueryError = MessageParser.GetError(message);
-
-        Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(liveQueryError.Code, liveQueryError.Message, liveQueryError.Reconnect));
+        try
+        {
+            var e = MessageParser.GetError(message);
+            Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(e.Code, e.Message, e.Reconnect));
+        }
+        catch (ArgumentException ex)
+        {
+            Debug.WriteLine($"Invalid error message payload: {ex.Message}");
+            Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(31, $"Failed to parse error: {ex.Message}", true, ex));
+        }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea60936 and 8341e12.

📒 Files selected for processing (10)
  • Parse.Tests/LiveQueryMessageBuilderTests.cs (1 hunks)
  • Parse.Tests/LiveQueryMessageParserTests.cs (1 hunks)
  • Parse.Tests/LiveQuerySubscriptionTests.cs (1 hunks)
  • Parse.Tests/LiveQueryTests.cs (1 hunks)
  • Parse.Tests/TextWebSocketClientTests.cs (1 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1 hunks)
  • Parse/Infrastructure/ServiceHub.cs (4 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • Parse.Tests/TextWebSocketClientTests.cs
  • Parse/Infrastructure/ServiceHub.cs
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs
  • Parse.Tests/LiveQuerySubscriptionTests.cs
  • Parse.Tests/LiveQueryTests.cs
  • Parse.Tests/LiveQueryMessageParserTests.cs
🧰 Additional context used
🧬 Code graph analysis (4)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (2)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (10)
  • Task (328-336)
  • Task (338-339)
  • Task (341-354)
  • Task (377-409)
  • Task (423-438)
  • Task (456-481)
  • Task (505-519)
  • Task (540-545)
  • Task (562-568)
  • Task (579-598)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (4)
  • Task (14-14)
  • Task (21-21)
  • Task (28-28)
  • IDictionary (34-34)
Parse.Tests/LiveQueryMessageBuilderTests.cs (5)
Parse/Infrastructure/MutableServiceHub.cs (2)
  • MutableServiceHub (36-124)
  • MutableServiceHub (75-123)
Parse/Platform/ParseClient.cs (1)
  • Publicize (170-176)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (10)
  • Task (328-336)
  • Task (338-339)
  • Task (341-354)
  • Task (377-409)
  • Task (423-438)
  • Task (456-481)
  • Task (505-519)
  • Task (540-545)
  • Task (562-568)
  • Task (579-598)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (6)
  • Task (10-20)
  • Task (22-33)
  • Task (35-49)
  • Task (51-65)
  • ParseLiveQueryMessageBuilder (8-78)
  • IDictionary (67-77)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (34-34)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (4)
  • IParseLiveQueryMessageParser (68-83)
  • GetClientId (19-28)
  • IDictionary (41-50)
  • GetRequestId (30-39)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (7)
Parse/Infrastructure/Utilities/JsonUtilities.cs (2)
  • Parse (332-372)
  • JsonUtilities (16-508)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (6)
  • IParseLiveQueryMessageParser (68-83)
  • IDictionary (41-50)
  • GetClientId (19-28)
  • GetRequestId (30-39)
  • IObjectState (52-58)
  • IObjectState (60-66)
Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs (2)
  • ParseLiveQueryErrorEventArgs (8-65)
  • ParseLiveQueryErrorEventArgs (59-64)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (67-77)
Parse/Platform/ParseClient.cs (7)
  • IDictionary (185-196)
  • IDictionary (198-201)
  • ParseClient (19-220)
  • ParseClient (58-58)
  • ParseClient (66-98)
  • ParseClient (107-159)
  • ParseClient (164-164)
Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (9)
  • OnDelete (143-144)
  • OnLeave (132-135)
  • OnUpdate (120-123)
  • OnEnter (109-112)
  • OnCreate (100-101)
  • Task (80-81)
  • Task (90-91)
  • ParseLiveQuerySubscription (14-145)
  • ParseLiveQuerySubscription (58-65)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (6)
  • Task (60-79)
  • Task (89-95)
  • Task (97-155)
  • Task (201-209)
  • Dispose (214-218)
  • Dispose (220-238)

Copy link

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

♻️ Duplicate comments (1)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)

10-20: Potential race condition when modifying the input dictionary

The AppendSessionToken method directly modifies the input dictionary, which could cause issues if the same dictionary is reused elsewhere or accessed concurrently.

Consider creating a defensive copy to avoid side effects:

 private async Task<IDictionary<string, object>> AppendSessionToken(IDictionary<string, object> message)
 {
     if (message is null)
         throw new ArgumentNullException(nameof(message));
 
     string sessionToken = await ParseClient.Instance.Services.GetCurrentSessionToken();
     if (sessionToken is not null)
-        message.Add("sessionToken", sessionToken);
+    {
+        var messageCopy = new Dictionary<string, object>(message);
+        messageCopy.Add("sessionToken", sessionToken);
+        return messageCopy;
+    }
 
     return message;
 }
🧹 Nitpick comments (5)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1)

15-27: Consider making LiveQueryError a record instead of a struct

Since LiveQueryError is immutable and used primarily for data transfer, consider using a record for better equality semantics and concise syntax.

-    readonly struct LiveQueryError
-    {
-        public int Code { get; }
-        public string Message { get; }
-        public bool Reconnect { get; }
-
-        public LiveQueryError(int code, string message, bool reconnect)
-        {
-            Code = code;
-            Message = message;
-            Reconnect = reconnect;
-        }
-    }
+    readonly record struct LiveQueryError(int Code, string Message, bool Reconnect);
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (4)

180-191: Consider returning false instead of 0 for invalid requestId

The method returns requestId = 0 when validation fails, but 0 could be a valid request ID in some contexts. Consider using a nullable int or out parameter with a clear invalid state.

-private bool ValidateClientMessage(IDictionary<string, object> message, out int requestId)
+private bool ValidateClientMessage(IDictionary<string, object> message, out int? requestId)
 {
     ClientId = MessageParser.GetClientId(message);
     if (ClientId is null)
     {
-        requestId = 0;
+        requestId = null;
         return false;
     }
 
-    requestId = MessageParser.GetRequestId(message);
-    return requestId != 0;
+    try 
+    {
+        requestId = MessageParser.GetRequestId(message);
+        return true;
+    }
+    catch
+    {
+        requestId = null;
+        return false;
+    }
 }

Then update the calling methods to handle nullable int:

-if (!ValidateClientMessage(message, out int requestId))
+if (!ValidateClientMessage(message, out int? requestId) || !requestId.HasValue)
     return;

193-206: Add try-catch for message parsing to prevent crashes

The MessageParser.GetObjectState() method could throw exceptions based on invalid message content. Consider adding error handling to prevent crashes.

 private void ProcessDeleteEventMessage(IDictionary<string, object> message)
 {
     if (!ValidateClientMessage(message, out int requestId))
         return;
 
-    IObjectState objectState = MessageParser.GetObjectState(message);
-    if (objectState is null)
-        return;
+    IObjectState objectState;
+    try
+    {
+        objectState = MessageParser.GetObjectState(message);
+    }
+    catch (Exception ex)
+    {
+        Debug.WriteLine($"Failed to parse object state: {ex.Message}");
+        return;
+    }
 
     if (!Subscriptions.TryGetValue(requestId, out IParseLiveQuerySubscription subscription))
             return;
 
     subscription.OnDelete(objectState);
 }

Apply similar error handling to all event processing methods: ProcessLeaveEventMessage, ProcessUpdateEventMessage, ProcessEnterEventMessage, and ProcessCreateEventMessage.


280-284: Add error handling for message parsing

The MessageParser.GetError() method could throw exceptions. Consider adding try-catch to handle parsing errors gracefully.

 void ProcessErrorMessage(IDictionary<string, object> message)
 {
-    var liveQueryError = MessageParser.GetError(message);
-    Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(liveQueryError.Code, liveQueryError.Message, liveQueryError.Reconnect));
+    try
+    {
+        var liveQueryError = MessageParser.GetError(message);
+        Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(liveQueryError.Code, liveQueryError.Message, liveQueryError.Reconnect));
+    }
+    catch (Exception ex)
+    {
+        Debug.WriteLine($"Failed to parse error message: {ex.Message}");
+        Error?.Invoke(this, new ParseLiveQueryErrorEventArgs(-1, "Failed to parse error message", false, ex));
+    }
 }

615-634: Consider avoiding fire-and-forget in Dispose

The current implementation uses Task.Run for async disposal in the synchronous Dispose method, which could lead to unobserved exceptions and incomplete cleanup if the process terminates.

Consider using a synchronous cleanup approach or blocking with proper timeout:

 private void Dispose(bool disposing)
 {
     if (disposed)
         return;
     if (disposing)
     {
-        Task.Run(async () =>
+        try
         {
-            try
-            {
-                await DisposeAsync();
-            }
-            catch (Exception ex)
-            {
-                Debug.WriteLine($"Error during async disposal: {ex}");
-            }
-        });
+            using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));
+            CloseAsync(cts.Token).ConfigureAwait(false).GetAwaiter().GetResult();
+        }
+        catch (Exception ex)
+        {
+            Debug.WriteLine($"Error during disposal: {ex}");
+        }
     }
     disposed = true;
 }

This ensures cleanup completes before the object is considered disposed, with a reasonable timeout to prevent indefinite blocking.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8341e12 and a608262.

📒 Files selected for processing (5)
  • Parse.Tests/LiveQueryMessageBuilderTests.cs (1 hunks)
  • Parse.Tests/LiveQueryMessageParserTests.cs (1 hunks)
  • Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryController.cs (1 hunks)
  • Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • Parse.Tests/LiveQueryMessageBuilderTests.cs
  • Parse.Tests/LiveQueryMessageParserTests.cs
🧰 Additional context used
🧬 Code graph analysis (3)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageParser.cs (1)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (4)
  • IParseLiveQueryMessageParser (68-83)
  • GetClientId (19-28)
  • IDictionary (41-50)
  • GetRequestId (30-39)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (3)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (9)
  • Task (315-316)
  • Task (318-331)
  • Task (354-387)
  • Task (401-416)
  • Task (434-459)
  • Task (483-497)
  • Task (518-523)
  • Task (540-546)
  • Task (557-576)
Parse/Abstractions/Platform/LiveQueries/IParseLiveQueryMessageBuilder.cs (4)
  • Task (14-14)
  • Task (21-21)
  • Task (28-28)
  • IDictionary (34-34)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (1)
  • IDictionary (41-50)
Parse/Platform/LiveQueries/ParseLiveQueryController.cs (7)
Parse/Infrastructure/Utilities/JsonUtilities.cs (2)
  • Parse (332-372)
  • JsonUtilities (16-508)
Parse/Platform/LiveQueries/ParseLiveQueryMessageParser.cs (6)
  • IParseLiveQueryMessageParser (68-83)
  • IDictionary (41-50)
  • GetClientId (19-28)
  • GetRequestId (30-39)
  • IObjectState (52-58)
  • IObjectState (60-66)
Parse/Platform/LiveQueries/ParseLiveQueryErrorEventArgs.cs (2)
  • ParseLiveQueryErrorEventArgs (8-65)
  • ParseLiveQueryErrorEventArgs (59-64)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)
  • IDictionary (57-67)
Parse/Platform/ParseClient.cs (7)
  • IDictionary (185-196)
  • IDictionary (198-201)
  • ParseClient (19-220)
  • ParseClient (58-58)
  • ParseClient (66-98)
  • ParseClient (107-159)
  • ParseClient (164-164)
Parse/Platform/LiveQueries/ParseLiveQuerySubscription.cs (9)
  • OnDelete (143-144)
  • OnLeave (132-135)
  • OnUpdate (120-123)
  • OnEnter (109-112)
  • OnCreate (100-101)
  • Task (80-81)
  • Task (90-91)
  • ParseLiveQuerySubscription (14-145)
  • ParseLiveQuerySubscription (58-65)
Parse/Infrastructure/Execution/TextWebSocketClient.cs (6)
  • Task (60-79)
  • Task (89-95)
  • Task (97-155)
  • Task (201-209)
  • Dispose (214-218)
  • Dispose (220-238)
🔇 Additional comments (7)
Parse/Platform/LiveQueries/ParseLiveQueryMessageBuilder.cs (1)

22-33: Good defensive programming with null checks!

The implementation properly validates that LiveQueryServerConnectionData is configured before attempting to access its properties, throwing descriptive exceptions when not configured.

Parse/Platform/LiveQueries/ParseLiveQueryController.cs (6)

127-134: LGTM! Proper parameter validation in constructor

The constructor correctly validates all required parameters with null checks and appropriate exceptions.


354-387: Good connection establishment with proper cleanup!

The EstablishConnectionAsync method properly handles timeout scenarios with cleanup and state management. The use of linked cancellation tokens and proper disposal in the finally block is excellent.


401-416: Good handling of concurrent connection attempts!

The ConnectAsync method correctly handles different connection states and avoids race conditions by capturing the ConnectionSignal in a local variable.


434-459: Well-structured signal handling with proper cleanup!

The SendAndWaitForSignalAsync helper method effectively reduces code duplication and properly manages resources with timeout handling and cleanup in the finally block.


491-491: Thread-safe request ID generation!

Good use of Interlocked.Increment to ensure thread-safe generation of unique request IDs.


557-576: Excellent cleanup implementation!

The CloseAsync method properly:

  • Removes event handlers before closing the WebSocket
  • Cancels all pending signals to prevent hangs
  • Clears all collections

@mtrezza
Copy link
Member

mtrezza commented Sep 19, 2025

Started CI

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.

3 participants