From 9bad3ae21b7285185f6a91b7bdaa7259d5074fbb Mon Sep 17 00:00:00 2001 From: Andrew Welker Date: Wed, 21 Jan 2026 15:20:27 -0600 Subject: [PATCH] fix: handle subsequent join calls and clientid/websocket client mismatches --- .../MobileControlWebsocketServer.cs | 108 ++++++++++++++---- .../WebSocketServer/UiClient.cs | 9 ++ 2 files changed, 94 insertions(+), 23 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index 252d2814..3b55120f 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -69,10 +69,11 @@ namespace PepperDash.Essentials.WebSocketServer private readonly ConcurrentDictionary pendingClientRegistrations = new ConcurrentDictionary(); /// - /// Stores queues of pending client IDs per token for legacy clients (FIFO) - /// This ensures thread-safety when multiple legacy clients use the same token + /// Stores pending client registrations with timestamp for legacy clients + /// Key is token, Value is list of (clientId, timestamp) tuples + /// Most recent registration is used to handle duplicate join requests /// - private readonly ConcurrentDictionary> legacyClientIdQueues = new ConcurrentDictionary>(); + private readonly ConcurrentDictionary> legacyClientRegistrations = new ConcurrentDictionary>(); /// /// Gets the collection of UI clients @@ -736,15 +737,23 @@ namespace PepperDash.Essentials.WebSocketServer private UiClient BuildUiClient(string roomKey, JoinToken token, string key) { - // Dequeue the next clientId for legacy client support (FIFO per token) + // Get the most recent unused clientId for this token (legacy support) // New clients will override this ID in OnOpen with the validated query parameter value var clientId = "pending"; - if (legacyClientIdQueues.TryGetValue(key, out var queue) && queue.TryDequeue(out var dequeuedId)) + if (legacyClientRegistrations.TryGetValue(key, out var registrations)) { - clientId = dequeuedId; - this.LogVerbose("Dequeued legacy clientId {clientId} for token {token}", clientId, key); + // Get most recent registration + var sorted = registrations.OrderByDescending(r => r.timestamp).ToList(); + if (sorted.Any()) + { + clientId = sorted.First().clientId; + // Remove it from the bag + var newBag = new ConcurrentBag<(string, DateTime)>(sorted.Skip(1)); + legacyClientRegistrations.TryUpdate(key, newBag, registrations); + this.LogVerbose("Assigned most recent legacy clientId {clientId} for token {token}", clientId, key); + } } - + var c = new UiClient($"uiclient-{key}-{roomKey}-{clientId}", clientId, token.Token, token.TouchpanelKey); this.LogInformation("Constructing UiClient with key {key} and temporary ID (will be set from query param)", key); c.Controller = _parent; @@ -753,8 +762,8 @@ namespace PepperDash.Essentials.WebSocketServer c.Server = this; // Give UiClient access to server for ID registration // Don't add to uiClients yet - will be added in OnOpen after ID is set from query param - - c.ConnectionClosed += (o, a) => + + c.ConnectionClosed += (o, a) => { uiClients.TryRemove(a.ClientId, out _); // Clean up any pending registrations for this token @@ -765,11 +774,11 @@ namespace PepperDash.Essentials.WebSocketServer { pendingClientRegistrations.TryRemove(k, out _); } - - // Clean up legacy queue if empty - if (legacyClientIdQueues.TryGetValue(key, out var legacyQueue) && legacyQueue.IsEmpty) + + // Clean up legacy registrations if empty + if (legacyClientRegistrations.TryGetValue(key, out var legacyBag) && legacyBag.IsEmpty) { - legacyClientIdQueues.TryRemove(key, out _); + legacyClientRegistrations.TryRemove(key, out _); } }; return c; @@ -785,7 +794,7 @@ namespace PepperDash.Essentials.WebSocketServer public bool RegisterUiClient(UiClient client, string clientId, string tokenKey) { var registrationKey = $"{tokenKey}-{clientId}"; - + // Verify this clientId was generated during a join request for this token if (!pendingClientRegistrations.TryRemove(registrationKey, out _)) { @@ -799,11 +808,63 @@ namespace PepperDash.Essentials.WebSocketServer this.LogWarning("Replacing existing client with duplicate id {id}", id); return client; }); - + this.LogInformation("Successfully registered UiClient with ID {clientId} for token {token}", clientId, tokenKey); return true; } + /// + /// Updates a client's ID when a mismatch is detected between stored ID and message ID + /// + /// The current/old client ID + /// The new client ID from the message + /// The token key for validation + /// True if update successful, false otherwise + public bool UpdateClientId(string oldClientId, string newClientId, string tokenKey) + { + if (string.IsNullOrEmpty(oldClientId) || string.IsNullOrEmpty(newClientId)) + { + this.LogWarning("Cannot update client ID with null or empty values"); + return false; + } + + if (oldClientId == newClientId) + { + return true; // No update needed + } + + // Verify the new clientId was registered for this token + var registrationKey = $"{tokenKey}-{newClientId}"; + if (!pendingClientRegistrations.TryRemove(registrationKey, out _)) + { + this.LogWarning("Cannot update to unregistered clientId {newClientId} for token {token}", newClientId, tokenKey); + return false; + } + + // Get the existing client + if (!uiClients.TryRemove(oldClientId, out var client)) + { + this.LogWarning("Cannot find client with old ID {oldClientId}", oldClientId); + return false; + } + + // Update the client's ID + client.UpdateId(newClientId); + + // Re-add with new ID + if (!uiClients.TryAdd(newClientId, client)) + { + // If add fails, try to restore old entry + uiClients.TryAdd(oldClientId, client); + client.UpdateId(oldClientId); + this.LogError("Failed to update client ID from {oldClientId} to {newClientId}", oldClientId, newClientId); + return false; + } + + this.LogInformation("Successfully updated client ID from {oldClientId} to {newClientId}", oldClientId, newClientId); + return true; + } + /// /// Registers a UiClient using legacy flow (for backwards compatibility with older clients) /// @@ -821,7 +882,7 @@ namespace PepperDash.Essentials.WebSocketServer this.LogWarning("Replacing existing client with duplicate id {id} (legacy flow)", id); return client; }); - + this.LogInformation("Successfully registered UiClient with ID {clientId} using legacy flow", client.Id); } @@ -1133,16 +1194,17 @@ namespace PepperDash.Essentials.WebSocketServer // Generate a client ID for this join request var clientId = $"{Utilities.GetNextClientId()}"; - + var now = DateTime.UtcNow; + // Store in pending registrations for new clients that send clientId via query param var registrationKey = $"{token}-{clientId}"; pendingClientRegistrations.TryAdd(registrationKey, clientId); - - // Also enqueue for legacy clients (thread-safe FIFO per token) - var queue = legacyClientIdQueues.GetOrAdd(token, _ => new ConcurrentQueue()); - queue.Enqueue(clientId); - this.LogVerbose("Assigning ClientId: {clientId} for token: {token}", clientId, token); + // For legacy clients, store with timestamp instead of FIFO queue + var legacyBag = legacyClientRegistrations.GetOrAdd(token, _ => new ConcurrentBag<(string, DateTime)>()); + legacyBag.Add((clientId, now)); + + this.LogVerbose("Assigning ClientId: {clientId} for token: {token} at {timestamp}", clientId, token, now); // Construct WebSocket URL with clientId query parameter var wsProtocol = "ws"; diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs index fd012bbe..607323ba 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs @@ -26,6 +26,15 @@ namespace PepperDash.Essentials.WebSocketServer /// public string Id { get; private set; } + /// + /// Updates the client ID - only accessible from within the assembly (e.g., by the server) + /// + /// The new client ID + internal void UpdateId(string newId) + { + Id = newId; + } + /// /// Token associated with this client ///