Skip to content

Commit

Permalink
Further dynamic endpoint adjustments (#1430)
Browse files Browse the repository at this point in the history
* Always add assigned numbers to allocated

... this basically ensures consistency if "nextNumber" entry would be broken in storage to not reuse any numbers

* Manage pre/allocated Numbers on close/erase

This change makes sure the pre/allocatedNumbers in the EndpointStoreService are managed when a node is erased or just closed.
When erased the number will be removed from allocated numbers to be reusable also within the process because storage is also removed. On close the allocated number is re-set to be preallocated to allow adding the same endpoint again.

* tests

* Docs fix
  • Loading branch information
Apollon77 authored Nov 24, 2024
1 parent 9683465 commit 7fd6096
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/node/src/endpoint/Endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ export class Endpoint<T extends EndpointType = EndpointType.Empty> {
}

async close() {
await this.env.get(EndpointInitializer).deactivateDescendant(this);
await this.#construction.close();
}

Expand Down
5 changes: 5 additions & 0 deletions packages/node/src/endpoint/properties/EndpointInitializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ export abstract class EndpointInitializer {
*/
abstract eraseDescendant(_endpoint: Endpoint): Promise<void>;

/**
* Deactivate the storage for a {@link Endpoint}. This mainly manages internal state to deactivate the endpoint number assignment
*/
abstract deactivateDescendant(_endpoint: Endpoint): Promise<void>;

/**
* Create backing for a behavior of a descendent.
*
Expand Down
4 changes: 4 additions & 0 deletions packages/node/src/node/client/ClientEndpointInitializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
13 changes: 10 additions & 3 deletions packages/node/src/node/server/ServerEndpointInitializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
35 changes: 35 additions & 0 deletions packages/node/src/node/storage/EndpointStoreService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>;
}

export class EndpointStoreFactory extends EndpointStoreService {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.
*/
Expand Down
135 changes: 135 additions & 0 deletions packages/node/test/endpoints/BridgedNodeEndpointTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, StorageBackendMemory>();
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();
});
});
});
11 changes: 9 additions & 2 deletions packages/node/test/node/mock-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -52,17 +53,23 @@ class MockEndpointStoreService extends EndpointStoreService {
#stores = Array<MockEndpointStore>();
#nextNumber = 1;

override assignNumber(endpoint: Endpoint<EndpointType.Empty>): void {
assignNumber(endpoint: Endpoint<EndpointType.Empty>): void {
endpoint.number = this.#nextNumber++;
}

override storeForEndpoint(endpoint: Endpoint<EndpointType.Empty>): EndpointStore {
storeForEndpoint(endpoint: Endpoint<EndpointType.Empty>): EndpointStore {
if (this.#stores[endpoint.number]) {
return this.#stores[endpoint.number];
}

return (this.#stores[endpoint.number] = new MockEndpointStore(endpoint));
}

deactivateStoreForEndpoint(_endpoint: Endpoint<EndpointType.Empty>) {}

async eraseStoreForEndpoint(endpoint: Endpoint<EndpointType.Empty>) {
delete this.#stores[endpoint.number];
}
}

export class MockServerStore extends ServerNodeStore {
Expand Down

0 comments on commit 7fd6096

Please sign in to comment.