First approach to port user classes to abstract MVC classes as well

src/admin/abstract/controller.php

@@ -34,7 +34,7 @@ abstract class AbstractClubsController extends BaseController
         $this->filterPreCheck($values);
         
         // Check the input data
-        $error = $this->checkData($values);
+        $error = ! $this->checkDataIsValid($values, true);
         
         $view = $this->getNameOfView();
         
@@ -66,7 +66,7 @@ abstract class AbstractClubsController extends BaseController
         $this->filterPreCheck($values);
         
         // Check the input data
-        $error = $this->checkData($values);
+        $error = ! $this->checkDataIsValid($values, false);
         
         $view = $this->getNameOfView();
         
@@ -101,7 +101,7 @@ abstract class AbstractClubsController extends BaseController
     
     protected function filterPreCheck(&$values){}
     
-    protected function checkData($values)
+    protected function checkDataIsValid($values, bool $isNew)
     {
         $error = false;
         // Check existence of the required fields
@@ -119,7 +119,7 @@ abstract class AbstractClubsController extends BaseController
             }
         }
         
-        return $error;
+        return ! $error;
     }
     
     protected function fieldValid($name, $value, $options)
@@ -142,18 +142,29 @@ abstract class AbstractClubsController extends BaseController
     
     private function packData($values)
     {
+        // FIXME Multiple bugs: filtering not working as expected and Mapping msut be checked
+        $this->filterPrePacking($values);
+        
         $data = array();
-        foreach($this->getDataMapping() as $i)
+        foreach($this->getDataMapping() as $m => $i)
-            $data[$i] = $values[$i];
+            $data[$m] = $values[$m];
             
         return urlencode(json_encode($data));
     }
     
+    protected function filterPrePacking(&$values){}
+    
     public function applyData($obj, $values)
     {
         foreach($this->getDataMapping() as $m => $v)
         {
             $functionName = $this->getSetterMethodName($m, $v);
+            
+            if($functionName === null)
+            {
+                continue;
+            }
+            
             $value = (isset($values[$m])) ? $values[$m] : null;
             $obj->$functionName($value);
         }

src/admin/controllers/user.php

@@ -1,207 +1,106 @@
 <?php
 
-use Joomla\CMS\MVC\Controller\BaseController;
 use Joomla\CMS\Factory;
-use Joomla\CMS\Router\Route;
 
 // No direct access.
 defined('_JEXEC') or die;
 
-class ClubsControllerUser extends BaseController
+class ClubsControllerUser extends AbstractClubsController
 {
     
-    function new()
-    {
-        $app = Factory::getApplication();
-        $input = $app->input;
-        $u = ClubsUser::createUser();
-        
-        // Fetch the posted data
-        $user = $input->post->getCmd('user');
-        $pwd = $input->post->getCmd('pwd');
-        $pwdConfirm = $input->post->getCmd('pwd-confirm');
-        $name = $input->post->getString('name');
-        $address = $input->post->getString('address');
-        $city = $input->post->getString('city');
-        $mail = $input->post->getString('mail');
-        $phone = $input->post->getString('phone');
-        $mobile = $input->post->getString('mobile');
-        
-        // Check the input data
-        $error = false;
-        
-        if(empty($user))
-        {
-            $app->enqueueMessage("Es muss ein Benutzername angegeben werden.", 'error');
-            $error = true;
-        }
-        else
-        {
-            if(! $this->checkUserName(trim($user)))
-            {
-                $app->enqueueMessage('Username ' . $user . ' ist nicht gültig.', 'error');
-                $error = true;
-            }
-            else
-                $u->setUser($user);
-        }
-        
-        $pwderr = false;
-        if(isset($pwd))
-        {
-            if(trim($pwd) != trim($pwdConfirm))
-            {
-                $app->enqueueMessage('Die Passwörter stimmen nicht überein.', 'error');
-                $error = true;
-                $pwderr = true;
-            }
-            
-            if(! $u->checkPassword(trim($pwd)))
-            {
-                $app->enqueueMessage('Das Passwort ist nicht zulässig.', 'error');
-                $error = true;
-                $pwderr = true;
-            }
-            
-            if(! $pwderr)
-                $u->setPassword(trim($pwd));
-        }
-        
-        // Check existence of the other fields
-        $fields = array('name'=>'Bürgerlicher Name', 'address'=>'Adresse', 'city'=>"Stadt", 'mail'=>'E-Mail');
-        foreach ($fields as $f => $fname)
-        {
-            $fvalue = $$f;
-            if(! isset($fvalue) || empty(trim($fvalue)))
-            {
-                $app->enqueueMessage("Das Feld $fname ist obligatorisch.", 'error');
-                $error = true;
-            }
-        }
-        
-        $u->setName($name);
-        $u->setAddress($address);
-        $u->setCity($city);
-        $u->setMail($mail);
-        $u->setPhone($phone);
-        $u->setMobile($mobile);
-        
-        if($error)
-        {
-            $data = array();
-            foreach(array('user', 'name', 'address', 'city', 'mail', 'phone', 'mobile') as $i)
-                $data[$i] = $$i;
-                
-                $urldata = urlencode(json_encode($data));
-                $this->setRedirect(Route::_('index.php?option=com_clubs&view=user&id=new&data=' . $urldata, false));
-                return;
-        }
-        
-        // Do the actual work
-        $u->save();
-        $this->setRedirect(Route::_('index.php?option=com_clubs&view=users', false));
-    }
-    
-    function change()
-    {
-        $app = Factory::getApplication();
-        $input = $app->input;
-        $id = (int) $input->post->getInt('id');
-        $u = ClubsUser::loadUser((int) $id);
-        
-        // Fetch the posted data
-        $user = $input->post->getCmd('user');
-        $pwd = $input->post->getCmd('pwd');
-        $pwdConfirm = $input->post->getCmd('pwd-confirm');
-        $name = $input->post->getString('name');
-        $address = $input->post->getString('address');
-        $city = $input->post->getString('city');
-        $mail = $input->post->getString('mail');
-        $phone = $input->post->getString('phone');
-        $mobile = $input->post->getString('mobile');
-        
-        // Check the input data
-        $error = false;
-        
-        if(! empty($user))
-        {
-            if(! $this->checkUserName(trim($user), $id))
-            {
-                $app->enqueueMessage('Username ' . $user . ' ist nicht gültig.', 'error');
-                $error = true;
-            }
-            
-            $u->setUser($user);
-        }
-        
-        if(isset($pwd))
-        {
-            if(trim($pwd) != trim($pwdConfirm))
-            {
-                $app->enqueueMessage('Die Passwörter stimmen nicht überein.', 'error');
-                $error = true;
-            }
-            
-            if(! empty(trim($pwd)))
-            {
-                if(! $u->checkPassword(trim($pwd)))
-                {
-                    $app->enqueueMessage('Das Passwort ist nicht zulässig.', 'error');
-                    $error = true;
-                }
-                
-                $u->setPassword(trim($pwd));
-            }
-        }
-        
-        // Check existence of the other fields
-        $fields = array('name'=>'Bürgerlicher Name', 'address'=>'Adresse', 'city'=>"Stadt", 'mail'=>'E-Mail');
-        foreach ($fields as $f => $fname)
-        {
-            $fvalue = $$f;
-            if(! isset($fvalue) || empty(trim($fvalue)))
-            {
-                $app->enqueueMessage("Das Feld $fname ist obligatorisch.", 'error');
-                $error = true;
-            }
-        }
-        
-        $u->setName($name);
-        $u->setAddress($address);
-        $u->setCity($city);
-        $u->setMail($mail);
-        $u->setPhone($phone);
-        $u->setMobile($mobile);
-        
-        if($error)
-        {
-            $data = array();
-            foreach(array('user', 'name', 'address', 'city', 'mail', 'phone', 'mobile') as $i)
-                $data[$i] = $$i;
-            
-            $urldata = urlencode(json_encode($data));
-            $this->setRedirect(Route::_('index.php?option=com_clubs&view=user&id=' . $id . '&data=' . $urldata, false));
-            return;
-        }
-        
-        // Do the actual work
-        $u->save();
-        $this->setRedirect(Route::_('index.php?option=com_clubs&view=users', false));
-    }
-    
-    function delete()
-    {
-        $app = Factory::getApplication();
-        $id = $app->input->get->getInt('id');
-        $app->enqueueMessage("Removal of user with id $id.");
-        $user = ClubsUser::loadUser($id);
-        $user->delete();
-        $this->setRedirect(Route::_('index.php?option=com_clubs&view=users', false));
-    }
-    
     private function checkUserName($username, $id = -1)
     {
         return ClubsUser::isUserNameFree($username, $id);
     }
     
+    protected function getNameOfElement()
+    {
+        return 'user';
+    }
+
+    protected function getDataMapping()
+    {
+        return array(
+            'user'=>array('required'=>true, 'name'=>'Benutzername', 'filter'=>'cmd'),
+            'pwd'=>array('required'=>false, 'name'=>'Passwort', 'filter'=>'string', 'setter'=>'setPassword'),
+            'pwdConfirm'=>array('required'=>false, 'name'=>'Passwortwiederholung', 'filter'=>'string', 'setter'=>null),
+            'name'=>array('required'=>true, 'name'=>'Benutzername', 'filter'=>'string'),
+            'address'=>array('required'=>true, 'name'=>'Adresse', 'filter'=>'string'),
+            'city'=>array('required'=>true, 'name'=>'Stadt', 'filter'=>'string'),
+            'mail'=>array('required'=>true, 'name'=>'E-Mail', 'filter'=>'string'),
+            'phone'=>array('required'=>false, 'name'=>'Telefonnummer', 'filter'=>'string'),
+            'mobile'=>array('required'=>false, 'name'=>'Handynummer', 'filter'=>'string')
+            
+        );
+    }
+    /**
+     * {@inheritDoc}
+     * @see AbstractClubsController::checkData()
+     */
+    protected function checkDataIsValid($values, $isNew)
+    {
+        if(! parent::checkDataIsValid($values, $isNew))
+            return false;
+        
+        // TODO Auto-generated method stub
+        if(isset($values['pwd']))
+        {
+            $pwd = $values['pwd'];
+            $pwdConfirm = $values['pwdConfirm'];
+            
+            if(trim($pwd) != trim($pwdConfirm))
+            {
+                Factory::getApplication()->enqueueMessage('Die Passwörter stimmen nicht überein.', 'error');
+                return false;
+            }
+            
+            if(! ClubsUser::checkPasswordStrength(trim($pwd)))
+            {
+                Factory::getApplication()->enqueueMessage('Das Passwort ist zu schwach.', 'error');
+                return false;
+            }
+            
+        }
+        else
+        {
+            if($isNew)
+            {
+                Factory::getApplication()->enqueueMessage('Für einen neuen Benutzer muss ein Passwort vergeben werden.', 'error');
+                return false;
+            }
+        }
+        
+        if(! $this->checkUserName(trim($values['user'])))
+        {
+            Factory::getApplication()->enqueueMessage('Username ' . $$values['user'] . ' ist nicht gültig.', 'error');
+            return false;
+        }
+        
+        return true;
+    }
+    
+    /**
+     * {@inheritDoc}
+     * @see AbstractClubsController::filterPrePacking()
+     */
+    protected function filterPrePacking(&$values)
+    {
+        parent::filterPrePacking($values);
+        unset($values['pwd']);
+        unset($values['pwdConfirm']);
+    }
+    
+    /**
+     * {@inheritDoc}
+     * @see AbstractClubsController::applyData()
+     */
+    public function applyData($obj, $values)
+    {
+        // TODO Auto-generated method stub
+        parent::applyData($obj, $values);
+    }
+
+
+
+
 }

src/admin/mymodels/user.php

@@ -11,11 +11,10 @@ class UserInvalidException extends Exception
 class PasswordInvalidException extends Exception
 {}
 
-class ClubsUser
+class ClubsUser extends AbstractClubsModel
 {
-    protected $id;
     protected $user;
-    protected $hash;
+    protected $password;
     protected $name;
     protected $address;
     protected $city;
@@ -34,19 +33,11 @@ class ClubsUser
     /**
      * @param string $mail
      */
-    public function setMail(string $mail)
+    public function setMail($mail)
     {
         $this->mail = $mail;
     }
 
-    /**
-     * @return int
-     */
-    public function getId()
-    {
-        return $this->id;
-    }
-
     /**
      * @return string
      */
@@ -60,7 +51,7 @@ class ClubsUser
      */
     public function getHash()
     {
-        return $this->hash;
+        return $this->password;
     }
 
     /**
@@ -106,7 +97,7 @@ class ClubsUser
     /**
      * @param string $user
      */
-    public function setUser(string $user, bool $force = false)
+    public function setUser($user, bool $force = false)
     {
         if($this->id === 'new')
             $valid = self::isUserNameFree($user);
@@ -124,19 +115,19 @@ class ClubsUser
      */
     public function setPassword(string $password)
     {
-        if(! $this->checkPassword($password))
+        if(! $this->checkPasswordStrength($password))
             throw new PasswordInvalidException();
         
-        $this->hash = password_hash($password, PASSWORD_DEFAULT);
+        $this->password = password_hash($password, PASSWORD_DEFAULT);
     }
     
     public function isPasswordValid(string $password)
     {
-        $valid = password_verify($password, $this->hash);
+        $valid = password_verify($password, $this->password);
         
         if($valid)
         {
-            $this->checkHash($password);
+            $this->checkForRehashing($password);
         }
         
         return $valid;
@@ -145,7 +136,7 @@ class ClubsUser
     /**
      * @param string $name
      */
-    public function setName(string $name)
+    public function setName($name)
     {
         $this->name = $name;
     }
@@ -153,7 +144,7 @@ class ClubsUser
     /**
      * @param string $address
      */
-    public function setAddress(string $address)
+    public function setAddress($address)
     {
         $this->address = $address;
     }
@@ -161,7 +152,7 @@ class ClubsUser
     /**
      * @param string $city
      */
-    public function setCity(string $city)
+    public function setCity($city)
     {
         $this->city = $city;
     }
@@ -169,7 +160,7 @@ class ClubsUser
     /**
      * @param string $phone
      */
-    public function setPhone(string $phone)
+    public function setPhone($phone)
     {
         $this->phone = $phone;
     }
@@ -177,7 +168,7 @@ class ClubsUser
     /**
      * @param string $mobile
      */
-    public function setMobile(string $mobile)
+    public function setMobile($mobile)
     {
         $this->mobile = $mobile;
     }
@@ -185,60 +176,17 @@ class ClubsUser
     protected function __construct()
     {}
     
+    private const tableName = '#__club_users';
+    private const className = 'ClubsUser';
+    
     public static function loadUsers()
     {
-        $dbo = Factory::getDbo();
+        return self::loadElements(self::tableName, self::className);
-        $q = $dbo->getQuery(true);
-        $q->select('*')
-          ->from('#__club_users');
-        $dbo->setQuery($q);
-        $dbo->execute();
-        $list = $dbo->loadAssocList('id');
-        
-        $ret = array();
-        foreach($list as $u)
-        {
-            $uo = new ClubsUser($dbo);
-            $uo->loadData($u);
-            
-            $ret[] = $uo;
-        }
-        
-        return $ret;
-    }
-    
-    protected function loadData(array $data)
-    {
-        $this->id = $data['id'];
-        $this->user = $data['user'];
-        $this->hash = $data['password'];
-        $this->name = $data['name'];
-        $this->address = $data['address'];
-        $this->city = $data['city'];
-        $this->mail = $data['mail'];
-        $this->phone = isset($data['phone']) ? $data['phone'] : null;
-        $this->mobile = isset($data['mobile']) ? $data['mobile'] : null;
     }
     
     public static function loadUser(int $id)
     {
-        $dbo = Factory::getDbo();
+        return self::loadElement($id, self::tableName, self::className);
-        $q = $dbo->getQuery(true);
-        $q->select('*')->from('#__club_users')->where('id=' . (int) $id);
-        $dbo->setQuery($q);
-        $dbo->execute();
-        
-        $row = $dbo->loadAssoc();
-        
-        if($row == null)
-        {
-            throw new Exception("No user found.");
-            // TODO
-        }
-        
-        $user = new ClubsUser();
-        $user->loadData($row);
-        return $user;
     }
     
     public static function createUser()
@@ -248,54 +196,20 @@ class ClubsUser
         return $user;
     }
     
-    public function save()
-    {
-        if($this->id === 'new')
-            $this->insertUser();
-        else
-            $this->updateUser();
-    }
-        
-    private function insertUser()
-    {
-        $dbo = Factory::getDbo();
-        $q = $dbo->getQuery(true);
-        
-        $vuser = $q->q($this->user);
-        $vpassword = $q->q($this->hash);
-        $vname = $q->q($this->name);
-        $vaddress = $q->q($this->address);
-        $vcity = $q->q($this->city);
-        $vmail = $q->q($this->mail);
-        $vphone = empty($this->phone) ? 'NULL' : $q->q($this->phone);
-        $vmobile = empty($this->mobile) ? 'NULL' : $q->q($this->mobile);
-        
-        $q->insert('#__club_users')
-          ->columns(array('user', 'password', 'name', 'address', 'city', 'mail', 'phone', 'mobile'))
-          ->values("$vuser, $vpassword, $vname, $vaddress, $vcity, $vmail, $vphone, $vmobile")
-          ;
-        
-        $dbo->transactionStart();
-        $dbo->setQuery($q);
-        $dbo->execute();
-        $this->id = $dbo->insertid();
-        $dbo->transactionCommit();
-    }
-    
     private function updateUser()
     {
         $dbo = Factory::getDbo();
         $q = $dbo->getQuery(true);
         
         $vuser = $q->q($this->user);
-        $vpassword = $q->q($this->hash);
+        $vpassword = $q->q($this->password);
         $vname = $q->q($this->name);
         $vaddress = $q->q($this->address);
         $vcity = $q->q($this->city);
         $vmail = $q->q($this->mail);
         $vphone = empty($this->phone) ? 'NULL' : $q->q($this->phone);
         $vmobile = empty($this->mobile) ? 'NULL' : $q->q($this->mobile);
-        
+        // FIXME Check null vlaues
         $q->update('#__club_users')
             ->set(array(
                 "user=$vuser",
@@ -314,43 +228,29 @@ class ClubsUser
         $dbo->execute();
     }
     
-    private function checkHash(string $password)
+    private function checkForRehashing(string $password)
     {
         if($this->id === 'new')
             return;
         
-        if(password_needs_rehash($this->hash, PASSWORD_DEFAULT))
+        if(password_needs_rehash($this->password, PASSWORD_DEFAULT))
         {
-            $this->hash = password_hash($password, PASSWORD_DEFAULT);
+            $this->password = password_hash($password, PASSWORD_DEFAULT);
             
             $dbo = Factory::getDbo();
             
             $q = $dbo->getQuery(true);
-            $q->update('#__club_users')->set('password=' . $q->q($this->hash))->where('id=' . (int) $this->id);
+            $q->update(self::tableName)->set('password=' . $q->q($this->password))->where('id=' . (int) $this->id);
             $dbo->setQuery($q);
             $dbo->execute();
         }
     }
     
-    public function delete()
-    {
-        if($this->id === 'new')
-            return;
-        $dbo = Factory::getDbo();
-        
-        $q = $dbo->getQuery(true);
-        $q->delete('#__club_users')
-          ->where('id=' . (int) $this->id);
-        
-        $dbo->setQuery($q);
-        $dbo->execute();
-    }
-    
     public static function isUserNameFree($username, int $id = -1)
     {
         $db = Factory::getDbo();
         $q = $db->getQuery(true);
-        $q->select('COUNT(*)')->from('#__club_users')
+        $q->select('COUNT(*)')->from(self::tableName)
             ->where('id <> ' . (int) $id)
             ->where('user = ' . $q->q($username));
         $db->setQuery($q);
@@ -359,7 +259,7 @@ class ClubsUser
         return $row[0] == 0;
     }
     
-    public function checkPassword($pwd)
+    public static function checkPasswordStrength($pwd)
     {
         if(strlen($pwd) < 6)
             return false;
@@ -375,4 +275,20 @@ class ClubsUser
                 
         return true;
     }
+    
+    protected function getDataMappings()
+    {
+        return array('user', 'password', 'name', 'address', 'city', 'mail', 'phone', 'mobile');
+    }
+    
+    protected function getRequiredDataMappings()
+    {
+        return array('user', 'password', 'name', 'address', 'city', 'mail');
+    }
+
+    protected function getTableName()
+    {
+        return self::tableName;
+    }
+
 }

src/admin/views/user/tmpl/default.php

@@ -20,7 +20,7 @@ defined('_JEXEC') or die;
   </tr>
   <tr>
     <td>Passwort wiederholen</td>
-    <td><input type='password' name='pwd-confirm' value=''></td>
+    <td><input type='password' name='pwdConfirm' value=''></td>
   </tr>
   <tr>
     <td>Bürgerlicher Name</td>