doctrine / mongodb-odm

The Official PHP MongoDB ORM/ODM
https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/
MIT License
1.09k stars 504 forks source link

Allow typed collection mapping #2137

Open petr-buchin opened 4 years ago

petr-buchin commented 4 years ago

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;
malarzm commented 4 years ago

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

petr-buchin commented 4 years ago

@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?

malarzm commented 4 years ago

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?

Steveb-p commented 4 years ago

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.

petr-buchin commented 4 years ago

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 commented 4 years ago

@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 ?

malarzm commented 4 years ago

@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?

alcaeus commented 4 years ago

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.

ikallali commented 3 years ago

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

malarzm commented 3 years ago

Nothing really, still best to use own types