hps / heartland-php

Heartland Payment Systems Payment Gateway PHP SDK
https://developer.heartlandpaymentsystems.com/SecureSubmit/
GNU General Public License v2.0
25 stars 23 forks source link

inefficient autoloader #34

Open bkdotcom opened 6 years ago

bkdotcom commented 6 years ago

Installed via composer.

heartland-php's composer.json :

{
  "autoload":{
    "files": [
      "Hps.php"
    ]
  }
}

As a result my site now loads 184 files associated with the heartland-php sdk on every request (regardless of whether or not the request will even use the sdk, and regardless of what features I need)

the Hps.php file should only be necessary / used when composer isn't being used

Is there a reason heartland-php's composer.json doesn't define classmap rather than files?

ie

{
    "autoload": {
        "classmap": ["src/"]
    }
}

https://getcomposer.org/doc/04-schema.md#classmap

Files

If you want to require certain files explicitly on every request then you can use the files autoloading mechanism. This is useful if your package includes PHP functions that cannot be autoloaded by PHP.

(this isn't the case... the heartland-php sdk doesn't define any global functions. it only defines classes (and classes can be autoloaded)

Classmap

The classmap references are all combined, during install/update, into a single key => value array which may be found in the generated file vendor/composer/autoload_classmap.php. This map is built by scanning for classes in all .php and .inc files in the given directories/files.

You can use the classmap generation support to define autoloading for all libraries that do not follow PSR-0/4. To configure this you specify all directories or files to search for classes.

bkdotcom commented 6 years ago

FWIW, to "solve" / work around this issue, I've added a "post-autoload-dump" callback to my composer.json

"scripts": {
    "post-autoload-dump": "ComposerScripts::postAutoloadDump"
},

ComposerScripts.php:

<?php
/**
 * https://getcomposer.org/doc/articles/scripts.md
 */

use Composer\Script\Event;

class ComposerScripts
{
    public static function postAutoloadDump(Event $event)
    {
        $composer = $event->getComposer();
        $vendorDir = $composer->getConfig()->get('vendor-dir');

        $file = $vendorDir.'/composer/autoload_static.php';
        $str = file_get_contents($file);

        /*
            Remove Hps.php from files
        */
        $str = preg_replace('#^.+heartland-php/Hps.php\',\n#m', '', $str);

        /*
            Add heartland-php classmap
        */
        $hpsClassmap = \Composer\Autoload\ClassMapGenerator::createMap($vendorDir.'/hps/heartland-php/src');
        $hpsClassmap = var_export($hpsClassmap, true);
        // extract the part inside 'array()''
        $hpsClassmap = preg_replace('#array\s*\(\s+(.+)\)\s*#is', '$1', $hpsClassmap);
        // convert to relative filepaths
        $hpsClassmap = preg_replace('#\''.$vendorDir.'#', '__DIR__ . \'/..\' . \'', $hpsClassmap);
        // add leading whitespace
        $hpsClassmap = preg_replace('#^\s*#m', str_repeat(' ', 8), $hpsClassmap);
        $str = preg_replace(
            '#(\$classMap\s+=\s+array\s*\()
            ([^)]+)
            (\s{4}\))#x',
            '$1$2'.$hpsClassmap.'$3',
            $str
        );
        file_put_contents($file, $str);
    }
}
slogsdon commented 6 years ago

@bkdotcom Apologies for the delay, and thanks for the detailed report and proposed solution!

The current composer.json file we have today is from the initial addition of Composer support, where the previous version of the SDK before that needed an entry point (Hps.php). Doing so was a quick way to support Composer. We'll take a look on our end to see if the solution you've provided will work for all of our integrators, and if so, we'll push out an update as soon as we can.

As a note, our team is also building a new SDK that supports more Global Payments gateways across the organization, including Heartland, under a single interface. You can find the source on GitHub and the package on Packagist. It follows PSR-4 and shouldn't have the above problem. If you're interested in this, I can forward over a link to our documentation for this.