Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare major release #609

Merged
merged 7 commits into from
Oct 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions UPGRADE-3.0.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
UPGRADE FROM 2.X to 3.0
=======================

## guzzlehttp/guzzle

If you are using `sonata.seo.block.twitter.embed` service with Guzzle, you MUST create a custom service based on the Guzzle client and add it to configuration:

sontata_seo:
http:
client: `your_custom.guzzle_client` # Psr\Http\Client\ClientInterface
message_factory: `your_custom.message_facory` # Psr\Http\Message\RequestFactoryInterface

## SeoPage

If you have implemented a custom seo page, you must adapt the signature of the following new methods to match the one in `SeoPageInterface` again:
Expand All @@ -21,6 +12,11 @@ If you have implemented a custom seo page, you must adapt the signature of the f
* `hasHeadAttribute`
* `removeLangAlternate`
* `hasLangAlternate`
* `addTitlePrefix`
* `addTitleSuffix`
* `getOriginalTitle`
* `setBreadcrumb`
* `getBreadcrumbOptions`

## Closed API

Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"phpstan/extension-installer": "^1.0",
"phpstan/phpstan": "^0.12.84",
"phpstan/phpstan-phpunit": "^0.12.16",
"phpstan/phpstan-strict-rules": "^0.12.10",
"phpstan/phpstan-symfony": "^0.12.44",
"psalm/plugin-phpunit": "^0.15",
"symfony/console": "^4.4 || ^5.3",
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ includes:
- phpstan-baseline.neon

parameters:
level: 6
level: 8
bootstrapFiles:
- vendor/bin/.phpunit/phpunit/vendor/autoload.php
paths:
Expand Down
8 changes: 8 additions & 0 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="4.10.0@916b098b008f6de4543892b1e0651c1c3b92cbfa">
<file src="src/DependencyInjection/Configuration.php">
<PossiblyUndefinedMethod occurrences="1">
<code>children</code>
</PossiblyUndefinedMethod>
</file>
</files>
2 changes: 1 addition & 1 deletion psalm.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<psalm xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" errorLevel="4" resolveFromConfigFile="true" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" autoloader="vendor/bin/.phpunit/phpunit/vendor/autoload.php">
<psalm xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" errorLevel="3" resolveFromConfigFile="true" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" autoloader="vendor/bin/.phpunit/phpunit/vendor/autoload.php" errorBaseline="psalm-baseline.xml">
<projectFiles>
<directory name="src"/>
<directory name="tests"/>
Expand Down
2 changes: 1 addition & 1 deletion src/Block/Breadcrumb/BaseBreadcrumbMenuBlockService.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function configureSettings(OptionsResolver $resolver): void
]);
}

protected function getFactory(): FactoryInterface
final protected function getFactory(): FactoryInterface
{
return $this->factory;
}
Expand Down
5 changes: 3 additions & 2 deletions src/Block/Breadcrumb/BreadcrumbBlockService.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@

namespace Sonata\SeoBundle\Block\Breadcrumb;

use Sonata\SeoBundle\BreadcrumbInterface;
use Sonata\BlockBundle\Block\Service\BlockServiceInterface;

interface BreadcrumbBlockService extends BreadcrumbInterface
interface BreadcrumbBlockService extends BlockServiceInterface
{
public function handleContext(string $context): bool;
}
24 changes: 0 additions & 24 deletions src/BreadcrumbInterface.php

This file was deleted.

16 changes: 14 additions & 2 deletions src/Command/SitemapGeneratorCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,24 @@ private function moveTemporaryFile(string $tempDir, string $permanentDir): void
{
$oldFiles = Finder::create()->files()->name('sitemap*.xml')->in($permanentDir);
foreach ($oldFiles as $file) {
$this->filesystem->remove($file->getRealPath());
$pathname = $file->getRealPath();

if (false === $pathname) {
throw new \LogicException(sprintf('File %s does not exist', $file));
}

$this->filesystem->remove($pathname);
}

$newFiles = Finder::create()->files()->name('sitemap*.xml')->in($tempDir);
foreach ($newFiles as $file) {
$this->filesystem->rename($file->getRealPath(), sprintf('%s/%s', $permanentDir, $file->getFilename()));
$pathname = $file->getRealPath();

if (false === $pathname) {
throw new \LogicException(sprintf('File %s does not exist', $file));
}

$this->filesystem->rename($pathname, sprintf('%s/%s', $permanentDir, $file->getFilename()));
}
}
}
1 change: 1 addition & 0 deletions src/DependencyInjection/Compiler/ServiceCompilerPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ final class ServiceCompilerPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container): void
{
/** @var array<string, mixed> $config */
$config = $container->getParameter('sonata.seo.config');

$definition = $container->findDefinition($config['default']);
Expand Down
7 changes: 5 additions & 2 deletions src/DependencyInjection/SonataSeoExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

namespace Sonata\SeoBundle\DependencyInjection;

use Sonata\Exporter\Source\DoctrineDBALConnectionSourceIterator;
use Sonata\Exporter\Source\SymfonySitemapSourceIterator;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
Expand All @@ -31,6 +33,7 @@ public function load(array $configs, ContainerBuilder $container): void
$config = $this->processConfiguration($configuration, $configs);
$config = $this->fixConfiguration($config);

/** @var array<string, mixed> $bundles */
$bundles = $container->getParameter('kernel.bundles');

$loader = new Loader\PhpFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
Expand Down Expand Up @@ -75,7 +78,7 @@ private function configureSitemap(array $config, ContainerBuilder $container): v
// define the connectionIterator
$connectionIteratorId = 'sonata.seo.source.doctrine_connection_iterator_'.$pos;

$connectionIterator = new Definition('%sonata.seo.exporter.database_source_iterator.class%', [
$connectionIterator = new Definition(DoctrineDBALConnectionSourceIterator::class, [
Comment on lines -78 to +81
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this, but if we have paramteter here should not be better to keep it and set it in config?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could improve this by allowing a custom service via the config, but for now we should get rid of all class parameters.

new Reference($sitemap['connection']),
$sitemap['query'],
]);
Expand All @@ -86,7 +89,7 @@ private function configureSitemap(array $config, ContainerBuilder $container): v
// define the sitemap proxy iterator
$sitemapIteratorId = 'sonata.seo.source.doctrine_sitemap_iterator_'.$pos;

$sitemapIterator = new Definition('%sonata.seo.exporter.sitemap_source_iterator.class%', [
$sitemapIterator = new Definition(SymfonySitemapSourceIterator::class, [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this, but if we have paramteter here should not be better to keep it and set it in config?

new Reference($connectionIteratorId),
new Reference('router'),
$sitemap['route'],
Expand Down
6 changes: 3 additions & 3 deletions src/Event/BreadcrumbListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

use Sonata\BlockBundle\Event\BlockEvent;
use Sonata\BlockBundle\Model\Block;
use Sonata\SeoBundle\BreadcrumbInterface;
use Sonata\SeoBundle\Block\Breadcrumb\BreadcrumbBlockService;

/**
* BreadcrumbListener for Block Event.
Expand All @@ -25,14 +25,14 @@
final class BreadcrumbListener
{
/**
* @var array<string, BreadcrumbInterface>
* @var array<string, BreadcrumbBlockService>
*/
private array $blockServices = [];

/**
* Add a renderer to the status services list.
*/
public function addBlockService(string $type, BreadcrumbInterface $breadcrumb): void
public function addBlockService(string $type, BreadcrumbBlockService $breadcrumb): void
{
$this->blockServices[$type] = $breadcrumb;
}
Expand Down
5 changes: 1 addition & 4 deletions src/Resources/config/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,10 @@
return static function (ContainerConfigurator $containerConfigurator): void {
// Use "service" function for creating references to services when dropping support for Symfony 4.4
// Use "param" function for creating references to parameters when dropping support for Symfony 5.1
$containerConfigurator->parameters()
->set('sonata.seo.block.breadcrumb.homepage.class', HomepageBreadcrumbBlockService::class);

$containerConfigurator->services()

// Breadcrumb
->set('sonata.seo.block.breadcrumb.homepage', '%sonata.seo.block.breadcrumb.homepage.class%')
->set('sonata.seo.block.breadcrumb.homepage', HomepageBreadcrumbBlockService::class)
->public()
->tag('sonata.block')
->tag('sonata.breadcrumb')
Expand Down
21 changes: 3 additions & 18 deletions src/Resources/config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
* file that was distributed with this source code.
*/

use Sonata\Exporter\Source\DoctrineDBALConnectionSourceIterator;
use Sonata\Exporter\Source\SymfonySitemapSourceIterator;
use Sonata\SeoBundle\Seo\SeoPage;
use Sonata\SeoBundle\Sitemap\SourceManager;
use Sonata\SeoBundle\Twig\Extension\SeoExtension;
Expand All @@ -22,30 +20,17 @@
return static function (ContainerConfigurator $containerConfigurator): void {
// Use "service" function for creating references to services when dropping support for Symfony 4.4
// Use "param" function for creating references to parameters when dropping support for Symfony 5.1
$containerConfigurator->parameters()

->set('sonata.seo.exporter.database_source_iterator.class', DoctrineDBALConnectionSourceIterator::class)

->set('sonata.seo.exporter.sitemap_source_iterator.class', SymfonySitemapSourceIterator::class)

->set('sonata.seo.page.default.class', SeoPage::class)

->set('sonata.seo.twig.extension.class', SeoExtension::class)

->set('sonata.seo.sitemap.manager.class', SourceManager::class);

$containerConfigurator->services()

->set('sonata.seo.page.default', '%sonata.seo.page.default.class%')
->set('sonata.seo.page.default', SeoPage::class)
->public()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't checked the usages, but do we need services to be public?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if symfony still recommends using the service container. If it's deprecated we could make this private.

Copy link
Member

@franmomu franmomu Oct 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it depends on the usage:

In addition, services not meant to be used by the application directly, should be defined as private. For public services, aliases should be created from the interface/class to the service id.

(I haven checked, but I guess they don't recommend using the service container directly.)


->set('sonata.seo.twig.extension', '%sonata.seo.twig.extension.class%')
->set('sonata.seo.twig.extension', SeoExtension::class)
->tag('twig.extension')
->args([
new ReferenceConfigurator('sonata.seo.page'),
'',
])

->set('sonata.seo.sitemap.manager', '%sonata.seo.sitemap.manager.class%')
->public();
->set('sonata.seo.sitemap.manager', SourceManager::class);
};
2 changes: 2 additions & 0 deletions src/Sitemap/SourceManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public function addSource(string $group, SourceIteratorInterface $source, array
$this->sources[$group] = new Source();
}

\assert(null !== $this->sources[$group]);

$this->sources[$group]->addSource($source);

if ([] !== $types) {
Expand Down
2 changes: 0 additions & 2 deletions src/SonataSeoBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ final class SonataSeoBundle extends Bundle
{
public function build(ContainerBuilder $container): void
{
parent::build($container);

$container->addCompilerPass(new BreadcrumbBlockServicesCompilerPass());
$container->addCompilerPass(new ServiceCompilerPass());
}
Expand Down
6 changes: 3 additions & 3 deletions src/Twig/Extension/SeoExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ public function getMetadatas(): string
{
$html = '';
foreach ($this->page->getMetas() as $type => $metas) {
foreach ((array) $metas as $name => $meta) {
foreach ($metas as $name => $meta) {
[$content, $extras] = $meta;

if (!empty($content)) {
if ('' !== $content) {
$html .= sprintf(
"<meta %s=\"%s\" content=\"%s\" />\n",
$type,
Expand Down Expand Up @@ -112,7 +112,7 @@ public function getHeadAttributes(): string

public function getLinkCanonical(): string
{
if ($this->page->getLinkCanonical()) {
if ('' !== $this->page->getLinkCanonical()) {
return sprintf("<link rel=\"canonical\" href=\"%s\"/>\n", $this->page->getLinkCanonical());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ public function testGlobalTitle(): void

$page = $container->get('sonata.seo.custom.page');

\assert($page instanceof SeoPageInterface);

static::assertSame('Project name', $page->getOriginalTitle());
static::assertSame('Prefix Project name Suffix', $page->getTitle());
}
Expand Down
1 change: 1 addition & 0 deletions tests/DependencyInjection/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public function testKeysAreNotNormalized(): void
public function testWithYamlConfig(): void
{
$values = Yaml::parse(
/* @phpstan-ignore-next-line */
file_get_contents(__DIR__.'/data/config.yml'),
Yaml::PARSE_EXCEPTION_ON_INVALID_TYPE
);
Expand Down
7 changes: 0 additions & 7 deletions tests/Seo/SeoPageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,4 @@ public function testHasMeta(): void
static::assertTrue($page->hasMeta('property', 'test'));
static::assertFalse($page->hasMeta('property', 'fake'));
}

public function testSetSeparator(): void
{
$page = new SeoPage();

static::assertInstanceOf(SeoPage::class, $page->setSeparator('-'));
}
}