schmittjoh / JMSSerializerBundle

Easily serialize, and deserialize data of any complexity (supports XML, JSON, YAML)
http://jmsyst.com/bundles/JMSSerializerBundle
MIT License
1.8k stars 312 forks source link

Configured order of custom handlers is ignored #174

Closed pluess closed 12 years ago

pluess commented 12 years ago

The documentation at http://jmsyst.com/de/bundles/JMSSerializerBundle/master/cookbook/custom_handlers sais: The order in which the handlers are listed in the “handlers” section defines in which they are called while processing.

I'm not 100% sure what this means. My understanding is that I can put something like this

jms_serializer:
    handlers:
        my_custom_handler: true
        object_based: false
        datetime:
            format: "Y-m-dTH:i:s"
            default_timezone: "UTC"
        array_collection: true
        form_error: true
        constraint_violation: true

into app/config/config.yml which should make sure that my_custom_handler is always called first.

But looking at JMSSerializerExtension::load line 77ff makes clear that this configuration is ignored. The order depends on the order the configureSerializerExtension methods are called on the bundles.

        foreach ($config['handlers'] as $k => $handlerConfig) {
            $id = $this->factories[$k]->getHandlerId($container, $handlerConfig);
            $type = $this->factories[$k]->getType($handlerConfig);

            if (0 !== ($type & HandlerFactoryInterface::TYPE_SERIALIZATION)) {
                $serializationHandlers[] = new Reference($id);
            }

            if (0 !== ($type & HandlerFactoryInterface::TYPE_DESERIALIZATION)) {
                $deserializationHandlers[] = new Reference($id);
            }
        }

My sugestion would be to change the above code to obey what is configured in the handler section. A question that arises by implenting it that way is: What should happen to handlers which are not listed there. I think they should be ignored (and the developer should be warned, but I don't know how to do that).

schmittjoh commented 12 years ago

The code looks correct to me; $config['handlers'] is the order that you have defined in your configuration.

Could you give more information what did not work as expected?

pluess commented 12 years ago

When I have a look at the data in the debugger, I find my configuration in $configs[0]['handlers']. I think we should loop over $configs[0]['handlers'] instead of $config['handlers'].

PS: Many thanks for your great bundles! I'm just implementing a kind of remote doctrine over REST using the serializer and aop bundle with only a few 100 lines of code.

schmittjoh commented 12 years ago

$configs is the unprocessed configuration before validation/merging. You might need to explicitly list the built-in handlers if you want them to be after your handler in the processed config ($config).

pluess commented 12 years ago

Well that's what I tried. I put this

jms_serializer:
    handlers:
        my_custom_handler: true
        object_based: false
        datetime:
            format: "Y-m-dTH:i:s"
            default_timezone: "UTC"
        array_collection: true
        form_error: true
        constraint_violation: true

into app/config/config.yml

Isn't this the way to go?

schmittjoh commented 12 years ago

Yes that should work. Could you dump the content of $config['handlers]?

pluess commented 12 years ago

Here it is.

array(7) {
  ["object_based"]=>
  array(2) {
    ["serialization"]=>
    bool(false)
    ["deserialization"]=>
    bool(false)
  }
  ["array_collection"]=>
  array(0) {
  }
  ["constraint_violation"]=>
  array(0) {
  }
  ["datetime"]=>
  array(2) {
    ["format"]=>
    string(11) "Y-m-dTH:i:s"
    ["default_timezone"]=>
    string(3) "UTC"
  }
  ["form_error"]=>
  array(0) {
  }
  ["custom"]=>
  array(0) {
  }
  ["doctrine_proxy"]=>
  array(0) {
  }
}

As you can see, the custom handler shows up as the second last entry. As far as I understand, this is because it relies on the order JMSSerializerExtension::getConfiguration() calls configureSerializerExtension()on the bundles and not the order defined in app/config/config.yml.

schmittjoh commented 12 years ago

I see the problem now. This actually needs to be fixed in symfony itself (https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Config/Definition/ArrayNode.php#L268). Maybe you could open a ticket there?

For the time being, I think it should work if you just make sure that the bundle which is providing your handler is registered before this bundle, and we also need to change the documentation. If you like you could send a PR for this, otherwise I will do it at some point, just keep the issue open.

pluess commented 12 years ago

Thanks! Got it. Opened an issue: https://github.com/symfony/symfony/issues/5528

schmittjoh commented 12 years ago

Thanks, closing this here.