Allow custom id in create* endpoints (Typegoose + Mongoose)

[ruudvanbuul]

Allow custom id in create* endpoints (Typegoose + Mongoose)

[ruudvanbuul]

For the imports I do use mongo driver directly with bulk write operations. But documents can also be added later through the graphql api (1 at a time, but with custom id). I know my use case is pretty specific, but nestjs-query is such a nice tool that I like to use it as much as possible.

I think manually enabling custom ids should be enough of a safe guard to keep less experienced devs from going the wrong path, that means they have to enable it in nestjs-query and in their schema.

I wil just create a PR and then @TriPSs can decide what to do with it.

[TriPSs]

Is this something related to this library? I feel like this should be a ticket for Mongoose or Typegoose?

[ruudvanbuul]

Yes it is related to this library

It is code specifically in the createOne and createMany functions of the typegoose-query.service.ts and mongoose-query.service.ts. ensureIdIsNotPresent checks if an id is set. This does not happen in typeorm

[TriPSs]

Typeorm also does that but there the metadata of the entity is available so it can check the correct field. See the following in function (on mobile can't share other info)

private async ensureEntityDoesNotExist(e: Entity):

I have never used either of the other two libs so I don't know if that metadata is available or even possible.

[ruudvanbuul]

Typeorm does not check if you have given an id. It only checks that when you have given an id, it does not exist yet. So the functionality is different. I think it would be better to have the same functionality and thus allow giving an id in mongoose/typegoose.

[ruudvanbuul]

In mongo the id field is always _id. So you can just check if that field exists.

In the ensureEntityDoesNotExist function of typeorm it then does a findOne to check if the id already exists. You would do the exact same for mongo.

I am not sure why this was implemented this way. At the moment you can never insert a document with an _id field. So there is no way to create a document with a custom id value in query service.

[TriPSs]

I do not know why this was done, could it be that the Typegoos/Mongoose libs do not support this?

[ruudvanbuul]

No it is fully supported. When I comment out the ensureIdIsNotPresent line everything works fine.

I loaded all my data through custom bulk writes so didn't notice this before, but now I had a use case to use this function and it became a problem.

Really not sure why this is added, especially because it differs from the typeorm functionality. Although I did notice that the original creator of nestjs did not have much experience with mongo. Because the queries are also very inefficient and .lean isn't used (I'll make PRs for this later or create a separate mongo implementation library)

[smolinari]

The lack of supporting custom IDs is probably done because it's a bad idea to make your own ids with MongoDB. Especially sequenced ids. MongoDB ObjectIds were specifically engineered for the purpose of big data storage. If you mess with MongoDB's id setup, you will more than likely experience issues. My advice to anyone is, don't mess with them. There is absolutely no reason to change them.

Scott

[ruudvanbuul]

As stated here _id field can be lots of data types. ObjectID is just the default. In my use case my data has generated unique ids, which is a valid use case even outlined in the docs. Without going into too much detail, this saves me index space and a lot of id lookups when syncing large amounts of data daily.

So completely blocking custom ids seems like a weird functionality to me if the mongo docs say this is a valid use case. Also if ensureIdIsNotPresent is skipped and no id is present it will still create an ObjectID. Now I can not use create functionality in nestjs-query at all.

If making things fool proof is the main consideration, introducing a module setting to enable customIds can fix this (disabled by default). I'm happy to make a PR for that.

[smolinari]

If you are storing UUIDs, then they should be fine for big data.

I can only guess the check to enforce a missing id was built in because it's just smarter and easier to use the default ObjectId and I'd also venture to say, nestjs-query wasn't designed with the ingestion of data from an external system using its own ids in the Mongo id field in mind.

In fact, I'd personally avoid using nestjs-query to ingest data from an external system. I'd use a specialized endpoint just for the ingestion and synchronization and I'd use the Node driver directly (or even use a faster language like Rust or Go). It would perform much, much better.

And, knowing that you only want to store the external system's id, that tells me the creation of the data should only be done in that master system. Granted, UUIDs can be created anywhere, but then you'd need bi-directional sync'ing and well, that is asking for trouble. So, if you ingest and sync the data outside of the application (the one using nestjs-query), and you have to create the data in the master system, you don't necessarily need any creation methods in nestjs-query. Granted, I'm making a lot of assumptions of your use case here.

That all being said, I'm not the owner of this package. Just a user myself, who also did some work on the Typegoose package. 😁

And, I also don't see any proper technical reason to stop the usage of a self-generated ids. To me, it's just a way to avoid the (less experienced) devland dev from creating his own footgun. 😀 🤷

Scott