Add support for laminas-httphandlerrunner v2 series #82

weierophinney · 2021-12-14T17:41:37Z

This patch updates to the laminas/laminas-httphandlerrunner v2 series, which introduces a BC break.

To accomplish this, I have:

Changed the laminas/laminas-httphandlerrunner constraint from ^1.1 to ^2.1.
Modified the SwooleRequestHandlerRunner to implement Laminas\HttpHandlerRunner\RequestHandlerRunnerInterface, no longer extend Laminas\HttpHandlerRunner\RequestHandlerRunner (because it is marked final), and mark itself final.
Added a v4 documentation tree, and rewrote the migration chapter to cover v3 -> v4 migrations (which detail the above).

This is a BC break because:

The inheritance of SwooleRequestHandlerRunner has changed; hinting on its parent type no longer works the same.
You can no longer extend SwooleRequestHandlerRunner.

Fixes #78

- Adds `^2.1` to the list of laminas-httphandlerrunner constraints, while keeping the v1 constraint. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Extracts the `DEFAULT_PROCESS_NAME` constant into `RequestHandlerConstantsInterface`. Extracts the body of `SwooleRequestHandlerRunner` into `RequestHandlerRunnerTrait`. Updates `SwooleRequestHandlerRunner` to implement `RequestHandlerConstantsInterface` and use `RequestHandlerRunnerTrait` to define its work. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

…ests - Creates Mezzio\Swoole\RequestHandlerRunner\V2RequestHandlerRunner, which implements Laminas\HttpHandlerRunner\RequestHandlerRunnerInterface and RequestHandlerConstantsInterface, and uses the RequestHandlerRunnerTrait to create the implementation. - Extracts the SwooleRequestHandlerRunnerTest into an AbstractRequestHandlerRunnerTest, with two abstract methods, getRequestHandlerClass() and determineWhetherOrNotToSkipRequestHandlerRunnerTests(). The latter will mark tests skipped if they cannot be run for the current laminas-httphandlerrunner version. The former is used to get the class name to use for `new()` operations. - Modifies the SwooleRequestHandlerRunnerTest to extend the new AbstractRequestHandlerRunnerTest and fill out the abstract methods. - Creates a V2RequestHandlerRunnerTest that extends the new AbstractRequestHandlerRunnerTest and fills out the abstract methods. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

…NAME Switches references from SwooleRequestHandlerRunner::DEFAULT_PROCESS_NAME to RequestHandlerConstantsInterface::DEFAULT_PROCESS_NAME everywhere to ensure the references work regardless of the request handler runner version used. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

The class is now referred to as a "service"; this is done to indicate that what is fetched for that service may vary. Additionally, an informational section was added to the "How it Works" chapter to detail the updates for the version 3.6.0 release. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Most were due to the refactor to support both v1 and v2 of laminas-httphandlerrunner. When possible, I've added explicit suppressions in the Psalm configuration, as these can then be found and removed when we update implementations for v4. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

…iant Details planned changes. Updates Psalm configuration to ensure we do not see errors around deprecated classes. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

- Exclude `SwooleRequestHandlerRunnerFactory` file from scanning, as it raises an error under locked deps. - Fix issue flagged by CS Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Ocramius · 2021-12-14T21:09:44Z

composer.json

@@ -33,7 +33,7 @@
        "dflydev/fig-cookies": "^2.0.1 || ^3.0",
        "laminas/laminas-cli": "^0.1.5 || ^1.0",
        "laminas/laminas-diactoros": "^1.8 || ^2.0",
-        "laminas/laminas-httphandlerrunner": "^1.0.1",
+        "laminas/laminas-httphandlerrunner": "^1.0.1 || ^2.1",


Didn't check the rest of the patch yet, but can we just bump to ^2.1 and drop ^1.0.1 instead please?

No, we can't. It would break BC for anybody extending the SwooleRequestHandlerRunner currently. We'd have to bump major to make it happen.

I'm only doing it here because for some reason, we bumped the minimum supported in mezzio/mezzio, so right now you cannot install mezzio-swoole against a new skeleton install. (We shouldn't have bumped in mezzio/mezzio without a major version change; the difference in what Mezzio\Application typehints against in the constructor changed entirely, which is what is causing breakage with alternate laminas-httphandlerrunner implementations.)

I'm happy to do an immediate v4 release after this, but I think we need it in the v3 series first.

Actually, been thinking on this overnight, and I think I'll just target a new major version with this change, and release 3.6 immediately with the other changes I've merged this week. WIll reduce maintenance.

Done now, @Ocramius . 95% of this PR is just copying documentation for v4, so that I can add a migration guide.

Instead of doing a new minor and targeting both the v1 and v2 series of laminas-httphandlerrunner, we'll instead target just v2, and issue a new major of this library. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Since this will now target v4, I'm reverting the changes to the v3 documentation and will create v4 documentation instead. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Updates the documentation to reflect changes in v4, and rewrites the migration chapter to target v3 -> v4 migrations. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Still passes. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

weierophinney · 2021-12-15T21:17:56Z

test/Log/AccessLogFactoryTest.php

@@ -19,7 +19,6 @@
 use Psr\Log\LoggerInterface;
 use ReflectionProperty;
 use Webmozart\Assert\Assert;
-use Zend\Expressive\Swoole\Log\AccessLogFormatterInterface as LegacyAccessLogFormatterInterface;


This is removed, as we conflict with the zend-expressive-swoole package now, so the two cannot be installed in parallel.

weierophinney · 2021-12-15T21:20:05Z

test/SwooleRequestHandlerRunnerTest.php

-     * @var EventDispatcherInterface|MockObject
-     * @psalm-var EventDispatcherInterface&MockObject
-     */
+    /** @var EventDispatcherInterface&MockObject */


phpcs no longer barfs on intersection types, so we can combine the annotations.

weierophinney · 2021-12-15T21:20:30Z

test/SwooleRequestHandlerRunnerTest.php

@@ -63,17 +53,20 @@ public function setUp(): void

    public function testConstructorRaisesExceptionWhenMasterPidIsNotZero(): void
    {
+        /** @var SwooleHttpServer&MockObject $httpServer */


Adding this resolves IDE completion

Phpstorm already has autocomplete for this 🤔

intelephense does not. 😄

Beware:

phpcs rules (especially slevomat/*) tent to replace /** @var T&V $value */ with assert($value instanceof T && $value instanceof V)

assert($value instanceof T && $value instanceof V) is considered redundant by vimeo/psalm

Not a problem for now, but these hints will be removed in some non-far future, where inference can be obtained by psalm ;-)

Ocramius

LGTM, but I'll be honest, I'm not the right person for the docs section :|

weierophinney added this to the 3.6.0 milestone Dec 14, 2021

weierophinney added the Enhancement New feature or request label Dec 14, 2021

weierophinney added 8 commits December 14, 2021 11:43

feat: add laminas-httphandlerrunner v2 to constraints

9debd51

- Adds `^2.1` to the list of laminas-httphandlerrunner constraints, while keeping the v1 constraint. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

qa: resolve CS errors

a290801

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

docs: adds additional information in the docblocks of each runner var…

fa2862b

…iant Details planned changes. Updates Psalm configuration to ensure we do not see errors around deprecated classes. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

weierophinney force-pushed the feature/laminas-httphandlerrunner-v2 branch from ff1e0a7 to fa2862b Compare December 14, 2021 17:44

qa: fix CS issues

9593902

- Exclude `SwooleRequestHandlerRunnerFactory` file from scanning, as it raises an error under locked deps. - Fix issue flagged by CS Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Ocramius requested changes Dec 14, 2021

View reviewed changes

weierophinney modified the milestones: 3.6.0, 4.0.0 Dec 15, 2021

weierophinney added 3 commits December 15, 2021 14:46

docs: revert changes to v3 documentation

63935c2

Since this will now target v4, I'm reverting the changes to the v3 documentation and will create v4 documentation instead. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

docs: add v4 docs

af9ae92

Updates the documentation to reflect changes in v4, and rewrites the migration chapter to target v3 -> v4 migrations. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

weierophinney changed the base branch from 3.6.x to 4.0.x December 15, 2021 21:05

weierophinney added the BC Break label Dec 15, 2021

weierophinney added 4 commits December 15, 2021 15:10

feat: only allow laminas-httphandlerrunner 2.1+

c0ed9f1

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

qa: do not exclude SwooleRequestHandlerRunner file from CS checks

6652ed1

Still passes. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

qa: alphabetize properties

1ace71a

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

qa: remove unneeded import statement

b98acfe

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

weierophinney commented Dec 15, 2021

View reviewed changes

Ocramius approved these changes Dec 16, 2021

View reviewed changes

Ocramius requested a review from froschdesign December 16, 2021 10:11

weierophinney merged commit 458a57c into mezzio:4.0.x Dec 16, 2021

weierophinney deleted the feature/laminas-httphandlerrunner-v2 branch December 16, 2021 15:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for laminas-httphandlerrunner v2 series #82

Add support for laminas-httphandlerrunner v2 series #82

weierophinney commented Dec 14, 2021 •

edited

Loading

Ocramius Dec 14, 2021

weierophinney Dec 14, 2021

weierophinney Dec 15, 2021

weierophinney Dec 15, 2021

weierophinney Dec 15, 2021

weierophinney Dec 15, 2021

weierophinney Dec 15, 2021

Ocramius Dec 15, 2021

weierophinney Dec 15, 2021

Ocramius Dec 16, 2021

Ocramius left a comment

Add support for laminas-httphandlerrunner v2 series #82

Add support for laminas-httphandlerrunner v2 series #82

Conversation

weierophinney commented Dec 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ocramius left a comment

Choose a reason for hiding this comment

weierophinney commented Dec 14, 2021 •

edited

Loading