Skip to content

Commit 1b6833c

Browse files
committed
fix(concept-mapping): use the dataset's persisted cache id
Concept mapping opened trex connections against a cacheId derived at runtime from getDatabaseCredentials(): it looked up the credential whose code matched databaseCode, then recomputed the alias from the dataset UUID with a HANA special case. That derivation is not equivalent to the persisted portal.dataset.cache_id and diverges for several populations: - legacy rows the migration backfilled with database_code - snapshots, which inherit the SOURCE dataset's cacheId, not their own - operator-supplied custom cache ids It also cannot tell apart two datasets that share a databaseCode but use different schemas, so mappings could be read from or written to the wrong cache catalog. Read portal.dataset.cache_id instead and treat it as the source of truth. The route now accepts an optional datasetId; when supplied, the dataset record is fetched with the caller's token and resolution falls back to the record's databaseCode only when cacheId is genuinely absent. When a datasetId is given but cannot be resolved we surface 502 rather than guessing an alias, since a wrong alias silently targets the wrong database. Callers that omit datasetId keep the existing databaseCode path, so the endpoint stays backward compatible. The dataset lookup is injected into ConceptMappingRouter so the router remains importable without the env-dependent PortalAPI module. Adds regression coverage for the divergent populations (legacy/migrated, custom, cloned, and two datasets sharing a databaseCode), for concept mapping save/load over HTTP, and for the API error contract, including that a failed lookup opens no connection and writes nothing. Adds the test task the plugin was missing.
1 parent 48e956c commit 1b6833c

13 files changed

Lines changed: 631 additions & 43 deletions

File tree

plugins/functions/concept-mapping/deno.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
"workspace": [],
33
"nodeModulesDir": "auto",
44
"unstable": ["sloppy-imports"],
5+
"tasks": {
6+
"test": "deno test --allow-read --allow-env --allow-net --no-check"
7+
},
58
"compilerOptions": {
69
"lib": [
710
"deno.window"
@@ -19,6 +22,8 @@
1922
"pg-promise": "npm:pg-promise@^10.10.1",
2023
"axios": "npm:axios@^1.7.7",
2124
"@sap/textbundle": "npm:@sap/textbundle@4.2.0",
25+
"@std/assert": "jsr:@std/assert@1",
26+
"@std/testing": "jsr:@std/testing@1",
2227
"./src/env": "./src/env.ts",
2328
"./src/types": "./src/types.ts",
2429
"./src/constants": "./src/constants.ts",

plugins/functions/concept-mapping/deno.lock

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

plugins/functions/concept-mapping/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import express, { Application } from "express";
22
import { ConceptMappingRouter } from "./src/concept-mapping/routes";
3+
import { PortalAPI } from "./src/api/PortalAPI";
34

45
export class App {
56
private app: Application;
@@ -11,7 +12,12 @@ export class App {
1112

1213
async start() {
1314
this.app.use(express.json());
14-
this.app.use("/concept-mapping", new ConceptMappingRouter().router);
15+
this.app.use(
16+
"/concept-mapping",
17+
new ConceptMappingRouter(
18+
(token) => (datasetId) => new PortalAPI(token).getDataset(datasetId),
19+
).router,
20+
);
1521
this.app.listen(8000);
1622
this.logger.info(`Concept Mapping service started successfully!`);
1723
}

plugins/functions/concept-mapping/src/api/PortalAPI.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export class PortalAPI {
1717
}
1818
if (env.SERVICE_ROUTES.portalServer) {
1919
this.baseURL = env.SERVICE_ROUTES.portalServer;
20+
// @ts-ignore Cannot find name 'Trex'
2021
this.channel = Trex.tokioChannel("d2e-functions/portal");
2122
// this.httpsAgent = new https.Agent({
2223
// rejectUnauthorized: true,

plugins/functions/concept-mapping/src/concept-mapping/middleware.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ export const GetConceptMappingDto = () => [
99
.isString()
1010
.notEmpty()
1111
.withMessage("schemaName is required"),
12+
// Optional: when supplied, the dataset's persisted cacheId is authoritative.
13+
// Omitted by pre-dataset / infra callers, which keep the databaseCode path.
14+
query("datasetId")
15+
.optional()
16+
.isUUID()
17+
.withMessage("datasetId must be a UUID"),
1218
];
1319

1420
export const ConceptMappingDto = () => [
Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
import { describe, it } from "@std/testing/bdd";
2+
import { assertEquals, assertStringIncludes } from "@std/assert";
3+
import pako from "pako";
4+
import { encodeBase64 } from "base64";
5+
6+
/** Mirrors the encoding the UI performs before POSTing (pako.deflate + base64). */
7+
function encodePayload(mappings: unknown[]): string {
8+
return encodeBase64(pako.deflate(JSON.stringify(mappings)));
9+
}
10+
11+
// Records the cacheId trex is asked to open a connection against.
12+
let lastGetConnectionArgs: unknown[] = [];
13+
// Records the statement trex was asked to run, so save tests can assert the
14+
// INSERT actually targets the resolved cache/schema.
15+
let lastExecute: { sql: string; params: unknown[] } | null = null;
16+
let executeImpl: (sql: string, params: unknown[]) => unknown[] = () => [];
17+
18+
// deno-lint-ignore no-explicit-any
19+
(globalThis as any).Trex = {
20+
databaseManager: () => ({
21+
getConnection: (...args: unknown[]) => {
22+
lastGetConnectionArgs = args;
23+
return {
24+
execute: (
25+
sql: string,
26+
params: unknown[],
27+
cb: (err: unknown, res: unknown) => void,
28+
) => {
29+
lastExecute = { sql, params: params.map((p) => (p as { value: unknown }).value) };
30+
try {
31+
cb(null, executeImpl(sql, params));
32+
} catch (e) {
33+
cb(e, null);
34+
}
35+
},
36+
close: () => {},
37+
};
38+
},
39+
}),
40+
};
41+
42+
const express = (await import("express")).default;
43+
const { ConceptMappingRouter } = await import("./routes.ts");
44+
45+
const DS = "3f2a1c44-7777-4aaa-9bbb-000000000007";
46+
const EMPTY_PAYLOAD = "eJyLjgUAARUAuQ=="; // zlib+base64 of []
47+
48+
// deno-lint-ignore no-explicit-any
49+
async function withServer(fetcherFactory: any, fn: (base: string) => Promise<void>) {
50+
const app = express();
51+
app.use(express.json());
52+
app.use("/concept-mapping", new ConceptMappingRouter(fetcherFactory).router);
53+
54+
const server = app.listen(0);
55+
await new Promise((resolve) => server.once("listening", resolve));
56+
const { port } = server.address() as { port: number };
57+
try {
58+
await fn(`http://127.0.0.1:${port}/concept-mapping`);
59+
} finally {
60+
await new Promise((resolve) => server.close(resolve));
61+
}
62+
}
63+
64+
describe("GET /concept-mapping validation", () => {
65+
it("returns 400 when schemaName is missing", async () => {
66+
await withServer(undefined, async (base) => {
67+
const res = await fetch(`${base}/?databaseCode=PG_MAIN`);
68+
assertEquals(res.status, 400);
69+
await res.body?.cancel();
70+
});
71+
});
72+
73+
it("returns 400 when datasetId is not a UUID", async () => {
74+
await withServer(undefined, async (base) => {
75+
const res = await fetch(
76+
`${base}/?databaseCode=PG_MAIN&schemaName=cdm&datasetId=not-a-uuid`,
77+
);
78+
assertEquals(res.status, 400);
79+
await res.body?.cancel();
80+
});
81+
});
82+
});
83+
84+
describe("cacheId resolution over HTTP", () => {
85+
it("opens the connection with the dataset's persisted cacheId", async () => {
86+
lastGetConnectionArgs = [];
87+
executeImpl = () => [{ source_code: "X1" }];
88+
89+
const factory = () => (_id: string) =>
90+
Promise.resolve({ cacheId: "_stored_cache", databaseCode: "PG_SHARE" });
91+
92+
await withServer(factory, async (base) => {
93+
const res = await fetch(
94+
`${base}/?databaseCode=PG_SHARE&schemaName=cdm&datasetId=${DS}`,
95+
);
96+
assertEquals(res.status, 200);
97+
assertEquals(await res.json(), [{ source_code: "X1" }]);
98+
});
99+
100+
// Authoritative cacheId, NOT the request's databaseCode.
101+
assertEquals(lastGetConnectionArgs[0], "_stored_cache");
102+
});
103+
104+
it("loads a legacy dataset via the record's databaseCode when cacheId is absent", async () => {
105+
lastGetConnectionArgs = [];
106+
executeImpl = () => [{ source_code: "LEGACY" }];
107+
108+
// Migrated row the cache_id backfill left unset -> guarded fallback.
109+
const factory = () => (_id: string) =>
110+
Promise.resolve({ cacheId: null, databaseCode: "PG_OLDER" });
111+
112+
await withServer(factory, async (base) => {
113+
const res = await fetch(
114+
`${base}/?databaseCode=REQ_CODE&schemaName=cdm&datasetId=${DS}`,
115+
);
116+
assertEquals(res.status, 200);
117+
assertEquals(await res.json(), [{ source_code: "LEGACY" }]);
118+
});
119+
120+
// The record's databaseCode wins over the request's, which is never trusted
121+
// once a datasetId is in play.
122+
assertEquals(lastGetConnectionArgs[0], "PG_OLDER");
123+
});
124+
125+
it("uses the databaseCode when no datasetId is supplied (infra path)", async () => {
126+
lastGetConnectionArgs = [];
127+
executeImpl = () => [];
128+
129+
await withServer(undefined, async (base) => {
130+
const res = await fetch(`${base}/?databaseCode=PG_MAIN&schemaName=cdm`);
131+
assertEquals(res.status, 200);
132+
assertEquals(await res.json(), []);
133+
});
134+
135+
assertEquals(lastGetConnectionArgs[0], "PG_MAIN");
136+
});
137+
138+
it("returns 502 when the dataset lookup fails, never a guessed cache", async () => {
139+
const factory = () => (_id: string) => Promise.reject(new Error("portal down"));
140+
141+
await withServer(factory, async (base) => {
142+
const res = await fetch(
143+
`${base}/?databaseCode=PG_SHARE&schemaName=cdm&datasetId=${DS}`,
144+
);
145+
assertEquals(res.status, 502);
146+
assertStringIncludes(await res.text(), "cache id");
147+
});
148+
});
149+
});
150+
151+
describe("POST /concept-mapping save path", () => {
152+
it("writes the mappings to the dataset's persisted cacheId", async () => {
153+
lastGetConnectionArgs = [];
154+
lastExecute = null;
155+
// rowCount is derived from the driver result length.
156+
executeImpl = () => [{}, {}];
157+
158+
const factory = () => (_id: string) =>
159+
Promise.resolve({ cacheId: "_stored_cache", databaseCode: "PG_SHARE" });
160+
161+
const payload = encodePayload([
162+
{ source_code: "A1", target_concept_id: 111 },
163+
{ source_code: "B2", target_concept_id: 222 },
164+
]);
165+
166+
await withServer(factory, async (base) => {
167+
const res = await fetch(
168+
`${base}/?databaseCode=PG_SHARE&schemaName=cdm&datasetId=${DS}`,
169+
{
170+
method: "POST",
171+
headers: { "Content-Type": "application/json" },
172+
body: JSON.stringify({
173+
sourceVocabularyId: "MY_VOCAB",
174+
conceptMappings: payload,
175+
}),
176+
},
177+
);
178+
assertEquals(res.status, 200);
179+
assertStringIncludes(await res.text(), "Inserted 2 rows");
180+
});
181+
182+
// The write targets the authoritative cacheId, NOT the request databaseCode.
183+
assertEquals(lastGetConnectionArgs[0], "_stored_cache");
184+
assertStringIncludes(lastExecute!.sql, "INSERT INTO cdm.source_to_concept_map");
185+
// sourceVocabularyId is stamped onto every row.
186+
assertEquals(
187+
lastExecute!.params.filter((p) => p === "MY_VOCAB").length,
188+
2,
189+
);
190+
assertEquals(lastExecute!.params.includes("A1"), true);
191+
assertEquals(lastExecute!.params.includes("B2"), true);
192+
});
193+
194+
it("saves against the databaseCode when no datasetId is supplied (infra path)", async () => {
195+
lastGetConnectionArgs = [];
196+
executeImpl = () => [{}];
197+
198+
await withServer(undefined, async (base) => {
199+
const res = await fetch(`${base}/?databaseCode=PG_MAIN&schemaName=cdm`, {
200+
method: "POST",
201+
headers: { "Content-Type": "application/json" },
202+
body: JSON.stringify({
203+
sourceVocabularyId: "V",
204+
conceptMappings: encodePayload([{ source_code: "A1" }]),
205+
}),
206+
});
207+
assertEquals(res.status, 200);
208+
await res.body?.cancel();
209+
});
210+
211+
assertEquals(lastGetConnectionArgs[0], "PG_MAIN");
212+
});
213+
214+
it("returns 502 without writing when the dataset lookup fails", async () => {
215+
lastGetConnectionArgs = [];
216+
lastExecute = null;
217+
executeImpl = () => [{}];
218+
219+
const factory = () => (_id: string) => Promise.reject(new Error("portal down"));
220+
221+
await withServer(factory, async (base) => {
222+
const res = await fetch(
223+
`${base}/?databaseCode=PG_SHARE&schemaName=cdm&datasetId=${DS}`,
224+
{
225+
method: "POST",
226+
headers: { "Content-Type": "application/json" },
227+
body: JSON.stringify({
228+
sourceVocabularyId: "V",
229+
conceptMappings: encodePayload([{ source_code: "A1" }]),
230+
}),
231+
},
232+
);
233+
assertEquals(res.status, 502);
234+
assertStringIncludes(await res.text(), "cache id");
235+
});
236+
237+
// Critically: no connection opened and nothing written to a guessed cache.
238+
assertEquals(lastGetConnectionArgs.length, 0);
239+
assertEquals(lastExecute, null);
240+
});
241+
});
242+
243+
describe("POST /concept-mapping error contract", () => {
244+
it("returns 400 for an empty mappings payload even when portal is down", async () => {
245+
const factory = () => (_id: string) => Promise.reject(new Error("portal down"));
246+
247+
await withServer(factory, async (base) => {
248+
const res = await fetch(
249+
`${base}/?databaseCode=PG_SHARE&schemaName=cdm&datasetId=${DS}`,
250+
{
251+
method: "POST",
252+
headers: { "Content-Type": "application/json" },
253+
body: JSON.stringify({
254+
sourceVocabularyId: "vocab",
255+
conceptMappings: EMPTY_PAYLOAD,
256+
}),
257+
},
258+
);
259+
// Empty payload is rejected before cacheId resolution -> 400, not 502.
260+
assertEquals(res.status, 400);
261+
assertStringIncludes(await res.text(), "No concept mappings to save");
262+
});
263+
});
264+
265+
it("returns 400 when conceptMappings is an empty string", async () => {
266+
await withServer(undefined, async (base) => {
267+
const res = await fetch(`${base}/?databaseCode=PG_MAIN&schemaName=cdm`, {
268+
method: "POST",
269+
headers: { "Content-Type": "application/json" },
270+
body: JSON.stringify({ sourceVocabularyId: "v", conceptMappings: "" }),
271+
});
272+
assertEquals(res.status, 400);
273+
await res.body?.cancel();
274+
});
275+
});
276+
});

0 commit comments

Comments
 (0)