From c91bbabdbe512e3e254d8348ae9aac69f2e961e3 Mon Sep 17 00:00:00 2001
From: Carson McManus <dyc3@users.noreply.github.com>
Date: Thu, 4 Jul 2024 18:52:30 -0400
Subject: [PATCH] server: refactor video highlighting, fix videos not getting
 highlighted in add previews (#1778)

* add and update types

* server side impl

* update client

* fix some tests

* fix some tests

* fix some tests

* fix behavior

* fix length not showing up
---
 client/src/components/AddPreview.vue        |  9 ++-
 client/tests/e2e/component/AddPreview.cy.ts |  6 +-
 common/models/rest-api.ts                   |  1 +
 server/api/data.ts                          |  6 +-
 server/infoextractor.ts                     | 68 ++++++++++++++-------
 server/serviceadapter.ts                    |  3 +-
 server/services/youtube.ts                  | 43 +++++++------
 server/tests/unit/services/youtube.spec.ts  | 47 +++++++-------
 8 files changed, 114 insertions(+), 69 deletions(-)

diff --git a/client/src/components/AddPreview.vue b/client/src/components/AddPreview.vue
index 944364e22..9da150153 100644
--- a/client/src/components/AddPreview.vue
+++ b/client/src/components/AddPreview.vue
@@ -157,9 +157,7 @@ watch(inputAddPreview, () => {
 	onInputAddPreviewChange();
 });
 
-const highlightedAddPreviewItem = computed(() => {
-	return _.find(videos.value, { highlight: true });
-});
+const highlightedAddPreviewItem = ref<Video | undefined>(undefined);
 const isAddPreviewInputUrl = computed(() => {
 	try {
 		return !!new URL(inputAddPreview.value).host;
@@ -184,6 +182,7 @@ async function requestAddPreview() {
 		hasAddPreviewFailed.value = false;
 		if (res.data.success) {
 			videos.value = res.data.result;
+			highlightedAddPreviewItem.value = res.data.highlighted;
 			console.log(`Got add preview with ${videos.value.length}`);
 		} else {
 			throw new Error(res.data.error.message);
@@ -237,6 +236,7 @@ async function requestAddPreviewExplicit() {
 	isLoadingAddPreview.value = true;
 	hasAddPreviewFailed.value = false;
 	videos.value = [];
+	highlightedAddPreviewItem.value = undefined;
 	await requestAddPreview();
 }
 async function addAllToQueue() {
@@ -262,16 +262,19 @@ function onInputAddPreviewChange() {
 	hasAddPreviewFailed.value = false;
 	if (!inputAddPreview.value || _.trim(inputAddPreview.value).length === 0) {
 		videos.value = [];
+		highlightedAddPreviewItem.value = undefined;
 		return;
 	}
 	if (!isAddPreviewInputUrl.value) {
 		videos.value = [];
+		highlightedAddPreviewItem.value = undefined;
 		// Don't send API requests for non URL inputs without the user's explicit input to do so.
 		// This is to help conserve youtube API quota.
 		return;
 	}
 	isLoadingAddPreview.value = true;
 	videos.value = [];
+	highlightedAddPreviewItem.value = undefined;
 	requestAddPreviewDebounced();
 }
 function onInputAddPreviewKeyDown(e) {
diff --git a/client/tests/e2e/component/AddPreview.cy.ts b/client/tests/e2e/component/AddPreview.cy.ts
index 314b394e5..45ce16874 100644
--- a/client/tests/e2e/component/AddPreview.cy.ts
+++ b/client/tests/e2e/component/AddPreview.cy.ts
@@ -1,7 +1,7 @@
 import { defineComponent, h } from "vue";
 import { useStore } from "../../../src/store";
 import AddPreview from "../../../src/components/AddPreview.vue";
-import Norifier from "../../../src/components/Notifier.vue";
+import Notifier from "../../../src/components/Notifier.vue";
 
 let page = defineComponent({
 	setup() {
@@ -10,7 +10,7 @@ let page = defineComponent({
 		return {};
 	},
 	render() {
-		return h("div", {}, [h(AddPreview), h(Norifier)]);
+		return h("div", {}, [h(AddPreview), h(Notifier)]);
 	},
 });
 
@@ -83,7 +83,7 @@ describe("<AddPreview />", () => {
 				return {};
 			},
 			render() {
-				return h("div", {}, [h(AddPreview), h(Norifier)]);
+				return h("div", {}, [h(AddPreview), h(Notifier)]);
 			},
 		});
 		cy.mount(page);
diff --git a/common/models/rest-api.ts b/common/models/rest-api.ts
index e4c2109a1..3d3476ca0 100644
--- a/common/models/rest-api.ts
+++ b/common/models/rest-api.ts
@@ -78,6 +78,7 @@ export type OttApiRequestRemoveFromQueue = z.infer<typeof OttApiRequestRemoveFro
 
 export type OttApiResponseAddPreview = {
 	result: Video[];
+	highlighted?: Video;
 };
 
 export type OttApiRequestVote = z.infer<typeof OttApiRequestVoteSchema>;
diff --git a/server/api/data.ts b/server/api/data.ts
index f906e3f64..c385bf402 100644
--- a/server/api/data.ts
+++ b/server/api/data.ts
@@ -33,7 +33,6 @@ const addPreview: RequestHandler<
 			req.query.input.trim(),
 			conf.get("add_preview.search.provider")
 		);
-		const videos = result.videos;
 
 		res.setHeader(
 			"Cache-Control",
@@ -42,9 +41,10 @@ const addPreview: RequestHandler<
 
 		res.json({
 			success: true,
-			result: videos,
+			result: result.videos,
+			highlighted: result.highlighted,
 		});
-		log.info(`Sent add preview response with ${videos.length} items`);
+		log.info(`Sent add preview response with ${result.videos.length} items`);
 	} catch (err) {
 		if (
 			err.name === "UnsupportedServiceException" ||
diff --git a/server/infoextractor.ts b/server/infoextractor.ts
index a30840bba..29aa5f630 100644
--- a/server/infoextractor.ts
+++ b/server/infoextractor.ts
@@ -26,6 +26,7 @@ import { conf } from "./ott-config";
 import PeertubeAdapter from "./services/peertube";
 import PlutoAdapter from "./services/pluto";
 import DashVideoAdapter from "./services/dash";
+import { Ls } from "dayjs";
 
 const log = getLogger("infoextract");
 
@@ -355,30 +356,44 @@ export default {
 
 			const fetchResults = await adapter.resolveURL(query);
 			const resolvedResults: VideoId[] = [];
-			for (let video of fetchResults) {
-				if ("url" in video) {
-					try {
-						const adapter = this.getServiceAdapterForURL(video.url);
-						if (!adapter) {
+			if (Array.isArray(fetchResults)) {
+				for (let video of fetchResults) {
+					if ("url" in video) {
+						try {
+							const adapter = this.getServiceAdapterForURL(video.url);
+							if (!adapter) {
+								continue;
+							}
+							if (adapter.isCollectionURL(video.url)) {
+								continue;
+							}
+							resolvedResults.push({
+								service: adapter.serviceId,
+								id: adapter.getVideoId(video.url),
+							});
+						} catch (e) {
+							log.warn(`Failed to resolve video URL ${video.url}: ${e.message}`);
 							continue;
 						}
-						if (adapter.isCollectionURL(video.url)) {
-							continue;
-						}
-						resolvedResults.push({
-							service: adapter.serviceId,
-							id: adapter.getVideoId(video.url),
-						});
-					} catch (e) {
-						log.warn(`Failed to resolve video URL ${video.url}: ${e.message}`);
-						continue;
+					} else {
+						resolvedResults.push(video);
 					}
-				} else {
-					resolvedResults.push(video);
 				}
+				const completeResults = await this.getManyVideoInfo(resolvedResults);
+				return new AddPreview(completeResults, cacheDuration);
+			} else {
+				const videos = fetchResults.videos;
+				const completeResults: BulkVideoResult = {
+					videos: await this.getManyVideoInfo(videos),
+					highlighted: fetchResults.highlighted
+						? await this.getVideoInfo(
+								fetchResults.highlighted.service,
+								fetchResults.highlighted.id
+						  )
+						: undefined,
+				};
+				return new AddPreview(completeResults, cacheDuration);
 			}
-			const completeResults = await this.getManyVideoInfo(resolvedResults);
-			results.push(...completeResults);
 		} else {
 			if (query.length < ADD_PREVIEW_SEARCH_MIN_LENGTH) {
 				throw new InvalidAddPreviewInputException(ADD_PREVIEW_SEARCH_MIN_LENGTH);
@@ -424,13 +439,24 @@ export class AddPreview {
 	videos: Video[];
 	/** The number of seconds to allow downstream caches to cache the response. Affects caching HTTP headers. */
 	cacheDuration: number;
+	highlighted?: Video;
 
-	constructor(videos: Video[], cacheDuration: number) {
-		this.videos = videos;
+	constructor(videos: Video[] | BulkVideoResult, cacheDuration: number) {
+		if (Array.isArray(videos)) {
+			this.videos = videos;
+		} else {
+			this.videos = videos.videos;
+			this.highlighted = videos.highlighted;
+		}
 		this.cacheDuration = cacheDuration;
 	}
 }
 
+export interface BulkVideoResult {
+	videos: Video[];
+	highlighted?: Video;
+}
+
 const counterAddPreviewsRequested = new Counter({
 	name: "ott_infoextractor_add_previews_requested",
 	help: "The number of add previews that have been requested",
diff --git a/server/serviceadapter.ts b/server/serviceadapter.ts
index 06c5f809a..5b1d4f5c6 100644
--- a/server/serviceadapter.ts
+++ b/server/serviceadapter.ts
@@ -3,6 +3,7 @@
 import { Video, VideoId, VideoMetadata, VideoService } from "ott-common/models/video";
 import { IncompleteServiceAdapterException } from "./exceptions";
 import { getLogger } from "./logger";
+import { BulkVideoResult } from "./infoextractor";
 
 const log = getLogger("serviceadapter");
 export interface VideoRequest {
@@ -92,7 +93,7 @@ export class ServiceAdapter {
 	async resolveURL(
 		url: string,
 		properties?: (keyof VideoMetadata)[]
-	): Promise<(Video | { url: string })[]> {
+	): Promise<(Video | { url: string })[] | BulkVideoResult> {
 		throw new IncompleteServiceAdapterException(
 			`Service ${this.serviceId} does not implement method resolveURL`
 		);
diff --git a/server/services/youtube.ts b/server/services/youtube.ts
index 4c149438c..945686eb3 100644
--- a/server/services/youtube.ts
+++ b/server/services/youtube.ts
@@ -16,6 +16,7 @@ import storage from "../storage";
 import { OttException } from "ott-common/exceptions";
 import { conf } from "../ott-config";
 import { parseIso8601Duration } from "./parsing/iso8601";
+import { BulkVideoResult } from "server/infoextractor";
 
 const log = getLogger("youtube");
 
@@ -203,7 +204,10 @@ export default class YouTubeAdapter extends ServiceAdapter {
 		}
 	}
 
-	async resolveURL(link: string, onlyProperties?: (keyof VideoMetadata)[]): Promise<Video[]> {
+	async resolveURL(
+		link: string,
+		onlyProperties?: (keyof VideoMetadata)[]
+	): Promise<BulkVideoResult> {
 		log.debug(`resolveURL: ${link}, ${onlyProperties ? onlyProperties.toString() : ""}`);
 		const url = new URL(link);
 
@@ -215,10 +219,12 @@ export default class YouTubeAdapter extends ServiceAdapter {
 			url.pathname.startsWith("/user/") ||
 			url.pathname.startsWith("/@")
 		) {
-			return this.fetchChannelVideos(this.getChannelId(url));
+			const videos = await this.fetchChannelVideos(this.getChannelId(url));
+			return { videos };
 		} else if (url.pathname === "/playlist") {
 			if (qPlaylist) {
-				return this.fetchPlaylistVideos(qPlaylist);
+				const videos = await this.fetchPlaylistVideos(qPlaylist);
+				return { videos };
 			} else {
 				throw new BadApiArgumentException("input", "Link is missing playlist ID");
 			}
@@ -228,10 +234,14 @@ export default class YouTubeAdapter extends ServiceAdapter {
 					return await this.fetchVideoWithPlaylist(this.getVideoId(link), qPlaylist);
 				} catch {
 					log.debug("Falling back to fetching video without playlist");
-					return [await this.fetchVideoInfo(this.getVideoId(link), onlyProperties)];
+					return {
+						videos: [await this.fetchVideoInfo(this.getVideoId(link), onlyProperties)],
+					};
 				}
 			} else {
-				return [await this.fetchVideoInfo(this.getVideoId(link), onlyProperties)];
+				return {
+					videos: [await this.fetchVideoInfo(this.getVideoId(link), onlyProperties)],
+				};
 			}
 		}
 	}
@@ -410,23 +420,22 @@ export default class YouTubeAdapter extends ServiceAdapter {
 		}
 	}
 
-	async fetchVideoWithPlaylist(videoId: string, playlistId: string): Promise<Video[]> {
+	async fetchVideoWithPlaylist(videoId: string, playlistId: string): Promise<BulkVideoResult> {
 		const playlist = await this.fetchPlaylistVideos(playlistId);
-		let highlighted = false;
-		playlist.forEach(video => {
-			if (video.id === videoId) {
-				highlighted = true;
-				video.highlight = true;
-			}
-		});
+		let highlighted = playlist.find(video => video.id === videoId);
 
 		if (!highlighted) {
-			const video = await this.fetchVideoInfo(videoId);
-			video.highlight = true;
-			playlist.unshift(video);
+			try {
+				highlighted = await this.fetchVideoInfo(videoId);
+			} catch (err) {
+				log.warn(`Failed to fetch highlighted video from playlist, skipping: ${err}`);
+			}
 		}
 
-		return playlist;
+		return {
+			videos: playlist,
+			highlighted,
+		};
 	}
 
 	async videoApiRequest(
diff --git a/server/tests/unit/services/youtube.spec.ts b/server/tests/unit/services/youtube.spec.ts
index 7a9118198..192d9229e 100644
--- a/server/tests/unit/services/youtube.spec.ts
+++ b/server/tests/unit/services/youtube.spec.ts
@@ -119,7 +119,7 @@ async function mockYoutubeApi(
 			};
 		} catch (e) {
 			let content = fs.readFileSync(`${FIXTURE_DIRECTORY}/errors/${e.message}.json`, "utf8");
-			return JSON.parse(content);
+			throw JSON.parse(content);
 		}
 	} else if (path === "/channels") {
 		return {
@@ -255,8 +255,8 @@ describe("Youtube", () => {
 			].map(x => x.replace("%s", "BTZ5KVRUy1Q"))
 		)("Resolves single video URL: %s", async link => {
 			const videos = await adapter.resolveURL(link);
-			expect(videos).toHaveLength(1);
-			expect(videos[0]).toEqual({
+			expect(videos.videos).toHaveLength(1);
+			expect(videos.videos[0]).toEqual({
 				service: "youtube",
 				id: "BTZ5KVRUy1Q",
 				title: "tmpIwT4T4",
@@ -280,7 +280,7 @@ describe("Youtube", () => {
 			expect(fetchSpy).toHaveBeenCalledTimes(1);
 			expect(fetchPlaylist).toHaveBeenCalledTimes(1);
 			expect(fetchVideo).toHaveBeenCalledTimes(0);
-			expect(videos).toEqual([
+			expect(videos.videos).toEqual([
 				{
 					service: "youtube",
 					id: "zgxj_0xPleg",
@@ -289,7 +289,6 @@ describe("Youtube", () => {
 					thumbnail: "https://i.ytimg.com/vi/zgxj_0xPleg/mqdefault.jpg",
 					// length expected to be undefined because the youtube api doesn't return video length in playlist items
 					// feature requested here: https://issuetracker.google.com/issues/173420445
-					highlight: true,
 				},
 				{
 					service: "youtube",
@@ -299,6 +298,13 @@ describe("Youtube", () => {
 					thumbnail: "https://i.ytimg.com/vi/_3QMqssyBwQ/default.jpg",
 				},
 			]);
+			expect(videos.highlighted).toEqual({
+				service: "youtube",
+				id: "zgxj_0xPleg",
+				title: "Chris Chan: A Comprehensive History - Part 1",
+				description: "(1982-2000)",
+				thumbnail: "https://i.ytimg.com/vi/zgxj_0xPleg/mqdefault.jpg",
+			});
 			expect(apiGet).toHaveBeenCalledTimes(1);
 
 			fetchSpy.mockRestore();
@@ -321,16 +327,15 @@ describe("Youtube", () => {
 				expect(fetchSpy).toHaveBeenCalledTimes(1);
 				expect(fetchPlaylist).toHaveBeenCalledTimes(1);
 				expect(fetchVideo).toHaveBeenCalledTimes(1);
-				expect(videos).toEqual([
-					{
-						service: "youtube",
-						id: "BTZ5KVRUy1Q",
-						title: "tmpIwT4T4",
-						description: "tmpIwT4T4",
-						thumbnail: "https://i.ytimg.com/vi/BTZ5KVRUy1Q/mqdefault.jpg",
-						length: 10,
-						highlight: true,
-					},
+				expect(videos.highlighted).toEqual({
+					service: "youtube",
+					id: "BTZ5KVRUy1Q",
+					title: "tmpIwT4T4",
+					description: "tmpIwT4T4",
+					thumbnail: "https://i.ytimg.com/vi/BTZ5KVRUy1Q/mqdefault.jpg",
+					length: 10,
+				});
+				expect(videos.videos).toEqual([
 					{
 						service: "youtube",
 						id: "zgxj_0xPleg",
@@ -359,8 +364,8 @@ describe("Youtube", () => {
 		it("Recovers after not being able to fetch playlist information for a video", async () => {
 			const link = "https://youtube.com/watch?v=BTZ5KVRUy1Q&list=fakelistid";
 			const videos = await adapter.resolveURL(link);
-			expect(videos).toHaveLength(1);
-			expect(videos[0]).toEqual({
+			expect(videos.videos).toHaveLength(1);
+			expect(videos.videos[0]).toEqual({
 				service: "youtube",
 				id: "BTZ5KVRUy1Q",
 				title: "tmpIwT4T4",
@@ -375,9 +380,9 @@ describe("Youtube", () => {
 			async link => {
 				const fetchVideoWithPlaylist = vi.spyOn(adapter, "fetchVideoWithPlaylist");
 				const fetchVideo = vi.spyOn(adapter, "fetchVideoInfo");
-				const videos: Video[] = await adapter.resolveURL(link);
-				expect(videos).toHaveLength(1);
-				expect(videos[0]).toEqual({
+				const videos = await adapter.resolveURL(link);
+				expect(videos.videos).toHaveLength(1);
+				expect(videos.videos[0]).toEqual({
 					service: "youtube",
 					id: "BTZ5KVRUy1Q",
 					title: "tmpIwT4T4",
@@ -404,7 +409,7 @@ describe("Youtube", () => {
 			expect(fetchVideoWithPlaylist).toHaveBeenCalledTimes(0);
 			expect(fetchPlaylist).toHaveBeenCalledTimes(1);
 			expect(fetchVideo).toHaveBeenCalledTimes(0);
-			expect(videos).toEqual([
+			expect(videos.videos).toEqual([
 				{
 					service: "youtube",
 					id: "zgxj_0xPleg",