Skip to content

Commit be47165

Browse files
aitboudadaitboudad
committed
feature #15562 [translation] Deprecated DiffOperation (zerustech)
This PR was merged into the 2.8 branch. Discussion ---------- [translation] Deprecated DiffOperation ## Summary: The ``DiffOperation`` class has been deprecated and ``TargetOperation`` should be used instead, because ``DiffOperation`` has nothing to do with 'diff', thus its class name is misleading. Also added detailed documents for all operation interface and classes. ## Background: The following names should have consistent meanings for all operations: The name of ``intersection`` is temporarily introduced here to explain this issue. * [x] ``intersection`` = source ∩ target = {x: x ∈ source ∧ x ∈ target} * [x] ``all`` = **result of the operation, depends on the operation.** * [x] ``new`` = all ∖ source = {x: x ∈ all ∧ x ∉ source} * [x] ``obsolete`` = source ∖ all = {x: x ∈ source ∧ x ∉ all} The following analysis explains why ``DiffOperation`` should be deprecated. ## Logic of ``MergeOperation``: * [x] ``all`` = source ∪ target = {x: x ∈ source ∨ x ∈ target} * [x] ``new`` = all ∖ source = {x: x ∈ target ∧ ∉ source} * [x] ``obsolete`` = source ∖ all = {x: x ∈ source ∧ x ∉ source ∧ x ∉ target} = ∅ This absolutely makes sense. ## Logic of ``DiffOperation``: * [ ] ``all`` = intersection ∪ (target ∖ intersection) = target * [x] ``new`` = all ∖ source = {x: x ∈ target ∧ x ∉ source} * [x] ``obsolete`` = source ∖ all = source ∖ target = {x: x ∈ source ∧ x ∉ target} The ``all`` part is confusing because 'diff' should either mean 'relative complement' or 'symmetric difference' operation: ### Relative Complement: * ``all`` = source ∖ target = {x: x ∈ source ∧ x ∉ target} ### Symmetric Difference: * ``all`` = (source ∖ target) ∪ (target ∖ source) = {x: x ∈ source ∧ x ∉ target ∨ x ∈ target ∧ x ∉ source} ### Current Logic has Nothing to do with "Diff": * ``all`` = intersection ∪ (target ∖ intersection) = target So the name of ``DiffOperation`` is misleading and inappropriate. Unfortunately, there is no corresponding set operation for this class, so it's hard to give it an apppriate name. From my point of view, I believe the most accurate name for this class should be ``TargetOperation`` because its result is same as the target set. | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Commits ------- 353c94d [translation][framework-bundle] Deprecated DiffOperation
2 parents 22b17b2 + 353c94d commit be47165

File tree

10 files changed

+233
-111
lines changed

10 files changed

+233
-111
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
namespace Symfony\Bundle\FrameworkBundle\Command;
1313

1414
use Symfony\Component\Console\Style\SymfonyStyle;
15-
use Symfony\Component\Translation\Catalogue\DiffOperation;
15+
use Symfony\Component\Translation\Catalogue\TargetOperation;
1616
use Symfony\Component\Translation\Catalogue\MergeOperation;
1717
use Symfony\Component\Console\Input\InputInterface;
1818
use Symfony\Component\Console\Output\OutputInterface;
@@ -139,7 +139,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
139139

140140
// process catalogues
141141
$operation = $input->getOption('clean')
142-
? new DiffOperation($currentCatalogue, $extractedCatalogue)
142+
? new TargetOperation($currentCatalogue, $extractedCatalogue)
143143
: new MergeOperation($currentCatalogue, $extractedCatalogue);
144144

145145
// Exit if no messages found.

‎src/Symfony/Bundle/FrameworkBundle/composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
"symfony/security-csrf": "~2.6|~3.0.0",
3030
"symfony/stopwatch": "~2.3|~3.0.0",
3131
"symfony/templating": "~2.1|~3.0.0",
32-
"symfony/translation": "~2.7",
32+
"symfony/translation": "~2.8",
3333
"doctrine/annotations": "~1.0"
3434
},
3535
"require-dev": {

‎src/Symfony/Component/Translation/CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ CHANGELOG
66

77
* deprecated Translator::getMessages(), use TranslatorBagInterface::getCatalogue() instead.
88
* added options 'as_tree', 'inline' to YamlFileDumper
9+
* [DEPRECATION] The `DiffOperation` class has been deprecated and
10+
will be removed in Symfony 3.0, since its operation has nothing to do with 'diff',
11+
so the class name is misleading. The `TargetOperation` class should be used for
12+
this use-case instead.
13+
914

1015
2.7.0
1116
-----

‎src/Symfony/Component/Translation/Catalogue/AbstractOperation.php‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,38 +17,60 @@
1717
/**
1818
* Base catalogues binary operation class.
1919
*
20+
* A catalogue binary operation performs operation on
21+
* source (the left argument) and target (the right argument) catalogues.
22+
*
2023
* @author Jean-François Simon <contact@jfsimon.fr>
2124
*/
2225
abstract class AbstractOperation implements OperationInterface
2326
{
2427
/**
25-
* @var MessageCatalogueInterface
28+
* @var MessageCatalogueInterface The source catalogue
2629
*/
2730
protected $source;
2831

2932
/**
30-
* @var MessageCatalogueInterface
33+
* @var MessageCatalogueInterface The target catalogue
3134
*/
3235
protected $target;
3336

3437
/**
35-
* @var MessageCatalogue
38+
* @var MessageCatalogue The result catalogue
3639
*/
3740
protected $result;
3841

3942
/**
40-
* @var null|array
43+
* @var null|array The domains affected by this operation
4144
*/
4245
private $domains;
4346

4447
/**
45-
* @var array
48+
* This array stores 'all', 'new' and 'obsolete' messages for all valid domains.
49+
*
50+
* The data structure of this array is as follows:
51+
* ```php
52+
* array(
53+
* 'domain 1' => array(
54+
* 'all' => array(...),
55+
* 'new' => array(...),
56+
* 'obsolete' => array(...)
57+
* ),
58+
* 'domain 2' => array(
59+
* 'all' => array(...),
60+
* 'new' => array(...),
61+
* 'obsolete' => array(...)
62+
* ),
63+
* ...
64+
* )
65+
* ```
66+
*
67+
* @var array The array that stores 'all', 'new' and 'obsolete' messages
4668
*/
4769
protected $messages;
4870

4971
/**
50-
* @param MessageCatalogueInterface $source
51-
* @param MessageCatalogueInterface $target
72+
* @param MessageCatalogueInterface $source The source catalogue
73+
* @param MessageCatalogueInterface $target The target catalogue
5274
*
5375
* @throws \LogicException
5476
*/
@@ -140,7 +162,10 @@ public function getResult()
140162
}
141163

142164
/**
143-
* @param string $domain
165+
* Performs operation on source and target catalogues for the given domain and
166+
* stores the results.
167+
*
168+
* @param string $domain The domain which the operation will be performed for
144169
*/
145170
abstract protected function processDomain($domain);
146171
}

‎src/Symfony/Component/Translation/Catalogue/DiffOperation.php‎

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,45 +11,23 @@
1111

1212
namespace Symfony\Component\Translation\Catalogue;
1313

14+
@trigger_error('The '.__NAMESPACE__.'\DiffOperation class is deprecated since version 2.8 and will be removed in 3.0. Use the TargetOperation class in the same namespace instead.', E_USER_DEPRECATED);
15+
1416
/**
1517
* Diff operation between two catalogues.
1618
*
19+
* The name of 'Diff' is misleading because the operation
20+
* has nothing to do with diff:
21+
*
22+
* intersection = source ∩ target = {x: x ∈ source ∧ x ∈ target}
23+
* all = intersection ∪ (target ∖ intersection) = target
24+
* new = all ∖ source = {x: x ∈ target ∧ x ∉ source}
25+
* obsolete = source ∖ all = source ∖ target = {x: x ∈ source ∧ x ∉ target}
26+
*
1727
* @author Jean-François Simon <contact@jfsimon.fr>
28+
*
29+
* @deprecated since version 2.8, to be removed in 3.0. Use TargetOperation instead.
1830
*/
19-
class DiffOperation extends AbstractOperation
31+
class DiffOperation extends TargetOperation
2032
{
21-
/**
22-
* {@inheritdoc}
23-
*/
24-
protected function processDomain($domain)
25-
{
26-
$this->messages[$domain] = array(
27-
'all' => array(),
28-
'new' => array(),
29-
'obsolete' => array(),
30-
);
31-
32-
foreach ($this->source->all($domain) as $id => $message) {
33-
if ($this->target->has($id, $domain)) {
34-
$this->messages[$domain]['all'][$id] = $message;
35-
$this->result->add(array($id => $message), $domain);
36-
if (null !== $keyMetadata = $this->source->getMetadata($id, $domain)) {
37-
$this->result->setMetadata($id, $keyMetadata, $domain);
38-
}
39-
} else {
40-
$this->messages[$domain]['obsolete'][$id] = $message;
41-
}
42-
}
43-
44-
foreach ($this->target->all($domain) as $id => $message) {
45-
if (!$this->source->has($id, $domain)) {
46-
$this->messages[$domain]['all'][$id] = $message;
47-
$this->messages[$domain]['new'][$id] = $message;
48-
$this->result->add(array($id => $message), $domain);
49-
if (null !== $keyMetadata = $this->target->getMetadata($id, $domain)) {
50-
$this->result->setMetadata($id, $keyMetadata, $domain);
51-
}
52-
}
53-
}
54-
}
5533
}

‎src/Symfony/Component/Translation/Catalogue/MergeOperation.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@
1212
namespace Symfony\Component\Translation\Catalogue;
1313

1414
/**
15-
* Merge operation between two catalogues.
15+
* Merge operation between two catalogues as follows:
16+
* all = source ∪ target = {x: x ∈ source ∨ x ∈ target}
17+
* new = all ∖ source = {x: x ∈ target ∧ x ∉ source}
18+
* obsolete = source ∖ all = {x: x ∈ source ∧ x ∉ source ∧ x ∉ target} = ∅
19+
* Basically, the result contains messages from both catalogues.
1620
*
1721
* @author Jean-François Simon <contact@jfsimon.fr>
1822
*/

‎src/Symfony/Component/Translation/Catalogue/OperationInterface.php‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,20 @@
1616
/**
1717
* Represents an operation on catalogue(s).
1818
*
19+
* An instance of this interface performs an operation on one or more catalogues and
20+
* stores intermediate and final results of the operation.
21+
*
22+
* The first catalogue in its argument(s) is called the 'source catalogue' or 'source' and
23+
* the following results are stored:
24+
*
25+
* Messages: also called 'all', are valid messages for the given domain after the operation is performed.
26+
*
27+
* New Messages: also called 'new' (new = all ∖ source = {x: x ∈ all ∧ x ∉ source}).
28+
*
29+
* Obsolete Messages: also called 'obsolete' (obsolete = source ∖ all = {x: x ∈ source ∧ x ∉ all}).
30+
*
31+
* Result: also called 'result', is the resulting catalogue for the given domain that holds the same messages as 'all'.
32+
*
1933
* @author Jean-François Simon <jeanfrancois.simon@sensiolabs.com>
2034
*/
2135
interface OperationInterface
@@ -28,7 +42,7 @@ interface OperationInterface
2842
public function getDomains();
2943

3044
/**
31-
* Returns all valid messages after operation.
45+
* Returns all valid messages ('all') after operation.
3246
*
3347
* @param string $domain
3448
*
@@ -37,7 +51,7 @@ public function getDomains();
3751
public function getMessages($domain);
3852

3953
/**
40-
* Returns new messages after operation.
54+
* Returns new messages ('new') after operation.
4155
*
4256
* @param string $domain
4357
*
@@ -46,7 +60,7 @@ public function getMessages($domain);
4660
public function getNewMessages($domain);
4761

4862
/**
49-
* Returns obsolete messages after operation.
63+
* Returns obsolete messages ('obsolete') after operation.
5064
*
5165
* @param string $domain
5266
*
@@ -55,7 +69,7 @@ public function getNewMessages($domain);
5569
public function getObsoleteMessages($domain);
5670

5771
/**
58-
* Returns resulting catalogue.
72+
* Returns resulting catalogue ('result').
5973
*
6074
* @return MessageCatalogueInterface
6175
*/

‎src/Symfony/Component/Translation/Catalogue/TargetOperation.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Translation\Catalogue;
13+
14+
/**
15+
* Target operation between two catalogues:
16+
* intersection = source ∩ target = {x: x ∈ source ∧ x ∈ target}
17+
* all = intersection ∪ (target ∖ intersection) = target
18+
* new = all ∖ source = {x: x ∈ target ∧ x ∉ source}
19+
* obsolete = source ∖ all = source ∖ target = {x: x ∈ source ∧ x ∉ target}
20+
* Basically, the result contains messages from the target catalogue.
21+
*
22+
* @author Michael Lee <michael.lee@zerustech.com>
23+
*/
24+
class TargetOperation extends AbstractOperation
25+
{
26+
/**
27+
* {@inheritdoc}
28+
*/
29+
protected function processDomain($domain)
30+
{
31+
$this->messages[$domain] = array(
32+
'all' => array(),
33+
'new' => array(),
34+
'obsolete' => array(),
35+
);
36+
37+
// For 'all' messages, the code can't be simplified as ``$this->messages[$domain]['all'] = $target->all($domain);``,
38+
// because doing so will drop messages like {x: x ∈ source ∧ x ∉ target.all ∧ x ∈ target.fallback}
39+
//
40+
// For 'new' messages, the code can't be simplied as ``array_diff_assoc($this->target->all($domain), $this->source->all($domain));``
41+
// because doing so will not exclude messages like {x: x ∈ target ∧ x ∉ source.all ∧ x ∈ source.fallback}
42+
//
43+
// For 'obsolete' messages, the code can't be simplifed as ``array_diff_assoc($this->source->all($domain), $this->target->all($domain))``
44+
// because doing so will not exclude messages like {x: x ∈ source ∧ x ∉ target.all ∧ x ∈ target.fallback}
45+
46+
foreach ($this->source->all($domain) as $id => $message) {
47+
if ($this->target->has($id, $domain)) {
48+
$this->messages[$domain]['all'][$id] = $message;
49+
$this->result->add(array($id => $message), $domain);
50+
if (null !== $keyMetadata = $this->source->getMetadata($id, $domain)) {
51+
$this->result->setMetadata($id, $keyMetadata, $domain);
52+
}
53+
} else {
54+
$this->messages[$domain]['obsolete'][$id] = $message;
55+
}
56+
}
57+
58+
foreach ($this->target->all($domain) as $id => $message) {
59+
if (!$this->source->has($id, $domain)) {
60+
$this->messages[$domain]['all'][$id] = $message;
61+
$this->messages[$domain]['new'][$id] = $message;
62+
$this->result->add(array($id => $message), $domain);
63+
if (null !== $keyMetadata = $this->target->getMetadata($id, $domain)) {
64+
$this->result->setMetadata($id, $keyMetadata, $domain);
65+
}
66+
}
67+
}
68+
}
69+
}

‎src/Symfony/Component/Translation/Tests/Catalogue/DiffOperationTest.php‎

Lines changed: 4 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -12,69 +12,13 @@
1212
namespace Symfony\Component\Translation\Tests\Catalogue;
1313

1414
use Symfony\Component\Translation\Catalogue\DiffOperation;
15-
use Symfony\Component\Translation\MessageCatalogue;
1615
use Symfony\Component\Translation\MessageCatalogueInterface;
1716

18-
class DiffOperationTest extends AbstractOperationTest
17+
/**
18+
* @group legacy
19+
*/
20+
class DiffOperationTest extends TargetOperationTest
1921
{
20-
public function testGetMessagesFromSingleDomain()
21-
{
22-
$operation = $this->createOperation(
23-
new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b'))),
24-
new MessageCatalogue('en', array('messages' => array('a' => 'new_a', 'c' => 'new_c')))
25-
);
26-
27-
$this->assertEquals(
28-
array('a' => 'old_a', 'c' => 'new_c'),
29-
$operation->getMessages('messages')
30-
);
31-
32-
$this->assertEquals(
33-
array('c' => 'new_c'),
34-
$operation->getNewMessages('messages')
35-
);
36-
37-
$this->assertEquals(
38-
array('b' => 'old_b'),
39-
$operation->getObsoleteMessages('messages')
40-
);
41-
}
42-
43-
public function testGetResultFromSingleDomain()
44-
{
45-
$this->assertEquals(
46-
new MessageCatalogue('en', array(
47-
'messages' => array('a' => 'old_a', 'c' => 'new_c'),
48-
)),
49-
$this->createOperation(
50-
new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b'))),
51-
new MessageCatalogue('en', array('messages' => array('a' => 'new_a', 'c' => 'new_c')))
52-
)->getResult()
53-
);
54-
}
55-
56-
public function testGetResultWithMetadata()
57-
{
58-
$leftCatalogue = new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b')));
59-
$leftCatalogue->setMetadata('a', 'foo', 'messages');
60-
$leftCatalogue->setMetadata('b', 'bar', 'messages');
61-
$rightCatalogue = new MessageCatalogue('en', array('messages' => array('b' => 'new_b', 'c' => 'new_c')));
62-
$rightCatalogue->setMetadata('b', 'baz', 'messages');
63-
$rightCatalogue->setMetadata('c', 'qux', 'messages');
64-
65-
$diffCatalogue = new MessageCatalogue('en', array('messages' => array('b' => 'old_b', 'c' => 'new_c')));
66-
$diffCatalogue->setMetadata('b', 'bar', 'messages');
67-
$diffCatalogue->setMetadata('c', 'qux', 'messages');
68-
69-
$this->assertEquals(
70-
$diffCatalogue,
71-
$this->createOperation(
72-
$leftCatalogue,
73-
$rightCatalogue
74-
)->getResult()
75-
);
76-
}
77-
7822
protected function createOperation(MessageCatalogueInterface $source, MessageCatalogueInterface $target)
7923
{
8024
return new DiffOperation($source, $target);

0 commit comments

Comments
 (0)