Skip to content

Commit

Permalink
chore: Add locking on reads for storage file (#303)
Browse files Browse the repository at this point in the history
  • Loading branch information
scalvert authored Dec 2, 2021
1 parent 7aff8b8 commit 9d37795
Showing 1 changed file with 37 additions and 16 deletions.
53 changes: 37 additions & 16 deletions src/io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ export function writeTodos(

try {
const existing: Map<FilePath, TodoMatcher> = options.filePath
? readTodosForFilePath(baseDir, options.filePath)
: readTodos(baseDir);
? readTodosForFilePath(baseDir, options.filePath, false)
: readTodos(baseDir, false);
batches = getTodoBatches(maybeTodos, existing, options);

applyTodoChanges(baseDir, batches.add, batches.remove, false);
Expand All @@ -154,17 +154,26 @@ export function writeTodos(
};
}

// TODO: change from using TodoFilePathHash to just FilePath, but do that once this is all working
/**
* Reads all todo files in the .lint-todo directory.
*
* @param baseDir - The base directory that contains the .lint-todo storage directory.
* @returns - A {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map|Map} of {@link https://github.com/ember-template-lint/ember-template-lint-todo-utils/blob/master/src/types/todo.ts#L25|FilePath}/{@link https://github.com/ember-template-lint/ember-template-lint-todo-utils/blob/master/src/todo-matcher.ts#L4|TodoMatcher}.
*/
export function readTodos(baseDir: string): Map<FilePath, TodoMatcher> {
const todoOperations = readTodoStorageFile(getTodoStorageFilePath(baseDir));
export function readTodos(baseDir: string, shouldLock = true): Map<FilePath, TodoMatcher> {
const release =
shouldLock && todoStorageFileExists(baseDir)
? tryLockStorageFile(baseDir)
: // eslint-disable-next-line @typescript-eslint/no-empty-function
() => {};

try {
const todoOperations = readTodoStorageFile(getTodoStorageFilePath(baseDir));

return buildFromTodoOperations(todoOperations);
return buildFromTodoOperations(todoOperations);
} finally {
release();
}
}

/**
Expand All @@ -176,15 +185,14 @@ export function readTodos(baseDir: string): Map<FilePath, TodoMatcher> {
*/
export function readTodosForFilePath(
baseDir: string,
filePath: string
filePath: string,
shouldLock = true
): Map<FilePath, TodoMatcher> {
const todoOperations = readFileSync(getTodoStorageFilePath(baseDir), {
encoding: 'utf-8',
}).split(EOL);
const existingTodos = readTodos(baseDir, shouldLock);

return buildFromTodoOperations(
todoOperations.filter((operation) => operation.endsWith(filePath))
);
const matcher = existingTodos.get(filePath) || new TodoMatcher();

return new Map([[filePath, matcher]]);
}

/**
Expand Down Expand Up @@ -272,20 +280,33 @@ export const EXCLUDE_EXPIRED = (operation: Operation): boolean => {
*
* @param baseDir - The base directory that contains the .lint-todo storage directory.
* @param compactStrategy - The strategy to use when compacting the storage file. Default: ADD_OPERATIONS_ONLY
* @returns The count of compacted todos.
*/
export function compactTodoStorageFile(
baseDir: string,
compactStrategy: (operation: Operation) => boolean = ADD_OPERATIONS_ONLY
): void {
): {
originalOperations: Operation[];
compactedOperations: Operation[];
compacted: number;
} {
const todoStorageFilePath = getTodoStorageFilePath(baseDir);
const release = tryLockStorageFile(baseDir);

try {
const operations = readTodoStorageFile(todoStorageFilePath).filter((operation) =>
const originalOperations = readTodoStorageFile(todoStorageFilePath);
const originalCount = originalOperations.length;
const compactedOperations = originalOperations.filter((operation) =>
compactStrategy(operation)
);

writeTodoStorageFile(todoStorageFilePath, operations);
writeTodoStorageFile(todoStorageFilePath, compactedOperations);

return {
originalOperations,
compactedOperations,
compacted: originalCount - compactedOperations.length,
};
} finally {
release();
}
Expand Down

0 comments on commit 9d37795

Please sign in to comment.