Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autodiscovery #1215

Merged
merged 8 commits into from
Sep 13, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
RouteCollection revised to use new centralized auto-discovery mechanism.
  • Loading branch information
lonnieezell committed Sep 10, 2018
commit f79c6a867a7bd2b0dc6cf5a99e5296b39f4d1ad2
19 changes: 15 additions & 4 deletions application/Config/Modules.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@
// Cannot extend BaseConfig or looping resources occurs.
class Modules
{
/*
|--------------------------------------------------------------------------
| Auto-Discovery Enabled?
|--------------------------------------------------------------------------
|
| If true, then auto-discovery will happen across all elements listed in
| $activeExplorers below. If false, no auto-discovery will happen at all,
| giving a slight performance boost.
*/
public $enabled = true;

/*
|--------------------------------------------------------------------------
| Auto-discover Rules
Expand All @@ -12,9 +23,8 @@ class Modules
| and used during the current application request. If it is not
| listed here, only the base application elements will be used.
*/
public $enabled = [
public $activeExplorers = [
'events',
'helpers',
'registrars',
'routes',
'services',
Expand All @@ -39,7 +49,6 @@ class Modules
*
* Valid values are:
* - events
* - helpers
* - registrars
* - routes
* - services
Expand All @@ -51,8 +60,10 @@ class Modules
*/
public function shouldDiscover(string $alias)
{
if (! $this->enabled) return false;

$alias = strtolower($alias);

return in_array($alias, $this->enabled);
return in_array($alias, $this->activeExplorers);
}
}
1 change: 0 additions & 1 deletion application/Config/Routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
$routes->setTranslateURIDashes(false);
$routes->set404Override();
$routes->setAutoRoute(true);
$routes->discoverLocal(false);

/**
* --------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion system/Config/Services.php
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ public static function routes($getShared = true)
return self::getSharedInstance('routes');
}

return new \CodeIgniter\Router\RouteCollection(self::locator());
return new \CodeIgniter\Router\RouteCollection(self::locator(), config('Modules'));
}

//--------------------------------------------------------------------
Expand Down
46 changes: 15 additions & 31 deletions system/Router/RouteCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -188,34 +188,37 @@ class RouteCollection implements RouteCollectionInterface
protected $currentOptions = null;

/**
* Determines whether locally specified, PSR4
* compatible code is automatically scanned
* for addition routes in a {namespace}/Config/Routes.php file.
*
* A little performance booster.
* @var bool
*/
protected $discoverLocal = false;
protected $didDiscover = false;

/**
* A little performance booster.
* @var bool
* @var \CodeIgniter\Autoloader\FileLocator
*/
protected $didDiscover = false;
protected $fileLocator;

/**
* @var \Config\Modules
*/
protected $moduleConfig;

//--------------------------------------------------------------------

/**
* Constructor
*
* @param FileLocator $locator
* @param FileLocator $locator
* @param Config/Modules $moduleConfig
*/
public function __construct(FileLocator $locator)
public function __construct(FileLocator $locator, $moduleConfig)
{
// Get HTTP verb
$this->HTTPVerb = strtolower($_SERVER['REQUEST_METHOD'] ?? 'cli');

$this->fileLocator = $locator;

$this->moduleConfig = $moduleConfig;
}

//--------------------------------------------------------------------
Expand Down Expand Up @@ -373,33 +376,14 @@ public function get404Override()

//--------------------------------------------------------------------

/**
* If true, will attempt to auto-discover new route files
* based on any PSR4 namespaces that have been set
* in Config/Autoload.php.
*
* @param bool $discover
*
* @return $this
*/
public function discoverLocal(bool $discover)
{
$this->discoverLocal = $discover;

return $this;
}

//--------------------------------------------------------------------

/**
* Will attempt to discover any additional routes, either through
* the local PSR4 namespaces, or through selected Composer packages.
* (Composer coming soon...)
*/
protected function discoverRoutes()
{
if ($this->didDiscover)
return;
if ($this->didDiscover) return;

// We need this var in local scope
// so route files can access it.
Expand All @@ -408,7 +392,7 @@ protected function discoverRoutes()
/*
* Discover Local Files
*/
if ($this->discoverLocal === true)
if ($this->moduleConfig->shouldDiscover('routes'))
{
$files = $this->fileLocator->search('Config/Routes.php');

Expand Down
22 changes: 16 additions & 6 deletions tests/system/Router/RouteCollectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public function tearDown()

//--------------------------------------------------------------------

protected function getCollector(array $config=[], array $files=[])
protected function getCollector(array $config=[], array $files=[], $moduleConfig = null)
{
$defaults = [
'Config' => APPPATH.'Config',
Expand All @@ -28,7 +28,13 @@ protected function getCollector(array $config=[], array $files=[])
$loader = new MockFileLocator($autoload);
$loader->setFiles($files);

return new RouteCollection($loader);
if ($moduleConfig === null)
{
$moduleConfig = new \Config\Modules();
$moduleConfig->enabled = false;
}

return new RouteCollection($loader, $moduleConfig);
}

public function testBasicAdd()
Expand Down Expand Up @@ -772,8 +778,10 @@ public function testWillDiscoverLocal()
'SampleSpace' => TESTPATH .'_support'
];

$routes = $this->getCollector($config);
$routes->discoverLocal(true);
$moduleConfig = new \Config\Modules();
$moduleConfig->enabled = true;

$routes = $this->getCollector($config, [], $moduleConfig);

$match = $routes->getRoutes();

Expand All @@ -789,8 +797,10 @@ public function testDiscoverLocalAllowsConfigToOverridePackages()
'SampleSpace' => TESTPATH .'_support'
];

$routes = $this->getCollector($config);
$routes->discoverLocal(true);
$moduleConfig = new \Config\Modules();
$moduleConfig->enabled = true;

$routes = $this->getCollector($config, [], $moduleConfig);

$routes->add('testing', 'MainRoutes::index');

Expand Down
4 changes: 3 additions & 1 deletion tests/system/Router/RouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ public function setUp()
{
parent::setUp();

$this->collection = new RouteCollection(new MockFileLocator(new \Config\Autoload()));
$moduleConfig = new \Config\Modules;
$moduleConfig->enabled = false;
$this->collection = new RouteCollection(new MockFileLocator(new \Config\Autoload()), $moduleConfig);

$routes = [
'users' => 'Users::index',
Expand Down
10 changes: 0 additions & 10 deletions user_guide_src/source/general/routing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -540,13 +540,3 @@ a valid class/method pair, just like you would show in any route, or a Closure::
{
echo view('my_errors/not_found.html');
});

Discovering Module Routes
-------------------------

If you are using :doc:`modular code </general/modules>`, then this setting will specify whether or not additional
Routes files should be scanned for within each of the PSR4 namespaces defined in **/application/Config/Autoload.php**.

::

$routes->discoverLocal(false);