We need a way to retrieve all serialization formats that are currently available in the system. Example: REST module wants to expose configuration options which formats are allowed on which route.
How can we achieve this in a clean way? JsonldBundle registers the encoders with a tag "encoder". First attempt:
$container = drupal_container();
$encoders = $container->findTaggedServiceIds('encoder');
That fails:
Fatal error: Call to undefined method service_container_prod_::findTaggedServiceIds()
So the findTaggedServiceIds() method is not available at runtime. Meh.
Do we want a CompilerPass that can execute findTaggedServiceIds() and stores the encoder somewhere?
This looks like another typical Drupal use case that is not supported by Symfony at all to me. Serialization formats are not statically defined and we need a list of what is available.
Comments
Comment #1
klausiforgot the WSCCI tag.
Comment #2
Crell commentedIdea we discussed today on the REST Team call:
1) In the compiler pass, build up a list of available formats and stick it in a container parameter. Then we can just grab that list from the container (and can do it via injection, too, rather than needing the actual container passed to us).
2) Make each format its own serializer. So we register a serializer.jsonld, a serializer.atom, a serializer.svg, and whatever other formats. Then you can enable or disable different serializers for different paths very easily.
Comment #3
Crell commentedDAMNIT FIREFOX!
1) In the compiler pass, build a list of formats and stick it into a container parameter. Then we can access that list via the container easily. We can even have it injected.
2) Register each format as a separate service. So we get serializer.jsonld, serializer.atom, serializer.svg, etc. That makes it easier to wire up different serialization formats for different paths based on user configuration.
Comment #4
Crell commentedDAMNIT DRUPAL!
Comment #5
kylebrowning commentedIs this as easy as grabbing the encoders from this line of code and storing them in the container, then building a form off that for configuration options?
(code taken from http://drupalcode.org/project/drupal.git/blob/9e28980b6f5e1ca6273cd93a7f...)
Comment #6
Crell commentedAlmost. You wouldn't build an array of Reference objects but of either the service IDs or the name of the format, or maybe an associative array of that, or whatever it is klausi says he needs. But either way it would be a PHP primitive structure, no objects. Then you'd assign that to the container with $container->setProperty().
Comment #7
damiankloip commentedJust referencing #1819760: Add a REST export display plugin and serializer integration. here, I think this could be useful for that.
Comment #8
klausiFirst attempt: Added a format tag and container parameter.
You can now read all available formats with
The tag is currently used on the encoder stuff of jsonld module, which is probably wrong.
Comment #9
damiankloip commentedShould this patch also include the regular JSON/AJAX stuff that got in too? #1846000: Add serializer support for JSON and AJAX
Comment #10
klausiYes, I think so. I'm still not sure if jsonld module should really provide its own two serializer services, or if just tagging the encoders is good enough.
Comment #11
Anonymous (not verified) commentedIt would be nice if the Encoder could be aware of this tag itself. For example, if we could add supported formats to Symfony's JsonEncoder by tagging, then we wouldn't need to have the JsonldEncoder. However, I don't think that's likely.
This is a simple solution. While I was initially concerned that there's a slight conceptual mismatch, I'm having a hard time figuring out the corner cases to demonstrate a mismatch. At least for now, I think this is a reasonable solution.
Once it includes the JSON/AJAX stuff, I think we can RTBC this.
Comment #12
Anonymous (not verified) commentedI'm noticing that in your code, you use 'format' as the tag. We should probably make this more descriptive, since format is a widely used word.
Comment #13
damiankloip commentedChanged the tag to format_info and added one for the json encoder.
Comment #14
Crell commentedformat_info is still too generic. That could mean any number of things. The tag should probably be namespaced. (Are dots legal in tags?)
That said, why do we need a separate tag at all? Why can't it just be attributes on the encoder tag?
Comment #15
damiankloip commentedYeah, that makes alot of sense, and encoder is exactly what it is.
Comment #17
damiankloip commented#15: 1850704-15.patch queued for re-testing.
Comment #19
klausiI think that is wrong, jsonld should only be added to serailizer.encoder.jsonld and drupal_jsonld to serializer.encoder.drupal_jsonld.
Comment #20
klausiForget my comment, we only have one encoder anyway in jsonld module. Should be fine.
I found the installation problem: instead of accessing $attributes directly we need to access the first item, because that is a nested array.
Comment #22
klausiOk, the serialization test format also needs a correctly formatted tag. Not sure we should add an isset() check if the format attribute actually exists on the encoder tag, but on the other hand every encoder should have it.
Comment #23
moshe weitzman commentedLooks simple enough
Comment #24
Crell commented#22 looks good to me, too.
Comment #25
klausiPatch does not apply anymore, rerolled.
The interdiff contains the merge conflict resolution.
Git, did I tell you already today that I love you? I do, deeply.
Comment #27
damiankloip commentedThis fails again because #1854874: Add serializer support for XML got reverted. Could wait for that?
Comment #28
klausiSure!
Comment #29
klausiBack to needs work.
Comment #30
klausiRerolled.
Comment #31
moshe weitzman commentedBack to RTBC
Comment #32
damiankloip commentedYep, this looks good now. Let's get this in. This will be useful for views related issues.
Comment #33
dries commentedCommitted to 8.x. Thanks!