From 16e7ed0bc0f222fa6a7bdf9b71343aeee2e4392d Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Tue, 4 Jun 2019 16:51:50 +0200 Subject: [PATCH] Simple controllers (offer and position) are workign in the new oo format --- src/admin/abstract/controller.php | 118 ++++++++++++------ src/admin/clubs.php | 1 + .../common/abstract/controller/mapping.php | 55 ++++++++ src/admin/common/abstract/model.php | 12 +- src/admin/common/abstract/model/column.php | 19 +-- src/admin/common/controllermappings/cmp.php | 14 +++ src/admin/common/controllermappings/float.php | 14 +++ src/admin/common/controllermappings/int.php | 14 +++ src/admin/common/controllermappings/ref.php | 44 +++++++ .../common/controllermappings/string.php | 14 +++ src/admin/common/models/column/ref.php | 4 +- src/admin/common/models/factory/offer.php | 2 +- src/admin/common/models/factory/position.php | 2 +- src/admin/controllers/club.php | 13 +- src/admin/controllers/offer.php | 9 +- src/admin/controllers/position.php | 6 - src/admin/controllers/user.php | 4 +- 17 files changed, 275 insertions(+), 70 deletions(-) create mode 100644 src/admin/common/abstract/controller/mapping.php create mode 100644 src/admin/common/controllermappings/cmp.php create mode 100644 src/admin/common/controllermappings/float.php create mode 100644 src/admin/common/controllermappings/int.php create mode 100644 src/admin/common/controllermappings/ref.php create mode 100644 src/admin/common/controllermappings/string.php diff --git a/src/admin/abstract/controller.php b/src/admin/abstract/controller.php index 6c6efd8..e80f41c 100644 --- a/src/admin/abstract/controller.php +++ b/src/admin/abstract/controller.php @@ -7,6 +7,9 @@ use Joomla\CMS\Router\Route; // No direct access. defined('_JEXEC') or die; +class DataParsingException extends Exception {} +class DataInvalidException extends Exception {} + abstract class AbstractClubsController extends BaseController { /** @@ -27,11 +30,6 @@ abstract class AbstractClubsController extends BaseController return $this->getSingleBaseName(); } - /** - * @todo Make this OO conforme - */ - protected abstract function getDataMapping(); - public function new() { $factory = $this->getFactory(); @@ -58,67 +56,109 @@ abstract class AbstractClubsController extends BaseController */ protected function saveToDatabase($obj, $id) { - // Fetch the posted data - $values = $this->loadData(); - - $this->filterPreCheck($values); - - // Check the input data - $error = ! $this->checkDataIsValid($values, $obj); - - $view = $this->getSingleViewName(); - - if($error) + try { + // Fetch the posted data + $values = $this->loadData(); + + $this->filterRawCheck($values); + + // Check the input data + if( ! $this->requiredDataIsAvailable($values) ) + throw new DataParsingException(); + + if( ! $this->rawDataIsValid($values) ) + throw new DataParsingException(); + + $obj->setValues($values, true); + + // Do some additional tests by the controller + if( ! $this->objectValid($obj) ) + throw new DataInvalidException(); + + // Check if the object complains about valitity + if( ! $obj->dataIsValid() ) + throw new DataInvalidException(); + + // Do the actual work + $obj->save(); + + // Redirect to the list of objects + $view = $this->getSingleViewName(); + $this->setRedirect(Route::_("index.php?option=com_clubs&view={$view}s", false)); + } + catch(DataParsingException $e) + { + // FIXME Make this robust (are external refs already dereferenced?) + $view = $this->getSingleViewName(); + $obj->setValues($values, true); + $urldata = $obj->pack(); + $this->setRedirect(Route::_("index.php?option=com_clubs&view={$view}&id={$id}&data={$urldata}", false)); + } + catch(DataInvalidException $e) + { + $view = $this->getSingleViewName(); $urldata = $obj->pack(); $this->setRedirect(Route::_("index.php?option=com_clubs&view={$view}&id={$id}&data={$urldata}", false)); - return; } - - $obj->setValues($values); - - // Do the actual work - $obj->save(); - $this->setRedirect(Route::_("index.php?option=com_clubs&view={$view}s", false)); - } protected function loadData() { $values = array(); + $factory = $this->getFactory(); $input = Factory::getApplication()->input->post; - foreach($this->getDataMapping() as $m => $f) + foreach($factory->getAttributes() as $column) { - $filter = (isset($f['filter'])) ? $f['filter'] : 'string'; - - $values[$m] = $input->get($m, null, $filter); + $values[$column->getAlias()] = $column->getFilter()->getFilteredValue($input, $column->getAlias()); } return $values; } - protected function filterPreCheck(&$values){} + protected function filterRawCheck(&$values){} + + protected function objectValid($obj) + { + return true; + } + + private function requiredDataIsAvailable($values) + { + $ok = true; + + foreach($this->getFactory()->getAttributes() as $column) + { + $filter = $column->getFilter(); + if(! $filter->requiredDataAvailable($values[$column->getAlias()])) + { + $fname = $filter->getName(); + Factory::getApplication()->enqueueMessage("Das Feld $fname ist obligatorisch.", 'error'); + $ok = false; + } + } + + return $ok; + } /** * @param array $values * @param AbstractCommonClubsModel $obj * @return boolean */ - protected function checkDataIsValid($values, $obj) + protected function rawDataIsValid($values) { $error = false; - // Check existence of the required fields - foreach ($this->getDataMapping() as $m => $v) + + $factory = $this->getFactory(); + + foreach($factory->getAttributes() as $column) { - if(! isset($v['required']) || ! $v['required']) - continue; - - // Field is required - if(! $this->fieldValid($m, $values[$m], $v)) + if(! $column->getFilter()->rawValueValid($values[$column->getAlias()])) { - $fname = (isset($v['name'])) ? $v['name'] : $m; - Factory::getApplication()->enqueueMessage("Das Feld $fname ist obligatorisch.", 'error'); + $fname = $column->getFilter()->getName(); + Factory::getApplication()->enqueueMessage("Das Feld $fname ist fehlerhaft.", 'error'); $error = true; } } diff --git a/src/admin/clubs.php b/src/admin/clubs.php index debe58e..726f58f 100644 --- a/src/admin/clubs.php +++ b/src/admin/clubs.php @@ -10,6 +10,7 @@ JLoader::discover('Clubs', JPATH_ROOT . '/administrator/components/com_clubs/mym JLoader::registerPrefix('AbstractClubs', JPATH_ROOT . '/administrator/components/com_clubs/abstract'); JLoader::registerPrefix('AbstractCommonClubs', JPATH_ROOT . '/administrator/components/com_clubs/common/abstract'); JLoader::registerPrefix('CommonClubsModel', JPATH_ROOT . '/administrator/components/com_clubs/common/models'); +JLoader::registerPrefix('CommonClubsControllerMapping', JPATH_ROOT . '/administrator/components/com_clubs/common/controllermappings'); $controller = BaseController::getInstance("Clubs"); $input = Factory::getApplication()->input; diff --git a/src/admin/common/abstract/controller/mapping.php b/src/admin/common/abstract/controller/mapping.php new file mode 100644 index 0000000..b445609 --- /dev/null +++ b/src/admin/common/abstract/controller/mapping.php @@ -0,0 +1,55 @@ +name = $name; + $this->required = $required; + } + + + /** + * @param mixed $value + * @return bool + */ + public function requiredDataAvailable($value) + { + if($this->required && ($value === null || $value === '')) + return false; + + return true; + } + + /** + * @return string + */ + public function getName() + { + return $this->name; + } + + /** + * @param JInput $input + * @param string $name + * @return string + */ + public abstract function getFilteredValue($input, $name); + + /** + * @param mixed $value + * @return boolean + */ + public function rawValueValid($value) + { + return true; + } + +} diff --git a/src/admin/common/abstract/model.php b/src/admin/common/abstract/model.php index 7e97b67..806263c 100644 --- a/src/admin/common/abstract/model.php +++ b/src/admin/common/abstract/model.php @@ -40,9 +40,12 @@ abstract class AbstractCommonClubsModel return $this->new; } - public function setValues($values) + public function setValues($values, $unpack = false) { - $this->values = $values; + if($unpack) + $this->values = $this->unpackExternalReferencesFromKeys($values); + else + $this->values = $values; } protected function setValue($key, $value) @@ -351,4 +354,9 @@ abstract class AbstractCommonClubsModel $this->setValues($vals); } + public function dataIsValid() + { + return true; + } + } diff --git a/src/admin/common/abstract/model/column.php b/src/admin/common/abstract/model/column.php index a21ceee..cfacc87 100644 --- a/src/admin/common/abstract/model/column.php +++ b/src/admin/common/abstract/model/column.php @@ -8,7 +8,10 @@ abstract class AbstractCommonClubsModelColumn protected $alias; protected $column; - protected $required; + /** + * @var AbstractCommonClubsControllerMapping + */ + protected $filter; public function getAlias() { @@ -20,17 +23,12 @@ abstract class AbstractCommonClubsModelColumn return $this->column; } - public function isRequired() - { - return $this->required; - } - public abstract function isSimpleType(); - public function __construct($alias, $required = true, $column = null) + public function __construct($alias, $filter, $column = null) { $this->alias = $alias; - $this->required = $required; + $this->filter = $filter; if(isset($column)) $this->column = $column; else @@ -71,4 +69,9 @@ abstract class AbstractCommonClubsModelColumn return $value; } + public function getFilter() + { + return $this->filter; + } + } diff --git a/src/admin/common/controllermappings/cmp.php b/src/admin/common/controllermappings/cmp.php new file mode 100644 index 0000000..04a48c4 --- /dev/null +++ b/src/admin/common/controllermappings/cmp.php @@ -0,0 +1,14 @@ +getCmd($name); + } + +} diff --git a/src/admin/common/controllermappings/float.php b/src/admin/common/controllermappings/float.php new file mode 100644 index 0000000..a3167ae --- /dev/null +++ b/src/admin/common/controllermappings/float.php @@ -0,0 +1,14 @@ +getFloat($name); + } + +} diff --git a/src/admin/common/controllermappings/int.php b/src/admin/common/controllermappings/int.php new file mode 100644 index 0000000..bac15d7 --- /dev/null +++ b/src/admin/common/controllermappings/int.php @@ -0,0 +1,14 @@ +getInt($name); + } + +} diff --git a/src/admin/common/controllermappings/ref.php b/src/admin/common/controllermappings/ref.php new file mode 100644 index 0000000..24d9d3f --- /dev/null +++ b/src/admin/common/controllermappings/ref.php @@ -0,0 +1,44 @@ +factory = $factory; + } + + public function getFilteredValue($input, $name) + { + return $input->getInt($name); + } + + public function rawValueValid($value) + { + try + { + $this->factory->loadById((int) $value); + } + catch(ElementNotFoundException $e) + { + return false; + } + + return true; + } + +} diff --git a/src/admin/common/controllermappings/string.php b/src/admin/common/controllermappings/string.php new file mode 100644 index 0000000..5f14cfa --- /dev/null +++ b/src/admin/common/controllermappings/string.php @@ -0,0 +1,14 @@ +getString($name); + } + +} diff --git a/src/admin/common/models/column/ref.php b/src/admin/common/models/column/ref.php index 18b79e3..fe30bff 100644 --- a/src/admin/common/models/column/ref.php +++ b/src/admin/common/models/column/ref.php @@ -14,9 +14,9 @@ class CommonClubsModelColumnRef extends AbstractCommonClubsModelColumn protected $className; - public function __construct($alias, $className, $required=true, $column=null) + public function __construct($alias, $className, $column=null) { - parent::__construct($alias, $required, $column); + parent::__construct($alias, $column); if(empty($className)) throw new Exception('Classname must be non-empty.'); diff --git a/src/admin/common/models/factory/offer.php b/src/admin/common/models/factory/offer.php index 4f3a875..e75bdc9 100644 --- a/src/admin/common/models/factory/offer.php +++ b/src/admin/common/models/factory/offer.php @@ -8,7 +8,7 @@ class CommonClubsModelFactoryOffer extends AbstractCommonClubsModelFactory protected function fetchAttributes() { return array( - new CommonClubsModelColumnString('name') + new CommonClubsModelColumnString('name', new CommonClubsControllerMappingString('Bezeichnung')) ); } diff --git a/src/admin/common/models/factory/position.php b/src/admin/common/models/factory/position.php index 651a856..13395af 100644 --- a/src/admin/common/models/factory/position.php +++ b/src/admin/common/models/factory/position.php @@ -8,7 +8,7 @@ class CommonClubsModelFactoryPosition extends AbstractCommonClubsModelFactory protected function fetchAttributes() { return array( - new CommonClubsModelColumnString('name') + new CommonClubsModelColumnString('name', new CommonClubsControllerMappingString('Bezeichnung')) ); } diff --git a/src/admin/controllers/club.php b/src/admin/controllers/club.php index 8083f6a..2983c69 100644 --- a/src/admin/controllers/club.php +++ b/src/admin/controllers/club.php @@ -6,11 +6,12 @@ defined('_JEXEC') or die; class ClubsControllerClub extends AbstractClubsController { - protected function getNameOfElement() + + protected function getSingleBaseName() { return 'club'; } - + protected function getDataMapping() { return array( @@ -27,7 +28,7 @@ class ClubsControllerClub extends AbstractClubsController ); } - protected function filterPreCheck(&$values) + protected function filterRawCheck(&$values) { if(is_null($values['charitable'])) $values['charitable'] = false; @@ -41,4 +42,10 @@ class ClubsControllerClub extends AbstractClubsController { $values['president'] = $values['president']->getId(); } + protected function getFactory() + { + return new CommonClubsModelFactoryClub(); + } + + } diff --git a/src/admin/controllers/offer.php b/src/admin/controllers/offer.php index 384ba1e..323c928 100644 --- a/src/admin/controllers/offer.php +++ b/src/admin/controllers/offer.php @@ -6,18 +6,15 @@ defined('_JEXEC') or die; class ClubsControllerOffer extends AbstractClubsController { - protected function getNameOfElement() + protected function getSingleBaseName() { return 'offer'; } - protected function getDataMapping() + protected function getFactory() { - return array( - 'name' => array('required'=>true, 'name'=>'Bezeichnung', 'filter'=>'string') - ); + return new CommonClubsModelFactoryOffer(); } - } diff --git a/src/admin/controllers/position.php b/src/admin/controllers/position.php index 2c0cf9a..3810df6 100644 --- a/src/admin/controllers/position.php +++ b/src/admin/controllers/position.php @@ -6,12 +6,6 @@ defined('_JEXEC') or die; class ClubsControllerPosition extends AbstractClubsController { - protected function getDataMapping() - { - return array( - 'name'=>array('required'=>true, 'filter'=>'string', 'name'=>'Bezeichung') - ); - } protected function getFactory() { return new CommonClubsModelFactoryPosition(); diff --git a/src/admin/controllers/user.php b/src/admin/controllers/user.php index 4cd1f0d..b2a2a67 100644 --- a/src/admin/controllers/user.php +++ b/src/admin/controllers/user.php @@ -42,9 +42,9 @@ class ClubsControllerUser extends AbstractClubsController * {@inheritDoc} * @see AbstractClubsController::checkData() */ - protected function checkDataIsValid($values, $isNew, $obj) + protected function rawDataIsValid($values, $isNew, $obj) { - if(! parent::checkDataIsValid($values, $isNew, $obj)) + if(! parent::rawDataIsValid($values, $isNew, $obj)) return false; // TODO Auto-generated method stub