Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NodeFSStorageAdapter: refactor #215

Merged
merged 1 commit into from
Nov 3, 2023
Merged

NodeFSStorageAdapter: refactor #215

merged 1 commit into from
Nov 3, 2023

Conversation

HerbCaudill
Copy link
Collaborator

@HerbCaudill HerbCaudill commented Oct 31, 2023

No changes in behavior, just some minor refactoring for clarity.

})
)

return result
}

async removeRange(keyPrefix: string[]): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should refuse to run this without a key value; pretty sure someone accidentally passing in undefined / empty array would delete everything they have

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok - that should be a separate PR

@pvh
Copy link
Member

pvh commented Oct 31, 2023

Okay! Some description would be nice for this one.

export class NodeFSStorageAdapter extends StorageAdapter {
private baseDirectory: string
private cache: {
[key: string]: { storageKey: StorageKey; data: Uint8Array }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no reason to store the storageKey in the cache as both a value and a key.

} else {
throw error
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic has been moved to walkdir


// The "keys" in the cache don't include the baseDirectory.
// We want to de-dupe with the cached keys so we'll use getKey to normalize them.
const diskKeys: string[] = diskFiles
.flat(Infinity) // the walk function returns a nested array
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

walkdir returns a flattened array

await rimraf(dirPath)
}

private getKey(key: StorageKey): string {
return path.join(...key)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getKey and cacheKey do the same thing

@HerbCaudill HerbCaudill marked this pull request as ready for review November 1, 2023 11:39
@pvh pvh merged commit 0a1a1d7 into main Nov 3, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants