Code review: Minor fixes and todos

This commit is contained in:
Matias Griese
2016-03-03 14:15:29 +02:00
parent 99bd25f805
commit d573ebbfe3
22 changed files with 112 additions and 78 deletions

8
composer.lock generated
View File

@@ -638,12 +638,12 @@
"source": {
"type": "git",
"url": "https://github.com/rockettheme/toolbox.git",
"reference": "f9005449d8430a96c28eda3d786ddd2aae7040d3"
"reference": "958e5ff3e184195327a4a59f1860f62b244e2268"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/rockettheme/toolbox/zipball/f9005449d8430a96c28eda3d786ddd2aae7040d3",
"reference": "f9005449d8430a96c28eda3d786ddd2aae7040d3",
"url": "https://api.github.com/repos/rockettheme/toolbox/zipball/958e5ff3e184195327a4a59f1860f62b244e2268",
"reference": "958e5ff3e184195327a4a59f1860f62b244e2268",
"shasum": ""
},
"require": {
@@ -678,7 +678,7 @@
"php",
"rockettheme"
],
"time": "2016-03-02 18:28:47"
"time": "2016-03-03 09:32:40"
},
{
"name": "symfony/console",

View File

@@ -177,13 +177,14 @@ class Assets
*/
public function init()
{
$grav = Grav::instance();
/** @var Config $config */
$config = Grav::instance()['config'];
$base_url = Grav::instance()['base_url'];
$config = $grav['config'];
$base_url = $grav['base_url'];
$asset_config = (array)$config->get('system.assets');
/** @var UniformResourceLocator $locator */
$locator = Grav::instance()['locator'];
$locator = $grav['locator'];
$this->assets_dir = $locator->findResource('asset://') . DS;
$this->assets_url = $locator->findResource('asset://', false);

View File

@@ -40,10 +40,9 @@ class ZipBackup
if (!$destination) {
$destination = Grav::instance()['locator']->findResource('backup://', true);
if (!$destination)
if (!$destination) {
throw new \RuntimeException('The backup folder is missing.');
Folder::mkdir($destination);
}
}
$name = Grav::instance()['config']->get('site.title', basename(GRAV_ROOT));

View File

@@ -230,27 +230,29 @@ class Cache extends Getters
/**
* Deletes an item in the cache based on the id
*
* @param $id the id of the cached data entry
* @return bool true if the item was deleted successfully
* @param string $id the id of the cached data entry
* @return bool true if the item was deleted successfully
*/
public function delete($id)
{
if ($this->enabled) {
return $this->driver->delete($id);
}
return false;
}
/**
* Returns a boolean state of whether or not the item exists in the cache based on id key
*
* @param $id the id of the cached data entry
* @return bool true if the cached items exists
* @param string $id the id of the cached data entry
* @return bool true if the cached items exists
*/
public function contains($id)
{
if ($this->enabled) {
return $this->driver->contains(($id));
}
return false;
}
/**
@@ -311,7 +313,7 @@ class Cache extends Getters
$anything = true;
}
} elseif (is_dir($file)) {
if (@Folder::delete($file)) {
if (Folder::delete($file)) {
$anything = true;
}
}

View File

@@ -4,6 +4,7 @@ namespace Grav\Common\Config;
use Grav\Common\File\CompiledYamlFile;
use Grav\Common\Data\Data;
use Grav\Common\Utils;
use Pimple\Container;
use RocketTheme\Toolbox\File\YamlFile;
use RocketTheme\Toolbox\ResourceLocator\UniformResourceLocator;
@@ -113,6 +114,9 @@ class Setup extends Data
],
];
/**
* @param Container|array $container
*/
public function __construct($container)
{
$environment = $container['uri']->environment() ?: 'localhost';

View File

@@ -50,15 +50,15 @@ class Blueprints
if ($this->types === null) {
$this->types = array();
// Check if search is a stream.
if (strpos($this->search, '://')) {
// Stream: use UniformResourceIterator.
$grav = Grav::instance();
/** @var UniformResourceLocator $locator */
$locator = $grav['locator'];
$iterator = $locator->getIterator($this->search, null);
$grav = Grav::instance();
/** @var UniformResourceLocator $locator */
$locator = $grav['locator'];
// Get stream / directory iterator.
if ($locator->isStream($this->search)) {
$iterator = $locator->getIterator($this->search);
} else {
// Not a stream: use DirectoryIterator.
$iterator = new \DirectoryIterator($this->search);
}

View File

@@ -199,7 +199,7 @@ class GPM extends Iterator
*
* @param $package_name
*
* @return string
* @return string|null
*/
public function getLatestVersionOfPackage($package_name)
{
@@ -214,6 +214,8 @@ class GPM extends Iterator
if (isset($repository[$package_name])) {
return $repository[$package_name]->version;
}
return null;
}
/**

View File

@@ -223,6 +223,7 @@ class Grav extends Container
// Initialize configuration.
$debugger->startTimer('_config', 'Configuration');
/** @var Plugins $plugins */
$plugins = $this['plugins']->setup();
$this['config']->init();
$debugger->stopTimer('_config');

View File

@@ -5,6 +5,7 @@ namespace Grav\Common;
* Class GravTrait
*
* @package Grav\Common
* @deprecated
*/
trait GravTrait
{

View File

@@ -87,7 +87,7 @@ class Media extends Getters
$medium = MediumFactory::scaledFromMedium($altMedium, $ratio, 1)['file'];
}
if (!$medium) {
if (empty($medium)) {
continue;
}

View File

@@ -59,12 +59,9 @@ class MediumFactory
$locator = Grav::instance()['locator'];
$lookup = $locator->findResources('image://');
foreach ($lookup as $lookupPath) {
if (is_file($lookupPath . '/' . $params['thumb'])) {
$params['thumbnails']['default'] = $lookupPath . '/' . $params['thumb'];
break;
}
$file = $locator->findResource("image://{$params['thumb']}");
if ($file) {
$params['thumbnails']['default'] = $file;
}
return static::fromArray($params);

View File

@@ -100,8 +100,6 @@ class Page
/**
* Page Object Constructor
*
* @return $this
*/
public function __construct()
{
@@ -111,8 +109,6 @@ class Page
$this->taxonomy = [];
$this->process = $config->get('system.pages.process');
$this->published = true;
return $this;
}
/**
@@ -883,12 +879,14 @@ class Page
*/
public function blueprints()
{
$grav = Grav::instance();
/** @var Pages $pages */
$pages = Grav::instance()['pages'];
$pages = $grav['pages'];
$blueprint = $pages->blueprints($this->blueprintName());
$fields = $blueprint->fields();
$edit_mode = isset(Grav::instance()['admin']) ? Grav::instance()['config']->get('plugins.admin.edit_mode') : null;
$edit_mode = isset($grav['admin']) ? $grav['config']->get('plugins.admin.edit_mode') : null;
// override if you only want 'normal' mode
if (empty($fields) && ($edit_mode == 'auto' || $edit_mode == 'normal')) {
@@ -1419,17 +1417,19 @@ class Page
*/
public function url($include_host = false, $canonical = false, $include_lang = true)
{
$grav = Grav::instance();
/** @var Pages $pages */
$pages = Grav::instance()['pages'];
$pages = $grav['pages'];
/** @var Config $config */
$config = Grav::instance()['config'];
$config = $grav['config'];
/** @var Language $language */
$language = Grav::instance()['language'];
$language = $grav['language'];
/** @var Uri $uri */
$uri = Grav::instance()['uri'];
$uri = $grav['uri'];
// get pre-route
if ($include_lang && $language->enabled()) {

View File

@@ -682,6 +682,7 @@ class Pages
/** @var UniformResourceLocator $locator */
$locator = $this->grav['locator'];
// TODO: Use streams to allow page overrides. Right now only one folder gets looked at.
$pages_dir = $locator->findResource('page://');
if ($config->get('system.cache.enabled')) {
@@ -718,6 +719,7 @@ class Pages
$taxonomy->taxonomy($taxonomy_map);
}
} else {
// TODO: This function needs to be streams safe if we want to allow page overrides.
$this->recurse($pages_dir);
$this->buildRoutes();
}
@@ -758,6 +760,7 @@ class Pages
*/
protected function recurse($directory, Page &$parent = null)
{
// TODO: Cannot do this with streams.
$directory = rtrim($directory, DS);
$page = new Page;
@@ -776,6 +779,7 @@ class Pages
}
}
// TODO: Do we want to use real folders or streams as page path?
$page->path($directory);
if ($parent) {
$page->parent($parent);
@@ -795,6 +799,7 @@ class Pages
}
$content_exists = false;
// TODO: Another solution needed for streams support.
$pages_found = glob($directory . '/*' . CONTENT_EXT);
$page_extension = '';
@@ -825,8 +830,11 @@ class Pages
// set current modified of page
$last_modified = $page->modified();
// TODO: Add iterator from streams.
$iterator = new \FilesystemIterator($directory);
/** @var \DirectoryIterator $file */
foreach (new \FilesystemIterator($directory) as $file) {
foreach ($iterator as $file) {
$name = $file->getFilename();
// Ignore all hidden files if set.
@@ -843,6 +851,7 @@ class Pages
}
} elseif ($file->isDir() && !in_array($file->getFilename(), $this->ignore_folders)) {
if (!$page->path()) {
// TODO: Do we want to use real folders or streams as page path?
$page->path($file->getPath());
}

View File

@@ -64,10 +64,13 @@ class Plugin implements EventSubscriberInterface
* @param Grav $grav
* @param Config $config
*/
public function __construct($name, Grav $grav)
public function __construct($name, Grav $grav, Config $config = null)
{
$this->name = $name;
$this->grav = $grav;
if ($config) {
$this->setConfig($config);
}
}
/**
@@ -168,6 +171,7 @@ class Plugin implements EventSubscriberInterface
*/
protected function mergeConfig(Page $page, $deep = false, $params = [])
{
// FIXME: This should be done with Config class; it has configuration merging with blueprints.
$class_name = $this->name;
$class_name_merged = $class_name . '.merged';
$defaults = $this->config->get('plugins.' . $class_name, []);
@@ -219,10 +223,11 @@ class Plugin implements EventSubscriberInterface
return false;
}
$locator = Grav::instance()['locator'];
$grav = Grav::instance();
$locator = $grav['locator'];
$filename = 'config://plugins/' . $plugin_name . '.yaml';
$file = YamlFile::instance($locator->findResource($filename, true, true));
$content = Grav::instance()['config']->get('plugins.' . $plugin_name);
$content = $grav['config']->get('plugins.' . $plugin_name);
$file->save($content);
$file->free();

View File

@@ -89,11 +89,13 @@ class Plugins extends Iterator
*/
public function init()
{
$grav = Grav::instance();
/** @var Config $config */
$config = Grav::instance()['config'];
$config = $grav['config'];
/** @var EventDispatcher $events */
$events = Grav::instance()['events'];
$events = $grav['events'];
foreach ($this->items as $instance) {
// Register only enabled plugins.

View File

@@ -11,6 +11,7 @@ class LoggerServiceProvider implements ServiceProviderInterface
public function register(Container $container)
{
$log = new Logger('grav');
// TODO: Use streams.
$log_file = LOG_DIR.'grav.log';
$log->pushHandler(new StreamHandler($log_file, Logger::DEBUG));

View File

@@ -34,7 +34,7 @@ class StreamsServiceProvider implements ServiceProviderInterface
Stream::setLocator($locator);
ReadOnlyStream::setLocator($locator);
return new StreamBuilder($setup->getStreams($c));
return new StreamBuilder($setup->getStreams());
};
}
}

View File

@@ -1,10 +1,12 @@
<?php
namespace Grav\Common;
use RocketTheme\Toolbox\Session\Session as BaseSession;
/**
* Wrapper for Session
*/
class Session extends \RocketTheme\Toolbox\Session\Session
class Session extends BaseSession
{
protected $grav;
protected $session;
@@ -16,6 +18,7 @@ class Session extends \RocketTheme\Toolbox\Session\Session
*/
public function __construct(Grav $grav)
{
// FIXME: We really should have wrapper and not to extend the class!!
$this->grav = $grav;
}
@@ -45,6 +48,7 @@ class Session extends \RocketTheme\Toolbox\Session\Session
if ($config->get('system.session.enabled') || $is_admin) {
// Define session service.
// FIXME: NOT LIKE THIS!
parent::__construct($session_timeout, $session_path);
$domain = $uri->host();

View File

@@ -39,10 +39,11 @@ class Theme extends Plugin
return false;
}
$locator = Grav::instance()['locator'];
$grav = Grav::instance();
$locator = $grav['locator'];
$filename = 'config://themes/' . $theme_name . '.yaml';
$file = YamlFile::instance($locator->findResource($filename, true, true));
$content = Grav::instance()['config']->get('themes.' . $theme_name);
$content = $grav['config']->get('themes.' . $theme_name);
$file->save($content);
$file->free();

View File

@@ -79,24 +79,23 @@ class Themes extends Iterator
public function all()
{
$list = [];
$locator = Grav::instance()['locator'];
$themes = (array)$locator->findResources('themes://', false);
foreach ($themes as $path) {
$iterator = new \DirectoryIterator($path);
/** @var UniformResourceLocator $locator */
$locator = $this->grav['locator'];
/** @var \DirectoryIterator $directory */
foreach ($iterator as $directory) {
if (!$directory->isDir() || $directory->isDot()) {
continue;
}
$iterator = $locator->getIterator('themes://');
$theme = $directory->getBasename();
$result = self::get($theme);
/** @var \DirectoryIterator $directory */
foreach ($iterator as $directory) {
if (!$directory->isDir() || $directory->isDot()) {
continue;
}
if ($result) {
$list[$theme] = $result;
}
$theme = $directory->getBasename();
$result = self::get($theme);
if ($result) {
$list[$theme] = $result;
}
}
ksort($list);
@@ -141,7 +140,7 @@ class Themes extends Iterator
$obj = new Data($file->content(), $blueprint);
// Override with user configuration.
$obj->merge($this->grav['config']->get('themes.' . $name) ?: []);
$obj->merge($this->config->get('themes.' . $name) ?: []);
// Save configuration always to user/config.
$file = CompiledYamlFile::instance("config://themes/{$name}" . YAML_EXT);

View File

@@ -314,7 +314,7 @@ class Uri
$valid_page_types = implode('|', $config->get('system.pages.types'));
// Strip the file extension for valid page types
if (preg_match("/\.(" . $valid_page_types . ")$/", $parts['basename'])) {
if (preg_match('/\.(' . $valid_page_types . ')$/', $parts['basename'])) {
$uri = rtrim(str_replace(DIRECTORY_SEPARATOR, DS, $parts['dirname']), DS) . '/' . $parts['filename'];
}
@@ -722,7 +722,7 @@ class Uri
* @param Page $page the current page to use as reference
* @param string $url the URL as it was written in the markdown
* @param string $type the type of URL, image | link
* @param null $absolute if null, will use system default, if true will use absolute links internally
* @param bool $absolute if null, will use system default, if true will use absolute links internally
*
* @return string the more friendly formatted url
*/
@@ -749,6 +749,7 @@ class Uri
$external = false;
$base = $grav['base_url_relative'];
$base_url = rtrim($base . $grav['pages']->base(), '/') . $language_append;
// TODO: Add streams support for pages.
$pages_dir = $grav['locator']->findResource('page://');
// if absolute and starts with a base_url move on
@@ -939,7 +940,7 @@ class Uri
if ($type == 'link' && $language->enabled()) {
$language_append = $language->getLanguageURLPrefix();
}
// TODO: Add streams support for pages.
$pages_dir = $grav['locator']->findResource('page://');
if (is_null($relative)) {
$base = $grav['base_url'];

View File

@@ -69,18 +69,21 @@ class Group extends Data
*/
public function save()
{
$grav = Grav::instance();
$config = $grav['config'];
$blueprints = new Blueprints;
$blueprint = $blueprints->get('user/group');
$fields = $blueprint->fields();
Grav::instance()['config']->set("groups.$this->groupname", []);
$config->set("groups.$this->groupname", []);
foreach ($fields as $field) {
if ($field['type'] == 'text') {
$value = $field['name'];
if (isset($this->items[$value])) {
Grav::instance()['config']->set("groups.$this->groupname.$value", $this->items[$value]);
$config->set("groups.$this->groupname.$value", $this->items[$value]);
}
}
if ($field['type'] == 'array') {
@@ -89,7 +92,7 @@ class Group extends Data
if ($arrayValues) {
foreach ($arrayValues as $arrayIndex => $arrayValue) {
Grav::instance()['config']->set("groups.$this->groupname.$value.$arrayIndex", $arrayValue);
$config->set("groups.$this->groupname.$value.$arrayIndex", $arrayValue);
}
}
}
@@ -97,8 +100,8 @@ class Group extends Data
$type = 'groups';
$blueprints = $this->blueprints("config/{$type}");
$obj = new Data(Grav::instance()['config']->get($type), $blueprints);
$file = CompiledYamlFile::instance(Grav::instance()['locator']->findResource("config://{$type}.yaml"));
$obj = new Data($config->get($type), $blueprints);
$file = CompiledYamlFile::instance($grav['locator']->findResource("config://{$type}.yaml"));
$obj->file($file);
$obj->save();
}
@@ -112,16 +115,18 @@ class Group extends Data
*/
public static function remove($groupname)
{
$grav = Grav::instance();
$config = $grav['config'];
$blueprints = new Blueprints;
$blueprint = $blueprints->get('user/group');
$groups = Grav::instance()['config']->get("groups");
$groups = $config->get("groups");
unset($groups[$groupname]);
Grav::instance()['config']->set("groups", $groups);
$config->set("groups", $groups);
$type = 'groups';
$obj = new Data(Grav::instance()['config']->get($type), $blueprint);
$file = CompiledYamlFile::instance(Grav::instance()['locator']->findResource("config://{$type}.yaml"));
$obj = new Data($config->get($type), $blueprint);
$file = CompiledYamlFile::instance($grav['locator']->findResource("config://{$type}.yaml"));
$obj->file($file);
$obj->save();