From 038ffdbd90d2c63a2e9abb32b2d5cc6eb7c0d093 Mon Sep 17 00:00:00 2001 From: kpawelczak <42094017+kpawelczak@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:26:49 +0200 Subject: [PATCH] feat: update optimized ssr engine tests (#17728) --- .../optimized-ssr-engine.spec.ts | 289 ++++++++++++++---- 1 file changed, 235 insertions(+), 54 deletions(-) diff --git a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts index ac97df28ba5..754946a8665 100644 --- a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts +++ b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts @@ -8,9 +8,9 @@ import { NgExpressEngineInstance } from '../engine-decorator/ng-express-engine-d import { ExpressServerLogger, ExpressServerLoggerContext } from '../logger'; import { OptimizedSsrEngine, SsrCallbackFn } from './optimized-ssr-engine'; import { + defaultSsrOptimizationOptions, RenderingStrategy, SsrOptimizationOptions, - defaultSsrOptimizationOptions, } from './ssr-optimization-options'; const defaultRenderTime = 100; @@ -38,8 +38,8 @@ class MockExpressServerLogger implements Partial { * 3. Examine renders property for the renders */ class TestEngineRunner { - /** Accumulates html output for engine runs */ - renders: string[] = []; + /** Accumulates responses produced by engine runs (either successful html or Error) */ + responses: (string | Error)[] = []; /** Accumulates response parameters for engine runs */ responseParams: object[] = []; @@ -48,7 +48,11 @@ class TestEngineRunner { optimizedSsrEngine: OptimizedSsrEngine; engineInstance: NgExpressEngineInstance; - constructor(options: SsrOptimizationOptions, renderTime?: number) { + constructor( + options: SsrOptimizationOptions, + renderTime?: number, + renderError?: boolean // SPIKE NEW + ) { // mocked engine instance that will render test output in 100 milliseconds const engineInstanceMock = ( filePath: string, @@ -56,7 +60,14 @@ class TestEngineRunner { callback: SsrCallbackFn ) => { setTimeout(() => { - callback(undefined, `${filePath}-${this.renderCount++}`); + const result = `${filePath}-${this.renderCount++}`; + + if (renderError) { + const err = new Error(result); + callback(err); + } else { + callback(undefined, result); + } }, renderTime ?? defaultRenderTime); }; @@ -96,14 +107,14 @@ class TestEngineRunner { app, connection: >{}, res: >{ - set: (key: string, value: any) => (response[key] = value), locals: {}, + set: (key: string, value: any) => (response[key] = value), }, }, }; - this.engineInstance(url, optionsMock, (_, html): void => { - this.renders.push(html ?? ''); + this.engineInstance(url, optionsMock, (err, html): void => { + this.responses.push(err ?? html ?? ''); // SPIKE TODO: change to `?? undefined` this.responseParams.push(response); }); @@ -165,35 +176,35 @@ describe('OptimizedSsrEngine', () => { const engineRunner = new TestEngineRunner({ timeout: 50 }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); })); it('should return timed out render in the followup request', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 50 }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); engineRunner.request('a'); - expect(engineRunner.renders[1]).toEqual('a-0'); + expect(engineRunner.responses[1]).toEqual('a-0'); })); it('should return render if rendering meets timeout', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 150 }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); + expect(engineRunner.responses).toEqual(['a-0']); })); it('should fallback instantly if is set to 0', () => { const engineRunner = new TestEngineRunner({ timeout: 0 }).request('a'); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); }); it('should return timed out render in the followup request, also when timeout is set to 0', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 0 }).request('a'); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 1, }); @@ -204,18 +215,43 @@ describe('OptimizedSsrEngine', () => { }); engineRunner.request('a'); - expect(engineRunner.renders[1]).toEqual('a-0'); + expect(engineRunner.responses[1]).toEqual('a-0'); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 0, }); })); + + it('should fallback to CSR if rendering request fails', fakeAsync(() => { + const engineRunner = new TestEngineRunner( + { timeout: 50 }, + defaultRenderTime, + true + ).request('b'); + + tick(200); + expect(engineRunner.responses).toEqual(['']); + })); + + it('should return timed out render if followup request fails', fakeAsync(() => { + const engineRunner = new TestEngineRunner( + { timeout: 50 }, + defaultRenderTime, + true + ).request('b'); + + tick(200); + expect(engineRunner.responses).toEqual(['']); + + engineRunner.request('b'); + expect(engineRunner.responses[1]).toEqual(new Error('b-0')); + })); }); describe('no-store cache control header', () => { it('should be applied for a fallback', () => { const engineRunner = new TestEngineRunner({ timeout: 0 }).request('a'); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); expect(engineRunner.responseParams).toEqual([ { 'Cache-Control': 'no-store' }, ]); @@ -225,7 +261,7 @@ describe('OptimizedSsrEngine', () => { const engineRunner = new TestEngineRunner({ timeout: 200 }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); + expect(engineRunner.responses).toEqual(['a-0']); expect(engineRunner.responseParams).toEqual([{}]); })); it('should not be applied for a render served with next response', fakeAsync(() => { @@ -233,7 +269,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); expect(engineRunner.responseParams).toEqual([ { 'Cache-Control': 'no-store' }, {}, @@ -253,7 +289,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-1', 'a-2']); + expect(engineRunner.responses).toEqual(['a-0', 'a-1', 'a-2']); })); it('should cache requests if enabled', fakeAsync(() => { @@ -281,7 +317,7 @@ describe('OptimizedSsrEngine', () => { tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-0', 'a-0']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0', 'a-0']); })); }); @@ -298,7 +334,7 @@ describe('OptimizedSsrEngine', () => { .request('e'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ '', // CSR fallback for 'd' '', // CSR fallback for 'e' 'a-0', @@ -321,7 +357,7 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('f').request('g'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ '', // CSR fallback for 'c' 'a-0', '', // CSR fallback for 'e' @@ -334,7 +370,7 @@ describe('OptimizedSsrEngine', () => { }); describe('ttl option', () => { - it('should invalidate expired renders', fakeAsync(() => { + it('should invalidate expired responses', fakeAsync(() => { let currentDate = 100; jest.spyOn(Date, 'now').mockImplementation(() => currentDate); @@ -353,7 +389,7 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-0', 'a-1']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0', 'a-1']); })); }); @@ -418,7 +454,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('elu'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ 'ala-0', 'ala-0', 'ela-1', @@ -426,8 +462,23 @@ describe('OptimizedSsrEngine', () => { 'ela-1', ]); })); - }); + it('should apply no-store cache control header for a timed-out render when followup request fails', fakeAsync(() => { + const engineRunner = new TestEngineRunner( + { timeout: 50 }, + defaultRenderTime, + true + ).request('c'); + + tick(200); + engineRunner.request('c'); + expect(engineRunner.responses).toEqual(['', new Error('c-0')]); + expect(engineRunner.responseParams).toEqual([ + { 'Cache-Control': 'no-store' }, + {}, + ]); + })); + }); describe('renderingStrategyResolver option', () => { describe('ALWAYS_SSR', () => { it('should ignore timeout', fakeAsync(() => { @@ -438,7 +489,7 @@ describe('OptimizedSsrEngine', () => { }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); + expect(engineRunner.responses).toEqual(['a-0']); })); it('should ignore timeout also when it is set to 0', fakeAsync(() => { @@ -451,7 +502,7 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); + expect(engineRunner.responses).toEqual(['a-0']); })); it('when reuseCurrentRendering is false, it should render each request separately, even if there is already a pending render for the same rendering key', fakeAsync(() => { @@ -475,10 +526,10 @@ describe('OptimizedSsrEngine', () => { expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 2, }); - expect(engineRunner.renders).toEqual([]); + expect(engineRunner.responses).toEqual([]); tick(100); - expect(engineRunner.renders).toEqual(['a-0', 'a-1']); + expect(engineRunner.responses).toEqual(['a-0', 'a-1']); expect( engineRunner.optimizedSsrEngine['expressEngine'] ).toHaveBeenCalledTimes(2); @@ -501,7 +552,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['', '']); + expect(engineRunner.responses).toEqual(['', '']); })); it('should not start the actual render in the background', fakeAsync(() => { @@ -516,7 +567,7 @@ describe('OptimizedSsrEngine', () => { ); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); expect( engineRunner.optimizedSsrEngine['expressEngine'] @@ -535,7 +586,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); })); it('when reuseCurrentRendering is false, it should fallback to CSR when there is already pending a render for the same rendering key', fakeAsync(() => { @@ -554,10 +605,10 @@ describe('OptimizedSsrEngine', () => { currentConcurrency: 1, }); - expect(engineRunner.renders).toEqual(['']); // immediate fallback to CSR for the 2nd request for the same key + expect(engineRunner.responses).toEqual(['']); // immediate fallback to CSR for the 2nd request for the same key tick(100); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 0, }); @@ -578,9 +629,42 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a', { httpHeaders: { 'User-Agent': 'bot' } }); tick(200); - expect(engineRunner.renders).toEqual(['', 'a-1']); + expect(engineRunner.responses).toEqual(['', 'a-1']); })); }); + + it('should render fallback to CSR when rendering request fails', fakeAsync(() => { + const engineRunner = new TestEngineRunner( + { + renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR, + timeout: 50, + }, + defaultRenderTime, + true + ); + engineRunner.request('a'); + tick(200); + expect(engineRunner.responses).toEqual([new Error('a-0')]); + })); + + it('should render fallback to CSR when followup request fails', fakeAsync(() => { + const engineRunner = new TestEngineRunner( + { + renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR, + timeout: 50, + }, + defaultRenderTime, + true + ); + engineRunner.request('a'); + tick(200); + engineRunner.request('a'); + tick(200); + expect(engineRunner.responses).toEqual([ + new Error('a-0'), + new Error('a-1'), + ]); + })); }); describe('forcedSsrTimeout option', () => { @@ -597,15 +681,15 @@ describe('OptimizedSsrEngine', () => { }); tick(60); - expect(engineRunner.renders).toEqual([]); + expect(engineRunner.responses).toEqual([]); tick(50); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 0, }); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 0, }); @@ -620,11 +704,44 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a'); tick(60); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); tick(50); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); + })); + + it('should fallback to CSR when forcedSsrTimeout is exceeded for a request', fakeAsync(() => { + const engineRunner = new TestEngineRunner( + { + renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR, + timeout: 50, + forcedSsrTimeout: 150, + }, + defaultRenderTime, + true + ); + engineRunner.request('a'); + tick(200); + expect(engineRunner.responses).toEqual([new Error('a-0')]); + flush(); + })); + + it('should return timed out render in the followup request when forcedSsrTimeout is exceeded', fakeAsync(() => { + const engineRunner = new TestEngineRunner( + { + renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR, + timeout: 50, + forcedSsrTimeout: 80, + }, + defaultRenderTime, + true + ); + + engineRunner.request('a'); + tick(200); + engineRunner.request('a'); + expect(engineRunner.responses).toEqual(['', new Error('a-0')]); })); }); describe('maxRenderTime option', () => { @@ -756,7 +873,7 @@ describe('OptimizedSsrEngine', () => { renderTime ).request(requestUrl); jest.spyOn(engineRunner.optimizedSsrEngine as any, 'log'); - expect(engineRunner.renders).toEqual([]); + expect(engineRunner.responses).toEqual([]); tick(fiveMinutes + 101); expect(engineRunner.optimizedSsrEngine['log']).toHaveBeenCalledWith( @@ -764,10 +881,10 @@ describe('OptimizedSsrEngine', () => { false, { request: expect.objectContaining({ originalUrl: requestUrl }) } ); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); engineRunner.request(requestUrl); - expect(engineRunner.renders).toEqual(['']); // if the result was cached, the 2nd request would get immediately 'a-0' + expect(engineRunner.responses).toEqual(['']); // if the result was cached, the 2nd request would get immediately 'a-0' flush(); })); }); @@ -822,7 +939,7 @@ describe('OptimizedSsrEngine', () => { false, { request: expect.objectContaining({ originalUrl: requestUrl }) } ); - expect(engineRunner.renders).toEqual(['', '']); + expect(engineRunner.responses).toEqual(['', '']); flush(); })); @@ -852,7 +969,7 @@ describe('OptimizedSsrEngine', () => { tick(100); expect(engineRunner.renderCount).toEqual(1); - expect(engineRunner.renders).toEqual(['', `${requestUrl}-0`]); + expect(engineRunner.responses).toEqual(['', `${requestUrl}-0`]); flush(); })); @@ -890,7 +1007,7 @@ describe('OptimizedSsrEngine', () => { expect(renderExceedMessageCount).toBe(2); expect(engineRunner.renderCount).toEqual(0); - expect(engineRunner.renders).toEqual(['', '']); + expect(engineRunner.responses).toEqual(['', '']); flush(); })); @@ -926,7 +1043,7 @@ describe('OptimizedSsrEngine', () => { tick(200); expect(engineRunner.renderCount).toEqual(1); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ `${requestUrl}-0`, `${requestUrl}-0`, ]); @@ -976,7 +1093,7 @@ describe('OptimizedSsrEngine', () => { false, { request: expect.objectContaining({ originalUrl: requestUrl }) } ); - expect(engineRunner.renders).toEqual(['']); // the first request fallback to CSR due to timeout + expect(engineRunner.responses).toEqual(['']); // the first request fallback to CSR due to timeout expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 1, }); // the render still continues in the background @@ -984,7 +1101,7 @@ describe('OptimizedSsrEngine', () => { // eventually the render succeeds and 2 remaining requests get the same response: tick(100); expect(engineRunner.renderCount).toEqual(1); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ '', // CSR fallback of the 1st request due to it timed out `${requestUrl}-0`, `${requestUrl}-0`, @@ -1016,7 +1133,7 @@ describe('OptimizedSsrEngine', () => { ).not.toHaveBeenCalledWith( `CSR fallback: Concurrency limit exceeded (1)` ); - expect(engineRunner.renders).toEqual(['a-0', 'a-0']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0']); })); it('combined with a different request should take up two concurrency slots', fakeAsync(() => { @@ -1051,7 +1168,7 @@ describe('OptimizedSsrEngine', () => { }); tick(250); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ 'a-0', 'a-0', 'a-0', @@ -1168,13 +1285,77 @@ describe('OptimizedSsrEngine', () => { ); expect(engineRunner.renderCount).toEqual(1); - expect(engineRunner.renders).toEqual(['', '']); + expect(engineRunner.responses).toEqual(['', '']); flush(); })); }); - }); + it('should reuse the same render for subsequent requests for the same rendering key and first request should timeout', fakeAsync(() => { + const timeout = 300; + const engineRunner = new TestEngineRunner( + { timeout, reuseCurrentRendering: true }, + 400, + true + ); + jest.spyOn(engineRunner.optimizedSsrEngine as any, 'log'); + engineRunner.request(requestUrl); + tick(200); + + engineRunner.request(requestUrl); + + tick(100); + expect(engineRunner.optimizedSsrEngine['log']).toHaveBeenCalledWith( + `SSR rendering exceeded timeout ${timeout}, fallbacking to CSR for ${requestUrl}`, + false, + { request: expect.objectContaining({ originalUrl: requestUrl }) } + ); + + tick(100); + expect(engineRunner.renderCount).toEqual(1); + expect(engineRunner.responses).toEqual(['', new Error('a-0')]); + flush(); + })); + + it('should reuse the same render for subsequent requests for the same rendering key when the rendering strategy is ALWAYS_SSR', fakeAsync(() => { + const timeout = 300; + const engineRunner = new TestEngineRunner( + { + timeout, + reuseCurrentRendering: true, + renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR, + }, + 400, + true + ); + + engineRunner.request(requestUrl); + expect(getCurrentConcurrency(engineRunner)).toEqual({ + currentConcurrency: 1, + }); + expect(getRenderCallbacksCount(engineRunner, requestUrl)).toEqual({ + renderCallbacksCount: 1, + }); + + tick(200); + engineRunner.request(requestUrl); + expect(getCurrentConcurrency(engineRunner)).toEqual({ + currentConcurrency: 1, + }); + expect(getRenderCallbacksCount(engineRunner, requestUrl)).toEqual({ + renderCallbacksCount: 2, + }); + + tick(200); + + expect(engineRunner.renderCount).toEqual(1); + expect(engineRunner.responses).toEqual([ + new Error('a-0'), + new Error('a-0'), + ]); + flush(); + })); + }); describe('logger option', () => { it('should use ExpressServerLogger if logger is true', () => { jest.useFakeTimers().setSystemTime(new Date('2023-05-26'));