Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Make include_path functionality of Config and Translator opt-in #4767

Merged
merged 5 commits into from

3 participants

@weierophinney

#4515 (which addresses #4443) and #4574 both introduce features that use PHP's include_path to identify application assets. While the functionality has reasonable use cases, as implemented, it introduces some performance impact, as the components look on the include_path for each and every file. Additionally, if . is not on the include_path, files that resolve from the current working directory will not resolve.

weierophinney added some commits
@weierophinney weierophinney [#4766] wrote tests to indicate behavior is opt-in
- Added `setUseIncludePath()` call to translator adapters
- added flag, Factory::USE_INCLUDE_PATH, as second argument to config
  factory
44b944c
@weierophinney weierophinney [#4766] Config makes include_path usage opt-in only
- You must pass the `$useIncludePath` to either `fromFile()` or
  `fromFiles()` for it to be used. Constants were created with boolean
  values for those using IDEs.
d1904c6
@weierophinney weierophinney [#4766] Make include_path abilities opt-in in Translators
- Created AbstractFileLoader, which provides implementation details for
  when/how to use the include_path to load translation file assets.
- Updated INI, gettext, and PHP loaders to extend AbstractFileLoader.
6511a16
@weierophinney weierophinney [#4766] Add docblock detailing how to set include_path flag
- Added to translator loader plugin manager
5a3d9e7
@weierophinney weierophinney was assigned
@weierophinney

This replaces #4766

@coveralls

Coverage Status

Coverage remained the same when pulling 5a3d9e7 on weierophinney:hotfix/4767 into ecbb3dd on zendframework:develop.

@coveralls

Coverage Status

Coverage remained the same when pulling 5d57d1d on weierophinney:hotfix/4767 into ecbb3dd on zendframework:develop.

@ezimuel ezimuel merged commit 5d57d1d into zendframework:develop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 1, 2013
  1. @weierophinney

    [#4766] wrote tests to indicate behavior is opt-in

    weierophinney authored
    - Added `setUseIncludePath()` call to translator adapters
    - added flag, Factory::USE_INCLUDE_PATH, as second argument to config
      factory
  2. @weierophinney

    [#4766] Config makes include_path usage opt-in only

    weierophinney authored
    - You must pass the `$useIncludePath` to either `fromFile()` or
      `fromFiles()` for it to be used. Constants were created with boolean
      values for those using IDEs.
  3. @weierophinney

    [#4766] Make include_path abilities opt-in in Translators

    weierophinney authored
    - Created AbstractFileLoader, which provides implementation details for
      when/how to use the include_path to load translation file assets.
    - Updated INI, gettext, and PHP loaders to extend AbstractFileLoader.
  4. @weierophinney

    [#4766] Add docblock detailing how to set include_path flag

    weierophinney authored
    - Added to translator loader plugin manager
Commits on Jul 2, 2013
  1. @weierophinney

    Remove constants

    weierophinney authored
    - Remove constants, per @DASPRiD
This page is out of date. Refresh to see the latest.
View
15 library/Zend/Config/Factory.php
@@ -59,14 +59,22 @@ class Factory
*
* @param string $filename
* @param bool $returnConfigObject
+ * @param bool $useIncludePath
* @return array|Config
* @throws Exception\InvalidArgumentException
* @throws Exception\RuntimeException
*/
- public static function fromFile($filename, $returnConfigObject = false)
+ public static function fromFile($filename, $returnConfigObject = false, $useIncludePath = false)
{
$filepath = $filename;
if (!file_exists($filename)) {
+ if (!$useIncludePath) {
+ throw new Exception\RuntimeException(sprintf(
+ 'Filename "%s" cannot be found relative to the working directory',
+ $filename
+ ));
+ }
+
$fromIncludePath = stream_resolve_include_path($filename);
if (!$fromIncludePath) {
throw new Exception\RuntimeException(sprintf(
@@ -122,14 +130,15 @@ public static function fromFile($filename, $returnConfigObject = false)
*
* @param array $files
* @param bool $returnConfigObject
+ * @param bool $useIncludePath
* @return array|Config
*/
- public static function fromFiles(array $files, $returnConfigObject = false)
+ public static function fromFiles(array $files, $returnConfigObject = false, $useIncludePath = false)
{
$config = array();
foreach ($files as $file) {
- $config = ArrayUtils::merge($config, static::fromFile($file));
+ $config = ArrayUtils::merge($config, static::fromFile($file, false, $useIncludePath));
}
return ($returnConfigObject) ? new Config($config) : $config;
View
82 library/Zend/I18n/Translator/Loader/AbstractFileLoader.php
@@ -0,0 +1,82 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace Zend\I18n\Translator\Loader;
+
+/**
+ * Abstract file loader implementation; provides facilities around resolving
+ * files via the include_path.
+ */
+abstract class AbstractFileLoader implements FileLoaderInterface
+{
+ /**
+ * Whether or not to consult the include_path when locating files
+ *
+ * @var bool
+ */
+ protected $useIncludePath = false;
+
+ /**
+ * Indicate whether or not to use the include_path to resolve translation files
+ *
+ * @param bool $flag
+ * @return self
+ */
+ public function setUseIncludePath($flag = true)
+ {
+ $this->useIncludePath = (bool) $flag;
+ return $this;
+ }
+
+ /**
+ * Are we using the include_path to resolve translation files?
+ *
+ * @return bool
+ */
+ public function useIncludePath()
+ {
+ return $this->useIncludePath;
+ }
+
+ /**
+ * Resolve a translation file
+ *
+ * Checks if the file exists and is readable, returning a boolean false if not; if the "useIncludePath"
+ * flag is enabled, it will attempt to resolve the file from the
+ * include_path if the file does not exist on the current working path.
+ *
+ * @param string $filename
+ * @return string|false
+ */
+ protected function resolveFile($filename)
+ {
+ if (!is_file($filename) || !is_readable($filename)) {
+ if (!$this->useIncludePath()) {
+ return false;
+ }
+ return $this->resolveViaIncludePath($filename);
+ }
+ return $filename;
+ }
+
+ /**
+ * Resolve a translation file via the include_path
+ *
+ * @param string $filename
+ * @return string|false
+ */
+ protected function resolveViaIncludePath($filename)
+ {
+ $resolvedIncludePath = stream_resolve_include_path($filename);
+ if (!$resolvedIncludePath || !is_file($resolvedIncludePath) || !is_readable($resolvedIncludePath)) {
+ return false;
+ }
+ return $resolvedIncludePath;
+ }
+}
View
9 library/Zend/I18n/Translator/Loader/Gettext.php
@@ -17,7 +17,7 @@
/**
* Gettext loader.
*/
-class Gettext implements FileLoaderInterface
+class Gettext extends AbstractFileLoader
{
/**
* Current file pointer.
@@ -44,9 +44,8 @@ class Gettext implements FileLoaderInterface
*/
public function load($locale, $filename)
{
- $resolvedIncludePath = stream_resolve_include_path($filename);
- $fromIncludePath = ($resolvedIncludePath !== false) ? $resolvedIncludePath : $filename;
- if (!$fromIncludePath || !is_file($fromIncludePath) || !is_readable($fromIncludePath)) {
+ $resolvedFile = $this->resolveFile($filename);
+ if (!$resolvedFile) {
throw new Exception\InvalidArgumentException(sprintf(
'Could not find or open file %s for reading',
$filename
@@ -56,7 +55,7 @@ public function load($locale, $filename)
$textDomain = new TextDomain();
ErrorHandler::start();
- $this->file = fopen($fromIncludePath, 'rb');
+ $this->file = fopen($resolvedFile, 'rb');
$error = ErrorHandler::stop();
if (false === $this->file) {
throw new Exception\InvalidArgumentException(sprintf(
View
2  library/Zend/I18n/Translator/Loader/Ini.php
@@ -17,7 +17,7 @@
/**
* PHP INI format loader.
*/
-class Ini implements FileLoaderInterface
+class Ini extends AbstractFileLoader
{
/**
* load(): defined by FileLoaderInterface.
View
2  library/Zend/I18n/Translator/Loader/PhpArray.php
@@ -16,7 +16,7 @@
/**
* PHP array loader.
*/
-class PhpArray implements FileLoaderInterface
+class PhpArray extends AbstractFileLoader
{
/**
* load(): defined by FileLoaderInterface.
View
33 library/Zend/I18n/Translator/LoaderPluginManager.php
@@ -18,6 +18,39 @@
* Enforces that loaders retrieved are either instances of
* Loader\FileLoaderInterface or Loader\RemoteLoaderInterface. Additionally,
* it registers a number of default loaders.
+ *
+ * If you are wanting to use the ability to load translation files from the
+ * include_path, you will need to create a factory to override the defaults
+ * defined in this class. A simple factory might look like:
+ *
+ * <code>
+ * function ($translators) {
+ * $adapter = new Gettext();
+ * $adapter->setUseIncludePath(true);
+ * return $adapter;
+ * }
+ * </code>
+ *
+ * You may need to override the Translator service factory to make this happen
+ * more easily. That can be done by extending it:
+ *
+ * <code>
+ * use Zend\I18n\Translator\TranslatorServiceFactory;
+ * // or Zend\Mvc\I18n\TranslatorServiceFactory
+ * use Zend\ServiceManager\ServiceLocatorInterface;
+ *
+ * class MyTranslatorServiceFactory extends TranslatorServiceFactory
+ * {
+ * public function createService(ServiceLocatorInterface $services)
+ * {
+ * $translator = parent::createService($services);
+ * $translator->getLoaderPluginManager()->setFactory(...);
+ * return $translator;
+ * }
+ * }
+ * </code>
+ *
+ * You would then specify your custom factory in your service configuration.
*/
class LoaderPluginManager extends AbstractPluginManager
{
View
2  tests/ZendTest/Config/FactoryTest.php
@@ -122,7 +122,7 @@ public function testFromIniAndXmlAndPhpFilesFromIncludePath()
'Xml/include-base2.xml',
'Php/include-base3.php',
);
- $config = Factory::fromFiles($files);
+ $config = Factory::fromFiles($files, false, true);
$this->assertEquals('bar', $config['base']['foo']);
$this->assertEquals('baz', $config['test']['bar']);
View
2  tests/ZendTest/I18n/Translator/Loader/GettextTest.php
@@ -90,6 +90,7 @@ public function testLoaderLoadsPluralRules()
public function testLoaderLoadsFromIncludePath()
{
$loader = new GettextLoader();
+ $loader->setUseIncludePath(true);
$textDomain = $loader->load('en_EN', 'translation_en.mo');
$this->assertEquals('Message 1 (en)', $textDomain['Message 1']);
@@ -99,6 +100,7 @@ public function testLoaderLoadsFromIncludePath()
public function testLoaderLoadsFromPhar()
{
$loader = new GettextLoader();
+ $loader->setUseIncludePath(true);
$textDomain = $loader->load('en_EN', 'phar://' . $this->testFilesDir . '/translations.phar/translation_en.mo');
$this->assertEquals('Message 1 (en)', $textDomain['Message 1']);
View
2  tests/ZendTest/I18n/Translator/Loader/IniTest.php
@@ -104,6 +104,7 @@ public function testLoaderLoadsPluralRules()
public function testLoaderLoadsFromIncludePath()
{
$loader = new IniLoader();
+ $loader->setUseIncludePath(true);
$textDomain = $loader->load('en_EN', 'translation_en.ini');
$this->assertEquals('Message 1 (en)', $textDomain['Message 1']);
@@ -113,6 +114,7 @@ public function testLoaderLoadsFromIncludePath()
public function testLoaderLoadsFromPhar()
{
$loader = new IniLoader();
+ $loader->setUseIncludePath(true);
$textDomain = $loader->load('en_EN', 'phar://' . $this->testFilesDir . '/translations.phar/translation_en.ini');
$this->assertEquals('Message 1 (en)', $textDomain['Message 1']);
View
2  tests/ZendTest/I18n/Translator/Loader/PhpArrayTest.php
@@ -83,6 +83,7 @@ public function testLoaderLoadsPluralRules()
public function testLoaderLoadsFromIncludePath()
{
$loader = new PhpArrayLoader();
+ $loader->setUseIncludePath(true);
$textDomain = $loader->load('en_EN', 'translation_en.php');
$this->assertEquals('Message 1 (en)', $textDomain['Message 1']);
@@ -92,6 +93,7 @@ public function testLoaderLoadsFromIncludePath()
public function testLoaderLoadsFromPhar()
{
$loader = new PhpArrayLoader();
+ $loader->setUseIncludePath(true);
$textDomain = $loader->load('en_EN', 'phar://' . $this->testFilesDir . '/translations.phar/translation_en.php');
$this->assertEquals('Message 1 (en)', $textDomain['Message 1']);
Something went wrong with that request. Please try again.