8000 merged branch lsmith77/serializer_interface (PR #2530) · symfony/symfony@b7c7ed4 · GitHub
[go: up one dir, main page]
Skip to content

Commit b7c7ed4

Browse files
fabpotfabpot
committed
merged branch lsmith77/serializer_interface (PR #2530)
Commits ------- 0776b50 removed supports(De)Serializiation() 72b9083 SerializerAwareNormalizer now only implements SerializerAwareInterface 97389fa use Serializer specific RuntimeException cb495fd added additional unit tests for deserialization 967531f fixed various typos from the refactoring 067242d updated serializer tests to use the new interfaces d811e29 CS fix 351eaa8 require a (de)normalizer inside the (de)normalizable interfaces instead of a serializer c3d6123 re-added supports(de)normalization() 078f7f3 more typo fixes c3a711d abstract class children should also implement dernormalization 2a6741c typo fix d021dc8 refactored encoder handling to use the supports*() methods to determine which encoder handles what format f8e2787 refactored Normalizer interfaces 58bd0f5 refactored the EncoderInterface b0daf35 split off an EncoderInterface and NormalizerInterface from SerializerInterface Discussion ---------- [Serializer] split off an EncoderInterface and NormalizerInterface from SerializerInte Bug fix: no Feature addition: no Backwards compatibility break: yes (but not inside a stable API) Symfony2 tests pass: ![Build Status](https://secure.travis-ci.org/lsmith77/symfony.png?branch=serializer_interface) Fixes the following tickets: #2153 The purpose is to make it easier for other implementations that only implement parts of the interface due to different underlying approaches like the JMSSerializerBundle. --------------------------------------------------------------------------- by schmittjoh at 2011/11/01 03:36:17 -0700 Actually, you can keep the current interface and I will just provide an adapter, sth like the following: ```php <?php class SymfonyAdapter implements SymfonyInterface { public function __construct(BundleInterface $serializer) { /* ... */ } // symfony serializer methods mapped to bundle methods } ``` I like to provide an adapter instead of implementing the interface directly since the bundle can be used standalone right now, and I don't want to add a dependency on the component just for the sake of the interface. However, I do not completely see the purpose of the component. When would someone be recommended to use it? Everything the component does, the bundles does at the same level with the same complexity or simplicity (however you want to view that). --------------------------------------------------------------------------- by lsmith77 at 2011/11/01 03:40:55 -0700 standalone in what way? you mean even out of the context of Symfony? In that context imho you should ship that code outside of a Bundle. Regardless, how will that adaptor work? How would you implement methods like ``getEncoder()``? Afaik you can't and this is what this PR is about, splitting the interface to enable people to more finely specify what they provide. --------------------------------------------------------------------------- by schmittjoh at 2011/11/01 04:03:56 -0700 I would just throw exceptions when something is not supported. The more important question though is what is the goal of the component in the long-term, i.e. what problems is it supposed to solve, or in which cases should it be used? Because right now it seems to me - correct me if I'm wrong - that the only purpose is that people don't have to install an extra library. However, that might even be frustrating for users because they need to migrate their code to the bundle as soon as they need to customize the serialization process which you need in 99% of the cases. For deserialization, the situation in the component is even worse. So, if my assessment is correct here (i.e. component to get started fast, if you need more migrate to the bundle), I think it would be better and less painful to have them start with the bundle right away. --------------------------------------------------------------------------- by lsmith77 at 2011/11/01 04:15:10 -0700 Well then imho it would be better to split the interface. I think the serializer component is sufficient for many situations and imho its easier to grok. Furthermore the normalizer/encoder concept it can be used in situations where JMSSerializerBundle cannot be used. And splitting up the interfaces has exactly the goal of reducing the "frustrations" caused by out growing the the component. --------------------------------------------------------------------------- by schmittjoh at 2011/11/01 04:29:39 -0700 I don't agree, but it's a subjective thing anyway. So, whatever interface you come up with (preferably as few methods as possible), I will provide an adapter for it. --------------------------------------------------------------------------- by fabpot at 2011/11/07 08:45:25 -0800 What's the status of this PR? --------------------------------------------------------------------------- by lsmith77 at 2011/11/07 10:28:14 -0800 from my POV its good to go. but would like a nod from someone else in terms of the naming of the new interfaces On 07.11.2011, at 17:45, Fabien Potencier <reply@reply.github.com> wrote: > What's the status of this PR? > > --- > Reply to this email directly or view it on GitHub: > #2530 (comment) --------------------------------------------------------------------------- by stof at 2011/11/08 11:37:40 -0800 @lsmith77 what about doing the same for the ``NormalizerInterface`` instead of adding a new interface with a confusing name ? The Serializer class could implement ``Normalizer\NormalizerInterface`` by adding the 2 needed methods instead of duplicating part of the interface. The next step is to refactor the Serializer class so that it choose the encoder and the decoder based on the ``support*`` methods. But this could probably be done in a separate PR. --------------------------------------------------------------------------- by lsmith77 at 2011/11/08 11:51:27 -0800 yeah .. i wanted to do that once we are in agreement on the encoder stuff. question then is if we should again split off Denormalization. i guess yes. --------------------------------------------------------------------------- by lsmith77 at 2011/11/08 12:06:34 -0800 ok done .. --------------------------------------------------------------------------- by lsmith77 at 2011/11/08 12:59:51 -0800 i guess the next big task is to add more tests .. had to fix way too few unit tests with all this shuffling around .. will also help validating the concept. i should also test this out in a production application. --------------------------------------------------------------------------- by lsmith77 at 2011/11/14 13:27:48 -0800 @ericclemmons can you also have a look at this PR and potentially help me adding tests? --------------------------------------------------------------------------- by fabpot at 2011/12/07 07:32:06 -0800 @lsmith77: Is it ready to be merged? Should I wait for more unit tests? --------------------------------------------------------------------------- by lsmith77 at 2011/12/07 07:34:56 -0800 If you merge it I am afraid I might get lazy and not write tests. This is why I changed the topic to WIP. I promise to finish this on the weekend. Note however I was planning to write the tests for 2.0 and send them via a separate PR. Once that PR is merged into 2.0 and master. I would then refactor them to work for this PR. This way both 2.0 and master will have tests. --------------------------------------------------------------------------- by fabpot at 2011/12/07 07:42:15 -0800 @lsmith77: sounds good. Thanks. --------------------------------------------------------------------------- by lsmith77 at 2011/12/11 12:02:12 -0800 @fabpot ok i am done from my end. @schmittjoh would be great if you could look over the final interfaces one time and give your blessing that you will indeed be able to provide implementations for these interfaces inside JMSSerializerBundle (even if just via an adapter) --------------------------------------------------------------------------- by stof at 2011/12/12 12:43:49 -0800 @schmittjoh can you take a look as requested by @lsmith77 ? --------------------------------------------------------------------------- by schmittjoh at 2011/12/13 03:33:23 -0800 Are the supports methods necessary? This is what I'm using in the bundle: https://github.com/schmittjoh/JMSSerializerBundle/blob/master/Serializer/SerializerInterface.php --------------------------------------------------------------------------- by lsmith77 at 2011/12/13 04:08:49 -0800 @schmittjoh without them determining if something is supported will always require an exception, 8000 which is pretty expensive. especially if one iterates over a data structure this can cause a lot of overhead. --------------------------------------------------------------------------- by schmittjoh at 2011/12/13 04:24:18 -0800 my question was more if you have a real-world use case where this is useful? On Tue, Dec 13, 2011 at 1:08 PM, Lukas Kahwe Smith < reply@reply.github.com > wrote: > @schmittjoh without them determining if something is supported will always > require an exception, which is pretty expensive. especially if one iterates > over a data structure this can cause a lot of overhead. > > --- > Reply to this email directly or view it on GitHub: > #2530 (comment) > --------------------------------------------------------------------------- by lsmith77 at 2011/12/13 04:28:08 -0800 yes .. this serializer .. since it traverses the tree and decides what is the current normalizer one by one (aka not via visitors as in your implementation). without the supports*() methods it would need to have the normalizer throw exceptions, but this is not exceptional, its the normal code flow to have to iterate to find the correct normalizer. --------------------------------------------------------------------------- by schmittjoh at 2011/12/13 04:30:36 -0800 can we split it off into a second interface? On Tue, Dec 13, 2011 at 1:28 PM, Lukas Kahwe Smith < reply@reply.github.com > wrote: > yes .. this serializer .. since it traverses the tree and decides what is > the current normalizer one by one (aka not via visitors as in your > implementation). without the supports*() methods it would need to have the > normalizer throw exceptions, but this is not exceptional, its the normal > code flow to have to iterate to find the correct normalizer. > > --- > Reply to this email directly or view it on GitHub: > #2530 (comment) > --------------------------------------------------------------------------- by lsmith77 at 2011/12/13 04:33:27 -0800 hmm .. i guess we could .. these methods in a way are implementation specific and are mainly public because its different objects interacting with each other, though for users of the lib they can also be convenient at times. --------------------------------------------------------------------------- by lsmith77 at 2011/12/14 09:13:53 -0800 ok i reviewed things again and just removed those two methods, since the possibility for these methods to be feasible is too tied to the implementation and for this particular implementation supportsEncoding() and supportsDecoding() are still available. so all ready to be merged .. --------------------------------------------------------------------------- by lsmith77 at 2011/12/14 09:15:44 -0800 hmm i realized one thing just now: lsmith77@cb495fd that commit should also be included in 2.0 .. i am not sure what the most elegant way is to make that happen .. --------------------------------------------------------------------------- by fabpot at 2011/12/14 10:10:16 -0800 @lsmith77: commit cb495fd cannot be cherry picked in 2.0 as is as the tests do not pass: "Fatal error: Call to undefined method Symfony\Component\Serializer\Serializer::supportsDenormalization() in tests/Symfony/Tests/Component/Serializer/SerializerTest.php on line 150" --------------------------------------------------------------------------- by lsmith77 at 2011/12/14 10:11:55 -0800 ah of course .. i just removed that method :) .. then never mind .. all is well.
2 parents a6cdddd + 0776b50 commit b7c7ed4

18 files changed

+305
-191
lines changed

‎src/Symfony/Component/Serializer/Encoder/DecoderInterface.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*/
1414

1515
/**
16-
* Defines the interface of encoders that are able to decode their own format
16+
* Defines the interface of decoders
1717
*
1818
* @author Jordi Boggiano <j.boggiano@seld.be>
1919
*/
@@ -22,10 +22,18 @@ interface DecoderInterface
2222
/**
2323
* Decodes a string into PHP data
2424
*
25-
* @param string $data Data to decode
26-
* @param string $format Format to decode from
25+
* @param scalar $data Data to decode
26+
* @param string $format Format name
2727
*
2828
* @return mixed
2929
*/
3030
function decode($data, $format);
31+
32+
/**
33+
* Checks whether the serializer can decode from given format
34+
*
35+
* @param string $format format name
36+
* @return Boolean
37+
*/
38+
function supportsDecoding($format);
3139
}

‎src/Symfony/Component/Serializer/Encoder/EncoderInterface.php

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace Symfony\Component\Serializer\Encoder;
44

5-
65
/*
76
* This file is part of the Symfony framework.
87
*
@@ -20,12 +19,20 @@
2019
interface EncoderInterface
2120
{
2221
/**
23-
* Encodes data into a string
22+
* Encodes data into the given format
2423
*
2524
* @param mixed $data Data to encode
26-
* @param string $format Format to encode to
25+
* @param string $format Format name
2726
*
28-
* @return string
27+
* @return scalar
2928
*/
3029
function encode($data, $format);
30+
31+
/**
32+
* Checks whether the serializer can encode to given format
33+
*
34+
* @param string $format format name
35+
* @return Boolean
36+
*/
37+
function supportsEncoding($format);
3138
}

‎src/Symfony/Component/Serializer/Encoder/JsonEncoder.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,26 @@ public function decode($data, $format)
3434
{
3535
return json_decode($data, true);
3636
}
37+
38+
/**
39+
* Checks whether the serializer can encode to given format
40+
*
41+
* @param string $format format name
42+
* @return Boolean
43+
*/
44+
public function supportsEncoding($format)
45+
{
46+
return 'json' === $format;
47+
}
48+
49+
/**
50+
* Checks whether the serializer can decode from given format
51+
*
52+
* @param string $format format name
53+
* @return Boolean
54+
*/
55+
public function supportsDecoding($format)
56+
{
57+
return 'json' === $format;
58+
}
3759
}

‎src/Symfony/Component/Serializer/Encoder/XmlEncoder.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,28 @@ public function decode($data, $format)
7171
return $this->parseXml($xml);
7272
}
7373

74+
/**
75+
* Checks whether the serializer can encode to given format
76+
*
77+
* @param string $format format name
78+
* @return Boolean
79+
*/
80+
public function supportsEncoding($format)
81+
{
82+
return 'xml' === $format;
83+
}
84+
85+
/**
86+
* Checks whether the serializer can decode from given format
87+
*
88+
* @param string $format format name
89+
* @return Boolean
90+
*/
91+
public function supportsDecoding($format)
92+
{
93+
return 'xml' === $format;
94+
}
95+
7496
/**
7597
* Sets the root node name
7698
* @param string $name root node name

‎src/Symfony/Component/Serializer/Normalizer/CustomNormalizer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
/**
1616
* @author Jordi Boggiano <j.boggiano@seld.be>
1717
*/
18-
class CustomNormalizer extends SerializerAwareNormalizer
18+
class CustomNormalizer extends SerializerAwareNormalizer implements NormalizerInterface, DenormalizerInterface
1919
{
2020
/**
2121
* {@inheritdoc}

‎src/Symfony/Component/Serializer/Normalizer/DenormalizableInterface.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
namespace Symfony\Component\Serializer\Normalizer;
4+
5+
/*
6+
* This file is part of the Symfony framework.
7+
*
8+
* (c) Fabien Potencier <fabien@symfony.com>
9+
*
10+
* This source file is subject to the MIT license that is bundled
11+
* with this source code in the file LICENSE.
12+
*/
13+
14+
/**
15+
* Defines the most basic interface a class must implement to be denormalizable
16+
*
17+
* If a denormalizer is registered for the class and it doesn't implement
18+
* the Denormalizable interfaces, the normalizer will be used instead
19+
*
20+
* @author Jordi Boggiano <j.boggiano@seld.be>
21+
*/
22+
interface DenormalizableInterface
23+
{
24+
/**
25+
* Denormalizes the object back from an array of scalars|arrays.
26+
*
27+
* It is important to understand that the normalize() call should denormalize
28+
* recursively all child objects of the implementor.
29+
*
30+
* @param DenormalizerInterface $denormalizer The denormalizer is given so that you
31+
* can use it to denormalize objects contained within this object.
32+
* @param array|scalar $data The data from which to re-create the object.
33+
* @param string|null $format The format is optionally given to be able to denormalize differently
34+
* based on different input formats.
35+
*/
36+
function denormalize(DenormalizerInterface $denormalizer, $data, $format = null);
37+
}

‎src/Symfony/Component/Serializer/Normalizer/DenormalizerInterface.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
namespace Symfony\Component\Serializer\Normalizer;
4+
5+
/*
6+
* This file is part of the Symfony framework.
7+
*
8+
* (c) Fabien Potencier <fabien@symfony.com>
9+
*
10+
* This source file is subject to the MIT license that is bundled
11+
* with this source code in the file LICENSE.
12+
*/
13+
14+
/**
15+
* Defines the interface of denormalizers.
16+
*
17+
* @author Jordi Boggiano <j.boggiano@seld.be>
18+
*/
19+
interface DenormalizerInterface
20+
{
21+
/**
22+
* Denormalizes data back into an object of the given class
23+
*
24+
* @param mixed $data data to restore
25+
* @param string $class the expected class to instantiate
26+
* @param string $format format the given data was extracted from
27+
* @return object
28+
*/
29+
function denormalize($data, $class, $format = null);
30+
31+
/**
32+
* Checks whether the given class is supported for denormalization by this normalizer
33+
*
34+
* @param mixed $data Data to denormalize from.
35+
* @param string $type The class to which the data should be denormalized.
36+
* @param string $format The format being deserialized from.
37+
* @return Boolean
38+
*/
39+
function supportsDenormalization($data, $type, $format = null);
40+
}

‎src/Symfony/Component/Serializer/Normalizer/GetSetMethodNormalizer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
*
3434
* @author Nils Adermann <naderman@naderman.de>
3535
*/
36-
class GetSetMethodNormalizer extends SerializerAwareNormalizer
36+
class GetSetMethodNormalizer extends SerializerAwareNormalizer implements NormalizerInterface, DenormalizerInterface
3737
{
3838
/**
3939
* {@inheritdoc}

‎src/Symfony/Component/Serializer/Normalizer/NormalizableInterface.php

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
namespace Symfony\Component\Serializer\Normalizer;
44

5-
use Symfony\Component\Serializer\SerializerInterface;
6-
75
/*
86
* This file is part of the Symfony framework.
97
*
@@ -29,25 +27,11 @@ interface NormalizableInterface
2927
* It is important to understand that the normalize() call should normalize
3028
* recursively all child objects of the implementor.
3129
*
32-
* @param SerializerInterface $serializer The serializer is given so that you
30+
* @param NormalizerInterface $normalizer The normalizer is given so that you
3331
* can use it to normalize objects contained within this object.
3432
* @param string|null $format The format is optionally given to be able to normalize differently
3533
* based on different output formats.
3634
* @return array|scalar
3735
*/
38-
function normalize(SerializerInterface $serializer, $format = null);
39-
40-
/**
41-
* Denormalizes the object back from an array of scalars|arrays.
42-
*
43-
* It is important to understand that the normalize() call should denormalize
44-
* recursively all child objects of the implementor.
45-
*
46-
* @param SerializerInterface $serializer The serializer is given so that you
47-
* can use it to denormalize objects contained within this object.
48-
* @param array|scalar $data The data from which to re-create the object.
49-
* @param string|null $format The format is optionally given to be able to denormalize differently
50-
* based on different input formats.
51-
*/
52-
function denormalize(SerializerInterface $serializer, $data, $format = null);
36+
function normalize(NormalizerInterface $normalizer, $format = null);
5337
}

‎src/Symfony/Component/Serializer/Normalizer/NormalizerInterface.php

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace Symfony\Component\Serializer\Normalizer;
44

5-
65
/*
76
* This file is part of the Symfony framework.
87
*
@@ -28,16 +27,6 @@ interface NormalizerInterface
2827
*/
2928
function normalize($object, $format = null);
3029

31-
/**
32-
* Denormalizes data back into an object of the given class
33-
*
34-
* @param mixed $data data to restore
35-
* @param string $class the expected class to instantiate
36-
* @param string $format format the given data was extracted from
37-
* @return object
38-
*/
39-
function denormalize($data, $class, $format = null);
40-
4130
/**
4231
* Checks whether the given class is supported for normalization by this normalizer
4332
*
@@ -46,14 +35,4 @@ function denormalize($data, $class, $format = null);
4635
* @return Boolean
4736
*/
4837
function supportsNormalization($data, $format = null);
49-
50-
/**
51-
* Checks whether the given class is supported for denormalization by this normalizer
52-
*
53-
* @param mixed $data Data to denormalize from.
54-
* @param string $type The class to which the data should be denormalized.
55-
* @param string $format The format being deserialized from.
56-
* @return Boolean
57-
*/
58-
function supportsDenormalization($data, $type, $format = null);
5938
}

‎src/Symfony/Component/Serializer/Normalizer/SerializerAwareNormalizer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*
2020
* @author Jordi Boggiano <j.boggiano@seld.be>
2121
*/
22-
abstract class SerializerAwareNormalizer implements SerializerAwareInterface, NormalizerInterface
22+
abstract class SerializerAwareNormalizer implements SerializerAwareInterface
2323
{
2424
protected $serializer;
2525

0 commit comments

Comments
 (0)
0