mongodb-odm icon indicating copy to clipboard operation
mongodb-odm copied to clipboard

Allow typed collection mapping

Open petr-buchin opened this issue 5 years ago • 10 comments
trafficstars

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

Do you have plans on allowing typed collections mapping? I would like to have something like

/**
 * @MongoDB\Field(type="collection", itemType="MyCustomType")
 */
private $myCollection;

petr-buchin avatar Dec 14 '19 17:12 petr-buchin

What's the use case? The collection field can already hold scalars and for objects there's an @EmbedMany?

malarzm avatar Dec 14 '19 21:12 malarzm

@malarzm for example I have my custom mapping type, let's say it's Username class, which is an object in my PHP code, and should be represented as a string in MongoDB.

And for example I want to have a field usernames, which should be an array of Username instances on PHP level, and should be stored as a list of strings in a MongoDB.

How do I implement that, without implementing some mapping type like UsernamesCollection?

petr-buchin avatar Dec 14 '19 22:12 petr-buchin

Indeed it might make sense 👍 IIRC we wanted to keep collection and hash a simple arrays but maybe we should allow setting an inner type. Would you like to take a shot at implementing this?

malarzm avatar Dec 14 '19 23:12 malarzm

This behavior can however be easily achieved by using PostLoad event for loading and filling up the collection/array with object instances, and PreFlush for saving them back to database. I'm not sure such a use case warrants creating a special case for it in Doctrine ODM, unless it doesn't affect complexity too much.

Steveb-p avatar Dec 14 '19 23:12 Steveb-p

I'm not sure such a use case warrants creating a special case for it in Doctrine ODM

@Steveb-p actually Domain-Driven Design methodology, which is widely used today, suggests to use ValueObjects for any scalar value you have in your app. Username example above is just one simple example of a value object class.

So I think my use case is not something very special, since there are many people who use DDD methodology, thus I believe that enabling ODM to be used with DDD is something good.

petr-buchin avatar Dec 15 '19 00:12 petr-buchin

@malarzm yeah, I can do that. Just wanna ensure that we are on the same page:

I think it's a bad idea to add itemType property to the Field annotation class, since this property will be shown by the IDEs for every field. Instead, I think it's better to create new annotation Collection, which will have itemType property.

Having that said, how do you think, is it a good idea to implement new type TypedCollectionType (or some better name), or should I just add this behavior to the good old CollectionType ?

petr-buchin avatar Dec 15 '19 01:12 petr-buchin

@Steveb-p but if you change the values on PreFlush to strings then you'll need to change it back again on PostFlush - that seems like a lot of hassle and is error prone as there might be an exception during flush and you'll end up with strings instead of objects. Also I believe adding Type conversion will not add a lot of complexity to the ODM.

Having that said, how do you think, is it a good idea to implement new type TypedCollectionType (or some better name), or should I just add this behavior to the good old CollectionType ?

Ah that's reminding me why we haven't done this - currently there's no way to pass field's mapping configuration to the underlying Type. With this I must take back that adding this feature won't bring complexity. But anyway this was on our list of things todo so it's not a no-go.

@petr-buchin on first thought I was against @ODM\Collection but actually it will make sense, it could just extend normal Field and add itemType or innerType or whatever. @alcaeus WDYT?

malarzm avatar Dec 15 '19 10:12 malarzm

I've put this on the roadmap and we'll be discussing this in our hangout tonight - I'll post an update afterwards.

I remember us wanting to make some changes to collections and hashes in general (e.g. allowing to use Collection instances for them instead of only plain arrays), so this might could be a good step towards a better implementation for that.

alcaeus avatar Dec 17 '19 07:12 alcaeus

+1 I need to store an array of CustomType, is there anything new on this request ? thx

ikallali avatar Oct 12 '21 20:10 ikallali

Nothing really, still best to use own types

malarzm avatar Oct 23 '21 13:10 malarzm