From 65003bb08c4817188eaad2b56ae5a1c7ceb0e2c6 Mon Sep 17 00:00:00 2001 From: robbond Date: Wed, 25 Mar 2026 11:31:39 +0000 Subject: [PATCH] refactor(document): extract file route builder and normalize query composition --- actions/clients/fileRouteBuilder.js | 33 +++ actions/clients/index.js | 1 + actions/services/documentDirectService.js | 226 ++++++++++-------- memory-bank/change-log.md | 88 +++++++ tests/phase22/client-utils-behaviour.test.cjs | 63 +++++ tests/serviceHarness.cjs | 31 +++ 6 files changed, 336 insertions(+), 106 deletions(-) create mode 100644 actions/clients/fileRouteBuilder.js diff --git a/actions/clients/fileRouteBuilder.js b/actions/clients/fileRouteBuilder.js new file mode 100644 index 00000000..944c366f --- /dev/null +++ b/actions/clients/fileRouteBuilder.js @@ -0,0 +1,33 @@ +const encodeRouteParam = (value) => encodeURIComponent(String(value)); + +export const buildFileQuery = (path, params = {}, options = {}) => { + const { encode = false } = options; + + const entries = Object.entries(params).filter(([, value]) => { + return value !== undefined && value !== null; + }); + + if (entries.length === 0) { + return path; + } + + const query = entries + .map(([key, value]) => { + if (!encode) { + return `${key}=${String(value)}`; + } + + return `${encodeRouteParam(key)}=${encodeRouteParam(value)}`; + }) + .join("&"); + + return `${path}?${query}`; +}; + +export const withBaseUrl = (baseUrl, route) => { + return `${baseUrl}${route}`; +}; + +export const appendQuerySuffix = (route, suffix = "") => { + return `${route}${suffix}`; +}; diff --git a/actions/clients/index.js b/actions/clients/index.js index e2c2f03c..3c9b8ee9 100644 --- a/actions/clients/index.js +++ b/actions/clients/index.js @@ -1,3 +1,4 @@ export * from "./relayClient"; export * from "./endpointClient"; export * from "./fileClient"; +export * from "./fileRouteBuilder"; diff --git a/actions/services/documentDirectService.js b/actions/services/documentDirectService.js index e86819eb..b3d0d3c6 100644 --- a/actions/services/documentDirectService.js +++ b/actions/services/documentDirectService.js @@ -1,8 +1,11 @@ import { BASE_URL } from "../core/env"; import { consoleLogger } from "../core/logger"; import { hashAPIPath } from "../core/hash"; -import { buildHashedQueryUrl } from "../clients/relayClient"; -import { getJson, requestJson } from "../clients/endpointClient"; +import { + buildFileQuery, + withBaseUrl, + appendQuerySuffix +} from "../clients/fileRouteBuilder"; import { getFileJson, getSignedFileJson, @@ -11,55 +14,59 @@ import { } from "../clients/fileClient"; export const getAwaitingSubmissionFromBlob = (containerName) => { - return getJson( - BASE_URL + - "/api/file/getawaitingsubmissionfromblob?container=" + - containerName + - hashAPIPath( - "/api/file/getawaitingsubmissionfromblob?container=" + - containerName - ) + const route = buildFileQuery("/api/file/getawaitingsubmissionfromblob", { + container: containerName + }); + + return getFileJson( + withBaseUrl(BASE_URL, appendQuerySuffix(route, hashAPIPath(route))) ).catch((error) => { consoleLogger(error); }); }; export const getRepsFromBlob = (containerName) => { - return getJson( - BASE_URL + - "/api/file/getrepsblob?container=" + - containerName + - hashAPIPath("/api/file/getrepsblob?container=" + containerName) + const route = buildFileQuery("/api/file/getrepsblob", { + container: containerName + }); + + return getFileJson( + withBaseUrl(BASE_URL, appendQuerySuffix(route, hashAPIPath(route))) ).catch((error) => { consoleLogger(error); }); }; export const getRepsFromBlobProxy = async (containerName) => { - return getJson( - "/api/file/getrepsblobproxy?container=" + containerName - ).catch((error) => { + const route = buildFileQuery("/api/file/getrepsblobproxy", { + container: containerName + }); + + return getFileJson(route).catch((error) => { consoleLogger(error); }); }; export const getAwaitingSubmissionFromBlobProxy = async (containerName) => { - return getJson( - BASE_URL + - "/api/file/getawaitingsubmissionfromblobproxy?container=" + - containerName - ).catch((error) => { + const route = buildFileQuery( + "/api/file/getawaitingsubmissionfromblobproxy", + { + container: containerName + } + ); + + return getFileJson(withBaseUrl(BASE_URL, route)).catch((error) => { consoleLogger(error); }); }; export const getFilesFromBlobproxy = (containerName, casefolderID) => { - return getFileJson( - "/api/file/getbloblistproxy?container=" + - containerName + - "&casefolderID=" + - casefolderID - ).catch((error) => { + const route = buildFileQuery("/api/file/getbloblistproxy", { + container: containerName, + casefolderID + }); + + return getFileJson(route).catch((error) => { consoleLogger(error); }); }; @@ -68,11 +75,10 @@ export const deleteAwaitingSubmissionsFromBlob = ( containerID, casefolderID ) => { - var queryUrl = - "/api/file/deleteblobcase?container=" + - containerID + - "&casefolderID=" + - casefolderID; + var queryUrl = buildFileQuery("/api/file/deleteblobcase", { + container: containerID, + casefolderID + }); return getSignedFileJson(queryUrl).catch((error) => { consoleLogger(error); @@ -84,13 +90,11 @@ export const deleteMyRepresentationsFromBlob = ( casefolderID, repfile ) => { - var queryUrl = - "/api/file/deleteblobrep?container=" + - containerID + - "&casefolderID=" + - casefolderID + - "&repfile=" + - repfile; + var queryUrl = buildFileQuery("/api/file/deleteblobrep", { + container: containerID, + casefolderID, + repfile + }); return getSignedFileJson(queryUrl).catch((error) => { consoleLogger(error); @@ -217,18 +221,13 @@ export const generateAppealPDF = async ( }; export const getFilesFromBlob = (containerName, casefolderID) => { - return getJson( - BASE_URL + - "/api/file/getbloblist?container=" + - containerName + - "&casefolderID=" + - casefolderID + - hashAPIPath( - "/api/file/getbloblist?container=" + - containerName + - "&casefolderID=" + - casefolderID - ) + const route = buildFileQuery("/api/file/getbloblist", { + container: containerName, + casefolderID + }); + + return getFileJson( + withBaseUrl(BASE_URL, appendQuerySuffix(route, hashAPIPath(route))) ).catch((error) => { consoleLogger(error); }); @@ -239,15 +238,16 @@ export const getFilesFromBlobHashed = ( getblobshash, casefolderID ) => { - return getJson( - "/api/file/getbloblist?container=" + - containerName + - "&casefolderID=" + - casefolderID + - getblobshash - ).catch((error) => { - consoleLogger(error); + const route = buildFileQuery("/api/file/getbloblist", { + container: containerName, + casefolderID }); + + return getFileJson(appendQuerySuffix(route, getblobshash)).catch( + (error) => { + consoleLogger(error); + } + ); }; export const deleteBlob = async ( @@ -256,17 +256,23 @@ export const deleteBlob = async ( deleteblobhash, casefolderID ) => { - return getFileJson( - "/api/file/deleteblob?container=" + - containerName + - "&casefolderID=" + - encodeURIComponent(casefolderID) + - "&blobname=" + - encodeURIComponent(blobName) + - deleteblobhash - ).catch((error) => { - consoleLogger(error); - }); + const route = buildFileQuery( + "/api/file/deleteblob", + { + container: containerName, + casefolderID, + blobname: blobName + }, + { + encode: true + } + ); + + return getFileJson(appendQuerySuffix(route, deleteblobhash)).catch( + (error) => { + consoleLogger(error); + } + ); }; export const deleteRepBlob = async ( @@ -276,26 +282,33 @@ export const deleteRepBlob = async ( casefolderID, filenamePrefix ) => { - return getFileJson( - "/api/file/deleteblob?container=" + - containerName + - "&casefolderID=" + - encodeURIComponent(casefolderID + "/" + filenamePrefix) + - "&blobname=" + - encodeURIComponent(blobName) + - deleteblobhash - ).catch((error) => { - consoleLogger(error); - }); + const route = buildFileQuery( + "/api/file/deleteblob", + { + container: containerName, + casefolderID: casefolderID + "/" + filenamePrefix, + blobname: blobName + }, + { + encode: true + } + ); + + return getFileJson(appendQuerySuffix(route, deleteblobhash)).catch( + (error) => { + consoleLogger(error); + } + ); }; export const downloadBlob = (containerName, blobName) => { - const queryUrl = - BASE_URL + - "/api/file/downloadblob?container=" + - containerName.toLowerCase() + - "&blobname=" + - blobName; + const queryUrl = withBaseUrl( + BASE_URL, + buildFileQuery("/api/file/downloadblob", { + container: containerName.toLowerCase(), + blobname: blobName + }) + ); return downloadFileBlob(queryUrl).catch((error) => { consoleLogger(error); @@ -303,30 +316,31 @@ export const downloadBlob = (containerName, blobName) => { }; export const getProgressFromBlob = async (containerName, casereference) => { - return getJson( - BASE_URL + - "/api/file/getprogressobjblob?container=" + - encodeURIComponent(containerName) + - "&casefolderID=" + - encodeURIComponent(casereference) + - hashAPIPath( - "/api/file/getprogressobjblob?container=" + - encodeURIComponent(containerName) + - "&casefolderID=" + - encodeURIComponent(casereference) - ) + const route = buildFileQuery( + "/api/file/getprogressobjblob", + { + container: containerName, + casefolderID: casereference + }, + { + encode: true + } + ); + + return getFileJson( + withBaseUrl(BASE_URL, appendQuerySuffix(route, hashAPIPath(route))) ).catch((error) => { consoleLogger(error); }); }; export const createContainerProxy = (containerName) => { - var queryUrl = - "/api/file/setupcontainer?ident=" + - containerName + - hashAPIPath("/api/file/setupcontainer?ident=" + containerName); + var route = buildFileQuery("/api/file/setupcontainer", { + ident: containerName + }); + var queryUrl = appendQuerySuffix(route, hashAPIPath(route)); - return getJson(BASE_URL + queryUrl).catch((error) => { + return getFileJson(withBaseUrl(BASE_URL, queryUrl)).catch((error) => { consoleLogger(error); return JSON.stringify(error); diff --git a/memory-bank/change-log.md b/memory-bank/change-log.md index dff8f864..b4ec2ab6 100644 --- a/memory-bank/change-log.md +++ b/memory-bank/change-log.md @@ -2429,3 +2429,91 @@ Validation: Follow-ups: - Optional next widened slice: evaluate consolidating remaining direct `getJson` file-read flows in `documentDirectService` behind `fileClient` for full per-module client symmetry. + +--- + +### CL-069: TASK22260 widened module-completion slice — documentDirectService file-read client symmetry + +date: 2026-03-25 +author: Cline +scope: `actions/services/documentDirectService.js` +type: change +rationale: Continue widened slice cadence by completing per-module client symmetry in `documentDirectService`, moving all file-read helper calls to `fileClient` instead of mixed endpoint client usage. +impact: Simplifies module dependency shape and centralizes file-route read behavior through a single client abstraction without changing runtime contracts. +status: completed + +Summary: + +- Removed mixed `endpointClient` usage from `documentDirectService` for file reads. +- Migrated remaining file-read/helper routes from `getJson` to `getFileJson`: + - `getAwaitingSubmissionFromBlob` + - `getRepsFromBlob` + - `getRepsFromBlobProxy` + - `getAwaitingSubmissionFromBlobProxy` + - `getFilesFromBlob` + - `getFilesFromBlobHashed` + - `getProgressFromBlob` + - `createContainerProxy` +- Removed now-unused imports from `documentDirectService`: + - `buildHashedQueryUrl` + - `getJson` + - `requestJson` + +Validation: + +- `node tests/phase22/index.test.cjs` -> pass + - core-token: 2/2 + - client-utils: 5/5 + - file-client: 4/4 + - phase22 combined: pass +- `node tests/phase7/service-behaviour.test.cjs` -> pass (13/13) +- `npm run lint` -> warnings only (pre-existing `react-hooks/exhaustive-deps`; no new lint errors) + +Follow-ups: + +- Optional next widened slice: introduce a small `fileClient` URL-builder helper set for repeated query-string composition in `documentDirectService` (container/casefolder/blob parameters) to reduce string-concat drift risk. + +--- + +### CL-070: TASK22260 widened cross-file slice — file route builder extraction + document service query normalization + +date: 2026-03-25 +author: Cline +scope: `actions/clients/{fileRouteBuilder,index}.js`, `actions/services/documentDirectService.js`, `tests/{serviceHarness,phase22/client-utils-behaviour}.cjs` +type: change +rationale: Deliver a wider-than-previous slice by extracting reusable file-route query composition helpers and applying them across document service paths, reducing repeated string concatenation and encoding drift risk. +impact: Improves maintainability and consistency of file-route URL construction while preserving existing runtime contracts and hash behavior. +status: completed + +Summary: + +- Added new shared helper module: + - `actions/clients/fileRouteBuilder.js` + - exports: + - `buildFileQuery(path, params, options)` (supports optional encoded query composition) + - `withBaseUrl(baseUrl, route)` + - `appendQuerySuffix(route, suffix)` +- Exported route-builder helpers via `actions/clients/index.js`. +- Refactored `actions/services/documentDirectService.js` to use route-builder helpers across read/delete/download/query flows: + - normalized composition for file routes and hash suffix append behavior + - preserved encoded-path behavior for sensitive params (`casefolderID`, `blobname`) where previously encoded + - preserved base URL prefix behavior and existing logger/catch semantics +- Updated test harness defaults in `tests/serviceHarness.cjs` for new helper symbols: + - `buildFileQuery` + - `withBaseUrl` + - `appendQuerySuffix` +- Expanded phase22 utility coverage in `tests/phase22/client-utils-behaviour.test.cjs` with file route-builder behavior assertions. + +Validation: + +- `node tests/phase22/index.test.cjs` -> pass + - core-token: 2/2 + - client-utils: 6/6 + - file-client: 4/4 + - phase22 combined: pass +- `node tests/phase7/service-behaviour.test.cjs` -> pass (13/13) +- `npm run lint` -> warnings only (pre-existing `react-hooks/exhaustive-deps`; no new lint errors) + +Follow-ups: + +- Optional next widened slice: evaluate applying `fileRouteBuilder` to portal/case service file-route call sites for cross-module query-builder consistency. diff --git a/tests/phase22/client-utils-behaviour.test.cjs b/tests/phase22/client-utils-behaviour.test.cjs index b20a82df..76bad6fe 100644 --- a/tests/phase22/client-utils-behaviour.test.cjs +++ b/tests/phase22/client-utils-behaviour.test.cjs @@ -58,6 +58,32 @@ const loadRelayClientModule = (injected = {}) => { return context.module.exports; }; +const loadFileRouteBuilderModule = (injected = {}) => { + const filePath = path.join( + rootDir, + "actions", + "clients", + "fileRouteBuilder.js" + ); + let source = fs.readFileSync(filePath, "utf8"); + + source = source.replace(/import[\s\S]*?from\s+"[^"]+";\n?/g, ""); + source = source.replace(/export const\s+/g, "const "); + source += + "\nmodule.exports = { buildFileQuery, withBaseUrl, appendQuerySuffix };\n"; + + const context = { + module: { exports: {} }, + exports: {}, + require, + encodeURIComponent, + ...injected + }; + + vm.runInNewContext(source, context, { filename: filePath }); + return context.module.exports; +}; + const tests = []; const test = (name, fn) => tests.push({ name, fn }); @@ -150,6 +176,43 @@ test("clients/relayClient rethrows when browser hash call fails and no HASHKEY", ); }); +test("clients/fileRouteBuilder builds query with optional encoding and suffix helpers", async () => { + const mod = loadFileRouteBuilderModule(); + + const unencoded = mod.buildFileQuery("/api/file/getbloblist", { + container: "abc", + casefolderID: "x/y" + }); + const encoded = mod.buildFileQuery( + "/api/file/deleteblob", + { + container: "abc", + casefolderID: "x/y", + blobname: "doc one.pdf" + }, + { + encode: true + } + ); + + assert.strictEqual( + unencoded, + "/api/file/getbloblist?container=abc&casefolderID=x/y" + ); + assert.strictEqual( + encoded, + "/api/file/deleteblob?container=abc&casefolderID=x%2Fy&blobname=doc%20one.pdf" + ); + assert.strictEqual( + mod.withBaseUrl("http://example.local", unencoded), + "http://example.local/api/file/getbloblist?container=abc&casefolderID=x/y" + ); + assert.strictEqual( + mod.appendQuerySuffix(unencoded, "&hash=123"), + "/api/file/getbloblist?container=abc&casefolderID=x/y&hash=123" + ); +}); + const run = async () => { let passed = 0; diff --git a/tests/serviceHarness.cjs b/tests/serviceHarness.cjs index cdb7b43d..4cf494ff 100644 --- a/tests/serviceHarness.cjs +++ b/tests/serviceHarness.cjs @@ -126,6 +126,33 @@ const loadServiceModule = (fileName, injected = {}) => { }); }; + const defaultBuildFileQuery = (pathValue, params = {}, options = {}) => { + const { encode = false } = options; + const entries = Object.entries(params).filter(([, value]) => { + return value !== undefined && value !== null; + }); + + if (entries.length === 0) { + return pathValue; + } + + const query = entries + .map(([key, value]) => { + if (!encode) { + return `${key}=${String(value)}`; + } + + return `${encodeURIComponent(key)}=${encodeURIComponent(String(value))}`; + }) + .join("&"); + + return `${pathValue}?${query}`; + }; + + const defaultWithBaseUrl = (baseUrl, route) => `${baseUrl}${route}`; + const defaultAppendQuerySuffix = (route, suffix = "") => + `${route}${suffix}`; + const context = { module: { exports: {} }, exports: {}, @@ -147,6 +174,10 @@ const loadServiceModule = (fileName, injected = {}) => { downloadFileBlob: injected.downloadFileBlob || defaultDownloadFileBlob, buildHashedQueryUrl: injected.buildHashedQueryUrl || defaultBuildHashedQueryUrl, + buildFileQuery: injected.buildFileQuery || defaultBuildFileQuery, + withBaseUrl: injected.withBaseUrl || defaultWithBaseUrl, + appendQuerySuffix: + injected.appendQuerySuffix || defaultAppendQuerySuffix, ...injected };