From 6ddec2685f6dd58921eacb18dcafd39bc64ed18a Mon Sep 17 00:00:00 2001 From: simpgps Date: Tue, 9 Jun 2026 22:12:55 +0800 Subject: [PATCH] refactor: improve withTrim identity stability, caching, listener removal, and fix ReceivedChannelItems free payload id bug (#8) --- common/ReceivedChannelItems.ts | 5 +- package-lock.json | 60 +++++++++- server/ServerSession.ts | 56 +++++++++- server/util/ClientCallbackSet.ts | 27 ++++- server/util/ClientCallbackSetPerItem.ts | 32 ++++-- .../clientServer/runtime-typechecking.test.ts | 105 +++++++++++++++++- 6 files changed, 258 insertions(+), 27 deletions(-) diff --git a/common/ReceivedChannelItems.ts b/common/ReceivedChannelItems.ts index 1464d09..196d6b0 100644 --- a/common/ReceivedChannelItems.ts +++ b/common/ReceivedChannelItems.ts @@ -38,12 +38,13 @@ export class ReceivedChannelItems extends WeakValueMap{ } free(item: any) { - item.id !== undefined && typeof item.id === "number" || throwError(`Value does not seem to be a ${DIAGNOSIS_WHATISACHANNELITEM}`); + const id = item.id; + id !== undefined && typeof id === "number" || throwError(`Value does not seem to be a ${DIAGNOSIS_WHATISACHANNELITEM}`); delete(item.id); if(!this.socketConnection.isClosed()) { this.socketConnection.sendMessage({ type: "channelItemNotUsedAnymore", - payload: {id: item.id, time: this.socketConnection.lastReceivedSequenceNumber} + payload: {id: id, time: this.socketConnection.lastReceivedSequenceNumber} }); } } diff --git a/package-lock.json b/package-lock.json index 01029f4..a9fc9ce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -39,7 +39,7 @@ "dependencies": { "@brillout/json-serializer": "^0.5.3", "clone": "^2.1.2", - "engine.io-client": "^6.5.2", + "engine.io-client": "^6.6.3", "restfuncs-common": "^3.1", "underscore": "^1.13.3" }, @@ -2362,14 +2362,60 @@ } }, "node_modules/engine.io-client": { - "version": "6.5.2", + "version": "6.6.5", + "resolved": "https://repo.huaweicloud.com/repository/npm/engine.io-client/-/engine.io-client-6.6.5.tgz", + "integrity": "sha512-QCwxUDULPlXv8F6tqMMKx5dNkTe6OaBYRMPYeXKBlyOoKvAmE0ac6pW7fFhSscJ/5SI7666/U/B+MElbsrJlIg==", "license": "MIT", "dependencies": { "@socket.io/component-emitter": "~3.1.0", - "debug": "~4.3.1", + "debug": "~4.4.1", "engine.io-parser": "~5.2.1", - "ws": "~8.11.0", - "xmlhttprequest-ssl": "~2.0.0" + "ws": "~8.20.1", + "xmlhttprequest-ssl": "~2.1.1" + } + }, + "node_modules/engine.io-client/node_modules/debug": { + "version": "4.4.3", + "resolved": "https://repo.huaweicloud.com/repository/npm/debug/-/debug-4.4.3.tgz", + "integrity": "sha512-RGwwWnwQvkVfavKVt22FGLw+xYSdzARwm0ru6DhTVA3umU5hZc28V3kO4stgYryrTlLpuvgI9GiijltAjNbcqA==", + "license": "MIT", + "dependencies": { + "ms": "^2.1.3" + }, + "engines": { + "node": ">=6.0" + }, + "peerDependenciesMeta": { + "supports-color": { + "optional": true + } + } + }, + "node_modules/engine.io-client/node_modules/ms": { + "version": "2.1.3", + "resolved": "https://repo.huaweicloud.com/repository/npm/ms/-/ms-2.1.3.tgz", + "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", + "license": "MIT" + }, + "node_modules/engine.io-client/node_modules/ws": { + "version": "8.20.1", + "resolved": "https://repo.huaweicloud.com/repository/npm/ws/-/ws-8.20.1.tgz", + "integrity": "sha512-It4dO0K5v//JtTXuPkfEOaI3uUN87iYPnqo/ZzqCoG3g8uhA66QUMs/SrM0YK7/NAu+r4LMh/9dq2A7k+rHs+w==", + "license": "MIT", + "engines": { + "node": ">=10.0.0" + }, + "peerDependencies": { + "bufferutil": "^4.0.1", + "utf-8-validate": ">=5.0.2" + }, + "peerDependenciesMeta": { + "bufferutil": { + "optional": true + }, + "utf-8-validate": { + "optional": true + } } }, "node_modules/engine.io-parser": { @@ -5875,7 +5921,9 @@ } }, "node_modules/xmlhttprequest-ssl": { - "version": "2.0.0", + "version": "2.1.2", + "resolved": "https://repo.huaweicloud.com/repository/npm/xmlhttprequest-ssl/-/xmlhttprequest-ssl-2.1.2.tgz", + "integrity": "sha512-TEU+nJVUUnA4CYJFLvK5X9AOeH4KvDvhIfm0vV1GaQRtchnG0hgK5p8hw/xjv8cunWYCsiPCSDzObPyhEwq3KQ==", "engines": { "node": ">=0.4.0" } diff --git a/server/ServerSession.ts b/server/ServerSession.ts index 92ad563..7af03a9 100644 --- a/server/ServerSession.ts +++ b/server/ServerSession.ts @@ -112,6 +112,13 @@ export type ClientCallbackProperties = { * @returns a Promise or undefined for callbacks where we know by the meta, that they are (sync) void */ _validateAndCall: (args: unknown[], trimArguments: boolean, trimResult: boolean, useSignatureForTrim?: UnknownFunction, diagnosis?:{isFromClientCallbacks?:boolean, isFromClientCallbacks_CallForSure: boolean}) => unknown + + _withTrimCache?: Array<{ + trimArguments: boolean; + trimResult: boolean; + useSignatureFrom?: UnknownFunction; + wrapper: UnknownFunction; + }>; } /** @@ -2919,7 +2926,7 @@ export function remote(targetOrOptions?: RemoteMethodOptions | ServerSession, me */ export function free(resource: (...args: any[]) => any | Readable_fromNodePackage | Readable_fromReadableStreamPackage | ReadableStream | ReadableStreamDefaultReader) { // TODO: list writables if(typeof resource === "function") { - const clientCallback = resource as ClientCallback; + const clientCallback = ((resource as any).originalCallback || resource) as ClientCallback; if(clientCallback.socketConnection === undefined) { // throw new Error("The passed argument is not a client callback function.") } @@ -2972,16 +2979,57 @@ export function withTrim(callbackFn: CB, trimArgumen if (typeof callbackFn !== "function") { throw new Error("Unsupported resource type") } - if(!isClientCallback(callbackFn)) { + const unwrapped = (callbackFn as any).originalCallback || callbackFn; + if(!isClientCallback(unwrapped)) { throw new Error("The passed argument is not a client callback function."); } - const clientCallback = callbackFn as any as ClientCallback; + const clientCallback = unwrapped as any as ClientCallback; + + if(!clientCallback._withTrimCache) { + clientCallback._withTrimCache = []; + } + const cached = clientCallback._withTrimCache.find(entry => + entry.trimArguments === trimArguments && + entry.trimResult === trimResult && + entry.useSignatureFrom === useSignatureFrom + ); + if(cached) { + return cached.wrapper as any as CB; + } //@ts-ignore - return (...args: unknown[]) => { + const wrapper = (...args: unknown[]) => { return clientCallback._validateAndCall(args, trimArguments, trimResult, useSignatureFrom); } + + Object.setPrototypeOf(wrapper, clientCallback); + + Object.defineProperty(wrapper, "originalCallback", { + value: clientCallback, + writable: false, + configurable: false, + enumerable: false + }); + + wrapper._validateAndCall = (args: unknown[], trimArgs: boolean, trimRes: boolean, useSig?: UnknownFunction, diagnosis?: any) => { + return clientCallback._validateAndCall( + args, + trimArgs || trimArguments, + trimRes || trimResult, + useSig || useSignatureFrom, + diagnosis + ); + } + + clientCallback._withTrimCache.push({ + trimArguments, + trimResult, + useSignatureFrom, + wrapper + }); + + return wrapper as any as CB; } /** diff --git a/server/util/ClientCallbackSet.ts b/server/util/ClientCallbackSet.ts index 641258a..1be3b40 100644 --- a/server/util/ClientCallbackSet.ts +++ b/server/util/ClientCallbackSet.ts @@ -78,18 +78,35 @@ export class ClientCallbackSet extends Set<(...args: P delete(callback: (...args: PARAMS) => unknown): boolean { const clientCallback = this.common.checkIsSocketAssociatedCallbackFunction(callback); - const entriesForClient = this.entriesPerClient.get(clientCallback.socketConnection); + + let foundCallback: any = undefined; + if (this.has(callback)) { + foundCallback = callback; + } else { + for (const cb of this) { + if ((cb as any).originalCallback === clientCallback) { + foundCallback = cb; + break; + } + } + } + + if (foundCallback === undefined) { + return false; + } + + const entriesForClient = this.entriesPerClient.get(foundCallback.socketConnection); if(entriesForClient !== undefined) { - entriesForClient.delete(clientCallback); + entriesForClient.delete(foundCallback); if(entriesForClient.size === 0) { // Was the last one for the client? - this.entriesPerClient.delete(clientCallback.socketConnection); + this.entriesPerClient.delete(foundCallback.socketConnection); } } - const result = super.delete(callback); + const result = super.delete(foundCallback); if(this.common.freeOnClientImmediately) { - free(callback); + free(foundCallback); } return result; diff --git a/server/util/ClientCallbackSetPerItem.ts b/server/util/ClientCallbackSetPerItem.ts index 05aaff8..b0253de 100644 --- a/server/util/ClientCallbackSetPerItem.ts +++ b/server/util/ClientCallbackSetPerItem.ts @@ -143,24 +143,40 @@ export class ClientCallbackSetPerItem { } const clientCallback = this.common.checkIsSocketAssociatedCallbackFunction(callback); + let foundCallback: any = undefined; if(this.members !== undefined) { const forItem = this.members.get(item); if (forItem) { - forItem.delete(clientCallback); - if (forItem.size === 0) { - this.members.delete(item); + if (forItem.has(clientCallback)) { + foundCallback = clientCallback; + } else { + for (const cb of forItem) { + if ((cb as any).originalCallback === clientCallback) { + foundCallback = cb; + break; + } + } } - const entriesForClient = this.entriesPerClient.get(clientCallback.socketConnection); - entriesForClient!.delete(clientCallback); // also remove here - if (entriesForClient!.size === 0) { // Was the last one for the client? - this.entriesPerClient.delete(clientCallback.socketConnection); + if (foundCallback !== undefined) { + forItem.delete(foundCallback); + if (forItem.size === 0) { + this.members.delete(item); + } + + const entriesForClient = this.entriesPerClient.get(foundCallback.socketConnection); + if (entriesForClient !== undefined) { + entriesForClient.delete(foundCallback); // also remove here + if (entriesForClient.size === 0) { // Was the last one for the client? + this.entriesPerClient.delete(foundCallback.socketConnection); + } + } } } } if(this.common.freeOnClientImmediately) { - free(callback); + free(foundCallback || callback); } } diff --git a/tests/clientServer/runtime-typechecking.test.ts b/tests/clientServer/runtime-typechecking.test.ts index 7689b86..b4e3399 100644 --- a/tests/clientServer/runtime-typechecking.test.ts +++ b/tests/clientServer/runtime-typechecking.test.ts @@ -1,7 +1,7 @@ import { tags } from "typia"; import 'reflect-metadata' -import {ClientCallback, ServerSession} from "restfuncs-server"; -import {withTrim} from "restfuncs-server/ServerSession"; +import {ClientCallback, ServerSession, ClientCallbackSet, ClientCallbackSetPerItem} from "restfuncs-server"; +import {withTrim, free, isClientCallback} from "restfuncs-server/ServerSession"; import express from "express"; import {reflect} from "typescript-rtti"; import {extendPropsAndFunctions, isTypeInfoAvailable} from "restfuncs-server/Util"; @@ -781,6 +781,83 @@ describe("callbacks", () => { withTrim(cb)(objWithExtraProps); return objWithExtraProps.extraProp === "extra"; // is still intact ? } + + @remote() + testWithTrimIdentities(cb: () => void) { + const w1 = withTrim(cb); + const w2 = withTrim(cb); + if (w1 !== w2) throw new Error("withTrim wrapper instances are not identical"); + + const w3 = withTrim(cb, true, false); + const w4 = withTrim(cb, true, false); + if (w3 !== w4) throw new Error("withTrim wrapper instances with same custom options are not identical"); + if (w1 === w3) throw new Error("withTrim wrapper instances with different options must not be identical"); + + const wNested = withTrim(w1); + if (wNested !== w1) throw new Error("nested withTrim should return the same wrapper instance"); + + // Test prototype chain: + if (Object.getPrototypeOf(w1) !== cb) throw new Error("wrapper prototype is not the original callback"); + if ((w1 as any).originalCallback !== cb) throw new Error("originalCallback property does not point to original callback"); + + // Check that isClientCallback works on wrapper: + if (!isClientCallback(w1)) throw new Error("wrapper is not recognized as client callback"); + + return "OK"; + } + + @remote() + async testClientCallbackSetRemovalWithWrapper(cb: () => void) { + const set = new ClientCallbackSet<[]>(); + const w = withTrim(cb); + set.add(w); + if ((set.size as number) !== 1) throw new Error("Failed to add wrapper to Set"); + + // Try removing by passing the original callback + const removed = set.delete(cb); + if (!removed || (set.size as number) !== 0) throw new Error("Failed to remove wrapper via original callback"); + + // Add again + set.add(w); + if ((set.size as number) !== 1) throw new Error("Failed to add wrapper again"); + + // Try removing by passing the wrapper itself + const removed2 = set.delete(w); + if (!removed2 || (set.size as number) !== 0) throw new Error("Failed to remove wrapper via wrapper itself"); + + return "OK"; + } + + @remote() + async testClientCallbackSetPerItemRemovalWithWrapper(cb: () => void) { + const set = new ClientCallbackSetPerItem(); + const w = withTrim(cb); + set.add("item1", w); + if (set.getCallbacksFor("item1").size !== 1) throw new Error("Failed to add wrapper to PerItem Set"); + + // Try removing by passing the original callback + set.delete("item1", cb); + if (set.getCallbacksFor("item1").size !== 0) throw new Error("Failed to remove wrapper from PerItem Set via original callback"); + + // Add again + set.add("item1", w); + if (set.getCallbacksFor("item1").size !== 1) throw new Error("Failed to add wrapper again to PerItem Set"); + + // Try removing by passing the wrapper itself + set.delete("item1", w); + if (set.getCallbacksFor("item1").size !== 0) throw new Error("Failed to remove wrapper from PerItem Set via wrapper itself"); + + return "OK"; + } + + @remote() + async testFreeWithWrapper(cb: () => void) { + const w = withTrim(cb); + // Calling free(w) should unwrap it and call freeClientCallback(cb) which deletes cb.id + free(w); + if ((cb as any).id !== undefined) throw new Error("free(wrapper) failed to delete callback ID"); + return "OK"; + } } it("should allow legal args in a simple callback", () => runClientServerTests(new ServerAPI, async (apiProxy) => { @@ -929,6 +1006,30 @@ describe("callbacks", () => { }, { useSocket: true })); + + test("withTrim wrapper identity and caching", () => runClientServerTests(new ServerAPI, async (apiProxy) => { + expect(await apiProxy.testWithTrimIdentities(() => {})).toBe("OK"); + }, { + useSocket: true + })); + + test("ClientCallbackSet removal with wrapper", () => runClientServerTests(new ServerAPI, async (apiProxy) => { + expect(await apiProxy.testClientCallbackSetRemovalWithWrapper(() => {})).toBe("OK"); + }, { + useSocket: true + })); + + test("ClientCallbackSetPerItem removal with wrapper", () => runClientServerTests(new ServerAPI, async (apiProxy) => { + expect(await apiProxy.testClientCallbackSetPerItemRemovalWithWrapper(() => {})).toBe("OK"); + }, { + useSocket: true + })); + + test("free with wrapper", () => runClientServerTests(new ServerAPI, async (apiProxy) => { + expect(await apiProxy.testFreeWithWrapper(() => {})).toBe("OK"); + }, { + useSocket: true + })); }); describe("callbacks with mixed security requirements", () => {