Skip to content

Commit

Permalink
refactor #872 Move fos rest, jms serializer and hateoas on optional r…
Browse files Browse the repository at this point in the history
…equirements (loic425)

This PR was merged into the 1.12 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | mentioned in upgrade
| Deprecations?   | no
| Related tickets | 
| License         | MIT

It's time to get rid of these packages. Let's start by moving them on optional requirements.

- [x] move on dev dependencies
- [x] fix DI
- [x] FosRestPassTest
- [x] HateoasPassTest

Commits
-------

7f1762f Move fos rest, jms serializer and hateoas on dev requirements
496c369 Add tests for compiler passes
f5d84ad Fix step name
472bfea Apply suggestions from code review
10e80cf Rename compiler passes
  • Loading branch information
GSadee authored May 24, 2024
2 parents 46a2b13 + 10e80cf commit 1c7cc9c
Show file tree
Hide file tree
Showing 11 changed files with 207 additions and 21 deletions.
5 changes: 3 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,11 @@ jobs:
sed -i -e 's/state_machine_component: symfony/state_machine_component: winzou/g' tests/Application/config/packages/test/sylius_resource.yaml
-
name: Run smoke tests without friendsofsymfony/rest-bundle willdurand/hateoas-bundle jms/serializer-bundle packages
name: Run lint container without friendsofsymfony/rest-bundle willdurand/hateoas-bundle jms/serializer-bundle packages
run: |
composer remove friendsofsymfony/rest-bundle willdurand/hateoas-bundle jms/serializer-bundle --no-scripts
composer remove --dev friendsofsymfony/rest-bundle willdurand/hateoas-bundle jms/serializer-bundle --no-scripts
(cd tests/Application && bin/console cache:clear --env=test_without_fosrest)
(cd tests/Application && bin/console lint:container --env=test_without_fosrest)
composer require friendsofsymfony/rest-bundle willdurand/hateoas-bundle jms/serializer-bundle --no-scripts
-
Expand Down
10 changes: 10 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## UPGRADE FOR `1.12.x`

### FROM `1.11.x` to `1.12.x`

In preparation of removal, following dependencies were moved to optional requirements. If still in use by your app, require them explicitly in your `composer.json`.

* friendsofsymfony/rest-bundle
* jms/serializer-bundle
* willdurand/hateoas-bundle

## UPGRADE FOR `1.11.x`

### FROM `1.10.x` to `1.11.x`
Expand Down
13 changes: 9 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@
"doctrine/event-manager": "^1.1",
"doctrine/inflector": "^1.4 || ^2.0",
"doctrine/persistence": "^2.0 || ^3.0",
"friendsofsymfony/rest-bundle": "^3.0",
"gedmo/doctrine-extensions": "^2.4.12 || ^3.0",
"jms/serializer-bundle": "^3.5 || ^4.0 || ^5.0",
"sylius/registry": "^1.2",
"symfony/config": "^5.4 || ^6.4",
"symfony/deprecation-contracts": "^2.1 || ^3.0",
Expand All @@ -52,7 +50,6 @@
"symfony/validator": "^5.4 || ^6.4",
"symfony/yaml": "^5.4 || ^6.4",
"webmozart/assert": "^1.8",
"willdurand/hateoas-bundle": "^2.0",
"winzou/state-machine-bundle": "^0.6",
"willdurand/negotiation": "^3.1"
},
Expand All @@ -61,6 +58,8 @@
},
"require-dev": {
"doctrine/orm": "^2.5",
"friendsofsymfony/rest-bundle": "^3.0",
"jms/serializer-bundle": "^3.5 || ^4.0 || ^5.0",
"lchrusciel/api-test-case": "^5.0",
"matthiasnoback/symfony-dependency-injection-test": "^4.2.1",
"pagerfanta/pagerfanta": "^3.7 || ^4.0",
Expand All @@ -84,7 +83,13 @@
"rector/rector": "^0.18.2",
"symfony/messenger": "^5.4 || ^6.4",
"symfony/serializer": "^5.4 || ^6.4",
"symfony/security-bundle": "^5.4 || ^6.4"
"symfony/security-bundle": "^5.4 || ^6.4",
"willdurand/hateoas-bundle": "^2.0"
},
"conflict": {
"friendsofsymfony/rest-bundle": "<3.0",
"jms/serializer-bundle": "<3.5",
"willdurand/hateoas-bundle": "<2.0"
},
"suggest": {
"doctrine/orm": "^2.5",
Expand Down
16 changes: 8 additions & 8 deletions src/Bundle/Controller/ResourcesCollectionProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,10 @@

final class ResourcesCollectionProvider implements ResourcesCollectionProviderInterface
{
private ResourcesResolverInterface $resourcesResolver;

private PagerfantaFactory $pagerfantaRepresentationFactory;

public function __construct(ResourcesResolverInterface $resourcesResolver, PagerfantaFactory $pagerfantaRepresentationFactory)
{
$this->resourcesResolver = $resourcesResolver;
$this->pagerfantaRepresentationFactory = $pagerfantaRepresentationFactory;
public function __construct(
private ResourcesResolverInterface $resourcesResolver,
private ?PagerfantaFactory $pagerfantaRepresentationFactory = null,
) {
}

/**
Expand Down Expand Up @@ -61,6 +57,10 @@ public function get(RequestConfiguration $requestConfiguration, RepositoryInterf
$paginator->getCurrentPageResults();

if (!$requestConfiguration->isHtmlRequest()) {
if (null === $this->pagerfantaRepresentationFactory) {
throw new \LogicException('The "willdurand/hateoas-bundle" must be installed and configured to render a resource collection on non-HTML request. Try running "composer require willdurand/hateoas-bundle"');
}

$route = new Route($request->attributes->get('_route'), array_merge($request->attributes->get('_route_params'), $request->query->all()));

return $this->pagerfantaRepresentationFactory->createRepresentation($paginator, $route);
Expand Down
8 changes: 3 additions & 5 deletions src/Bundle/Controller/ViewHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,17 @@

final class ViewHandler implements ViewHandlerInterface
{
private ConfigurableViewHandlerInterface $restViewHandler;

public function __construct(ConfigurableViewHandlerInterface $restViewHandler)
public function __construct(private ConfigurableViewHandlerInterface $restViewHandler)
{
$this->restViewHandler = $restViewHandler;
}

public function handle(RequestConfiguration $requestConfiguration, View $view): Response
{
if (!$requestConfiguration->isHtmlRequest()) {
$this->restViewHandler->setExclusionStrategyGroups($requestConfiguration->getSerializationGroups() ?? []);

if ($version = $requestConfiguration->getSerializationVersion()) {
$version = $requestConfiguration->getSerializationVersion();
if (null !== $version) {
$this->restViewHandler->setExclusionStrategyVersion($version);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Sylius Sp. z o.o.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler;

use FOS\RestBundle\FOSRestBundle;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

final class UnregisterFosRestDefinitionsPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container): void
{
/** @var array $bundles */
$bundles = $container->getParameter('kernel.bundles');

if (in_array(FOSRestBundle::class, $bundles, true)) {
return;
}

$container->removeDefinition('sylius.resource_controller.view_handler');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Sylius Sp. z o.o.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler;

use Bazinga\Bundle\HateoasBundle\BazingaHateoasBundle;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

final class UnregisterHateoasDefinitionsPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container): void
{
/** @var array $bundles */
$bundles = $container->getParameter('kernel.bundles');

if (in_array(BazingaHateoasBundle::class, $bundles, true)) {
return;
}

$container->removeDefinition('sylius.resource_controller.pagerfanta_representation_factory');
}
}
4 changes: 2 additions & 2 deletions src/Bundle/Resources/config/services/controller.xml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@

<service id="sylius.resource_controller.resources_collection_provider" class="Sylius\Bundle\ResourceBundle\Controller\ResourcesCollectionProvider" public="false">
<argument type="service" id="sylius.resource_controller.resources_resolver" />
<argument type="service" id="sylius.resource_controller.pagerfanta_representation_factory" />
<argument type="service" id="sylius.resource_controller.pagerfanta_representation_factory" on-invalid="null" />
</service>
<service id="Sylius\Bundle\ResourceBundle\Controller\ResourcesCollectionProviderInterface" alias="sylius.resource_controller.resources_collection_provider" public="false" />

Expand Down Expand Up @@ -80,7 +80,7 @@
<service id="Sylius\Bundle\ResourceBundle\Controller\EventDispatcherInterface" alias="sylius.resource_controller.event_dispatcher" public="false" />

<service id="sylius.resource_controller.view_handler" class="Sylius\Bundle\ResourceBundle\Controller\ViewHandler" public="false">
<argument type="service" id="fos_rest.view_handler" on-invalid="null" />
<argument type="service" id="fos_rest.view_handler" />
</service>
<service id="Sylius\Bundle\ResourceBundle\Controller\ViewHandlerInterface" alias="sylius.resource_controller.view_handler" public="false" />

Expand Down
4 changes: 4 additions & 0 deletions src/Bundle/SyliusResourceBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\RegisterResourceStateMachinePass;
use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\RegisterStateMachinePass;
use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\TwigPass;
use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\UnregisterFosRestDefinitionsPass;
use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\UnregisterHateoasDefinitionsPass;
use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\WinzouStateMachinePass;
use Sylius\Bundle\ResourceBundle\DependencyInjection\PagerfantaExtension;
use Sylius\Resource\Symfony\DependencyIjection\Compiler\DisableMetadataCachePass;
Expand Down Expand Up @@ -56,6 +58,8 @@ public function build(ContainerBuilder $container): void
$container->addCompilerPass(new RegisterResourcesPass());
$container->addCompilerPass(new RegisterStateMachinePass());
$container->addCompilerPass(new RegisterResourceStateMachinePass());
$container->addCompilerPass(new UnregisterFosRestDefinitionsPass());
$container->addCompilerPass(new UnregisterHateoasDefinitionsPass());
$container->addCompilerPass(new TwigPass());
$container->addCompilerPass(new WinzouStateMachinePass());

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Sylius Sp. z o.o.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace Bundle\DependencyInjection\Compiler;

use FOS\RestBundle\FOSRestBundle;
use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase;
use Sylius\Bundle\ResourceBundle\Controller\ViewHandler;
use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\UnregisterFosRestDefinitionsPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;

final class UnregisterFosRestDefinitionsPassTest extends AbstractCompilerPassTestCase
{
/** @test */
public function it_remove_view_handler_if_fos_rest_is_not_available(): void
{
$this->setParameter('kernel.bundles', []);

$this->compile();

$this->assertContainerBuilderNotHasService('sylius.resource_controller.view_handler');
}

/** @test */
public function it_keeps_the_view_handler_if_fos_rest_is_available(): void
{
$this->setParameter('kernel.bundles', [FOSRestBundle::class]);

$this->compile();

$this->assertContainerBuilderHasService('sylius.resource_controller.view_handler');
}

protected function registerCompilerPass(ContainerBuilder $container): void
{
$this->registerService('sylius.resource_controller.view_handler', ViewHandler::class);
$this->setParameter('kernel.bundles', []);

$container->addCompilerPass(new UnregisterFosRestDefinitionsPass());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Sylius Sp. z o.o.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace Bundle\DependencyInjection\Compiler;

use Bazinga\Bundle\HateoasBundle\BazingaHateoasBundle;
use Hateoas\Representation\Factory\PagerfantaFactory;
use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase;
use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\UnregisterHateoasDefinitionsPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;

final class UnregisterHateoasDefinitionsPassTest extends AbstractCompilerPassTestCase
{
/** @test */
public function it_remove_pagerfanta_representation_factory_if_hateoas_is_not_available(): void
{
$this->setParameter('kernel.bundles', []);

$this->compile();

$this->assertContainerBuilderNotHasService('sylius.resource_controller.pagerfanta_representation_factory');
}

/** @test */
public function it_keeps_the_view_handler_if_fos_rest_is_available(): void
{
$this->setParameter('kernel.bundles', [BazingaHateoasBundle::class]);

$this->compile();

$this->assertContainerBuilderHasService('sylius.resource_controller.pagerfanta_representation_factory');
}

protected function registerCompilerPass(ContainerBuilder $container): void
{
$this->registerService('sylius.resource_controller.pagerfanta_representation_factory', PagerfantaFactory::class);
$this->setParameter('kernel.bundles', []);

$container->addCompilerPass(new UnregisterHateoasDefinitionsPass());
}
}

0 comments on commit 1c7cc9c

Please sign in to comment.