Re: [Postfixadmin-devel] refactoring

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

Christian Boltz wrote :
> Hello,
> 
> Am Mittwoch, 23. Juli 2008 schrieb Christian Boltz:
> > My idea in short:
> > - create a basic class with the required functions/methods.
> 
> I have written a proof-of-concept class postfixadminBaseclass as 
> described in my proposal. It is attached to this mail.
> 
> Please note that the class does not contain real code yet, only the 
> structure and sometimes some dummy code. It also still misses some 
> (internal) functions like permission check.
> 
> Functions and variable definitions are currently sorted by action - this 
> needs to be changed to "variables first", but the current sorting is 
> easier for the initial development.
> 
> I did some small changes compared to the proposal in my mail - see the 
> comments inside the file for details.

Right, I'll include the code inline, as I'm lazy (and using mutt,
while my office computer is dead thanks to the thunder+lightening)

<?php

# template for all postfixadmin classes (admins, domains, mailboxes etc.)
#
# What it DOES:
# * handling of listing (database -> array)
# * handling of editing/adding items (database -> variables for item-to-edit and variables -> database)
# * input validation for editing/adding items
# * permission checks
# * accepts / returns data as variables/arrays
#
# What it DOES NOT:
# * rendering of lists -> table_class
# * rendering of edit forms -> form_class
# * output HTML etc.

class postfixadminBaseclass {

    protected function __construct() {
        $this->initStruct;
        $this->initDefaults;
    }

    # name, also used for generating links
    public function name($listmode=0) {
        # usually:
        return "baseclass";

        # for "virtual" list vs. "mailbox" editing
        # if ($listmode == 0) {
        #     return "mailbox";
        # } else {
        #     return "virtual";
        # }
    }

^^ What's name() trying to do?

    #  * [read-only] table structure as array (like $fm_struct in
    #    fetchmail.php)

^^ You love fetchmail.php don't you ? :)

    #  * function get_list() (for list view)
    #  * function get_list_for_domain() (for list view)
    #    - both get_list* should have an optional $search parameter
    #  -> I decided to merge this to one function
    #  -> "list" is a reserved word, switched to "items"
    public function items ($domain="", $admin="", $search="") {
        # get list from database
        # return array (array(key -> value), array(key -> value), ...)
    }

^^ OK, this makes sense.

    #  * function get_item() (current values, for edit form)
    public function item ($primarykey) {
        # get item from database
        # return array (key -> value)
    }

^^ Ditto; seems fine.

    # handles add and edit calls internally
    protected function addOrEdit($primarykey, Array $newvalues, $mode) {
        # mode: 0 = edit, 1 = new

        #    calls: [TODO]
        #    - [non-public] function validate_by_type() (simple check against
        #      field type given in table structure)

^^ I think this is unnecessary; database introspection might be a
better bet.

        #      -> see fetchmail.php _inp_*()
        #      -> also check that only allowed fields are set
        #    - [non-public] function validate_special() (other checks that are
        #      not covered by type check)

^^ OK; good plan.

    }

    # TODO
    #  * [non-public] check_domain_permission() (check if the admin has
    #    permissions for this domain)
    #  * [non-public] check_other_permission() (check other permissions, for
    #    example if editing mailbox aliases is allowed)
    #  * other non-public functions as needed - target should be to have most
    #    code in the common class and as least as possible in the
    #    mailbox/alias/whatever class.
    #    -> this also means that functions should be split into subparts where needed

It would be nice for the permission definitions to be done in one
place, where ever that may be.

Could permissions be listed a bit like :

e.g. 

/**
 * decorator for permission enforcement
 */
class PermissionProxy {
        public function __construct($real) {
                $this->real = $real;
        }
        $perms = array ( 'object.functionName' => array('role','role','role'),
                         'otherobject.somethingElse' => array('role', 'role'), // etc
                        )

        function __call($name, $values) {
                // see if $name is in permissions array, and role(s)
                // match.
                $key = get_class($this->real) . '.' $name;
                $permission = array_key_exists($key, $perms)) && ..  etc ..
                if($permission) {
                        return call_user_func_array(array($whatever,$name), $values);
                }
                else { 
                        throw new PermissionException("Access denied"); 
                }
        }

}

So, exposed via xmlrpc etc...

/**
 * phpdoc needs adding IRL 
 * clients (whether 'local' or via xmlrpc/soap etc) connect to this.
 */
class AliasServer {
        protected $backend = null;
        public function __construct() {
                $this->backend = new PermissionProxy(new Alias());
        }

        public function addAlias($p, $p, $p) {
                return $this->backend->addAlias($u, $p);
        }

        public function deleteAlias($u, $p) {
                return $this->backend->deleteAlias($admin);
        }
        public function getAliasList($domain, $offset, $limit) {
                return $this->backend->getAliasList($domain, $offset, $limit);
        }

        public function search($domain, $string) {
                // whatever
        }
}
/* The above isn't quite right yet, as $username/$password need
 * passing in to each method call... as xmlrpc isn't very stateful ...
 * perhaps my decorator idea can't/won't work
 */

Then for the actual Model object (e.g. Alias) :

/*
 * see http://framework.zend.com/manual/en/zend.db.table.html 
 * perhaps have a PostfixAdminAbstract class with additional
 * functionality over Zend_Db_Abstract in it for Alias to inherit from?
 */
class Alias extends Zend_Db_Abstract {
        protected $_name = 'alias';
        protected $_primary = 'id'; // whatever the pkey field is
        protected $_sequence = false; // force use of pkey field value in insert .
        public function __construct() {
                $db = Zend_Db::factory('....whatever...'); //?
                parent::__construct($db); 
        }

        // Zend_Db_Abstract provides:
        // insert(array(key=>value, key=>value ... ));
        // update($data, $where);
        // delete($where);
        // find($pkey);
        // fetchall($select_query);

        // perhaps we'd want to wrap e.g. update in a call like ...
        public function edit($pkey, $data) {
                // perform validation check(s); throw some sort of
                // exception if they fail.
                return $this->update($data, $this->getAdapter()->quoteInto('pkey = ?', $pkey));
        }
}

> Comments welcome: (ditto)
> - do you think this could become the basis of all postfixadmin pages?

I'm keen to separate a page from the underlying logic used for it. We
should provide e.g. xmlrpc or soap integration, and decouple the
business logic from the controller scripts that generate html pages. 
Having some sort of base controller and/or model class will be a good
thing.

> - what should I/we do better? (We have 35 °C outside, so I'm sure the 
>   baseclass isn't perfect ;-)

Heh; it was warm here, but also humid (as is nearly always the case in
the UK); thankfully we've just had a few thunderstorms and things seem
better. The dog wasn't happy though.

I think a good start is perhaps to list all classes, and the functions
they need :

e.g.

Vacation:
  - isEnabled($email);
  - create($email, $subject, $body, $whatever);
  - delete($email, $subject, $body, $whatever);
  - retrieve($email);

Domain:
  - list($offset, $limit = 20)
  - retrieve($name);
  - create($name, $backup_mx, etc);
  - delete($name);

Mailbox:
  - list($offset, $limit = 20);
  - retrieve($name);
  - create($name, $fullname, $password, ... etc)
  - delete($name);

etc

There would need to be some sort of shared 'authentication'/'user'
data/object so we can enforce permission checks (e.g that a user can only
delete their own mailbox, and not someone elses; but a global admin
can delete any... etc)

</ramble>

And so on,

David.
-- 
David Goodwin 

[ david at codepoets dot co dot uk ]
[ http://www.codepoets.co.uk       ]