Optionally use xautoload for class loading. [#1829754]

Currently, htmlpurifier uses its own mechanism for class autoload.
That's not a real problem. However, if too many custom class loaders pile up, it can have (minor) performance impacts.
It is probably a good idea to use an existing class loader instead, if one is available.

Drupal 7 (with contrib) currently has three options for class loading:
- The core one, which is based on file scans. Noone likes that, I do not seriously suggest using it.
- xautoload. That's a rewrite of the Symfony core class loader currently in Drupal 8.
- classloader. That's a direct port of the Drupal 8 Symfony class loader.

Seeing who is the author of xautoload, I probably don't need to tell you which one I prefer :)

Disclaimer:
I do not absolutely insist on the xautoload integration. I just came across it while playing around with libraries support, looking for a real-world example.
I did not test whether or how much it benefits from using a shared class loader.

Comment	File	Size	Author
#1	htmlpurifier-7.x-1.x-xautoload.patch	828 bytes	donquixote
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

donquixote CreditAttribution: donquixote commented 2 November 2012 at 03:54

Status:

Active

» Needs review

File	Size
htmlpurifier-7.x-1.x-xautoload.patch	828 bytes

The patch is for the 7.x-1.x branch.
I did not really test the 7.x-2.x branch yet.

X Autoload offers three methods of registering additional prefixes:

Explicit registration via
xautoload_get_finder()->registerPrefixRoot('htmlpurifier', "$library_path/library");
Implicit registration via hook_xautoload().
Implicit registration via hook_libraries_info()

The patch below is using the first method, which is probably a good choice for htmlpurifier.

Comment #2

ezyang CreditAttribution: ezyang commented 3 November 2012 at 18:59

Status:

Needs review

» Patch (to be ported)

Looks basically reasonable, heddn, can you make sure it applies to the current code and push it?

Comment #3

heddn

English

Nicaragua

CreditAttribution: heddn commented 8 November 2012 at 21:31

I'm about to re-work htmlpurifier to use Libraries 2.0. I'm reading through the documentation for xautoload, would I eliminate the files php array? And would you suggest anything special for the post-load callback?

function htmlpurifier_libraries_info() {
  $libraries['htmlpurifier'] = array(
      'name' => 'HTML Purifier library',
      'vendor url' => 'http://htmlpurifier.org/',
      'download url' => 'http://htmlpurifier.org/download',
      'version arguments' => array(
        'file' => 'VERSION', 'pattern' => '@([0-9\.]+)@', 'lines' => 1, 'cols' => 8,
      ),
      'versions' => array(
          '4.4.0' => array(
              'files' => array(
                'php' => array(
                  'library/HTMLPurifier.auto.php',
                ),
              ),
          ),
      ),
      'callbacks' => array(
        'post-load' => array(
          'htmlpurifier_libraries_postload_callback',
        ),
      ),
  );
return $libraries;
}
function htmlpurifier_libraries_postload_callback(&$library, $version, $variant) {
  $module_path = drupal_get_path('module', 'htmlpurifier');
  require_once "$module_path/HTMLPurifier_DefinitionCache_Drupal.php";
  $factory = HTMLPurifier_DefinitionCacheFactory::instance();
  $factory->register('Drupal', 'HTMLPurifier_DefinitionCache_Drupal');
}

Comment #4

donquixote CreditAttribution: donquixote commented 8 November 2012 at 21:58

Your post makes me think :)
Last time I looked into htmlpurifier, I did not notice any hook_libraries_info() implementation.
And I did not know about the "post-load callback" and about the "files" array in that hook.

X Autoload does provide the "xautoload" key in hook_libraries_info(), but in the current design this means that those prefixes / namespaces will be registered to the class finder in every request on hook_init(). For htmlpurifier this might be overkill, because we don't need it on every request. On the other hand, it isn't really that big of a deal.

Alternatively, you can actively call xautoload_get_finder()->registerPrefixRoot(), if you want to choose the time when to register this stuff. This could happen in your "post-load" callback.

"files": I suppose having a file in the files php array is equivalent with requiring it in the post-load callback?
And this file is really only for the class loading, so if that is already taken care of, you don't need the "files" thing.

--------

My suggestion:
At first finish your rewrite to support libraries 2.x, with no respect to xautoload. You have your HTMLPurifier.auto.php, that's good enough for a start.

Once that is done, you can still consider if it is worth to optionally use xautoload (or classloader, if you prefer that). If you do that, you should probably still provide a fallback, in case that neither of these two is in place. Unless you want to add a dependency.

Comment #5

heddn

English

Nicaragua

CreditAttribution: heddn commented 9 November 2012 at 19:24

Version:	7.x-1.x-dev	» 7.x-2.x-dev
Status:	Patch (to be ported)	» Needs work

I've committed the changes to support Libraries 2.x #1817412: ExtractStyleBlocks requires invalid module. Did you want to poke around at the code and provide further suggestions? Moving this to 2.x (where all new features are getting added).

Alternatively, you can actively call xautoload_get_finder()->registerPrefixRoot(), if you want to choose the time when to register this stuff. This could happen in your "post-load" callback.

I'm leaning towards this option (and removing the files array(). All it does is tell libraries to require_once.