Refactoring (env, require, test)

Author: oklemenz2

CHANGELOG.md

@@ -9,7 +9,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Fixed
 
-- tbd
+- Add `cds.requires.kinds` for websockets and merge config
+- Pass all `cds.env.websocket` config to adapter and redis implementation
+- Streamline `cds.env` access in socket and redis implementation
+- Refactor unit-tests to be grouped by implementation
 
 ## Version 1.0.1 - 2024-06-03
 

README.md

@@ -624,6 +624,7 @@ cds.test(__dirname + "/..");
 
 cds.env.websocket = {
   kind: "socket.io",
+  impl: null,
 };
 
 const authorization = `Basic ${Buffer.from("alice:alice").toString("base64")}`;

package-lock.json

@@ -54,19 +54,19 @@
     "@eslint/js": "9.4.0",
     "@sap/cds": "^7.9.2",
     "@sap/cds-dk": "^7.9.2",
-    "@sap/xssec": "4.0.1",
+    "@sap/xssec": "4.0.3",
     "@socket.io/redis-adapter": "^8.3.0",
     "@socket.io/redis-streams-adapter": "^0.2.2",
     "@types/express": "^4.17.21",
     "eslint": "^9.4.0",
     "eslint-config-prettier": "^9.1.0",
-    "eslint-plugin-jest": "^28.5.0",
-    "eslint-plugin-n": "17.7.0",
+    "eslint-plugin-jest": "^28.6.0",
+    "eslint-plugin-n": "17.8.1",
     "express-request-proxy": "^2.2.2",
-    "globals": "15.3.0",
+    "globals": "15.4.0",
     "jest": "^29.7.0",
     "passport": "0.7.0",
-    "prettier": "^3.3.0",
+    "prettier": "^3.3.1",
     "socket.io-client": "^4.7.5"
   },
   "license": "Apache-2.0",
@@ -75,6 +75,16 @@
     "url": "https://github.com/cap-js-community/websocket.git"
   },
   "cds": {
+    "requires": {
+      "kinds": {
+        "websocket-ws": {
+          "impl": "@cap-js-community/websocket/src/socket/ws.js"
+        },
+        "websocket-socket.io": {
+          "impl": "@cap-js-community/websocket/src/socket/socket.io.js"
+        }
+      }
+    },
     "schema": {
       "cds": {
         "websocket": {
@@ -116,6 +126,13 @@
                     "options": {
                       "type": "object",
                       "description": "Websocket adapter implementation options",
+                      "properties": {
+                        "key": {
+                          "type": "string",
+                          "description": "Websocket adapter channel prefix",
+                          "default": "websocket"
+                        }
+                      },
                       "additionalProperties": true
                     },
                     "config": {

package.json

@@ -6,11 +6,10 @@ const cds = require("@sap/cds");
 const LOG = cds.log("/websocket/redis");
 
 class RedisAdapter {
-  constructor(server, options, config) {
+  constructor(server, config) {
     this.server = server;
-    this.options = options;
     this.config = config;
-    this.prefix = options?.key ?? "websocket";
+    this.prefix = config?.options?.key ?? "websocket";
   }
 
   async setup() {

src/adapter/redis.js

@@ -95,10 +95,11 @@ async function initWebSocketServer(server, path) {
   }
   try {
     cds.env.websocket ??= {};
+    cds.env.websocket = { ...cds.env.requires?.websocket, ...cds.env.websocket };
     cds.env.websocket.kind ??= "ws";
     const serverImpl = cds.env.websocket.impl || cds.env.websocket.kind;
     const socketModule = SocketServer.require(serverImpl, "socket");
-    socketServer = new socketModule(server, path);
+    socketServer = new socketModule(server, path, cds.env.websocket);
     await socketServer.setup();
     return socketServer;
   } catch (err) {

src/index.js

@@ -43,20 +43,20 @@ const createSecondaryClientAndConnect = (options) => {
 };
 
 const createClientBase = (options = {}) => {
-  const adapterActive = cds.env.websocket?.adapter?.active !== false;
+  const adapterActive = options?.active !== false;
   if (!adapterActive) {
     LOG?.info("Redis adapter is disabled");
     return;
   }
-  const adapterActiveExplicit = !!cds.env.websocket?.adapter?.active;
-  const adapterLocal = !!cds.env.websocket?.adapter?.local;
+  const adapterActiveExplicit = !!options?.active;
+  const adapterLocal = !!options?.local;
   if (!(IS_ON_CF || adapterActiveExplicit || adapterLocal)) {
     LOG?.info("Redis not available in local environment");
     return;
   }
   let credentials;
   try {
-    credentials = xsenv.serviceCredentials({ label: "redis-cache", ...cds.env.websocket?.adapter?.vcap });
+    credentials = xsenv.serviceCredentials({ label: "redis-cache", ...options?.vcap });
   } catch (err) {
     LOG?.info(err.message);
   }
@@ -74,11 +74,11 @@ const createClientBase = (options = {}) => {
         defaults: {
           password: credentials.password,
           socket: { tls: credentials.tls },
-          ...options,
+          ...options?.config,
         },
       });
     }
-    return redis.createClient({ url, ...options });
+    return redis.createClient({ url, ...options?.config });
   } catch (err) {
     throw new Error("Error during create client with redis-cache service:" + err);
   }

src/redis/index.js

@@ -14,11 +14,13 @@ class SocketServer {
    * Constructor for websocket server
    * @param {Object} server HTTP server from express app
    * @param {string} path Protocol path, e.g. '/ws'
+   * @param {Object} config Websocket server configuration
    */
-  constructor(server, path) {
+  constructor(server, path, config) {
     this.id = crypto.randomUUID();
     this.server = server;
     this.path = path;
+    this.config = config;
     this.adapter = null;
     this.adapterActive = false;
     cds.ws = null;
@@ -299,10 +301,13 @@ class SocketServer {
     if (impl.startsWith("./") || impl.startsWith("../")) {
       return require(path.join(process.cwd(), impl));
     } else if (context) {
-      return require(path.join("..", context, impl));
-    } else {
-      return require(impl);
+      try {
+        return require(path.join("..", context, impl));
+      } catch {
+        // ignore
+      }
     }
+    return require(impl);
   }
 }
 

src/socket/base.js

@@ -10,11 +10,11 @@ const LOG = cds.log("/websocket/socket.io");
 const DEBUG = cds.debug("websocket");
 
 class SocketIOServer extends SocketServer {
-  constructor(server, path) {
-    super(server, path);
+  constructor(server, path, config) {
+    super(server, path, config);
     this.io = new Server(server, {
       path,
-      ...cds.env.websocket?.options,
+      ...config?.options,
     });
     this.io.engine.on("connection_error", (err) => {
       delete err.req;
@@ -137,20 +137,13 @@ class SocketIOServer extends SocketServer {
 
   async applyAdapter() {
     try {
-      const adapterImpl = cds.env.websocket?.adapter?.impl;
-      if (adapterImpl) {
-        let options = {};
-        if (cds.env.websocket?.adapter?.options) {
-          options = { ...options, ...cds.env.websocket?.adapter?.options };
-        }
-        let config = {};
-        if (cds.env.websocket?.adapter?.config) {
-          config = { ...config, ...cds.env.websocket?.adapter?.config };
-        }
+      const config = { ...this.config?.adapter };
+      if (config.impl) {
         let client;
         let subClient;
-        const adapterFactory = SocketServer.require(adapterImpl);
-        switch (adapterImpl) {
+        const options = { ...config?.options };
+        const adapterFactory = SocketServer.require(config.impl);
+        switch (config.impl) {
           case "@socket.io/redis-adapter":
             if (await redis.connectionCheck(config)) {
               client = await redis.createPrimaryClientAndConnect(config);

src/socket/socket.io.js

@@ -10,8 +10,8 @@ const LOG = cds.log("/websocket/ws");
 const DEBUG = cds.debug("websocket");
 
 class SocketWSServer extends SocketServer {
-  constructor(server, path) {
-    super(server, path);
+  constructor(server, path, config) {
+    super(server, path, config);
     this.wss = new WebSocket.Server({ server });
     this.services = {};
     cds.ws = this;
@@ -196,18 +196,10 @@ class SocketWSServer extends SocketServer {
 
   async applyAdapter() {
     try {
-      const adapterImpl = cds.env.websocket?.adapter?.impl;
-      if (adapterImpl) {
-        let options = {};
-        if (cds.env.websocket?.adapter?.options) {
-          options = { ...options, ...cds.env.websocket?.adapter?.options };
-        }
-        let config = {};
-        if (cds.env.websocket?.adapter?.config) {
-          config = { ...config, ...cds.env.websocket?.adapter?.config };
-        }
-        const adapterFactory = SocketServer.require(adapterImpl, "adapter");
-        this.adapter = new adapterFactory(this, options, config);
+      const config = { ...this.config?.adapter };
+      if (config.impl) {
+        const adapterFactory = SocketServer.require(config.impl, "adapter");
+        this.adapter = new adapterFactory(this, config);
         await this.adapter?.setup?.();
         this.adapterActive = !!this.adapter?.client;
       }

src/socket/ws.js

@@ -75,18 +75,20 @@ module.exports = {
         break;
     }
   },
-  createClient: jest.fn(() => {
+  createClient: jest.fn((options) => {
     if (createClientError) {
       createClientError = false;
       throw new Error("create client error");
     }
+    client.options = options;
     return client;
   }),
-  createCluster: jest.fn(() => {
+  createCluster: jest.fn((options) => {
     if (createClientError) {
       createClientError = false;
       throw new Error("create cluster error");
     }
+    client.options = options;
     return client;
   }),
   commandOptions: jest.fn(() => {

test/_env/mocks/redis.js

@@ -48,6 +48,9 @@
       }
     },
     "requires": {
+      "websocket": {
+        "kind": "ws"
+      },
       "auth": {
         "[production]": {
           "kind": "xsuaa"

test/_env/package.json

@@ -2,7 +2,7 @@
 
 const cds = require("@sap/cds");
 
-const SocketServer = require("../src/socket/base");
+const SocketServer = require("../../src/socket/base");
 
 describe("Base", () => {
   beforeAll(async () => {});

test/base.test.js test/base/base.test.js

@@ -2,20 +2,26 @@
 
 const cds = require("@sap/cds");
 const xsenv = require("@sap/xsenv");
-const redisMock = require("./_env/mocks/redis");
-jest.mock("redis", () => require("./_env/mocks/redis"));
-const redis = require("../src/redis");
+const redisMock = require("../_env/mocks/redis");
+jest.mock("redis", () => require("../_env/mocks/redis"));
+const redis = require("../../src/redis");
 
-cds.test(__dirname + "/_env");
+cds.test(__dirname + "/../_env");
 
 jest.spyOn(xsenv, "serviceCredentials").mockReturnValue({ uri: "uri" });
 
+const redisConfig = {
+  a: 1,
+};
+const adapterOptions = {
+  impl: "redis",
+  local: true,
+  config: redisConfig,
+};
+
 cds.env.websocket = {
   kind: "ws",
-  adapter: {
-    impl: "redis",
-    local: true,
-  },
+  adapter: adapterOptions,
 };
 
 describe("Redis", () => {
@@ -24,14 +30,16 @@ describe("Redis", () => {
   afterAll(async () => {});
 
   test("Client", async () => {
-    const main = await redis.createPrimaryClientAndConnect();
+    const main = await redis.createPrimaryClientAndConnect(adapterOptions);
     expect(main).toBeDefined();
-    const second = await redis.createSecondaryClientAndConnect();
+    expect(main.options).toMatchObject(redisConfig);
+    const second = await redis.createSecondaryClientAndConnect(adapterOptions);
     expect(second).toBeDefined();
+    expect(second.options).toMatchObject(redisConfig);
   });
 
   test("Client fail", async () => {
-    const main = await redis.createPrimaryClientAndConnect();
+    const main = await redis.createPrimaryClientAndConnect(adapterOptions);
     expect(main).toBeDefined();
     main.error(new Error("Failed"));
     expect(main.on).toHaveBeenNthCalledWith(1, "error", expect.any(Function));
@@ -40,33 +48,33 @@ describe("Redis", () => {
   test("Client createClient exception", async () => {
     await redis.closeClients();
     redisMock.throwError("createClient");
-    let main = await redis.createPrimaryClientAndConnect();
+    let main = await redis.createPrimaryClientAndConnect(adapterOptions);
     expect(main).toBeUndefined();
     redisMock.throwError("createClient");
-    let secondary = await redis.createSecondaryClientAndConnect();
+    let secondary = await redis.createSecondaryClientAndConnect(adapterOptions);
     expect(secondary).toBeUndefined();
   });
 
   test("Client connect exception", async () => {
     await redis.closeClients();
     redisMock.throwError("connect");
-    let main = await redis.createPrimaryClientAndConnect();
+    let main = await redis.createPrimaryClientAndConnect(adapterOptions);
     expect(main).toBeUndefined();
     redisMock.throwError("connect");
-    let secondary = await redis.createSecondaryClientAndConnect();
+    let secondary = await redis.createSecondaryClientAndConnect(adapterOptions);
     expect(secondary).toBeUndefined();
   });
 
   test("Client error", async () => {
     await redis.closeClients();
-    const main = await redis.createPrimaryClientAndConnect();
+    const main = await redis.createPrimaryClientAndConnect(adapterOptions);
     expect(main).toBeDefined();
     main.error(new Error("error"));
   });
 
   test("Client reconnect", async () => {
     await redis.closeClients();
-    const main = await redis.createPrimaryClientAndConnect();
+    const main = await redis.createPrimaryClientAndConnect(adapterOptions);
     expect(main).toBeDefined();
     main.reconnect();
   });