Skip to content

Commit

Permalink
fix(socketio): compatibility and memory footprint issues with the ses…
Browse files Browse the repository at this point in the history 
…sion

closes tsedio#2394
  • Loading branch information
@derevnjuk
derevnjuk committed Jul 26, 2023
1 parent 3181a16 commit cd10a22
Show file tree
Hide file tree
Showing 15 changed files with 366 additions and 163 deletions.
6 changes: 3 additions & 3 deletions packages/third-parties/socketio/jest.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ module.exports = {
},
coverageThreshold: {
global: {
statements: 99.6,
branches: 94.59,
statements: 99.68,
branches: 95.52,
functions: 100,
lines: 99.6
lines: 99.68
}
}
};
3 changes: 2 additions & 1 deletion packages/third-parties/socketio/readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ Middlewares chain use the `Promise` to run it. If one of this middlewares/method
import {SocketService, SocketUseAfter, SocketUseBefore, Emit, Input, Args, SocketSession} from "@tsed/socketio";
import {UserConverterSocketMiddleware, ErrorHandlerSocketMiddleware} from "../middlewares";
import {User} from "../models/User";
import {SocketSessionData} from "@tsed/socketio/lib/cjs";

@SocketService("/my-namespace")
@SocketUseBefore(UserConverterSocketMiddleware) // global version
Expand All @@ -267,7 +268,7 @@ export class MySocketService {
@Emit("responseEventName") // or Broadcast or BroadcastOthers
@SocketUseBefore(UserConverterSocketMiddleware)
@SocketUseAfter(ErrorHandlerSocketMiddleware)
async myMethod(@Args(0) userName: User) {
async myMethod(@Args(0) userName: User, @SocketSessionData session: SocketSessionData) {
const user = session.get("user") || {};
user.name = userName;

Expand Down
121 changes: 24 additions & 97 deletions packages/third-parties/socketio/src/class/SocketHandlersBuilder.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ describe("SocketHandlersBuilder", () => {
const {instance} = createServiceFixture();
expect(instance.nsp).toEqual("namespace1");
});

it("should init the nspSession", () => {
const {instance} = createServiceFixture();
expect(instance._nspSession).toBeInstanceOf(Map);
});
});
describe("onConnection()", () => {
it("should build handler and invoke onConnection instance method", async () => {
Expand Down Expand Up @@ -138,12 +133,10 @@ describe("SocketHandlersBuilder", () => {

const invokeStub = jest.spyOn(builder, "invoke").mockReturnValue(undefined);
const buildHandlersStub = jest.spyOn(builder, "buildHandlers").mockReturnValue(undefined);
const createSessionStub = jest.spyOn(builder, "createSession").mockReturnValue(undefined);

await builder.onConnection(socketStub, nspStub);

expect(buildHandlersStub).toBeCalledWith(socketStub, nspStub);
expect(createSessionStub).toBeCalledWith(socketStub);
expect(invokeStub).toBeCalledWith(
instance,
{eventName: "onConnection"},
Expand All @@ -155,7 +148,7 @@ describe("SocketHandlersBuilder", () => {
});
});
describe("onDisconnect()", () => {
it("should call the createSession method and create the $onDisconnect method if is missing", async () => {
it("should create the $onDisconnect method if is missing", async () => {
const instance = {
$onDisconnect: jest.fn()
};
Expand Down Expand Up @@ -183,11 +176,9 @@ describe("SocketHandlersBuilder", () => {
}
} as any);
const invokeStub = jest.spyOn(builder, "invoke").mockReturnValue(undefined);
const destroySessionStub = jest.spyOn(builder, "destroySession").mockReturnValue(undefined);

await builder.onDisconnect(socketStub, nspStub);

expect(destroySessionStub).toBeCalledWith(socketStub);
expect(invokeStub).toBeCalledWith(
instance,
{eventName: "onDisconnect"},
Expand Down Expand Up @@ -227,7 +218,6 @@ describe("SocketHandlersBuilder", () => {
}
} as any);
const invokeStub = jest.spyOn(builder, "invoke").mockReturnValue(undefined);
jest.spyOn(builder, "destroySession").mockReturnValue(undefined);

await builder.onDisconnect(socketStub, nspStub, reason);

Expand All @@ -241,87 +231,8 @@ describe("SocketHandlersBuilder", () => {
}
);
});

it("should destroy the session only when $onDisconnect is completed invocation", async () => {
const instance = {
$onDisconnect: jest.fn()
};

const provider: any = {
store: {
get: jest.fn().mockReturnValue({
injectNamespace: "nsp",
handlers: {
$onDisconnect: {
eventName: "onDisconnect"
}
}
})
}
};
const nspStub: any = {nsp: "nsp"};
const socketStub: any = {
on: jest.fn()
};

const builder: any = new SocketHandlersBuilder(provider, {
get() {
return instance;
}
} as any);
const invokeStub = jest.spyOn(builder, "invoke").mockReturnValue(undefined);
const destroySessionStub = jest.spyOn(builder, "destroySession").mockReturnValue(undefined);

await builder.onDisconnect(socketStub, nspStub);

expect(destroySessionStub.mock.invocationCallOrder[0]).toBeGreaterThan(invokeStub.mock.invocationCallOrder[0]);
});
});
describe("createSession()", () => {
it("should create session for the socket", () => {
const instance = {
_nspSession: new Map()
};
const provider: any = {
store: {
get: jest.fn()
}
};

const builder: any = new SocketHandlersBuilder(provider, {
get() {
return instance;
}
} as any);
builder.createSession({id: "id"});

expect(instance._nspSession.get("id")).toBeInstanceOf(Map);
});
});
describe("destroySession()", () => {
it("should destroy session for the socket", () => {
const instance = {
_nspSession: new Map()
};
const provider: any = {
store: {
get: jest.fn()
}
};

instance._nspSession.set("id", new Map());

const builder: any = new SocketHandlersBuilder(provider, {
get() {
return instance;
}
} as any);

builder.destroySession({id: "id"});

expect(instance._nspSession.get("id")).toBeUndefined();
});
});
describe("buildHandlers()", () => {
it("should call socket.on() method", async () => {
const metadata = {
Expand Down Expand Up @@ -521,23 +432,39 @@ describe("SocketHandlersBuilder", () => {

describe("when SESSION", () => {
it("should return a list of parameters", () => {
const map = new Map();
map.set("id", new Map());

const {builder, instance, provider} = createFixture();
const data = {id: "id"};

instance._nspSession = map;
const {builder} = createFixture();

const result = builder.buildParameters(
{
0: {
filter: SocketFilters.SESSION
}
},
{socket: {id: "id"}}
{socket: {data, id: "id"}}
);

expect(new Map(result[0])).toEqual(new Map(Object.entries(data)));
});
});

describe("when RAW_SESSION", () => {
it("should return a list of parameters", () => {
const data = {id: "id"};

const {builder} = createFixture();

const result = builder.buildParameters(
{
0: {
filter: SocketFilters.RAW_SESSION
}
},
{socket: {data, id: "id"}}
);

expect(result[0]).toBeInstanceOf(Map);
expect(result[0]).toEqual(data);
});
});

Expand Down
37 changes: 8 additions & 29 deletions packages/third-parties/socketio/src/class/SocketHandlersBuilder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import {SocketInjectableNsp} from "../interfaces/SocketInjectableNsp";
import {SocketParamMetadata} from "../interfaces/SocketParamMetadata";
import {SocketProviderTypes} from "../interfaces/SocketProviderTypes";
import {SocketReturnsTypes} from "../interfaces/SocketReturnsTypes";
import {getNspSession} from "../registries/NspSessionRegistry";
import {SocketProviderMetadata} from "./SocketProviderMetadata";
import {SocketSessionData} from "./SocketSessionData";

/**
* @ignore
*/
export class SocketHandlersBuilder {
private socketProviderMetadata: SocketProviderMetadata;
private readonly socketProviderMetadata: SocketProviderMetadata;

constructor(private provider: Provider, private injector: InjectorService) {
this.socketProviderMetadata = new SocketProviderMetadata(this.provider.store.get("socketIO"));
Expand Down Expand Up @@ -60,8 +60,6 @@ export class SocketHandlersBuilder {
instance.$onConnection && this.socketProviderMetadata.createHook("$onConnection", "connection");
instance.$onDisconnect && this.socketProviderMetadata.createHook("$onDisconnect", "disconnect");

instance._nspSession = getNspSession(namespace);

injectNamespaces.forEach((setting: SocketInjectableNsp) => {
instance[setting.propertyKey] = nsps.get(setting.nsp || namespace);
});
Expand All @@ -85,7 +83,6 @@ export class SocketHandlersBuilder {
const instance = this.injector.get(this.provider.token);

this.buildHandlers(socket, nsp);
this.createSession(socket);

if (instance.$onConnection) {
await this.invoke(instance, socketProviderMetadata.$onConnection, {socket, nsp});
Expand All @@ -99,26 +96,6 @@ export class SocketHandlersBuilder {
if (instance.$onDisconnect) {
await this.invoke(instance, socketProviderMetadata.$onDisconnect, {socket, nsp, reason});
}

this.destroySession(socket);
}

/**
*
* @param {Socket} socket
*/
private createSession(socket: Socket) {
const instance = this.injector.get(this.provider.token);
instance._nspSession.set(socket.id, new Map());
}

/**
*
* @param {Socket} socket
*/
private destroySession(socket: Socket) {
const instance = this.injector.get(this.provider.token);
instance._nspSession.delete(socket.id);
}

private buildHandlers(socket: Socket, nsp: Namespace) {
Expand Down Expand Up @@ -183,9 +160,9 @@ export class SocketHandlersBuilder {

if (filter === SocketFilters.ARGS && useMapper) {
value = deserialize(value, {
useAlias: true,
type,
collectionType
collectionType,
useAlias: true
});
scope.args[mapIndex!] = value;
}
Expand Down Expand Up @@ -247,8 +224,10 @@ export class SocketHandlersBuilder {
return scope.error;

case SocketFilters.SESSION:
const instance = this.injector.get(this.provider.token);
return instance._nspSession.get(scope.socket.id);
return new SocketSessionData(scope.socket.data);

case SocketFilters.RAW_SESSION:
return scope.socket.data;

case SocketFilters.SOCKET_NSP:
return scope.socket.nsp;
Expand Down

0 comments on commit cd10a22

Please sign in to comment.