Skip to content
Snippets Groups Projects
Commit fe8b4ac6 authored by Kunal Sachdev's avatar Kunal Sachdev Committed by Adam G-H
Browse files

Issue #3281379 by kunal.sachdev, tedbow, phenaproxima, omkar.podey, Wim Leers:...

Issue #3281379 by kunal.sachdev, tedbow, phenaproxima, omkar.podey, Wim Leers: VersionPolicyValidator should return all error messages in its validation result
parent 25b69e4a
No related branches found
No related tags found
No related merge requests found
<?xml version="1.0" encoding="utf-8"?>
<!--
This fixture is used by \Drupal\Tests\automatic_updates\Kernel\StatusCheck\VersionPolicyValidatorTest.
Contains metadata about the following (fake) releases of Drupal core, all of which are secure, in order:
- 9.8.1-rc3
- 9.8.1-beta2
- 9.8.1-alpha1
- 9.7.1
- 9.7.0
-->
<project xmlns:dc="http://purl.org/dc/elements/1.1/">
<title>Drupal</title>
<short_name>drupal</short_name>
<dc:creator>Drupal</dc:creator>
<supported_branches>9.7.,9.8.</supported_branches>
<project_status>published</project_status>
<link>http://example.com/project/drupal</link>
<terms>
<term>
<name>Projects</name>
<value>Drupal project</value>
</term>
</terms>
<releases>
<release>
<name>Drupal 9.8.1-rc3</name>
<version>9.8.1-rc3</version>
<status>published</status>
<release_link>http://example.com/drupal-9-8-1-rc3-release</release_link>
<download_link>http://example.com/drupal-9-8-1-rc3.tar.gz</download_link>
<date>1250424521</date>
<terms>
<term>
<name>Release type</name>
<value>New features</value>
</term>
<term>
<name>Release type</name>
<value>Bug fixes</value>
</term>
</terms>
</release>
<release>
<name>Drupal 9.8.1-beta2</name>
<version>9.8.1-beta2</version>
<status>published</status>
<release_link>http://example.com/drupal-9-8-1-beta2-release</release_link>
<download_link>http://example.com/drupal-9-8-1-beta2.tar.gz</download_link>
<date>1250424521</date>
<terms>
<term>
<name>Release type</name>
<value>New features</value>
</term>
<term>
<name>Release type</name>
<value>Bug fixes</value>
</term>
</terms>
</release>
<release>
<name>Drupal 9.8.1-alpha1</name>
<version>9.8.1-alpha1</version>
<status>published</status>
<release_link>http://example.com/drupal-9-8-1-alpha1-release</release_link>
<download_link>http://example.com/drupal-9-8-1-alpha1.tar.gz</download_link>
<date>1250424521</date>
<terms>
<term>
<name>Release type</name>
<value>New features</value>
</term>
<term>
<name>Release type</name>
<value>Bug fixes</value>
</term>
</terms>
</release>
<release>
<name>Drupal 9.8.1-beta2</name>
<version>9.8.1-beta2</version>
<status>published</status>
<release_link>http://example.com/drupal-9-8-1-beta2-release</release_link>
<download_link>http://example.com/drupal-9-8-1-beta2.tar.gz</download_link>
<date>1250424521</date>
<terms>
<term>
<name>Release type</name>
<value>New features</value>
</term>
<term>
<name>Release type</name>
<value>Bug fixes</value>
</term>
</terms>
</release>
<release>
<name>Drupal 9.8.0</name>
<version>9.8.0</version>
<status>published</status>
<release_link>http://example.com/drupal-9-8-0-release</release_link>
<download_link>http://example.com/drupal-9-8-0.tar.gz</download_link>
<date>1250424521</date>
<terms>
<term>
<name>Release type</name>
<value>New features</value>
</term>
<term>
<name>Release type</name>
<value>Bug fixes</value>
</term>
<term>
<name>Release type</name>
<value>Insecure</value>
</term>
</terms>
</release>
<release>
<name>Drupal 9.7.1</name>
<version>9.7.1</version>
<status>published</status>
<release_link>http://example.com/drupal-9-7-1-release</release_link>
<download_link>http://example.com/drupal-9-7-1.tar.gz</download_link>
<date>1250425521</date>
<terms>
<term>
<name>Release type</name>
<value>New features</value>
</term>
<term>
<name>Release type</name>
<value>Bug fixes</value>
</term>
</terms>
</release>
<release>
<name>Drupal 9.7.0</name>
<version>9.7.0</version>
<status>published</status>
<release_link>http://example.com/drupal-9-7-0-release</release_link>
<download_link>http://example.com/drupal-9-7-0.tar.gz</download_link>
<date>1250424521</date>
<terms>
<term>
<name>Release type</name>
<value>New features</value>
</term>
<term>
<name>Release type</name>
<value>Bug fixes</value>
</term>
</terms>
</release>
</releases>
</project>
<?php
declare(strict_types = 1);
namespace Drupal\automatic_updates\Validator\VersionPolicy;
use Drupal\automatic_updates\VersionParsingTrait;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Symfony\Component\DependencyInjection\ContainerInterface;
/**
* A policy rule that allows minor updates if enabled in configuration.
*
* @internal
* This is an internal part of Automatic Updates' version policy for
* Drupal core. It may be changed or removed at any time without warning.
* External code should not interact with this class.
*/
final class MinorUpdatesEnabled implements ContainerInjectionInterface {
use StringTranslationTrait;
use VersionParsingTrait;
/**
* The config factory service.
*
* @var \Drupal\Core\Config\ConfigFactoryInterface
*/
private $configFactory;
/**
* Constructs a MinorUpdatesEnabled object.
*
* @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
* The config factory service.
*/
public function __construct(ConfigFactoryInterface $config_factory) {
$this->configFactory = $config_factory;
}
/**
* {@inheritdoc}
*/
public static function create(ContainerInterface $container) {
return new static(
$container->get('config.factory')
);
}
/**
* Checks that the target minor version of Drupal can be updated to.
*
* The update will only be allowed if the allow_core_minor_updates flag is
* set to TRUE in config.
*
* @param string $installed_version
* The installed version of Drupal.
* @param string|null $target_version
* The target version of Drupal, or NULL if not known.
*
* @return \Drupal\Core\StringTranslation\TranslatableMarkup[]
* The error messages, if any.
*/
public function validate(string $installed_version, ?string $target_version): array {
$installed_minor = static::getMajorAndMinorVersion($installed_version);
$target_minor = static::getMajorAndMinorVersion($target_version);
if ($installed_minor === $target_minor) {
return [];
}
$minor_updates_allowed = $this->configFactory->get('automatic_updates.settings')
->get('allow_core_minor_updates');
if (!$minor_updates_allowed) {
return [
$this->t('Drupal cannot be automatically updated from @installed_version to @target_version because automatic updates from one minor version to another are not supported.', [
'@installed_version' => $installed_version,
'@target_version' => $target_version,
]),
];
}
return [];
}
}
......@@ -4,22 +4,52 @@ declare(strict_types = 1);
namespace Drupal\automatic_updates\Validator\VersionPolicy;
use Drupal\automatic_updates\VersionParsingTrait;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Symfony\Component\DependencyInjection\ContainerInterface;
/**
* A policy rule requiring the target version to be an installable release.
* A policy rule requiring the target version to be installable.
*
* @internal
* This is an internal part of Automatic Updates' version policy for
* Drupal core. It may be changed or removed at any time without warning.
* External code should not interact with this class.
*/
final class TargetVersionInstallable {
final class TargetVersionInstallable implements ContainerInjectionInterface {
use StringTranslationTrait;
use VersionParsingTrait;
/**
* Checks that the target version of Drupal is a known installable release.
* Constructs a TargetVersionInstallable object.
*
* @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
* The config factory service.
*/
public function __construct(private ConfigFactoryInterface $configFactory) {}
/**
* {@inheritdoc}
*/
public static function create(ContainerInterface $container) {
return new static(
$container->get('config.factory')
);
}
/**
* Checks that the target version can be installed.
*
* This means two things must be true:
* - The target minor version of Drupal can be updated to. The update will
* only be allowed if the allow_core_minor_updates flag is TRUE in config.
* - The target version of Drupal is a known installable release.
*
* If the first check fails, there is no need to do the second check because
* the first check implies that the target version isn't installable.
*
* @param string $installed_version
* The installed version of Drupal.
......@@ -32,6 +62,22 @@ final class TargetVersionInstallable {
* The error messages, if any.
*/
public function validate(string $installed_version, ?string $target_version, array $available_releases): array {
$installed_minor = static::getMajorAndMinorVersion($installed_version);
$target_minor = static::getMajorAndMinorVersion($target_version);
if ($installed_minor !== $target_minor) {
$minor_updates_allowed = $this->configFactory->get('automatic_updates.settings')
->get('allow_core_minor_updates');
if (!$minor_updates_allowed) {
return [
$this->t('Drupal cannot be automatically updated from @installed_version to @target_version because automatic updates from one minor version to another are not supported.', [
'@installed_version' => $installed_version,
'@target_version' => $target_version,
]),
];
}
}
// If the target version isn't in the list of installable releases, we
// should flag an error.
if (empty($available_releases) || !array_key_exists($target_version, $available_releases)) {
......
......@@ -5,6 +5,7 @@ declare(strict_types = 1);
namespace Drupal\automatic_updates\Validator;
use Drupal\automatic_updates\CronUpdateStage;
use Drupal\Component\Utility\NestedArray;
use Drupal\package_manager\ComposerInspector;
use Drupal\package_manager\Event\StatusCheckEvent;
use Drupal\package_manager\PathLocator;
......@@ -13,7 +14,6 @@ use Drupal\automatic_updates\UpdateStage;
use Drupal\automatic_updates\Validator\VersionPolicy\ForbidDowngrade;
use Drupal\automatic_updates\Validator\VersionPolicy\ForbidMinorUpdates;
use Drupal\automatic_updates\Validator\VersionPolicy\MajorVersionMatch;
use Drupal\automatic_updates\Validator\VersionPolicy\MinorUpdatesEnabled;
use Drupal\automatic_updates\Validator\VersionPolicy\StableReleaseInstalled;
use Drupal\automatic_updates\Validator\VersionPolicy\ForbidDevSnapshot;
use Drupal\automatic_updates\Validator\VersionPolicy\SupportedBranchInstalled;
......@@ -110,28 +110,56 @@ final class VersionPolicyValidator implements EventSubscriberInterface {
}
}
}
// If this is not a cron update, and we know the target version, minor
// version updates are allowed if configuration says so.
elseif ($target_version) {
$rules[] = MinorUpdatesEnabled::class;
}
$installed_version = $this->getInstalledVersion();
$available_releases = $this->getAvailableReleases($stage);
// Invoke each rule in the order that they were added to $rules, stopping
// when one returns error messages.
// @todo Return all the error messages in https://www.drupal.org/i/3281379.
// Let all the rules flag whatever messages they need to.
$messages = [];
foreach ($rules as $rule) {
$messages = $this->classResolver
->getInstanceFromDefinition($rule)
$messages[$rule] = $this->classResolver->getInstanceFromDefinition($rule)
->validate($installed_version, $target_version, $available_releases);
}
// Remove any messages that are superseded by other, more specific ones.
$filtered_rule_messages = array_filter($messages, fn ($rule) => !self::isRuleSuperseded($rule, $messages), ARRAY_FILTER_USE_KEY);
// Collapse all the rules' messages into a single array.
return NestedArray::mergeDeepArray($filtered_rule_messages);
}
if ($messages) {
return $messages;
/**
* Check if a given rule's messages are superseded by a more specific rule.
*
* @param string $rule
* The rule to check.
* @param array[] $rule_messages
* The messages that were returned by the various rules, keyed by the name
* of the rule that returned them.
*
* @return bool
* TRUE if the given rule is superseded by another rule, FALSE otherwise.
*/
private static function isRuleSuperseded(string $rule, array $rule_messages): bool {
// Some rules' messages are more specific than other rules' messages. For
// example, if the message "… automatic updates from one major version to
// another are not supported" is returned, then the message "… not in the
// list of installable releases" is not needed because the new major version
// will not be in the list of installable releases. The keys of this array
// are the rules which supersede messages from the values, which are the
// less specific rules.
$more_specific_rule_sets = [
ForbidDowngrade::class => [TargetVersionInstallable::class, MajorVersionMatch::class],
ForbidDevSnapshot::class => [StableReleaseInstalled::class],
MajorVersionMatch::class => [TargetVersionInstallable::class],
ForbidMinorUpdates::class => [TargetVersionInstallable::class],
];
foreach ($more_specific_rule_sets as $more_specific_rule => $less_specific_rules) {
// If the more specific rule flagged any messages, the given rule is
// superseded.
if (!empty($rule_messages[$more_specific_rule]) && in_array($rule, $less_specific_rules, TRUE)) {
return TRUE;
}
}
return [];
return FALSE;
}
/**
......
......@@ -112,4 +112,11 @@ class TestCronUpdateStage extends CronUpdateStage {
$this->handlePostApply($stage_id, $start_version, $target_version);
}
/**
* {@inheritdoc}
*/
public function setMetadata(string $key, $data): void {
parent::setMetadata($key, $data);
}
}
<?php
declare(strict_types = 1);
namespace Drupal\Tests\automatic_updates\Unit\VersionPolicy;
use Drupal\automatic_updates\Validator\VersionPolicy\MinorUpdatesEnabled;
use Drupal\Tests\automatic_updates\Traits\VersionPolicyTestTrait;
use Drupal\Tests\UnitTestCase;
/**
* @covers \Drupal\automatic_updates\Validator\VersionPolicy\MinorUpdatesEnabled
* @group automatic_updates
* @internal
*/
class MinorUpdatesEnabledTest extends UnitTestCase {
use VersionPolicyTestTrait;
/**
* Data provider for testMinorUpdatesEnabled().
*
* @return mixed[][]
* The test cases.
*/
public function providerMinorUpdatesEnabled(): array {
return [
'same versions, minor updates forbidden' => [
FALSE,
'9.8.0',
'9.8.0',
[],
],
'same versions, minor updates allowed' => [
TRUE,
'9.8.0',
'9.8.0',
[],
],
'target version newer in same minor, minor updates forbidden' => [
FALSE,
'9.8.0',
'9.8.2',
[],
],
'target version newer in same minor, minor updates allowed' => [
TRUE,
'9.8.0',
'9.8.2',
[],
],
'target version in newer minor, minor updates forbidden' => [
FALSE,
'9.8.0',
'9.9.2',
['Drupal cannot be automatically updated from 9.8.0 to 9.9.2 because automatic updates from one minor version to another are not supported.'],
],
'target version in newer minor, minor updates allowed' => [
TRUE,
'9.8.0',
'9.9.2',
[],
],
'target version older in same minor, minor updates forbidden' => [
FALSE,
'9.8.2',
'9.8.0',
[],
],
'target version older in same minor, minor updates allowed' => [
TRUE,
'9.8.2',
'9.8.0',
[],
],
'target version in older minor, minor updates forbidden' => [
FALSE,
'9.8.0',
'9.7.2',
['Drupal cannot be automatically updated from 9.8.0 to 9.7.2 because automatic updates from one minor version to another are not supported.'],
],
'target version in older minor, minor updates allowed' => [
TRUE,
'9.8.0',
'9.7.2',
[],
],
'target version in older major, minor updates forbidden' => [
FALSE,
'9.8.0',
'8.8.0',
['Drupal cannot be automatically updated from 9.8.0 to 8.8.0 because automatic updates from one minor version to another are not supported.'],
],
'target version in older major, minor updates allowed' => [
FALSE,
'9.8.0',
'8.8.0',
['Drupal cannot be automatically updated from 9.8.0 to 8.8.0 because automatic updates from one minor version to another are not supported.'],
],
'target version in newer major, minor updates forbidden' => [
FALSE,
'9.8.0',
'10.8.0',
['Drupal cannot be automatically updated from 9.8.0 to 10.8.0 because automatic updates from one minor version to another are not supported.'],
],
'target version in newer major, minor updates allowed' => [
FALSE,
'9.8.0',
'10.8.0',
['Drupal cannot be automatically updated from 9.8.0 to 10.8.0 because automatic updates from one minor version to another are not supported.'],
],
];
}
/**
* Tests that trying to update across minor versions depends on configuration.
*
* @param bool $allowed
* Whether or not updating across minor versions is allowed.
* @param string $installed_version
* The installed version of Drupal core.
* @param string|null $target_version
* The target version of Drupal core, or NULL if not known.
* @param string[] $expected_errors
* The expected error messages, if any.
*
* @dataProvider providerMinorUpdatesEnabled
*/
public function testMinorUpdatesEnabled(bool $allowed, string $installed_version, ?string $target_version, array $expected_errors): void {
$config_factory = $this->getConfigFactoryStub([
'automatic_updates.settings' => [
'allow_core_minor_updates' => $allowed,
],
]);
$rule = new MinorUpdatesEnabled($config_factory);
$this->assertPolicyErrors($rule, $installed_version, $target_version, $expected_errors);
}
}
......@@ -27,10 +27,16 @@ class TargetVersionInstallableTest extends UnitTestCase {
public function providerTargetVersionInstallable(): array {
return [
'no available releases' => [
[TRUE, FALSE],
'9.8.1',
'9.8.2',
[],
['Cannot update Drupal core to 9.8.2 because it is not in the list of installable releases.'],
],
'unknown target' => [
[TRUE, FALSE],
'9.8.1',
'9.8.2',
[
'9.8.1' => ProjectRelease::createFromArray([
'status' => 'published',
......@@ -41,6 +47,9 @@ class TargetVersionInstallableTest extends UnitTestCase {
['Cannot update Drupal core to 9.8.2 because it is not in the list of installable releases.'],
],
'valid target' => [
[TRUE, FALSE],
'9.8.1',
'9.8.2',
[
'9.8.2' => ProjectRelease::createFromArray([
'status' => 'published',
......@@ -50,12 +59,126 @@ class TargetVersionInstallableTest extends UnitTestCase {
],
[],
],
'installed version and target version are the same' => [
[TRUE, FALSE],
'9.8.0',
'9.8.0',
[],
['Cannot update Drupal core to 9.8.0 because it is not in the list of installable releases.'],
],
'unknown patch update' => [
[TRUE, FALSE],
'9.8.0',
'9.8.2',
[],
['Cannot update Drupal core to 9.8.2 because it is not in the list of installable releases.'],
],
'valid target version newer in same minor' => [
[TRUE, FALSE],
'9.8.0',
'9.8.2',
[
'9.8.2' => ProjectRelease::createFromArray([
'status' => 'published',
'release_link' => 'http://example.com/drupal-9-8-2-release',
'version' => '9.8.2',
]),
],
[],
],
'target version in newer minor, minor updates forbidden' => [
[FALSE],
'9.8.0',
'9.9.2',
[],
['Drupal cannot be automatically updated from 9.8.0 to 9.9.2 because automatic updates from one minor version to another are not supported.'],
],
'unknown target version in newer minor, minor updates allowed' => [
[TRUE],
'9.8.0',
'9.9.2',
[],
['Cannot update Drupal core to 9.9.2 because it is not in the list of installable releases.'],
],
'valid target version in newer minor, minor updates allowed' => [
[TRUE],
'9.8.0',
'9.9.2',
[
'9.9.2' => ProjectRelease::createFromArray([
'status' => 'published',
'release_link' => 'http://example.com/drupal-9-9-2-release',
'version' => '9.9.2',
]),
],
[],
],
'target version older in same minor' => [
[TRUE, FALSE],
'9.8.2',
'9.8.0',
[],
['Cannot update Drupal core to 9.8.0 because it is not in the list of installable releases.'],
],
'target version in older minor, minor updates forbidden' => [
[FALSE],
'9.8.0',
'9.7.2',
[],
['Drupal cannot be automatically updated from 9.8.0 to 9.7.2 because automatic updates from one minor version to another are not supported.'],
],
'target version in older minor, minor updates allowed' => [
[TRUE],
'9.8.0',
'9.7.2',
[],
['Cannot update Drupal core to 9.7.2 because it is not in the list of installable releases.'],
],
// In practice, the message produced by the next four cases will be
// superseded by the MajorVersionMatch rule.
// @see \Drupal\automatic_updates\Validator\VersionPolicy\MajorVersionMatch
// @see \Drupal\automatic_updates\Validator\VersionPolicyValidator::isRuleSuperseded()
'target version in older major, minor updates forbidden' => [
[FALSE],
'9.8.0',
'8.8.0',
[],
['Drupal cannot be automatically updated from 9.8.0 to 8.8.0 because automatic updates from one minor version to another are not supported.'],
],
'target version in older major, minor updates allowed' => [
[TRUE],
'9.8.0',
'8.8.0',
[],
['Cannot update Drupal core to 8.8.0 because it is not in the list of installable releases.'],
],
'target version in newer major, minor updates forbidden' => [
[FALSE],
'9.8.0',
'10.8.0',
[],
['Drupal cannot be automatically updated from 9.8.0 to 10.8.0 because automatic updates from one minor version to another are not supported.'],
],
'target version in newer major, minor updates allowed' => [
[TRUE],
'9.8.0',
'10.8.0',
[],
['Cannot update Drupal core to 10.8.0 because it is not in the list of installable releases.'],
],
];
}
/**
* Tests that the target version must be a known, installable release.
*
* @param bool[] $minor_updates_allowed
* The values of the allow_core_minor_updates config flag. The rule will be
* tested separately with each value.
* @param string $installed_version
* The installed version of Drupal core.
* @param string $target_version
* The target version of Drupal core, or NULL if not known.
* @param \Drupal\update\ProjectRelease[] $available_releases
* The available releases of Drupal core, keyed by version.
* @param string[] $expected_errors
......@@ -63,9 +186,16 @@ class TargetVersionInstallableTest extends UnitTestCase {
*
* @dataProvider providerTargetVersionInstallable
*/
public function testTargetVersionInstallable(array $available_releases, array $expected_errors): void {
$rule = new TargetVersionInstallable();
$this->assertPolicyErrors($rule, '9.8.1', '9.8.2', $expected_errors, $available_releases);
public function testTargetVersionInstallable(array $minor_updates_allowed, string $installed_version, string $target_version, array $available_releases, array $expected_errors): void {
foreach ($minor_updates_allowed as $value) {
$config_factory = $this->getConfigFactoryStub([
'automatic_updates.settings' => [
'allow_core_minor_updates' => $value,
],
]);
$rule = new TargetVersionInstallable($config_factory);
$this->assertPolicyErrors($rule, $installed_version, $target_version, $expected_errors, $available_releases);
}
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment