From 526bdaaed8260e2955a30749f731283d25263399 Mon Sep 17 00:00:00 2001
From: Alexandre Rousseau <alexandre@rsseau.fr>
Date: Mon, 23 Dec 2024 11:07:55 +0100
Subject: [PATCH] chore(ui): improve icon fetching. WF-141

---
 .../builder/sidebar/BuilderSidebarToolkit.vue | 33 ++++------
 .../components/core/content/CoreChatbot.vue   |  2 +-
 .../shared/SharedImgWithFallback.spec.ts      | 37 +++++++++++
 .../shared/SharedImgWithFallback.vue          | 31 +++++++++
 .../workflows/abstract/WorkflowsNode.vue      | 38 +++--------
 .../composables/useAssetContentType.spec.ts   | 65 +++++++++++++++++++
 src/ui/src/composables/useAssetContentType.ts | 29 +++++++++
 src/ui/src/utils/url.spec.ts                  | 22 +++++++
 src/ui/src/utils/url.ts                       | 14 ++++
 9 files changed, 219 insertions(+), 52 deletions(-)
 create mode 100644 src/ui/src/components/shared/SharedImgWithFallback.spec.ts
 create mode 100644 src/ui/src/components/shared/SharedImgWithFallback.vue
 create mode 100644 src/ui/src/composables/useAssetContentType.spec.ts
 create mode 100644 src/ui/src/composables/useAssetContentType.ts
 create mode 100644 src/ui/src/utils/url.spec.ts
 create mode 100644 src/ui/src/utils/url.ts

diff --git a/src/ui/src/builder/sidebar/BuilderSidebarToolkit.vue b/src/ui/src/builder/sidebar/BuilderSidebarToolkit.vue
index d9bc28bfc..dcd447b8c 100644
--- a/src/ui/src/builder/sidebar/BuilderSidebarToolkit.vue
+++ b/src/ui/src/builder/sidebar/BuilderSidebarToolkit.vue
@@ -23,19 +23,10 @@
 					@dragend="handleDragEnd($event)"
 					@dragstart="handleDragStart($event, tool.type)"
 				>
-					<img
+					<SharedImgWithFallback
 						:alt="`(Icon for ${tool.name})`"
-						:src="`./../../../../components/${tool.type}.svg`"
 						draggable="false"
-						@error="
-							!isImageFallback[tool.type]
-								? handleImageError(
-										$event,
-										tool.type,
-										tool.category,
-									)
-								: undefined
-						"
+						:urls="getToolIcons(tool)"
 					/>
 					<div class="name">{{ tool.name }}</div>
 				</div>
@@ -54,12 +45,13 @@ import {
 import injectionKeys from "@/injectionKeys";
 import { useDragDropComponent } from "../useDragDropComponent";
 import { Component } from "@/writerTypes";
+import SharedImgWithFallback from "@/components/shared/SharedImgWithFallback.vue";
+import { convertAbsolutePathtoFullURL } from "@/utils/url";
 
 const wf = inject(injectionKeys.core);
 const wfbm = inject(injectionKeys.builderManager);
 const { removeInsertionCandidacy } = useDragDropComponent(wf);
 const query = ref("");
-const isImageFallback = ref<Record<Component["type"], boolean>>({});
 
 const displayedCategories = [
 	"Layout",
@@ -114,16 +106,6 @@ function getRelevantToolsInCategory(categoryId: string) {
 	return queryApplied;
 }
 
-function handleImageError(
-	ev: Event,
-	type: Component["type"],
-	categoryId: string,
-) {
-	isImageFallback.value[type] = true; // Prevent calling more than once
-	const imageEl = ev.target as HTMLImageElement;
-	imageEl.src = `./../../../../components/category_${categoryId}.svg`;
-}
-
 function handleDragStart(ev: DragEvent, type: Component["type"]) {
 	wfbm.setSelection(null);
 	ev.dataTransfer.setData(`application/json;writer=${type},`, "{}");
@@ -133,6 +115,13 @@ function handleDragEnd(ev: DragEvent) {
 	removeInsertionCandidacy(ev);
 }
 
+function getToolIcons(tool: ReturnType<typeof getRelevantToolsInCategory>[0]) {
+	return [
+		`/components/${tool.type}.svg`,
+		`/components/category_${tool.category}.svg`,
+	].map((p) => convertAbsolutePathtoFullURL(p));
+}
+
 watch(activeToolkit, () => {
 	query.value = "";
 });
diff --git a/src/ui/src/components/core/content/CoreChatbot.vue b/src/ui/src/components/core/content/CoreChatbot.vue
index 521bfa16c..0cba76b74 100644
--- a/src/ui/src/components/core/content/CoreChatbot.vue
+++ b/src/ui/src/components/core/content/CoreChatbot.vue
@@ -401,7 +401,7 @@ function scrollToBottom() {
 }
 
 const encodeFile = async (file: File) => {
-	var reader = new FileReader();
+	const reader = new FileReader();
 	reader.readAsDataURL(file);
 
 	return new Promise((resolve, reject) => {
diff --git a/src/ui/src/components/shared/SharedImgWithFallback.spec.ts b/src/ui/src/components/shared/SharedImgWithFallback.spec.ts
new file mode 100644
index 000000000..846fc944f
--- /dev/null
+++ b/src/ui/src/components/shared/SharedImgWithFallback.spec.ts
@@ -0,0 +1,37 @@
+import { describe, it, expect, vi, beforeEach, Mock } from "vitest";
+import SharedImgWithFallback from "./SharedImgWithFallback.vue";
+import { flushPromises, shallowMount } from "@vue/test-utils";
+
+describe("SharedImgWithFallback", () => {
+	let fetch: Mock;
+
+	beforeEach(() => {
+		fetch = vi.fn().mockResolvedValue({
+			ok: true,
+			headers: new Map([["Content-Type", "image/png"]]),
+		});
+		global.fetch = fetch;
+	});
+
+	it("should use the last image because the first two are not valid", async () => {
+		fetch
+			.mockRejectedValueOnce(new Error())
+			.mockResolvedValueOnce({
+				ok: true,
+				headers: new Map([["Content-Type", "text/html"]]),
+			})
+			.mockResolvedValue({
+				ok: true,
+				headers: new Map([["Content-Type", "image/png"]]),
+			});
+
+		const wrapper = shallowMount(SharedImgWithFallback, {
+			props: { urls: ["/img1.svg", "/img2.svg", "/img3.svg"] },
+		});
+		expect(wrapper.get("img").attributes().src).toBe("");
+
+		await flushPromises();
+
+		expect(wrapper.get("img").attributes().src).toBe("/img3.svg");
+	});
+});
diff --git a/src/ui/src/components/shared/SharedImgWithFallback.vue b/src/ui/src/components/shared/SharedImgWithFallback.vue
new file mode 100644
index 000000000..d4b0d7b47
--- /dev/null
+++ b/src/ui/src/components/shared/SharedImgWithFallback.vue
@@ -0,0 +1,31 @@
+<template>
+	<img :src="src" />
+</template>
+
+<script lang="ts" setup>
+import { useAssetContentType } from "@/composables/useAssetContentType";
+import { PropType, ref, toRef, watch } from "vue";
+
+const props = defineProps({
+	urls: { type: Array as PropType<string[]>, required: true },
+});
+
+const src = ref("");
+
+const { fetchAssetContentType } = useAssetContentType();
+
+watch(
+	toRef(props, "urls"),
+	async (urls) => {
+		src.value = "";
+
+		for (const url of urls) {
+			const contentType = await fetchAssetContentType(url);
+			// ensure that the content type is valid and not HTML (the server can responds with a default HTML page)
+			if (!contentType || contentType === "text/html") continue;
+			return (src.value = url);
+		}
+	},
+	{ immediate: true },
+);
+</script>
diff --git a/src/ui/src/components/workflows/abstract/WorkflowsNode.vue b/src/ui/src/components/workflows/abstract/WorkflowsNode.vue
index 5db673f1a..757e4ec81 100644
--- a/src/ui/src/components/workflows/abstract/WorkflowsNode.vue
+++ b/src/ui/src/components/workflows/abstract/WorkflowsNode.vue
@@ -1,7 +1,7 @@
 <template>
 	<div class="WorkflowsNode">
 		<div class="title">
-			<img :src="imagePath" />
+			<SharedImgWithFallback :urls="possibleImageUrls" />
 			<WorkflowsNodeNamer
 				:component-id="componentId"
 				class="nodeNamer"
@@ -69,17 +69,18 @@ export default {
 };
 </script>
 <script setup lang="ts">
-import { computed, inject, onMounted, ref, watch } from "vue";
+import { computed, inject, watch } from "vue";
 import injectionKeys from "@/injectionKeys";
 import { FieldType, WriterComponentDefinition } from "@/writerTypes";
 import WorkflowsNodeNamer from "../base/WorkflowsNodeNamer.vue";
+import SharedImgWithFallback from "@/components/shared/SharedImgWithFallback.vue";
+import { convertAbsolutePathtoFullURL } from "@/utils/url";
 
 const emit = defineEmits(["outMousedown", "engaged"]);
 const wf = inject(injectionKeys.core);
 const wfbm = inject(injectionKeys.builderManager);
 const componentId = inject(injectionKeys.componentId);
 const fields = inject(injectionKeys.evaluatedFields);
-const imagePath = ref<string>(null);
 
 const component = computed(() => {
 	const component = wf.getComponentById(componentId);
@@ -142,38 +143,17 @@ function handleOutMousedown(ev: DragEvent, outId: string) {
 	emit("outMousedown", outId);
 }
 
-async function checkIfUrlExists(url: string) {
-	try {
-		const response = await fetch(url, { method: "HEAD" });
-		return response.ok;
-	} catch {
-		return false;
-	}
-}
-
-async function getBestAvailableImagePath() {
+const possibleImageUrls = computed(() => {
 	const paths = [
-		`./../../../../components/${component.value.type}.svg`,
-		`./../../../../components/workflows_category_${def.value.category}.svg`,
+		`/components/${component.value.type}.svg`,
+		`/components/workflows_category_${def.value.category}.svg`,
 	];
 
 	if (wf.featureFlags.value.includes("custom_block_icons")) {
-		paths.unshift(
-			`./../../../../static/components/${component.value.id}.svg`,
-		);
+		paths.unshift(`/static/components/${component.value.id}.svg`);
 	}
 
-	for (let i = 0; i < paths.length; i++) {
-		const path = paths[i];
-		if (await checkIfUrlExists(path)) {
-			return path;
-		}
-	}
-	return "";
-}
-
-onMounted(async () => {
-	imagePath.value = await getBestAvailableImagePath();
+	return paths.map((p) => convertAbsolutePathtoFullURL(p));
 });
 
 watch(isEngaged, () => {
diff --git a/src/ui/src/composables/useAssetContentType.spec.ts b/src/ui/src/composables/useAssetContentType.spec.ts
new file mode 100644
index 000000000..1c9cbda79
--- /dev/null
+++ b/src/ui/src/composables/useAssetContentType.spec.ts
@@ -0,0 +1,65 @@
+import { useAssetContentType } from "./useAssetContentType";
+import { beforeEach, describe, it, expect, Mock, vi } from "vitest";
+
+describe(useAssetContentType.name, () => {
+	let fetch: Mock;
+
+	beforeEach(() => {
+		fetch = vi.fn().mockResolvedValue({
+			ok: true,
+			headers: new Map([["Content-Type", "image/png"]]),
+		});
+		global.fetch = fetch;
+
+		useAssetContentType().clearCache();
+	});
+
+	it("should handle error ", async () => {
+		fetch.mockRejectedValue(new Error());
+		const { fetchAssetContentType } = useAssetContentType();
+
+		expect(await fetchAssetContentType("https://test.com")).toBeUndefined();
+		expect(fetch).toHaveBeenCalledOnce();
+	});
+
+	it("should cache fetch call in sequential calls", async () => {
+		const { fetchAssetContentType } = useAssetContentType();
+
+		expect(await fetchAssetContentType("https://test.com")).toBe(
+			"image/png",
+		);
+		expect(await fetchAssetContentType("https://test.com")).toBe(
+			"image/png",
+		);
+		expect(fetch).toHaveBeenCalledOnce();
+	});
+
+	it("should cache fetch call in parrallel call", async () => {
+		vi.useFakeTimers();
+
+		fetch.mockResolvedValue(
+			new Promise((res) =>
+				setTimeout(
+					() =>
+						res({
+							ok: true,
+							headers: new Map([["Content-Type", "image/png"]]),
+						}),
+					3_000,
+				),
+			),
+		);
+
+		const { fetchAssetContentType } = useAssetContentType();
+
+		const res1 = fetchAssetContentType("https://test.com");
+		const res2 = fetchAssetContentType("https://test.com");
+
+		vi.advanceTimersByTime(3_000);
+
+		expect(await res1).toBe("image/png");
+		expect(await res2).toBe("image/png");
+
+		expect(fetch).toHaveBeenCalledOnce();
+	});
+});
diff --git a/src/ui/src/composables/useAssetContentType.ts b/src/ui/src/composables/useAssetContentType.ts
new file mode 100644
index 000000000..f8a993f51
--- /dev/null
+++ b/src/ui/src/composables/useAssetContentType.ts
@@ -0,0 +1,29 @@
+const cacheUrlContentType = new Map<string, Promise<undefined | string>>();
+
+/**
+ * Do an HTTP `HEAD` call to get the `Content-Type` of an URL. Handle parrallel calls and use a cache mechanism.
+ */
+export function useAssetContentType() {
+	function fetchAssetContentType(url: string) {
+		const cachedValue = cacheUrlContentType.get(url);
+		if (cachedValue !== undefined) return cachedValue;
+
+		// we store the promise instead of the result to handle concurent calls
+		const promise = fetch(url, { method: "HEAD" })
+			.then((r) => {
+				if (!r.ok) return undefined;
+				return r.headers.get("Content-Type") || undefined;
+			})
+			.catch(() => undefined);
+
+		cacheUrlContentType.set(url, promise);
+
+		return promise;
+	}
+
+	function clearCache() {
+		cacheUrlContentType.clear();
+	}
+
+	return { fetchAssetContentType, clearCache };
+}
diff --git a/src/ui/src/utils/url.spec.ts b/src/ui/src/utils/url.spec.ts
new file mode 100644
index 000000000..bf03ea356
--- /dev/null
+++ b/src/ui/src/utils/url.spec.ts
@@ -0,0 +1,22 @@
+import { describe, expect, it } from "vitest";
+import { convertAbsolutePathtoFullURL } from "./url";
+
+describe(convertAbsolutePathtoFullURL.name, () => {
+	it("should convert the URL", () => {
+		expect(
+			convertAbsolutePathtoFullURL(
+				"/assets/image.png",
+				"http://localhost:3000/",
+			),
+		).toBe("http://localhost:3000/assets/image.png");
+	});
+
+	it("should convert the URL with a current path", () => {
+		expect(
+			convertAbsolutePathtoFullURL(
+				"/assets/image.png",
+				"http://localhost:3000/hello/?foo=bar",
+			),
+		).toBe("http://localhost:3000/hello/assets/image.png");
+	});
+});
diff --git a/src/ui/src/utils/url.ts b/src/ui/src/utils/url.ts
new file mode 100644
index 000000000..a47261035
--- /dev/null
+++ b/src/ui/src/utils/url.ts
@@ -0,0 +1,14 @@
+/**
+ * Convert absoule URL to full URL in case the application is hosted on a subpath.
+ *
+ * ```js
+ * convertAbsolutePathtoFullURL("/assets/image.png", "http://localhost:3000/hello/?foo=bar")
+ * // => 'http://localhost:3000/hello/assets/image.png'
+ * ```
+ */
+export function convertAbsolutePathtoFullURL(
+	path: string,
+	base = window.location.toString(),
+) {
+	return new URL(`.${path}`, base).toString();
+}