Serialization uses addTag incorrectly [#1958484]

Per https://github.com/symfony/symfony/issues/3251 the attributes can only be a one dimensional array. That PHP does not throw an error on them is a bug https://github.com/symfony/symfony/issues/7543 (igorw told me to file that as a bug).

The serialization bundle uses a multidimensional array, for now in #1939660-31: Use YAML as the primary means for service registration i have cut it back to a) only support one format per registration b) use format => xml and then we can do more / better here. igorw suggested container parameters to do the tunes of serializer.encoder.xml => XML.

Comment	File	Size	Author
#3	1958484-do-not-test.patch	13.59 KB	chx
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

chx CreditAttribution: chx commented 2 April 2013 at 02:39

Assigned:

Unassigned

» klausi

Comment #2

chx CreditAttribution: chx commented 3 April 2013 at 02:21

Title:	Serialization uses addTag incorrectly	» Serialization classes are not plugins
Priority:	Normal	» Major

Why are these not plugins in the first place?

Comment #3

chx CreditAttribution: chx commented 3 April 2013 at 20:15

Title:	Serialization classes are not plugins	» Serialization uses addTag incorrectly
Priority:	Major	» Normal

File	Size
1958484-do-not-test.patch	13.59 KB

OK, so I hear they don't want to be plugins.

For easier review, here's the patch for the jsonld,hal and serialization dirs from the YAML service issue.

Comment #4

Anonymous (not verified) CreditAttribution: Anonymous commented 3 April 2013 at 21:04

To unblock chx's patch, I'd say we can go ahead with this.

However, it introduces the possibility of a hard-to-debug issue in some cases (described below). We should fix this in this issue once chx's patch is in.

Lets say that JsonLdEncoder supports both jsonld and drupal_jsonld. With the new approach, two services would be registered for the Encoder, encoder.jsonld and encoder.drupal_jsonld. These would both be added to the chain of encoders, for example

JsonLdEncoder - JsonLdEncoder - XmlEncoder - JsonEncoder

If a developer comes along and decides they want to change which Encoder responds to drupal_jsonld requests, they would switch out the service named encoder.drupal_jsonld. This could result in the following chain.

JsonLdEncoder - CustomDrupalJsonLdEncoder - XmlEncoder - JsonEncoder

In this case, since JsonLdEncoder::supportsEncoding returns TRUE for both jsonld and drupal_jsonld, the developer's custom encoder would never be reached. This seems like a developer WTF to me.

Comment #5

damiankloip CreditAttribution: damiankloip commented 10 September 2013 at 15:39

Status:

Active

» Fixed

This has already happened, was committed from the issue chx was referring to in#3

Comment #6

chx CreditAttribution: chx commented 10 September 2013 at 20:43

Status:

Fixed

» Active

Well it happened but I have dumbed it down to make it happen and this issue is whether it needs to be smartened up somehow again :)

Comment #7

damiankloip CreditAttribution: damiankloip commented 10 September 2013 at 20:48

OK, fair enough. Let's talk :)

Comment #8

damiankloip CreditAttribution: damiankloip commented 21 September 2015 at 15:29

Issue summary:	View changes
Status:	Active	» Fixed

I don't think this needs to be smartened up. The YAML conversion was good I think. chx if you read this, and think it should still be open, please re open. thanks!