diff --git a/packages/node/src/endpoint/Endpoint.ts b/packages/node/src/endpoint/Endpoint.ts index 72a4644382..faec7c58f1 100644 --- a/packages/node/src/endpoint/Endpoint.ts +++ b/packages/node/src/endpoint/Endpoint.ts @@ -625,6 +625,7 @@ export class Endpoint { } async close() { + await this.env.get(EndpointInitializer).deactivateDescendant(this); await this.#construction.close(); } diff --git a/packages/node/src/endpoint/properties/EndpointInitializer.ts b/packages/node/src/endpoint/properties/EndpointInitializer.ts index 67c3e1ac01..30eac51d5c 100644 --- a/packages/node/src/endpoint/properties/EndpointInitializer.ts +++ b/packages/node/src/endpoint/properties/EndpointInitializer.ts @@ -22,6 +22,11 @@ export abstract class EndpointInitializer { */ abstract eraseDescendant(_endpoint: Endpoint): Promise; + /** + * Deactivate the storage for a {@link Endpoint}. This mainly manages internal state to deactivate the endpoint number assignment + */ + abstract deactivateDescendant(_endpoint: Endpoint): Promise; + /** * Create backing for a behavior of a descendent. * diff --git a/packages/node/src/node/client/ClientEndpointInitializer.ts b/packages/node/src/node/client/ClientEndpointInitializer.ts index f8bfe34e75..32bdcb8717 100644 --- a/packages/node/src/node/client/ClientEndpointInitializer.ts +++ b/packages/node/src/node/client/ClientEndpointInitializer.ts @@ -37,6 +37,10 @@ export class ClientEndpointInitializer extends EndpointInitializer { await store.erase(); } + async deactivateDescendant(_endpoint: Endpoint) { + // nothing to do + } + get ready() { return this.#store.construction.ready; } diff --git a/packages/node/src/node/server/ServerEndpointInitializer.ts b/packages/node/src/node/server/ServerEndpointInitializer.ts index b77030fb19..0409799cca 100644 --- a/packages/node/src/node/server/ServerEndpointInitializer.ts +++ b/packages/node/src/node/server/ServerEndpointInitializer.ts @@ -33,13 +33,20 @@ export class ServerEndpointInitializer extends EndpointInitializer { endpoint.behaviors.require(DescriptorServer); } - override async eraseDescendant(endpoint: Endpoint) { + async eraseDescendant(endpoint: Endpoint) { if (!endpoint.lifecycle.hasId) { return; } - const store = this.#store.endpointStores.storeForEndpoint(endpoint); - await store.erase(); + await this.#store.endpointStores.eraseStoreForEndpoint(endpoint); + } + + async deactivateDescendant(endpoint: Endpoint) { + if (!endpoint.lifecycle.hasId || endpoint.number === 0) { + return; + } + + this.#store.endpointStores.deactivateStoreForEndpoint(endpoint); } /** diff --git a/packages/node/src/node/storage/EndpointStoreService.ts b/packages/node/src/node/storage/EndpointStoreService.ts index 3f22772a77..03644d02e9 100644 --- a/packages/node/src/node/storage/EndpointStoreService.ts +++ b/packages/node/src/node/storage/EndpointStoreService.ts @@ -41,6 +41,16 @@ export abstract class EndpointStoreService { * These stores are cached internally by ID. */ abstract storeForEndpoint(endpoint: Endpoint): EndpointStore; + + /** + * Deactivate the store for a single {@link Endpoint}. This puts the endpoint number back into pre-allocated state + */ + abstract deactivateStoreForEndpoint(endpoint: Endpoint): void; + + /** + * Erase storage for a single {@link Endpoint}. + */ + abstract eraseStoreForEndpoint(endpoint: Endpoint): Promise; } export class EndpointStoreFactory extends EndpointStoreService { @@ -143,6 +153,7 @@ export class EndpointStoreFactory extends EndpointStoreService { logger.warn(`Stored number ${knownNumber} is already allocated to another endpoint, ignoring`); } else { this.#preAllocatedNumbers.delete(knownNumber); + this.#allocatedNumbers.add(knownNumber); endpoint.number = knownNumber; return; } @@ -188,6 +199,30 @@ export class EndpointStoreFactory extends EndpointStoreService { return this.storeForEndpoint(endpoint.owner).childStoreFor(endpoint); } + deactivateStoreForEndpoint(endpoint: Endpoint) { + this.#construction.assert(); + + if (endpoint.maybeNumber === 0) { + throw new InternalError("Cannot deactivate root node store"); + } + + if (!this.#allocatedNumbers.has(endpoint.number)) { + return; + } + this.#allocatedNumbers.delete(endpoint.number); + this.#preAllocatedNumbers.add(endpoint.number); + } + + async eraseStoreForEndpoint(endpoint: Endpoint) { + this.#construction.assert(); + + const store = this.storeForEndpoint(endpoint); + await store.erase(); + + this.#allocatedNumbers.delete(endpoint.number); + this.#preAllocatedNumbers.delete(endpoint.number); + } + /** * Lazily persist a newly allocated number and the next number. */ diff --git a/packages/node/test/endpoints/BridgedNodeEndpointTest.ts b/packages/node/test/endpoints/BridgedNodeEndpointTest.ts index 19309db808..22ea780028 100644 --- a/packages/node/test/endpoints/BridgedNodeEndpointTest.ts +++ b/packages/node/test/endpoints/BridgedNodeEndpointTest.ts @@ -156,6 +156,141 @@ describe("BridgedNodeEndpointTest", () => { assert.strictEqual(bridge2.parts.require("light2").number, light2); assert.strictEqual(bridge2.parts.require("light3").number, light3); assert.strictEqual(bridge2.parts.require("light4").number, light3 + 1); + + await bridge2.owner?.close(); + }); + + it("with multiple dynamic endpoints in different re-init order with reset nextNumber", async () => { + const environment = new Environment("test"); + const storages = new Map(); + const storage = environment.get(StorageService); + storage.location = "(memory-for-test)"; + storage.factory = namespace => { + const existing = storages.get(namespace); + if (existing) { + return existing; + } + const store = new StorageBackendMemory(); + storages.set(namespace, store); + return store; + }; + + // Have a bridge with 4 endpoints + const bridge = await createBridge(AggregatorEndpoint, { environment }); + + await bridge.add({ + type: BridgedLightDevice, + id: "light1", + }); + await MockTime.yield(); + + await bridge.add({ + type: BridgedLightDevice, + id: "light2", + }); + await MockTime.yield(); + + await bridge.add({ + type: BridgedLightDevice, + id: "light2-1", + }); + await MockTime.yield(); + + await bridge.add({ + type: BridgedLightDevice, + id: "light3", + }); + await MockTime.yield(); + + // Store their numbers + const light1 = bridge.parts.require("light1").number; + const light2 = bridge.parts.require("light2").number; + const light3 = bridge.parts.require("light3").number; + + await bridge.owner?.close(); + + const store = storages.get("node0")!; + store.initialize(); + assert.deepEqual(store.get(["root"], "__nextNumber__"), 6); + store.delete(["root"], "__nextNumber__"); + store.close(); + + // Initialize second bridge with same storage + const bridge2 = await createBridge(AggregatorEndpoint, { environment }); + + // Initialize the bridges in a different order, leave one out and add one more + await bridge2.add({ + type: BridgedLightDevice, + id: "light3", + }); + await MockTime.yield(); + + await bridge2.add({ + type: BridgedLightDevice, + id: "light1", + }); + await MockTime.yield(); + + await bridge2.add({ + type: BridgedLightDevice, + id: "light2", + }); + await MockTime.yield(); + + await bridge2.add({ + type: BridgedLightDevice, + id: "light4", + }); + await MockTime.yield(); + + // Verify that the endpoint numbers are preserved and new ones are allocated + assert.strictEqual(bridge2.parts.require("light1").number, light1); + assert.strictEqual(bridge2.parts.require("light2").number, light2); + assert.strictEqual(bridge2.parts.require("light3").number, light3); + assert.strictEqual(bridge2.parts.require("light4").number, light3 + 1); + assert.deepEqual(store.get(["root"], "__nextNumber__"), 7); + + await bridge2.owner?.close(); + }); + + it("removing and re-adding dynamic endpoints", async () => { + // Have a bridge with 4 endpoints + const bridge = await createBridge(AggregatorEndpoint); + + await bridge.add({ + type: BridgedLightDevice, + id: "light1", + }); + await MockTime.yield(); + + const ep2 = new Endpoint({ + type: BridgedLightDevice, + id: "light2", + }); + await bridge.add(ep2); + await MockTime.yield(); + + await bridge.add({ + type: BridgedLightDevice, + id: "light3", + }); + await MockTime.yield(); + + const light2 = bridge.parts.require("light2").number; + + await ep2.close(); + await MockTime.yield(); + + const ep2New = new Endpoint({ + type: BridgedLightDevice, + id: "light2", + }); + await bridge.add(ep2New); + await MockTime.yield(); + + assert.strictEqual(bridge.parts.require("light2").number, light2); + + await bridge.owner?.close(); }); }); }); diff --git a/packages/node/test/node/mock-node.ts b/packages/node/test/node/mock-node.ts index 9bd2973aef..492c5af808 100644 --- a/packages/node/test/node/mock-node.ts +++ b/packages/node/test/node/mock-node.ts @@ -32,6 +32,7 @@ export class MockPartInitializer extends EndpointInitializer { } async eraseDescendant(_endpoint: Endpoint) {} + async deactivateDescendant(_endpoint: Endpoint) {} createBacking(endpoint: Endpoint, behavior: Behavior.Type) { return new ServerBehaviorBacking(endpoint, behavior); @@ -52,17 +53,23 @@ class MockEndpointStoreService extends EndpointStoreService { #stores = Array(); #nextNumber = 1; - override assignNumber(endpoint: Endpoint): void { + assignNumber(endpoint: Endpoint): void { endpoint.number = this.#nextNumber++; } - override storeForEndpoint(endpoint: Endpoint): EndpointStore { + storeForEndpoint(endpoint: Endpoint): EndpointStore { if (this.#stores[endpoint.number]) { return this.#stores[endpoint.number]; } return (this.#stores[endpoint.number] = new MockEndpointStore(endpoint)); } + + deactivateStoreForEndpoint(_endpoint: Endpoint) {} + + async eraseStoreForEndpoint(endpoint: Endpoint) { + delete this.#stores[endpoint.number]; + } } export class MockServerStore extends ServerNodeStore {