Skip to content

Commit

Permalink
refactor(CommandlineUtility): added maxBuffer option (#1247)
Browse files Browse the repository at this point in the history
* refactor: wrap arguments into one "options" argument

* Added maxbuffer option
  • Loading branch information
oliverschwendener authored Nov 11, 2024
1 parent df6ae8f commit 80df378
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 51 deletions.
14 changes: 11 additions & 3 deletions src/main/Core/CommandlineUtility/Contract/CommandlineUtility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,17 @@ export interface CommandlineUtility {
/**
* Executes a command and returns the output.
* @param command The command to execute, e.g. `"echo Hello World"`.
* @param ignoreErr If `true`, the error is ignored and not thrown as an error.
* @param ignoreStdErr If `true`, the standard error output is ignored and not thrown as an error.
* @param options.ignoreErr If `true`, the error is ignored and not thrown as an error.
* @param options.ignoreStdErr If `true`, the standard error output is ignored and not thrown as an error.
* @param options.maxBuffer The maximum buffer size in bytes allowed for the standard output and standard error streams.
* @returns A promise that resolves with the output of the command.
*/
executeCommand(command: string, ignoreStdErr?: boolean, ignoreErr?: boolean): Promise<string>;
executeCommand(
command: string,
options?: {
ignoreStdErr?: boolean;
ignoreErr?: boolean;
maxBuffer?: number;
},
): Promise<string>;
}
59 changes: 46 additions & 13 deletions src/main/Core/CommandlineUtility/NodeJsCommandlineUtility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,16 @@ vi.mock("child_process", async (importOriginal) => {

return {
...original,
exec: (command: string, callback: (_: Error | null, __: string, ___?: string) => void) => {
execMock(command);
callback(error, stdout, stderr);
exec: (
command: string,
options?: { maxBuffer: number },
callback?: (_: Error | null, __: string, ___?: string) => void,
) => {
execMock(command, options);

if (callback) {
callback(error, stdout, stderr);
}
},
};
});
Expand All @@ -32,32 +39,41 @@ describe(NodeJsCommandlineUtility, () => {
stdout = "output";
stderr = undefined;

const output = await new NodeJsCommandlineUtility().executeCommand("test", false, false);
const output = await new NodeJsCommandlineUtility().executeCommand("test", {
ignoreStdErr: false,
ignoreErr: false,
});

expect(output).toBe("output");
expect(execMock).toHaveBeenCalledWith("test");
expect(execMock).toHaveBeenCalledWith("test", { maxBuffer: undefined });
});

it("should execute the command and return the stdout if error occurs but ignore error is set to true", async () => {
error = new Error("This is a test");
stdout = "output";
stderr = undefined;

const output = await new NodeJsCommandlineUtility().executeCommand("test", false, true);
const output = await new NodeJsCommandlineUtility().executeCommand("test", {
ignoreStdErr: false,
ignoreErr: true,
});

expect(output).toBe("output");
expect(execMock).toHaveBeenCalledWith("test");
expect(execMock).toHaveBeenCalledWith("test", { maxBuffer: undefined });
});

it("should execute the command and return the stdout if stderr occurs but ignore stderr is set to true", async () => {
error = null;
stdout = "output";
stderr = undefined;

const output = await new NodeJsCommandlineUtility().executeCommand("test", true, false);
const output = await new NodeJsCommandlineUtility().executeCommand("test", {
ignoreStdErr: true,
ignoreErr: false,
});

expect(output).toBe("output");
expect(execMock).toHaveBeenCalledWith("test");
expect(execMock).toHaveBeenCalledWith("test", { maxBuffer: undefined });
});

it("should reject the promise if an error occurs and ignore error is set to false", async () => {
Expand All @@ -66,10 +82,14 @@ describe(NodeJsCommandlineUtility, () => {
stderr = undefined;

expect(
async () => await new NodeJsCommandlineUtility().executeCommand("test", false, false),
async () =>
await new NodeJsCommandlineUtility().executeCommand("test", {
ignoreStdErr: false,
ignoreErr: false,
}),
).rejects.toThrow(error.message);

expect(execMock).toHaveBeenCalledWith("test");
expect(execMock).toHaveBeenCalledWith("test", { maxBuffer: undefined });
});

it("should reject the promise if a std error occurs and ignore std error is set to false", async () => {
Expand All @@ -78,10 +98,23 @@ describe(NodeJsCommandlineUtility, () => {
stderr = "This is a std err";

expect(
async () => await new NodeJsCommandlineUtility().executeCommand("test", false, false),
async () =>
await new NodeJsCommandlineUtility().executeCommand("test", {
ignoreStdErr: false,
ignoreErr: false,
}),
).rejects.toThrow("This is a std err");

expect(execMock).toHaveBeenCalledWith("test");
expect(execMock).toHaveBeenCalledWith("test", { maxBuffer: undefined });
});

it("should respect the max buffer size", async () => {
error = null;
stdout = "output";
stderr = "";

await new NodeJsCommandlineUtility().executeCommand("test", { maxBuffer: 100 });
expect(execMock).toHaveBeenCalledWith("test", { maxBuffer: 100 });
});
});
});
15 changes: 11 additions & 4 deletions src/main/Core/CommandlineUtility/NodeJsCommandlineUtility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,19 @@ import { exec } from "child_process";
import type { CommandlineUtility } from "./Contract";

export class NodeJsCommandlineUtility implements CommandlineUtility {
public executeCommand(command: string, ignoreStdErr?: boolean, ignoreErr?: boolean): Promise<string> {
public executeCommand(
command: string,
options?: {
ignoreStdErr?: boolean;
ignoreErr?: boolean;
maxBuffer?: number;
},
): Promise<string> {
return new Promise((resolve, reject) => {
exec(command, (error, stdout, stderr) => {
if (error && !ignoreErr) {
exec(command, { maxBuffer: options?.maxBuffer }, (error, stdout, stderr) => {
if (error && !options?.ignoreErr) {
reject(error);
} else if (stderr && !ignoreStdErr) {
} else if (stderr && !options?.ignoreStdErr) {
reject(stderr);
} else {
resolve(stdout.toString());
Expand Down
6 changes: 4 additions & 2 deletions src/main/Core/PowershellUtility/Contract/PowershellUtility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ export interface PowershellUtility {
/**
* Executes a PowerShell command.
* @param command The command to execute, e.g. `Get-Process`.
* @param options.maxBuffer The maximum buffer size in bytes allowed for the standard output and standard error streams.
* @returns The output of the powershell command.
*/
executeCommand(command: string): Promise<string>;
executeCommand(command: string, options?: { maxBuffer?: number }): Promise<string>;

/**
* Executes a PowerShell script. The script can also be multi-line and include functions.
Expand All @@ -17,7 +18,8 @@ export interface PowershellUtility {
* Write-Host "This is a PowerShell script."
* Write-Host "It is executed by the PowerShellUtility."
* ```
* @param options.maxBuffer The maximum buffer size in bytes allowed for the standard output and standard error streams.
* @returns The output of the powershell script.
*/
executeScript(script: string): Promise<string>;
executeScript(script: string, options?: { maxBuffer?: number }): Promise<string>;
}
32 changes: 26 additions & 6 deletions src/main/Core/PowershellUtility/PowershellUtility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { describe, expect, it, vi } from "vitest";
import { PowershellUtility } from "./PowershellUtility";

describe(PowershellUtility, () => {
it("should call commandline utility to execute powershell command", async () => {
const testExecuteCommand = async ({ command, options }: { command: string; options?: { maxBuffer: number } }) => {
const executeCommandMock = vi.fn().mockReturnValue("test output");

const commandlineUtility = <CommandlineUtility>{
Expand All @@ -20,13 +20,21 @@ describe(PowershellUtility, () => {
<RandomStringProvider>{},
);

expect(await powershellUtility.executeCommand("test command")).toBe("test output");
expect(await powershellUtility.executeCommand(command, options)).toBe("test output");
expect(executeCommandMock).toHaveBeenCalledWith(
`${PowershellUtility.PowershellPath} -Command "& {test command}"`,
`${PowershellUtility.PowershellPath} -Command "& {${command}}"`,
);
};

it("should call commandline utility to execute powershell command", async () => {
await testExecuteCommand({ command: "test command" });
});

it("should write a temporary file, execute it and remove the file again", async () => {
it("should call commandline utility to execute powershell command with maxBuffer", async () => {
await testExecuteCommand({ command: "test command", options: { maxBuffer: 100 } });
});

const testExecuteScript = async ({ script, options }: { script: string; options?: { maxBuffer: number } }) => {
const writeTextFileMock = vi.fn().mockReturnValue(Promise.resolve());
const removeFileMock = vi.fn().mockReturnValue(Promise.resolve());
const executeCommandMock = vi.fn().mockReturnValue("test output");
Expand All @@ -37,7 +45,7 @@ describe(PowershellUtility, () => {
removeFile: (filePath) => removeFileMock(filePath),
};

const commandlineUtility = <CommandlineUtility>{ executeCommand: (command) => executeCommandMock(command) };
const commandlineUtility = <CommandlineUtility>{ executeCommand: (c, o) => executeCommandMock(c, o) };
const randomStringProvider = <RandomStringProvider>{ getRandomUUid: () => getRandomHexStringMock() };

const powershellUtility = new PowershellUtility(
Expand All @@ -47,14 +55,26 @@ describe(PowershellUtility, () => {
randomStringProvider,
);

expect(await powershellUtility.executeScript("my script")).toBe("test output");
expect(await powershellUtility.executeScript(script, options)).toBe("test output");

expect(writeTextFileMock).toHaveBeenCalledWith(
"\ufeffmy script",
join("temp", "directory", "randomHexString.ps1"),
);

expect(executeCommandMock).toHaveBeenCalledWith(
`${PowershellUtility.PowershellPath} -NoProfile -NonInteractive -ExecutionPolicy bypass -File "${join("temp", "directory", "randomHexString.ps1")}"`,
options,
);

expect(getRandomHexStringMock).toHaveBeenCalledOnce();
};

it("should write a temporary file, execute it and remove the file again", async () => {
await testExecuteScript({ script: "my script" });
});

it("should write a temporary file, execute it and remove the file again, with maxBuffer", async () => {
await testExecuteScript({ script: "my script", options: { maxBuffer: 100 } });
});
});
25 changes: 11 additions & 14 deletions src/main/Core/PowershellUtility/PowershellUtility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,25 @@ export class PowershellUtility implements PowershellUtilityInterface {
private readonly randomStringProvider: RandomStringProvider,
) {}

public async executeCommand(command: string): Promise<string> {
return this.commandlineUtility.executeCommand(`${PowershellUtility.PowershellPath} -Command "& {${command}}"`);
public async executeCommand(command: string, options?: { maxBuffer: number }): Promise<string> {
return await this.commandlineUtility.executeCommand(
`${PowershellUtility.PowershellPath} -Command "& {${command}}"`,
options,
);
}

public async executeScript(script: string): Promise<string> {
const filePath = this.getTemporaryPowershellFilePath();
public async executeScript(script: string, options?: { maxBuffer: number }): Promise<string> {
const filePath = join(this.temporaryDirectoryFilePath, `${this.randomStringProvider.getRandomUUid()}.ps1`);

await this.fileSystemUtility.writeTextFile(`${this.byteOrderMark}${script}`, filePath);

const powershellCommand = `${PowershellUtility.PowershellPath} -NoProfile -NonInteractive -ExecutionPolicy bypass -File "${filePath}"`;
const stdout = await this.commandlineUtility.executeCommand(powershellCommand);
const stdout = await this.commandlineUtility.executeCommand(
`${PowershellUtility.PowershellPath} -NoProfile -NonInteractive -ExecutionPolicy bypass -File "${filePath}"`,
options,
);

await this.fileSystemUtility.removeFile(filePath);

return stdout;
}

private getTemporaryPowershellFilePath(): string {
return join(this.temporaryDirectoryFilePath, this.getRandomFileName());
}

private getRandomFileName(): string {
return `${this.randomStringProvider.getRandomUUid()}.ps1`;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ export class LaunchDesktopFileActionHandler implements ActionHandler {
* Throws an error if the file could not be launched or desktop environment isn't supported.
*/
public async invokeAction(action: SearchResultItemAction): Promise<void> {
await this.commandlineUtility.executeCommand(`gio launch ${action.argument}`, true);
await this.commandlineUtility.executeCommand(`gio launch ${action.argument}`, { ignoreStdErr: true });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import type { WindowsStoreApplication } from "./WindowsStoreApplication";
import { usePowershellScripts } from "./usePowershellScripts";

export class WindowsApplicationRepository implements ApplicationRepository {
private static readonly POWERSHELL_MAX_BUFFER_SIZE = 4096 * 4096;

public constructor(
private readonly powershellUtility: PowershellUtility,
private readonly settings: Settings,
Expand All @@ -37,6 +39,7 @@ export class WindowsApplicationRepository implements ApplicationRepository {

const stdout = await this.powershellUtility.executeScript(
this.getPowershellScript(folderPaths, fileExtensions),
{ maxBuffer: WindowsApplicationRepository.POWERSHELL_MAX_BUFFER_SIZE },
);

const windowsApplicationRetrieverResults = <WindowsApplicationRetrieverResult[]>JSON.parse(stdout);
Expand Down Expand Up @@ -64,7 +67,9 @@ export class WindowsApplicationRepository implements ApplicationRepository {

const { getWindowsStoreApps } = usePowershellScripts();

const stdout = await this.powershellUtility.executeScript(getWindowsStoreApps);
const stdout = await this.powershellUtility.executeScript(getWindowsStoreApps, {
maxBuffer: WindowsApplicationRepository.POWERSHELL_MAX_BUFFER_SIZE,
});

const windowStoreApplications = <WindowsStoreApplication[]>JSON.parse(stdout);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe(MacOsApplicationRepository, () => {
`);

const commandlineUtility = <CommandlineUtility>{
executeCommand: (c, istd, ierr) => executeCommandMock(c, istd, ierr),
executeCommand: (c, options) => executeCommandMock(c, options),
};

const getImagesMock = vi.fn().mockResolvedValue({
Expand Down Expand Up @@ -78,11 +78,7 @@ describe(MacOsApplicationRepository, () => {
new MacOsApplication("Finder", "/Applications/Finder.app", { url: "file://genericIcon.png" }),
]);

expect(executeCommandMock).toHaveBeenCalledWith(
`mdfind "kMDItemKind == 'Application'"`,
undefined,
undefined,
);
expect(executeCommandMock).toHaveBeenCalledWith(`mdfind "kMDItemKind == 'Application'"`, undefined);

expect(getImagesMock).toHaveBeenCalledWith([
"/Applications/iTerm.app",
Expand Down
2 changes: 1 addition & 1 deletion src/main/Extensions/FileSearch/macOS/MdfindFileSearcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export class MdfindFileSearcher implements FileSearcher {

public async getFilePathsBySearchTerm(searchTerm: string, maxSearchResultCount: number): Promise<string[]> {
const commands = [`mdfind -name "${searchTerm}"`, `head -n ${maxSearchResultCount}`];
const stdout = await this.commandlineUtility.executeCommand(commands.join(" | "), true);
const stdout = await this.commandlineUtility.executeCommand(commands.join(" | "), { ignoreStdErr: true });
return stdout.split("\n").filter((filePath) => filePath.trim().length > 0);
}
}

0 comments on commit 80df378

Please sign in to comment.