Problem/Motivation
We have some limitations in the current approach for field mapping in the media module it at the moment. In comment #289 there was a proposal to move this forward. These are some examples of limitations from that comment:
- It doesn't handle complex field types: currently the code takes metadata value and assigns it to the main property of the field. It doesn't handle fields that might rely on more than one property. An example of that would be the Link field with "uri" and "title" properties. Currently we can only set uri. There is a workaround for that: if getMetadata() is aware of this it can prepare metadata value in a right way. I don't think that getMetadata() should care what the destination of the value will be.
- Pre-processing of the metadata value is not possible: Example are tags. They usually come in form of a comma-separated list. If you throw that into an entity reference field nothing useful will happen. You would generally want to preprocess that to convert that comma-separated list to a list of entities and use their IDs. Same workaround as in the first item applies.
- Multi-value fields: Only single value fields are supported. Same workaround as in the first item applies.
- Updating mapped values: Currently we map values on initial save only. If metadata changes upstream at some later time we won't update. There is currently no workaround for that (except custom code).
Proposed resolution
It seems that we could use a separate plugin type to solve this problem (MediaMetadataMapperInterface
or ComplexFieldMappingInterface
something along those lines). It is true that it isn't the job of the MediaSource
to solve complex media-metadata-to-field mappings. By introducing a new plugin type we separate responsibilities and clearly document them.
Doing this would not break BC. This was explicitly validated. See #2831274-310: Bring Media entity module to core as Media module for the full explanation. Quoting #2831274-311: Bring Media entity module to core as Media module to make this very clear in this issue too:
I find #310 sufficiently convincing that I'd be in favor of postponing "complex field mapping" functionality to a follow-up.
It'd change this:
function preSave() { … // FIELD MAPPING LOGIC … }
to:
function preSave() { … if ($this->getSource() instanceof ComplexFieldMappingInterface) { $this->getSource()->doComplexfieldMapping(); } else { // FIELD MAPPING LOGIC } … }
That's a trivial change, which does not require API changes.
Remaining tasks
- Discuss the mapping features.
- Implement mapping
User interface changes
API changes
TBD
Comments
Comment #2
Wim LeersComment #3
slashrsm CreditAttribution: slashrsm at MD Systems GmbH commentedIt feels that this issue could address the problems that were brought up in #2855630: Field mapping not working properly in contrib.
Comment #4
geek-merlinIs this related to #2879969: Make Media field mappings reusable between media types? Or is that even fixed or on the way already?
Comment #5
seanBHere are some thoughts based on what I've learned so far. We need to support at least the following features (or at least be able to do it in the future):
Right now we don't want metadata to care about storage. I do think we should have some configuration to make sure only logical mapping options are presented to the user. For instance it wouldn't make sense to map metadata containing a date to a boolean field. Since media sources define the metadata, it is probably the best place to store such configuration.
At a minimum we should require metadata to define a data type. For date we can allow text/number/date fields and handle mapping accordingly. For booleans we can allow text/number/boolean fields etc.
Like I said, just some thoughts, feedback is more than welcome!
Comment #6
geek-merlinSome random thoughts:
* From a distant point of view, we want a generalized verison of computed_field (while that only computes / denormalizes a special field type, we want to compute arbitrary field types).
* Even if the concrete implementations are currently for media, there is nothing that prevents using this for other entity types (pluginification is #2879969: Make Media field mappings reusable between media types)
* Mapping metadata to taxonomy terms, creating new terms and/or reusing existing terms. - So it is not only about mapping, but some values want to be preprocessed.
My gut feeling what can happen is
a) we either pick some use cases and bake them in which feels a bit creepy and random but might just work(tm)
b) we build a something along the lines of feeds mapper and feeds tamper (huh)
c) we leverage the token system for field default values
d) ???
Comment #8
geek-merlin@SeanB #5:
My first gut feeling was that the mapping is an important part of media, but in the current state a really odd child.
After some sleeping over this i think the most drupal-ish way is to leverage typed data api for this.
Imagine a configuration like this:
Leveraging typed data, we can stay typesafe, be it text, term or whatever. We just need property providers and no additional API.
We just need a typed-data browser (like rules has, or was proposed in #514990: Add a UI for browsing tokens for tokens)
We can even do something like:
(Just last week i hacked that use case to have terms for extracted text properties.)
So
* Closed my #2879969: Make Media field mappings reusable between media types in favor of this
* Added #2924821: Help media with typed data browser? in Rules to ping fago&friends about this
* Added #2924819: Make Tokens a special case of Typed Data to further the idea
Comment #10
seanBWe just had an interesting discussion about how this could look. Here is a summary:
Some screens of how this could look.
Comment #11
geek-merlinNice writeup! That child looks to me as if the father is the feeds mapper and the mother a reinvention of typed data. ;-)
Comment #15
seanBLinking #2836153: Improve metadata mapping UI on Media type form as well.
Comment #16
eelkeblokComment #18
phenaproximaThis is a major feature request, so escalating the priority here.