Skip to content

Commit

Permalink
Improved error logging when file icon extraction fails
Browse files Browse the repository at this point in the history
  • Loading branch information
oliverschwendener committed Feb 23, 2024
1 parent 98f8f81 commit db4cde8
Show file tree
Hide file tree
Showing 36 changed files with 360 additions and 103 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions src/common/Core/ContextBridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export type ContextBridge = {
on: IpcRenderer["on"];
};

resetChache: () => Promise<void>;
copyTextToClipboard: (textToCopy: string) => void;
extensionDisabled: (extensionId: string) => void;
extensionEnabled: (extensionId: string) => void;
Expand Down
5 changes: 5 additions & 0 deletions src/main/Core/ExtensionManager/ExtensionManagerModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ export class ExtensionManagerModule {

await extensionManager.populateSearchIndex();

ipcMain.handle("resetCache", async () => {
await dependencyRegistry.get("FileImageGenerator").clearCache();
await extensionManager.populateSearchIndex();
});

ipcMain.on("getExtensionTranslations", (event) => {
event.returnValue = extensionManager.getSupportedExtensions().map((extension) => ({
extensionId: extension.id,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export interface FileSystemUtility {
createFolderIfDoesntExist(folderPath: string): Promise<void>;
clearFolder(folderPath: string): Promise<void>;
pathExists(fileOrFolderPath: string): Promise<boolean>;
readJsonFile<T>(filePath: string): Promise<T>;
readJsonFileSync<T>(filePath: string): T;
Expand Down
19 changes: 19 additions & 0 deletions src/main/Core/FileSystemUtility/NodeJsFileSystemUtility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,21 @@ import {
mkdir,
readFile,
readFileSync,
readdir,
statSync,
unlink,
writeFile,
writeFileSync,
} from "fs";
import { join } from "path";
import type { FileSystemUtility } from "./Contract";

export class NodeJsFileSystemUtility implements FileSystemUtility {
public async clearFolder(folderPath: string): Promise<void> {
const filePaths = await this.readDirectory(folderPath);
await Promise.all(filePaths.map((filePath) => this.removeFile(filePath)));
}

public async createFolderIfDoesntExist(folderPath: string): Promise<void> {
const exists = await this.pathExists(folderPath);

Expand Down Expand Up @@ -110,4 +117,16 @@ export class NodeJsFileSystemUtility implements FileSystemUtility {
private readFileSync(filePath: string): Buffer {
return readFileSync(filePath);
}

private readDirectory(folderPath: string): Promise<string[]> {
return new Promise((resolve, reject) => {
readdir(folderPath, (error, fileNames) => {
if (error) {
reject(error);
} else {
resolve(fileNames.map((fileName) => join(folderPath, fileName)));
}
});
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ import type { Image } from "@common/Core/Image";

export interface FileImageGenerator {
getImage(filePath: string): Promise<Image>;
clearCache: () => Promise<void>;
}
26 changes: 26 additions & 0 deletions src/main/Core/ImageGenerator/FileImageGenerator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,32 @@ import { FileImageGenerator } from "./FileImageGenerator";
describe(FileImageGenerator, () => {
const cacheFolderPath = "cacheFolderPath";

it("should clear cache folder if it exists", async () => {
const pathExistsMock = vi.fn().mockReturnValue(true);
const clearFolderMock = vi.fn().mockReturnValue(Promise.resolve());

const fileSystemUtility = <FileSystemUtility>{
pathExists: (f) => pathExistsMock(f),
clearFolder: (f) => clearFolderMock(f),
};

const fileImageGenerator = new FileImageGenerator(cacheFolderPath, fileSystemUtility);

await fileImageGenerator.clearCache();
expect(pathExistsMock).toHaveBeenCalledWith(cacheFolderPath);
expect(clearFolderMock).toHaveBeenCalledWith(cacheFolderPath);
});

it("should do nothing if cache folder does not exist", async () => {
const pathExistsMock = vi.fn().mockReturnValue(false);
const fileSystemUtility = <FileSystemUtility>{ pathExists: (f) => pathExistsMock(f) };

const fileImageGenerator = new FileImageGenerator(cacheFolderPath, fileSystemUtility);

await fileImageGenerator.clearCache();
expect(pathExistsMock).toHaveBeenCalledWith(cacheFolderPath);
});

it("should create the cached file if it doesn't exist and return the image", async () => {
const pathExistsMock = vi.fn().mockReturnValue(false);
const writePngMock = vi.fn().mockReturnValue(Promise.resolve());
Expand Down
16 changes: 15 additions & 1 deletion src/main/Core/ImageGenerator/FileImageGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ export class FileImageGenerator implements FileImageGeneratorInterface {
private readonly fileSystemUtility: FileSystemUtility,
) {}

public async clearCache(): Promise<void> {
const exists = await this.fileSystemUtility.pathExists(this.cacheFolderPath);

if (exists) {
await this.fileSystemUtility.clearFolder(this.cacheFolderPath);
}
}

public async getImage(filePath: string): Promise<Image> {
const cachedPngFilePath = await this.ensureCachedPngFileExists(filePath);

Expand All @@ -23,7 +31,13 @@ export class FileImageGenerator implements FileImageGeneratorInterface {
const exists = await this.fileSystemUtility.pathExists(cachedPngFilePath);

if (!exists) {
await this.fileSystemUtility.writePng(getFileIcon(filePath), cachedPngFilePath);
const buffer = getFileIcon(filePath);

if (!buffer.byteLength) {
throw new Error(`getFileIcon returned Buffer with length 0`);
}

await this.fileSystemUtility.writePng(buffer, cachedPngFilePath);
}

return cachedPngFilePath;
Expand Down
29 changes: 25 additions & 4 deletions src/main/Core/Logger/Logger.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { BrowserWindowNotifier } from "@Core/BrowserWindowNotifier";
import type { Clock } from "@Core/Clock/Clock";
import { describe, expect, it, vi } from "vitest";
import { Logger } from "./Logger";
Expand All @@ -7,45 +8,65 @@ describe(Logger, () => {

it("should log an error", () => {
const getCurrentTimeAsStringMock = vi.fn().mockReturnValue(nowAsString);
const notifyMock = vi.fn();

const logger = new Logger(<Clock>{ getCurrentTimeAsString: () => getCurrentTimeAsStringMock() });
const logger = new Logger(
<Clock>{ getCurrentTimeAsString: () => getCurrentTimeAsStringMock() },
<BrowserWindowNotifier>{ notify: (c) => notifyMock(c) },
);

logger.error("This is an error");

expect(logger.getLogs()).toEqual([`[${nowAsString}][ERROR] This is an error`]);
expect(getCurrentTimeAsStringMock).toHaveBeenCalledOnce();
expect(notifyMock).toHaveBeenCalledWith("logsUpdated");
});

it("should log a debug message", () => {
const getCurrentTimeAsStringMock = vi.fn().mockReturnValue(nowAsString);
const notifyMock = vi.fn();

const logger = new Logger(<Clock>{ getCurrentTimeAsString: () => getCurrentTimeAsStringMock() });
const logger = new Logger(
<Clock>{ getCurrentTimeAsString: () => getCurrentTimeAsStringMock() },
<BrowserWindowNotifier>{ notify: (c) => notifyMock(c) },
);

logger.debug("This is a debug message");

expect(logger.getLogs()).toEqual([`[${nowAsString}][DEBUG] This is a debug message`]);
expect(getCurrentTimeAsStringMock).toHaveBeenCalledOnce();
expect(notifyMock).toHaveBeenCalledWith("logsUpdated");
});

it("should log a info message", () => {
const getCurrentTimeAsStringMock = vi.fn().mockReturnValue(nowAsString);
const notifyMock = vi.fn();

const logger = new Logger(<Clock>{ getCurrentTimeAsString: () => getCurrentTimeAsStringMock() });
const logger = new Logger(
<Clock>{ getCurrentTimeAsString: () => getCurrentTimeAsStringMock() },
<BrowserWindowNotifier>{ notify: (c) => notifyMock(c) },
);

logger.info("This is a info message");

expect(logger.getLogs()).toEqual([`[${nowAsString}][INFO] This is a info message`]);
expect(getCurrentTimeAsStringMock).toHaveBeenCalledOnce();
expect(notifyMock).toHaveBeenCalledWith("logsUpdated");
});

it("should log a warning", () => {
const getCurrentTimeAsStringMock = vi.fn().mockReturnValue(nowAsString);
const notifyMock = vi.fn();

const logger = new Logger(<Clock>{ getCurrentTimeAsString: () => getCurrentTimeAsStringMock() });
const logger = new Logger(
<Clock>{ getCurrentTimeAsString: () => getCurrentTimeAsStringMock() },
<BrowserWindowNotifier>{ notify: (c) => notifyMock(c) },
);

logger.warn("This is a warning");

expect(logger.getLogs()).toEqual([`[${nowAsString}][WARNING] This is a warning`]);
expect(getCurrentTimeAsStringMock).toHaveBeenCalledOnce();
expect(notifyMock).toHaveBeenCalledWith("logsUpdated");
});
});
7 changes: 6 additions & 1 deletion src/main/Core/Logger/Logger.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import type { BrowserWindowNotifier } from "@Core/BrowserWindowNotifier";
import type { Clock } from "../Clock";
import type { Logger as LoggerInterface } from "./Contract";

export class Logger implements LoggerInterface {
private logs: string[] = [];

public constructor(private readonly clock: Clock) {}
public constructor(
private readonly clock: Clock,
private readonly browserWindowNotifier: BrowserWindowNotifier,
) {}

public error(message: string): void {
this.writeLog("ERROR", message);
Expand All @@ -28,5 +32,6 @@ export class Logger implements LoggerInterface {

private writeLog(logLevel: string, message: string) {
this.logs.push(`[${this.clock.getCurrentTimeAsString()}][${logLevel}] ${message}`);
this.browserWindowNotifier.notify("logsUpdated");
}
}
3 changes: 1 addition & 2 deletions src/main/Core/Logger/LoggerModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import { Logger } from "./Logger";
export class LoggerModule {
public static bootstrap(dependencyRegistry: DependencyRegistry<Dependencies>) {
const ipcMain = dependencyRegistry.get("IpcMain");
const clock = dependencyRegistry.get("Clock");

const logger = new Logger(clock);
const logger = new Logger(dependencyRegistry.get("Clock"), dependencyRegistry.get("BrowserWindowNotifier"));

dependencyRegistry.register("Logger", logger);
ipcMain.on("getLogs", (event) => (event.returnValue = logger.getLogs()));
Expand Down
12 changes: 9 additions & 3 deletions src/main/Extensions/ApplicationSearch/ApplicationSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class ApplicationSearch implements Extension {
};

public constructor(
private readonly currentOperatingSystem: OperatingSystem,
private readonly operatingSystem: OperatingSystem,
private readonly applicationRepository: ApplicationRepository,
private readonly settings: Settings,
private readonly assetPathResolver: AssetPathResolver,
Expand All @@ -34,7 +34,7 @@ export class ApplicationSearch implements Extension {

public isSupported(): boolean {
const supportedOperatingSystems: OperatingSystem[] = ["Windows", "macOS"];
return supportedOperatingSystems.includes(this.currentOperatingSystem);
return supportedOperatingSystems.includes(this.operatingSystem);
}

public getSettingDefaultValue<T>(key: string): T {
Expand All @@ -51,8 +51,14 @@ export class ApplicationSearch implements Extension {
}

public getImage(): Image {
const fileNames: Record<OperatingSystem, string> = {
Linux: null, // not supported,
macOS: "macos-applications.png",
Windows: "windows-applications.png",
};

return {
url: `file://${this.assetPathResolver.getExtensionAssetPath(this.id, "macos-applications.png")}`,
url: `file://${this.assetPathResolver.getExtensionAssetPath(this.id, fileNames[this.operatingSystem])}`,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ export class ApplicationSearchModule implements ExtensionModule {
dependencyRegistry.get("FileImageGenerator"),
dependencyRegistry.get("Logger"),
settings,
dependencyRegistry.get("AssetPathResolver"),
),
Windows: new WindowsApplicationRepository(
dependencyRegistry.get("PowershellUtility"),
settings,
dependencyRegistry.get("FileImageGenerator"),
dependencyRegistry.get("Logger"),
dependencyRegistry.get("AssetPathResolver"),
),
Linux: undefined, // not supported
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { AssetPathResolver } from "@Core/AssetPathResolver";
import type { FileImageGenerator } from "@Core/ImageGenerator";
import type { Logger } from "@Core/Logger";
import type { PowershellUtility } from "@Core/PowershellUtility";
Expand All @@ -15,6 +16,7 @@ export class WindowsApplicationRepository implements ApplicationRepository {
private readonly settings: Settings,
private readonly fileImageGenerator: FileImageGenerator,
private readonly logger: Logger,
private readonly assetPathResolver: AssetPathResolver,
) {}

public async getApplications(): Promise<Application[]> {
Expand All @@ -40,7 +42,14 @@ export class WindowsApplicationRepository implements ApplicationRepository {
const appIcons = await this.getAppIcons(windowsApplicationRetrieverResults.map(({ FullName }) => FullName));

return windowsApplicationRetrieverResults.map(
({ BaseName, FullName }) => new Application(BaseName, FullName, appIcons[FullName]),
({ BaseName, FullName }) =>
new Application(
BaseName,
FullName,
appIcons[FullName] ?? {
url: `file://${this.assetPathResolver.getExtensionAssetPath("ApplicationSearch", "windows-generic-app-icon.png")}`,
},
),
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { AssetPathResolver } from "@Core/AssetPathResolver";
import type { CommandlineUtility } from "@Core/CommandlineUtility";
import type { FileImageGenerator } from "@Core/ImageGenerator";
import type { Logger } from "@Core/Logger";
Expand All @@ -13,6 +14,7 @@ export class MacOsApplicationRepository implements ApplicationRepository {
private readonly fileImageGenerator: FileImageGenerator,
private readonly logger: Logger,
private readonly settings: Settings,
private readonly assetPathResolver: AssetPathResolver,
) {}

public async getApplications(): Promise<Application[]> {
Expand All @@ -21,7 +23,16 @@ export class MacOsApplicationRepository implements ApplicationRepository {

return filePaths
.filter((filePath) => !!icons[filePath])
.map((filePath) => new Application(parse(filePath).name, filePath, icons[filePath]));
.map(
(filePath) =>
new Application(
parse(filePath).name,
filePath,
icons[filePath] ?? {
url: `file://${this.assetPathResolver.getExtensionAssetPath("ApplicationSearch", "macos-generic-app-icon.png")}`,
},
),
);
}

private async getAllFilePaths(): Promise<string[]> {
Expand Down
6 changes: 3 additions & 3 deletions src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ import * as Extensions from "./Extensions";
dependencyRegistry.register("SafeStorage", Electron.safeStorage);

// Core Modules
Core.EventEmitterModule.bootstrap(dependencyRegistry);
Core.EventSubscriberModule.bootstrap(dependencyRegistry);
Core.RandomStringProviderModule.bootstrap(dependencyRegistry);
Core.SafeStorageEncryptionModule.bootstrap(dependencyRegistry);
Core.AssetPathResolverModule.bootstrap(dependencyRegistry);
Core.ShellModule.bootstrap(dependencyRegistry);
Core.ClipboardModule.bootstrap(dependencyRegistry);
Core.ClockModule.bootstrap(dependencyRegistry);
Core.AboutUeliModule.bootstrap(dependencyRegistry);
Core.LoggerModule.bootstrap(dependencyRegistry);
Core.EventEmitterModule.bootstrap(dependencyRegistry);
Core.EventSubscriberModule.bootstrap(dependencyRegistry);
Core.BrowserWindowNotifierModule.bootstrap(dependencyRegistry);
Core.LoggerModule.bootstrap(dependencyRegistry);
Core.CommandlineUtilityModule.bootstrap(dependencyRegistry);
Core.FileSystemUtilityModule.bootstrap(dependencyRegistry);
await Core.PowershellUtilityModule.bootstrap(dependencyRegistry);
Expand Down
1 change: 1 addition & 0 deletions src/preload/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const contextBridgeImplementation: ContextBridge = {
on: (channel, listener) => ipcRenderer.on(channel, listener),
},

resetChache: () => ipcRenderer.invoke("resetCache"),
copyTextToClipboard: (textToCopy) => ipcRenderer.send("copyTextToClipboard", { textToCopy }),
extensionDisabled: (extensionId) => ipcRenderer.send("extensionDisabled", { extensionId }),
extensionEnabled: (extensionId) => ipcRenderer.send("extensionEnabled", { extensionId }),
Expand Down
Loading

0 comments on commit db4cde8

Please sign in to comment.