diff --git a/.github/workflows/client_js.yml b/.github/workflows/client_js.yml new file mode 100644 index 0000000..61731ca --- /dev/null +++ b/.github/workflows/client_js.yml @@ -0,0 +1,93 @@ +name: CI (@topical/core) + +on: + push: + branches: [main] + tags: ["client_js/v*"] + paths: + - "client_js/**" + - ".github/workflows/client_js.yml" + pull_request: + branches: [main] + paths: + - "client_js/**" + - ".github/workflows/client_js.yml" + +jobs: + test: + name: Test + runs-on: ubuntu-latest + + defaults: + run: + working-directory: client_js + + steps: + - uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: "20" + cache: "npm" + cache-dependency-path: client_js/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Build + run: npm run build + + - name: Check + run: npm run check + + release: + name: Release to npm + needs: test + if: startsWith(github.ref, 'refs/tags/client_js/v') + runs-on: ubuntu-latest + + concurrency: + group: release-client-js + cancel-in-progress: false + + permissions: + contents: write + id-token: write + + defaults: + run: + working-directory: client_js + + steps: + - uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: "20" + registry-url: "https://registry.npmjs.org" + cache: "npm" + cache-dependency-path: client_js/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Verify tag matches package.json version + run: | + TAG_VERSION="${GITHUB_REF#refs/tags/client_js/v}" + PKG_VERSION=$(node -p "require('./package.json').version") + echo "Tag version: $TAG_VERSION" + echo "package.json version: $PKG_VERSION" + if [ "$TAG_VERSION" != "$PKG_VERSION" ]; then + echo "::error::Tag version ($TAG_VERSION) does not match package.json version ($PKG_VERSION)" + exit 1 + fi + + - name: Publish to npm + run: npm publish + + - name: Create GitHub Release + uses: softprops/action-gh-release@v2 + with: + generate_release_notes: true diff --git a/.github/workflows/client_react.yml b/.github/workflows/client_react.yml new file mode 100644 index 0000000..7a1e7a3 --- /dev/null +++ b/.github/workflows/client_react.yml @@ -0,0 +1,93 @@ +name: CI (@topical/react) + +on: + push: + branches: [main] + tags: ["client_react/v*"] + paths: + - "client_react/**" + - ".github/workflows/client_react.yml" + pull_request: + branches: [main] + paths: + - "client_react/**" + - ".github/workflows/client_react.yml" + +jobs: + test: + name: Test + runs-on: ubuntu-latest + + defaults: + run: + working-directory: client_react + + steps: + - uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: "20" + cache: "npm" + cache-dependency-path: client_react/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Build + run: npm run build + + - name: Check + run: npm run check + + release: + name: Release to npm + needs: test + if: startsWith(github.ref, 'refs/tags/client_react/v') + runs-on: ubuntu-latest + + concurrency: + group: release-client-react + cancel-in-progress: false + + permissions: + contents: write + id-token: write + + defaults: + run: + working-directory: client_react + + steps: + - uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: "20" + registry-url: "https://registry.npmjs.org" + cache: "npm" + cache-dependency-path: client_react/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Verify tag matches package.json version + run: | + TAG_VERSION="${GITHUB_REF#refs/tags/client_react/v}" + PKG_VERSION=$(node -p "require('./package.json').version") + echo "Tag version: $TAG_VERSION" + echo "package.json version: $PKG_VERSION" + if [ "$TAG_VERSION" != "$PKG_VERSION" ]; then + echo "::error::Tag version ($TAG_VERSION) does not match package.json version ($PKG_VERSION)" + exit 1 + fi + + - name: Publish to npm + run: npm publish + + - name: Create GitHub Release + uses: softprops/action-gh-release@v2 + with: + generate_release_notes: true diff --git a/.github/workflows/ci.yml b/.github/workflows/server_ex.yml similarity index 88% rename from .github/workflows/ci.yml rename to .github/workflows/server_ex.yml index bf210ea..4148fe1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/server_ex.yml @@ -1,11 +1,17 @@ -name: CI +name: CI (Elixir) on: push: branches: [main] - tags: ["v*"] + tags: ["server_ex/v*"] + paths: + - "server_ex/**" + - ".github/workflows/server_ex.yml" pull_request: branches: [main] + paths: + - "server_ex/**" + - ".github/workflows/server_ex.yml" jobs: test: @@ -52,7 +58,7 @@ jobs: release: name: Release to Hex needs: test - if: startsWith(github.ref, 'refs/tags/v') + if: startsWith(github.ref, 'refs/tags/server_ex/v') runs-on: ubuntu-latest defaults: @@ -82,7 +88,7 @@ jobs: - name: Verify tag matches mix.exs version run: | - TAG_VERSION="${GITHUB_REF#refs/tags/v}" + TAG_VERSION="${GITHUB_REF#refs/tags/server_ex/v}" MIX_VERSION=$(grep 'version:' mix.exs | sed 's/.*version: "\(.*\)".*/\1/') echo "Tag version: $TAG_VERSION" echo "mix.exs version: $MIX_VERSION" diff --git a/client_js/biome.json b/client_js/biome.json new file mode 100644 index 0000000..1e3ba42 --- /dev/null +++ b/client_js/biome.json @@ -0,0 +1,18 @@ +{ + "$schema": "https://biomejs.dev/schemas/2.3.11/schema.json", + "files": { + "includes": ["src/**"] + }, + "assist": { "actions": { "source": { "organizeImports": "on" } } }, + "linter": { + "enabled": true, + "rules": { + "recommended": true + } + }, + "formatter": { + "enabled": true, + "indentStyle": "space", + "indentWidth": 2 + } +} diff --git a/client_js/package-lock.json b/client_js/package-lock.json index 11c311c..3a38ad0 100644 --- a/client_js/package-lock.json +++ b/client_js/package-lock.json @@ -7,7 +7,268 @@ "": { "name": "@topical/core", "version": "0.2.2", - "license": "Apache-2.0" + "license": "Apache-2.0", + "devDependencies": { + "@biomejs/biome": "^2.3.11", + "typescript": "^5.9.3" + } + }, + "node_modules/@biomejs/biome": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/biome/-/biome-2.3.11.tgz", + "integrity": "sha512-/zt+6qazBWguPG6+eWmiELqO+9jRsMZ/DBU3lfuU2ngtIQYzymocHhKiZRyrbra4aCOoyTg/BmY+6WH5mv9xmQ==", + "dev": true, + "license": "MIT OR Apache-2.0", + "bin": { + "biome": "bin/biome" + }, + "engines": { + "node": ">=14.21.3" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/biome" + }, + "optionalDependencies": { + "@biomejs/cli-darwin-arm64": "2.3.11", + "@biomejs/cli-darwin-x64": "2.3.11", + "@biomejs/cli-linux-arm64": "2.3.11", + "@biomejs/cli-linux-arm64-musl": "2.3.11", + "@biomejs/cli-linux-x64": "2.3.11", + "@biomejs/cli-linux-x64-musl": "2.3.11", + "@biomejs/cli-win32-arm64": "2.3.11", + "@biomejs/cli-win32-x64": "2.3.11" + } + }, + "node_modules/@biomejs/cli-darwin-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-arm64/-/cli-darwin-arm64-2.3.11.tgz", + "integrity": "sha512-/uXXkBcPKVQY7rc9Ys2CrlirBJYbpESEDme7RKiBD6MmqR2w3j0+ZZXRIL2xiaNPsIMMNhP1YnA+jRRxoOAFrA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-darwin-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-x64/-/cli-darwin-x64-2.3.11.tgz", + "integrity": "sha512-fh7nnvbweDPm2xEmFjfmq7zSUiox88plgdHF9OIW4i99WnXrAC3o2P3ag9judoUMv8FCSUnlwJCM1B64nO5Fbg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64/-/cli-linux-arm64-2.3.11.tgz", + "integrity": "sha512-l4xkGa9E7Uc0/05qU2lMYfN1H+fzzkHgaJoy98wO+b/7Gl78srbCRRgwYSW+BTLixTBrM6Ede5NSBwt7rd/i6g==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-arm64-musl": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64-musl/-/cli-linux-arm64-musl-2.3.11.tgz", + "integrity": "sha512-XPSQ+XIPZMLaZ6zveQdwNjbX+QdROEd1zPgMwD47zvHV+tCGB88VH+aynyGxAHdzL+Tm/+DtKST5SECs4iwCLg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64/-/cli-linux-x64-2.3.11.tgz", + "integrity": "sha512-/1s9V/H3cSe0r0Mv/Z8JryF5x9ywRxywomqZVLHAoa/uN0eY7F8gEngWKNS5vbbN/BsfpCG5yeBT5ENh50Frxg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-x64-musl": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64-musl/-/cli-linux-x64-musl-2.3.11.tgz", + "integrity": "sha512-vU7a8wLs5C9yJ4CB8a44r12aXYb8yYgBn+WeyzbMjaCMklzCv1oXr8x+VEyWodgJt9bDmhiaW/I0RHbn7rsNmw==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-win32-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-arm64/-/cli-win32-arm64-2.3.11.tgz", + "integrity": "sha512-PZQ6ElCOnkYapSsysiTy0+fYX+agXPlWugh6+eQ6uPKI3vKAqNp6TnMhoM3oY2NltSB89hz59o8xIfOdyhi9Iw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-win32-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-x64/-/cli-win32-x64-2.3.11.tgz", + "integrity": "sha512-43VrG813EW+b5+YbDbz31uUsheX+qFKCpXeY9kfdAx+ww3naKxeVkTD9zLIWxUPfJquANMHrmW3wbe/037G0Qg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/typescript": { + "version": "5.9.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", + "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=14.17" + } + } + }, + "dependencies": { + "@biomejs/biome": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/biome/-/biome-2.3.11.tgz", + "integrity": "sha512-/zt+6qazBWguPG6+eWmiELqO+9jRsMZ/DBU3lfuU2ngtIQYzymocHhKiZRyrbra4aCOoyTg/BmY+6WH5mv9xmQ==", + "dev": true, + "requires": { + "@biomejs/cli-darwin-arm64": "2.3.11", + "@biomejs/cli-darwin-x64": "2.3.11", + "@biomejs/cli-linux-arm64": "2.3.11", + "@biomejs/cli-linux-arm64-musl": "2.3.11", + "@biomejs/cli-linux-x64": "2.3.11", + "@biomejs/cli-linux-x64-musl": "2.3.11", + "@biomejs/cli-win32-arm64": "2.3.11", + "@biomejs/cli-win32-x64": "2.3.11" + } + }, + "@biomejs/cli-darwin-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-arm64/-/cli-darwin-arm64-2.3.11.tgz", + "integrity": "sha512-/uXXkBcPKVQY7rc9Ys2CrlirBJYbpESEDme7RKiBD6MmqR2w3j0+ZZXRIL2xiaNPsIMMNhP1YnA+jRRxoOAFrA==", + "dev": true, + "optional": true + }, + "@biomejs/cli-darwin-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-x64/-/cli-darwin-x64-2.3.11.tgz", + "integrity": "sha512-fh7nnvbweDPm2xEmFjfmq7zSUiox88plgdHF9OIW4i99WnXrAC3o2P3ag9judoUMv8FCSUnlwJCM1B64nO5Fbg==", + "dev": true, + "optional": true + }, + "@biomejs/cli-linux-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64/-/cli-linux-arm64-2.3.11.tgz", + "integrity": "sha512-l4xkGa9E7Uc0/05qU2lMYfN1H+fzzkHgaJoy98wO+b/7Gl78srbCRRgwYSW+BTLixTBrM6Ede5NSBwt7rd/i6g==", + "dev": true, + "optional": true + }, + "@biomejs/cli-linux-arm64-musl": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64-musl/-/cli-linux-arm64-musl-2.3.11.tgz", + "integrity": "sha512-XPSQ+XIPZMLaZ6zveQdwNjbX+QdROEd1zPgMwD47zvHV+tCGB88VH+aynyGxAHdzL+Tm/+DtKST5SECs4iwCLg==", + "dev": true, + "optional": true + }, + "@biomejs/cli-linux-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64/-/cli-linux-x64-2.3.11.tgz", + "integrity": "sha512-/1s9V/H3cSe0r0Mv/Z8JryF5x9ywRxywomqZVLHAoa/uN0eY7F8gEngWKNS5vbbN/BsfpCG5yeBT5ENh50Frxg==", + "dev": true, + "optional": true + }, + "@biomejs/cli-linux-x64-musl": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64-musl/-/cli-linux-x64-musl-2.3.11.tgz", + "integrity": "sha512-vU7a8wLs5C9yJ4CB8a44r12aXYb8yYgBn+WeyzbMjaCMklzCv1oXr8x+VEyWodgJt9bDmhiaW/I0RHbn7rsNmw==", + "dev": true, + "optional": true + }, + "@biomejs/cli-win32-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-arm64/-/cli-win32-arm64-2.3.11.tgz", + "integrity": "sha512-PZQ6ElCOnkYapSsysiTy0+fYX+agXPlWugh6+eQ6uPKI3vKAqNp6TnMhoM3oY2NltSB89hz59o8xIfOdyhi9Iw==", + "dev": true, + "optional": true + }, + "@biomejs/cli-win32-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-x64/-/cli-win32-x64-2.3.11.tgz", + "integrity": "sha512-43VrG813EW+b5+YbDbz31uUsheX+qFKCpXeY9kfdAx+ww3naKxeVkTD9zLIWxUPfJquANMHrmW3wbe/037G0Qg==", + "dev": true, + "optional": true + }, + "typescript": { + "version": "5.9.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", + "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", + "dev": true } } } diff --git a/client_js/package.json b/client_js/package.json index 9769c3b..31fe797 100644 --- a/client_js/package.json +++ b/client_js/package.json @@ -8,7 +8,11 @@ "test": "echo \"Error: no test specified\" && exit 1", "prepublishOnly": "npm run clean && npm run build", "build": "tsc", - "clean": "rm -rf dist" + "clean": "rm -rf dist", + "lint": "biome check", + "typecheck": "tsc --noEmit", + "check": "npm run lint && npm run typecheck", + "format": "biome format --write" }, "repository": { "type": "git", @@ -17,5 +21,11 @@ "author": "Joe Freeman", "license": "Apache-2.0", "homepage": "https://github.com/joefreeman/topical#readme", - "files": ["dist"] + "files": [ + "dist" + ], + "devDependencies": { + "@biomejs/biome": "^2.3.11", + "typescript": "^5.9.3" + } } diff --git a/client_js/src/index.ts b/client_js/src/index.ts index 55d57f7..ba98b6a 100644 --- a/client_js/src/index.ts +++ b/client_js/src/index.ts @@ -1 +1 @@ -export { default as Socket, SocketState } from "./socket"; +export { default as Socket, Params, ParamsInput, SocketState } from "./socket"; diff --git a/client_js/src/socket.ts b/client_js/src/socket.ts index 0ceaa56..1794729 100644 --- a/client_js/src/socket.ts +++ b/client_js/src/socket.ts @@ -1,38 +1,65 @@ -import { applyUpdate, Update } from "./updates"; +import { applyUpdate, type Update } from "./updates"; export type SocketState = "connecting" | "connected" | "disconnected"; +export type Params = Record; + +// Input type that allows undefined values (validated at runtime) +export type ParamsInput = Record; + type Listener = { onUpdate: (value: T) => void; - onError?: (error: any) => void; + onError?: (error: unknown) => void; }; type Topic = { listeners: Listener[]; topic: string[]; + params: Params; channelId?: number; value?: T; }; type Request = { onSuccess: (value: unknown) => void; - onError: (reason?: any) => void; + onError: (reason?: unknown) => void; }; -function validateTopic(parts: (string | undefined)[]) { - if (parts.some((p) => typeof p == "undefined")) { +function validateTopic( + parts: (string | undefined)[], +): asserts parts is string[] { + if (parts.some((p) => typeof p === "undefined")) { throw new Error("topic component is undefined"); } } +function validateParams(params: ParamsInput): asserts params is Params { + for (const key of Object.keys(params)) { + if (typeof params[key] === "undefined") { + throw new Error(`param "${key}" is undefined`); + } + } +} + +// Generate a deterministic key for topic + params combination +function topicKey(topic: string[], params: Params): string { + const sortedParams = Object.keys(params) + .sort() + .map((k) => `${encodeURIComponent(k)}=${encodeURIComponent(params[k])}`) + .join("&"); + return `${topic.map(encodeURIComponent).join("/")}?${sortedParams}`; +} + export default class Socket { private socket: WebSocket; private closed = false; private state: SocketState = "disconnected"; private lastChannelId = 0; - private topics: Record> = {}; + private topics: Record> = {}; private requests: Record = {}; private subscriptions: Record = {}; + // Maps aliased channel IDs to their target channel IDs + private aliases: Record = {}; private listeners: ((state: SocketState) => void)[] = []; constructor(private readonly url: string) { @@ -55,7 +82,7 @@ export default class Socket { } isConnected() { - return this.state == "connected"; + return this.state === "connected"; } addListener(listener: (state: SocketState) => void) { @@ -74,41 +101,97 @@ export default class Socket { this.socket.close(); } - execute(topic: (string | undefined)[], action: string, ...args: any[]) { + execute( + topic: (string | undefined)[], + action: string, + args: unknown[] = [], + params: ParamsInput = {}, + ) { if (!this.isConnected()) { return Promise.reject("not connected"); } const channelId = ++this.lastChannelId; validateTopic(topic); - this.socket.send(JSON.stringify([1, channelId, topic, action, args])); + validateParams(params); + const message = + Object.keys(params).length > 0 + ? [1, channelId, topic, action, args, params] + : [1, channelId, topic, action, args]; + this.socket.send(JSON.stringify(message)); return new Promise((resolve, reject) => { this.requests[channelId] = { onError: reject, onSuccess: resolve }; }); } - notify(topic: (string | undefined)[], action: string, ...args: any[]) { + notify( + topic: (string | undefined)[], + action: string, + args: unknown[] = [], + params: ParamsInput = {}, + ) { if (!this.isConnected()) { - return Promise.reject("not connected"); + return; } validateTopic(topic); - this.socket.send(JSON.stringify([0, topic, action, args])); + validateParams(params); + const message = + Object.keys(params).length > 0 + ? [0, topic, action, args, params] + : [0, topic, action, args]; + this.socket.send(JSON.stringify(message)); } subscribe( topic: (string | undefined)[], onUpdate: (value: T) => void, - onError?: (error: any) => void, - ) { - const listener = { onUpdate, onError }; + onError?: (error: unknown) => void, + ): () => void; + subscribe( + topic: (string | undefined)[], + params: ParamsInput, + onUpdate: (value: T) => void, + onError?: (error: unknown) => void, + ): () => void; + subscribe( + topic: (string | undefined)[], + paramsOrOnUpdate: ParamsInput | ((value: T) => void), + onUpdateOrOnError?: ((value: T) => void) | ((error: unknown) => void), + onError?: (error: unknown) => void, + ): () => void { + let params: ParamsInput; + let onUpdate: (value: T) => void; + let errorHandler: ((error: unknown) => void) | undefined; + + if (typeof paramsOrOnUpdate === "function") { + params = {}; + onUpdate = paramsOrOnUpdate; + errorHandler = onUpdateOrOnError as + | ((error: unknown) => void) + | undefined; + } else { + params = paramsOrOnUpdate; + onUpdate = onUpdateOrOnError as (value: T) => void; + errorHandler = onError; + } + + const listener = { + onUpdate: onUpdate as (value: unknown) => void, + onError: errorHandler, + }; validateTopic(topic); - const key = topic.join("/"); + validateParams(params); + const key = topicKey(topic, params); if (key in this.topics) { this.topics[key].listeners.push(listener); if ("value" in this.topics[key]) { - listener.onUpdate(this.topics[key].value); + onUpdate(this.topics[key].value as T); } } else { - this.topics[key] = { listeners: [listener], topic: topic as string[] }; + this.topics[key] = { + listeners: [listener], + topic: topic, + params, + }; if (this.isConnected()) { this.setupSubscription(key); } @@ -131,13 +214,19 @@ export default class Socket { private setState(state: SocketState) { this.state = state; - this.listeners.forEach((listener) => listener(state)); + this.listeners.forEach((listener) => { + listener(state); + }); } private setupSubscription(key: string) { const channelId = ++this.lastChannelId; - const topic = this.topics[key].topic; - this.socket.send(JSON.stringify([2, channelId, topic])); + const { topic, params } = this.topics[key]; + const message = + Object.keys(params).length > 0 + ? [2, channelId, topic, params] + : [2, channelId, topic]; + this.socket.send(JSON.stringify(message)); this.topics[key].channelId = channelId; this.subscriptions[channelId] = key; } @@ -171,15 +260,18 @@ export default class Socket { case 3: this.handleTopicUpdates(message[1], message[2]); break; + case 4: + this.handleTopicAlias(message[1], message[2]); + break; } }; - private handleError(channelId: number, error: any) { + private handleError(channelId: number, error: unknown) { if (channelId in this.subscriptions) { const key = this.subscriptions[channelId]; - this.topics[key].listeners.forEach( - ({ onError }) => onError && onError(error), - ); + this.topics[key].listeners.forEach(({ onError }) => { + onError?.(error); + }); delete this.topics[key]; delete this.subscriptions[channelId]; } else { @@ -188,16 +280,18 @@ export default class Socket { } } - private handleResult(channelId: number, result: any) { + private handleResult(channelId: number, result: unknown) { this.requests[channelId].onSuccess(result); delete this.requests[channelId]; } - private handleTopicReset(channelId: number, value: any) { + private handleTopicReset(channelId: number, value: unknown) { const key = this.subscriptions[channelId]; if (key) { this.topics[key].value = value; - this.topics[key].listeners.forEach((l) => l.onUpdate(value)); + this.topics[key].listeners.forEach((l) => { + l.onUpdate(value); + }); } } @@ -206,8 +300,39 @@ export default class Socket { if (key) { const value = updates.reduce(applyUpdate, this.topics[key].value); this.topics[key].value = value; - this.topics[key].listeners.forEach((l) => l.onUpdate(value)); + this.topics[key].listeners.forEach((l) => { + l.onUpdate(value); + }); + } + } + + private handleTopicAlias(aliasedChannelId: number, targetChannelId: number) { + const aliasedKey = this.subscriptions[aliasedChannelId]; + const targetKey = this.subscriptions[targetChannelId]; + + if (!aliasedKey || !targetKey) { + return; } + + const aliasedTopic = this.topics[aliasedKey]; + const targetTopic = this.topics[targetKey]; + + // Move listeners from aliased topic to target topic + targetTopic.listeners.push(...aliasedTopic.listeners); + + // If target has a value, notify the new listeners + if ("value" in targetTopic) { + aliasedTopic.listeners.forEach((l) => { + l.onUpdate(targetTopic.value); + }); + } + + // Clean up the aliased topic + delete this.topics[aliasedKey]; + delete this.subscriptions[aliasedChannelId]; + + // Track the alias so unsubscribe works correctly + this.aliases[aliasedChannelId] = targetChannelId; } private handleSocketClose = () => { @@ -220,6 +345,7 @@ export default class Socket { this.topics[key].channelId = undefined; } this.subscriptions = {}; + this.aliases = {}; for (const requestId in this.requests) { this.requests[requestId].onError(); } diff --git a/client_js/src/updates.ts b/client_js/src/updates.ts index 36c7c32..758c366 100644 --- a/client_js/src/updates.ts +++ b/client_js/src/updates.ts @@ -1,73 +1,93 @@ type Path = (string | number)[]; export type Update = - | [0, Path, any] + | [0, Path, unknown] | [1, Path, string] - | [2, Path, number | null, any[]] + | [2, Path, number | null, unknown[]] | [3, Path, number, number] - | [4, Path, Record]; + | [4, Path, Record]; -function updateIn(value: any, path: Path, callback: (value: any) => any): any { - if (path.length == 0) { +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function updateIn( + value: unknown, + path: Path, + callback: (value: unknown) => unknown, +): unknown { + if (path.length === 0) { return callback(value); - } else { - const [key, ...rest] = path; - if (typeof key == "number") { - if (!Array.isArray(value)) { - throw new Error("expected array"); - } - if (key <= value.length) { - throw new Error("index out of range"); - } - return [ - ...value.slice(0, key), - updateIn(value[key], rest, callback), - ...value.slice(key + 1), - ]; - } else { - return { - ...value, - [key]: updateIn(value?.[key], rest, callback), - }; + } + const [key, ...rest] = path; + if (typeof key === "number") { + if (!Array.isArray(value)) { + throw new Error("expected array"); } + if (key <= value.length) { + throw new Error("index out of range"); + } + return [ + ...value.slice(0, key), + updateIn(value[key], rest, callback), + ...value.slice(key + 1), + ]; + } + if (isRecord(value)) { + return { + ...value, + [key]: updateIn(value[key], rest, callback), + }; } + // Handle null/undefined by creating a new object + return { + [key]: updateIn(undefined, rest, callback), + }; } export function applyUpdate(current: T, update: Update): T { - const [operation, path, ...rest] = update; - switch (operation) { + switch (update[0]) { case 0: { - const [value] = rest; - return updateIn(current, path, () => value); + const [, path, value] = update; + return updateIn(current, path, () => value) as T; } case 1: { - const [key] = rest; + const [, path, key] = update; return updateIn(current, path, (value) => { - const { [key]: _, ...rest } = value; - return rest; - }); + if (!isRecord(value)) { + throw new Error("expected object"); + } + const { [key]: _, ...remaining } = value; + return remaining; + }) as T; } case 2: { - const [index, values] = rest; - - return updateIn(current, path, (list: any[]) => { + const [, path, index, values] = update; + return updateIn(current, path, (list) => { if (!Array.isArray(list)) { throw new Error("expected array"); } const i = index === null ? list.length : index; - return [...list.slice(0, i), ...(values as any[]), ...list.slice(i)]; - }); + return [...list.slice(0, i), ...values, ...list.slice(i)]; + }) as T; } case 3: { - const [index, count] = rest; - return updateIn(current, path, (list) => [ - ...list.slice(0, index), - ...list.slice(index + count), - ]); + const [, path, index, count] = update; + return updateIn(current, path, (list) => { + if (!Array.isArray(list)) { + throw new Error("expected array"); + } + return [...list.slice(0, index), ...list.slice(index + count)]; + }) as T; } case 4: { - const [value] = rest; - return updateIn(current, path, (existing) => ({ ...existing, ...value })); + const [, path, value] = update; + return updateIn(current, path, (existing) => { + if (!isRecord(existing)) { + throw new Error("expected object"); + } + return { ...existing, ...value }; + }) as T; } default: throw new Error("unhandled update type"); diff --git a/client_react/biome.json b/client_react/biome.json new file mode 100644 index 0000000..1e3ba42 --- /dev/null +++ b/client_react/biome.json @@ -0,0 +1,18 @@ +{ + "$schema": "https://biomejs.dev/schemas/2.3.11/schema.json", + "files": { + "includes": ["src/**"] + }, + "assist": { "actions": { "source": { "organizeImports": "on" } } }, + "linter": { + "enabled": true, + "rules": { + "recommended": true + } + }, + "formatter": { + "enabled": true, + "indentStyle": "space", + "indentWidth": 2 + } +} diff --git a/client_react/package-lock.json b/client_react/package-lock.json index ec92ff0..0e3f4da 100644 --- a/client_react/package-lock.json +++ b/client_react/package-lock.json @@ -12,12 +12,177 @@ "@topical/core": "^0.2.2" }, "devDependencies": { - "@types/react": "^18.0.26" + "@biomejs/biome": "^2.3.11", + "@types/react": "^18.0.26", + "typescript": "^5.9.3" }, "peerDependencies": { "react": "^18.2.0" } }, + "node_modules/@biomejs/biome": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/biome/-/biome-2.3.11.tgz", + "integrity": "sha512-/zt+6qazBWguPG6+eWmiELqO+9jRsMZ/DBU3lfuU2ngtIQYzymocHhKiZRyrbra4aCOoyTg/BmY+6WH5mv9xmQ==", + "dev": true, + "license": "MIT OR Apache-2.0", + "bin": { + "biome": "bin/biome" + }, + "engines": { + "node": ">=14.21.3" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/biome" + }, + "optionalDependencies": { + "@biomejs/cli-darwin-arm64": "2.3.11", + "@biomejs/cli-darwin-x64": "2.3.11", + "@biomejs/cli-linux-arm64": "2.3.11", + "@biomejs/cli-linux-arm64-musl": "2.3.11", + "@biomejs/cli-linux-x64": "2.3.11", + "@biomejs/cli-linux-x64-musl": "2.3.11", + "@biomejs/cli-win32-arm64": "2.3.11", + "@biomejs/cli-win32-x64": "2.3.11" + } + }, + "node_modules/@biomejs/cli-darwin-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-arm64/-/cli-darwin-arm64-2.3.11.tgz", + "integrity": "sha512-/uXXkBcPKVQY7rc9Ys2CrlirBJYbpESEDme7RKiBD6MmqR2w3j0+ZZXRIL2xiaNPsIMMNhP1YnA+jRRxoOAFrA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-darwin-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-x64/-/cli-darwin-x64-2.3.11.tgz", + "integrity": "sha512-fh7nnvbweDPm2xEmFjfmq7zSUiox88plgdHF9OIW4i99WnXrAC3o2P3ag9judoUMv8FCSUnlwJCM1B64nO5Fbg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64/-/cli-linux-arm64-2.3.11.tgz", + "integrity": "sha512-l4xkGa9E7Uc0/05qU2lMYfN1H+fzzkHgaJoy98wO+b/7Gl78srbCRRgwYSW+BTLixTBrM6Ede5NSBwt7rd/i6g==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-arm64-musl": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64-musl/-/cli-linux-arm64-musl-2.3.11.tgz", + "integrity": "sha512-XPSQ+XIPZMLaZ6zveQdwNjbX+QdROEd1zPgMwD47zvHV+tCGB88VH+aynyGxAHdzL+Tm/+DtKST5SECs4iwCLg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64/-/cli-linux-x64-2.3.11.tgz", + "integrity": "sha512-/1s9V/H3cSe0r0Mv/Z8JryF5x9ywRxywomqZVLHAoa/uN0eY7F8gEngWKNS5vbbN/BsfpCG5yeBT5ENh50Frxg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-x64-musl": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64-musl/-/cli-linux-x64-musl-2.3.11.tgz", + "integrity": "sha512-vU7a8wLs5C9yJ4CB8a44r12aXYb8yYgBn+WeyzbMjaCMklzCv1oXr8x+VEyWodgJt9bDmhiaW/I0RHbn7rsNmw==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-win32-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-arm64/-/cli-win32-arm64-2.3.11.tgz", + "integrity": "sha512-PZQ6ElCOnkYapSsysiTy0+fYX+agXPlWugh6+eQ6uPKI3vKAqNp6TnMhoM3oY2NltSB89hz59o8xIfOdyhi9Iw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-win32-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-x64/-/cli-win32-x64-2.3.11.tgz", + "integrity": "sha512-43VrG813EW+b5+YbDbz31uUsheX+qFKCpXeY9kfdAx+ww3naKxeVkTD9zLIWxUPfJquANMHrmW3wbe/037G0Qg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=14.21.3" + } + }, "node_modules/@topical/core": { "version": "0.2.2", "resolved": "https://registry.npmjs.org/@topical/core/-/core-0.2.2.tgz", @@ -81,9 +246,95 @@ "engines": { "node": ">=0.10.0" } + }, + "node_modules/typescript": { + "version": "5.9.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", + "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=14.17" + } } }, "dependencies": { + "@biomejs/biome": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/biome/-/biome-2.3.11.tgz", + "integrity": "sha512-/zt+6qazBWguPG6+eWmiELqO+9jRsMZ/DBU3lfuU2ngtIQYzymocHhKiZRyrbra4aCOoyTg/BmY+6WH5mv9xmQ==", + "dev": true, + "requires": { + "@biomejs/cli-darwin-arm64": "2.3.11", + "@biomejs/cli-darwin-x64": "2.3.11", + "@biomejs/cli-linux-arm64": "2.3.11", + "@biomejs/cli-linux-arm64-musl": "2.3.11", + "@biomejs/cli-linux-x64": "2.3.11", + "@biomejs/cli-linux-x64-musl": "2.3.11", + "@biomejs/cli-win32-arm64": "2.3.11", + "@biomejs/cli-win32-x64": "2.3.11" + } + }, + "@biomejs/cli-darwin-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-arm64/-/cli-darwin-arm64-2.3.11.tgz", + "integrity": "sha512-/uXXkBcPKVQY7rc9Ys2CrlirBJYbpESEDme7RKiBD6MmqR2w3j0+ZZXRIL2xiaNPsIMMNhP1YnA+jRRxoOAFrA==", + "dev": true, + "optional": true + }, + "@biomejs/cli-darwin-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-x64/-/cli-darwin-x64-2.3.11.tgz", + "integrity": "sha512-fh7nnvbweDPm2xEmFjfmq7zSUiox88plgdHF9OIW4i99WnXrAC3o2P3ag9judoUMv8FCSUnlwJCM1B64nO5Fbg==", + "dev": true, + "optional": true + }, + "@biomejs/cli-linux-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64/-/cli-linux-arm64-2.3.11.tgz", + "integrity": "sha512-l4xkGa9E7Uc0/05qU2lMYfN1H+fzzkHgaJoy98wO+b/7Gl78srbCRRgwYSW+BTLixTBrM6Ede5NSBwt7rd/i6g==", + "dev": true, + "optional": true + }, + "@biomejs/cli-linux-arm64-musl": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64-musl/-/cli-linux-arm64-musl-2.3.11.tgz", + "integrity": "sha512-XPSQ+XIPZMLaZ6zveQdwNjbX+QdROEd1zPgMwD47zvHV+tCGB88VH+aynyGxAHdzL+Tm/+DtKST5SECs4iwCLg==", + "dev": true, + "optional": true + }, + "@biomejs/cli-linux-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64/-/cli-linux-x64-2.3.11.tgz", + "integrity": "sha512-/1s9V/H3cSe0r0Mv/Z8JryF5x9ywRxywomqZVLHAoa/uN0eY7F8gEngWKNS5vbbN/BsfpCG5yeBT5ENh50Frxg==", + "dev": true, + "optional": true + }, + "@biomejs/cli-linux-x64-musl": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64-musl/-/cli-linux-x64-musl-2.3.11.tgz", + "integrity": "sha512-vU7a8wLs5C9yJ4CB8a44r12aXYb8yYgBn+WeyzbMjaCMklzCv1oXr8x+VEyWodgJt9bDmhiaW/I0RHbn7rsNmw==", + "dev": true, + "optional": true + }, + "@biomejs/cli-win32-arm64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-arm64/-/cli-win32-arm64-2.3.11.tgz", + "integrity": "sha512-PZQ6ElCOnkYapSsysiTy0+fYX+agXPlWugh6+eQ6uPKI3vKAqNp6TnMhoM3oY2NltSB89hz59o8xIfOdyhi9Iw==", + "dev": true, + "optional": true + }, + "@biomejs/cli-win32-x64": { + "version": "2.3.11", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-x64/-/cli-win32-x64-2.3.11.tgz", + "integrity": "sha512-43VrG813EW+b5+YbDbz31uUsheX+qFKCpXeY9kfdAx+ww3naKxeVkTD9zLIWxUPfJquANMHrmW3wbe/037G0Qg==", + "dev": true, + "optional": true + }, "@topical/core": { "version": "0.2.2", "resolved": "https://registry.npmjs.org/@topical/core/-/core-0.2.2.tgz", @@ -141,6 +392,12 @@ "requires": { "loose-envify": "^1.1.0" } + }, + "typescript": { + "version": "5.9.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", + "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", + "dev": true } } } diff --git a/client_react/package.json b/client_react/package.json index cfa3813..95455b5 100644 --- a/client_react/package.json +++ b/client_react/package.json @@ -8,7 +8,11 @@ "test": "echo \"Error: no test specified\" && exit 1", "prepublishOnly": "npm run clean && npm run build", "build": "tsc", - "clean": "rm -rf dist" + "clean": "rm -rf dist", + "lint": "biome check", + "typecheck": "tsc --noEmit", + "check": "npm run lint && npm run typecheck", + "format": "biome format --write" }, "repository": { "type": "git", @@ -24,7 +28,9 @@ "react": "^18.2.0" }, "devDependencies": { - "@types/react": "^18.0.26" + "@biomejs/biome": "^2.3.11", + "@types/react": "^18.0.26", + "typescript": "^5.9.3" }, "dependencies": { "@topical/core": "^0.2.2" diff --git a/client_react/src/index.ts b/client_react/src/index.ts index 34d5de5..b9233d0 100644 --- a/client_react/src/index.ts +++ b/client_react/src/index.ts @@ -1,3 +1,3 @@ export { default as SocketProvider } from "./provider"; -export { default as useTopic } from "./useTopic"; export { default as useSocket } from "./useSocket"; +export { default as useTopic } from "./useTopic"; diff --git a/client_react/src/provider.ts b/client_react/src/provider.ts index 2843f36..0901504 100644 --- a/client_react/src/provider.ts +++ b/client_react/src/provider.ts @@ -1,11 +1,11 @@ +import { Socket } from "@topical/core"; import { createContext, createElement, - ReactNode, + type ReactNode, useEffect, useState, } from "react"; -import { Socket } from "@topical/core"; export const Context = createContext(undefined); diff --git a/client_react/src/useSocket.ts b/client_react/src/useSocket.ts index a74ee3a..9e2b148 100644 --- a/client_react/src/useSocket.ts +++ b/client_react/src/useSocket.ts @@ -1,5 +1,5 @@ -import { useState, useEffect, useContext } from "react"; -import { Socket, SocketState } from "@topical/core"; +import type { Socket, SocketState } from "@topical/core"; +import { useContext, useEffect, useState } from "react"; import { Context } from "./provider"; diff --git a/client_react/src/useTopic.ts b/client_react/src/useTopic.ts index f51f3f6..55d9a39 100644 --- a/client_react/src/useTopic.ts +++ b/client_react/src/useTopic.ts @@ -1,45 +1,93 @@ -import { useState, useEffect, useCallback, useContext } from "react"; +import type { ParamsInput } from "@topical/core"; +import { useCallback, useContext, useEffect, useRef, useState } from "react"; import { Context } from "./provider"; -function arrayEqual(a: any[], b: any[]) { - return a.length == b.length && a.every((v, i) => v == b[i]); +function subscriptionReady( + topic: (string | undefined)[], + params: ParamsInput, +): boolean { + return ( + topic.every((p) => typeof p !== "undefined") && + Object.keys(params).every((k) => typeof params[k] !== "undefined") + ); +} + +// Generate a stable key for topic + params (handles undefined values) +function identityKey( + topic: (string | undefined)[], + params: ParamsInput, +): string { + const topicPart = topic.map((p) => encodeURIComponent(p ?? "")).join("/"); + const paramsPart = Object.keys(params) + .sort() + .map( + (k) => `${encodeURIComponent(k)}=${encodeURIComponent(params[k] ?? "")}`, + ) + .join("&"); + return `${topicPart}?${paramsPart}`; } -export default function useTopic(...topicParts: (string | undefined)[]): [ +// Stabilize topic and params - only update references when the key changes +function useStableIdentity( + topic: (string | undefined)[], + params: ParamsInput, + key: string, +): [(string | undefined)[], ParamsInput] { + const keyRef = useRef(key); + const stableRef = useRef({ topic, params }); + if (keyRef.current !== key) { + keyRef.current = key; + stableRef.current = { topic, params }; + } + return [stableRef.current.topic, stableRef.current.params]; +} + +export default function useTopic( + topic: (string | undefined)[], + params: ParamsInput = {}, +): [ T | undefined, { - notify: (action: string, ...args: any[]) => void; - execute: (action: string, ...args: any[]) => Promise; - error: any; + notify: (action: string, args?: unknown[]) => void; + execute: (action: string, args?: unknown[]) => Promise | undefined; + error: unknown; loading: boolean; }, ] { const socket = useContext(Context); - const [state, setState] = - useState<[(string | undefined)[], T | undefined, any]>(); + const [state, setState] = useState<[string, T | undefined, unknown]>(); + + const key = identityKey(topic, params); + const [stableTopic, stableParams] = useStableIdentity(topic, params, key); + const notify = useCallback( - (action: string, ...args: any[]) => { - return socket!.notify(topicParts, action, ...args); + (action: string, args: unknown[] = []) => { + return socket?.notify(stableTopic, action, args, stableParams); }, - [socket, ...topicParts], + [socket, stableTopic, stableParams], ); + const execute = useCallback( - (action: string, ...args: any[]) => { - return socket!.execute(topicParts, action, ...args); + (action: string, args: unknown[] = []) => { + return socket?.execute(stableTopic, action, args, stableParams); }, - [socket, ...topicParts], + [socket, stableTopic, stableParams], ); + useEffect(() => { - if (!topicParts.some((p) => typeof p == "undefined")) { + if (subscriptionReady(stableTopic, stableParams)) { return socket?.subscribe( - topicParts, - (v) => setState([topicParts, v, undefined]), - (e) => setState([topicParts, undefined, e]), + stableTopic, + stableParams, + (v) => setState([key, v, undefined]), + (e) => setState([key, undefined, e]), ); } - }, [socket, ...topicParts]); - const [stateTopic, value, error] = state || [undefined, undefined, undefined]; - const loading = !stateTopic || !arrayEqual(topicParts, stateTopic); + }, [socket, stableTopic, stableParams, key]); + + const [stateKey, value, error] = state || [undefined, undefined, undefined]; + const loading = stateKey !== key; + return [value, { notify, execute, error, loading }]; } diff --git a/server_ex/docs/getting-started.md b/server_ex/docs/getting-started.md index 1b4ea4e..f825ffb 100644 --- a/server_ex/docs/getting-started.md +++ b/server_ex/docs/getting-started.md @@ -80,6 +80,36 @@ end Return `:ok` to allow access, or `{:error, reason}` to deny. The default implementation allows all access. +## Parameters + +Topics can declare optional parameters with default values. Different parameter values create +separate topic instances: + +```elixir +defmodule MyApp.Topics.Leaderboard do + use Topical.Topic, route: ["leaderboards", :game_id], params: [region: "global"] + + def init(params) do + game_id = Keyword.fetch!(params, :game_id) + region = Keyword.fetch!(params, :region) + + {:ok, Topic.new(%{game_id: game_id, region: region, entries: []})} + end +end +``` + +Clients subscribe with params to access specific instances: + +```elixir +# Uses default region "global" +Topical.subscribe(MyApp.Topical, ["leaderboards", "chess"], self()) + +# Subscribes to EU region (separate instance) +Topical.subscribe(MyApp.Topical, ["leaderboards", "chess"], self(), nil, %{"region" => "eu"}) +``` + +Parameters are also available in `authorize/2` for access control based on param values. + ## Supervision Then add a Topical registry to your application supervision tree, referencing the topic: diff --git a/server_ex/docs/introduction.md b/server_ex/docs/introduction.md index 6283877..513cfa3 100644 --- a/server_ex/docs/introduction.md +++ b/server_ex/docs/introduction.md @@ -4,9 +4,10 @@ Topical is a library for defining and serving _topics_. A topic is a value that observed in real-time by connected clients. The server-side implementation takes care of defining how to intialise the state, and then how to keep it updated. -Multiple clients can subscribe to an instance of a topic - instances are identifier by a path, +Multiple clients can subscribe to an instance of a topic - instances are identified by a path, which matches the route defined by a topic. Multiple instances of a topic can exist by using route -placeholders. +placeholders. Topics can also declare optional parameters; different parameter values create +separate instances sharing the same route. Topical takes care of starting topic instances as needed, and stopping them once all clients have disconnected. diff --git a/server_ex/docs/javascript-client.md b/server_ex/docs/javascript-client.md index 03db2a4..3ed1a5d 100644 --- a/server_ex/docs/javascript-client.md +++ b/server_ex/docs/javascript-client.md @@ -18,20 +18,20 @@ type ListModel = { // Setup the socket const socket = new Socket("ws://example.com/socket"); -// Subscribe to a topic (returns a function to unsibscribe) +// Subscribe to a topic (returns a function to unsubscribe) const unsubscribe = socket.subscribe( ["lists", "foo"], - (list: ListModel) => { console.log(value); }, + (list: ListModel) => { console.log(list); }, (error) => { ... } ); -// Execute an action -const itemId = await socket.execute(["lists", "foo"], "add_item", "First item"); +// Execute an action (args is an array) +const itemId = await socket.execute(["lists", "foo"], "add_item", ["First item"]); // (The subscription should have been updated) -// Send a notification -socket.notify(["lists", "foo"], "update_item", itemId, "Inaugural item") +// Send a notification (args is an array) +socket.notify(["lists", "foo"], "update_item", [itemId, "Inaugural item"]); // (The subscription should have been updated again) @@ -39,6 +39,41 @@ socket.notify(["lists", "foo"], "update_item", itemId, "Inaugural item") unsubscribe(); ``` +## Parameters + +Topics can declare optional parameters. For `subscribe`, pass params after the topic and +before the callbacks. For `execute` and `notify`, pass params as the last argument: + +```typescript +// Subscribe to a regional leaderboard (params before callbacks) +const unsubscribe = socket.subscribe( + ["leaderboards", "chess"], + { region: "eu" }, // params + (leaderboard) => { console.log(leaderboard); }, + (error) => { ... } +); + +// Execute with params (params at end) +await socket.execute( + ["leaderboards", "chess"], + "add_score", + ["alice", 100], // args array + { region: "eu" } // params +); + +// Notify with params (params at end) +socket.notify( + ["leaderboards", "chess"], + "add_score", + ["bob", 200], // args array + { region: "eu" } // params +); +``` + +Different param values create separate topic instances. The server handles deduplication +automatically - if you subscribe to the same topic with equivalent params multiple times, +the client will receive an alias response and merge the subscriptions. + ## React client Instead of using the JavaScript client directly, you can use the React client. Install it from npm: @@ -72,9 +107,11 @@ Then use the `useTopic` hook in your components to subscribe to your topic: import { useTopic } from "@topical/react"; function List({ id }) { - const [list, { execute, notify, loading, error }] = useTopic("lists", id); + const [list, { execute, notify, loading, error }] = useTopic( + ["lists", id] + ); const addItem = useCallback( - (text: string) => execute("add_item", text), + (text: string) => execute("add_item", [text]), [execute] ); if (loading) { @@ -89,6 +126,26 @@ function List({ id }) { } ``` +### Parameters with useTopic + +Pass params as the second argument: + +```typescript +function Leaderboard({ gameId, region }) { + const [leaderboard, { execute, loading, error }] = useTopic( + ["leaderboards", gameId], + { region } // params + ); + + const addScore = useCallback( + (player: string, score: number) => execute("add_score", [player, score]), + [execute] + ); + + // ... +} +``` + If you need access to the underlying socket (or the status), you can use the `useSocket` hook: ```typescript diff --git a/server_ex/lib/topical.ex b/server_ex/lib/topical.ex index 2bfb207..7f6ca03 100644 --- a/server_ex/lib/topical.ex +++ b/server_ex/lib/topical.ex @@ -33,47 +33,62 @@ defmodule Topical do @doc """ Subscribes to the specified `topic` (in the specified `registry`). - Returns `{:ok, ref}`, where the `ref` is a reference to the subscription. + Returns `{:ok, ref, server}`, where `ref` is a reference to the subscription and + `server` is the topic server PID (used for unsubscribing). - The `pid` will be send messages, as described above. + The `pid` will be sent messages, as described above. + + ## Options + + * `params` - Optional map of params to pass to the topic (default: `%{}`). + These are merged with route params and passed to `init/1` and `authorize/2`. + Topics with different param values are separate instances and do not share state. ## Example Topical.subscribe(MyApp.Topical, ["lists", "foo"], self()) - #=> {:ok, #Reference<0.4021726225.4145020932.239110>} + #=> {:ok, #Reference<0.4021726225.4145020932.239110>, #PID<0.123.0>} """ - def subscribe(registry, topic, pid, context \\ nil) do - with {:ok, server} <- Registry.get_topic(registry, topic, context) do + def subscribe(registry, topic, pid, context \\ nil, params \\ %{}) do + with {:ok, module, all_params, topic_key} <- Registry.resolve_topic(registry, topic, params), + {:ok, server} <- Registry.get_topic(registry, module, all_params, topic_key, context) do # TODO: monitor/link server? - {:ok, GenServer.call(server, {:subscribe, pid, context})} + ref = GenServer.call(server, {:subscribe, pid, context}) + {:ok, ref, server} end end @doc """ - Unsubscribes from a `topic` (in the specified `registry`). + Unsubscribes from a topic. + + Takes the `server` PID and `ref` returned from `subscribe/5`. ## Example - Topical.unsubscribe(MyApp.Topical, ["lists", "foo"], ref) + {:ok, ref, server} = Topical.subscribe(MyApp.Topical, ["lists", "foo"], self()) + Topical.unsubscribe(server, ref) """ - def unsubscribe(registry, topic, ref) do - with {:ok, server} <- Registry.lookup_topic(registry, topic) do - GenServer.cast(server, {:unsubscribe, ref}) - end + def unsubscribe(server, ref) do + GenServer.cast(server, {:unsubscribe, ref}) end @doc """ Captures the state of the `topic` (in the specified `registry`) without subscribing. + ## Options + + * `params` - Optional map of params to pass to the topic (default: `%{}`). + ## Example Topical.capture(MyApp.Topical, ["lists", "foo"]) # => {:ok, %{items: %{}, order: []}} """ - def capture(registry, topic, context \\ nil) do - with {:ok, server} <- Registry.get_topic(registry, topic, context) do + def capture(registry, topic, context \\ nil, params \\ %{}) do + with {:ok, module, all_params, topic_key} <- Registry.resolve_topic(registry, topic, params), + {:ok, server} <- Registry.get_topic(registry, module, all_params, topic_key, context) do {:ok, GenServer.call(server, {:capture, context})} end end @@ -81,14 +96,19 @@ defmodule Topical do @doc """ Executes an action in a `topic`. + ## Options + + * `params` - Optional map of params to pass to the topic (default: `%{}`). + ## Example Topical.execute(MyApp.Topical, ["lists", "foo"], "add_item", {"Test", false}) #=> {:ok, "item123"} """ - def execute(registry, topic, action, args \\ {}, context \\ nil) do - with {:ok, server} <- Registry.get_topic(registry, topic, context) do + def execute(registry, topic, action, args \\ {}, context \\ nil, params \\ %{}) do + with {:ok, module, all_params, topic_key} <- Registry.resolve_topic(registry, topic, params), + {:ok, server} <- Registry.get_topic(registry, module, all_params, topic_key, context) do {:ok, GenServer.call(server, {:execute, action, args, context})} end end @@ -98,14 +118,19 @@ defmodule Topical do This is similar to `execute/4`, except no result is waited for. + ## Options + + * `params` - Optional map of params to pass to the topic (default: `%{}`). + ## Example Topical.notify(MyApp.Topical, ["lists", "foo"], "update_done", {"item123", true}) #=> :ok """ - def notify(registry, topic, action, args \\ {}, context \\ nil) do - with {:ok, server} <- Registry.get_topic(registry, topic, context) do + def notify(registry, topic, action, args \\ {}, context \\ nil, params \\ %{}) do + with {:ok, module, all_params, topic_key} <- Registry.resolve_topic(registry, topic, params), + {:ok, server} <- Registry.get_topic(registry, module, all_params, topic_key, context) do GenServer.cast(server, {:notify, action, args, context}) end end diff --git a/server_ex/lib/topical/adapters/base/websocket.ex b/server_ex/lib/topical/adapters/base/websocket.ex index 5f341a0..4705456 100644 --- a/server_ex/lib/topical/adapters/base/websocket.ex +++ b/server_ex/lib/topical/adapters/base/websocket.ex @@ -2,6 +2,7 @@ defmodule Topical.Adapters.Base.WebSocket do @moduledoc false alias Topical.Protocol.{Request, Response} + alias Topical.Registry def init(opts, init_arg \\ nil) do registry = Keyword.fetch!(opts, :registry) @@ -14,7 +15,9 @@ defmodule Topical.Adapters.Base.WebSocket do registry: registry, context: context, channels: %{}, - channel_ids: %{} + channel_ids: %{}, + # Maps normalized topic key -> channel_id for alias detection + topic_keys: %{} } {:ok, state} @@ -23,14 +26,14 @@ defmodule Topical.Adapters.Base.WebSocket do def handle_text(text, state) do case Request.decode(text) do - {:ok, :notify, topic, action, args} -> - handle_notify(topic, action, args, state) + {:ok, :notify, topic, action, args, params} -> + handle_notify(topic, action, args, params, state) - {:ok, :execute, channel_id, topic, action, args} -> - handle_execute(channel_id, topic, action, args, state) + {:ok, :execute, channel_id, topic, action, args, params} -> + handle_execute(channel_id, topic, action, args, params, state) - {:ok, :subscribe, channel_id, topic} -> - handle_subscribe(channel_id, topic, state) + {:ok, :subscribe, channel_id, topic, params} -> + handle_subscribe(channel_id, topic, params, state) {:ok, :unsubscribe, channel_id} -> handle_unsubscribe(channel_id, state) @@ -62,17 +65,24 @@ defmodule Topical.Adapters.Base.WebSocket do end end - defp handle_notify(topic, action, args, state) do + defp handle_notify(topic, action, args, params, state) do # TODO: handle some errors (e.g., with lookup/init topic?) - case Topical.notify(state.registry, topic, action, List.to_tuple(args), state.context) do + case Topical.notify(state.registry, topic, action, List.to_tuple(args), state.context, params) do :ok -> {:ok, [], state} end end - defp handle_execute(channel_id, topic, action, args, state) do + defp handle_execute(channel_id, topic, action, args, params, state) do # TODO: don't block? - case Topical.execute(state.registry, topic, action, List.to_tuple(args), state.context) do + case Topical.execute( + state.registry, + topic, + action, + List.to_tuple(args), + state.context, + params + ) do {:ok, result} -> {:ok, [Response.encode_result(channel_id, result)], state} @@ -81,13 +91,35 @@ defmodule Topical.Adapters.Base.WebSocket do end end - defp handle_subscribe(channel_id, topic, state) do - case Topical.subscribe(state.registry, topic, self(), state.context) do - {:ok, ref} -> + defp handle_subscribe(channel_id, topic, params, state) do + # Resolve topic once - use for both alias detection and subscribing + case Registry.resolve_topic(state.registry, topic, params) do + {:ok, module, all_params, topic_key} -> + case Map.fetch(state.topic_keys, topic_key) do + {:ok, existing_channel_id} -> + # Already subscribed to this topic - send alias response + {:ok, [Response.encode_topic_alias(channel_id, existing_channel_id)], state} + + :error -> + # New subscription + do_subscribe(channel_id, module, all_params, topic_key, state) + end + + {:error, error} -> + {:ok, [Response.encode_error(channel_id, error)], state} + end + end + + defp do_subscribe(channel_id, module, all_params, topic_key, state) do + case Registry.get_topic(state.registry, module, all_params, topic_key, state.context) do + {:ok, server} -> + ref = GenServer.call(server, {:subscribe, self(), state.context}) + state = state - |> put_in([:channels, channel_id], {topic, ref}) + |> put_in([:channels, channel_id], {server, ref, topic_key}) |> put_in([:channel_ids, ref], channel_id) + |> put_in([:topic_keys, topic_key], channel_id) {:ok, [], state} @@ -98,14 +130,19 @@ defmodule Topical.Adapters.Base.WebSocket do defp handle_unsubscribe(channel_id, state) do case Map.fetch(state.channels, channel_id) do - {:ok, {topic, ref}} -> - :ok = Topical.unsubscribe(state.registry, topic, ref) + {:ok, {server, ref, topic_key}} -> + Topical.unsubscribe(server, ref) state = state |> Map.update!(:channels, &Map.delete(&1, channel_id)) |> Map.update!(:channel_ids, &Map.delete(&1, ref)) + |> Map.update!(:topic_keys, &Map.delete(&1, topic_key)) + + {:ok, [], state} + :error -> + # Channel not found (maybe it was an alias that was never stored) {:ok, [], state} end end diff --git a/server_ex/lib/topical/protocol.ex b/server_ex/lib/topical/protocol.ex index db5c098..44c6613 100644 --- a/server_ex/lib/topical/protocol.ex +++ b/server_ex/lib/topical/protocol.ex @@ -6,15 +6,28 @@ defmodule Topical.Protocol do def decode(text) do case Jason.decode(text) do + # Notify: [0, topic, action, args] or [0, topic, action, args, params] {:ok, [0, topic, action, args]} -> - {:ok, :notify, topic, action, args} + {:ok, :notify, topic, action, args, %{}} + {:ok, [0, topic, action, args, params]} when is_map(params) -> + {:ok, :notify, topic, action, args, params} + + # Execute: [1, channel_id, topic, action, args] or [1, channel_id, topic, action, args, params] {:ok, [1, channel_id, topic, action, args]} -> - {:ok, :execute, channel_id, topic, action, args} + {:ok, :execute, channel_id, topic, action, args, %{}} + + {:ok, [1, channel_id, topic, action, args, params]} when is_map(params) -> + {:ok, :execute, channel_id, topic, action, args, params} + # Subscribe: [2, channel_id, topic] or [2, channel_id, topic, params] {:ok, [2, channel_id, topic]} -> - {:ok, :subscribe, channel_id, topic} + {:ok, :subscribe, channel_id, topic, %{}} + + {:ok, [2, channel_id, topic, params]} when is_map(params) -> + {:ok, :subscribe, channel_id, topic, params} + # Unsubscribe: [3, channel_id] {:ok, [3, channel_id]} -> {:ok, :unsubscribe, channel_id} @@ -46,6 +59,10 @@ defmodule Topical.Protocol do Jason.encode!([3, channel_id, Enum.map(updates, &encode_update/1)]) end + def encode_topic_alias(channel_id, existing_channel_id) do + Jason.encode!([4, channel_id, existing_channel_id]) + end + defp encode_update(update) do case update do {:set, path, value} -> [0, path, value] diff --git a/server_ex/lib/topical/registry.ex b/server_ex/lib/topical/registry.ex index 8d577d5..8881e94 100644 --- a/server_ex/lib/topical/registry.ex +++ b/server_ex/lib/topical/registry.ex @@ -75,58 +75,96 @@ defmodule Topical.Registry do end @doc """ - Looks up an existing topic without starting it. + Resolves a route and normalizes request params. - Returns `{:ok, pid}` if the topic is running, or `{:error, :not_running}` if not. + Returns `{:ok, module, all_params, topic_key}` or `{:error, reason}`. + + The returned values can be passed to `get_topic/4` to avoid resolving twice. """ - def lookup_topic(name, route) do + def resolve_topic(name, route, request_params \\ %{}) do {registry_name, _supervisor_name} = resolve_names(name) + {:ok, routes} = Registry.meta(registry_name, :routes) + + case resolve_route(route, routes) do + {module, route_params} -> + case normalize_params(request_params, module.params()) do + {:ok, normalized_params} -> + topic_key = {route, normalized_params} + all_params = Keyword.merge(route_params, normalized_params) + {:ok, module, all_params, topic_key} + + {:error, reason} -> + {:error, reason} + end - case Registry.lookup(registry_name, route) do - [{pid, _}] -> {:ok, pid} - [] -> {:error, :not_running} + nil -> + {:error, :not_found} end end @doc """ Gets or starts a topic, after checking authorization. + Accepts the resolved values from `resolve_topic/3`. + Returns `{:ok, pid}` if authorized and the topic is running (or was started), or `{:error, reason}` if authorization fails or the topic cannot be started. """ - def get_topic(name, route, context) do + def get_topic(name, module, all_params, topic_key, context) do {registry_name, supervisor_name} = resolve_names(name) - {:ok, routes} = Registry.meta(registry_name, :routes) - case resolve_route(route, routes) do - {module, params} -> - case module.authorize(params, context) do - :ok -> - case Registry.lookup(registry_name, route) do - [{pid, _}] -> - {:ok, pid} - - [] -> - spec = - {Topical.Topic.Server, - name: {:via, Registry, {registry_name, route}}, - id: route, - module: module, - init_arg: params} - - case DynamicSupervisor.start_child(supervisor_name, spec) do - {:ok, pid} -> {:ok, pid} - {:error, {:already_started, pid}} -> {:ok, pid} - {:error, reason} -> {:error, reason} - end + case module.authorize(all_params, context) do + :ok -> + case Registry.lookup(registry_name, topic_key) do + [{pid, _}] -> + {:ok, pid} + + [] -> + spec = + {Topical.Topic.Server, + name: {:via, Registry, {registry_name, topic_key}}, + id: topic_key, + module: module, + init_arg: all_params} + + case DynamicSupervisor.start_child(supervisor_name, spec) do + {:ok, pid} -> {:ok, pid} + {:error, {:already_started, pid}} -> {:ok, pid} + {:error, reason} -> {:error, reason} end + end - {:error, reason} -> - {:error, reason} + {:error, reason} -> + {:error, reason} + end + end + + # Normalizes request params against declared params: + # - Filters to only declared param names (ignores unknown params) + # - Converts string keys to atoms (safe because we only use declared names) + # - Normalizes empty strings to nil + # - Applies default values for missing params + # - Returns {:ok, sorted_keyword_list} or {:error, {:invalid_param, name}} + defp normalize_params(request_params, declared_params) do + declared_params + |> Enum.reduce_while({:ok, []}, fn {name, default}, {:ok, acc} -> + # Try both atom and string key + value = + case Map.get(request_params, name) do + nil -> Map.get(request_params, Atom.to_string(name)) + v -> v end - nil -> - {:error, :not_found} + case value do + nil -> {:cont, {:ok, [{name, default} | acc]}} + "" -> {:cont, {:ok, [{name, default} | acc]}} + v when is_binary(v) -> {:cont, {:ok, [{name, v} | acc]}} + _ -> {:halt, {:error, {:invalid_param, name}}} + end + end) + |> case do + {:ok, params} -> {:ok, Enum.sort_by(params, fn {name, _} -> name end)} + {:error, reason} -> {:error, reason} end end diff --git a/server_ex/lib/topical/topic.ex b/server_ex/lib/topical/topic.ex index 91d47e7..f333206 100644 --- a/server_ex/lib/topical/topic.ex +++ b/server_ex/lib/topical/topic.ex @@ -75,6 +75,19 @@ defmodule Topical.Topic do @doc false defmacro __using__(opts) do + route = Keyword.fetch!(opts, :route) + params = Keyword.get(opts, :params, []) + + # Compile-time validation: check for conflicts between param names and route placeholders + route_placeholders = for part <- route, is_atom(part), do: part + param_names = Keyword.keys(params) + conflicts = param_names -- (param_names -- route_placeholders) + + if conflicts != [] do + raise ArgumentError, + "param names conflict with route placeholders: #{inspect(conflicts)}" + end + quote location: :keep, bind_quoted: [opts: opts] do @behaviour Topical.Topic.Server @@ -84,6 +97,10 @@ defmodule Topical.Topic do unquote(Keyword.fetch!(opts, :route)) end + def params() do + unquote(Keyword.get(opts, :params, [])) + end + def authorize(_params, _context) do :ok end diff --git a/server_ex/lib/topical/topic/server.ex b/server_ex/lib/topical/topic/server.ex index 3d94eb4..b171e19 100644 --- a/server_ex/lib/topical/topic/server.ex +++ b/server_ex/lib/topical/topic/server.ex @@ -10,7 +10,8 @@ defmodule Topical.Topic.Server do @doc """ Invoked to check whether a client is authorized to access this topic. - `params` are the values associated with the placeholders in the route. + `params` are the values associated with the placeholders in the route, + merged with the request params (with defaults applied). `context` is the context established during the WebSocket connection. Return `:ok` to allow access, or `{:error, reason}` to deny. @@ -22,9 +23,10 @@ defmodule Topical.Topic.Server do @doc """ Invoked when the topic is started to get the initial state. - `params` are the values associated with the placeholders in the route. + `params` are the values associated with the placeholders in the route, + merged with the request params (with defaults applied). """ - @callback init(params :: [...]) :: {:ok, %Topic{}} | {:error, reason :: any} + @callback init(params :: keyword()) :: {:ok, %Topic{}} | {:error, reason :: any} @doc """ Invoked before a client subscribes (but after initialisation). diff --git a/server_ex/test/integration_test.exs b/server_ex/test/integration_test.exs index 3ebf145..d32e07d 100644 --- a/server_ex/test/integration_test.exs +++ b/server_ex/test/integration_test.exs @@ -17,7 +17,9 @@ defmodule Topical.IntegrationTest do Topical.Test.CallbackTopic, Topical.Test.FailingTopic, Topical.Test.ListTopic, - Topical.Test.MergeTopic + Topical.Test.MergeTopic, + Topical.Test.LeaderboardTopic, + Topical.Test.DocumentTopic ] ] ]} @@ -28,14 +30,14 @@ defmodule Topical.IntegrationTest do describe "subscribe/4" do test "subscriber receives reset message", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref, _server} = Topical.subscribe(registry, ["counters", "1"], self()) assert_receive {:reset, ^ref, %{count: 0}} end test "returns ref that matches messages", %{registry: registry} do - {:ok, ref1} = Topical.subscribe(registry, ["counters", "1"], self()) - {:ok, ref2} = Topical.subscribe(registry, ["counters", "2"], self()) + {:ok, ref1, _server1} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref2, _server2} = Topical.subscribe(registry, ["counters", "2"], self()) assert ref1 != ref2 @@ -44,7 +46,7 @@ defmodule Topical.IntegrationTest do end test "subscriber receives updates after subscribe", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref, _server} = Topical.subscribe(registry, ["counters", "1"], self()) assert_receive {:reset, ^ref, %{count: 0}} Topical.execute(registry, ["counters", "1"], "increment", {}) @@ -53,24 +55,17 @@ defmodule Topical.IntegrationTest do end end - describe "unsubscribe/3" do + describe "unsubscribe/2" do test "stops receiving updates after unsubscribe", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref, server} = Topical.subscribe(registry, ["counters", "1"], self()) assert_receive {:reset, ^ref, %{count: 0}} - :ok = Topical.unsubscribe(registry, ["counters", "1"], ref) + Topical.unsubscribe(server, ref) Topical.execute(registry, ["counters", "1"], "increment", {}) refute_receive {:updates, _, _}, 100 end - - test "returns error for non-running topic", %{registry: registry} do - ref = make_ref() - - assert {:error, :not_running} = - Topical.unsubscribe(registry, ["counters", "nonexistent"], ref) - end end describe "execute/5" do @@ -83,7 +78,7 @@ defmodule Topical.IntegrationTest do end test "subscribers receive updates from execute", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref, _server} = Topical.subscribe(registry, ["counters", "1"], self()) assert_receive {:reset, ^ref, %{count: 0}} {:ok, _} = Topical.execute(registry, ["counters", "1"], "set", {42}) @@ -99,7 +94,7 @@ defmodule Topical.IntegrationTest do describe "notify/5" do test "sends notification to topic", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref, _server} = Topical.subscribe(registry, ["counters", "1"], self()) assert_receive {:reset, ^ref, %{count: 0}} :ok = Topical.notify(registry, ["counters", "1"], "increment", {}) @@ -140,8 +135,8 @@ defmodule Topical.IntegrationTest do describe "multiple subscribers" do test "all subscribers receive same updates", %{registry: registry} do - {:ok, ref1} = Topical.subscribe(registry, ["counters", "1"], self()) - {:ok, ref2} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref1, _server1} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref2, _server2} = Topical.subscribe(registry, ["counters", "1"], self()) assert_receive {:reset, ^ref1, %{count: 0}} assert_receive {:reset, ^ref2, %{count: 0}} @@ -153,13 +148,13 @@ defmodule Topical.IntegrationTest do end test "unsubscribing one does not affect others", %{registry: registry} do - {:ok, ref1} = Topical.subscribe(registry, ["counters", "1"], self()) - {:ok, ref2} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref1, server} = Topical.subscribe(registry, ["counters", "1"], self()) + {:ok, ref2, _server2} = Topical.subscribe(registry, ["counters", "1"], self()) assert_receive {:reset, ^ref1, _} assert_receive {:reset, ^ref2, _} - Topical.unsubscribe(registry, ["counters", "1"], ref1) + Topical.unsubscribe(server, ref1) Topical.execute(registry, ["counters", "1"], "increment", {}) # ref1 should not receive update @@ -174,7 +169,7 @@ defmodule Topical.IntegrationTest do assert {:error, :forbidden} = Topical.subscribe(registry, ["private", "owner1"], self(), %{user_id: "other"}) - {:ok, _ref} = + {:ok, _ref, _server} = Topical.subscribe(registry, ["private", "owner1"], self(), %{user_id: "owner1"}) end @@ -223,7 +218,7 @@ defmodule Topical.IntegrationTest do describe "callback invocations" do test "handle_subscribe is called on subscribe", %{registry: registry} do context = %{user: "test"} - {:ok, ref} = Topical.subscribe(registry, ["callbacks", "1"], self(), context) + {:ok, ref, _server} = Topical.subscribe(registry, ["callbacks", "1"], self(), context) assert_receive {:reset, ^ref, %{callbacks: callbacks}} assert [{:subscribe, ^context}] = callbacks @@ -231,10 +226,10 @@ defmodule Topical.IntegrationTest do test "handle_unsubscribe is called on unsubscribe", %{registry: registry} do context = %{user: "test"} - {:ok, ref} = Topical.subscribe(registry, ["callbacks", "1"], self(), context) + {:ok, ref, server} = Topical.subscribe(registry, ["callbacks", "1"], self(), context) assert_receive {:reset, ^ref, _} - Topical.unsubscribe(registry, ["callbacks", "1"], ref) + Topical.unsubscribe(server, ref) # Give time for unsubscribe to process Process.sleep(50) @@ -284,32 +279,32 @@ defmodule Topical.IntegrationTest do describe "topic timeout" do test "topic stops after timeout with no subscribers", %{registry: registry} do - # Start a topic - {:ok, _} = Topical.execute(registry, ["counters", "timeout-test"], "increment", {}) + # Start a topic and get the server PID + {:ok, sub_ref, server} = Topical.subscribe(registry, ["counters", "timeout-test"], self()) + assert_receive {:reset, _, _} + assert Process.alive?(server) - {:ok, pid} = Topical.Registry.lookup_topic(registry, ["counters", "timeout-test"]) - assert Process.alive?(pid) + # Unsubscribe so there are no subscribers + Topical.unsubscribe(server, sub_ref) - # Wait for timeout (10 seconds is default, but we'll just check the behavior) # We use a reference to monitor the process - ref = Process.monitor(pid) + ref = Process.monitor(server) - # Topic should still be running after short delay - refute_receive {:DOWN, ^ref, :process, ^pid, _}, 100 + # Topic should still be running after short delay (timeout is 10 seconds) + refute_receive {:DOWN, ^ref, :process, ^server, _}, 100 # Clean up Process.demonitor(ref, [:flush]) end test "topic does not timeout while subscribed", %{registry: registry} do - {:ok, sub_ref} = Topical.subscribe(registry, ["counters", "sub-test"], self()) - assert_receive {:reset, ^sub_ref, _} + {:ok, _sub_ref, server} = Topical.subscribe(registry, ["counters", "sub-test"], self()) + assert_receive {:reset, _, _} - {:ok, pid} = Topical.Registry.lookup_topic(registry, ["counters", "sub-test"]) - mon_ref = Process.monitor(pid) + mon_ref = Process.monitor(server) # Should not timeout while subscribed - refute_receive {:DOWN, ^mon_ref, :process, ^pid, _}, 200 + refute_receive {:DOWN, ^mon_ref, :process, ^server, _}, 200 Process.demonitor(mon_ref, [:flush]) end @@ -317,7 +312,7 @@ defmodule Topical.IntegrationTest do describe "list operations" do test "insert operations broadcast to subscribers", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["lists", "1"], self()) + {:ok, ref, _server} = Topical.subscribe(registry, ["lists", "1"], self()) assert_receive {:reset, ^ref, %{items: [], next_id: 1}} {:ok, 1} = Topical.execute(registry, ["lists", "1"], "add", {"first"}) @@ -327,7 +322,7 @@ defmodule Topical.IntegrationTest do end test "delete operations broadcast to subscribers", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["lists", "1"], self()) + {:ok, ref, _server} = Topical.subscribe(registry, ["lists", "1"], self()) assert_receive {:reset, ^ref, _} {:ok, _} = Topical.execute(registry, ["lists", "1"], "add", {"first"}) @@ -344,7 +339,7 @@ defmodule Topical.IntegrationTest do describe "merge operations" do test "merge operations broadcast to subscribers", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["merge", "1"], self()) + {:ok, ref, _server} = Topical.subscribe(registry, ["merge", "1"], self()) assert_receive {:reset, ^ref, %{data: %{}}} {:ok, _} = Topical.execute(registry, ["merge", "1"], "merge", {%{a: 1, b: 2}}) @@ -353,7 +348,7 @@ defmodule Topical.IntegrationTest do end test "unset operations broadcast to subscribers", %{registry: registry} do - {:ok, ref} = Topical.subscribe(registry, ["merge", "1"], self()) + {:ok, ref, _server} = Topical.subscribe(registry, ["merge", "1"], self()) assert_receive {:reset, ^ref, _} {:ok, _} = Topical.execute(registry, ["merge", "1"], "set", {:key, "value"}) @@ -372,7 +367,7 @@ defmodule Topical.IntegrationTest do subscriber = spawn(fn -> - {:ok, _ref} = Topical.subscribe(registry, ["callbacks", "death-test"], self()) + {:ok, _ref, _server} = Topical.subscribe(registry, ["callbacks", "death-test"], self()) send(test_pid, :subscribed) receive do diff --git a/server_ex/test/support/test_topics.ex b/server_ex/test/support/test_topics.ex index c73012d..169d7b5 100644 --- a/server_ex/test/support/test_topics.ex +++ b/server_ex/test/support/test_topics.ex @@ -243,3 +243,88 @@ defmodule Topical.Test.MergeTopic do {:ok, :ok, topic} end end + +defmodule Topical.Test.LeaderboardTopic do + @moduledoc """ + Leaderboard topic for testing params feature. + Route: ["leaderboards", :game_id] + Params: [region: "global"] + + Different regions have separate leaderboards - global vs regional rankings. + """ + use Topical.Topic, route: ["leaderboards", :game_id], params: [region: "global"] + + def init(params) do + game_id = Keyword.fetch!(params, :game_id) + region = Keyword.fetch!(params, :region) + + value = %{ + game_id: game_id, + region: region, + entries: [] + } + + {:ok, Topic.new(value)} + end + + def handle_execute("get_info", {}, topic, _context) do + result = %{ + game_id: topic.value.game_id, + region: topic.value.region + } + + {:ok, result, topic} + end + + def handle_execute("add_score", {player, score}, topic, _context) do + entry = %{player: player, score: score} + topic = Topic.insert(topic, [:entries], entry) + {:ok, :ok, topic} + end + + def handle_notify("add_score", {player, score}, topic, _context) do + entry = %{player: player, score: score} + topic = Topic.insert(topic, [:entries], entry) + {:ok, topic} + end +end + +defmodule Topical.Test.DocumentTopic do + @moduledoc """ + Document topic for testing authorization with params. + Route: ["documents", :doc_id] + Params: [mode: "view"] + + Viewing a document is allowed for anyone, but editing requires :can_edit permission. + """ + use Topical.Topic, route: ["documents", :doc_id], params: [mode: "view"] + + def authorize(params, context) do + mode = Keyword.fetch!(params, :mode) + + cond do + mode == "edit" and context[:can_edit] != true -> + {:error, :edit_not_allowed} + + true -> + :ok + end + end + + def init(params) do + doc_id = Keyword.fetch!(params, :doc_id) + mode = Keyword.fetch!(params, :mode) + + value = %{doc_id: doc_id, mode: mode, content: ""} + {:ok, Topic.new(value)} + end + + def handle_execute("get_mode", {}, topic, _context) do + {:ok, topic.value.mode, topic} + end + + def handle_execute("set_content", {content}, topic, _context) do + topic = Topic.set(topic, [:content], content) + {:ok, :ok, topic} + end +end diff --git a/server_ex/test/topical/params_test.exs b/server_ex/test/topical/params_test.exs new file mode 100644 index 0000000..32f5b09 --- /dev/null +++ b/server_ex/test/topical/params_test.exs @@ -0,0 +1,339 @@ +defmodule Topical.ParamsTest do + use ExUnit.Case + + alias Topical.Registry + + describe "compile-time validation" do + test "raises when param name conflicts with route placeholder" do + assert_raise ArgumentError, ~r/param names conflict with route placeholders/, fn -> + defmodule ConflictingTopic do + # This should fail because :id is both in route and params + use Topical.Topic, route: ["things", :id], params: [id: "default"] + end + end + end + end + + setup do + registry_name = :"params_registry_#{System.unique_integer([:positive])}" + + start_supervised!(%{ + id: registry_name, + start: + {Registry, :start_link, + [ + [ + name: registry_name, + topics: [ + Topical.Test.CounterTopic, + Topical.Test.LeaderboardTopic, + Topical.Test.DocumentTopic + ] + ] + ]} + }) + + {:ok, registry: registry_name} + end + + describe "params normalization" do + test "uses default values when params not provided", %{registry: registry} do + {:ok, result} = Topical.execute(registry, ["leaderboards", "chess"], "get_info", {}) + + assert result.game_id == "chess" + assert result.region == "global" + end + + test "uses provided params over defaults", %{registry: registry} do + params = %{"region" => "eu"} + + {:ok, result} = + Topical.execute(registry, ["leaderboards", "chess"], "get_info", {}, nil, params) + + assert result.game_id == "chess" + assert result.region == "eu" + end + + test "ignores unknown params", %{registry: registry} do + params = %{"region" => "na", "unknown_param" => "ignored"} + + {:ok, result} = + Topical.execute(registry, ["leaderboards", "chess"], "get_info", {}, nil, params) + + assert result.region == "na" + # unknown_param is silently ignored + end + + test "normalizes empty string to default", %{registry: registry} do + params = %{"region" => ""} + + {:ok, result} = + Topical.execute(registry, ["leaderboards", "chess"], "get_info", {}, nil, params) + + # Empty string should use default + assert result.region == "global" + end + + test "returns error for non-string param values", %{registry: registry} do + # Integer value + params = %{"region" => 123} + + assert {:error, {:invalid_param, :region}} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, params) + + # Boolean value + params = %{"region" => true} + + assert {:error, {:invalid_param, :region}} = + Topical.execute(registry, ["leaderboards", "chess"], "get_info", {}, nil, params) + + # List value + params = %{"region" => ["eu", "na"]} + + assert {:error, {:invalid_param, :region}} = + Topical.capture(registry, ["leaderboards", "chess"], nil, params) + + # Map value + params = %{"region" => %{"code" => "eu"}} + + assert {:error, {:invalid_param, :region}} = + Topical.notify( + registry, + ["leaderboards", "chess"], + "add_score", + {"alice", 100}, + nil, + params + ) + end + + test "accepts both string and atom keys in params", %{registry: registry} do + params = %{region: "asia"} + + {:ok, result} = + Topical.execute(registry, ["leaderboards", "chess"], "get_info", {}, nil, params) + + assert result.region == "asia" + end + end + + describe "topic identity with params" do + test "different regions create different leaderboard instances", %{registry: registry} do + global_params = %{"region" => "global"} + eu_params = %{"region" => "eu"} + + {:ok, ref_global, _server1} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, global_params) + + {:ok, ref_eu, _server2} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, eu_params) + + assert_receive {:reset, ^ref_global, value_global} + assert_receive {:reset, ^ref_eu, value_eu} + + assert value_global.region == "global" + assert value_eu.region == "eu" + + # Add score to global leaderboard + Topical.execute( + registry, + ["leaderboards", "chess"], + "add_score", + {"alice", 100}, + nil, + global_params + ) + + # Only global subscribers should receive update + assert_receive {:updates, ^ref_global, _} + refute_receive {:updates, ^ref_eu, _}, 100 + end + + test "same region resolves to same leaderboard instance", %{registry: registry} do + params = %{"region" => "na"} + + {:ok, ref1, _server1} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, params) + + {:ok, ref2, _server2} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, params) + + assert_receive {:reset, ^ref1, _} + assert_receive {:reset, ^ref2, _} + + # Add score + Topical.execute(registry, ["leaderboards", "chess"], "add_score", {"bob", 200}, nil, params) + + # Both refs should receive update (same leaderboard) + assert_receive {:updates, ^ref1, _} + assert_receive {:updates, ^ref2, _} + end + + test "default params and explicit default params resolve to same topic", %{registry: registry} do + # No params (uses default region: "global") + {:ok, ref1, _server1} = Topical.subscribe(registry, ["leaderboards", "chess"], self()) + # Explicit default params + {:ok, ref2, _server2} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, %{ + "region" => "global" + }) + + assert_receive {:reset, ^ref1, _} + assert_receive {:reset, ^ref2, _} + + # Add score with no params (uses defaults) + Topical.execute(registry, ["leaderboards", "chess"], "add_score", {"charlie", 150}) + + # Both should receive update (same topic) + assert_receive {:updates, ^ref1, _} + assert_receive {:updates, ^ref2, _} + end + end + + describe "authorization with params" do + test "view mode works without special permissions", %{registry: registry} do + # View mode should work without context + {:ok, _ref, _server} = Topical.subscribe(registry, ["documents", "doc1"], self()) + assert_receive {:reset, _, %{mode: "view"}} + end + + test "edit mode requires can_edit permission", %{registry: registry} do + # Edit mode without can_edit context should fail + params = %{"mode" => "edit"} + + assert {:error, :edit_not_allowed} = + Topical.subscribe(registry, ["documents", "doc1"], self(), nil, params) + end + + test "edit mode allowed with can_edit permission", %{registry: registry} do + params = %{"mode" => "edit"} + context = %{can_edit: true} + + {:ok, _ref, _server} = + Topical.subscribe(registry, ["documents", "doc1"], self(), context, params) + + assert_receive {:reset, _, %{mode: "edit"}} + end + end + + describe "unsubscribe with params" do + test "unsubscribe uses correct topic based on params", %{registry: registry} do + global_params = %{"region" => "global"} + eu_params = %{"region" => "eu"} + + {:ok, ref_global, server_global} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, global_params) + + {:ok, ref_eu, _server_eu} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, eu_params) + + assert_receive {:reset, ^ref_global, _} + assert_receive {:reset, ^ref_eu, _} + + # Unsubscribe from global + :ok = Topical.unsubscribe(server_global, ref_global) + + # Add to global - ref_global should not receive (unsubscribed) + Topical.execute( + registry, + ["leaderboards", "chess"], + "add_score", + {"alice", 100}, + nil, + global_params + ) + + refute_receive {:updates, ^ref_global, _}, 100 + + # Add to EU - ref_eu should still receive + Topical.execute( + registry, + ["leaderboards", "chess"], + "add_score", + {"bob", 200}, + nil, + eu_params + ) + + assert_receive {:updates, ^ref_eu, _} + end + end + + describe "capture with params" do + test "capture uses params to find correct topic", %{registry: registry} do + params = %{"region" => "asia"} + + # Create topic by subscribing + {:ok, _ref, _server} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, params) + + assert_receive {:reset, _, _} + + # Add a score + Topical.execute( + registry, + ["leaderboards", "chess"], + "add_score", + {"yuki", 300}, + nil, + params + ) + + # Capture with same params should get the modified state + {:ok, value} = Topical.capture(registry, ["leaderboards", "chess"], nil, params) + assert value.region == "asia" + assert length(value.entries) == 1 + assert hd(value.entries).player == "yuki" + end + end + + describe "notify with params" do + test "notify targets correct topic based on params", %{registry: registry} do + global_params = %{"region" => "global"} + eu_params = %{"region" => "eu"} + + {:ok, ref_global, _server1} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, global_params) + + {:ok, ref_eu, _server2} = + Topical.subscribe(registry, ["leaderboards", "chess"], self(), nil, eu_params) + + assert_receive {:reset, ^ref_global, _} + assert_receive {:reset, ^ref_eu, _} + + # Notify on global leaderboard + :ok = + Topical.notify( + registry, + ["leaderboards", "chess"], + "add_score", + {"alice", 100}, + nil, + global_params + ) + + # Only global subscriber should receive update + assert_receive {:updates, ^ref_global, _} + refute_receive {:updates, ^ref_eu, _}, 100 + end + end + + describe "topics without params" do + test "topics without params work as before", %{registry: registry} do + # CounterTopic has no params defined + {:ok, ref, _server} = Topical.subscribe(registry, ["counters", "1"], self()) + assert_receive {:reset, ^ref, %{count: 0}} + + {:ok, 1} = Topical.execute(registry, ["counters", "1"], "increment", {}) + assert_receive {:updates, ^ref, [{:set, [:count], 1}]} + end + + test "passing params to topic without declared params ignores them", %{registry: registry} do + # CounterTopic has no params, so these should be ignored + params = %{"ignored" => "value"} + + {:ok, ref, _server} = Topical.subscribe(registry, ["counters", "1"], self(), nil, params) + assert_receive {:reset, ^ref, %{count: 0}} + end + end +end diff --git a/server_ex/test/topical/protocol_test.exs b/server_ex/test/topical/protocol_test.exs index e2b4300..ca09a64 100644 --- a/server_ex/test/topical/protocol_test.exs +++ b/server_ex/test/topical/protocol_test.exs @@ -7,19 +7,40 @@ defmodule Topical.ProtocolTest do test "decodes notify request" do json = Jason.encode!([0, ["lists", "1"], "add", ["test"]]) - assert {:ok, :notify, ["lists", "1"], "add", ["test"]} = Request.decode(json) + assert {:ok, :notify, ["lists", "1"], "add", ["test"], %{}} = Request.decode(json) + end + + test "decodes notify request with params" do + json = Jason.encode!([0, ["lists", "1"], "add", ["test"], %{"layer" => "bg"}]) + + assert {:ok, :notify, ["lists", "1"], "add", ["test"], %{"layer" => "bg"}} = + Request.decode(json) end test "decodes execute request" do json = Jason.encode!([1, "ch1", ["counters", "1"], "increment", []]) - assert {:ok, :execute, "ch1", ["counters", "1"], "increment", []} = Request.decode(json) + assert {:ok, :execute, "ch1", ["counters", "1"], "increment", [], %{}} = + Request.decode(json) + end + + test "decodes execute request with params" do + json = Jason.encode!([1, "ch1", ["counters", "1"], "increment", [], %{"ns" => "test"}]) + + assert {:ok, :execute, "ch1", ["counters", "1"], "increment", [], %{"ns" => "test"}} = + Request.decode(json) end test "decodes subscribe request" do json = Jason.encode!([2, "ch1", ["lists", "abc"]]) - assert {:ok, :subscribe, "ch1", ["lists", "abc"]} = Request.decode(json) + assert {:ok, :subscribe, "ch1", ["lists", "abc"], %{}} = Request.decode(json) + end + + test "decodes subscribe request with params" do + json = Jason.encode!([2, "ch1", ["lists", "abc"], %{"layer" => "fg"}]) + + assert {:ok, :subscribe, "ch1", ["lists", "abc"], %{"layer" => "fg"}} = Request.decode(json) end test "decodes unsubscribe request" do @@ -54,19 +75,19 @@ defmodule Topical.ProtocolTest do args = %{"name" => "Test", "items" => [1, 2, 3]} json = Jason.encode!([0, ["topic"], "action", args]) - assert {:ok, :notify, ["topic"], "action", ^args} = Request.decode(json) + assert {:ok, :notify, ["topic"], "action", ^args, %{}} = Request.decode(json) end test "decodes execute with string channel_id" do json = Jason.encode!([1, "channel-123", ["topic"], "action", []]) - assert {:ok, :execute, "channel-123", ["topic"], "action", []} = Request.decode(json) + assert {:ok, :execute, "channel-123", ["topic"], "action", [], %{}} = Request.decode(json) end test "decodes execute with integer channel_id" do json = Jason.encode!([1, 42, ["topic"], "action", []]) - assert {:ok, :execute, 42, ["topic"], "action", []} = Request.decode(json) + assert {:ok, :execute, 42, ["topic"], "action", [], %{}} = Request.decode(json) end end diff --git a/server_ex/test/topical/registry_test.exs b/server_ex/test/topical/registry_test.exs index 8d48e65..24fc416 100644 --- a/server_ex/test/topical/registry_test.exs +++ b/server_ex/test/topical/registry_test.exs @@ -5,6 +5,13 @@ defmodule Topical.RegistryTest do alias Topical.Registry + # Helper to resolve and get topic in one call (for test convenience) + defp resolve_and_get_topic(registry, route, context, params \\ %{}) do + with {:ok, module, all_params, topic_key} <- Registry.resolve_topic(registry, route, params) do + Registry.get_topic(registry, module, all_params, topic_key, context) + end + end + setup do registry_name = :"test_registry_#{System.unique_integer([:positive])}" @@ -21,7 +28,9 @@ defmodule Topical.RegistryTest do Topical.Test.CallbackTopic, Topical.Test.FailingTopic, Topical.Test.ListTopic, - Topical.Test.MergeTopic + Topical.Test.MergeTopic, + Topical.Test.LeaderboardTopic, + Topical.Test.DocumentTopic ] ] ]} @@ -30,50 +39,39 @@ defmodule Topical.RegistryTest do {:ok, registry: registry_name} end - describe "lookup_topic/2" do - test "returns {:error, :not_running} for non-existent topic", %{registry: registry} do - assert {:error, :not_running} = Registry.lookup_topic(registry, ["counters", "nonexistent"]) - end - - test "returns {:ok, pid} for running topic", %{registry: registry} do - {:ok, pid} = Registry.get_topic(registry, ["counters", "1"], nil) - assert {:ok, ^pid} = Registry.lookup_topic(registry, ["counters", "1"]) - end - end - - describe "get_topic/3" do + describe "resolve_topic and get_topic" do test "starts topic on first access", %{registry: registry} do - {:ok, pid} = Registry.get_topic(registry, ["counters", "1"], nil) + {:ok, pid} = resolve_and_get_topic(registry, ["counters", "1"], nil) assert is_pid(pid) assert Process.alive?(pid) end test "returns same pid for same route", %{registry: registry} do - {:ok, pid1} = Registry.get_topic(registry, ["counters", "1"], nil) - {:ok, pid2} = Registry.get_topic(registry, ["counters", "1"], nil) + {:ok, pid1} = resolve_and_get_topic(registry, ["counters", "1"], nil) + {:ok, pid2} = resolve_and_get_topic(registry, ["counters", "1"], nil) assert pid1 == pid2 end test "returns different pids for different routes", %{registry: registry} do - {:ok, pid1} = Registry.get_topic(registry, ["counters", "1"], nil) - {:ok, pid2} = Registry.get_topic(registry, ["counters", "2"], nil) + {:ok, pid1} = resolve_and_get_topic(registry, ["counters", "1"], nil) + {:ok, pid2} = resolve_and_get_topic(registry, ["counters", "2"], nil) assert pid1 != pid2 end test "returns {:error, :not_found} for unknown route", %{registry: registry} do - assert {:error, :not_found} = Registry.get_topic(registry, ["unknown", "route"], nil) + assert {:error, :not_found} = Registry.resolve_topic(registry, ["unknown", "route"]) end test "returns {:error, :not_found} for partial route match", %{registry: registry} do - assert {:error, :not_found} = Registry.get_topic(registry, ["counters"], nil) + assert {:error, :not_found} = Registry.resolve_topic(registry, ["counters"]) end test "returns {:error, :not_found} for too long route", %{registry: registry} do - assert {:error, :not_found} = Registry.get_topic(registry, ["counters", "1", "extra"], nil) + assert {:error, :not_found} = Registry.resolve_topic(registry, ["counters", "1", "extra"]) end test "passes params to topic init", %{registry: registry} do - {:ok, pid} = Registry.get_topic(registry, ["counters", "my-id"], nil) + {:ok, pid} = resolve_and_get_topic(registry, ["counters", "my-id"], nil) state = :sys.get_state(pid) assert state.topic.state.id == "my-id" end @@ -81,22 +79,22 @@ defmodule Topical.RegistryTest do describe "route matching" do test "matches static route parts", %{registry: registry} do - {:ok, _pid} = Registry.get_topic(registry, ["counters", "test"], nil) + {:ok, _pid} = resolve_and_get_topic(registry, ["counters", "test"], nil) end test "captures placeholder values", %{registry: registry} do - {:ok, pid} = Registry.get_topic(registry, ["private", "user123"], %{user_id: "user123"}) + {:ok, pid} = resolve_and_get_topic(registry, ["private", "user123"], %{user_id: "user123"}) state = :sys.get_state(pid) assert state.topic.value.owner == "user123" end test "accepts string route format", %{registry: registry} do - {:ok, pid} = Registry.get_topic(registry, "counters/1", nil) + {:ok, pid} = resolve_and_get_topic(registry, "counters/1", nil) assert is_pid(pid) end test "accepts URI-encoded route parts", %{registry: registry} do - {:ok, pid} = Registry.get_topic(registry, "counters/hello%20world", nil) + {:ok, pid} = resolve_and_get_topic(registry, "counters/hello%20world", nil) state = :sys.get_state(pid) assert state.topic.state.id == "hello world" end @@ -105,35 +103,44 @@ defmodule Topical.RegistryTest do describe "authorization" do test "allows access when authorized", %{registry: registry} do context = %{user_id: "owner1"} - assert {:ok, _pid} = Registry.get_topic(registry, ["private", "owner1"], context) + assert {:ok, _pid} = resolve_and_get_topic(registry, ["private", "owner1"], context) end test "denies access when unauthorized", %{registry: registry} do context = %{user_id: "other"} - assert {:error, :forbidden} = Registry.get_topic(registry, ["private", "owner1"], context) + + assert {:error, :forbidden} = + resolve_and_get_topic(registry, ["private", "owner1"], context) end test "denies access when context is nil for authorized topic", %{registry: registry} do - assert {:error, :unauthorized} = Registry.get_topic(registry, ["private", "owner1"], nil) + assert {:error, :unauthorized} = resolve_and_get_topic(registry, ["private", "owner1"], nil) end test "runs authorize before starting topic", %{registry: registry} do context = %{user_id: "wrong"} - assert {:error, :forbidden} = Registry.get_topic(registry, ["private", "owner1"], context) - # Topic should not have been started - assert {:error, :not_running} = Registry.lookup_topic(registry, ["private", "owner1"]) + assert {:error, :forbidden} = + resolve_and_get_topic(registry, ["private", "owner1"], context) + + # Topic should not have been started - trying to get it again with correct + # context should start a new topic (proving it wasn't started before) + context2 = %{user_id: "owner1"} + {:ok, pid} = resolve_and_get_topic(registry, ["private", "owner1"], context2) + assert Process.alive?(pid) end test "authorize is called on every get_topic call", %{registry: registry} do # First call should succeed and start topic context1 = %{user_id: "owner1"} - {:ok, pid} = Registry.get_topic(registry, ["private", "owner1"], context1) + {:ok, pid} = resolve_and_get_topic(registry, ["private", "owner1"], context1) assert Process.alive?(pid) # Second call with wrong context should still fail context2 = %{user_id: "wrong"} - assert {:error, :forbidden} = Registry.get_topic(registry, ["private", "owner1"], context2) + + assert {:error, :forbidden} = + resolve_and_get_topic(registry, ["private", "owner1"], context2) end end @@ -142,7 +149,7 @@ defmodule Topical.RegistryTest do # Capture the expected SASL error log from GenServer init failure capture_log(fn -> assert {:error, :init_failed} = - Registry.get_topic(registry, ["failing", "init_error"], nil) + resolve_and_get_topic(registry, ["failing", "init_error"], nil) end) end end @@ -168,8 +175,8 @@ defmodule Topical.RegistryTest do id: :reg2 ) - {:ok, pid1} = Registry.get_topic(registry1, ["counters", "1"], nil) - {:ok, pid2} = Registry.get_topic(registry2, ["counters", "1"], nil) + {:ok, pid1} = resolve_and_get_topic(registry1, ["counters", "1"], nil) + {:ok, pid2} = resolve_and_get_topic(registry2, ["counters", "1"], nil) assert pid1 != pid2 end