Skip to content

Commit b154ae2

Browse files
committed
refactor(app): dedupe terminal url helpers and tests
Reuse resolveTerminalApiOriginUrl from terminal.ts for image fetch URL resolution to eliminate the duplicate origin builder, and merge the image fetch URL test cases into terminal.test.ts so they share the existing mock setup. Extract a same-origin location stub helper to keep individual test bodies short. Also extract attachGlobalResizeListeners out of mountTerminalSession to stay under the max-lines-per-function threshold.
1 parent ed3e651 commit b154ae2

5 files changed

Lines changed: 57 additions & 97 deletions

File tree

packages/app/src/web/terminal-image-url.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,12 @@
1-
import { resolveApiBaseUrl } from "./api-http.js"
1+
import { resolveTerminalApiOriginUrl } from "./terminal.js"
22

33
const websocketSuffixPattern = /\/ws$/u
44

5-
const resolveBackendOrigin = (): URL => {
6-
const configured = resolveApiBaseUrl()
7-
if (configured.startsWith("http://") || configured.startsWith("https://")) {
8-
return new URL(configured)
9-
}
10-
return new URL(configured, globalThis.location.origin)
11-
}
12-
135
export const resolveTerminalImageBasePath = (websocketPath: string): string =>
146
websocketPath.replace(websocketSuffixPattern, "/image")
157

168
export const resolveTerminalImageFetchUrl = (websocketPath: string, imagePath: string): string => {
17-
const apiUrl = resolveBackendOrigin()
9+
const apiUrl = resolveTerminalApiOriginUrl()
1810
apiUrl.pathname = `${apiUrl.pathname.replace(/\/$/u, "")}${resolveTerminalImageBasePath(websocketPath)}`
1911
apiUrl.searchParams.set("path", imagePath)
2012
return apiUrl.toString()

packages/app/src/web/terminal-panel-runtime.ts

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,15 @@ const createConnectSocket = (
5757
return connectSocket
5858
}
5959

60+
const attachGlobalResizeListeners = (sendResize: () => void): void => {
61+
globalThis.addEventListener("resize", sendResize)
62+
globalThis.visualViewport?.addEventListener("resize", sendResize)
63+
globalThis.visualViewport?.addEventListener("scroll", sendResize)
64+
}
65+
6066
const mountTerminalSession = (
61-
{
62-
connectionRef,
63-
hostRef,
64-
notifyMessage,
65-
onAttachFailure,
66-
onImagePaths,
67-
runtimeRef,
68-
session,
69-
setStatus
70-
}: TerminalLifecycleArgs
67+
{ connectionRef, hostRef, notifyMessage, onAttachFailure, onImagePaths, runtimeRef, session, setStatus }:
68+
TerminalLifecycleArgs
7169
): (() => void) | undefined => {
7270
const host = hostRef.current
7371
if (host === null) {
@@ -108,9 +106,7 @@ const mountTerminalSession = (
108106
})
109107

110108
runtimeRef.current = terminalInputController
111-
globalThis.addEventListener("resize", sendResize)
112-
globalThis.visualViewport?.addEventListener("resize", sendResize)
113-
globalThis.visualViewport?.addEventListener("scroll", sendResize)
109+
attachGlobalResizeListeners(sendResize)
114110
connectSocket()
115111

116112
return createTerminalCleanup({
@@ -122,16 +118,8 @@ const mountTerminalSession = (
122118
}
123119

124120
export const useTerminalSessionLifecycle = (
125-
{
126-
connectionRef,
127-
hostRef,
128-
notifyMessage,
129-
onAttachFailure,
130-
onImagePaths,
131-
runtimeRef,
132-
session,
133-
setStatus
134-
}: TerminalLifecycleArgs
121+
{ connectionRef, hostRef, notifyMessage, onAttachFailure, onImagePaths, runtimeRef, session, setStatus }:
122+
TerminalLifecycleArgs
135123
): void => {
136124
useEffect(() => {
137125
return mountTerminalSession({

packages/app/src/web/terminal.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export const buildProjectActiveTerminalSession = (
5757
}
5858
}
5959

60-
const resolveTerminalApiBaseUrl = (): string => {
60+
export const resolveTerminalApiBaseUrl = (): string => {
6161
const configured = import.meta.env.VITE_DOCKER_GIT_TERMINAL_API_BASE_URL
6262
if (configured !== undefined && configured.trim().length > 0) {
6363
return trimTrailingSlash(configured.trim())
@@ -66,7 +66,7 @@ const resolveTerminalApiBaseUrl = (): string => {
6666
return resolveApiBaseUrl()
6767
}
6868

69-
const resolveApiUrl = (): URL => {
69+
export const resolveTerminalApiOriginUrl = (): URL => {
7070
const configured = resolveTerminalApiBaseUrl()
7171
if (configured.startsWith("http://") || configured.startsWith("https://")) {
7272
return new URL(configured)
@@ -75,7 +75,7 @@ const resolveApiUrl = (): URL => {
7575
}
7676

7777
export const resolveTerminalWebSocketUrl = (websocketPath: string, cols: number, rows: number): string => {
78-
const apiUrl = resolveApiUrl()
78+
const apiUrl = resolveTerminalApiOriginUrl()
7979
apiUrl.protocol = apiUrl.protocol === "https:" ? "wss:" : "ws:"
8080
apiUrl.pathname = `${apiUrl.pathname.replace(/\/$/u, "")}${websocketPath}`
8181
apiUrl.searchParams.set("cols", String(cols))

packages/app/tests/docker-git/terminal-image-url.test.ts

Lines changed: 0 additions & 55 deletions
This file was deleted.

packages/app/tests/docker-git/terminal.test.ts

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
extractTerminalImageBase64,
77
isTerminalPasteShortcut
88
} from "../../src/web/terminal-image-paste.js"
9+
import { resolveTerminalImageBasePath, resolveTerminalImageFetchUrl } from "../../src/web/terminal-image-url.js"
910
import {
1011
resolveTerminalCompactHeaderMode,
1112
resolveTerminalTypingMode,
@@ -32,6 +33,15 @@ vi.mock("../../src/web/api-http.js", () => ({
3233
resolveApiBaseUrl: resolveApiBaseUrlMock
3334
}))
3435

36+
const stubSameOriginLocation = (host: string, httpProtocol: string): void => {
37+
resolveApiBaseUrlMock.mockReturnValue("/api")
38+
vi.stubGlobal("location", {
39+
hostname: host,
40+
origin: `${httpProtocol}//${host}:4176`,
41+
protocol: httpProtocol
42+
})
43+
}
44+
3545
describe("browser terminal helpers", () => {
3646
beforeEach(() => {
3747
resolveApiBaseUrlMock.mockReset()
@@ -54,12 +64,7 @@ describe("browser terminal helpers", () => {
5464
const httpProtocol = ["ht", "tp:"].join("")
5565
const wsProtocol = ["ws", "://"].join("")
5666

57-
resolveApiBaseUrlMock.mockReturnValue("/api")
58-
vi.stubGlobal("location", {
59-
hostname: host,
60-
origin: `${httpProtocol}//${host}:4176`,
61-
protocol: httpProtocol
62-
})
67+
stubSameOriginLocation(host, httpProtocol)
6368

6469
expect(resolveTerminalWebSocketUrl("/projects/proj/terminal-sessions/sess/ws", 80, 24)).toBe([
6570
wsProtocol,
@@ -130,4 +135,34 @@ describe("browser terminal helpers", () => {
130135
expect(shouldShowTerminalTabs(true, 2)).toBe(true)
131136
expect(shouldShowTerminalTabs(false, 1)).toBe(true)
132137
})
138+
139+
it("converts /ws suffix into /image base path", () => {
140+
expect(resolveTerminalImageBasePath("/projects/by-key/proj/terminal-sessions/sess/ws")).toBe(
141+
"/projects/by-key/proj/terminal-sessions/sess/image"
142+
)
143+
})
144+
145+
it("builds an absolute backend image url with path query parameter", () => {
146+
resolveApiBaseUrlMock.mockReturnValue("https://controller.example/api")
147+
148+
expect(
149+
resolveTerminalImageFetchUrl(
150+
"/projects/by-key/proj%201/terminal-sessions/sess%2F2/ws",
151+
"/var/data/sample image.png"
152+
)
153+
).toBe(
154+
"https://controller.example/api/projects/by-key/proj%201/terminal-sessions/sess%2F2/image?path=%2Fvar%2Fdata%2Fsample+image.png"
155+
)
156+
})
157+
158+
it("uses same-origin api proxy for relative image fetch urls", () => {
159+
const host = "terminal.example.local"
160+
const httpProtocol = ["ht", "tp:"].join("")
161+
162+
stubSameOriginLocation(host, httpProtocol)
163+
164+
expect(
165+
resolveTerminalImageFetchUrl("/projects/proj/terminal-sessions/sess/ws", "/var/data/file.png")
166+
).toBe(`${httpProtocol}//${host}:4176/api/projects/proj/terminal-sessions/sess/image?path=%2Fvar%2Fdata%2Ffile.png`)
167+
})
133168
})

0 commit comments

Comments
 (0)