From d173daa14e873f2c2b86eb89da69cbf993027d15 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Fri, 3 Jan 2025 11:42:27 +0100 Subject: [PATCH 1/2] test(formatDownloadTitle): add basic test for some reason this will not run well with `tsx -r esm` though I've used the built-in node TS runner for now, which works: `node --experimental-transform-types` --- spec-es6/utils/formatDownloadTitle.spec.ts | 129 +++++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 spec-es6/utils/formatDownloadTitle.spec.ts diff --git a/spec-es6/utils/formatDownloadTitle.spec.ts b/spec-es6/utils/formatDownloadTitle.spec.ts new file mode 100644 index 000000000..923786915 --- /dev/null +++ b/spec-es6/utils/formatDownloadTitle.spec.ts @@ -0,0 +1,129 @@ +import { formatDownloadTitle } from "../../src/services/utils.ts"; +import { describe, it, execute, expect } from "../mini_test.ts"; + +const testCases: [fnValue: Parameters, expectedValue: ReturnType][] = [ + // empty fileName tests + [ + ["", "text", ""], + "untitled.html" + ], + + [ + ["", "canvas", ""], + "untitled.json" + ], + + [ + ["", null, ""], + "untitled" + ], + + // json extension from type tests + [ + ["test_file", "canvas", ""], + "test_file.json" + ], + + [ + ["test_file", "relationMap", ""], + "test_file.json" + ], + + [ + ["test_file", "search", ""], + "test_file.json" + ], + + // extension based on mime type + [ + ["test_file", null, "text/csv"], + "test_file.csv" + ], + + [ + ["test_file_wo_ext", "image", "image/svg+xml"], + "test_file_wo_ext.svg" + ], + + [ + ["test_file_wo_ext", "file", "application/json"], + "test_file_wo_ext.json" + ], + + [ + ["test_file_w_fake_ext.ext", "image", "image/svg+xml"], + "test_file_w_fake_ext.ext.svg" + ], + + [ + ["test_file_w_correct_ext.svg", "image", "image/svg+xml"], + "test_file_w_correct_ext.svg" + ], + + [ + ["test_file_w_correct_ext.svgz", "image", "image/svg+xml"], + "test_file_w_correct_ext.svgz" + ], + + [ + ["test_file.zip", "file", "application/zip"], + "test_file.zip" + ], + + [ + ["test_file", "file", "application/zip"], + "test_file.zip" + ], + + // application/octet-stream tests + [ + ["test_file", "file", "application/octet-stream"], + "test_file" + ], + + [ + ["test_file.zip", "file", "application/octet-stream"], + "test_file.zip" + ], + + [ + ["test_file.unknown", null, "application/octet-stream"], + "test_file.unknown" + ], + + // sanitized filename tests + [ + ["test/file", null, "application/octet-stream"], + "testfile" + ], + + [ + ["test:file.zip", "file", "application/zip"], + "testfile.zip" + ], + + [ + [":::", "file", "application/zip"], + ".zip" + ], + + [ + [":::a", "file", "application/zip"], + "a.zip" + ], +] + + +describe("utils/formatDownloadTitle unit tests", () => { + + testCases.forEach(testCase => { + return it(`With args '${JSON.stringify(testCase[0])}' it should return '${testCase[1]}'`, () => { + const [value, expected] = testCase; + const actual = formatDownloadTitle(...value); + expect(actual).toEqual(expected); + }) + }) + +}) + +execute() \ No newline at end of file From 6da656cd6747bceffbbd55ac4102e2585551a01f Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Fri, 3 Jan 2025 12:50:17 +0100 Subject: [PATCH 2/2] refactor(formatDownloadTitle): simplify function I've kept the "extension determination process" in a nested function, that reuses the formatDownloadTitle arguments, however it could also be refactored into an own util function later, if it is ever required. The for loop got replaced by the built functions in `mimeType` --- src/services/utils.ts | 50 +++++++++++++++---------------------------- 1 file changed, 17 insertions(+), 33 deletions(-) diff --git a/src/services/utils.ts b/src/services/utils.ts index ddb51609d..854953f75 100644 --- a/src/services/utils.ts +++ b/src/services/utils.ts @@ -178,45 +178,29 @@ export function replaceAll(string: string, replaceWhat: string, replaceWith: str } export function formatDownloadTitle(fileName: string, type: string | null, mime: string) { - if (!fileName) { - fileName = "untitled"; - } + const fileNameBase = (!fileName) ? "untitled" : sanitize(fileName); - fileName = sanitize(fileName); + const getExtension = () => { + if (type === "text") return ".html"; + if (type === "relationMap" || type === "canvas" || type === "search") return ".json"; + if (!mime) return ""; - if (type === 'text') { - return `${fileName}.html`; - } else if (type && ['relationMap', 'canvas', 'search'].includes(type)) { - return `${fileName}.json`; - } else { - if (!mime) { - return fileName; - } + const mimeLc = mime.toLowerCase(); - mime = mime.toLowerCase(); - const filenameLc = fileName.toLowerCase(); - const extensions = mimeTypes.extensions[mime]; + // better to just return the current name without a fake extension + // it's possible that the title still preserves the correct extension anyways + if (mimeLc === 'application/octet-stream') return ""; - if (!extensions || extensions.length === 0) { - return fileName; - } + // if fileName has an extension matching the mime already - reuse it + const mimeTypeFromFileName = mimeTypes.lookup(fileName); + if (mimeTypeFromFileName === mimeLc) return ""; - for (const ext of extensions) { - if (filenameLc.endsWith(`.${ext}`)) { - return fileName; - } - } + // as last resort try to get extension from mimeType + const extensions = mimeTypes.extension(mime); + return extensions ? `.${extensions}` : ""; + }; - if (mime === 'application/octet-stream') { - // we didn't find any good guess for this one, it will be better to just return - // the current name without a fake extension. It's possible that the title still preserves the correct - // extension too - - return fileName; - } - - return `${fileName}.${extensions[0]}`; - } + return `${fileNameBase}${getExtension()}`; } export function removeTextFileExtension(filePath: string) {