Document id forced to ObjectId

[ingemaradahl]

Hi!

It seems that mongo4cats imposes the limitation that stored document must have their _id field be of the ObjectId type: https://github.com/Kirill5k/mongo4cats/blob/2ddd402b23671be2d9b61136540953763c7b827a/modules/kernel/src/main/scala/mongo4cats/codecs/ContainerValueWriter.scala#L34.

Where I work, we're trying to incorporate more of the Typelevel ecosystem (cats, CE...), but we're explicitly not using auto generated values for the _id field, or ObjectId instances in any way. What's the reasoning behind mongo4cats requiring the ObjectId type, or did it just end up being that way?

I could submit a PR to change this, but first I just want to raise the issue before doing the work, in case the limitation is intentional and the PR would never be accepted..

[Kirill5k]

Hey, @ingemaradahl

Don't think there is any specific reasoning behind this decision.

Feel free to submit a PR for that change 👍

[Kirill5k]

@ingemaradahl

I've updated ContainerValueWriter to be able to handle ids of different types: a9f223c

Will release it later today

[ingemaradahl]

That was quick, thanks! I did an identical change, but I'm worried that https://github.com/Kirill5k/mongo4cats/blob/99b2c3699cab4cb1ff6d0ded5621f4e8c5626903/modules/kernel/src/main/scala/mongo4cats/codecs/DocumentCodecProvider.scala#L46 needs to change as well? I don't know how the underlying MongoDB driver calls this method, but I suspect that it shouldn't throw just because the document id isn't an ObjectId. The type signature indicates that the method should just convert the mongo4cats.bson.BsonValue to org.bson.BsonValue.

[Kirill5k]

Good spot! I totally forgot about that one.

I'll make an update 👍

[Kirill5k]

I've pushed a change: 35a832d

Should be available in 0.6.15