Refactor thumbnail cache

The thumbnail cache was trying to fit two concepts into one structure --
icons and multi-dimensional thumbnail arrays.

This was awkward originally, but now that we are moving towards ditching
the concept of a list of thumbnail types it really doesn't fit any
longer.

We can do better by having the cache store two types of thumbnails:
icons and images.  These are reflected explicitly now via TS interfaces.

Author: slifty

src/app/shared/utilities/thumbnail-cache/thumbnail-cache.spec.ts

@@ -30,7 +30,7 @@ describe('ThumbnailCache', () => {
 	it('should return an empty FolderThumbData object for uncached thumbnail', () => {
 		const thumbs = cache.getThumbnail(folder);
 
-		expect(thumbs.folderThumb).toBeDefined();
+		expect(thumbs.folderThumb).toBe('');
 		expect(thumbs.folderContentsType).toBe(
 			FolderContentsType.BROKEN_THUMBNAILS,
 		);
@@ -91,8 +91,20 @@ describe('ThumbnailCache', () => {
 			expect(cache.hasThumbnail(folder)).toBeFalse();
 		});
 
-		it('handles linking another value instead of a string tuple', () => {
-			storage.session.set('folderThumbnailCache', [[1234, 'potato']]);
+		it('reports no cache hit for stale entries', () => {
+			storage.session.set('folderThumbnailCache', [
+				[1234, ['https://old200', 'https://old500']],
+			]);
+			cache = new ThumbnailCache(storage);
+
+			expect(cache.hasThumbnail(folder)).toBeFalse();
+			expect(storage.session.get('folderThumbnailCache').length).toBe(0);
+		});
+
+		it('wipes stale [thumb200, thumb500] tuples from old format', () => {
+			storage.session.set('folderThumbnailCache', [
+				[1234, ['https://old200', 'https://old500']],
+			]);
 			cache = new ThumbnailCache(storage);
 			const thumbz = cache.getThumbnail(folder);
 
@@ -104,13 +116,19 @@ describe('ThumbnailCache', () => {
 			expect(storage.session.get('folderThumbnailCache').length).toBe(0);
 		});
 
-		it('handles properly casting other values to string if a tuple is provided', () => {
-			storage.session.set('folderThumbnailCache', [[1234, [3.141, {}]]]);
+		it('wipes entries with unrecognized shape', () => {
+			storage.session.set('folderThumbnailCache', [
+				[1234, { something: 'unexpected' }],
+			]);
 			cache = new ThumbnailCache(storage);
 			const thumbz = cache.getThumbnail(folder);
 
-			expect(thumbz.folderThumb).toBe('3.141');
-			expect(thumbz.folderContentsType).toBe(FolderContentsType.NORMAL);
+			expect(thumbz.folderThumb).toBe('');
+			expect(thumbz.folderContentsType).toBe(
+				FolderContentsType.BROKEN_THUMBNAILS,
+			);
+
+			expect(storage.session.get('folderThumbnailCache').length).toBe(0);
 		});
 	});
 });

src/app/shared/utilities/thumbnail-cache/thumbnail-cache.ts

@@ -7,8 +7,40 @@ export interface FolderThumbData {
 	folderContentsType: FolderContentsType;
 }
 
+enum CachedThumbnailType {
+	Image = 'image',
+	Icon = 'icon',
+}
+
+interface CachedThumbnailBase {
+	type: CachedThumbnailType;
+}
+
+interface CachedImage extends CachedThumbnailBase {
+	type: CachedThumbnailType.Image;
+	url: string;
+}
+
+interface CachedIcon extends CachedThumbnailBase {
+	type: CachedThumbnailType.Icon;
+	icon: FolderContentsType;
+}
+
+type CachedThumbnail = CachedImage | CachedIcon;
+
+function isCachedThumbnail(entry: unknown): entry is CachedThumbnail {
+	return (
+		typeof entry === 'object' &&
+		entry !== null &&
+		'type' in entry &&
+		Object.values(CachedThumbnailType).includes(
+			(entry as CachedThumbnailBase).type,
+		)
+	);
+}
+
 export class ThumbnailCache {
-	private cache: Map<number, [string, string]>;
+	private cache: Map<number, CachedThumbnail>;
 	private readonly STORAGE_KEY: string = 'folderThumbnailCache';
 
 	constructor(private storage: StorageService) {
@@ -18,35 +50,38 @@ export class ThumbnailCache {
 	public saveThumbnail(item: ItemVO, thumbs: FolderThumbData): void {
 		this.fetchCacheMapFromStorage();
 		if (thumbs.folderContentsType === FolderContentsType.NORMAL) {
-			this.cache.set(item.folder_linkId, [
-				thumbs.folderThumb,
-				'',
-			]);
+			this.cache.set(item.folder_linkId, {
+				type: CachedThumbnailType.Image,
+				url: thumbs.folderThumb,
+			});
 		} else {
-			this.cache.set(item.folder_linkId, ['icon', thumbs.folderContentsType]);
+			this.cache.set(item.folder_linkId, {
+				type: CachedThumbnailType.Icon,
+				icon: thumbs.folderContentsType,
+			});
 		}
 		this.saveMapToStorage();
 	}
 
 	public getThumbnail(item: ItemVO): FolderThumbData {
 		if (this.cache.has(item.folder_linkId)) {
-			const thumbs = this.cache.get(item.folder_linkId);
-			if (thumbs && Array.isArray(thumbs) && thumbs.length > 1) {
-				if (thumbs[0] === 'icon') {
+			const entry = this.cache.get(item.folder_linkId);
+			if (isCachedThumbnail(entry)) {
+				if (entry.type === CachedThumbnailType.Image) {
+					return {
+						folderThumb: entry.url,
+						folderContentsType: FolderContentsType.NORMAL,
+					};
+				}
+				if (entry.type === CachedThumbnailType.Icon) {
 					return {
 						folderThumb: '',
-						folderContentsType: thumbs[1] as FolderContentsType,
+						folderContentsType: entry.icon,
 					};
 				}
-				// Cast to string just to be sure we actually have strings from our data structure.
-				return {
-					folderThumb: `${thumbs[0]}`,
-					folderContentsType: FolderContentsType.NORMAL,
-				};
-			} else {
-				this.cache.delete(item.folder_linkId);
-				this.saveMapToStorage();
 			}
+			this.cache.delete(item.folder_linkId);
+			this.saveMapToStorage();
 		}
 		return {
 			folderThumb: '',
@@ -55,7 +90,15 @@ export class ThumbnailCache {
 	}
 
 	public hasThumbnail(item: ItemVO): boolean {
-		return this.cache.has(item.folder_linkId);
+		if (!this.cache.has(item.folder_linkId)) {
+			return false;
+		}
+		if (isCachedThumbnail(this.cache.get(item.folder_linkId))) {
+			return true;
+		}
+		this.cache.delete(item.folder_linkId);
+		this.saveMapToStorage();
+		return false;
 	}
 
 	public invalidateFolder(folderLinkId: number): void {
@@ -68,9 +111,9 @@ export class ThumbnailCache {
 	private fetchCacheMapFromStorage(): void {
 		const cacheData = this.storage.session.get(this.STORAGE_KEY);
 		if (cacheData && Array.isArray(cacheData)) {
-			this.cache = new Map<number, [string, string]>(cacheData);
+			this.cache = new Map<number, CachedThumbnail>(cacheData);
 		} else {
-			this.cache = new Map<number, [string, string]>();
+			this.cache = new Map<number, CachedThumbnail>();
 			this.saveMapToStorage();
 		}
 	}