From fcf1ffde4e8e3ebf5b69b1d5fd755e1e759fb648 Mon Sep 17 00:00:00 2001 From: Josh Sherman Date: Sat, 4 Oct 2008 23:48:03 +0000 Subject: [PATCH] Updated the Config class to use objects instead of an array (overriding __get() saved some effort). Also refactored some code, and updated the controller (partially) to handle shared models. git-svn-id: http://svn.cleancode.org/svn/pickles@60 4d10bc64-7434-11dc-a737-d2d0f8310089 --- Pickles.php | 32 +++++------ classes/ArrayUtils.php | 53 ------------------ classes/Config.php | 114 +++++++++++++++++++++++--------------- classes/Controller.php | 54 ++++++++++-------- classes/DB.php | 27 ++++----- classes/Mail.php | 18 +++--- classes/Model.php | 8 ++- classes/Object.php | 10 ++++ classes/Request.php | 51 ----------------- classes/Singleton.php | 8 ++- classes/Viewer/Smarty.php | 33 +++-------- models/store.php | 2 +- 12 files changed, 164 insertions(+), 246 deletions(-) delete mode 100755 classes/ArrayUtils.php delete mode 100755 classes/Request.php diff --git a/Pickles.php b/Pickles.php index 4d07085..0b76cf1 100755 --- a/Pickles.php +++ b/Pickles.php @@ -4,9 +4,9 @@ * PICKLES core include file * * This is the file that you include on the page you're instantiating the - * controller from (typically index.php). The path to the PICKLES code base is - * established as well as the path that Smarty will use to store the compiled - * pages. + * controller from (typically index.php). The path to the PICKLES code base + * is established as well as the path that Smarty will use to store the + * compiled pages. * * @package PICKLES * @author Joshua Sherman @@ -22,9 +22,9 @@ if (ini_get('date.timezone') == '') { } /** - * @todo Change the TEMP_PATH to be named more accordingly (Smarty-centric) - * and ditch sys_get_temp_dir() and point to a directory inside the - * PICKLES path. + * @todo Change the TEMP_PATH to be named more accordingly (Smarty-centric) + * and ditch sys_get_temp_dir() and point to a directory inside the + * PICKLES path. */ define('PICKLES_PATH', (phpversion() < 5.3 ? dirname(__FILE__) : __DIR__) . '/'); define('TEMP_PATH', sys_get_temp_dir() . '/pickles/smarty/' . $_SERVER['SERVER_NAME'] . '/'); @@ -39,20 +39,18 @@ define('TEMP_PATH', sys_get_temp_dir() . '/pickles/smarty/' . $_SERVER['SERVE * @return boolean Return value of require_once() or false (default) */ function __autoload($class) { - $file = PICKLES_PATH . 'classes/' . str_replace('_', '/', $class) . '.php'; - - if (file_exists($file)) { - return require_once $file; + $class_file = PICKLES_PATH . 'classes/' . str_replace('_', '/', $class) . '.php'; + $model_file = PICKLES_PATH . 'models/' . str_replace('_', '/', $class) . '.php'; + + if (file_exists($class_file)) { + return require_once $class_file; + } + else if (file_exists($model_file)) { + return require_once $model_file; } else { - $file = PICKLES_PATH . 'models/' . str_replace('_', '/', $class) . '.php'; - - if (file_exists($file)) { - return require_once $file; - } + return false; } - - return false; } ?> diff --git a/classes/ArrayUtils.php b/classes/ArrayUtils.php deleted file mode 100755 index 14ade05..0000000 --- a/classes/ArrayUtils.php +++ /dev/null @@ -1,53 +0,0 @@ - - * @copyright 2007-2008 Joshua Sherman - * @todo Just so it doesn't seem so bare, I need to add a few more - * common array manipulation functions. - */ -class ArrayUtils { - - /** - * Converts an object into an array (recursive) - * - * This only gets used by the Config class because simplexml_load_file() - * returns an object by default, and I prefered having it in an array - * format. - * - * @param object $object Object to be converted into an array - * @return array Resulting array formed from the passed object - */ - public static function object2array($object) { - $array = null; - - if (is_array($object)) { - foreach ($object as $key => $value) { - $array[$key] = self::object2array($value); - } - } - else { - $variables = get_object_vars($object); - - if (is_array($variables)) { - foreach ($variables as $key => $value) { - $array[$key] = ($key && !$value) ? null : self::object2array($value); - } - } - else { - return $object; - } - } - - return $array; - } -} - -?> diff --git a/classes/Config.php b/classes/Config.php index 8ffbd46..11e0437 100755 --- a/classes/Config.php +++ b/classes/Config.php @@ -3,11 +3,8 @@ /** * Configuration class * - * Handles loading and caching of the configuration into a Singleton object. - * Contains logic to determine if the configuration has already been loaded - * and will opt to use a "frozen" object rather than instantiate a new one. - * Also contains logic to reload configurations if the configuration file - * had been modified since it was originally instantiated (smart caching). + * Handles loading a configuration file and parsing the data for any public + * nodes that need to be made available to the viewer. * * @package PICKLES * @author Joshua Sherman @@ -55,44 +52,30 @@ class Config extends Singleton { * Handles the potential loading of the configuration file and * sanitizing the boolean strings into actual boolean values. * - * @param string $file File name of the configuration file to be loaded + * @param string $file File name of the config file to be loaded * @return boolean Based on the success or failure of the file load * @todo Either get rid of or make better use of the Error object - * @todo Some of this code seems screwy, (bool) on an array?? */ public function load($file) { if (file_exists($file)) { $this->file = $file; - + /** - * @todo LIBXML_NOCDATA is 5.1+ and I want PICKLES to be 5.0+ compatible + * @todo LIBXML_NOCDATA is 5.1+ and I want PICKLES to be 5.0+ + * compatible. Potential fix is to read the file in as + * a string, and if it has CDATA, throw an internal + * warning. */ - $config_array = ArrayUtils::object2array(simplexml_load_file($file, 'SimpleXMLElement', LIBXML_NOCDATA)); + $this->data = simplexml_load_file($file, 'SimpleXMLElement', LIBXML_NOCDATA); - /** - * @todo Loop through the object and deal with it accordingly - */ - if (is_array($config_array)) { - foreach ($config_array as $variable => $value) { - if ($value == 'true' || $value == array()) { - $value = (bool)$value; + // Loops through the top level nodes to find public nodes + $variables = get_object_vars($this->data); + + if (is_array($variables)) { + foreach ($variables as $key => $value) { + if (is_object($value) && isset($value->attributes()->public) && $value->attributes()->public == true) { + $this->viewer_data[$key] = $value; } - - if (isset($value['@attributes']['public'])) { - if ($value['@attributes']['public'] == true) { - - if (count($value['@attributes']) == 1) { - unset($value['@attributes']); - } - else { - unset($value['@attributes']['public']); - } - - $this->viewer_data[$variable] = $value; - } - } - - $this->data[$variable] = $value == array() ? (bool)$value : $value; } } @@ -105,34 +88,73 @@ class Config extends Singleton { } /** - * Gets the default model + * Alias for $config->models->default * - * @return Returns the default model as set + * @return Returns the default model set or null + * @todo Need to add a PICKLES fallback model just in case */ public function getDefaultModel() { - if (isset($this->data['models']['default'])) { - return $this->data['models']['default']; + if (isset($this->data->models->default)) { + return (string)$this->data->models->default; } - return false; + return null; } /** - * Gets the viewer data * - * @return Returns either the variable value or false if no variable. - * @todo Need better checking if the passed variable is an array when - * the array element value is present - * @todo Returning false could be misleading, especially if you're - * expecting a boolean value to begin with. Perhaps an error - * should be thrown? + */ + public function getSharedModel($requested_model) { + + $additional = null; + + if (strpos($requested_model, '/') !== false) { + list($requested_model, $additional) = split('/', $requested_model, 2); + $additional = '/' . $additional; + } + + if (isset($this->data->models->shared->model)) { + foreach ($this->data->models->shared->model as $shared_model) { + if (isset($shared_model->alias)) { + if ($requested_model == $shared_model->alias) { + return (string)$shared_model->name . $additional; + } + } + else { + if ($requested_model == $shared_model->name) { + return (string)$shared_model->name . $additional; + } + } + } + } + + return null; + } + + /** + * Alias for $config->viewer_data + * + * @return Returns either the variable value or null if no variable. */ public function getViewerData() { if (isset($this->viewer_data)) { return $this->viewer_data; } - return false; + return null; + } + + public function __get($node) { + if (isset($this->data->$node) && $this->data->$node != '') { + if (in_array($this->data->$node, array('true', 'false'))) { + return (bool)$this->data->$node; + } + else if (is_object($this->data->$node)) { + return $this->data->$node; + } + } + + return null; } } diff --git a/classes/Controller.php b/classes/Controller.php index 343b6e4..0b891d5 100755 --- a/classes/Controller.php +++ b/classes/Controller.php @@ -41,46 +41,55 @@ class Controller extends Object { parent::__construct(); // Load the config for the site passed in - $this->config = Config::getInstance(); - $this->config->load($file); + $config = Config::getInstance(); + $config->load($file); // Generate a generic "site down" message - if ($this->config->get('disabled')) { + if ($config->disabled === true) { exit("

{$_SERVER['SERVER_NAME']} is currently down for maintenance

"); } // Grab the passed in model or use the default - $name = isset($_REQUEST['model']) ? str_replace('-', '_', $_REQUEST['model']) : $this->config->getDefaultModel(); + $model_name = isset($_REQUEST['model']) ? str_replace('-', '_', $_REQUEST['model']) : $config->getDefaultModel(); - if ($name == 'logout') { + if ($model_name == 'logout') { Security::logout(); } else { // Load the model - $file = '../models/' . $name . '.php'; - $shared_file = '../../pickles/models/' . $name . '.php'; + $model_file = '../models/' . $model_name . '.php'; - if (strpos($name, '/') === false) { - $class = $name; - $section = $name; + if (strpos($model_name, '/') === false) { + $class = $model_name; + $section = $model_name; $event = null; } else { - $class = str_replace('/', '_', $name); - list($section, $event) = split('/', $name); + $class = str_replace('/', '_', $model_name); + list($section, $event) = split('/', $model_name); } - if (file_exists($file)) { - require_once $file; + $shared_model_name = $config->getSharedModel($model_name); + $shared_model_file = PICKLES_PATH . 'models/' . $shared_model_name . '.php'; + + if (file_exists($model_file)) { + require_once $model_file; if (class_exists($class)) { $this->model = new $class; } } - /** - * @todo Fix the shared stuff - */ - elseif (file_exists($shared_file) && $this->config->get('models', 'shared') != false) { + else if (file_exists($shared_model_file) && $shared_model_name != false) { + if (strpos($shared_model_name, '/') === false) { + $class = $shared_model_name; + //$section = $shared_model_name; + //$event = null; + } + else { + $class = str_replace('/', '_', $shared_model_name); + //list($section, $event) = split('/', $shared_model_name); + } + if (class_exists($class)) { $this->model = new $class; } @@ -101,12 +110,9 @@ class Controller extends Object { Security::authenticate(); } - /** - * @todo are any of these relevant any more? - */ - $this->model->set('name', $name); - $this->model->set('section', $section); - //$this->model->set('event', $event); + $this->model->set('name', $model_name); + $this->model->set('shared_name', $shared_model_name); + $this->model->set('section', $section); // Execute the model's logic if (method_exists($this->model, '__default')) { diff --git a/classes/DB.php b/classes/DB.php index 523fefc..2c9e2c5 100755 --- a/classes/DB.php +++ b/classes/DB.php @@ -16,6 +16,7 @@ * @todo Eventually finish adding in my ActiveRecord class, even * though I feel active record dumbs people down since it's a * crutch for actually being able to write SQL. + * @todo Rename me to Database (maybe) */ class DB extends Singleton { @@ -24,14 +25,6 @@ class DB extends Singleton { */ private static $instance; - /** - * Private database information and login credentials - */ - private $hostname; - private $username; - private $password; - private $database; - /** * Private MySQL resources */ @@ -74,17 +67,19 @@ class DB extends Singleton { if (!is_resource($this->connection)) { $config = Config::getInstance(); - $this->hostname = $config->get('database', 'hostname'); - $this->username = $config->get('database', 'username'); - $this->password = $config->get('database', 'password'); - $this->database = $config->get('database', 'database'); + if ($config->database->hostname == '') { + $config->database->hostname = 'localhost'; + } - if (isset($this->hostname) && isset($this->username) && isset($this->password) && isset($this->database)) { - $this->connection = @mysql_connect($this->hostname, $this->username, $this->password); + if (isset($config->database->username, $config->database->password, $config->database->database)) { + /** + * @todo I removed the @ and changed to be pconnect... let's see + */ + $this->connection = mysql_pconnect($config->database->hostname, $config->database->username, $config->database->password); if (is_resource($this->connection)) { - if (!mysql_select_db($this->database, $this->connection)) { - Error::addWarning("There was an error selecting the '" . $this->database , "' database"); + if (!mysql_select_db($config->database->database, $this->connection)) { + Error::addWarning("There was an error selecting the '" . $this->database->database , "' database"); return false; } else { diff --git a/classes/Mail.php b/classes/Mail.php index 4c885bd..f84e383 100755 --- a/classes/Mail.php +++ b/classes/Mail.php @@ -35,16 +35,16 @@ class Mail { */ static function send($recipients = null, $prefix = null) { $config = Config::getInstance(); - $defaults = $config->get('contact'); + $defaults = $config->contact; if (!isset($recipients)) { - $recipients = $defaults['recipients']['recipient']; + $recipients = $defaults->recipients->recipient; } - if (is_array($recipients)) { + if (is_object($recipients)) { $to = null; foreach ($recipients as $recipient) { - $to .= (isset($to) ? ',' : '') . $recipient; + $to .= (isset($to) ? ',' : '') . (string)$recipient; } } else { @@ -52,14 +52,16 @@ class Mail { } if (!isset($prefix)) { - $prefix = isset($defaults['prefix']) ? $defaults['prefix'] : null; + $prefix = isset($defaults->prefix) && $defaults->prefix != '' ? $defaults->prefix : null; } - if (isset($defaults['subject'])) { - $subject = $defaults['subject']; + $subject = str_replace("\n", '', (isset($prefix) ? "[{$prefix}] " : '')); + + if (isset($defaults->subject)) { + $subject .= $defaults->subject; } else { - $subject = str_replace("\n", '', (isset($prefix) ? "[{$prefix}] " : '') . $_REQUEST['subject']); + $subject .= $_REQUEST['subject']; } if (isset($_REQUEST['name'])) { diff --git a/classes/Model.php b/classes/Model.php index 3cad3c7..193d9ed 100644 --- a/classes/Model.php +++ b/classes/Model.php @@ -98,12 +98,16 @@ class Model extends Object { } /** - * Gets the data variable + * Alias for $model->data * * @return array Associative array of data that was set by the model */ public function getData() { - return $this->get('data'); + if (isset($this->data)) { + return $this->data; + } + + return null; } /** diff --git a/classes/Object.php b/classes/Object.php index 26b254b..856e0c4 100644 --- a/classes/Object.php +++ b/classes/Object.php @@ -52,6 +52,7 @@ class Object { * expecting a boolean value to begin with. Perhaps an error should * be thrown? */ + /* public function get($variable, $array_element = null) { if (isset($this->$variable)) { if (isset($array_element)) { @@ -68,6 +69,15 @@ class Object { return false; } + */ + + public function __get($variable) { + if (isset($this->$variable)) { + return $this->$variable; + } + + return null; + } /** * Sets a variable diff --git a/classes/Request.php b/classes/Request.php deleted file mode 100755 index d8893dd..0000000 --- a/classes/Request.php +++ /dev/null @@ -1,51 +0,0 @@ - - * @copyright 2007-2008 Joshua Sherman - * @todo Phase out entirely or make use of it. - */ -class Request { - - /** - * Private request array - */ - private static $request; - - /** - * Loads the $_REQUEST super global into the object's array - */ - public static function load() { - if (is_array($_REQUEST)) { - foreach ($_REQUEST as $key => $value) { - self::$request[$key] = $value; - unset($_REQUEST[$key]); - } - } - - return true; - } - - /** - * Gets a variable - * - * @return Returns either the variable value or false if no variable. - * @todo Returning false could be misleading, especially if you're - * expecting a boolean value to begin with. Perhaps an error should - * be thrown? - */ - public static function get($variable) { - if (isset(self::$request[$variable])) { - return self::$request[$variable]; - } - - return false; - } -} - -?> diff --git a/classes/Singleton.php b/classes/Singleton.php index e29c05f..a982f9c 100644 --- a/classes/Singleton.php +++ b/classes/Singleton.php @@ -44,6 +44,7 @@ class Singleton { * expecting a boolean value to begin with. Perhaps an error * should be thrown? */ + /* public function get($variable, $array_element = null) { if (isset($this->data[$variable])) { if (isset($array_element)) { @@ -60,6 +61,7 @@ class Singleton { return false; } + */ /** * Sets a variable @@ -67,9 +69,9 @@ class Singleton { * @param string $variable Name of the variable to be set * @param mixed $value Value to be assigned to the passed variable */ - public function set($variable, $value) { - $this->data[$variable] = $value; - } + //public function set($variable, $value) { + // $this->data[$variable] = $value; + //} } ?> diff --git a/classes/Viewer/Smarty.php b/classes/Viewer/Smarty.php index 77b4ec3..32748c7 100644 --- a/classes/Viewer/Smarty.php +++ b/classes/Viewer/Smarty.php @@ -17,6 +17,7 @@ class Viewer_Smarty extends Viewer_Common { * Displays the Smarty generated pages */ public function display() { + // Obliterates any passed in PHPSESSID (thanks Google) if (stripos($_SERVER['REQUEST_URI'], '?PHPSESSID=') !== false) { list($request_uri, $phpsessid) = split('\?PHPSESSID=', $_SERVER['REQUEST_URI'], 2); @@ -72,9 +73,12 @@ class Viewer_Smarty extends Viewer_Common { /** * @todo Maybe the template path should be part of the configuration? */ - $template = '../templates/' . $this->model->get('name') . '.tpl'; - $shared_template = str_replace('../', '../../pickles/', $template); + $template = '../templates/' . $this->model->name . '.tpl'; + $shared_template = getcwd() . '/templates/' . $this->model->shared_name . '.tpl'; + /** + * @todo There's a bug with the store home page since it's a redirect + */ if (!file_exists($template)) { if (file_exists($shared_template)) { $template = $shared_template; @@ -82,31 +86,10 @@ class Viewer_Smarty extends Viewer_Common { } // Pass all of our controller values to Smarty - $smarty->assign('section', $this->model->get('section')); - $smarty->assign('model', $this->model->get('name')); - /** - * @todo Rename action to event - * @todo I'm not entirely sure that these values are necessary at all due - * to new naming conventions. - */ - //$smarty->assign('action', $this->model->get('action')); - //$smarty->assign('event', $this->model->get('action')); - - // Thanks to new naming conventions - $smarty->assign('admin', $this->config->get('admin', 'sections')); + $smarty->assign('section', $this->model->section); + $smarty->assign('model', $this->model->name); $smarty->assign('template', $template); - // Only load the session if it's available - /** - * @todo Not entirely sure that the view needs full access to the session - * (seems insecure at best) - */ - /* - if (isset($_SESSION)) { - $smarty->assign('session', $_SESSION); - } - */ - // Loads the data from the config $data = $this->config->getViewerData(); diff --git a/models/store.php b/models/store.php index c94f128..6138fdc 100644 --- a/models/store.php +++ b/models/store.php @@ -28,7 +28,7 @@ class store extends Model { // Loads the navigation $config = Config::getInstance(); - $this->data['subnav'] = $config->get('store', 'sections'); + $this->data['subnav'] = $config->store->sections; // Loads the categories $categories = $this->db->getArray('SELECT id, name, permalink FROM categories WHERE parent_id IS NULL AND visible = "Y" ORDER BY weight;');