Zend Framework refactoring using stream_resolve_include_path

Both Zend_Cache::_isReadable() and Zend_Loader::isReadable() code and performance could be improved using the new PHP 5.3.2 stream_resolve_include_path() function and removing the error suppression (@). Zend_Cache::_isReadable() can be particularly slow due to error suppression so

private static function _isReadable($filename)
{
    if (!$fh = @fopen($filename, 'r', true)) {
        return false;
    }
    @fclose($fh);
    return true;
}

first of all could be rewritten to:

private static function _isReadable($filename)
{
    return is_string(stream_resolve_include_path($filename));
}

or, for backward compatibility:

private static function _isReadable($filename)
{
    if (function_exists('stream_resolve_include_path')) {
        return is_string(stream_resolve_include_path($filename));
    }

    if (!$fh = @fopen($filename, 'r', true)) {
        return false;
    }
    @fclose($fh);
    return true;
}

Something similar applies to Zend_Loader

public static function isReadable($filename)
{
    // ADDED CODE

    if (function_exists('stream_resolve_include_path')) {
        return is_string(stream_resolve_include_path($filename));
    }
    
    // OLD CODE

    [....]

    foreach (self::explodeIncludePath() as $path) {
        if ($path == '.') {
            if (is_readable($filename)) {
                return true;
            }
            continue;
        }
        $file = $path . '/' . $filename;
        if (is_readable($file)) {
            return true;
        }
    }
    return false;
}

so that the foreach loop will be avoided with PHP 5.3.2.

Zend_Cache method comes from Zend_Loader (see #ZF-2891 for details) so it would make sense to extrapolate the code in a separate class, taking into consideration to use Zend_Loader code instead of the error suppression, something like this (comments removed for brevity):

class Zend_File
{
    public static function isReadable($filename)
    {
        if (function_exists('stream_resolve_include_path')) {
            return is_string(stream_resolve_include_path($filename));
        }

        if (is_readable($filename)) {
            return true;
        }

        if (strtoupper(substr(PHP_OS, 0, 3)) == 'WIN'
            && preg_match('/^[a-z]:/i', $filename)
        ) {
            return false;
        }

        foreach (Zend_Application::getIncludePathArray() as $path) {
            if ($path == '.') {
                if (is_readable($filename)) {
                    return true;
                }
                continue;
            }
            $file = $path . '/' . $filename;
            if (is_readable($file)) {
                return true;
            }
        }
        return false;
    }
}

adding this to Zend_Application:

/**
 * explodeIncludePath() method renamed to getIncludePathArray()
 * Todo: Cache the result ?
 */
public static function getIncludePathArray($path = null)
{
    if (null === $path) {
        $path = get_include_path();
    }

    if (PATH_SEPARATOR == ':') {
        $paths = preg_split('#:(?!//)#', $path);
    } else {
        $paths = explode(PATH_SEPARATOR, $path);
    }
    return $paths;
}

where I suggest to rename explodeIncludePath() to getIncludePathArray() that sounds more appropriate. explodeIncludePath() should also be removed from Zend_Loader taking care to notice the users since it is a public method.

To avoid confusion with PHP is_readable() function, why not renaming isReadable() to isAccessible() deprecating the old methods with a trigger_error('...', E_USER_NOTICE) ?

1 comment

Issue report?

Submitted by Guest on Fri, 07/23/2010 - 18:03.

With ZF2 in development, there's no time like the present to throw this in as an issue report against the mentioned components Wink.

© 2012 Devis Lucato @itbus.