You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/08/10 01:48:23 UTC
(texera) branch gh-readonly-queue/main/pr-7384-fceef70e32260c9dc29762a2f70d87f8fef03f3b created (now e328232b43)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-7384-fceef70e32260c9dc29762a2f70d87f8fef03f3b
in repository https://gitbox.apache.org/repos/asf/texera.git
at e328232b43 test(agent-service): cover the auth, workflow, and backend API clients (#7384)
This branch includes the following new commits:
new e328232b43 test(agent-service): cover the auth, workflow, and backend API clients (#7384)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(texera) 01/01: test(agent-service): cover the auth, workflow, and backend API clients (#7384)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7384-fceef70e32260c9dc29762a2f70d87f8fef03f3b
in repository https://gitbox.apache.org/repos/asf/texera.git
commit e328232b436b3c57936f1ef8618a14b5e87ba559
Author: Xinyuan Lin <xi...@uci.edu>
AuthorDate: Sun Aug 9 18:48:03 2026 -0700
test(agent-service): cover the auth, workflow, and backend API clients (#7384)
### What changes were proposed in this PR?
Three of the four API clients under `agent-service/src/api` had no spec.
`auth-api.ts` is the one that decides whether a request is authenticated
at all, and none of its decisions were pinned.
Adds 30 tests across three spec files, following the `fetch`-spy pattern
already established by `compile-api.spec.ts`.
**auth-api** — several of these are policy choices that read like
oversights, so the tests state the intent rather than just the
behaviour:
| Input | Result |
|---|---|
| token with no `exp` | valid — tokens minted without an expiry never
expire |
| malformed token | invalid — the decode error is swallowed and reported
as expired, not thrown |
| payload with no `role` | `REGULAR`, so absent means least privilege |
| `bearer` / `BEARER` | accepted; the scheme is matched
case-insensitively |
| two-segment token whose payload parses | rejected |
**workflow-api** — the workflow `content` round-trips as a nested JSON
**string**: the request sends `JSON.stringify(content)` and the response
is re-parsed when it comes back as a string. Sending the object directly
is the obvious-looking mistake and the backend rejects it, so both
directions are pinned, along with the empty-description default and the
error text on a refused save or a missing workflow.
**backend-api** — the endpoint set, the defensive copy of the
module-level config, and the two failure paths of the metadata fetch.
**Verified by mutation**, all reverted (production diff empty):
| Mutation | Result |
|---|---|
| default a missing role to `ADMIN` | red |
| remove the three-segment check | red |
| treat a token with no `exp` as expired | red |
| compare `exp` as milliseconds instead of seconds | red |
| make the Bearer scheme case-sensitive | red |
| report a malformed token as valid | red |
| send `content` as a nested object | red |
| drop the empty-description default | red |
| stop re-parsing a stringified response `content` | red |
| drop the wid from the retrieve URL | red |
| return the shared config by reference | red |
The three-segment mutation initially **survived**: the test used
`"only.two"`, whose payload fails `JSON.parse` regardless, so the
segment check was never actually exercised. Replaced with a two-segment
token carrying a valid payload — an unsigned token — which is the case
the check exists for.
No production file is touched.
### Any related issues, documentation, discussions?
Closes #7381
### How was this PR tested?
```
bun test
```
```
232 pass
0 fail
Ran 232 tests across 18 files.
```
`bun run typecheck` and `bun run format:check` both pass.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
---
agent-service/src/api/auth-api.spec.ts | 137 ++++++++++++++++++++++++++
agent-service/src/api/backend-api.spec.ts | 78 +++++++++++++++
agent-service/src/api/workflow-api.spec.ts | 150 +++++++++++++++++++++++++++++
3 files changed, 365 insertions(+)
diff --git a/agent-service/src/api/auth-api.spec.ts b/agent-service/src/api/auth-api.spec.ts
new file mode 100644
index 0000000000..0037790ffc
--- /dev/null
+++ b/agent-service/src/api/auth-api.spec.ts
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { describe, expect, test } from "bun:test";
+import { createAuthHeaders, extractBearerToken, extractUserFromToken, validateToken } from "./auth-api";
+
+/**
+ * Builds a well-formed three-segment token carrying the given payload. All three segments are
+ * required — `decodeJWT` rejects anything that is not exactly three — but only the payload one is
+ * decoded, so the header and signature just have to be present.
+ */
+function tokenWith(payload: Record<string, unknown>): string {
+ const encoded = Buffer.from(JSON.stringify(payload), "utf-8").toString("base64");
+ return `header.${encoded}.signature`;
+}
+
+/** `exp` is a UNIX second count, so expiries are built in seconds throughout. */
+function nowInSeconds(): number {
+ return Math.floor(Date.now() / 1000);
+}
+
+describe("extractUserFromToken", () => {
+ test("maps the JWT payload onto the user record", () => {
+ const token = tokenWith({ userId: 42, sub: "ada", email: "ada@example.com", role: "ADMIN" });
+
+ expect(extractUserFromToken(token)).toEqual({
+ uid: 42,
+ name: "ada",
+ email: "ada@example.com",
+ role: "ADMIN",
+ });
+ });
+
+ test("defaults a missing email to empty and a missing role to REGULAR", () => {
+ // Role defaulting is a privilege decision: absent must mean least privilege, never admin.
+ const user = extractUserFromToken(tokenWith({ userId: 7, sub: "grace" }));
+
+ expect(user.email).toBe("");
+ expect(user.role).toBe("REGULAR");
+ });
+
+ test("rejects a token missing its signature segment even when the payload decodes", () => {
+ // The payload here is perfectly good JSON, so only the segment-count check stands between an
+ // unsigned token and a populated user record.
+ const payload = Buffer.from(JSON.stringify({ userId: 1, sub: "ada" }), "utf-8").toString("base64");
+
+ expect(() => extractUserFromToken(`header.${payload}`)).toThrow(/Failed to decode JWT/);
+ });
+
+ test("rejects a token with too many segments", () => {
+ const payload = Buffer.from(JSON.stringify({ userId: 1, sub: "ada" }), "utf-8").toString("base64");
+
+ expect(() => extractUserFromToken(`header.${payload}.signature.extra`)).toThrow(/Failed to decode JWT/);
+ });
+
+ test("rejects a token whose payload is not JSON", () => {
+ const notJson = Buffer.from("<html>", "utf-8").toString("base64");
+ expect(() => extractUserFromToken(`header.${notJson}.signature`)).toThrow(/Failed to decode JWT/);
+ });
+});
+
+describe("validateToken", () => {
+ test("accepts a token whose expiry is still in the future", () => {
+ expect(validateToken(tokenWith({ userId: 1, exp: nowInSeconds() + 60 }))).toBe(true);
+ });
+
+ test("rejects a token whose expiry has passed", () => {
+ expect(validateToken(tokenWith({ userId: 1, exp: nowInSeconds() - 60 }))).toBe(false);
+ });
+
+ test("treats a token with no expiry as valid forever", () => {
+ // Deliberate: tokens minted without `exp` never expire. Pinned because it is a policy choice
+ // that reads like an oversight, and flipping it would lock out every non-expiring token.
+ expect(validateToken(tokenWith({ userId: 1 }))).toBe(true);
+ });
+
+ test("rejects a malformed token rather than letting the decode failure escape", () => {
+ // isTokenExpired swallows the decode error and reports "expired", so callers get false here
+ // instead of an exception. A caller that only catches would otherwise let a junk token through.
+ expect(validateToken("not-a-token")).toBe(false);
+ });
+
+ test("expiry is compared in seconds, not milliseconds", () => {
+ // exp is a UNIX second count; reading it as milliseconds would place every real token in 1970
+ // and reject it. A far-future second count must still validate.
+ expect(validateToken(tokenWith({ exp: nowInSeconds() + 3600 }))).toBe(true);
+ });
+});
+
+describe("extractBearerToken", () => {
+ test("returns the token from a Bearer header", () => {
+ expect(extractBearerToken("Bearer abc123")).toBe("abc123");
+ });
+
+ test("accepts the scheme in any case", () => {
+ expect(extractBearerToken("bearer abc123")).toBe("abc123");
+ expect(extractBearerToken("BEARER abc123")).toBe("abc123");
+ });
+
+ test("ignores a non-Bearer scheme", () => {
+ expect(extractBearerToken("Basic abc123")).toBeUndefined();
+ });
+
+ test("returns undefined for a missing header", () => {
+ expect(extractBearerToken(undefined)).toBeUndefined();
+ });
+
+ test("returns undefined when the scheme carries no token", () => {
+ expect(extractBearerToken("Bearer")).toBeUndefined();
+ expect(extractBearerToken("Bearer ")).toBeUndefined();
+ });
+});
+
+describe("createAuthHeaders", () => {
+ test("sends the token as a Bearer credential alongside a JSON content type", () => {
+ expect(createAuthHeaders("abc123")).toEqual({
+ Authorization: "Bearer abc123",
+ "Content-Type": "application/json",
+ });
+ });
+});
diff --git a/agent-service/src/api/backend-api.spec.ts b/agent-service/src/api/backend-api.spec.ts
new file mode 100644
index 0000000000..1ce1cf0557
--- /dev/null
+++ b/agent-service/src/api/backend-api.spec.ts
@@ -0,0 +1,78 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { afterEach, describe, expect, mock, spyOn, test } from "bun:test";
+import { fetchOperatorMetadata, getBackendConfig, type OperatorMetadata } from "./backend-api";
+
+const metadata: OperatorMetadata = { operators: [], groups: [] };
+
+describe("getBackendConfig", () => {
+ test("exposes the four service endpoints", () => {
+ const config = getBackendConfig();
+
+ expect(Object.keys(config).sort()).toEqual([
+ "apiEndpoint",
+ "compileEndpoint",
+ "executionEndpoint",
+ "modelsEndpoint",
+ ]);
+ });
+
+ test("hands out a copy, so a caller cannot repoint the service endpoints", () => {
+ // The config is module-level state shared by every API client; returning the live object would
+ // let one caller's edit silently redirect everyone else's requests.
+ const first = getBackendConfig();
+ first.apiEndpoint = "http://evil.example.com";
+
+ expect(getBackendConfig().apiEndpoint).not.toBe("http://evil.example.com");
+ });
+});
+
+describe("fetchOperatorMetadata", () => {
+ afterEach(() => {
+ mock.restore();
+ });
+
+ test("reads the metadata from the dashboard service", async () => {
+ const fetchSpy = spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response(JSON.stringify(metadata), { status: 200 })
+ );
+
+ const result = await fetchOperatorMetadata();
+
+ expect(result).toEqual(metadata);
+ const [url] = fetchSpy.mock.calls[0] as [string];
+ expect(url).toBe(`${getBackendConfig().apiEndpoint}/api/resources/operator-metadata`);
+ });
+
+ test("throws with the status when the metadata cannot be fetched", async () => {
+ spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response("down", { status: 503, statusText: "Service Unavailable" })
+ );
+
+ await expect(fetchOperatorMetadata()).rejects.toThrow(/Failed to fetch operator metadata: 503 Service Unavailable/);
+ });
+
+ test("lets a network failure surface rather than returning empty metadata", async () => {
+ // A silent empty result here would leave the agent believing the cluster has zero operators.
+ spyOn(globalThis, "fetch").mockRejectedValue(new Error("network down"));
+
+ await expect(fetchOperatorMetadata()).rejects.toThrow(/network down/);
+ });
+});
diff --git a/agent-service/src/api/workflow-api.spec.ts b/agent-service/src/api/workflow-api.spec.ts
new file mode 100644
index 0000000000..6ddaea515b
--- /dev/null
+++ b/agent-service/src/api/workflow-api.spec.ts
@@ -0,0 +1,150 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { afterEach, describe, expect, mock, spyOn, test } from "bun:test";
+import { persistWorkflow, retrieveWorkflow, type Workflow } from "./workflow-api";
+import type { WorkflowContent } from "../types/workflow";
+
+const TOKEN = "tok-123";
+
+const content = {
+ operators: [{ operatorID: "opX", operatorType: "CSVFileScan" }],
+ links: [],
+} as unknown as WorkflowContent;
+
+/** The backend stores `content` as a JSON string, so responses echo it back in that form. */
+function storedWorkflow(overrides: Partial<Workflow> = {}): Record<string, unknown> {
+ return { wid: 5, name: "flow", content: JSON.stringify(content), ...overrides };
+}
+
+describe("persistWorkflow", () => {
+ afterEach(() => {
+ mock.restore();
+ });
+
+ test("POSTs the workflow with the caller's bearer token", async () => {
+ const fetchSpy = spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response(JSON.stringify(storedWorkflow()), { status: 200 })
+ );
+
+ await persistWorkflow(TOKEN, 5, "flow", content, "a description");
+
+ const [url, init] = fetchSpy.mock.calls[0] as [string, RequestInit];
+ expect(url).toMatch(/\/api\/workflow\/persist$/);
+ expect(init.method).toBe("POST");
+ expect(init.headers).toEqual({
+ Authorization: `Bearer ${TOKEN}`,
+ "Content-Type": "application/json",
+ });
+ });
+
+ test("serializes the content as a nested JSON string, not a nested object", async () => {
+ // The persist endpoint takes `content` as a string field. Sending the object directly is the
+ // obvious-looking mistake and the backend rejects it, so the double encoding is pinned here.
+ const fetchSpy = spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response(JSON.stringify(storedWorkflow()), { status: 200 })
+ );
+
+ await persistWorkflow(TOKEN, 5, "flow", content, "a description");
+
+ const body = JSON.parse((fetchSpy.mock.calls[0] as [string, RequestInit])[1].body as string);
+ expect(typeof body.content).toBe("string");
+ expect(JSON.parse(body.content)).toEqual(content);
+ expect(body).toMatchObject({ wid: 5, name: "flow", description: "a description", isPublic: false });
+ });
+
+ test("sends an empty description when none is given", async () => {
+ const fetchSpy = spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response(JSON.stringify(storedWorkflow()), { status: 200 })
+ );
+
+ await persistWorkflow(TOKEN, 5, "flow", content);
+
+ const body = JSON.parse((fetchSpy.mock.calls[0] as [string, RequestInit])[1].body as string);
+ expect(body.description).toBe("");
+ });
+
+ test("parses the stringified content on the way back out", async () => {
+ spyOn(globalThis, "fetch").mockResolvedValue(new Response(JSON.stringify(storedWorkflow()), { status: 200 }));
+
+ const saved = await persistWorkflow(TOKEN, 5, "flow", content);
+
+ expect(saved.content).toEqual(content);
+ });
+
+ test("leaves an already-parsed content object alone", async () => {
+ spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response(JSON.stringify({ wid: 5, name: "flow", content }), { status: 200 })
+ );
+
+ const saved = await persistWorkflow(TOKEN, 5, "flow", content);
+
+ expect(saved.content).toEqual(content);
+ });
+
+ test("reports the status and the server's message when the save is refused", async () => {
+ spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response("workflow is read-only", { status: 403, statusText: "Forbidden" })
+ );
+
+ await expect(persistWorkflow(TOKEN, 5, "flow", content)).rejects.toThrow(
+ /Failed to persist workflow: 403 Forbidden - workflow is read-only/
+ );
+ });
+});
+
+describe("retrieveWorkflow", () => {
+ afterEach(() => {
+ mock.restore();
+ });
+
+ test("GETs the workflow by id with the caller's bearer token", async () => {
+ const fetchSpy = spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response(JSON.stringify(storedWorkflow()), { status: 200 })
+ );
+
+ await retrieveWorkflow(TOKEN, 5);
+
+ const [url, init] = fetchSpy.mock.calls[0] as [string, RequestInit];
+ expect(url).toMatch(/\/api\/workflow\/5$/);
+ expect(init.method).toBe("GET");
+ expect(init.headers).toEqual({
+ Authorization: `Bearer ${TOKEN}`,
+ "Content-Type": "application/json",
+ });
+ });
+
+ test("parses the stringified content on the way back out", async () => {
+ spyOn(globalThis, "fetch").mockResolvedValue(new Response(JSON.stringify(storedWorkflow()), { status: 200 }));
+
+ const loaded = await retrieveWorkflow(TOKEN, 5);
+
+ expect(loaded.content).toEqual(content);
+ });
+
+ test("reports the status and the server's message when the workflow is missing", async () => {
+ spyOn(globalThis, "fetch").mockResolvedValue(
+ new Response("no such workflow", { status: 404, statusText: "Not Found" })
+ );
+
+ await expect(retrieveWorkflow(TOKEN, 5)).rejects.toThrow(
+ /Failed to retrieve workflow: 404 Not Found - no such workflow/
+ );
+ });
+});