From 57940a23aa813e86cffc514a28ffba3f5dcf33c9 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Thu, 4 Apr 2019 08:19:05 +0200 Subject: [PATCH] [Components] Avoid creating two connections when resuming circuits * Fix double connection bug * Fix broken tests * Add test to detect two connections * clean up tests * Fix test bug * Isolate duplicate connection tests --- src/Components/Browser.JS/src/.eslintrc.js | 1 + src/Components/Browser.JS/src/Boot.Server.ts | 39 +++++++++++-------- .../src/Platform/Circuits/RenderQueue.ts | 6 +-- .../ServerExecutionTests/PrerenderingTest.cs | 3 +- .../ServerExecutionTests/ServerSideAppTest.cs | 29 ++++++++++++-- .../PrerenderedToInteractiveTransition.razor | 14 +++++++ .../ComponentsApp.Server/Pages/Index.cshtml | 7 +++- .../TestServer/Pages/PrerenderedHost.cshtml | 9 ++++- .../test/testassets/TestServer/Startup.cs | 2 +- 9 files changed, 82 insertions(+), 28 deletions(-) diff --git a/src/Components/Browser.JS/src/.eslintrc.js b/src/Components/Browser.JS/src/.eslintrc.js index 0d1a71b45d..7bf73a2402 100644 --- a/src/Components/Browser.JS/src/.eslintrc.js +++ b/src/Components/Browser.JS/src/.eslintrc.js @@ -14,6 +14,7 @@ module.exports = { // e.g. "@typescript-eslint/explicit-function-return-type": "off", "@typescript-eslint/indent": ["error", 2], "@typescript-eslint/no-use-before-define": [ "off" ], + "@typescript-eslint/no-unused-vars": ["error", { "varsIgnorePattern": "^_", "argsIgnorePattern": "^_" }], "no-var": "error", "prefer-const": "error", "quotes": ["error", "single", { "avoidEscape": true }], diff --git a/src/Components/Browser.JS/src/Boot.Server.ts b/src/Components/Browser.JS/src/Boot.Server.ts index 04c3e0305d..02f5dc54ae 100644 --- a/src/Components/Browser.JS/src/Boot.Server.ts +++ b/src/Components/Browser.JS/src/Boot.Server.ts @@ -13,26 +13,34 @@ import { discoverPrerenderedCircuits, startCircuit } from './Platform/Circuits/C type SignalRBuilder = (builder: signalR.HubConnectionBuilder) => void; interface BlazorOptions { - configureSignalR?: SignalRBuilder, -}; + configureSignalR: SignalRBuilder; + logLevel: LogLevel; +} let renderingFailed = false; let started = false; -async function boot(options?: BlazorOptions): Promise { +async function boot(userOptions?: Partial): Promise { if (started) { throw new Error('Blazor has already started.'); } started = true; + const defaultOptions: BlazorOptions = { + configureSignalR: (_) => { }, + logLevel: LogLevel.Warning, + }; + + const options: BlazorOptions = { ...defaultOptions, ...userOptions }; + // For development. // Simply put a break point here and modify the log level during // development to get traces. // In the future we will allow for users to configure this. - const logger = new ConsoleLogger(LogLevel.Error); + const logger = new ConsoleLogger(options.logLevel); - logger.log(LogLevel.Information, 'Booting blazor.'); + logger.log(LogLevel.Information, 'Starting up blazor server-side application.'); const circuitHandlers: CircuitHandler[] = [new AutoReconnectCircuitHandler(logger)]; window['Blazor'].circuitHandlers = circuitHandlers; @@ -43,8 +51,7 @@ async function boot(options?: BlazorOptions): Promise { }); // pass options.configureSignalR to configure the signalR.HubConnectionBuilder - const configureSignalR = (options && options.configureSignalR) || null; - const initialConnection = await initializeConnection(configureSignalR, circuitHandlers, logger); + const initialConnection = await initializeConnection(options, circuitHandlers, logger); const circuits = discoverPrerenderedCircuits(document); for (let i = 0; i < circuits.length; i++) { @@ -64,12 +71,12 @@ async function boot(options?: BlazorOptions): Promise { logger.log(LogLevel.Information, 'No preregistered components to render.'); } - const reconnect = async (): Promise => { + const reconnect = async (existingConnection?: signalR.HubConnection): Promise => { if (renderingFailed) { // We can't reconnect after a failure, so exit early. return false; } - const reconnection = await initializeConnection(configureSignalR, circuitHandlers, logger); + const reconnection = existingConnection || await initializeConnection(options, circuitHandlers, logger); const results = await Promise.all(circuits.map(circuit => circuit.reconnect(reconnection))); if (reconnectionFailed(results)) { @@ -82,7 +89,7 @@ async function boot(options?: BlazorOptions): Promise { window['Blazor'].reconnect = reconnect; - const reconnectTask = reconnect(); + const reconnectTask = reconnect(initialConnection); if (circuit) { circuits.push(circuit); @@ -90,29 +97,29 @@ async function boot(options?: BlazorOptions): Promise { await reconnectTask; + logger.log(LogLevel.Information, 'Blazor server-side application started.'); + function reconnectionFailed(results: boolean[]): boolean { return !results.reduce((current, next) => current && next, true); } } -async function initializeConnection(configureSignalR: SignalRBuilder | null, circuitHandlers: CircuitHandler[], logger: ILogger): Promise { +async function initializeConnection(options: Required, circuitHandlers: CircuitHandler[], logger: ILogger): Promise { const hubProtocol = new MessagePackHubProtocol(); - (hubProtocol as any).name = 'blazorpack'; + (hubProtocol as unknown as { name: string }).name = 'blazorpack'; const connectionBuilder = new signalR.HubConnectionBuilder() .withUrl('_blazor') .withHubProtocol(hubProtocol); - if (configureSignalR) { - configureSignalR(connectionBuilder); - } + options.configureSignalR(connectionBuilder); const connection = connectionBuilder.build(); connection.on('JS.BeginInvokeJS', DotNet.jsCallDispatcher.beginInvokeJSFromDotNet); connection.on('JS.RenderBatch', (browserRendererId: number, batchId: number, batchData: Uint8Array) => { - logger.log(LogLevel.Information, `Received render batch for ${browserRendererId} with id ${batchId} and ${batchData.byteLength} bytes.`); + logger.log(LogLevel.Debug, `Received render batch for ${browserRendererId} with id ${batchId} and ${batchData.byteLength} bytes.`); const queue = RenderQueue.getOrCreateQueue(browserRendererId, logger); diff --git a/src/Components/Browser.JS/src/Platform/Circuits/RenderQueue.ts b/src/Components/Browser.JS/src/Platform/Circuits/RenderQueue.ts index be82d8ee71..94f756d850 100644 --- a/src/Components/Browser.JS/src/Platform/Circuits/RenderQueue.ts +++ b/src/Components/Browser.JS/src/Platform/Circuits/RenderQueue.ts @@ -28,18 +28,18 @@ export default class RenderQueue { public processBatch(receivedBatchId: number, batchData: Uint8Array, connection: HubConnection): void { if (receivedBatchId < this.nextBatchId) { - this.logger.log(LogLevel.Information, `Batch ${receivedBatchId} already processed. Waiting for batch ${this.nextBatchId}.`); + this.logger.log(LogLevel.Debug, `Batch ${receivedBatchId} already processed. Waiting for batch ${this.nextBatchId}.`); return; } if (receivedBatchId > this.nextBatchId) { - this.logger.log(LogLevel.Information, `Waiting for batch ${this.nextBatchId}. Batch ${receivedBatchId} not processed.`); + this.logger.log(LogLevel.Debug, `Waiting for batch ${this.nextBatchId}. Batch ${receivedBatchId} not processed.`); return; } try { this.nextBatchId++; - this.logger.log(LogLevel.Information, `Applying batch ${receivedBatchId}.`); + this.logger.log(LogLevel.Debug, `Applying batch ${receivedBatchId}.`); renderBatch(this.browserRendererId, new OutOfProcessRenderBatch(batchData)); this.completeBatch(connection, receivedBatchId); } catch (error) { diff --git a/src/Components/test/E2ETest/ServerExecutionTests/PrerenderingTest.cs b/src/Components/test/E2ETest/ServerExecutionTests/PrerenderingTest.cs index 33c11a1771..be5e996196 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/PrerenderingTest.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/PrerenderingTest.cs @@ -48,8 +48,9 @@ namespace Microsoft.AspNetCore.Components.E2ETests.ServerExecutionTests Browser.Equal("No value yet", () => Browser.FindElement(By.Id("val-get-by-interop")).Text); Browser.Equal(string.Empty, () => Browser.FindElement(By.Id("val-set-by-interop")).GetAttribute("value")); - // Once connected, we can BeginInteractivity(); + + // Once connected, we can Browser.Equal("Hello from interop call", () => Browser.FindElement(By.Id("val-get-by-interop")).Text); Browser.Equal("Hello from interop call", () => Browser.FindElement(By.Id("val-set-by-interop")).GetAttribute("value")); } diff --git a/src/Components/test/E2ETest/ServerExecutionTests/ServerSideAppTest.cs b/src/Components/test/E2ETest/ServerExecutionTests/ServerSideAppTest.cs index 685e0838e5..8127505172 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/ServerSideAppTest.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/ServerSideAppTest.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Components.E2ETest.Infrastructure; @@ -26,14 +28,23 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests _serverFixture.BuildWebHostMethod = ComponentsApp.Server.Program.BuildWebHost; } + public DateTime LastLogTimeStamp { get; set; } = DateTime.MinValue; public override async Task InitializeAsync() { await base.InitializeAsync(); - Navigate("/", noReload: false); - Browser.True(() => Browser.Manage().Logs.GetLog(LogType.Browser) - .Any(l => l.Level == LogLevel.Info && l.Message.Contains("blazorpack"))); + // Capture the last log timestamp so that we can filter logs when we + // check for duplicate connections. + var lastLog = Browser.Manage().Logs.GetLog(LogType.Browser).LastOrDefault(); + if (lastLog != null) + { + LastLogTimeStamp = lastLog.Timestamp; + } + + Navigate("/", noReload: false); + Browser.True(() => ((IJavaScriptExecutor)Browser) + .ExecuteScript("return window['__aspnetcore__testing__blazor__started__'];") == null ? false : true); } [Fact] @@ -42,6 +53,18 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Equal("Razor Components", Browser.Title); } + [Fact] + public void DoesNotStartTwoConnections() + { + Browser.True(() => + { + var logs = Browser.Manage().Logs.GetLog(LogType.Browser).ToArray(); + var curatedLogs = logs.Where(l => l.Timestamp > LastLogTimeStamp); + + return curatedLogs.Count(e => e.Message.Contains("blazorpack")) == 1; + }); + } + [Fact] public void HasHeading() { diff --git a/src/Components/test/testassets/BasicTestApp/PrerenderedToInteractiveTransition.razor b/src/Components/test/testassets/BasicTestApp/PrerenderedToInteractiveTransition.razor index 749b490782..00fee3f53f 100644 --- a/src/Components/test/testassets/BasicTestApp/PrerenderedToInteractiveTransition.razor +++ b/src/Components/test/testassets/BasicTestApp/PrerenderedToInteractiveTransition.razor @@ -17,4 +17,18 @@ @functions { int count; + bool firstRender = false; + protected override Task OnAfterRenderAsync() + { + if (!firstRender) + { + firstRender = true; + + // We need to queue another render when we connect, otherwise the + // browser won't see anything. + StateHasChanged(); + } + + return Task.CompletedTask; + } } diff --git a/src/Components/test/testassets/ComponentsApp.Server/Pages/Index.cshtml b/src/Components/test/testassets/ComponentsApp.Server/Pages/Index.cshtml index 9c4a241fb1..8525e0d3aa 100644 --- a/src/Components/test/testassets/ComponentsApp.Server/Pages/Index.cshtml +++ b/src/Components/test/testassets/ComponentsApp.Server/Pages/Index.cshtml @@ -1,4 +1,4 @@ -@page +@page @using ComponentsApp.App @@ -20,7 +20,10 @@ Blazor.start({ configureSignalR: function (builder) { builder.configureLogging(2); // LogLevel.Information - } + }, + logLevel: 2 // LogLevel.Information + }).then(function () { + window['__aspnetcore__testing__blazor__started__'] = true; }); diff --git a/src/Components/test/testassets/TestServer/Pages/PrerenderedHost.cshtml b/src/Components/test/testassets/TestServer/Pages/PrerenderedHost.cshtml index ba3bf3812a..4d2a6455c2 100644 --- a/src/Components/test/testassets/TestServer/Pages/PrerenderedHost.cshtml +++ b/src/Components/test/testassets/TestServer/Pages/PrerenderedHost.cshtml @@ -1,6 +1,5 @@ @page @using BasicTestApp.RouterTest -@using Microsoft.AspNetCore.Mvc.ViewFeatures @@ -16,7 +15,7 @@ *@
- + diff --git a/src/Components/test/testassets/TestServer/Startup.cs b/src/Components/test/testassets/TestServer/Startup.cs index 86f4acdcc1..ab5c480bcc 100644 --- a/src/Components/test/testassets/TestServer/Startup.cs +++ b/src/Components/test/testassets/TestServer/Startup.cs @@ -79,7 +79,7 @@ namespace TestServer subdirApp.UseEndpoints(endpoints => { endpoints.MapFallbackToPage("/PrerenderedHost"); - endpoints.MapComponentHub("app"); + endpoints.MapComponentHub(); }); }); }