Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Templates for namespaced controller classes #163

Closed
wants to merge 1 commit into from

3 participants

@AngryPHPNerd

I just tryed to create a controller class in a namespace.
The problem I found is that the lookup for template files currently include the Namespace-Separator \. It should be replaced by a more filesystem friendly character.

Another point witch I found in this case is if you write something like this:

<?php
namespace Foo\Bar;

use \Controller;

class Baz extends Controller
{}

the descent will not be regocnized, because the manifest builder search about Foo\Bar\Controller. I envisaging to create another pull request for that.

@AngryPHPNerd

For point 2 I add a class parser witch I wrote almost 2 years ago.

I used regexp instead of the php tokenizer here, because in my tests it was mutch faster ...

<?php
namespace amber\core\manifest;
/**
 * Amber - Just another PHP Framework
 *
 * @version   1.0
 * @require   PHP 5.3
 * @author    Dominik Beerbohm
 * @copyright (c) <a href="http://x-scripter.com/">x-Scripter.com</a> 2007-2010
 * @license   MIT Licence
 */

/**
 * Load libraries
 */
require_once __DIR__.'/DirectoryFilter.class.php';

/**
 * Create manifest
 *
 * @package amber\core\manifest
 */
class Builder
{
    protected $type_map             = array();
    protected $classes              = array();
    protected $extends_direct       = array();
    protected $extends              = array();
    protected $implements           = array();
    protected $namespaces           = array();
    protected $current_namespace    = null;
    protected $current_filename     = null;

    /**
     * Initialize manifest builder
     */
    public function __construct()
    {
        // Increase max_execution_time
        if (ini_get('safe_mode') != 1 && function_exists('set_time_limit')) {
            @set_time_limit(300);
        }
    }

    /**
     * Build manifest
     *
     * @param  string $path
     * @return array
     */
    public function build($path)
    {
        // Get all php-class-files in this path
        $tree    = new \RecursiveDirectoryIterator($path);
        $filter  = new DirectoryFilter($tree);
        $files   = new \RecursiveIteratorIterator($filter, \RecursiveIteratorIterator::SELF_FIRST);

        // Iterate over all files
        foreach ($files as $filename => $file) {
            if ($file->isFile()) {
                $this->getClassesFromFile($filename);
            }
        }
    }

    /**
     * Parse file and find all classes
     *
     * @param string $filename
     */
    protected function getClassesFromFile($filename)
    {
        // Set current filename
        $this->current_filename = $filename;
        $source = file_get_contents($filename);

        // Remove comments
        $source = preg_replace('#/\*(.*?)\*/#s', '', $source);
        $source = preg_replace('#/'.'/(.*?)$#m', '', $source);

        // Find namespaces
        preg_match_all('/namespace([\w\s\\\\]*)(;|\{)/isU', $source, $namespaces, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);

        if (($len = count($namespaces)) > 1) {
            for ($i=0; $i<$len; $i++) {
                $offset     = $namespaces[$i][2][1];
                $offset_end = isset($namespaces[$i+1]) ? $namespaces[$i+1][1][1] - $offset : strlen($source);
                $snippet    = substr($source, $offset, $offset_end);

                // Set the active namespace
                $this->current_namespace = preg_replace('/\s+/', '', $namespaces[$i][1][0]);
                $this->namespaces        = array();

                $this->fetchUse($snippet);
                $this->fetchClasses($snippet);
            }
        } else {
            $this->current_namespace = isset($namespaces[0][1][0]) ? preg_replace('/\s+/', '', $namespaces[0][1][0]) : '';
            $this->namespaces        = array();

            $this->fetchUse($source);
            $this->fetchClasses($source);
        }
    }

    /**
     * Resolve class aliases
     *
     * @param string $source
     */
    protected function fetchUse($source)
    {
        // Fetch all use's
        preg_match_all(
            '/use\s+((?>(?:[\w\s\\\\]+)(?:\s+as\s+(?:[\w\s]+))?(?:\s*,\s*)?|(?R))*);/is',
            $source,
            $use,
            PREG_SET_ORDER
        );

        foreach ($use as $u) {
            foreach (preg_split('/\s*,\s*/', $u[1]) as $namespace) {
                $parts      = preg_split('/\s+as\s+/', $namespace, 2);
                $namespace  = trim($parts[0], " \t\n\t\0\x0B\\");
                $name       = isset($parts[1]) ? trim($parts[1]) : (strrpos($namespace, '\\') > 0 ? substr($namespace, strrpos($namespace, '\\') + 1) : $namespace);

                $this->namespaces[ $name ] = $namespace;
            }
        }
    }

    /**
     * Get all classes in current file
     *
     * @param string $source
     */
    protected function fetchClasses($source)
    {
        // Fetch all classes
        preg_match_all(
            '/
                (final\s+class|abstract\s+class|class|interface)\s+     # Class Type
                ([a-z_]\w*)                                             # Class Name
                (\s+extends\s+([\w\\\\,[:space:]]+))?                   # Extends
                (\s+implements\s+([\w\\\\,[:space:]]+))?                # Implements
                \s*\{
            /isUx',
            $source,
            $_class,
            PREG_SET_ORDER
        );

        foreach ($_class as $class) {
            $className = $this->normelizeNamespacePath($class[2]);
            $this->classes[ $className ] = $this->current_filename;
            $this->type_map[ $className ] = preg_replace('/\s+/', '-', strToLower($class[1]));

            // Handle interfaces
            if ($class[1] == 'interface') {
                if (isset($class[3])) {
                    foreach (preg_split('/\s*,\s*/', $class[4]) as $intf) {
                        $interface = $this->normelizeNamespacePath(trim($intf));

                        if (!isset($this->extends_direct[ $interface ])) {
                            $this->extends_direct[ $interface ] = array();
                        }

                        $this->extends_direct[ $interface ][] = $className;
                    }
                }
            // Any other class
            } else {
                if (!empty($class[4])) {
                    $extendClass = $this->normelizeNamespacePath(trim($class[4]));

                    if (!isset($this->extends_direct[ $extendClass ])) {
                        $this->extends_direct[ $extendClass ] = array();
                    }

                    $this->extends_direct[ $extendClass ][] = $className;
                }
            }

            // Interfaces
            if (!empty($class[6])) {
                foreach (preg_split('/\s*,\s*/', $class[6]) as $intf) {
                    $interface = $this->normelizeNamespacePath(trim($intf));

                    if (!isset($this->implements[ $className ])) {
                        $this->implements[ $className ] = array();
                    }

                    $this->implements[ $className ][] = $interface;
                }
            }
        }
    }

    /**
     * Get the full qualified class name
     *
     * @return string
     */
    protected function normelizeNamespacePath($class_name)
    {
        if (isset($this->namespaces[ $class_name ])) {
            return $this->namespaces[ $class_name ];
        } elseif (strpos($class_name, '\\') === false) {
            return trim($this->current_namespace . '\\' . $class_name, '\\');
        } elseif (substr_count($class_name, '\\') == 1) {
            $name = strtok($class_name, '\\');

            if (isset($this->namespaces[ $this->current_namespace ][ $name ])) {
                return substr_replace($class_name, $this->namespaces[ $name ], 0, strlen($name));
            }
        }

        return trim($class_name, '\\');
    }

    /**
     * Determine class tree
     *
     * @return array
     */
    protected function getClassTree($var, $class)
    {
        $subClasses = array();

        if (isset($this->{$var}[ $class ])) {
            foreach ($this->{$var}[ $class ] as $subClass) {
                $subClasses[] = $subClass;
                $subClasses   = array_merge($subClasses, $this->getClassTree($var, $subClass));
            }
        }

        return $subClasses;
    }

    /**
     * Return class array
     *
     * @return array
     */
    public function getClasses()
    {
        $classes = array();

        // Get class tree
        foreach ($this->extends_direct as $class => $subClasses) {
            $this->extends[ $class ] = array_unique($this->getClassTree('extends_direct', $class));
        }

        // Create class array
        foreach ($this->classes as $class => $file) {
            $classes[ $class ] = array(
                'filename'  => $file,
                'type'      => $this->type_map[ $class ],
            );

            // Sub classes
            if (isset($this->extends_direct[ $class ])) {
                $classes[ $class ]['subclasses_direct'] = $this->extends_direct[ $class ];
                $classes[ $class ]['subclasses'] = $this->extends[ $class ];
            }

            // Interfaces
            if (isset($this->implements[ $class ])) {
                $classes[ $class ]['implements'] = $this->implements[ $class ];
            }
        }

        return $classes;
    }
}
@chillu
Owner

Do you have unit tests for the class parser? And some performance metrics comparing tokenizer vs. regex? This only needs to be done on manifest regeneration in our code - so while its good to make that faster, its not essential for production response times.

@AngryPHPNerd

I will look if I found my old benchmark files ... But I think I have deleted them.
My main reason to add this file was not just to reduce the parsing time it was more an example for howto handle the use statement.

@sminnee
Owner

Err, let's focus on the commit that's actually been included in the pull request, and just pretend that the unrelated code wasn't put there.

The reason we don't use a regex is because it's very hard to make a regex ignore class definitions that have been commented out, class definitions contained in string, etc. It's faster - it's also wrong. Frankly, I wouldn't bother going down that path.

As for the template filesystem changes, I have a few issues:

  • No documentation for the new feature.
  • No unit tests.
  • There are two arbitrary ways of defining template names without any explanation of why it's important to have two alternatives. Remember that we're going to be stuck supporting whatever decision we make forever, so it's a lot more than "just one more line of code"

@simon_w has been doing a lot of namespace stuff... does he have any feedback on this.

It raises another question: "what about table names?" It seems equally odd to have backslashes in table names, and we probably want to use the same separator in both.

This might be better fodder for a silverstripe-dev discussion.

@AngryPHPNerd

I think this pull request is no longer necessary since Namespace will come in a future version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 15, 2012
  1. @AngryPHPNerd
This page is out of date. Refresh to see the latest.
Showing with 18 additions and 9 deletions.
  1. +18 −9 control/Controller.php
View
27 control/Controller.php
@@ -266,35 +266,44 @@ function getAction() {
*/
function getViewer($action) {
// Hard-coded templates
- if($this->templates[$action]) {
+ if ($this->templates[$action]) {
$templates = $this->templates[$action];
- } else if($this->templates['index']) {
+ } elseif ($this->templates['index']) {
$templates = $this->templates['index'];
- } else if($this->template) {
+ } elseif ($this->template) {
$templates = $this->template;
} else {
// Add action-specific templates for inheritance chain
$parentClass = $this->class;
- if($action && $action != 'index') {
+
+ if ($action && $action != 'index') {
$parentClass = $this->class;
- while($parentClass != "Controller") {
- $templates[] = strtok($parentClass,'_') . '_' . $action;
+
+ while ($parentClass != 'Controller') {
+ $token = strtok($parentClass,'_');
+ $templates[] = str_replace('\\', '.', $token) . '_' . $action;
+ $templates[] = str_replace('\\', DIRECTORY_SEPARATOR, $token) . '_' . $action;
$parentClass = get_parent_class($parentClass);
}
}
+
// Add controller templates for inheritance chain
$parentClass = $this->class;
- while($parentClass != "Controller") {
- $templates[] = strtok($parentClass,'_');
+
+ while ($parentClass != 'Controller') {
+ $token = strtok($parentClass,'_');
+ $templates[] = str_replace('\\', '.', $token);
+ $templates[] = str_replace('\\', DIRECTORY_SEPARATOR, $token);
$parentClass = get_parent_class($parentClass);
}
// remove duplicates
$templates = array_unique($templates);
}
+
return new SSViewer($templates);
}
-
+
public function hasAction($action) {
return parent::hasAction($action) || $this->hasActionTemplate($action);
}
Something went wrong with that request. Please try again.