Router rewrite for PSR-7 #40

Xerkus · 2017-11-04T03:12:17Z

Work in progress

Goal of the rewrite is to switch component to the native support of PSR-7.

Console routing and http request routing are too different. Common interface proved to be limiting and of little practical value: routes were not shared between the two and consumers had to accept one and outright reject another in most cases.
Next implementation of zend-mvc will be PSR-7 based as well and zend-mvc-console will be dropped.

For that reason, support for Zend\Http\RequestInterface will be dropped as too incompatible with PSR-7 and support for the non-http routing with Zend\Stdlib\RequestInterface will not be retained as impractical.
Effectively that will make zend-router a PSR-7 only router.

Also, this PR bumps minimum php to 7.1, introduces strict types, scalar and return typehinting

Note: this PR is a subject for rebases and commit squashes. Ask before using as a basis for your work.

Xerkus · 2017-11-16T07:29:32Z

Small note:
I wanted to keep RouteMatch as a separate thing from RouteResult to keep it more or less backward compatible.
It is used extensively in zend-mvc, but consumers should really be using PSR-7 request attributes for matched parameters. While it will cause additional migration challenges for zend-mvc users, it will be more in line with zend-expressive and the rest of the industry.
Besides, attempts to keep RouteMatch and request attributes in sync are error prone and pretty much guaranteed to produce subtle bugs.

Xerkus · 2017-11-16T09:29:07Z

Routes doing partial match conditionally and depending on the concrete implementation are violating RouteInterface contract.
To make things more straightforward, RouteInterface is now not allowed to return partial match. PartialRouteInterface extending RouteInterface was introduced instead.
Chain, Hostname, Literal, Method, Regex, Scheme and Segment are now partial routes.
Chain was changed to accept partial routes only, Part was changed to accept only partial route for its base route.

michalbundyra · 2017-12-03T01:01:51Z

composer.json

    "require": {
-        "php": "^5.6 || ^7.0",
+        "php": "^7.1",
        "container-interop/container-interop": "^1.2",


Maybe we can use psr/container (PSR-11), or we need first SMv4?

michalbundyra · 2017-12-03T01:07:47Z

src/Route/Regex.php

     * @param  array  $defaults
     */
-    public function __construct($regex, $spec, array $defaults = [])
+    public function __construct(string $regex, $spec, array $defaults = [])


Should we also have typehint on $spec? In PHPDocs I can see "string" only.

michalbundyra · 2017-12-03T01:08:28Z

src/Route/Scheme.php

     * @throws Exception\InvalidArgumentException
     */
-    public static function factory($options = [])
+    public static function factory($options = []) : RouteInterface


Can we have "iterable" type hint here?

michalbundyra · 2017-12-03T01:08:59Z

src/Route/Segment.php

     * @throws Exception\InvalidArgumentException
     */
-    public static function factory($options = [])
+    public static function factory($options = []) : RouteInterface


"iterable" typehint here?

michalbundyra · 2017-12-03T01:09:19Z

src/Route/Segment.php

     * @throws Exception\RuntimeException
     */
-    protected function parseRouteDefinition($def)
+    protected function parseRouteDefinition($def) : array


"string" typehint on param

michalbundyra · 2017-12-03T01:09:41Z

src/Route/Segment.php

     * @return string
     */
-    protected function buildRegex(array $parts, array $constraints, &$groupIndex = 1)
+    protected function buildRegex(array $parts, array $constraints, &$groupIndex = 1) : string


can we add type hint on the last param here, please?

michalbundyra · 2017-12-03T01:13:15Z

src/SimpleRouteStack.php

     * @throws Exception\InvalidArgumentException
     */
-    public static function factory($options = [])
+    public static function factory($options = []) : RouteInterface


Hm... why not "SimpleRouteStack" as return type in signature?

not possible

…e route stack

…behavior

weierophinney · 2019-12-31T22:15:54Z

This repository has been closed and moved to laminas/laminas-router; a new issue has been opened at laminas/laminas-router#6.

weierophinney · 2019-12-31T22:15:59Z

This repository has been moved to laminas/laminas-router. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

Squash all commits in your branch (git rebase -i origin/{branch})
Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
Run the laminas/laminas-migration tool on the code.
Clone laminas/laminas-router to another directory.
Copy the files from the second bullet point to the clone of laminas/laminas-router.
In your clone of laminas/laminas-router, commit the files, push to your fork, and open the new PR.
We will be providing tooling via laminas/laminas-migration soon to help automate the process.

Xerkus added BC Break WIP labels Nov 4, 2017

Xerkus self-assigned this Nov 4, 2017

Xerkus added this to the 4.0.0 milestone Nov 4, 2017

Xerkus changed the base branch from master to develop November 4, 2017 03:14

Xerkus force-pushed the make-mvc-great-again branch 2 times, most recently from b19ed4a to c5cfea4 Compare November 4, 2017 03:32

Xerkus force-pushed the make-mvc-great-again branch 4 times, most recently from f1ed732 to 1eba2d3 Compare November 16, 2017 01:45

zendframework deleted a comment from coveralls Nov 16, 2017

Xerkus mentioned this pull request Nov 17, 2017

Rewrite for PSR-7 zendframework/zend-mvc#259

Closed

11 tasks

Xerkus force-pushed the make-mvc-great-again branch 6 times, most recently from f6e49ca to bf40d4f Compare November 22, 2017 16:21

michalbundyra reviewed Dec 3, 2017

View reviewed changes

Xerkus added 6 commits February 3, 2018 14:03

Bump minimum php to 7.1

cbc7fc8

Add strict types declaration

eecaa4f

Remove unneeded composer conflict

0fd81c3

Fix or suppress type errors

055d799

Bumped to next dev version (4.0.0)

ec527bf

Add CHANGELOG for zendframework#44

61bf30d

Xerkus force-pushed the make-mvc-great-again branch from bf40d4f to 89619be Compare February 3, 2018 04:49

Xerkus added 26 commits March 12, 2018 21:47

Refactor Method route for PSR-7

354e5dd

Refactor Scheme route for PSR-7

c2df89b

Refactor Segment route for PSR-7

0ad59c3

Refactor Regex route for PSR-7

5721438

Update PartialRouteResult and add allowed methods for successful match

1e31a6f

Make Method route provide allowed methods on match

296764a

Refactor Part route for PSR-7

5855c61

Refactor Chain route for PSR-7

a5e4f70

Remove now redundant HttpRouterFactory

e4a8cc7

Add route definitions to RoutePluginManager and update factory

01e3aad

Initial implementation for route config factory

6b16272

Move chain workaround to chain route factory

e0561e7

Add psr container to direct dependencies

94b1a5d

Remove prototypes from factory and accept as extra parameter instead

4ce4729

Add Router class and its factory

1b52b4b

Reintroduce TranslatorAwareRouteStackDecorator inplace of dropped tre…

d2634f9

…e route stack

Drop unused RouterFactory

8196c4e

Improve tests for route plugin manager factory

af430fd

Increment assertion count for test not doing assertions by intention

f2aa83c

Remove service manager v2 support in RouteInvokableFactory

2c2bad5

Remove unused test assets

d11ff0f

Comment out and keep test for ambiguous use case having less desired …

7c95e06

…behavior

Remove obsolete RouterConfigTrait

c342542

Apply cs fixes

14a1b51

Update copyright headers

f889b95

Add RouteConfigFactory factory

08f6307

Xerkus force-pushed the make-mvc-great-again branch from b0dea47 to 08f6307 Compare March 29, 2018 08:37

weierophinney mentioned this pull request Dec 31, 2019

Router rewrite for PSR-7 laminas/laminas-router#6

Closed

Router rewrite for PSR-7 #40

Are you sure you want to change the base?

Router rewrite for PSR-7 #40

Uh oh!

Conversation

Xerkus commented Nov 4, 2017

Uh oh!

Xerkus commented Nov 16, 2017

Uh oh!

Xerkus commented Nov 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weierophinney commented Dec 31, 2019

Uh oh!

weierophinney commented Dec 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants