Zend Action Controller - refactoring strategy

Question

I've built a first-run web service on Zend Framework (1.10), and now I'm looking at ways to refactor some of the logic in my Action Controllers so that it will be easier for me and the rest of my team to expand and maintain the service.

I can see where there are opportunities for refactoring, but I'm not clear on the best strategies on how. The best documentation and tutorials on controllers only talk about small scale applications, and don't really discuss how to abstract the more repetitive code that creeps into larger scales.

The basic structure for our action controllers are:

1. Extract XML message from the request body - This includes validation against an action-specific relaxNG schema
2. Prepare the XML response
3. Validate the data in the request message (invalid data throws an exception - a message is added to the response which is sent immediately)
4. Perform database action (select/insert/update/delete)
5. Return success or failure of action, with required information

A simple example is this action which returns a list of vendors based on a flexible set of criteria:

class Api_VendorController extends Lib_Controller_Action
{  
    public function getDetailsAction()
    {
        try {
            $request = new Lib_XML_Request('1.0');
            $request->load($this->getRequest()->getRawBody(), dirname(__FILE__) . '/../resources/xml/relaxng/vendor/getDetails.xml');
        } catch (Lib_XML_Request_Exception $e) {
            // Log exception, if logger available
            if ($log = $this->getLog()) {
                $log->warn('API/Vendor/getDetails: Error validating incoming request message', $e);
            }

            // Elevate as general error
            throw new Zend_Controller_Action_Exception($e->getMessage(), 400);
        }

        $response = new Lib_XML_Response('API/vendor/getDetails');

        try {
            $criteria = array();
            $fields = $request->getElementsByTagName('field');
            for ($i = 0; $i < $fields->length; $i++) {
                $name = trim($fields->item($i)->attributes->getNamedItem('name')->nodeValue);
                if (!isset($criteria[$name])) {
                    $criteria[$name] = array();
                }
                $criteria[$name][] = trim($fields->item($i)->childNodes->item(0)->nodeValue);
            }

            $vendors = $this->_mappers['vendor']->find($criteria);
            if (count($vendors) < 1) {
                throw new Api_VendorController_Exception('Could not find any vendors matching your criteria');
            }

            $response->append('success');
            foreach ($vendors as $vendor) {
                $v = $vendor->toArray();
                $response->append('vendor', $v);
            }

        } catch (Api_VendorController_Exception $e) {
            // Send failure message
            $error = $response->append('error');
            $response->appendChild($error, 'message', $e->getMessage());

            // Log exception, if logger available
            if ($log = $this->getLog()) {
                $log->warn('API/Account/GetDetails: ' . $e->getMessage(), $e);
            }
        }

        echo $response->save();
    }
}

So - knowing where the commonalities are in my controllers, what's the best strategy for refactoring while keeping it Zend-like and also testable with PHPUnit?

I did think about abstracting more of the controller logic into a parent class (Lib_Controller_Action), but this makes unit testing more complicated in a way that seems to me to be wrong.

Answer 1

Two ideas (just creating an answer from the comments above):

1. Push commonality down into service/repository classes? Such classes would be testable, would be usable across controllers, and could make controller code more compact.

2. Gather commonality into action helpers.

Answer 2

Since you have to do this step every time a request is made, you could store that receive, parse and validate the received request in a Zend_Controller_Plugin which would be run every PreDispatch of all controllers. (Only do-able if your XML request are standardized) (If you use XMLRPC, REST or some standard way to build requests to your service, you could look forward those modules built in ZF)

The validation of the data (action specific) could be done in a controller method (which would then be called by the action(s) needing it) (if your parametters are specific to one or many actions of that controller) or you could do it with the patterns Factory and Builder in the case that you have a lot of shared params between controllers/actions

// call to the factory
$filteredRequest = My_Param_Factory::factory($controller, $action, $paramsArray) // call the right builder based on the action/controller combination

// the actual Factory

class My_Param_Factory 
{
    public static function factory($controller, $action, $params)
    {
        $builderClass = "My_Param_Builder_" . ucfirst($controller) . '_' . ucfirst($action);

        $builder = new $builderClass($params);

        return $builder->build();
    }
}

Your builder would then call specific parameters validating classes based on that specific builder needs (which would improve re-usability)

In your controller, if every required params are valid, you pass the processing to the right method of the right model

$userModel->getUserInfo($id) // for example

Which would remove all of the dataprocessing operations from the controllers which would only have to check if input is ok and then dispatch accordingly.

Store the results (error or succes) in a variable that will be sent to the view

Process the data (format and escape (replace < with &lt; if they are to be included in the response for example)), send to a view helper to build XML then print (echo) the data in the view (which will be the response for your user).

public function getDetailsAction()
{
    if ($areRequestParamsValid === true) {
        // process data
    } else {
        // Build specific error message (or call action helper or controller method if error is general)
    }

    $this->view->data = $result
}