From a6a926c9160cecf276da75944ea0b1c85de1ce13 Mon Sep 17 00:00:00 2001 From: PS Date: Thu, 3 Oct 2024 11:12:28 +0200 Subject: [PATCH 01/32] concept --- packages/core/src/event/Dispatcher.ts | 3 +- packages/core/src/event/DispatcherImpl.ts | 67 ++++++++++++++- .../src/event/__tests__/DispatcherSpec.ts | 82 ++++++++++++++++++- 3 files changed, 149 insertions(+), 3 deletions(-) diff --git a/packages/core/src/event/Dispatcher.ts b/packages/core/src/event/Dispatcher.ts index 815ae0d456..99d599b219 100644 --- a/packages/core/src/event/Dispatcher.ts +++ b/packages/core/src/event/Dispatcher.ts @@ -116,7 +116,8 @@ export abstract class Dispatcher { fire( event: E, data: DispatcherEventsMap[E], - imaInternalEvent?: boolean + imaInternalEvent?: boolean, + eventType?: string ): this; fire(event: string, data: any, imaInternalEvent?: boolean): this; fire(event: string, data: any, imaInternalEvent?: boolean): this { diff --git a/packages/core/src/event/DispatcherImpl.ts b/packages/core/src/event/DispatcherImpl.ts index d9cd8ede98..891704eef0 100644 --- a/packages/core/src/event/DispatcherImpl.ts +++ b/packages/core/src/event/DispatcherImpl.ts @@ -15,6 +15,12 @@ const EMPTY_MAP: Readonly, Set>> = */ const EMPTY_SET = Object.freeze(new Set()); +export const EVENT_TYPE = { + APP: 'app', + PAGE: 'page', + NONE: 'none', +}; + /** * Default implementation of the {@link Dispatcher} interface. */ @@ -23,6 +29,13 @@ export class DispatcherImpl extends Dispatcher { string, Map, Set> >; + protected _firedEvents: Map< + string, + { + eventType: string; + data: unknown; + } + >; static $dependencies: Dependencies = []; @@ -38,6 +51,7 @@ export class DispatcherImpl extends Dispatcher { * the event. */ this._eventListeners = new Map(); + this._firedEvents = new Map(); } /** @@ -45,6 +59,49 @@ export class DispatcherImpl extends Dispatcher { */ clear(): this { this._eventListeners.clear(); + this._firedEvents.clear(); + + return this; + } + + clearPageEvents(): this { + this._firedEvents.forEach((value, key) => { + if (value.eventType === EVENT_TYPE.PAGE) { + this._firedEvents.delete(key); + } + }); + + return this; + } + + xlisten( + event: string, + listener: DispatcherListener, + scope?: unknown + ): this { + if ($Debug) { + if (typeof listener !== 'function') { + throw new GenericError( + `The listener must be a function, ${listener} provided.` + ); + } + } + + if (!this._eventListeners.has(event)) { + this._createNewEvent(event); + } + + const listeners = this._getListenersOf(event); + + if (!listeners.has(listener)) { + this._createNewListener(event, listener); + } + + this._getScopesOf(event, listener).add(scope); + + if (this._firedEvents.has(event)) { + listener.bind(scope)(this._firedEvents.get(event)?.data); + } return this; } @@ -123,7 +180,12 @@ export class DispatcherImpl extends Dispatcher { /** * @inheritDoc */ - fire(event: string, data: any, imaInternalEvent = false): this { + fire( + event: string, + data: any, + imaInternalEvent = false, + eventType = EVENT_TYPE.NONE + ): this { const listeners = this._getListenersOf(event); if (!listeners?.size && !imaInternalEvent) { @@ -142,6 +204,9 @@ export class DispatcherImpl extends Dispatcher { } } + eventType !== EVENT_TYPE.NONE && + this._firedEvents.set(event, { eventType, data }); + return this; } diff --git a/packages/core/src/event/__tests__/DispatcherSpec.ts b/packages/core/src/event/__tests__/DispatcherSpec.ts index c61fe89f90..08ff0841bd 100644 --- a/packages/core/src/event/__tests__/DispatcherSpec.ts +++ b/packages/core/src/event/__tests__/DispatcherSpec.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ /* eslint-disable @typescript-eslint/no-non-null-assertion */ -import { DispatcherImpl } from '../DispatcherImpl'; +import { DispatcherImpl, EVENT_TYPE } from '../DispatcherImpl'; describe('ima.core.event.DispatcherImpl', () => { const handlers = { @@ -185,4 +185,84 @@ describe('ima.core.event.DispatcherImpl', () => { expect(dispatcher['_eventListeners'].size).toBe(0); }); }); + + describe('brekeke', () => { + it('should work', () => { + jest.restoreAllMocks(); + + const event = 'prefired'; + dispatcher.fire(event, { mrkev: true }, false, EVENT_TYPE.PAGE); + + const listener = jest.fn(); + + // original listen => wont catch events fired before listen + dispatcher.listen(event, listener); + expect(listener).not.toHaveBeenCalled(); + + // xlisten => will catch events fired before listen + dispatcher.xlisten(event, listener); + expect(listener).toHaveBeenCalledWith({ mrkev: true }); + + // same event fired again => works normal + dispatcher.fire(event, { chleba: false }); + expect(listener).toHaveBeenCalledWith({ chleba: false }); + + // xlisten wont create new listener, if listen was already called + expect(dispatcher['_eventListeners'].size).toBe(1); + + dispatcher.clear(); + }); + + it('should work too', () => { + jest.restoreAllMocks(); + + const event = 'prefired'; + dispatcher.fire(event, { mrkev: true }); + + const listener = jest.fn(); + + // normal event have no history + dispatcher.xlisten(event, listener); + expect(listener).not.toHaveBeenCalled(); + + dispatcher.clear(); + + dispatcher.fire(event, { mrkev: true }, false, EVENT_TYPE.PAGE); + + // page event will be fired + dispatcher.xlisten(event, listener); + expect(listener).toHaveBeenCalledWith({ mrkev: true }); + + dispatcher.clearPageEvents(); + dispatcher.unlisten(event, listener); + listener.mockClear(); + expect(dispatcher['_eventListeners'].size).toBe(0); + + // page event was cleared + dispatcher.xlisten(event, listener); + expect(listener).not.toHaveBeenCalled(); + }); + + it('should work three', () => { + jest.restoreAllMocks(); + + const event = 'prefired'; + const listener = jest.fn(); + + dispatcher.fire(event, { mrkev: true }, false, EVENT_TYPE.APP); + + // app event will be fired + dispatcher.xlisten(event, listener); + expect(listener).toHaveBeenCalledWith({ mrkev: true }); + + dispatcher.clearPageEvents(); + dispatcher.unlisten(event, listener); + listener.mockClear(); + expect(dispatcher['_eventListeners'].size).toBe(0); + + // app event will be cleared only using dispatcher.clear() + dispatcher.xlisten(event, listener); + expect(listener).toHaveBeenCalledWith({ mrkev: true }); + }); + }); }); From 8ed8c79bf7be2cb140893c1996e398973165b672 Mon Sep 17 00:00:00 2001 From: PS Date: Wed, 9 Oct 2024 13:38:40 +0200 Subject: [PATCH 02/32] observable --- packages/core/src/event/Dispatcher.ts | 50 ++++++ packages/core/src/event/DispatcherImpl.ts | 103 ++++++------ packages/core/src/event/Observable.ts | 91 +++++++++++ .../src/event/__tests__/DispatcherSpec.ts | 152 +++++++++++------- .../src/event/__tests__/ObservableSpec.ts | 94 +++++++++++ 5 files changed, 383 insertions(+), 107 deletions(-) create mode 100644 packages/core/src/event/Observable.ts create mode 100644 packages/core/src/event/__tests__/ObservableSpec.ts diff --git a/packages/core/src/event/Dispatcher.ts b/packages/core/src/event/Dispatcher.ts index 99d599b219..0068873fac 100644 --- a/packages/core/src/event/Dispatcher.ts +++ b/packages/core/src/event/Dispatcher.ts @@ -10,6 +10,10 @@ export interface DispatcherEventsMap RouterDispatcherEvents, PageRendererDispatcherEvents {} export type DispatcherListener = (data: D) => void; +export type DispatcherListenerAll = ( + event: keyof DispatcherEventsMap | string, + data: D +) => void; /** * A Dispatcher is a utility that manager event listeners registered for events @@ -64,6 +68,32 @@ export abstract class Dispatcher { return this; } + /** + * Registers the provided event listener to be executed when the specified + * event is fired on this dispatcher. + * + * When the specified event is fired, the event listener will be executed + * with the data passed with the event as the first argument. + * + * The order in which the event listeners will be executed is unspecified + * and should not be relied upon. Registering the same listener for the + * same event and with the same scope multiple times has no effect. + * + * @param event The name of the event to listen for. + * @param listener The event listener to register. + * @param scope The object to which the `this` keyword + * will be bound in the event listener. + * @return This dispatcher. + */ + listenAll( + listener: DispatcherListenerAll, + scope?: unknown + ): this; + listenAll(listener: DispatcherListenerAll, scope?: unknown): this; + listenAll(listener: DispatcherListenerAll, scope?: unknown): this { + return this; + } + /** * Deregisters the provided event listener, so it will no longer be * executed with the specified scope when the specified event is fired. @@ -93,6 +123,26 @@ export abstract class Dispatcher { return this; } + /** + * Deregisters the provided event listener, so it will no longer be + * executed with the specified scope when the specified event is fired. + * + * @param event The name of the event for which the listener + * should be deregistered. + * @param listener The event listener to deregister. + * @param scope The object to which the `this` keyword + * would be bound in the event listener. + * @return This dispatcher. + */ + unlistenAll( + listener: DispatcherListenerAll, + scope?: unknown + ): this; + unlistenAll(listener: DispatcherListenerAll, scope?: unknown): this; + unlistenAll(listener: DispatcherListenerAll, scope?: unknown): this { + return this; + } + /** * Fires a new event of the specified name, carrying the provided data. * diff --git a/packages/core/src/event/DispatcherImpl.ts b/packages/core/src/event/DispatcherImpl.ts index 891704eef0..8c28449961 100644 --- a/packages/core/src/event/DispatcherImpl.ts +++ b/packages/core/src/event/DispatcherImpl.ts @@ -1,4 +1,9 @@ -import { Dispatcher, DispatcherListener } from './Dispatcher'; +import { + Dispatcher, + DispatcherEventsMap, + DispatcherListener, + DispatcherListenerAll, +} from './Dispatcher'; import { GenericError } from '../error/GenericError'; import { Dependencies } from '../oc/ObjectContainer'; @@ -15,12 +20,6 @@ const EMPTY_MAP: Readonly, Set>> = */ const EMPTY_SET = Object.freeze(new Set()); -export const EVENT_TYPE = { - APP: 'app', - PAGE: 'page', - NONE: 'none', -}; - /** * Default implementation of the {@link Dispatcher} interface. */ @@ -29,13 +28,8 @@ export class DispatcherImpl extends Dispatcher { string, Map, Set> >; - protected _firedEvents: Map< - string, - { - eventType: string; - data: unknown; - } - >; + + protected _eventListenersAll: Map, Set>; static $dependencies: Dependencies = []; @@ -51,7 +45,7 @@ export class DispatcherImpl extends Dispatcher { * the event. */ this._eventListeners = new Map(); - this._firedEvents = new Map(); + this._eventListenersAll = new Map(); } /** @@ -59,22 +53,14 @@ export class DispatcherImpl extends Dispatcher { */ clear(): this { this._eventListeners.clear(); - this._firedEvents.clear(); - - return this; - } - - clearPageEvents(): this { - this._firedEvents.forEach((value, key) => { - if (value.eventType === EVENT_TYPE.PAGE) { - this._firedEvents.delete(key); - } - }); return this; } - xlisten( + /** + * @inheritDoc + */ + listen( event: string, listener: DispatcherListener, scope?: unknown @@ -99,21 +85,13 @@ export class DispatcherImpl extends Dispatcher { this._getScopesOf(event, listener).add(scope); - if (this._firedEvents.has(event)) { - listener.bind(scope)(this._firedEvents.get(event)?.data); - } - return this; } /** * @inheritDoc */ - listen( - event: string, - listener: DispatcherListener, - scope?: unknown - ): this { + listenAll(listener: DispatcherListenerAll, scope?: unknown) { if ($Debug) { if (typeof listener !== 'function') { throw new GenericError( @@ -122,17 +100,13 @@ export class DispatcherImpl extends Dispatcher { } } - if (!this._eventListeners.has(event)) { - this._createNewEvent(event); - } + if (!this._eventListenersAll.has(listener)) { + const scopes = new Set(); - const listeners = this._getListenersOf(event); - - if (!listeners.has(listener)) { - this._createNewListener(event, listener); + this._eventListenersAll.set(listener, scopes); } - this._getScopesOf(event, listener).add(scope); + this._eventListenersAll.get(listener)?.add(scope); return this; } @@ -177,14 +151,40 @@ export class DispatcherImpl extends Dispatcher { return this; } + unlistenAll(listener: DispatcherListenerAll, scope?: unknown) { + const scopes = this._eventListenersAll.get(listener) ?? EMPTY_SET; + + if ($Debug) { + if (!scopes.has(scope)) { + console.warn( + 'ima.core.event.DispatcherImpl.unlistenAll(): the provided ' + + `listener '${listener}' is not registered for` + + `scope '${scope}'. Check ` + + `your workflow.`, + { + listener: listener, + scope: scope, + } + ); + } + } + + scopes.delete(scope); + + if (!scopes.size) { + this._eventListenersAll.delete(listener); + } + + return this; + } + /** * @inheritDoc */ - fire( - event: string, + fire( + event: E, data: any, - imaInternalEvent = false, - eventType = EVENT_TYPE.NONE + imaInternalEvent = false ): this { const listeners = this._getListenersOf(event); @@ -204,8 +204,11 @@ export class DispatcherImpl extends Dispatcher { } } - eventType !== EVENT_TYPE.NONE && - this._firedEvents.set(event, { eventType, data }); + for (const [listener, scopes] of this._eventListenersAll.entries()) { + for (const scope of scopes) { + listener.bind(scope)(event, data); + } + } return this; } diff --git a/packages/core/src/event/Observable.ts b/packages/core/src/event/Observable.ts new file mode 100644 index 0000000000..47f311c5a5 --- /dev/null +++ b/packages/core/src/event/Observable.ts @@ -0,0 +1,91 @@ +import { + Dispatcher, + DispatcherEventsMap, + DispatcherListener, +} from './Dispatcher'; +import { Dependencies } from '../oc/ObjectContainer'; + +export class Observable { + protected _dispatcher: Dispatcher; + protected _observers: Map, Set>>; + protected _activityHistory: Map; + + static $dependencies: Dependencies = ['$Dispatcher']; + + constructor(dispatcher: Dispatcher) { + this._dispatcher = dispatcher; + this._observers = new Map(); + this._activityHistory = new Map(); + } + + init() { + this._dispatcher.listenAll(this._handleDispatcherEvent, this); + } + + destroy() { + this._dispatcher.unlistenAll(this._handleDispatcherEvent, this); + } + + clear() { + this._observers.clear(); + this._activityHistory.clear(); + } + + subscribe( + event: keyof DispatcherEventsMap | string, + observer: DispatcherListener, + scope?: unknown + ) { + if (!this._observers.has(event)) { + this._observers.set(event, new Map()); + } + + if (!this._observers.get(event)!.has(observer)) { + this._observers.get(event)!.set(observer, new Set()); + } + + this._observers.get(event)!.get(observer)!.add(scope); + + if (this._activityHistory.has(event)) { + observer.bind(scope)(this._activityHistory.get(event)); + } + + return this; + } + + unsubscribe( + event: keyof DispatcherEventsMap | string, + observer: DispatcherListener, + scope?: unknown + ) { + if (!this._observers.has(event)) { + return; + } + + if (!this._observers.get(event)!.has(observer)) { + return; + } + + this._observers.get(event)!.get(observer)!.delete(scope); + + if (this._observers.get(event)!.get(observer)!.size === 0) { + this._observers.get(event)!.delete(observer); + } + + return this; + } + + _handleDispatcherEvent(event: string, data: any) { + this._activityHistory.set(event, data); + + if (!this._observers.has(event)) { + return; + } + + for (const [observer, scopes] of this._observers.get(event)!.entries()) { + for (const scope of scopes) { + observer.bind(scope)(data); + } + } + } +} diff --git a/packages/core/src/event/__tests__/DispatcherSpec.ts b/packages/core/src/event/__tests__/DispatcherSpec.ts index 08ff0841bd..d14e6a8d07 100644 --- a/packages/core/src/event/__tests__/DispatcherSpec.ts +++ b/packages/core/src/event/__tests__/DispatcherSpec.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ /* eslint-disable @typescript-eslint/no-non-null-assertion */ -import { DispatcherImpl, EVENT_TYPE } from '../DispatcherImpl'; +import { DispatcherImpl } from '../DispatcherImpl'; describe('ima.core.event.DispatcherImpl', () => { const handlers = { @@ -172,6 +172,29 @@ describe('ima.core.event.DispatcherImpl', () => { expect(console.warn).not.toHaveBeenCalled(); }); + + it('should fire event for all handlers', () => { + const handler1Spy = jest.spyOn(handlers, 'handler1'); + const handler2Spy = jest.spyOn(handlers, 'handler2'); + + //Instance of mocked Jest function !== Function, wrapper is needed => https://github.com/facebook/jest/issues/6329 + const handler1SpyWrapper = (...args: unknown[]) => { + // @ts-ignore + return handler1Spy(...args); + }; + const handler2SpyWrapper = (...args: unknown[]) => { + // @ts-ignore + return handler2Spy(...args); + }; + + dispatcher.listenAll(handler1SpyWrapper); + dispatcher.listenAll(handler2SpyWrapper); + + dispatcher.fire(event, data); + + expect(handler1Spy).toHaveBeenCalledWith(data); + expect(handler2Spy).toHaveBeenCalledWith(data); + }); }); describe('clear method', () => { @@ -186,83 +209,98 @@ describe('ima.core.event.DispatcherImpl', () => { }); }); - describe('brekeke', () => { - it('should work', () => { - jest.restoreAllMocks(); - - const event = 'prefired'; - dispatcher.fire(event, { mrkev: true }, false, EVENT_TYPE.PAGE); - - const listener = jest.fn(); + describe('listenAll method', () => { + it('should add handler', () => { + dispatcher.listenAll(handlers.handler1); + dispatcher.listenAll(handlers.handler2); - // original listen => wont catch events fired before listen - dispatcher.listen(event, listener); - expect(listener).not.toHaveBeenCalled(); - - // xlisten => will catch events fired before listen - dispatcher.xlisten(event, listener); - expect(listener).toHaveBeenCalledWith({ mrkev: true }); + expect(dispatcher['_eventListenersAll'].size).toBe(2); + expect( + dispatcher['_eventListenersAll'].get(handlers.handler1)!.size + ).toBe(1); + expect( + dispatcher['_eventListenersAll'].get(handlers.handler2)!.size + ).toBe(1); + }); - // same event fired again => works normal - dispatcher.fire(event, { chleba: false }); - expect(listener).toHaveBeenCalledWith({ chleba: false }); + it('should add handler with their scope', () => { + dispatcher.listenAll(handlers.handler1, handlers); + dispatcher.listenAll(handlers.handler2, handlers); + expect(dispatcher['_eventListenersAll'].size).toBe(2); + expect( + dispatcher['_eventListenersAll'].get(handlers.handler1)!.size + ).toBe(1); + expect( + dispatcher['_eventListenersAll'].get(handlers.handler2)!.size + ).toBe(1); + }); - // xlisten wont create new listener, if listen was already called - expect(dispatcher['_eventListeners'].size).toBe(1); + it('should throw error if handler isnt function', () => { + expect(() => { + // @ts-ignore + dispatcher.listenAll('string'); + }).toThrow(); + expect(() => { + // @ts-ignore + dispatcher.listenAll(1); + }).toThrow(); + expect(() => { + // @ts-ignore + dispatcher.listenAll({}); + }).toThrow(); + }); + }); + describe('unlistenAll method', () => { + beforeEach(() => { dispatcher.clear(); }); - it('should work too', () => { - jest.restoreAllMocks(); - - const event = 'prefired'; - dispatcher.fire(event, { mrkev: true }); + it('should remove handler for event', () => { + dispatcher.listenAll(handlers.handler1); + dispatcher.listenAll(handlers.handler2); - const listener = jest.fn(); + dispatcher.unlistenAll(handlers.handler1); - // normal event have no history - dispatcher.xlisten(event, listener); - expect(listener).not.toHaveBeenCalled(); + expect(dispatcher['_eventListenersAll'].size).toBe(1); + }); - dispatcher.clear(); + it('should remove handler with their scope', () => { + dispatcher.listenAll(handlers.handler1, handlers); + dispatcher.listenAll(handlers.handler2, handlers); - dispatcher.fire(event, { mrkev: true }, false, EVENT_TYPE.PAGE); + dispatcher.unlistenAll(handlers.handler1, handlers); - // page event will be fired - dispatcher.xlisten(event, listener); - expect(listener).toHaveBeenCalledWith({ mrkev: true }); + expect(dispatcher['_eventListenersAll'].size).toBe(1); + }); - dispatcher.clearPageEvents(); - dispatcher.unlisten(event, listener); - listener.mockClear(); - expect(dispatcher['_eventListeners'].size).toBe(0); + it('should remove handler with their scope, if scope is not changing', () => { + dispatcher.listenAll(handlers.handler1, handlers); + dispatcher.unlistenAll(handlers.handler1, handlers); - // page event was cleared - dispatcher.xlisten(event, listener); - expect(listener).not.toHaveBeenCalled(); + expect(dispatcher['_eventListenersAll'].size).toBe(0); }); - it('should work three', () => { - jest.restoreAllMocks(); + it('should remove handler with their scope, if scope is changing', () => { + dispatcher.listenAll(handlers.handler1, handlers); + + // @ts-ignore + handlers.handler3 = () => { + return; + }; - const event = 'prefired'; - const listener = jest.fn(); + dispatcher.unlistenAll(handlers.handler1, handlers); - dispatcher.fire(event, { mrkev: true }, false, EVENT_TYPE.APP); + expect(dispatcher['_eventListenersAll'].size).toBe(0); + }); - // app event will be fired - dispatcher.xlisten(event, listener); - expect(listener).toHaveBeenCalledWith({ mrkev: true }); + it('should show warning for undefined listener', () => { + jest.spyOn(console, 'warn').mockImplementation(); - dispatcher.clearPageEvents(); - dispatcher.unlisten(event, listener); - listener.mockClear(); - expect(dispatcher['_eventListeners'].size).toBe(0); + dispatcher.listen(event, handlers.handler1); + dispatcher.unlisten(event, handlers.handler2); - // app event will be cleared only using dispatcher.clear() - dispatcher.xlisten(event, listener); - expect(listener).toHaveBeenCalledWith({ mrkev: true }); + expect(console.warn).toHaveBeenCalled(); }); }); }); diff --git a/packages/core/src/event/__tests__/ObservableSpec.ts b/packages/core/src/event/__tests__/ObservableSpec.ts new file mode 100644 index 0000000000..23b08b8f89 --- /dev/null +++ b/packages/core/src/event/__tests__/ObservableSpec.ts @@ -0,0 +1,94 @@ +/* eslint-disable @typescript-eslint/ban-ts-comment */ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ + +import { DispatcherImpl } from '../DispatcherImpl'; +import { Observable } from '../Observable'; + +describe('ima.core.event.Observable', () => { + const dispatcher = new DispatcherImpl(); + const observable = new Observable(dispatcher); + const event = 'dispatcher.event'; + const eventData = { foo: 'bar' }; + + beforeEach(() => { + dispatcher.clear(); + observable.clear(); + }); + + describe('init method', () => { + it('should listen to all dispatcher events', () => { + expect(dispatcher['_eventListenersAll'].size).toBe(0); + + observable.init(); + + expect(dispatcher['_eventListenersAll'].size).toBe(1); + }); + }); + + describe('subscribe method', () => { + it('should subscribe to event', () => { + expect(observable['_observers'].size).toBe(0); + + const observer = jest.fn(); + observable.subscribe(event, observer); + + expect(observable['_observers'].size).toBe(1); + + dispatcher.fire(event, eventData); + + expect(observer).toHaveBeenCalledWith(eventData); + }); + + it('should subscribe to event and get data if already fired', () => { + dispatcher.fire(event, eventData); + const observer = jest.fn(); + observable.subscribe(event, observer); + + expect(observer).toHaveBeenCalledWith(eventData); + + dispatcher.fire(event, { 1: 2 }); + + expect(observer).toHaveBeenCalledWith({ 1: 2 }); + }); + + it('should work with scope', () => { + class Foo { + foo = jest.fn(); + + bar() { + this.foo(); + } + } + + const foo = new Foo(); + + observable.subscribe(event, foo.bar); + + expect(() => dispatcher.fire(event, eventData)).toThrow( + "Cannot read properties of undefined (reading 'foo')" + ); + + observable.unsubscribe(event, foo.bar); + observable.subscribe(event, foo.bar, foo); + + expect(foo.foo).toHaveBeenCalled(); + }); + }); + + describe('unsubscribe method', () => { + it('should unsubscribe from event', () => { + const observer = jest.fn(); + observable.subscribe(event, observer); + dispatcher.fire(event, eventData); + + expect(observer).toHaveBeenCalledWith(eventData); + + observer.mockClear(); + observable.unsubscribe(event, observer); + dispatcher.fire(event, { foor: 'bar' }); + + expect(observer).not.toHaveBeenCalled(); + expect(observable['_observers'].get(event)?.size).toBe(0); + }); + }); +}); From a20c7ddd089c7c7472b105a0aa934fa6695ad5ff Mon Sep 17 00:00:00 2001 From: PS Date: Thu, 10 Oct 2024 11:08:20 +0200 Subject: [PATCH 03/32] reset functionality --- packages/core/src/event/DispatcherImpl.ts | 13 +----------- packages/core/src/event/Observable.ts | 14 +++++++++++++ .../src/event/__tests__/DispatcherSpec.ts | 20 ++----------------- .../src/event/__tests__/ObservableSpec.ts | 17 ++++++++++++++++ 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/packages/core/src/event/DispatcherImpl.ts b/packages/core/src/event/DispatcherImpl.ts index 8c28449961..225986dfb8 100644 --- a/packages/core/src/event/DispatcherImpl.ts +++ b/packages/core/src/event/DispatcherImpl.ts @@ -183,21 +183,10 @@ export class DispatcherImpl extends Dispatcher { */ fire( event: E, - data: any, - imaInternalEvent = false + data: any ): this { const listeners = this._getListenersOf(event); - if (!listeners?.size && !imaInternalEvent) { - console.warn( - `There are no event listeners registered for the ${event} ` + `event`, - { - event: event, - data: data, - } - ); - } - for (const [listener, scopes] of listeners) { for (const scope of scopes) { listener.bind(scope)(data); diff --git a/packages/core/src/event/Observable.ts b/packages/core/src/event/Observable.ts index 47f311c5a5..346c5e230d 100644 --- a/packages/core/src/event/Observable.ts +++ b/packages/core/src/event/Observable.ts @@ -4,11 +4,13 @@ import { DispatcherListener, } from './Dispatcher'; import { Dependencies } from '../oc/ObjectContainer'; +import { RouterEvents } from '../router/RouterEvents'; export class Observable { protected _dispatcher: Dispatcher; protected _observers: Map, Set>>; protected _activityHistory: Map; + protected _pageResetEvents: Set; static $dependencies: Dependencies = ['$Dispatcher']; @@ -16,6 +18,7 @@ export class Observable { this._dispatcher = dispatcher; this._observers = new Map(); this._activityHistory = new Map(); + this._pageResetEvents = new Set(); } init() { @@ -29,6 +32,11 @@ export class Observable { clear() { this._observers.clear(); this._activityHistory.clear(); + this._pageResetEvents.clear(); + } + + registerPageReset(event: keyof DispatcherEventsMap | string) { + this._pageResetEvents.add(event); } subscribe( @@ -76,6 +84,12 @@ export class Observable { } _handleDispatcherEvent(event: string, data: any) { + if (event === RouterEvents.BEFORE_HANDLE_ROUTE) { + for (const pageResetEvent of this._pageResetEvents) { + this._activityHistory.delete(pageResetEvent); + } + } + this._activityHistory.set(event, data); if (!this._observers.has(event)) { diff --git a/packages/core/src/event/__tests__/DispatcherSpec.ts b/packages/core/src/event/__tests__/DispatcherSpec.ts index d14e6a8d07..24e3c47a6f 100644 --- a/packages/core/src/event/__tests__/DispatcherSpec.ts +++ b/packages/core/src/event/__tests__/DispatcherSpec.ts @@ -157,22 +157,6 @@ describe('ima.core.event.DispatcherImpl', () => { expect(handler2Spy).toHaveBeenCalledWith(data); }); - it('should show warning for none listeners', () => { - jest.spyOn(console, 'warn').mockImplementation(); - - dispatcher.fire(event, data); - - expect(console.warn).toHaveBeenCalled(); - }); - - it('should not show warning for $IMA internal event', () => { - jest.spyOn(console, 'warn').mockImplementation(); - - dispatcher.fire(event, data, true); - - expect(console.warn).not.toHaveBeenCalled(); - }); - it('should fire event for all handlers', () => { const handler1Spy = jest.spyOn(handlers, 'handler1'); const handler2Spy = jest.spyOn(handlers, 'handler2'); @@ -192,8 +176,8 @@ describe('ima.core.event.DispatcherImpl', () => { dispatcher.fire(event, data); - expect(handler1Spy).toHaveBeenCalledWith(data); - expect(handler2Spy).toHaveBeenCalledWith(data); + expect(handler1Spy).toHaveBeenCalledWith(event, data); + expect(handler2Spy).toHaveBeenCalledWith(event, data); }); }); diff --git a/packages/core/src/event/__tests__/ObservableSpec.ts b/packages/core/src/event/__tests__/ObservableSpec.ts index 23b08b8f89..e667af6a17 100644 --- a/packages/core/src/event/__tests__/ObservableSpec.ts +++ b/packages/core/src/event/__tests__/ObservableSpec.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ /* eslint-disable @typescript-eslint/no-non-null-assertion */ +import { RouterEvents } from '../../router/RouterEvents'; import { DispatcherImpl } from '../DispatcherImpl'; import { Observable } from '../Observable'; @@ -91,4 +92,20 @@ describe('ima.core.event.Observable', () => { expect(observable['_observers'].get(event)?.size).toBe(0); }); }); + + it('should reset page events', () => { + observable.registerPageReset(event); + dispatcher.fire(event, eventData); + dispatcher.fire(RouterEvents.BEFORE_HANDLE_ROUTE, { bhr: true }); + + const eventObserver = jest.fn(); + observable.subscribe(event, eventObserver); + + expect(eventObserver).not.toHaveBeenCalledWith(eventData); + + const bhrObserver = jest.fn(); + observable.subscribe(RouterEvents.BEFORE_HANDLE_ROUTE, bhrObserver); + + expect(bhrObserver).toHaveBeenCalledWith({ bhr: true }); + }); }); From f6cc85cc240b639087e217a4b69943e584dae648 Mon Sep 17 00:00:00 2001 From: PS Date: Thu, 17 Oct 2024 15:00:44 +0200 Subject: [PATCH 04/32] reworked history --- packages/core/src/event/Observable.ts | 23 ++++++++++++++++--- .../src/event/__tests__/ObservableSpec.ts | 15 ++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/core/src/event/Observable.ts b/packages/core/src/event/Observable.ts index 346c5e230d..cd67204d07 100644 --- a/packages/core/src/event/Observable.ts +++ b/packages/core/src/event/Observable.ts @@ -4,12 +4,13 @@ import { DispatcherListener, } from './Dispatcher'; import { Dependencies } from '../oc/ObjectContainer'; +import { RendererEvents } from '../page/renderer/RendererEvents'; import { RouterEvents } from '../router/RouterEvents'; export class Observable { protected _dispatcher: Dispatcher; protected _observers: Map, Set>>; - protected _activityHistory: Map; + protected _activityHistory: Map; protected _pageResetEvents: Set; static $dependencies: Dependencies = ['$Dispatcher']; @@ -19,6 +20,8 @@ export class Observable { this._observers = new Map(); this._activityHistory = new Map(); this._pageResetEvents = new Set(); + + this.clear(); } init() { @@ -33,6 +36,14 @@ export class Observable { this._observers.clear(); this._activityHistory.clear(); this._pageResetEvents.clear(); + + Object.values(RouterEvents).forEach(event => + this._pageResetEvents.add(event) + ); + + Object.values(RendererEvents).forEach(event => + this._pageResetEvents.add(event) + ); } registerPageReset(event: keyof DispatcherEventsMap | string) { @@ -55,7 +66,9 @@ export class Observable { this._observers.get(event)!.get(observer)!.add(scope); if (this._activityHistory.has(event)) { - observer.bind(scope)(this._activityHistory.get(event)); + this._activityHistory + .get(event)! + .forEach(data => observer.bind(scope)(data)); } return this; @@ -90,7 +103,11 @@ export class Observable { } } - this._activityHistory.set(event, data); + if (!this._activityHistory.has(event)) { + this._activityHistory.set(event, []); + } + + this._activityHistory.get(event)!.push(data); if (!this._observers.has(event)) { return; diff --git a/packages/core/src/event/__tests__/ObservableSpec.ts b/packages/core/src/event/__tests__/ObservableSpec.ts index e667af6a17..c2028cddaf 100644 --- a/packages/core/src/event/__tests__/ObservableSpec.ts +++ b/packages/core/src/event/__tests__/ObservableSpec.ts @@ -52,6 +52,21 @@ describe('ima.core.event.Observable', () => { expect(observer).toHaveBeenCalledWith({ 1: 2 }); }); + it('should subscribe to event and get data if already fired multiple times', () => { + dispatcher.fire(event, eventData); + const observer = jest.fn(); + observable.subscribe(event, observer); + + expect(observer).toHaveBeenCalledWith(eventData); + + dispatcher.fire(event, { 1: 2 }); + dispatcher.fire(event, { 1: 2 }); + dispatcher.fire(event, { 3: 4 }); + + expect(observer).toHaveBeenNthCalledWith(2, { 1: 2 }); + expect(observer).toHaveBeenLastCalledWith({ 3: 4 }); + }); + it('should work with scope', () => { class Foo { foo = jest.fn(); From 399f91f8b96746eaeda972f0bb98fe38027c8dfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0ime=C4=8Dek?= Date: Mon, 21 Oct 2024 14:33:26 +0200 Subject: [PATCH 05/32] Fixed cookie parse for multiple cookies on server --- .changeset/brown-rivers-confess.md | 5 +++++ packages/core/src/storage/CookieStorage.ts | 10 +++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 .changeset/brown-rivers-confess.md diff --git a/.changeset/brown-rivers-confess.md b/.changeset/brown-rivers-confess.md new file mode 100644 index 0000000000..797924bfbe --- /dev/null +++ b/.changeset/brown-rivers-confess.md @@ -0,0 +1,5 @@ +--- +"@ima/core": patch +--- + +Fixed cookie parsing from setCookie header when multiple cookies were sent to server. Previously only the first cookie was parsed while multiple set-cookies could alter the cookie settings diff --git a/packages/core/src/storage/CookieStorage.ts b/packages/core/src/storage/CookieStorage.ts index e0d28ad5d5..aa27bd36fe 100644 --- a/packages/core/src/storage/CookieStorage.ts +++ b/packages/core/src/storage/CookieStorage.ts @@ -266,10 +266,14 @@ export class CookieStorage extends Storage { * header. */ parseFromSetCookieHeader(setCookieHeader: string): void { - const cookie = this.#extractCookie(setCookieHeader); + const cookiesArray = setCookieHeader ? setCookieHeader.split(', ') : []; - if (typeof cookie.name === 'string') { - this.set(cookie.name, cookie.value, cookie.options); + for (const cookie of cookiesArray) { + const cookieItem = this.#extractCookie(cookie); + + if (typeof cookieItem.name === 'string') { + this.set(cookieItem.name, cookieItem.value, cookieItem.options); + } } } From 8759221a282a28ca1721f8c22153656e9c26ce24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0ime=C4=8Dek?= Date: Tue, 22 Oct 2024 14:34:11 +0200 Subject: [PATCH 06/32] Added cookie validity check --- packages/core/src/http/HttpAgentImpl.ts | 12 ++- packages/core/src/storage/CookieStorage.ts | 97 +++++++++++++++++++++- 2 files changed, 102 insertions(+), 7 deletions(-) diff --git a/packages/core/src/http/HttpAgentImpl.ts b/packages/core/src/http/HttpAgentImpl.ts index e980a17ebb..24e2107f83 100644 --- a/packages/core/src/http/HttpAgentImpl.ts +++ b/packages/core/src/http/HttpAgentImpl.ts @@ -246,7 +246,7 @@ export class HttpAgentImpl extends HttpAgent { data?: UnknownParameters, options?: Partial ): Promise> { - const optionsWithDefault = this._prepareOptions(options); + const optionsWithDefault = this._prepareOptions(options, url); if (optionsWithDefault.cache) { const cachedData = this._getCachedData(method, url, data); @@ -433,7 +433,8 @@ export class HttpAgentImpl extends HttpAgent { * internally. */ _prepareOptions( - options: Partial = {} + options: Partial = {}, + url: string ): HttpAgentRequestOptions { const composedOptions = { ...this._defaultRequestOptions, @@ -455,7 +456,7 @@ export class HttpAgentImpl extends HttpAgent { if (composedOptions.fetchOptions?.credentials === 'include') { // mock default browser behavior for server-side (sending cookie with a fetch request) composedOptions.fetchOptions.headers.Cookie = - this._cookie.getCookiesStringForCookieHeader(); + this._cookie.getCookiesStringForCookieHeader(url); } return composedOptions; @@ -500,7 +501,10 @@ export class HttpAgentImpl extends HttpAgent { const receivedCookies = agentResponse.headersRaw.get('set-cookie'); if (receivedCookies) { - this._cookie.parseFromSetCookieHeader(receivedCookies); + this._cookie.parseFromSetCookieHeader( + receivedCookies, + agentResponse.params.url + ); } } } diff --git a/packages/core/src/storage/CookieStorage.ts b/packages/core/src/storage/CookieStorage.ts index aa27bd36fe..2e50652c54 100644 --- a/packages/core/src/storage/CookieStorage.ts +++ b/packages/core/src/storage/CookieStorage.ts @@ -19,6 +19,12 @@ const MAX_EXPIRE_DATE = new Date('Fri, 31 Dec 9999 23:59:59 UTC'); */ const COOKIE_SEPARATOR = '; '; +/** + * Separator used to separate cookie declarations in the `Set-Cookie` HTTP + * header. + */ +const SERVER_COOKIE_SEPARATOR = ', '; + export type CookieOptions = { domain?: string; expires?: Date; @@ -96,6 +102,61 @@ export class CookieStorage extends Storage { return [Window, Request, Response]; } + /** + * Filters invalid cookies based on the provided validate options. + * We try to check validity of the domain based on secure, path and + * domain definitions. + */ + static validateCookieSecurity(cookie: Cookie, url: string): boolean { + const { pathname, hostname, protocol } = new URL(url); + const secure = protocol === 'https:'; + + /** + * Don't allow setting secure cookies without the secure + * defined in the validate options. + */ + if ( + typeof cookie.options.secure === 'boolean' && + secure !== cookie.options.secure + ) { + return false; + } + + /** + * Don't allow setting cookies with a path that doesn't start with + * the path defined in the validate options. + */ + if ( + typeof cookie.options.path === 'string' && + !cookie.options.path.endsWith(pathname) + ) { + return false; + } + + /** + * Final domain check, since this check uses guard clauses, it HAS + * TO BE THE LAST CHECK!. + */ + if (cookie.options.domain) { + const cookieDomain = cookie.options.domain.toLowerCase(); + + // Check subdomain match + if (cookieDomain === hostname) { + return true; + } + + // Check if cookieDomain is a subdomain of hostname + if (cookieDomain.endsWith(hostname)) { + return true; + } + + // Domains don't match + return false; + } + + return true; + } + /** * Initializes the cookie storage. * @@ -238,15 +299,29 @@ export class CookieStorage extends Storage { * Returns all cookies in this storage serialized to a string compatible * with the `Cookie` HTTP header. * + * When `url` is provided, the method validates the cookie security based on + * the `url` and the cookie's domain, path, and secure attributes. + * * @return All cookies in this storage serialized to a string * compatible with the `Cookie` HTTP header. */ - getCookiesStringForCookieHeader(): string { + getCookiesStringForCookieHeader(url?: string): string { const cookieStrings = []; for (const cookieName of this._storage.keys()) { const cookieItem = this._storage.get(cookieName); + // Validate cookie security + if ( + url && + cookieItem && + !CookieStorage.validateCookieSecurity(cookieItem, url) + ) { + // eslint-disable-next-line no-console + console.log('cookie not valid', cookieItem); + continue; + } + cookieStrings.push( this.#generateCookieString(cookieName, cookieItem!.value, {}) ); @@ -258,6 +333,9 @@ export class CookieStorage extends Storage { /** * Parses cookies from the provided `Set-Cookie` HTTP header value. * + * When `url` is provided, the method validates the cookie security based on + * the `url` and the cookie's domain, path, and secure attributes. + * * The parsed cookies will be set to the internal storage, and the current * HTTP response (via the `Set-Cookie` HTTP header) if at the server * side, or the browser (via the `document.cookie` property). @@ -265,12 +343,25 @@ export class CookieStorage extends Storage { * @param setCookieHeader The value of the `Set-Cookie` HTTP * header. */ - parseFromSetCookieHeader(setCookieHeader: string): void { - const cookiesArray = setCookieHeader ? setCookieHeader.split(', ') : []; + parseFromSetCookieHeader(setCookieHeader: string, url?: string): void { + const cookiesArray = setCookieHeader + ? setCookieHeader.split(SERVER_COOKIE_SEPARATOR) + : []; for (const cookie of cookiesArray) { const cookieItem = this.#extractCookie(cookie); + // Validate cookie security + if ( + url && + cookieItem && + !CookieStorage.validateCookieSecurity(cookieItem, url) + ) { + // eslint-disable-next-line no-console + console.log('cookie not valid', cookieItem); + continue; + } + if (typeof cookieItem.name === 'string') { this.set(cookieItem.name, cookieItem.value, cookieItem.options); } From b82b8b516c68131447f34eb9f0747d5c6f2b93d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0ime=C4=8Dek?= Date: Wed, 23 Oct 2024 14:35:29 +0200 Subject: [PATCH 07/32] Added tests and fixed validation --- packages/core/src/storage/CookieStorage.ts | 66 ++-- .../storage/__tests__/CookieStorageSpec.ts | 317 ++++++++++++++++++ 2 files changed, 358 insertions(+), 25 deletions(-) diff --git a/packages/core/src/storage/CookieStorage.ts b/packages/core/src/storage/CookieStorage.ts index 2e50652c54..8236f53791 100644 --- a/packages/core/src/storage/CookieStorage.ts +++ b/packages/core/src/storage/CookieStorage.ts @@ -103,7 +103,7 @@ export class CookieStorage extends Storage { } /** - * Filters invalid cookies based on the provided validate options. + * Filters invalid cookies based on the provided url. * We try to check validity of the domain based on secure, path and * domain definitions. */ @@ -124,34 +124,50 @@ export class CookieStorage extends Storage { /** * Don't allow setting cookies with a path that doesn't start with - * the path defined in the validate options. + * the path defined in the validate options. Root path is always valid. */ if ( typeof cookie.options.path === 'string' && - !cookie.options.path.endsWith(pathname) + cookie.options.path !== '/' ) { - return false; + const pathChunks = pathname.split('/'); + const cookiePathChunks = cookie.options.path.split('/'); + + /** + * Compare the path chunks of the request path and the cookie path. + */ + for (let i = 0; i < cookiePathChunks.length; i++) { + /** + * There are no more path chunks to compare, so the cookie path is a + * prefix of the request path. + */ + if (cookiePathChunks[i] === undefined) { + break; + } + + /** + * The path chunks don't match, the cookie path is not a prefix of + * the request path. + */ + if (pathChunks[i] !== cookiePathChunks[i]) { + return false; + } + } } /** - * Final domain check, since this check uses guard clauses, it HAS - * TO BE THE LAST CHECK!. + * Domain security check, we also check for subdomain match. */ if (cookie.options.domain) { const cookieDomain = cookie.options.domain.toLowerCase(); - - // Check subdomain match - if (cookieDomain === hostname) { - return true; - } - - // Check if cookieDomain is a subdomain of hostname - if (cookieDomain.endsWith(hostname)) { - return true; - } - - // Domains don't match - return false; + const normalizedCookieDomain = cookieDomain.startsWith('.') + ? cookieDomain.slice(1) + : cookieDomain; + + return normalizedCookieDomain === hostname || + hostname.endsWith(normalizedCookieDomain) + ? true + : false; } return true; @@ -311,14 +327,14 @@ export class CookieStorage extends Storage { for (const cookieName of this._storage.keys()) { const cookieItem = this._storage.get(cookieName); - // Validate cookie security + /** + * Skip cookies that are not secure for the provided url. + */ if ( url && cookieItem && !CookieStorage.validateCookieSecurity(cookieItem, url) ) { - // eslint-disable-next-line no-console - console.log('cookie not valid', cookieItem); continue; } @@ -351,14 +367,14 @@ export class CookieStorage extends Storage { for (const cookie of cookiesArray) { const cookieItem = this.#extractCookie(cookie); - // Validate cookie security + /** + * Skip cookies that are not secure for the provided url. + */ if ( url && cookieItem && !CookieStorage.validateCookieSecurity(cookieItem, url) ) { - // eslint-disable-next-line no-console - console.log('cookie not valid', cookieItem); continue; } diff --git a/packages/core/src/storage/__tests__/CookieStorageSpec.ts b/packages/core/src/storage/__tests__/CookieStorageSpec.ts index 3ba0e52a15..794dc8273a 100644 --- a/packages/core/src/storage/__tests__/CookieStorageSpec.ts +++ b/packages/core/src/storage/__tests__/CookieStorageSpec.ts @@ -342,4 +342,321 @@ describe('ima.storage.CookieStorage', () => { expect(cookie['_storage'].size).toBe(2); }); }); + + describe('validateCookieSecurity', () => { + it('should return true for a cookie with matching domain and path', () => { + const cookie = { + value: 'test', + options: { domain: 'example.com', path: '/' }, + }; + const url = 'http://example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + + it('should return true for a cookie with matching subdomain', () => { + const cookie = { + value: 'test', + options: { domain: '.example.com', path: '/' }, + }; + const url = 'http://sub.example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + + it('should return false for a cookie with non-matching domain', () => { + const cookie = { + value: 'test', + options: { domain: 'example.com', path: '/' }, + }; + const url = 'http://otherdomain.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(false); + }); + + it('should return false for a secure cookie on non-https URL', () => { + const cookie = { + value: 'test', + options: { domain: 'example.com', path: '/', secure: true }, + }; + const url = 'http://example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(false); + }); + + it('should return true for a secure cookie on https URL', () => { + const cookie = { + value: 'test', + options: { domain: 'example.com', path: '/', secure: true }, + }; + const url = 'https://example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + + it('should return false for a cookie with non-matching path', () => { + const cookie = { + value: 'test', + options: { domain: 'example.com', path: '/app' }, + }; + const url = 'http://example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(false); + }); + + it('should return true for a cookie with matching path prefix', () => { + const cookie = { + value: 'test', + options: { domain: 'example.com', path: '/app' }, + }; + const url = 'http://example.com/app/subpath'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + + it('should return true for a cookie without domain restriction', () => { + const cookie = { + value: 'test', + options: { path: '/' }, + }; + const url = 'http://example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + + it('should return false for a cookie with ip address domain on non-matching ip', () => { + const cookie = { + value: 'test', + options: { domain: '192.168.1.1', path: '/' }, + }; + const url = 'http://192.168.1.2/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(false); + }); + + it('should return true for a cookie with ip address domain on matching ip', () => { + const cookie = { + value: 'test', + options: { domain: '192.168.1.1', path: '/' }, + }; + const url = 'http://192.168.1.1/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + + describe('options that do not affect validation', () => { + it('should return true regardless of httpOnly flag', () => { + const cookie: Cookie = { + value: 'test', + options: { + domain: 'example.com', + path: '/', + httpOnly: true, + sameSite: 'strict', + }, + }; + const url = 'http://example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + + cookie.options.httpOnly = false; + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + + it('should return true regardless of sameSite value', () => { + const cookie: Cookie = { + value: 'test', + options: { + domain: 'example.com', + path: '/', + sameSite: 'strict', + }, + }; + const url = 'http://example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + + cookie.options.sameSite = 'lax'; + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + + cookie.options.sameSite = 'none'; + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + + it('should return true regardless of partitioned flag', () => { + const cookie = { + value: 'test', + options: { + domain: 'example.com', + path: '/', + partitioned: true, + }, + }; + const url = 'http://example.com/'; + + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + + cookie.options.partitioned = false; + expect(CookieStorage.validateCookieSecurity(cookie, url)).toBe(true); + }); + }); + + describe('complex combinations', () => { + it('should validate multiple security attributes together', () => { + const cookie: Cookie = { + value: 'test', + options: { + domain: 'example.com', + path: '/app', + secure: true, + httpOnly: true, + sameSite: 'strict', + }, + }; + + // Should fail - wrong protocol + expect( + CookieStorage.validateCookieSecurity(cookie, 'http://example.com/app') + ).toBe(false); + + // Should fail - wrong path + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'https://example.com/other' + ) + ).toBe(false); + + // Should fail - wrong domain + expect( + CookieStorage.validateCookieSecurity(cookie, 'https://other.com/app') + ).toBe(false); + + // Should pass - all conditions met + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'https://example.com/app' + ) + ).toBe(true); + }); + + it('should handle subdomain with path and secure combinations', () => { + const cookie: Cookie = { + value: 'test', + options: { + domain: '.example.com', + path: '/api', + secure: true, + httpOnly: true, + sameSite: 'strict', + }, + }; + + // Should fail - not secure + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'http://sub.example.com/api' + ) + ).toBe(false); + + // Should fail - wrong path + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'https://sub.example.com/app' + ) + ).toBe(false); + + // Should pass - subdomain with correct path and protocol + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'https://sub.example.com/api' + ) + ).toBe(true); + + // Should pass - main domain with correct path and protocol + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'https://example.com/api' + ) + ).toBe(true); + }); + + it('should validate path hierarchy correctly', () => { + const cookie = { + value: 'test', + options: { + domain: 'example.com', + path: '/api', + }, + }; + + // Should pass - exact path match + expect( + CookieStorage.validateCookieSecurity(cookie, 'http://example.com/api') + ).toBe(true); + + // Should pass - subpath + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'http://example.com/api/users' + ) + ).toBe(true); + + // Should fail - different path + expect( + CookieStorage.validateCookieSecurity(cookie, 'http://example.com/app') + ).toBe(false); + + // Should fail - partial path match but not at boundary + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'http://example.com/api-docs' + ) + ).toBe(false); + }); + + it('should handle root path with various domains', () => { + const cookie = { + value: 'test', + options: { + domain: '.example.com', + path: '/', + }, + }; + + // Should pass - root domain + expect( + CookieStorage.validateCookieSecurity(cookie, 'http://example.com/') + ).toBe(true); + + // Should pass - subdomain with any path + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'http://sub.example.com/any/path' + ) + ).toBe(true); + + // Should pass - deeper subdomain + expect( + CookieStorage.validateCookieSecurity( + cookie, + 'http://deep.sub.example.com/' + ) + ).toBe(true); + + // Should fail - different domain + expect( + CookieStorage.validateCookieSecurity(cookie, 'http://example.org/') + ).toBe(false); + }); + }); + }); }); From 2f30ad3bd8b51b9a11266f945bf6c6ceda3555b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0ime=C4=8Dek?= Date: Wed, 23 Oct 2024 14:49:16 +0200 Subject: [PATCH 08/32] Fixed tests --- packages/core/src/http/HttpAgentImpl.ts | 4 ++-- .../src/http/__tests__/HttpAgentImplSpec.ts | 8 +++---- packages/core/src/storage/CookieStorage.ts | 22 +++++++++---------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/packages/core/src/http/HttpAgentImpl.ts b/packages/core/src/http/HttpAgentImpl.ts index 24e2107f83..af715f877b 100644 --- a/packages/core/src/http/HttpAgentImpl.ts +++ b/packages/core/src/http/HttpAgentImpl.ts @@ -498,9 +498,9 @@ export class HttpAgentImpl extends HttpAgent { */ _setCookiesFromResponse(agentResponse: HttpAgentResponse): void { if (agentResponse.headersRaw) { - const receivedCookies = agentResponse.headersRaw.get('set-cookie'); + const receivedCookies = agentResponse.headersRaw.getSetCookie(); - if (receivedCookies) { + if (receivedCookies.length > 0) { this._cookie.parseFromSetCookieHeader( receivedCookies, agentResponse.params.url diff --git a/packages/core/src/http/__tests__/HttpAgentImplSpec.ts b/packages/core/src/http/__tests__/HttpAgentImplSpec.ts index 0377a6115e..005a3c2455 100644 --- a/packages/core/src/http/__tests__/HttpAgentImplSpec.ts +++ b/packages/core/src/http/__tests__/HttpAgentImplSpec.ts @@ -71,11 +71,9 @@ describe('ima.core.http.HttpAgentImpl', () => { 'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'], }, // @ts-ignore - headersRaw: new Map( - Object.entries({ - 'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'], - }) - ), + headersRaw: new Headers({ + 'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'], + }), }; }); diff --git a/packages/core/src/storage/CookieStorage.ts b/packages/core/src/storage/CookieStorage.ts index 8236f53791..7889172a58 100644 --- a/packages/core/src/storage/CookieStorage.ts +++ b/packages/core/src/storage/CookieStorage.ts @@ -19,12 +19,6 @@ const MAX_EXPIRE_DATE = new Date('Fri, 31 Dec 9999 23:59:59 UTC'); */ const COOKIE_SEPARATOR = '; '; -/** - * Separator used to separate cookie declarations in the `Set-Cookie` HTTP - * header. - */ -const SERVER_COOKIE_SEPARATOR = ', '; - export type CookieOptions = { domain?: string; expires?: Date; @@ -356,13 +350,17 @@ export class CookieStorage extends Storage { * HTTP response (via the `Set-Cookie` HTTP header) if at the server * side, or the browser (via the `document.cookie` property). * - * @param setCookieHeader The value of the `Set-Cookie` HTTP - * header. + * @param cookiesString The value of the `Set-Cookie` HTTP + * header. When there are multiple cookies, the value can be + * provided as an array of strings. */ - parseFromSetCookieHeader(setCookieHeader: string, url?: string): void { - const cookiesArray = setCookieHeader - ? setCookieHeader.split(SERVER_COOKIE_SEPARATOR) - : []; + parseFromSetCookieHeader( + cookiesString: string | string[], + url?: string + ): void { + const cookiesArray = Array.isArray(cookiesString) + ? cookiesString + : [cookiesString]; for (const cookie of cookiesArray) { const cookieItem = this.#extractCookie(cookie); From 7da74772748c862d14338d992b220fa7e6527375 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0ime=C4=8Dek?= Date: Wed, 23 Oct 2024 16:31:23 +0200 Subject: [PATCH 09/32] Added validation settings --- packages/core/src/http/HttpAgent.ts | 1 + packages/core/src/http/HttpAgentImpl.ts | 8 ++++++-- .../create-ima-app/template/ts/app/config/settings.ts | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/core/src/http/HttpAgent.ts b/packages/core/src/http/HttpAgent.ts index 709196b777..6b67c3fb68 100644 --- a/packages/core/src/http/HttpAgent.ts +++ b/packages/core/src/http/HttpAgent.ts @@ -39,6 +39,7 @@ export interface HttpAgentRequestOptions { ) => HttpAgentResponse)[]; abortController?: AbortController; keepSensitiveHeaders?: boolean; + validateCookies?: boolean; } /** diff --git a/packages/core/src/http/HttpAgentImpl.ts b/packages/core/src/http/HttpAgentImpl.ts index af715f877b..5b6184207b 100644 --- a/packages/core/src/http/HttpAgentImpl.ts +++ b/packages/core/src/http/HttpAgentImpl.ts @@ -456,7 +456,9 @@ export class HttpAgentImpl extends HttpAgent { if (composedOptions.fetchOptions?.credentials === 'include') { // mock default browser behavior for server-side (sending cookie with a fetch request) composedOptions.fetchOptions.headers.Cookie = - this._cookie.getCookiesStringForCookieHeader(url); + this._cookie.getCookiesStringForCookieHeader( + options.validateCookies ? url : undefined + ); } return composedOptions; @@ -503,7 +505,9 @@ export class HttpAgentImpl extends HttpAgent { if (receivedCookies.length > 0) { this._cookie.parseFromSetCookieHeader( receivedCookies, - agentResponse.params.url + this._defaultRequestOptions.validateCookies + ? agentResponse.params.url + : undefined ); } } diff --git a/packages/create-ima-app/template/ts/app/config/settings.ts b/packages/create-ima-app/template/ts/app/config/settings.ts index 48759cda2f..af942f3799 100644 --- a/packages/create-ima-app/template/ts/app/config/settings.ts +++ b/packages/create-ima-app/template/ts/app/config/settings.ts @@ -24,6 +24,7 @@ export const initSettings: InitSettingsFunction = (ns, oc, config) => { 'Accept-Language': config.$Language, }, }, + validateCookies: true, // Validate cookies when parsing from Set-Cookie header and when sending cookies from the server. cache: true, // if value exists in cache then returned it else make request to remote server. }, cacheOptions: { From f5f4be879269141a2bee1f18fe2086ecd056c9d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0ime=C4=8Dek?= Date: Fri, 25 Oct 2024 09:57:33 +0200 Subject: [PATCH 10/32] Added changeset --- .changeset/flat-beds-travel.md | 6 ++++++ packages/create-ima-app/template/ts/app/config/settings.ts | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 .changeset/flat-beds-travel.md diff --git a/.changeset/flat-beds-travel.md b/.changeset/flat-beds-travel.md new file mode 100644 index 0000000000..4167889fae --- /dev/null +++ b/.changeset/flat-beds-travel.md @@ -0,0 +1,6 @@ +--- +"create-ima-app": minor +"@ima/core": minor +--- + +Added new settings `validateCookies` to enable/disable cookie validation. It validates cookie options and request url before saving cookie or sending it to the server. This means that path, subdomain and secure options must match between the request url and the cookie, otherwise the cookie is not saved or sent. diff --git a/packages/create-ima-app/template/ts/app/config/settings.ts b/packages/create-ima-app/template/ts/app/config/settings.ts index af942f3799..6a927f4f34 100644 --- a/packages/create-ima-app/template/ts/app/config/settings.ts +++ b/packages/create-ima-app/template/ts/app/config/settings.ts @@ -24,7 +24,7 @@ export const initSettings: InitSettingsFunction = (ns, oc, config) => { 'Accept-Language': config.$Language, }, }, - validateCookies: true, // Validate cookies when parsing from Set-Cookie header and when sending cookies from the server. + validateCookies: false, // Validate cookies when parsing from Set-Cookie header and when sending cookies from the server. cache: true, // if value exists in cache then returned it else make request to remote server. }, cacheOptions: { From 71344d709f97b037167c508cdc187180f9877950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Sl=C3=A1vik?= Date: Wed, 30 Oct 2024 09:40:09 +0100 Subject: [PATCH 11/32] Add typescript snippets --- ...ng-some-state.md => adding-some-state.mdx} | 16 +++ .../{fetching-data.md => fetching-data.mdx} | 16 +++ .../{final-polish.md => final-polish.mdx} | 16 +++ docs/tutorial/introduction.md | 6 + .../{static-view.md => static-view.mdx} | 121 ++++++++++++++++++ .../{writing-posts.md => writing-posts.mdx} | 16 +++ 6 files changed, 191 insertions(+) rename docs/tutorial/{adding-some-state.md => adding-some-state.mdx} (98%) rename docs/tutorial/{fetching-data.md => fetching-data.mdx} (97%) rename docs/tutorial/{final-polish.md => final-polish.mdx} (99%) rename docs/tutorial/{static-view.md => static-view.mdx} (80%) rename docs/tutorial/{writing-posts.md => writing-posts.mdx} (97%) diff --git a/docs/tutorial/adding-some-state.md b/docs/tutorial/adding-some-state.mdx similarity index 98% rename from docs/tutorial/adding-some-state.md rename to docs/tutorial/adding-some-state.mdx index e00795e53c..78cd4af856 100644 --- a/docs/tutorial/adding-some-state.md +++ b/docs/tutorial/adding-some-state.mdx @@ -3,6 +3,22 @@ title: Adding Some State description: Tutorial > Adding Some State --- +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + + + + +This is an apple 🍎 + + + + +This is an orange 🍊 + + + + In [previous section](/tutorial/static-view) of the tutorial, we prepared basic markup and custom styling thanks to the [Bootstrap CSS library](https://getbootstrap.com/). In this section, we're going to add some functionality to our application. diff --git a/docs/tutorial/fetching-data.md b/docs/tutorial/fetching-data.mdx similarity index 97% rename from docs/tutorial/fetching-data.md rename to docs/tutorial/fetching-data.mdx index 3ed470167f..9d32c3459d 100644 --- a/docs/tutorial/fetching-data.md +++ b/docs/tutorial/fetching-data.mdx @@ -3,6 +3,22 @@ title: Fetching Data description: Tutorial > Fetching Data --- +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + + + + +This is an apple 🍎 + + + + +This is an orange 🍊 + + + + In [last part](./adding-some-state) of this series we tidied up our HomeView component and split our render method into much smaller and manageable components thanks to react. In this part we're going to mock the data fetching from server and learn more about IMA.js object container. diff --git a/docs/tutorial/final-polish.md b/docs/tutorial/final-polish.mdx similarity index 99% rename from docs/tutorial/final-polish.md rename to docs/tutorial/final-polish.mdx index 8a05d5fad4..f2694c447f 100644 --- a/docs/tutorial/final-polish.md +++ b/docs/tutorial/final-polish.mdx @@ -3,6 +3,22 @@ title: Final Polish description: Tutorial > Final Polish --- +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + + + + +This is an apple 🍎 + + + + +This is an orange 🍊 + + + + In the [5th part of the tutorial](/tutorial/writing-posts) we updated our application to be able to process input from user, learned something about ways different components can communicate with each other in IMA.js application and updated our guestbook API. diff --git a/docs/tutorial/introduction.md b/docs/tutorial/introduction.md index 618abd76f7..cf3c599202 100644 --- a/docs/tutorial/introduction.md +++ b/docs/tutorial/introduction.md @@ -38,6 +38,12 @@ To initialize new project, run following command and choose the "Empty" (Hello W ```bash npm2yarn npx create-ima-app imajs-tutorial ``` + +For TypeScript support run the initializing command with following parameters: +```bash npm2yarn +npx create-ima-app imajs-tutorial --typescript +``` + This will bootstrap the IMA.js directory structure and install all dependencies. To learn more information about `create-ima-app` package, take a look at its [github repository](https://github.com/seznam/ima/tree/master/packages/create-ima-app). diff --git a/docs/tutorial/static-view.md b/docs/tutorial/static-view.mdx similarity index 80% rename from docs/tutorial/static-view.md rename to docs/tutorial/static-view.mdx index d261589926..2df92593da 100644 --- a/docs/tutorial/static-view.md +++ b/docs/tutorial/static-view.mdx @@ -3,6 +3,9 @@ title: Static View description: Tutorial > Static View --- +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + In the [first part](./introduction) we went through introduction to IMA.js and initialized our first application using `create-ima-app` command. In the second part of the tutorial we'll actually do some coding and prepare basic Views for our guest book application. @@ -17,6 +20,9 @@ class). You can read more about components and views in the [documentation](../b Now let's replace the contents of the file with a blank view: + + + ```jsx import { PageContext, AbstractComponent } from '@ima/react-page-renderer'; import React from 'react'; @@ -33,6 +39,24 @@ export class HomeView extends AbstractComponent { } ``` + + + +```tsx +import { usePageContext } from '@ima/react-page-renderer'; +import './homeView.less'; + +export function HomeView() { + const _pageContext = usePageContext(); + + return null; +} +``` + + + + + The `HomeView` class defines the `render()` method. Notice that our current `HomeView` class does not have the `constructor()` method, as the default one provided by the `AbstractComponent` class will do in this case. @@ -61,10 +85,23 @@ not have a UI yet. Now that we know our way around our component, let's replace the contents of the `render()` method with the following code: + + + ```jsx return
Hello {'World'}!
; ``` + + + +```tsx +return
Hello {'World'}!
; +``` + + + + The "HTML" code you see is actually an XML markup with JavaScript expressions embedded within curly braces. This markup is processed automatically by Babel's JSX transformer into ordinary JavaScript expressions creating React elements @@ -90,6 +127,9 @@ If everything went well you should see the following page when you refresh your Let's modify the return value of the `render` method to look like this: + + + ```jsx return (
@@ -158,6 +198,87 @@ return ( ); ``` + + + +```tsx +import { usePageContext } from '@ima/react-page-renderer'; +import './homeView.less'; + +export function HomeView() { + const _pageContext = usePageContext(); + + return ( +
+

Guestbook

+ +
+
+
Add a post
+
+
+ + +
+
+ +