From 87263d4f0df31e9a7d079928666717f6ecf870cf Mon Sep 17 00:00:00 2001 From: Axel Guckelsberger Date: Tue, 28 Sep 2021 12:41:47 +0200 Subject: [PATCH 1/5] [Serializer] ensure ChainEncoder/ChainDecoder properly consider the context --- .../Component/Serializer/Encoder/ChainDecoder.php | 11 +---------- .../Component/Serializer/Encoder/ChainEncoder.php | 11 +---------- .../Serializer/Tests/Encoder/ChainDecoderTest.php | 12 ++++++++++++ .../Serializer/Tests/Encoder/ChainEncoderTest.php | 12 ++++++++++++ 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php index 2a55d93a6066f..070c05c317489 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php @@ -25,7 +25,6 @@ class ChainDecoder implements ContextAwareDecoderInterface { protected $decoders = []; - protected $decoderByFormat = []; public function __construct(array $decoders = []) { @@ -59,18 +58,10 @@ public function supportsDecoding($format, array $context = []): bool * * @throws RuntimeException if no decoder is found */ - private function getDecoder(string $format, array $context): DecoderInterface + public function getDecoder(string $format, array $context): DecoderInterface { - if (isset($this->decoderByFormat[$format]) - && isset($this->decoders[$this->decoderByFormat[$format]]) - ) { - return $this->decoders[$this->decoderByFormat[$format]]; - } - foreach ($this->decoders as $i => $decoder) { if ($decoder->supportsDecoding($format, $context)) { - $this->decoderByFormat[$format] = $i; - return $decoder; } } diff --git a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php index b13333e88aabb..af63fc2887b24 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php @@ -25,7 +25,6 @@ class ChainEncoder implements ContextAwareEncoderInterface { protected $encoders = []; - protected $encoderByFormat = []; public function __construct(array $encoders = []) { @@ -77,18 +76,10 @@ public function needsNormalization(string $format, array $context = []): bool * * @throws RuntimeException if no encoder is found */ - private function getEncoder(string $format, array $context): EncoderInterface + public function getEncoder(string $format, array $context): EncoderInterface { - if (isset($this->encoderByFormat[$format]) - && isset($this->encoders[$this->encoderByFormat[$format]]) - ) { - return $this->encoders[$this->encoderByFormat[$format]]; - } - foreach ($this->encoders as $i => $encoder) { if ($encoder->supportsEncoding($format, $context)) { - $this->encoderByFormat[$format] = $i; - return $encoder; } } diff --git a/src/Symfony/Component/Serializer/Tests/Encoder/ChainDecoderTest.php b/src/Symfony/Component/Serializer/Tests/Encoder/ChainDecoderTest.php index 5cac8d99a5270..a75a1e55410da 100644 --- a/src/Symfony/Component/Serializer/Tests/Encoder/ChainDecoderTest.php +++ b/src/Symfony/Component/Serializer/Tests/Encoder/ChainDecoderTest.php @@ -36,6 +36,7 @@ protected function setUp(): void [self::FORMAT_2, [], false], [self::FORMAT_3, [], false], [self::FORMAT_3, ['foo' => 'bar'], true], + [self::FORMAT_3, ['foo' => 'bar2'], false], ]); $this->decoder2 = $this->createMock(DecoderInterface::class); @@ -45,6 +46,8 @@ protected function setUp(): void [self::FORMAT_1, [], false], [self::FORMAT_2, [], true], [self::FORMAT_3, [], false], + [self::FORMAT_3, ['foo' => 'bar'], false], + [self::FORMAT_3, ['foo' => 'bar2'], true], ]); $this->chainDecoder = new ChainDecoder([$this->decoder1, $this->decoder2]); @@ -53,9 +56,18 @@ protected function setUp(): void public function testSupportsDecoding() { $this->assertTrue($this->chainDecoder->supportsDecoding(self::FORMAT_1)); + $this->assertSame($this->decoder1, $this->chainDecoder->getDecoder(self::FORMAT_1, [])); + $this->assertTrue($this->chainDecoder->supportsDecoding(self::FORMAT_2)); + $this->assertSame($this->decoder2, $this->chainDecoder->getDecoder(self::FORMAT_2, [])); + $this->assertFalse($this->chainDecoder->supportsDecoding(self::FORMAT_3)); + $this->assertTrue($this->chainDecoder->supportsDecoding(self::FORMAT_3, ['foo' => 'bar'])); + $this->assertSame($this->decoder1, $this->chainDecoder->getDecoder(self::FORMAT_3, ['foo' => 'bar'])); + + $this->assertTrue($this->chainDecoder->supportsDecoding(self::FORMAT_3, ['foo' => 'bar2'])); + $this->assertSame($this->decoder2, $this->chainDecoder->getDecoder(self::FORMAT_3, ['foo' => 'bar2'])); } public function testDecode() diff --git a/src/Symfony/Component/Serializer/Tests/Encoder/ChainEncoderTest.php b/src/Symfony/Component/Serializer/Tests/Encoder/ChainEncoderTest.php index e80dc4f1843a6..9bdc650b5a0aa 100644 --- a/src/Symfony/Component/Serializer/Tests/Encoder/ChainEncoderTest.php +++ b/src/Symfony/Component/Serializer/Tests/Encoder/ChainEncoderTest.php @@ -37,6 +37,7 @@ protected function setUp(): void [self::FORMAT_2, [], false], [self::FORMAT_3, [], false], [self::FORMAT_3, ['foo' => 'bar'], true], + [self::FORMAT_3, ['foo' => 'bar2'], false], ]); $this->encoder2 = $this->createMock(EncoderInterface::class); @@ -46,6 +47,8 @@ protected function setUp(): void [self::FORMAT_1, [], false], [self::FORMAT_2, [], true], [self::FORMAT_3, [], false], + [self::FORMAT_3, ['foo' => 'bar'], false], + [self::FORMAT_3, ['foo' => 'bar2'], true], ]); $this->chainEncoder = new ChainEncoder([$this->encoder1, $this->encoder2]); @@ -54,9 +57,18 @@ protected function setUp(): void public function testSupportsEncoding() { $this->assertTrue($this->chainEncoder->supportsEncoding(self::FORMAT_1)); + $this->assertSame($this->encoder1, $this->chainEncoder->getEncoder(self::FORMAT_1, [])); + $this->assertTrue($this->chainEncoder->supportsEncoding(self::FORMAT_2)); + $this->assertSame($this->encoder2, $this->chainEncoder->getEncoder(self::FORMAT_2, [])); + $this->assertFalse($this->chainEncoder->supportsEncoding(self::FORMAT_3)); + $this->assertTrue($this->chainEncoder->supportsEncoding(self::FORMAT_3, ['foo' => 'bar'])); + $this->assertSame($this->encoder1, $this->chainEncoder->getEncoder(self::FORMAT_3, ['foo' => 'bar'])); + + $this->assertTrue($this->chainEncoder->supportsEncoding(self::FORMAT_3, ['foo' => 'bar2'])); + $this->assertSame($this->encoder2, $this->chainEncoder->getEncoder(self::FORMAT_3, ['foo' => 'bar2'])); } public function testEncode() From 42f0d6ec3ad535ee639e01c7f8a8468f4627c919 Mon Sep 17 00:00:00 2001 From: Axel Guckelsberger Date: Fri, 1 Oct 2021 11:41:51 +0200 Subject: [PATCH 2/5] keep caching mechanism unless dynamic context is given --- .../Component/Serializer/Encoder/ChainDecoder.php | 15 +++++++++++++++ .../Component/Serializer/Encoder/ChainEncoder.php | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php index 070c05c317489..b93afd35934ea 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php @@ -25,6 +25,7 @@ class ChainDecoder implements ContextAwareDecoderInterface { protected $decoders = []; + protected $decoderByFormat = []; public function __construct(array $decoders = []) { @@ -60,8 +61,22 @@ public function supportsDecoding($format, array $context = []): bool */ public function getDecoder(string $format, array $context): DecoderInterface { + $hasContext = 0 < \count($context); + if ( + true !== $hasContext + && isset($this->decoderByFormat[$format]) + && isset($this->decoders[$this->decoderByFormat[$format]]) + ) { + return $this->decoders[$this->decoderByFormat[$format]]; + } + foreach ($this->decoders as $i => $decoder) { if ($decoder->supportsDecoding($format, $context)) { + if (true !== $hasContext) { + // cache decoder if no dynamic context is given + $this->decoderByFormat[$format] = $i; + } + return $decoder; } } diff --git a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php index af63fc2887b24..58c559f5ffe99 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php @@ -25,6 +25,7 @@ class ChainEncoder implements ContextAwareEncoderInterface { protected $encoders = []; + protected $encoderByFormat = []; public function __construct(array $encoders = []) { @@ -78,8 +79,22 @@ public function needsNormalization(string $format, array $context = []): bool */ public function getEncoder(string $format, array $context): EncoderInterface { + $hasContext = 0 < \count($context); + if ( + true !== $hasContext + && isset($this->encoderByFormat[$format]) + && isset($this->encoders[$this->encoderByFormat[$format]]) + ) { + return $this->encoders[$this->encoderByFormat[$format]]; + } + foreach ($this->encoders as $i => $encoder) { if ($encoder->supportsEncoding($format, $context)) { + if (true !== $hasContext) { + // cache encoder if no dynamic context is given + $this->encoderByFormat[$format] = $i; + } + return $encoder; } } From 4789747fe6ec5864ffb36effb84e9868aca62a41 Mon Sep 17 00:00:00 2001 From: Axel Guckelsberger Date: Thu, 28 Oct 2021 18:08:02 +0200 Subject: [PATCH 3/5] avoid explicite bool comparison --- src/Symfony/Component/Serializer/Encoder/ChainDecoder.php | 4 ++-- src/Symfony/Component/Serializer/Encoder/ChainEncoder.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php index b93afd35934ea..23cc4d3a1e168 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php @@ -63,7 +63,7 @@ public function getDecoder(string $format, array $context): DecoderInterface { $hasContext = 0 < \count($context); if ( - true !== $hasContext + $hasContext && isset($this->decoderByFormat[$format]) && isset($this->decoders[$this->decoderByFormat[$format]]) ) { @@ -72,7 +72,7 @@ public function getDecoder(string $format, array $context): DecoderInterface foreach ($this->decoders as $i => $decoder) { if ($decoder->supportsDecoding($format, $context)) { - if (true !== $hasContext) { + if ($hasContext) { // cache decoder if no dynamic context is given $this->decoderByFormat[$format] = $i; } diff --git a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php index 58c559f5ffe99..7d0e644725022 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php @@ -81,7 +81,7 @@ public function getEncoder(string $format, array $context): EncoderInterface { $hasContext = 0 < \count($context); if ( - true !== $hasContext + $hasContext && isset($this->encoderByFormat[$format]) && isset($this->encoders[$this->encoderByFormat[$format]]) ) { @@ -90,7 +90,7 @@ public function getEncoder(string $format, array $context): EncoderInterface foreach ($this->encoders as $i => $encoder) { if ($encoder->supportsEncoding($format, $context)) { - if (true !== $hasContext) { + if ($hasContext) { // cache encoder if no dynamic context is given $this->encoderByFormat[$format] = $i; } From 25cefc27843e560f376afb3ee85265dbc60a375c Mon Sep 17 00:00:00 2001 From: Axel Guckelsberger Date: Thu, 28 Oct 2021 18:34:24 +0200 Subject: [PATCH 4/5] invert logic --- src/Symfony/Component/Serializer/Encoder/ChainDecoder.php | 4 ++-- src/Symfony/Component/Serializer/Encoder/ChainEncoder.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php index 23cc4d3a1e168..89f3ac94ea919 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php @@ -63,7 +63,7 @@ public function getDecoder(string $format, array $context): DecoderInterface { $hasContext = 0 < \count($context); if ( - $hasContext + !$hasContext && isset($this->decoderByFormat[$format]) && isset($this->decoders[$this->decoderByFormat[$format]]) ) { @@ -72,7 +72,7 @@ public function getDecoder(string $format, array $context): DecoderInterface foreach ($this->decoders as $i => $decoder) { if ($decoder->supportsDecoding($format, $context)) { - if ($hasContext) { + if (!$hasContext) { // cache decoder if no dynamic context is given $this->decoderByFormat[$format] = $i; } diff --git a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php index 7d0e644725022..59508f8713d90 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php @@ -81,7 +81,7 @@ public function getEncoder(string $format, array $context): EncoderInterface { $hasContext = 0 < \count($context); if ( - $hasContext + !$hasContext && isset($this->encoderByFormat[$format]) && isset($this->encoders[$this->encoderByFormat[$format]]) ) { @@ -90,7 +90,7 @@ public function getEncoder(string $format, array $context): EncoderInterface foreach ($this->encoders as $i => $encoder) { if ($encoder->supportsEncoding($format, $context)) { - if ($hasContext) { + if (!$hasContext) { // cache encoder if no dynamic context is given $this->encoderByFormat[$format] = $i; } From 59ba9dda2d2944c0f9b4ec140574f9071ab7d45a Mon Sep 17 00:00:00 2001 From: Axel Guckelsberger Date: Thu, 28 Oct 2021 19:29:03 +0200 Subject: [PATCH 5/5] update tests --- .../Component/Serializer/Encoder/ChainDecoder.php | 2 +- .../Component/Serializer/Encoder/ChainEncoder.php | 2 +- .../Serializer/Tests/Encoder/ChainDecoderTest.php | 15 +++++++++++---- .../Serializer/Tests/Encoder/ChainEncoderTest.php | 15 +++++++++++---- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php index 89f3ac94ea919..747c70dfb4fa0 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainDecoder.php @@ -59,7 +59,7 @@ public function supportsDecoding($format, array $context = []): bool * * @throws RuntimeException if no decoder is found */ - public function getDecoder(string $format, array $context): DecoderInterface + private function getDecoder(string $format, array $context): DecoderInterface { $hasContext = 0 < \count($context); if ( diff --git a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php index 59508f8713d90..23c159ce65777 100644 --- a/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php +++ b/src/Symfony/Component/Serializer/Encoder/ChainEncoder.php @@ -77,7 +77,7 @@ public function needsNormalization(string $format, array $context = []): bool * * @throws RuntimeException if no encoder is found */ - public function getEncoder(string $format, array $context): EncoderInterface + private function getEncoder(string $format, array $context): EncoderInterface { $hasContext = 0 < \count($context); if ( diff --git a/src/Symfony/Component/Serializer/Tests/Encoder/ChainDecoderTest.php b/src/Symfony/Component/Serializer/Tests/Encoder/ChainDecoderTest.php index a75a1e55410da..847526990ebb3 100644 --- a/src/Symfony/Component/Serializer/Tests/Encoder/ChainDecoderTest.php +++ b/src/Symfony/Component/Serializer/Tests/Encoder/ChainDecoderTest.php @@ -55,19 +55,26 @@ protected function setUp(): void public function testSupportsDecoding() { + $this->decoder1 + ->method('decode') + ->willReturn('result1'); + $this->decoder2 + ->method('decode') + ->willReturn('result2'); + $this->assertTrue($this->chainDecoder->supportsDecoding(self::FORMAT_1)); - $this->assertSame($this->decoder1, $this->chainDecoder->getDecoder(self::FORMAT_1, [])); + $this->assertEquals('result1', $this->chainDecoder->decode('', self::FORMAT_1, [])); $this->assertTrue($this->chainDecoder->supportsDecoding(self::FORMAT_2)); - $this->assertSame($this->decoder2, $this->chainDecoder->getDecoder(self::FORMAT_2, [])); + $this->assertEquals('result2', $this->chainDecoder->decode('', self::FORMAT_2, [])); $this->assertFalse($this->chainDecoder->supportsDecoding(self::FORMAT_3)); $this->assertTrue($this->chainDecoder->supportsDecoding(self::FORMAT_3, ['foo' => 'bar'])); - $this->assertSame($this->decoder1, $this->chainDecoder->getDecoder(self::FORMAT_3, ['foo' => 'bar'])); + $this->assertEquals('result1', $this->chainDecoder->decode('', self::FORMAT_3, ['foo' => 'bar'])); $this->assertTrue($this->chainDecoder->supportsDecoding(self::FORMAT_3, ['foo' => 'bar2'])); - $this->assertSame($this->decoder2, $this->chainDecoder->getDecoder(self::FORMAT_3, ['foo' => 'bar2'])); + $this->assertEquals('result2', $this->chainDecoder->decode('', self::FORMAT_3, ['foo' => 'bar2'])); } public function testDecode() diff --git a/src/Symfony/Component/Serializer/Tests/Encoder/ChainEncoderTest.php b/src/Symfony/Component/Serializer/Tests/Encoder/ChainEncoderTest.php index 9bdc650b5a0aa..612397b54d9be 100644 --- a/src/Symfony/Component/Serializer/Tests/Encoder/ChainEncoderTest.php +++ b/src/Symfony/Component/Serializer/Tests/Encoder/ChainEncoderTest.php @@ -56,19 +56,26 @@ protected function setUp(): void public function testSupportsEncoding() { + $this->encoder1 + ->method('encode') + ->willReturn('result1'); + $this->encoder2 + ->method('encode') + ->willReturn('result2'); + $this->assertTrue($this->chainEncoder->supportsEncoding(self::FORMAT_1)); - $this->assertSame($this->encoder1, $this->chainEncoder->getEncoder(self::FORMAT_1, [])); + $this->assertEquals('result1', $this->chainEncoder->encode('', self::FORMAT_1, [])); $this->assertTrue($this->chainEncoder->supportsEncoding(self::FORMAT_2)); - $this->assertSame($this->encoder2, $this->chainEncoder->getEncoder(self::FORMAT_2, [])); + $this->assertEquals('result2', $this->chainEncoder->encode('', self::FORMAT_2, [])); $this->assertFalse($this->chainEncoder->supportsEncoding(self::FORMAT_3)); $this->assertTrue($this->chainEncoder->supportsEncoding(self::FORMAT_3, ['foo' => 'bar'])); - $this->assertSame($this->encoder1, $this->chainEncoder->getEncoder(self::FORMAT_3, ['foo' => 'bar'])); + $this->assertEquals('result1', $this->chainEncoder->encode('', self::FORMAT_3, ['foo' => 'bar'])); $this->assertTrue($this->chainEncoder->supportsEncoding(self::FORMAT_3, ['foo' => 'bar2'])); - $this->assertSame($this->encoder2, $this->chainEncoder->getEncoder(self::FORMAT_3, ['foo' => 'bar2'])); + $this->assertEquals('result2', $this->chainEncoder->encode('', self::FORMAT_3, ['foo' => 'bar2'])); } public function testEncode()