[Routing] Using Symfony string for routing path instead of Gedmo urlizer by loic425 · Pull Request #1108 · Sylius/SyliusResourceBundle

loic425 · 2025-12-05T11:46:38Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Related tickets
License	MIT

Behat transliteration is deprecated. It's not maintained anymore.
I reused the routing_path_bc_layer that we introduced in this branch that have no release yet.

Why reusing this one? Cause it might have a difference in the plurialization. So it could lead to a path change.

Configure default plural name on resource in a metadata factory without using legacy MetadataInterface.

loic425 · 2025-12-11T08:11:28Z

.github/workflows/build.yml

+                gedmo-doctrine-extensions: [""]
                symfony: ["6.4.*", "7.4.*"]
+                include:
+                    # with Behat transliterator and Gedmo Doctrine extensions optional packages


I started here a new way to handle optional packages.
I plan to remove all of them from dev requirements and add tests with them.
This way, we are sure they are really optional. Moreover we are able to support a new version of Symfony without waiting for their SF8 support.

loic425 · 2025-12-11T08:12:30Z

src/Bundle/DependencyInjection/Configuration.php

-                ->booleanNode('routing_path_bc_layer')
-                    ->defaultTrue()
+                ->booleanNode('routing_path_bc_layer')->end()
+                ->scalarNode('path_segment_name_generator')


this is inspired by API Platform code source.

loic425 · 2025-12-11T08:15:00Z

src/Bundle/DependencyInjection/SyliusResourceExtension.php

+            $routingPathBcLayer = class_exists(Transliterator::class);
+        }
+
+        if ($routingPathBcLayer && !class_exists(Transliterator::class)) {


Cause Behat Transliterator is moved to optional packages (it's deprecated), using another system to urlize might change the routing path. That's why I decided to use the existing routing_path_bc_layer key that we introduced on this branch which has no release yet.

loic425 · 2025-12-11T08:16:36Z

src/Bundle/Resources/config/services/metadata/resource_metadata_collection.php

        ]);

+    $services->set('sylius.resource_metadata_collection.factory.plural_name', PluralNameResourceMetadataCollectionFactory::class)
+        ->decorate('sylius.resource_metadata_collection.factory', null, 300)


we can reuse the same priority as the next one. It doesn't matter.
and of course, if this is configured before, it will be first.

loic425 · 2025-12-11T08:17:58Z

src/Bundle/Resources/config/services/routing.php

        ->private()
-        ->args([service('sylius.routing.factory.operation_route_path_factory')]);
+        ->args([
+            service('sylius.routing.factory.operation_route_path_factory'),


OperationRouteFactory is experimental and have been introduced recently.

loic425 · 2025-12-11T08:25:15Z

src/Component/tests/Symfony/Routing/Factory/OperationRouteFactoryTest.php

        $this->routePathFactory = $this->createMock(OperationRoutePathFactoryInterface::class);
-        $this->operationRouteFactory = new OperationRouteFactory($this->routePathFactory);
+        $this->operationRouteFactory = new OperationRouteFactory($this->routePathFactory, new DashPathSegmentNameGenerator(), false);
+        $this->legacyOperationRouteFactory = new OperationRouteFactory($this->routePathFactory, new DashPathSegmentNameGenerator(), true);


legacy is not well-named.... Maybe withBcLayerEnabledOperationRouteFactory

Think just bcOperationRouteFactory would suffice 🤷

done, thank you for the proposal.

loic425 · 2025-12-11T08:26:03Z

tests/Application/src/Tests/Controller/BlogPostApiTest.php

        $this->markAsSkippedIfNecessary();

-        $this->client->request('POST', '/blog-posts/', [], [], ['CONTENT_TYPE' => 'application/json'], '{}');
+        $this->client->request('POST', $this->isRoutingPathBcLayerEnabled() ? '/blog-posts/' : 'blog-posts', [], [], ['CONTENT_TYPE' => 'application/json'], '{}');


This way, we can run these tests with or without bc layer enabled.

Personally, I'd be for extracting these cases into separate tests (similar to OperationRouteFactoryTest), as it would be easier to track and remove them after the BC layer goes away.

Done, I agree, this is better.

loic425 · 2025-12-11T08:26:43Z

tests/Application/src/Tests/Controller/ComicBookApiTest.php

        $this->assertResponseStatusCodeSame(Response::HTTP_OK);
        $this->assertResponseHeaderSame('content-type', 'application/json');

+        $selfHref = $this->isRoutingPathBcLayerEnabled() ? "\/v1\/comic-books\/?page=1&limit=10" : "\/v1\/comic-books?page=1&limit=10";


we handle the trailing slash with bc layer enabled.

loic425 · 2025-12-11T08:27:44Z

tests/Application/src/Kernel.php

            ]);
        }
+
+        if (class_exists(Urlizer::class)) {


No more Symfony APP_ENV usage which was too much complicated.
We now handle the configuration depending on the optional packages that are installed.

loic425 · 2025-12-11T08:29:55Z

UPGRADE.md

 ### FROM `1.12.x` to `1.13.x`

-The `Sylius\Resource\Exception\VariantWithNoOptionsValuesException` and `Sylius\Component\Resource\Exception\VariantWithNoOptionsValuesException` 
+The `Sylius\Resource\Exception\VariantWithNoOptionsValuesException` and


I may have applied a reformat code...

NoResponseMate · 2025-12-16T08:19:45Z

src/Component/src/Metadata/Resource/Factory/PluralNameResourceMetadataCollectionFactory.php

+        ?InflectorInterface $inflector = null,
+        private readonly bool $routingBcLayerEnabled = true,
+        private readonly ?RegistryInterface $resourceRegistry = null,


The first 2 don't seem to need the default values; the last one is questionable, though it is set within the service definition.
Is it possible to not have the resource registry declared?

I've changed the inflector to be mandatory.

For the last one. I keep it like this, cause we already know that argument will be removed when bc layer will be removed.
Moreover, we don't know if we'll use that registry forever. It uses the MetadataInterface but we have the new ResourceMetadata system.

NoResponseMate · 2025-12-16T08:23:58Z

src/Component/tests/Symfony/Routing/Factory/OperationRouteFactoryTest.php

        $this->routePathFactory = $this->createMock(OperationRoutePathFactoryInterface::class);
-        $this->operationRouteFactory = new OperationRouteFactory($this->routePathFactory);
+        $this->operationRouteFactory = new OperationRouteFactory($this->routePathFactory, new DashPathSegmentNameGenerator(), false);
+        $this->legacyOperationRouteFactory = new OperationRouteFactory($this->routePathFactory, new DashPathSegmentNameGenerator(), true);


Think just bcOperationRouteFactory would suffice 🤷

UPGRADE.md

NoResponseMate · 2025-12-16T09:02:14Z

tests/Application/src/Tests/Controller/BlogPostApiTest.php

        $this->markAsSkippedIfNecessary();

-        $this->client->request('POST', '/blog-posts/', [], [], ['CONTENT_TYPE' => 'application/json'], '{}');
+        $this->client->request('POST', $this->isRoutingPathBcLayerEnabled() ? '/blog-posts/' : 'blog-posts', [], [], ['CONTENT_TYPE' => 'application/json'], '{}');


Personally, I'd be for extracting these cases into separate tests (similar to OperationRouteFactoryTest), as it would be easier to track and remove them after the BC layer goes away.

UPGRADE.md

Co-authored-by: Jan Góralski <jan.wojciech.goralski@gmail.com>

| Q | A | --------------- | ----- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | | License | MIT Based on #1108

#1151) …names | Q | A | --------------- | ----- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | | License | MIT We introduced the PathSegmentNameGeneratorInterface in #1108, but we do not use it when using the operation short name to generate the route path. You can look at the UPGRADE.md for more details.

loic425 marked this pull request as draft December 5, 2025 11:46

loic425 force-pushed the using-symfony-string branch 7 times, most recently from 3210bdb to 3da89d0 Compare December 8, 2025 15:51

loic425 marked this pull request as ready for review December 8, 2025 15:51

loic425 marked this pull request as draft December 8, 2025 15:53

loic425 force-pushed the using-symfony-string branch 11 times, most recently from ba32bea to fdd8689 Compare December 8, 2025 19:59

loic425 marked this pull request as ready for review December 8, 2025 20:13

loic425 marked this pull request as draft December 9, 2025 08:41

loic425 force-pushed the using-symfony-string branch 5 times, most recently from 6510148 to 9d78614 Compare December 10, 2025 19:55

[Routing] Using Symfony string for routing path instead of Gedmo urlizer

8e478b8

loic425 force-pushed the using-symfony-string branch from 9d78614 to 8e478b8 Compare December 10, 2025 19:56

loic425 marked this pull request as ready for review December 10, 2025 20:00

loic425 mentioned this pull request Dec 10, 2025

Remove FOS Rest Bundle from dev-requirements #1112

Merged

loic425 commented Dec 11, 2025

View reviewed changes

NoResponseMate reviewed Dec 16, 2025

View reviewed changes

loic425 and others added 2 commits December 16, 2025 10:43

Apply suggestions from code review

54795f5

Co-authored-by: Jan Góralski <jan.wojciech.goralski@gmail.com>

Create 2 methods for tests with or without bc layer

b7ef483

loic425 force-pushed the using-symfony-string branch from 4bbf54a to b7ef483 Compare December 16, 2025 19:15

Inflector is required in path segment name generators

ecbdd69

loic425 force-pushed the using-symfony-string branch from 866acd7 to ecbdd69 Compare December 18, 2025 08:03

loic425 mentioned this pull request Dec 18, 2025

Symfony 8 #1079

Closed

15 tasks

NoResponseMate approved these changes Dec 18, 2025

View reviewed changes

NoResponseMate merged commit 564f576 into Sylius:1.14 Dec 19, 2025
14 checks passed

loic425 deleted the using-symfony-string branch December 19, 2025 08:37

loic425 mentioned this pull request Dec 29, 2025

Use plural name from new resource metadata #999

Closed

loic425 mentioned this pull request Feb 16, 2026

feat(routing): Use the PathSegmentNameGenerator with operation short … #1151

Merged

Uh oh!

Conversation

loic425 commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

loic425 Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

loic425 Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

loic425 commented Dec 5, 2025 •

edited

Loading

loic425 Dec 11, 2025 •

edited

Loading

loic425 Dec 18, 2025 •

edited

Loading