Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Fix BC break with skeleton for Translator Service #5689

Merged

Conversation

weierophinney
Copy link
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 .

- 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.
- 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.
- Covers zendframework#5406, and the changes also present in this PR.
- Check for each of MvcTranslator, Translator, and TranslatorInterface
  in order to perform injections.
- Since "Translator" is usually an alias of "MvcTranslator", check this last.
- 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.
- No longer needed, as the MVC translator service factory now wraps a
  given translator in an Mvc\I18n\Translator instance.
- Also, CS fixes
@weierophinney
Copy link
Member Author

Okay, ready to merge now!

@ghost ghost assigned DASPRiD Jan 6, 2014
@DASPRiD DASPRiD merged commit 0c8e358 into zendframework:develop Jan 6, 2014
@coolmic
Copy link
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
Copy link

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
Copy link
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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants