refactor: improve withTrim identity stability, caching, listener removal, and fix ReceivedChannelItems free payload id bug (#8)

common/ReceivedChannelItems.ts

@@ -38,12 +38,13 @@ export class ReceivedChannelItems extends WeakValueMap<number, object>{
     }
 
     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}
             });
         }
     }

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<CB extends UnknownFunction>(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;
 }
 
 /**

server/util/ClientCallbackSet.ts

@@ -78,18 +78,35 @@ export class ClientCallbackSet<PARAMS extends unknown[]> 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;

server/util/ClientCallbackSetPerItem.ts

@@ -143,24 +143,40 @@ export class ClientCallbackSetPerItem<ITEM, PARAMS extends unknown[]> {
         }
         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);
         }
     }
 

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<string, []>();
+            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", () => {