Include support for banner/ajax/load controller (enterprise specific)

[davidalger]

trafficstars

Have a Magento Commerce 2.2 site which I just rolled out a modified version of this extension on a short while ago. Enterprise has a banner/ajax/load call which similarly reads from session, but never writes, so the changes here are adding support for that.

On my local test site with a sleep(5) added to exaggerate requests to both banner/ajax/load and customer/section/load for easily testing the lock release I get the following results:

Before installation (click to expand image):

After modified module is installed (click to expand image):

For now, I have this installed on the site by adding the following into the composer.json file to pull the modified module:

{
    "require": {
        "integer-net/magento2-session-unblocker": "dev-magento-enterprise-support as v0.1.4"
    },
    "minimum-stability": "stable",
    "repositories": {
        "magento2-session-unblocker": {
            "type": "vcs",
            "url": "https://github.com/davidalger/magento2-session-unblocker"
        }
    }
}

Given these changes introduce a hard dependency on the Commerce/Enterprise only Magento\Banner\Controller\Ajax\Load class in tests/Integration/AbstractSessionTest.php, the integration tests will fail if they are run from an open-source code base. DI rules targeting non-existent classes are merely ignored, so the module will install and run no problem on open-source however (verified this on a vanilla 2.3.3 OS with sample data).

I'm opening this PR both to share the work that I've done, but also ask for ideas on path forward to supporting controllers that do not exist in open-source. Have you guys thought about this yet and what might you suggest to support both open-source and enterprise?

One thought I had is perhaps splitting it to a separate module dependent on this one, keeping them nice and clean but that has the downside of multiple modules to maintain. Alternatively, the tests could be setup so only the open-source tests are run perhaps by using a different phpunit.xml file for enterprise and opensource integration testing where the enterprise one included additional tests, and then splitting AbstractSessionTest::setUpSpies out of the abstract to avoid hard dependencies on irrelevant controller classes.

Thoughts?

[schmengler]

Thank you very much for your contribution! This brings up an interesting topic, I'd definitely like to support improvements for Magento Commerce, while staying compatible with the open source version.

One thought I had is perhaps splitting it to a separate module dependent on this one, keeping them nice and clean but that has the downside of multiple modules to maintain

I don't see this as a downside at all. In fact I prefer having a single extension, consisting of multiple modules with different dependencies, where you can choose which modules to install.

How would this work?

• Move the files of the base module IntegerNet\SessionUnblocker to src/SessionUnblocker and create a second module, e.g. IntegerNet\SessionUnblockerEnterpriseBanner in src/SessionUnblockerEnterpriseBanner.
• The dependency to Magento_Banner should be declared in module.xml of this new module.
• We can keep the single composer.json in the repository root and may add magento/module-banner as suggested dependency.
• Make it clear in the installation instructions, that the enterprise module(s) should be explicitly disabled via bin/magento module:disable (or by editing app/etc/config.php)
• Tests would be separeted in two namespaces as well, and configured as two test suites

[wojtekn]

@schmengler another approach could be about moving it to separate composer module which depends on this one and then there would be two different ways to install it for Open Source and Commerce eg.:

Open Source:

composer require integer-net/magento2-session-unblocker

Commerce:

composer require integer-net/magento2-session-unblocker-commerce