diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[translation][framework-bundle] Deprecated DiffOperation

Browse Source 
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.

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.

* [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.

* [ ] ``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:

* ``all`` = source ∖ target = {x: x ∈ source ∧ x ∉ target}

* ``all`` = (source ∖ target) ∪ (target ∖ source) = {x: x ∈ source ∧ x ∉ target ∨ x ∈ target ∧ x ∉ source}

* ``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
This commit is contained in:
Michael Lee 2015-08-16 10:22:07 +08:00
parent d76cc03382
commit 353c94db64
10 changed files with 233 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php
View File

@ -12,7 +12,7 @@
namespace Symfony\Bundle\FrameworkBundle\Command;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\Translation\Catalogue\DiffOperation;
use Symfony\Component\Translation\Catalogue\TargetOperation;
use Symfony\Component\Translation\Catalogue\MergeOperation;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
@ -139,7 +139,7 @@ EOF
// process catalogues
$operation = $input->getOption('clean')
? new DiffOperation($currentCatalogue, $extractedCatalogue)
? new TargetOperation($currentCatalogue, $extractedCatalogue)
: new MergeOperation($currentCatalogue, $extractedCatalogue);
// Exit if no messages found.

2
src/Symfony/Bundle/FrameworkBundle/composer.json
View File

@ -29,7 +29,7 @@
"symfony/security-csrf": "~2.6|~3.0.0",
"symfony/stopwatch": "~2.3|~3.0.0",
"symfony/templating": "~2.1|~3.0.0",
"symfony/translation": "~2.7",
"symfony/translation": "~2.8",
"doctrine/annotations": "~1.0"
},
"require-dev": {

5
src/Symfony/Component/Translation/CHANGELOG.md
View File

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

41
src/Symfony/Component/Translation/Catalogue/AbstractOperation.php
View File

@ -17,38 +17,60 @@ use Symfony\Component\Translation\MessageCatalogueInterface;
/**
* Base catalogues binary operation class.
*
* A catalogue binary operation performs operation on
* source (the left argument) and target (the right argument) catalogues.
*
* @author Jean-François Simon <contact@jfsimon.fr>
*/
abstract class AbstractOperation implements OperationInterface
{
/**
* @var MessageCatalogueInterface
* @var MessageCatalogueInterface The source catalogue
*/
protected $source;
/**
* @var MessageCatalogueInterface
* @var MessageCatalogueInterface The target catalogue
*/
protected $target;
/**
* @var MessageCatalogue
* @var MessageCatalogue The result catalogue
*/
protected $result;
/**
* @var null|array
* @var null|array The domains affected by this operation
*/
private $domains;
/**
* @var array
* This array stores 'all', 'new' and 'obsolete' messages for all valid domains.
*
* The data structure of this array is as follows:
* ```php
* array(
* 'domain 1' => array(
* 'all' => array(...),
* 'new' => array(...),
* 'obsolete' => array(...)
* ),
* 'domain 2' => array(
* 'all' => array(...),
* 'new' => array(...),
* 'obsolete' => array(...)
* ),
* ...
* )
* ```
*
* @var array The array that stores 'all', 'new' and 'obsolete' messages
*/
protected $messages;
/**
* @param MessageCatalogueInterface $source
* @param MessageCatalogueInterface $target
* @param MessageCatalogueInterface $source The source catalogue
* @param MessageCatalogueInterface $target The target catalogue
*
* @throws \LogicException
*/
@ -140,7 +162,10 @@ abstract class AbstractOperation implements OperationInterface
}
/**
* @param string $domain
* Performs operation on source and target catalogues for the given domain and
* stores the results.
*
* @param string $domain The domain which the operation will be performed for
*/
abstract protected function processDomain($domain);
}

48
src/Symfony/Component/Translation/Catalogue/DiffOperation.php
View File

@ -11,45 +11,23 @@
namespace Symfony\Component\Translation\Catalogue;
@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);
/**
* Diff operation between two catalogues.
*
* The name of 'Diff' is misleading because the operation
* has nothing to do with diff:
*
* intersection = source ∩ target = {x: x ∈ source ∧ x ∈ target}
* all = intersection ∪ (target ∖ intersection) = target
* new = all ∖ source = {x: x ∈ target ∧ x ∉ source}
* obsolete = source ∖ all = source ∖ target = {x: x ∈ source ∧ x ∉ target}
*
* @author Jean-François Simon <contact@jfsimon.fr>
*
* @deprecated since version 2.8, to be removed in 3.0. Use TargetOperation instead.
*/
class DiffOperation extends AbstractOperation
class DiffOperation extends TargetOperation
{
/**
* {@inheritdoc}
*/
protected function processDomain($domain)
{
$this->messages[$domain] = array(
'all' => array(),
'new' => array(),
'obsolete' => array(),
);
foreach ($this->source->all($domain) as $id => $message) {
if ($this->target->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->result->add(array($id => $message), $domain);
if (null !== $keyMetadata = $this->source->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
} else {
$this->messages[$domain]['obsolete'][$id] = $message;
}
}
foreach ($this->target->all($domain) as $id => $message) {
if (!$this->source->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->messages[$domain]['new'][$id] = $message;
$this->result->add(array($id => $message), $domain);
if (null !== $keyMetadata = $this->target->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
}
}
}
}

6
src/Symfony/Component/Translation/Catalogue/MergeOperation.php
View File

@ -12,7 +12,11 @@
namespace Symfony\Component\Translation\Catalogue;
/**
* Merge operation between two catalogues.
* Merge operation between two catalogues as follows:
* all = source ∪ target = {x: x ∈ source ∨ x ∈ target}
* new = all ∖ source = {x: x ∈ target ∧ x ∉ source}
* obsolete = source ∖ all = {x: x ∈ source ∧ x ∉ source ∧ x ∉ target} = ∅
* Basically, the result contains messages from both catalogues.
*
* @author Jean-François Simon <contact@jfsimon.fr>
*/

22
src/Symfony/Component/Translation/Catalogue/OperationInterface.php
View File

@ -16,6 +16,20 @@ use Symfony\Component\Translation\MessageCatalogueInterface;
/**
* Represents an operation on catalogue(s).
*
* An instance of this interface performs an operation on one or more catalogues and
* stores intermediate and final results of the operation.
*
* The first catalogue in its argument(s) is called the 'source catalogue' or 'source' and
* the following results are stored:
*
* Messages: also called 'all', are valid messages for the given domain after the operation is performed.
*
* New Messages: also called 'new' (new = all ∖ source = {x: x ∈ all ∧ x ∉ source}).
*
* Obsolete Messages: also called 'obsolete' (obsolete = source ∖ all = {x: x ∈ source ∧ x ∉ all}).
*
* Result: also called 'result', is the resulting catalogue for the given domain that holds the same messages as 'all'.
*
* @author Jean-François Simon <jeanfrancois.simon@sensiolabs.com>
*/
interface OperationInterface
@ -28,7 +42,7 @@ interface OperationInterface
public function getDomains();
/**
* Returns all valid messages after operation.
* Returns all valid messages ('all') after operation.
*
* @param string $domain
*
@ -37,7 +51,7 @@ interface OperationInterface
public function getMessages($domain);
/**
* Returns new messages after operation.
* Returns new messages ('new') after operation.
*
* @param string $domain
*
@ -46,7 +60,7 @@ interface OperationInterface
public function getNewMessages($domain);
/**
* Returns obsolete messages after operation.
* Returns obsolete messages ('obsolete') after operation.
*
* @param string $domain
*
@ -55,7 +69,7 @@ interface OperationInterface
public function getObsoleteMessages($domain);
/**
* Returns resulting catalogue.
* Returns resulting catalogue ('result').
*
* @return MessageCatalogueInterface
*/

69
src/Symfony/Component/Translation/Catalogue/TargetOperation.php Normal file
View File

@ -0,0 +1,69 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Translation\Catalogue;
/**
* Target operation between two catalogues:
* intersection = source ∩ target = {x: x ∈ source ∧ x ∈ target}
* all = intersection ∪ (target ∖ intersection) = target
* new = all ∖ source = {x: x ∈ target ∧ x ∉ source}
* obsolete = source ∖ all = source ∖ target = {x: x ∈ source ∧ x ∉ target}
* Basically, the result contains messages from the target catalogue.
*
* @author Michael Lee <michael.lee@zerustech.com>
*/
class TargetOperation extends AbstractOperation
{
/**
* {@inheritdoc}
*/
protected function processDomain($domain)
{
$this->messages[$domain] = array(
'all' => array(),
'new' => array(),
'obsolete' => array(),
);
// For 'all' messages, the code can't be simplified as ``$this->messages[$domain]['all'] = $target->all($domain);``,
// because doing so will drop messages like {x: x ∈ source ∧ x ∉ target.all ∧ x ∈ target.fallback}
//
// For 'new' messages, the code can't be simplied as ``array_diff_assoc($this->target->all($domain), $this->source->all($domain));``
// because doing so will not exclude messages like {x: x ∈ target ∧ x ∉ source.all ∧ x ∈ source.fallback}
//
// For 'obsolete' messages, the code can't be simplifed as ``array_diff_assoc($this->source->all($domain), $this->target->all($domain))``
// because doing so will not exclude messages like {x: x ∈ source ∧ x ∉ target.all ∧ x ∈ target.fallback}
foreach ($this->source->all($domain) as $id => $message) {
if ($this->target->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->result->add(array($id => $message), $domain);
if (null !== $keyMetadata = $this->source->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
} else {
$this->messages[$domain]['obsolete'][$id] = $message;
}
}
foreach ($this->target->all($domain) as $id => $message) {
if (!$this->source->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->messages[$domain]['new'][$id] = $message;
$this->result->add(array($id => $message), $domain);
if (null !== $keyMetadata = $this->target->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
}
}
}
}

64
src/Symfony/Component/Translation/Tests/Catalogue/DiffOperationTest.php
View File

@ -12,69 +12,13 @@
namespace Symfony\Component\Translation\Tests\Catalogue;
use Symfony\Component\Translation\Catalogue\DiffOperation;
use Symfony\Component\Translation\MessageCatalogue;
use Symfony\Component\Translation\MessageCatalogueInterface;
class DiffOperationTest extends AbstractOperationTest
/**
* @group legacy
*/
class DiffOperationTest extends TargetOperationTest
{
public function testGetMessagesFromSingleDomain()
{
$operation = $this->createOperation(
new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b'))),
new MessageCatalogue('en', array('messages' => array('a' => 'new_a', 'c' => 'new_c')))
);
$this->assertEquals(
array('a' => 'old_a', 'c' => 'new_c'),
$operation->getMessages('messages')
);
$this->assertEquals(
array('c' => 'new_c'),
$operation->getNewMessages('messages')
);
$this->assertEquals(
array('b' => 'old_b'),
$operation->getObsoleteMessages('messages')
);
}
public function testGetResultFromSingleDomain()
{
$this->assertEquals(
new MessageCatalogue('en', array(
'messages' => array('a' => 'old_a', 'c' => 'new_c'),
)),
$this->createOperation(
new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b'))),
new MessageCatalogue('en', array('messages' => array('a' => 'new_a', 'c' => 'new_c')))
)->getResult()
);
}
public function testGetResultWithMetadata()
{
$leftCatalogue = new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b')));
$leftCatalogue->setMetadata('a', 'foo', 'messages');
$leftCatalogue->setMetadata('b', 'bar', 'messages');
$rightCatalogue = new MessageCatalogue('en', array('messages' => array('b' => 'new_b', 'c' => 'new_c')));
$rightCatalogue->setMetadata('b', 'baz', 'messages');
$rightCatalogue->setMetadata('c', 'qux', 'messages');
$diffCatalogue = new MessageCatalogue('en', array('messages' => array('b' => 'old_b', 'c' => 'new_c')));
$diffCatalogue->setMetadata('b', 'bar', 'messages');
$diffCatalogue->setMetadata('c', 'qux', 'messages');
$this->assertEquals(
$diffCatalogue,
$this->createOperation(
$leftCatalogue,
$rightCatalogue
)->getResult()
);
}
protected function createOperation(MessageCatalogueInterface $source, MessageCatalogueInterface $target)
{
return new DiffOperation($source, $target);

83
src/Symfony/Component/Translation/Tests/Catalogue/TargetOperationTest.php Normal file
View File

@ -0,0 +1,83 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Translation\Tests\Catalogue;
use Symfony\Component\Translation\Catalogue\TargetOperation;
use Symfony\Component\Translation\MessageCatalogue;
use Symfony\Component\Translation\MessageCatalogueInterface;
class TargetOperationTest extends AbstractOperationTest
{
public function testGetMessagesFromSingleDomain()
{
$operation = $this->createOperation(
new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b'))),
new MessageCatalogue('en', array('messages' => array('a' => 'new_a', 'c' => 'new_c')))
);
$this->assertEquals(
array('a' => 'old_a', 'c' => 'new_c'),
$operation->getMessages('messages')
);
$this->assertEquals(
array('c' => 'new_c'),
$operation->getNewMessages('messages')
);
$this->assertEquals(
array('b' => 'old_b'),
$operation->getObsoleteMessages('messages')
);
}
public function testGetResultFromSingleDomain()
{
$this->assertEquals(
new MessageCatalogue('en', array(
'messages' => array('a' => 'old_a', 'c' => 'new_c'),
)),
$this->createOperation(
new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b'))),
new MessageCatalogue('en', array('messages' => array('a' => 'new_a', 'c' => 'new_c')))
)->getResult()
);
}
public function testGetResultWithMetadata()
{
$leftCatalogue = new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b')));
$leftCatalogue->setMetadata('a', 'foo', 'messages');
$leftCatalogue->setMetadata('b', 'bar', 'messages');
$rightCatalogue = new MessageCatalogue('en', array('messages' => array('b' => 'new_b', 'c' => 'new_c')));
$rightCatalogue->setMetadata('b', 'baz', 'messages');
$rightCatalogue->setMetadata('c', 'qux', 'messages');
$diffCatalogue = new MessageCatalogue('en', array('messages' => array('b' => 'old_b', 'c' => 'new_c')));
$diffCatalogue->setMetadata('b', 'bar', 'messages');
$diffCatalogue->setMetadata('c', 'qux', 'messages');
$this->assertEquals(
$diffCatalogue,
$this->createOperation(
$leftCatalogue,
$rightCatalogue
)->getResult()
);
}
protected function createOperation(MessageCatalogueInterface $source, MessageCatalogueInterface $target)
{
return new TargetOperation($source, $target);
}
}