Friday, September 03, 2010

Cleaning up Custom Drupal Module with Object Oriented PHP

Earlier this year, I wrote about a little Drupal custom module that intercepts a node as it is being saved in Drupal, and sends off an XML-RPC request to publish it to a Java based publishing system.

The XMLRPC call to the external publisher in my original implementation looked like this (if you are looking for context, the full working code for the initial version can be found on the post referenced above).

1
2
3
4
5
6
7
<?php
function send_request($server_url, $op, $node) {
  ...
  $result = xmlrpc($server_url, 'publisher.' . $op, 
    $node->nid, $node->title, $node->body);
  ...
}

Of course, almost right off the bat, sending just these three fields was found to be insufficient. For this module to be useful, I needed to send over large denormalized views of data to be published (sometimes spanning multiple nodes, depending on the content type). So the first attempt was to refactor this out to a prepare_node() call, like so, where prepare_node() delegates to content type specific methods and returns a PHP associative array (thats a map for you Java guys).

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
<?php
function send_request($server_url, $op, $node) {
  ...
  $result = xmlrpc($server_url, 'publisher.' . $op, prepare_node($node));
  ...
}

function prepare_node($node) {
  if ($node->type == 'foo') {
    return prepare_foo($node);
  } else if ($node->type == 'bar') {
    return prepare_bar($node);
  } else {
    watchdog('dxi', 'Unrecognized content type: ' . $node->type);
    return get_object_vars($node);
  }
}

function prepare_foo($node) {
  $fields = array();
  $fields['id'] = $node->nid;
  $fields['title'] = $node->title;
  $fields['body'] = $node->body;
  // ... etc.
  return $fields;
}

function prepare_bar($node) {
  // ... more of the same
}

This worked for us for a while, through our first implementation. Soon, however, as our project manager puts it, we became victims of our own success, and it was decided to roll out similar infrastructure for other partners as well. Each partner had their own set of different (sometimes overlapping) content types. At one point, it became too confusing to maintain this prepare_node() function, so I decided to refactor. Conceptually, I wanted something like this:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
<?php
function prepare_node($node, $server_url) {
  $partner_name = get_partner_from_server_url($server_url);
  if ($partner_name == 'partnerA') {
    if ($node->type == 'foo') {
      return prepare_partnerA_foo($node);
    } else if ($node->type == 'bar') {
      return prepare_partnerA_bar($node);
    } else {
      watchdog('dxi', 'Unrecognized type ' . $node->type . 
       ' for partner: ' . $partner_name);
      return get_object_vars($node);
    }
  } else if ($partner_name == 'partnerB') {
    // more of the same
  } else {
    watchdog('dxi', 'Unrecognized partner: ' . $partner_name);
    return get_object_vars($node);
  }
}

In our case, we decided to give each partner a separate publish URL so the publisher could distinguish between calls from different partners (and do different things), and the partner name can be derived using a get_partner_from_server_url() (not shown) using plain string manipulation.

None of us were very familiar with Object Oriented PHP, but we were familar with OOP, and the above code looked like it would convert nicely to a Factory pattern. So I adapted the idea in this code snippet, along with the technique mentioned in the user contributed notes to reflectively instantiate a partner/content type specific class given the partner name and content type.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
<?php
class ContentProcessor {
  protected function prepare_node($node) {
    return get_object_vars($node);
  }
}

abstract class ContentProcessorFactory {
  public function create($partnerName, $nodeType) {
    if ((!empty($partnerName)) && (!empty($nodeType))) {
      $className = "ContentProcessor_" . $partnerName . "_" . $nodeType;
      if (class_exists($className)) {
        return new $className;
      } else {
        watchdog("dxi", "No ContentProcessor found for " . $nodeType .
          " for partner: " . $partnerName);
        $className = "ContentProcessor_" . $partnerName;
        if (class_exists($className)) {
          return new $className;
        }
      }
    }
    watchdog("dxi", "No ContentProcessor found for (partner,type)=(" .
      $partnerName . ',' . $nodeType . ")");
    return new ContentProcessor();
  }
}

Processing code for each partner was moved into separate .inc files. Each partner contains a base ContentProcessor_${partnerName} class which extends ContentProcessor, and multiple ContentProcessor_${partnerName}_${nodeType} classes, one per content type, which extends ContentProcessor_${partnerName}. Something like this:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
<?php
class ContentProcessor_partnerA extends ContentProcessor {

  // common functions go here, these are called by its subclasses
  protected utility_function_1($arg1, $arg2) {
    // do some common stuff here
  }
}

class ContentProcessor_partnerA_foo extends ContentProcessor_partnerA {
  public function prepare_node($node) {
    // returns an array
    $fields = array();
    $fields['id'] = $node->nid;
    $fields['title'] = $node->title;
    $fields['body'] = $node->body;
    // ... etc.
    return $fields;
  }
}

class ContentProcessor_partnerA_bar extends ContentProcesor_partnerA {
  public function prepare_node($node) {
    // similar to above
  }
}

In the main (non OOP) module, we instantiate the appropriate ContentProcessor and delegate to its prepare_node($node) method, like this. The partner name is configured as part of the custom action's properties.

1
2
3
4
5
6
7
<?php
function prepare_node($server_url, $node) {
  $partner_name = get_partner_from_server_url($server_url);
  $contentProcessor = ContentProcessorFactory::create(
    $partner_name, $node->type);
  return $contentProcessor->prepare_node($node);
}

With the old setup, every time we needed to add logic for a new partner, a developer would have to add a bunch of if-else calls in the main prepare_node(), and add some more prepare_xxx() methods to the already bloated module file. Using the new setup, the developer would create a new ${partnerName}.inc file, create the top level ContentProcessor_${partnerName} file and its subclasses for each content type, include them in the main module with a require_once() call.

Since we put the get_object_vars() call (the else part of the original if..else in prepare_node()), it is still just as easy to experiment with a new content type (dumping out the field names before writing code). Additionally, the exceptions tell us exactly what class we have to implement.

1 comments (moderated to prevent spam):

Clara Vargas said...

As soon as I viewed this web story containing the Facebook audience claim that's off by 17,000 times my conclusion was that Salmon Run's viewers have to comment on this: http://hubpages.com/hub/Lies-Damned-Lies-Statistics-ComScores-Facebook-Audience-Claims-Are-Wrong-By-A-Factor-Of-17-000 According to ComScore, the average Facebook user is online half a second per day! Yeah, sure!