Fix BC break with skeleton for Translator Service #5689

Merged
merged 6 commits into from Jan 6, 2014

Projects

None yet

5 participants

@weierophinney
Member
  • Mvc\TranslatorServiceFactory was missing the "implements FactoryInterface" declaration, making it fail as a valid factory.
  • Just fixing the factory did not work, however, as a circular dependency was occurring, as changes in TranslatorServiceFactory meant it was looking for existence of an alias already defined in the skeleton, and pointing back at the same service.

What I did was:

  • Add a check for a Zend\I18n\Translator\TranslatorInterface service in the factory; if it exists, it is pulled and pushed into a Zend\Mvc\I18n\Translator instance, and that instance is returned.
  • If a translator key is present in configuration, and non-empty, that is passed to Zend\I18n\Translator\Translator::factory, and the result passed to a Zend\Mvc\I18n\Translator instance, which is returned.
  • Finally, if none of the above are true, a DummyTranslator instance is created, passed to a Zend\Mvc\I18n\Translator instance, and that instance is returned.

This addresses all BC breaks with the existing skeleton.

Fixes #5684 .

weierophinney added some commits Jan 6, 2014
@weierophinney weierophinney Fixes BC break with existing skeleton
- Mvc\TranslatorServiceFactory was missing the "implements FactoryInterface"
  declaration, making it fail as a valid factory.
- Circular dependency was occurring, as changes in TranslatorServiceFactory
  meant it was looking for existence of an alias already defined in the
  skeleton, and pointing back at the same service. Added a
  'Zend\I18n\Translator\TranslatorInterface' service, pointing to the I18n
  TranslatorServiceFactory, and now check for that in the MVC
  TranslatorServiceFactory. This fixes the primary BC issue.
- Updated View\HelperPluginManager to look for (and conditionally fetch) the new
  'Zend\I18n\Translator\TranslatorInterface' service, instead of the
  'translator' service. This will preserve the intent. Developers may alias
  'translator' to 'Zend\I18n\Translator\TranslatorInterface' if desired.
5d1189c
@weierophinney weierophinney Remove service from ServiceListenerFactory
- Point of original PR that broke BC was to remove the requirement of
  ext-intl by default. Adding the TranslatorInterface as a service by
  default breaks this. Removed, and we can now document it.
276c8dd
@weierophinney weierophinney Added note to README
- Covers #5406, and the changes also present in this PR.
621bfca
@weierophinney weierophinney More BC fixes
- Check for each of MvcTranslator, Translator, and TranslatorInterface
  in order to perform injections.
- Since "Translator" is usually an alias of "MvcTranslator", check this last.
16c8786
weierophinney added some commits Jan 6, 2014
@weierophinney weierophinney Retain BC in all situations
- If no `TranslatorInterface` service is found, check for "translator"
  configuration, and use it to create a
  `Zend\I18n\Translator\Translator` instance, per original behavior.
- **Always** wrap a created translator instance in a
  `Zend\Mvc\I18n\Translator` instance.
175b4b7
@weierophinney weierophinney Remove Validator\TranslatorInterface implementation
- No longer needed, as the MVC translator service factory now wraps a
  given translator in an Mvc\I18n\Translator instance.
- Also, CS fixes
0c8e358
@weierophinney
Member

Okay, ready to merge now!

@DASPRiD DASPRiD was assigned Jan 6, 2014
@DASPRiD DASPRiD merged commit 0c8e358 into zendframework:develop Jan 6, 2014
@coolmic
Contributor
coolmic commented Jan 6, 2014

I preferred the behavious of DASPRID PR ( #5406 )

Before his PR :

I have like 10+ modules, with translation files for each one.

array(
    'translator' => array(
        'translation_file_patterns' => array(
            array(
                'text_domain' => 'ice-admin-auth',
                'type' => 'phparray',
                'base_dir' => dirname(__DIR__) . '/i18n',
                'pattern' => '%s.php'
            )
        )
    )
)

Translation files were loaded even if I didn't use translator.

With his PR :

None of my translation files are loaded if I didn't define 'translator' in the service manager.

Now with this PR, all my translation file are loaded, or I have to remove 'translator' entry in each module config.

@prendit
prendit commented Jan 7, 2014

The new Zend\Mvc\I18n\Translator (MvcTranslator) is lacking some public methods of the old Zend\I18n\Translator\Translator which it previously extended. The MvcTranslator also does not allow access to the protected $translator which makes it impossible now to change the locale on the fly for example.

Will the MvcTranslator be extended before 2.3 goes to master, or we have to extend it in the future, so we can use those public functions?

@aimfeld
Contributor
aimfeld commented Mar 13, 2014

I had to account for this minor BC break in my custom Translator extension when updating from ZF 2.2.5 to 2.3.0. I now extend my custom Translator from Zend\I18n\Translator\Translator instead of Zend\Mvc\I18n\Translator and implement Zend\Validator\Translator\TranslatorInterface
The following worked for me:

namespace MyLib\I18n\Translator;

use Zend\Validator\Translator\TranslatorInterface as ValidatorTranslatorInterface;

class Translator extends \Zend\I18n\Translator\Translator implements ValidatorTranslatorInterface
{
    // ...
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment