Skip to content

Commit a3dfb12

Browse files
jasperboerhofclaude
andcommitted
Replace applyServer* methods with broadcast subscription contract
Addresses review feedback: exposing applyServerUpdate/applyServerDelete as public store methods leaked a mutation surface that invited misuse as an HTTP-bypass shortcut. Drops both public methods. Adds an optional `broadcast` config slot accepting an AdapterStoreBroadcast<T> contract. The store calls `broadcast.subscribe({ onUpdate, onDelete })` exactly once at construction, routing incoming events directly into the internal setById/deleteById. Consumers never see the mutation handlers — they can only bind a broadcast source at store creation time. Channel lifecycle (join/leave per component) stays in the consuming app; the broadcast contract is the narrow bridge between that layer and the store. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6620c71 commit a3dfb12

3 files changed

Lines changed: 134 additions & 48 deletions

File tree

packages/adapter-store/src/adapter-store.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export const createAdapterStoreModule = <
1919
>(
2020
config: AdapterStoreConfig<T, E, N>,
2121
): StoreModuleForAdapter<T, E, N> => {
22-
const { domainName, adapter, httpService, storageService, loadingService } = config;
22+
const { domainName, adapter, httpService, storageService, loadingService, broadcast } = config;
2323

2424
const storedItems = storageService.get<{ [id: number]: T }>(domainName, {});
2525
const frozenStoredItems = Object.fromEntries(
@@ -60,6 +60,8 @@ export const createAdapterStoreModule = <
6060

6161
const storeModule: AdapterStoreModule<T> = { setById, deleteById };
6262

63+
broadcast?.subscribe({ onUpdate: setById, onDelete: deleteById });
64+
6365
const getById = (id: number): ComputedRef<E | undefined> => {
6466
const cached = getByIdComputedCache.get(id);
6567
if (cached) {
@@ -94,7 +96,5 @@ export const createAdapterStoreModule = <
9496
adaptedCache.clear();
9597
getByIdComputedCache.clear();
9698
},
97-
applyServerUpdate: setById,
98-
applyServerDelete: deleteById,
9999
};
100100
};

packages/adapter-store/src/types.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,20 @@ export type Adapter<
5555
(storeModule: AdapterStoreModule<T>, resourceGetter: () => T): E;
5656
};
5757

58+
/**
59+
* Contract for binding server-initiated events (e.g. WebSocket broadcasts)
60+
* to an adapter-store. The store calls `subscribe` once at construction and
61+
* routes incoming events straight into its internal mutation path. The
62+
* handlers are never exposed on the public store API, so consumers cannot
63+
* acquire them to bypass HTTP.
64+
*/
65+
export type AdapterStoreBroadcast<T extends Item> = {
66+
subscribe: (handlers: {
67+
onUpdate: (item: T) => void;
68+
onDelete: (id: number) => void;
69+
}) => () => void;
70+
};
71+
5872
/** Configuration for createAdapterStoreModule. */
5973
export type AdapterStoreConfig<
6074
T extends Item,
@@ -66,6 +80,7 @@ export type AdapterStoreConfig<
6680
httpService: Pick<HttpService, "getRequest">;
6781
storageService: Pick<StorageService, "get" | "put">;
6882
loadingService: Pick<LoadingService, "ensureLoadingFinished">;
83+
broadcast?: AdapterStoreBroadcast<T>;
6984
};
7085

7186
/** Public API of a store module. */
@@ -80,14 +95,4 @@ export type StoreModuleForAdapter<
8095
generateNew: () => N;
8196
retrieveById: (id: number) => Promise<void>;
8297
retrieveAll: () => Promise<void>;
83-
/**
84-
* Apply an externally-sourced item to the store (e.g. a WebSocket broadcast
85-
* pushed by another client). Replaces the item by id without triggering HTTP.
86-
*/
87-
applyServerUpdate: (item: T) => void;
88-
/**
89-
* Apply an externally-sourced deletion to the store (e.g. a WebSocket broadcast
90-
* pushed by another client). Removes the item by id without triggering HTTP.
91-
*/
92-
applyServerDelete: (id: number) => void;
9398
};

packages/adapter-store/tests/adapter-store.spec.ts

Lines changed: 116 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { LoadingService } from "@script-development/fs-loading";
55
import type {
66
Adapted,
77
Adapter,
8+
AdapterStoreBroadcast,
89
AdapterStoreConfig,
910
AdapterStoreModule,
1011
Item,
@@ -823,25 +824,82 @@ describe("createAdapterStoreModule", () => {
823824
});
824825
});
825826

826-
describe("applyServerUpdate", () => {
827-
it("should insert an externally-sourced item into the store without calling http", () => {
827+
describe("broadcast integration", () => {
828+
const captureBroadcast = (): {
829+
broadcast: AdapterStoreBroadcast<TestItem>;
830+
subscribe: ReturnType<typeof vi.fn>;
831+
unsubscribe: ReturnType<typeof vi.fn>;
832+
getHandlers: () => {
833+
onUpdate: (item: TestItem) => void;
834+
onDelete: (id: number) => void;
835+
};
836+
} => {
837+
let handlers: {
838+
onUpdate: (item: TestItem) => void;
839+
onDelete: (id: number) => void;
840+
} | null = null;
841+
const unsubscribe = vi.fn();
842+
const subscribe = vi.fn((h: typeof handlers) => {
843+
handlers = h;
844+
return unsubscribe;
845+
});
846+
return {
847+
broadcast: { subscribe } as AdapterStoreBroadcast<TestItem>,
848+
subscribe,
849+
unsubscribe,
850+
getHandlers: () => {
851+
if (!handlers) throw new Error("subscribe was not called");
852+
return handlers;
853+
},
854+
};
855+
};
856+
857+
it("should call subscribe exactly once at construction with onUpdate and onDelete", () => {
828858
// Arrange
829859
const httpService: Pick<HttpService, "getRequest"> = { getRequest: vi.fn() };
830860
const storageService: TestStorageService = { put: vi.fn(), get: vi.fn().mockReturnValue({}) };
831861
const loadingService: TestLoadingService = {
832862
ensureLoadingFinished: vi.fn().mockResolvedValue(undefined),
833863
};
834-
const config: AdapterStoreConfig<TestItem, TestAdapted, TestNewAdapted> = {
864+
const { broadcast, subscribe } = captureBroadcast();
865+
866+
// Act
867+
createAdapterStoreModule<TestItem, TestAdapted, TestNewAdapted>({
835868
domainName: "test-items",
836869
adapter: createTestAdapter,
837870
httpService,
838871
storageService,
839872
loadingService,
873+
broadcast,
874+
});
875+
876+
// Assert
877+
expect(subscribe).toHaveBeenCalledTimes(1);
878+
expect(subscribe).toHaveBeenCalledWith({
879+
onUpdate: expect.any(Function) as unknown,
880+
onDelete: expect.any(Function) as unknown,
881+
});
882+
});
883+
884+
it("should apply onUpdate events to the store without calling http", () => {
885+
// Arrange
886+
const httpService: Pick<HttpService, "getRequest"> = { getRequest: vi.fn() };
887+
const storageService: TestStorageService = { put: vi.fn(), get: vi.fn().mockReturnValue({}) };
888+
const loadingService: TestLoadingService = {
889+
ensureLoadingFinished: vi.fn().mockResolvedValue(undefined),
840890
};
841-
const store = createAdapterStoreModule(config);
891+
const { broadcast, getHandlers } = captureBroadcast();
892+
const store = createAdapterStoreModule<TestItem, TestAdapted, TestNewAdapted>({
893+
domainName: "test-items",
894+
adapter: createTestAdapter,
895+
httpService,
896+
storageService,
897+
loadingService,
898+
broadcast,
899+
});
842900

843901
// Act
844-
store.applyServerUpdate({
902+
getHandlers().onUpdate({
845903
id: 9,
846904
name: "Pushed Item",
847905
createdAt: "2024-01-01T00:00:00Z",
@@ -853,22 +911,23 @@ describe("createAdapterStoreModule", () => {
853911
expect(httpService.getRequest).not.toHaveBeenCalled();
854912
});
855913

856-
it("should replace an existing item and refresh its adapted view", () => {
914+
it("should replace existing items and refresh adapted views via onUpdate", () => {
857915
// Arrange
858916
const httpService: Pick<HttpService, "getRequest"> = { getRequest: vi.fn() };
859917
const storageService: TestStorageService = { put: vi.fn(), get: vi.fn().mockReturnValue({}) };
860918
const loadingService: TestLoadingService = {
861919
ensureLoadingFinished: vi.fn().mockResolvedValue(undefined),
862920
};
863-
const config: AdapterStoreConfig<TestItem, TestAdapted, TestNewAdapted> = {
921+
const { broadcast, getHandlers } = captureBroadcast();
922+
const store = createAdapterStoreModule<TestItem, TestAdapted, TestNewAdapted>({
864923
domainName: "test-items",
865924
adapter: createTestAdapter,
866925
httpService,
867926
storageService,
868927
loadingService,
869-
};
870-
const store = createAdapterStoreModule(config);
871-
store.applyServerUpdate({
928+
broadcast,
929+
});
930+
getHandlers().onUpdate({
872931
id: 1,
873932
name: "Original",
874933
createdAt: "2024-01-01T00:00:00Z",
@@ -877,7 +936,7 @@ describe("createAdapterStoreModule", () => {
877936
expect(store.getById(1).value?.name).toBe("Original");
878937

879938
// Act
880-
store.applyServerUpdate({
939+
getHandlers().onUpdate({
881940
id: 1,
882941
name: "Updated",
883942
createdAt: "2024-01-01T00:00:00Z",
@@ -888,24 +947,25 @@ describe("createAdapterStoreModule", () => {
888947
expect(store.getById(1).value?.name).toBe("Updated");
889948
});
890949

891-
it("should persist to storage service", () => {
950+
it("should persist onUpdate events to storage", () => {
892951
// Arrange
893952
const httpService: Pick<HttpService, "getRequest"> = { getRequest: vi.fn() };
894953
const storageService: TestStorageService = { put: vi.fn(), get: vi.fn().mockReturnValue({}) };
895954
const loadingService: TestLoadingService = {
896955
ensureLoadingFinished: vi.fn().mockResolvedValue(undefined),
897956
};
898-
const config: AdapterStoreConfig<TestItem, TestAdapted, TestNewAdapted> = {
957+
const { broadcast, getHandlers } = captureBroadcast();
958+
createAdapterStoreModule<TestItem, TestAdapted, TestNewAdapted>({
899959
domainName: "test-items",
900960
adapter: createTestAdapter,
901961
httpService,
902962
storageService,
903963
loadingService,
904-
};
905-
const store = createAdapterStoreModule(config);
964+
broadcast,
965+
});
906966

907967
// Act
908-
store.applyServerUpdate({
968+
getHandlers().onUpdate({
909969
id: 4,
910970
name: "Item 4",
911971
createdAt: "2024-01-01T00:00:00Z",
@@ -918,25 +978,24 @@ describe("createAdapterStoreModule", () => {
918978
expect.objectContaining({ 4: expect.any(Object) as unknown }),
919979
);
920980
});
921-
});
922981

923-
describe("applyServerDelete", () => {
924-
it("should remove an item from the store without calling http", () => {
982+
it("should apply onDelete events to the store without calling http", () => {
925983
// Arrange
926984
const httpService: Pick<HttpService, "getRequest"> = { getRequest: vi.fn() };
927985
const storageService: TestStorageService = { put: vi.fn(), get: vi.fn().mockReturnValue({}) };
928986
const loadingService: TestLoadingService = {
929987
ensureLoadingFinished: vi.fn().mockResolvedValue(undefined),
930988
};
931-
const config: AdapterStoreConfig<TestItem, TestAdapted, TestNewAdapted> = {
989+
const { broadcast, getHandlers } = captureBroadcast();
990+
const store = createAdapterStoreModule<TestItem, TestAdapted, TestNewAdapted>({
932991
domainName: "test-items",
933992
adapter: createTestAdapter,
934993
httpService,
935994
storageService,
936995
loadingService,
937-
};
938-
const store = createAdapterStoreModule(config);
939-
store.applyServerUpdate({
996+
broadcast,
997+
});
998+
getHandlers().onUpdate({
940999
id: 5,
9411000
name: "Doomed",
9421001
createdAt: "2024-01-01T00:00:00Z",
@@ -945,50 +1004,52 @@ describe("createAdapterStoreModule", () => {
9451004
expect(store.getById(5).value).toBeDefined();
9461005

9471006
// Act
948-
store.applyServerDelete(5);
1007+
getHandlers().onDelete(5);
9491008

9501009
// Assert
9511010
expect(store.getById(5).value).toBeUndefined();
9521011
expect(httpService.getRequest).not.toHaveBeenCalled();
9531012
});
9541013

955-
it("should be a no-op when the id is not in the store", () => {
1014+
it("should be a no-op when onDelete is fired for an unknown id", () => {
9561015
// Arrange
9571016
const httpService: Pick<HttpService, "getRequest"> = { getRequest: vi.fn() };
9581017
const storageService: TestStorageService = { put: vi.fn(), get: vi.fn().mockReturnValue({}) };
9591018
const loadingService: TestLoadingService = {
9601019
ensureLoadingFinished: vi.fn().mockResolvedValue(undefined),
9611020
};
962-
const config: AdapterStoreConfig<TestItem, TestAdapted, TestNewAdapted> = {
1021+
const { broadcast, getHandlers } = captureBroadcast();
1022+
const store = createAdapterStoreModule<TestItem, TestAdapted, TestNewAdapted>({
9631023
domainName: "test-items",
9641024
adapter: createTestAdapter,
9651025
httpService,
9661026
storageService,
9671027
loadingService,
968-
};
969-
const store = createAdapterStoreModule(config);
1028+
broadcast,
1029+
});
9701030

9711031
// Act & Assert
972-
expect(() => store.applyServerDelete(404)).not.toThrow();
1032+
expect(() => getHandlers().onDelete(404)).not.toThrow();
9731033
expect(store.getAll.value).toHaveLength(0);
9741034
});
9751035

976-
it("should persist the updated state to storage service", () => {
1036+
it("should persist onDelete events to storage", () => {
9771037
// Arrange
9781038
const httpService: Pick<HttpService, "getRequest"> = { getRequest: vi.fn() };
9791039
const storageService: TestStorageService = { put: vi.fn(), get: vi.fn().mockReturnValue({}) };
9801040
const loadingService: TestLoadingService = {
9811041
ensureLoadingFinished: vi.fn().mockResolvedValue(undefined),
9821042
};
983-
const config: AdapterStoreConfig<TestItem, TestAdapted, TestNewAdapted> = {
1043+
const { broadcast, getHandlers } = captureBroadcast();
1044+
createAdapterStoreModule<TestItem, TestAdapted, TestNewAdapted>({
9841045
domainName: "test-items",
9851046
adapter: createTestAdapter,
9861047
httpService,
9871048
storageService,
9881049
loadingService,
989-
};
990-
const store = createAdapterStoreModule(config);
991-
store.applyServerUpdate({
1050+
broadcast,
1051+
});
1052+
getHandlers().onUpdate({
9921053
id: 6,
9931054
name: "Item 6",
9941055
createdAt: "2024-01-01T00:00:00Z",
@@ -997,14 +1058,34 @@ describe("createAdapterStoreModule", () => {
9971058
vi.mocked(storageService.put).mockClear();
9981059

9991060
// Act
1000-
store.applyServerDelete(6);
1061+
getHandlers().onDelete(6);
10011062

10021063
// Assert
10031064
expect(storageService.put).toHaveBeenCalledWith(
10041065
"test-items",
10051066
expect.not.objectContaining({ 6: expect.anything() }),
10061067
);
10071068
});
1069+
1070+
it("should not attempt to subscribe when broadcast is omitted", () => {
1071+
// Arrange
1072+
const httpService: Pick<HttpService, "getRequest"> = { getRequest: vi.fn() };
1073+
const storageService: TestStorageService = { put: vi.fn(), get: vi.fn().mockReturnValue({}) };
1074+
const loadingService: TestLoadingService = {
1075+
ensureLoadingFinished: vi.fn().mockResolvedValue(undefined),
1076+
};
1077+
1078+
// Act & Assert
1079+
expect(() =>
1080+
createAdapterStoreModule<TestItem, TestAdapted, TestNewAdapted>({
1081+
domainName: "test-items",
1082+
adapter: createTestAdapter,
1083+
httpService,
1084+
storageService,
1085+
loadingService,
1086+
}),
1087+
).not.toThrow();
1088+
});
10081089
});
10091090

10101091
describe("localStorage persistence", () => {

0 commit comments

Comments
 (0)