Add attributes support & improve `MatchingResult` #196

rustamwin · 2023-03-28T10:21:17Z

Q	A
Is bugfix?	✔️/❌
New feature?	✔️
Breaks BC?	✔️
Fixed issues	comma-separated list of tickets # fixed by the PR, if any

ℹ️ yiisoft/demo#589

what-the-diff · 2023-03-28T10:21:46Z

PR Summary

🚀 Optimized Middleware Handling
Improved the way middlewares are managed in the Group class to avoid building them multiple times when they don't change, boosting efficiency.
🧹 Code Clean-up
Removed unused classes and fixed some typos for better readability and maintenance.
🛠️ Refactoring
Refactored various methods, attributes, and typehints for improved consistency, readability, and functionality.
✅ Improved Testing
Enhanced the tests for the Group and RouteCollection classes, and fixed the bug in the RouteCollectionTest.
💡 Convenience Enhancements
Added the withMiddlewares method to the DispatcherInterface for easier usage in certain cases.

codecov · 2023-03-28T10:25:01Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (04be2f6) 78.66% compared to head (5be62b4) 83.72%.

❗ Current head 5be62b4 differs from pull request most recent head 63467dc. Consider uploading reports for the commit 63467dc to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #196      +/-   ##
============================================
+ Coverage     78.66%   83.72%   +5.05%     
- Complexity      188      228      +40     
============================================
  Files            13       23      +10     
  Lines           586      762     +176     
============================================
+ Hits            461      638     +177     
+ Misses          125      124       -1

Files	Coverage Δ
src/Attribute/Delete.php	`100.00% <100.00%> (ø)`
src/Attribute/Get.php	`100.00% <100.00%> (ø)`
src/Attribute/Head.php	`100.00% <100.00%> (ø)`
src/Attribute/Options.php	`100.00% <100.00%> (ø)`
src/Attribute/Patch.php	`100.00% <100.00%> (ø)`
src/Attribute/Post.php	`100.00% <100.00%> (ø)`
src/Attribute/Put.php	`100.00% <100.00%> (ø)`
src/Attribute/Route.php	`100.00% <100.00%> (ø)`
src/Group.php	`100.00% <100.00%> (ø)`
src/MatchingResult.php	`100.00% <ø> (ø)`
... and 6 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/MatchingResult.php

src/Route.php

src/Attribute/Delete.php

src/Group.php

src/Route.php

src/RouteAttributesRegistrarInterface.php

src/RouteAttributesRegistrar.php

* Add routes via resource + attributes * Apply fixes from StyleCI * Apply Rector changes (CI) * Fix psalm * Add missing file * Apply fixes from StyleCI * Rename Resource to Provider, add more tests * Apply fixes from StyleCI * Increase coverage * Apply fixes from StyleCI * Use variadic --------- Co-authored-by: StyleCI Bot <[email protected]> Co-authored-by: rustamwin <[email protected]>

xepozz · 2023-10-15T04:26:28Z

tests/RouteTest.php

+                      ->middleware(TestMiddleware1::class)
+                      ->action(static fn () => new Response(200));
+
+        $builtMiddlewares = $route->getData('builtMiddlewares');


Due to the PR is going to raise the major version of package let's remove getData to regular getter?

Due to the PR is going to raise the major version of package let's remove getData to regular getter?

Rename getData() to get() or create separate getters?

The second option

This is very non-comfortable when configre routes. IDE suggestions will get in the way in application, but getters in most cases need into packages only.

xepozz · 2023-10-15T04:28:33Z

src/Group.php

-        MiddlewareDispatcher $dispatcher = null
-    ): self {
-        return new self($prefix, $dispatcher);
+    public static function create(?string $prefix = null): self


How about to eliminate this?

Create group via new Group is non-comfortable in application configuration.

Most case of usage groups required chain calls.

Now: Group::create('/xxx')->...

Your suggestion: (new Group('/xxx'))->...

Brackets in second way is non-comfortable =)

xepozz · 2023-10-15T04:29:54Z

src/MatchingResult.php

@@ -86,6 +72,9 @@ public function methods(): array
        return $this->methods;
    }

+    /**
+     * @psalm-assert-if-true !null $this->route


Why is it needed?

Without this assertion psalm occurs PossiblyNullReference error.

It shouldn't because there is check for null in the function body 😕

src/Route.php

samdark · 2023-10-19T14:38:59Z

TODO: move collector to https://github.com/yiisoft/router-composer-attribute-collector

rustamwin and others added 2 commits March 28, 2023 15:20

Add attributes support

a60d03b

Apply fixes from StyleCI

b328c82

rustamwin changed the title ~~Add attributes support~~ Add attributes support & improve MatchingResult Mar 28, 2023

vjik reviewed Mar 28, 2023

View reviewed changes

src/MatchingResult.php Outdated Show resolved Hide resolved

rustamwin added 2 commits March 28, 2023 16:54

Fix psalm annotation

3496866

Add tests

69d9832

samdark mentioned this pull request Apr 2, 2023

Support Attributes #194

Closed

Merge branch 'master' into add-attributes

bc421e9

vjik added the status:under development Someone is working on a pull request. label Jun 2, 2023

xepozz mentioned this pull request Jun 8, 2023

Remove usage middleware dispatcher in Route, Group and MatchingResult #210

Closed

xepozz and others added 6 commits June 8, 2023 21:50

Merge branch 'master' into add-attributes

875d7fd

Add attributes registrar

777ff54

Apply fixes from StyleCI

3959325

Add tests

1651071

Fix tests & psalm issues

838bbc4

Apply fixes from StyleCI

4e63476

rustamwin added status:code review The pull request needs review. and removed status:under development Someone is working on a pull request. labels Jun 15, 2023

rustamwin requested a review from a team June 15, 2023 14:40

rustamwin and others added 2 commits June 15, 2023 22:55

Add test cases

a0fb7a2

Apply fixes from StyleCI

ed2f60f

vjik reviewed Jun 15, 2023

View reviewed changes

src/Route.php Outdated Show resolved Hide resolved

src/Attribute/Delete.php Outdated Show resolved Hide resolved

src/Group.php Show resolved Hide resolved

src/Route.php Outdated Show resolved Hide resolved

src/RouteAttributesRegistrarInterface.php Outdated Show resolved Hide resolved

vjik added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Jun 16, 2023

samdark reviewed Jun 20, 2023

View reviewed changes

src/RouteAttributesRegistrar.php Outdated Show resolved Hide resolved

rustamwin and others added 3 commits August 19, 2023 10:28

Improve Route

c239d53

Add test

e8367aa

Merge branch 'master' into add-attributes

2ab8dc2

rustamwin added 2 commits August 19, 2023 16:22

Increase coverage

2a60945

Merge remote-tracking branch 'origin/add-attributes' into add-attributes

61eafb0

This was linked to issues Sep 16, 2023

Named arguments vs immutable methods #193

Open

Add attributes support #179

Open

xepozz and others added 4 commits October 9, 2023 21:08

Merge branch 'master' into add-attributes

01665f4

Fix AttributeRoutesProvider

ab43b76

Allow attributes to use in classes

7a0e222

rustamwin added status:code review The pull request needs review. and removed status:under development Someone is working on a pull request. labels Oct 15, 2023

Fix psalm annotation

2c38b06

xepozz reviewed Oct 15, 2023

View reviewed changes

vjik reviewed Oct 15, 2023

View reviewed changes

src/Route.php Outdated Show resolved Hide resolved

src/Route.php Outdated Show resolved Hide resolved

vjik and others added 4 commits October 15, 2023 17:04

Add RouteAttributeInterface (#221)

6a2bd4c

Adjust naming

ab467d8

Minor improvements

61cbc83

Minor

d85e667

vjik added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Oct 22, 2023

rustamwin and others added 5 commits October 23, 2023 11:53

Move attribute collector it's own package

0ebcb5a

Cleanup

3ff6921

Minor

a81c91a

Add changelog

5be62b4

Apply Rector changes (CI)

63467dc

vjik mentioned this pull request Nov 2, 2023

Make Route, Group and MatchingResult dispatcher independent #222

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add attributes support & improve `MatchingResult` #196

Add attributes support & improve `MatchingResult` #196

rustamwin commented Mar 28, 2023 •

edited

Loading

what-the-diff bot commented Mar 28, 2023

codecov bot commented Mar 28, 2023 •

edited

Loading

xepozz Oct 15, 2023

vjik Oct 15, 2023

xepozz Oct 15, 2023

vjik Oct 22, 2023

xepozz Oct 15, 2023

vjik Oct 15, 2023

vjik Oct 22, 2023

xepozz Oct 15, 2023

rustamwin Oct 15, 2023 •

edited

Loading

xepozz Oct 15, 2023

samdark commented Oct 19, 2023

Add attributes support & improve MatchingResult #196

Are you sure you want to change the base?

Add attributes support & improve MatchingResult #196

Conversation

rustamwin commented Mar 28, 2023 • edited Loading

what-the-diff bot commented Mar 28, 2023

PR Summary

codecov bot commented Mar 28, 2023 • edited Loading

Codecov Report

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

rustamwin Oct 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samdark commented Oct 19, 2023

Add attributes support & improve `MatchingResult` #196

Add attributes support & improve `MatchingResult` #196

rustamwin commented Mar 28, 2023 •

edited

Loading

codecov bot commented Mar 28, 2023 •

edited

Loading

rustamwin Oct 15, 2023 •

edited

Loading