Hi,

When I find myself checking permissions and manipulating models I've always
moved the code to a service layer.

If the form with the controls is a User Administration form, then I would
have the controller extract the pertinent request data and pass it to the
UserManagement Service. The UserManagement service would validate the
request data, fetch the model, authorize the User to perform the action and
then return a constant. As an example for the return values based on the
code you gave:

UserManagement::SUCCESS
UserManagement::FAILURE_PERMISSION_DENIED
UserManagement::FAILURE_PERMISSION_UNKNOWN
UserManagement::FAILURE (generic)

The controller can then have a switch statement which runs through the
possible return values from User Management.

I prefer putting the logic into a service layer as I often find that:

1) There tend to be very common actions throughout a system, and I hate code
duplication between controllers.

2) Most of the sites I work on have several different access points to
Controllers (e.g REST/JSON servers), so having a service
avoids repetition between server types.

If your unfamiliar with Service layers then having a google around would be
a wise 30 minutes spent. Matthew has blogged about services within Zend a
few times and there are many good resources around. A quick example of how a
service layer  sits within an application can be found here:
http://www.angryobjects.com/wp-content/uploads/2009/03/diagram11.png

If your preference is for a Controller/Model only setup, then I would
deffinately keep _that_ code in the Controller. I believe in the fat model
thin controller theory, but you shouldn't make your Models depend on the
Controllers.

Alex.

On Mon, Jun 6, 2011 at 5:31 AM, Peter Sharp <[email protected]> wrote:

> I'm hitting my usual problem of getting something working and then
> wondering
> if I've done things the right way and put things in the right place.
>
> I'm just wondering how skinny a controller should be, as in 'skinny
> controller, fat model'.
>
> As an example, I have a table listing database rows with a form per row
> with
> some controls (move up, move down, delete). These are submitted to
> processAction(), which looks something like this.
>
>  public function processAction()
>    {
>        $request = $this->getRequest();
>        if($request->isPost()) {
>            $post = $request->getPost();
>
>            // Specify rows with target ids
>            $id_keys = array('row_id', 'up_id', 'down_id');
>            foreach($post as $key=>$value) {
>                if (in_array($key, $id_keys)) $toTest[] = $value;
>            }
>
>            // Verify all specified ids are OK to use
>            $mapper = new Vendor_Model_Mapper_Size();
>            $keyTest = $mapper->hasPermission($toTest, $this->vendor);
>
>            if ($keyTest) {
>                // SUCCESS - Permission Granted
>                if(isset($post['promote_x'])) {
>                    if (isset($post['row_id']) && isset($post['up_id'])) {
>                        if (!$mapper->swapSortOrder($post['row_id'],
> $post['up_id'])) {
>                            // FAILURE - Unknown Error
>                            $this->_helper->FlashMessenger('Process Failure
> - Unknown Error.');
>                        }
>                    } else {
>                        // FAILURE
>                        $this->_helper->FlashMessenger('Process Failure -
> Malformed Request.');
>                    }
>                }
>                if(isset($post['demote_x'])) {
>                    if (isset($post['row_id']) && isset($post['down_id'])) {
>                        if (!$mapper->swapSortOrder($post['row_id'],
> $post['down_id'])) {
>                            // FAILURE
>                            $this->_helper->FlashMessenger('Process Failure
> - Unknown Error.');
>                        }
>                    } else {
>                        // FAILURE
>                        $this->_helper->FlashMessenger('Process Failure -
> Malformed Request.');
>                    }
>                }
>                if(isset($post['delete_x']) && isset($post['row_id'])) {
>                    if ($mapper->deactivate($post['row_id'])) {
>                        // SUCCESS - Deactivated
>                        $this->_helper->FlashMessenger('Deleted.');
>                    } else {
>                        // FAILURE - Unknown Error
>                        $this->_helper->FlashMessenger('Process Failure -
> Unknown Error.');
>                    }
>                }
>            } else {
>                // FAILURE - Permission Denied
>                $this->_helper->FlashMessenger('Permission Denied.');
>            }
>
>        }
>
>        return $this->_helper->redirector->gotoRoute(array(
>            'resource' => $request->getParam('resource'),
>            'controller' => 'size',
>            'action' => 'index'
>        ), 'vendorResource');
>
>    }
>
> Now, it seems pretty messy to me (made worse by the fact it needs a going
> over either way), but at the same time, it kind of 'feels' like the
> controller is the right place to handle interaction between the model and
> the view. But is this too 'fat'? How fat is too fat for a controller?
>
> An alternative I have been considering is pushing the bulk of that logic
> into the mapper and giving it a set of message constants, much like a
> validator. So the new mapper function would return true or false, with the
> error/success message exposed via a getter.
>
> Then I could lose the process action and just pass the form output via
> indexAction to the new function and then set the output based on a simple
> pass or fail.
>
> This is where it gets murky for me. It would certainly make my controller
> skinnier, but it is really the mappers place to be handling form input.
> Well
> ... possibly. Most of the failures are triggered from database
> interactions.
> A lot of it is based on the database failing to return the correct result.
>
> Of course, having written this, it now 'feels' like the latter method is
> more correct. No doubt I'll feel the other way again in an hour or so.
>
> Any thoughts?
>
> --
> View this message in context:
> http://zend-framework-community.634137.n4.nabble.com/Where-does-this-code-belong-tp3576128p3576128.html
> Sent from the Zend Framework mailing list archive at Nabble.com.
>
> --
> List: [email protected]
> Info: http://framework.zend.com/archives
> Unsubscribe: [email protected]
>
>
>

Reply via email to