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

Storage adapter for MongoDB #386

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kraenhansen
Copy link
Contributor

Implements the StorageAdapterInterface targeting MongoDB.

The constructor is overloaded to take either a MongoDBClient instance or a MongoDB server url.

Key storage and query strategy

Chunk keys are stored and queried as arrays of strings and I've prepared the adapter to take an option to change this to storing keys as strings (joined by some separator). I believe using array of strings is "good enough" and the simplest of the two, since the range query would have to be implemented using the $regex operator which would require sanitizing the key segments (and making assumptions on valid characters) before injecting them into the regular expression.

Why is it draft?

This contains changes from bb48906 which should not be considered when reviewing this PR and as such, this branch needs a rebase onto main once #385 gets merged 🤞

@alexjg
Copy link
Contributor

alexjg commented Sep 9, 2024

This looks like a solid implementation and I agree with your design decisions. We will be slightly changing the storage interface in future to explicitly disallow a separator character from the keys (probably "/") which would make it possible to use a regex query I think, though I think your concerns about sanitizing the input remain.

However, I would prefer not to add more storage adapters to this repository. In my view this repository should contain only the most basic and universal storage adapters (i.e. the file system and indexeddb ones) and anything else should live in other repositories maintained by their users. This reduces maintenance burden on the core repository and also allows storage adapters to adapt to the specific needs of the people using them. The storage interface is deliberately designed to be simple with very few requirements to make this mode of use easier.

I hope you don't see this as a snub, this is great work and I think lots of people would benefit from it if you are willing to maintain it in another repository!

One thing I think we should do is add links to community maintained storage adapters in the README.


let client: MongoClient

describe.skipIf(!hasDocker())("MongoDBStorageAdapter", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skipping tests if docker is not available on the machine.

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