feature #27715 [Serializer] Deprecate CsvEncoder as_collection false default value (ogizanagi)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Serializer] Deprecate CsvEncoder as_collection false default value

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

As already expressed in https://github.com/symfony/symfony/pull/25369 and related issues, this behavior is quite counter-intuitive. It may be fine for write-API with a single document in the body but I think such CSV APIs are way less common than file-based ones, expecting collections. So I think this behavior should be opt-in explicitly in required cases, always dealing with collections by default.
This is still an arbitrary decision, but trying to make it based on use-cases and user's experience with CSV.

Note: perhaps we could find a better name for this as the semantic of setting `as_collection` to `false` to get the single row directly would not be really obvious.
Also, it could throw an exception when getting multiple rows where only one was expected.

Commits
-------

bce59c8427 [Serializer] Deprecate CsvEncoder as_collection false default value
This commit is contained in:
Fabien Potencier 2018-06-30 11:26:58 +02:00
commit dd6ef5bee5
3 changed files with 36 additions and 8 deletions

View File

@ -53,6 +53,12 @@ SecurityBundle
the token classes is deprecated. To use the token classes is deprecated. To use
custom tokens extend the existing AnonymousToken and RememberMeToken. custom tokens extend the existing AnonymousToken and RememberMeToken.
Serializer
----------
* Relying on the default value (false) of the "as_collection" option is deprecated since 4.2.
You should set it to false explicitly instead as true will be the default value in 5.0.
DoctrineBridge DoctrineBridge
-------------- --------------

View File

@ -166,6 +166,10 @@ class CsvEncoder implements EncoderInterface, DecoderInterface
return $result; return $result;
} }
if (!isset($context['as_collection'])) {
@trigger_error('Relying on the default value (false) of the "as_collection" option is deprecated since 4.2. You should set it to false explicitly instead as true will be the default value in 5.0.', E_USER_DEPRECATED);
}
// If there is only one data line in the document, return it (the line), the result is not considered as a collection // If there is only one data line in the document, return it (the line), the result is not considered as a collection
return $result[0]; return $result[0];
} }

View File

@ -282,7 +282,11 @@ CSV
$this->assertFalse($this->encoder->supportsDecoding('foo')); $this->assertFalse($this->encoder->supportsDecoding('foo'));
} }
public function testDecode() /**
* @group legacy
* @expectedDeprecation Relying on the default value (false) of the "as_collection" option is deprecated since 4.2. You should set it to false explicitly instead as true will be the default value in 5.0.
*/
public function testDecodeLegacy()
{ {
$expected = array('foo' => 'a', 'bar' => 'b'); $expected = array('foo' => 'a', 'bar' => 'b');
@ -293,6 +297,17 @@ CSV
, 'csv')); , 'csv'));
} }
public function testDecodeAsSingle()
{
$expected = array('foo' => 'a', 'bar' => 'b');
$this->assertEquals($expected, $this->encoder->decode(<<<'CSV'
foo,bar
a,b
CSV
, 'csv', array(CsvEncoder::AS_COLLECTION_KEY => false)));
}
public function testDecodeCollection() public function testDecodeCollection()
{ {
$expected = array( $expected = array(
@ -311,10 +326,8 @@ CSV
, 'csv')); , 'csv'));
} }
public function testDecodeOnlyOneAsCollection() public function testDecode()
{ {
$this->encoder = new CsvEncoder(',', '"', '\\', '.');
$expected = array( $expected = array(
array('foo' => 'a'), array('foo' => 'a'),
); );
@ -324,7 +337,9 @@ foo
a a
CSV CSV
, 'csv', array(CsvEncoder::AS_COLLECTION_KEY => true))); , 'csv', array(
CsvEncoder::AS_COLLECTION_KEY => true, // Can be removed in 5.0
)));
} }
public function testDecodeToManyRelation() public function testDecodeToManyRelation()
@ -365,17 +380,19 @@ CSV
{ {
$this->encoder = new CsvEncoder(';', "'", '|', '-'); $this->encoder = new CsvEncoder(';', "'", '|', '-');
$expected = array('a' => 'hell\'o', 'bar' => array('baz' => 'b')); $expected = array(array('a' => 'hell\'o', 'bar' => array('baz' => 'b')));
$this->assertEquals($expected, $this->encoder->decode(<<<'CSV' $this->assertEquals($expected, $this->encoder->decode(<<<'CSV'
a;bar-baz a;bar-baz
'hell''o';b;c 'hell''o';b;c
CSV CSV
, 'csv')); , 'csv', array(
CsvEncoder::AS_COLLECTION_KEY => true, // Can be removed in 5.0
)));
} }
public function testDecodeCustomSettingsPassedInContext() public function testDecodeCustomSettingsPassedInContext()
{ {
$expected = array('a' => 'hell\'o', 'bar' => array('baz' => 'b')); $expected = array(array('a' => 'hell\'o', 'bar' => array('baz' => 'b')));
$this->assertEquals($expected, $this->encoder->decode(<<<'CSV' $this->assertEquals($expected, $this->encoder->decode(<<<'CSV'
a;bar-baz a;bar-baz
'hell''o';b;c 'hell''o';b;c
@ -385,6 +402,7 @@ CSV
CsvEncoder::ENCLOSURE_KEY => "'", CsvEncoder::ENCLOSURE_KEY => "'",
CsvEncoder::ESCAPE_CHAR_KEY => '|', CsvEncoder::ESCAPE_CHAR_KEY => '|',
CsvEncoder::KEY_SEPARATOR_KEY => '-', CsvEncoder::KEY_SEPARATOR_KEY => '-',
CsvEncoder::AS_COLLECTION_KEY => true, // Can be removed in 5.0
))); )));
} }