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

Support ObjectId in documents. #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support ObjectId in documents. #38

wants to merge 1 commit into from

Conversation

soldag
Copy link

@soldag soldag commented Mar 28, 2017

The MongoStore can now be configured to store ids as ObjectId instead of binary UUID representation.

@ju-Skinner
Copy link

@soldag did you ever get this working? I am running into an issue where I am populating data using mongoimport; and it isn't returning any data due to the _id

@pmcnr-hx
Copy link
Contributor

Thank you for your contribution @soldag. I was looking at your PR in order to merge it, but then I remembered why we didn't use ObjectId in the first place: ObjectId only has 12 bytes whereas a UUID will have 16 bytes. Mapping UUID's to ObjectId we're risking collisions and we won't be able to retrieve the original UUID if necessary, which means we can't interoperate with other data stores.

Are you using this patch successfully in production?

I realise that ObjectId performs considerably better than mongodb.Binary.SUBTYPE_UUID and we need to take this into account. I think the solution for this will be for jsonapi-server to support any IDs and not just RFC 4122 UUIDs, which will require changes in jsonapi-server but will allow implementations to optimise for a given data store.

Thoughts on this @soldag and @ju-Skinner?

@ju-Skinner
Copy link

@pmcnr-hx I agree with your statement about jsonapi-server supporting any ID's; but the biggest issue is not just the support, but the id field in which it needs. In all other data stores there is a default of id; whereas mongo is using _id. That minor change is what is killing most. While this plugin provides some form of a resolution, but it doesn't account for those that are importing data via CLI.

I have come up with a makeshift way to import data manually and copying the _id to the id field. But now I have duplicated data in each document. This is ok for me now as my data set is really small, but in the future, this would not be feasible.

Another thing to note about the jsonapi-server approach, is that it defines and requires. It would be nice to have a way to override the default requirement of id and be able to use _id.

@pmcnr-hx
Copy link
Contributor

@ju-Skinner: so if jsonapi-store-mongodb didn't overwrite the _id property and simply left it to be automatically generated by MongoDB (or some other tool manipulating the store) and only used id, would that solve your problem?

@ju-Skinner
Copy link

ju-Skinner commented May 23, 2017

@pmcnr-hx No, because it doesn't overwrite anything that I import via the CLI. Which is why this PR doesn't really address my issue exactly. I learned this as I dug deeper into both this plugin and jsonapi-server itself. This plugin works as expected when you are doing things via jsonapi-server request. It generates the document with the necessary id's needed for jsonapi-server to render the response.

What this plugin and jsonapi-server don't address is existing data that uses just the core implementation of _id as an ObjectId. It fails for two reasons. 1.) They both are depending on a key (field) named id, without that field present you will not get any data returned. 2.) They don't work will with the ObjectId in its raw state. @soldag is attempting to address that concern here, by introducing an option that will need to be passed to the MongoStore objectid; but it still doesn't account for existing data in the DocumentStore.

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.

3 participants