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

feature #15562 [translation] Deprecated DiffOperation (zerustech)

Browse Source 
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
This commit is contained in:
Abdellatif Ait boudad 2015-09-05 11:37:34 +00:00
commit be4716505f
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);
}
}