diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index 71e8e34ea3..4ec9e24f4f 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -126,7 +126,7 @@ Form FrameworkBundle --------------- - * Removed support for `bundle:controller:action` and `service:action` syntaxes to reference controllers. Use `serviceOrFqcn::method` + * Removed support for `bundle:controller:action` syntax to reference controllers. Use `serviceOrFqcn::method` instead where `serviceOrFqcn` is either the service ID when using controllers as services or the FQCN of the controller. Before: @@ -136,11 +136,6 @@ FrameworkBundle path: / defaults: _controller: FrameworkBundle:Redirect:redirect - - service_controller: - path: / - defaults: - _controller: app.my_controller:myAction ``` After: @@ -150,11 +145,6 @@ FrameworkBundle path: / defaults: _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction - - service_controller: - path: / - defaults: - _controller: app.my_controller::myAction ``` * Removed `Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser`. diff --git a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php index dd396eeee4..dc6df3f9b9 100644 --- a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php +++ b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php @@ -33,7 +33,7 @@ class DeprecationErrorHandler * - use "/some-regexp/" to stop the test suite whenever a deprecation * message matches the given regular expression; * - use a number to define the upper bound of allowed deprecations, - * making the test suite fail whenever more notices are trigerred. + * making the test suite fail whenever more notices are triggered. * * @param int|string|false $mode The reporting mode, defaults to not allowing any deprecations */ diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index ff01fcc052..f4b20ee92d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -507,6 +507,7 @@ class FrameworkExtension extends Extension foreach ($config['workflows'] as $name => $workflow) { $type = $workflow['type']; + $workflowId = sprintf('%s.%s', $type, $name); // Process Metadata (workflow + places (transition is done in the "create transition" block)) $metadataStoreDefinition = new Definition(Workflow\Metadata\InMemoryMetadataStore::class, array(array(), array(), null)); @@ -525,11 +526,25 @@ class FrameworkExtension extends Extension // Create transitions $transitions = array(); + $guardsConfiguration = array(); $transitionsMetadataDefinition = new Definition(\SplObjectStorage::class); + // Global transition counter per workflow + $transitionCounter = 0; foreach ($workflow['transitions'] as $transition) { if ('workflow' === $type) { $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to'])); - $transitions[] = $transitionDefinition; + $transitionDefinition->setPublic(false); + $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); + $container->setDefinition($transitionId, $transitionDefinition); + $transitions[] = new Reference($transitionId); + if (isset($transition['guard'])) { + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument(new Reference($transitionId)); + $configuration->addArgument($transition['guard']); + $configuration->setPublic(false); + $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); + $guardsConfiguration[$eventName][] = $configuration; + } if ($transition['metadata']) { $transitionsMetadataDefinition->addMethodCall('attach', array( $transitionDefinition, @@ -540,7 +555,18 @@ class FrameworkExtension extends Extension foreach ($transition['from'] as $from) { foreach ($transition['to'] as $to) { $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to)); - $transitions[] = $transitionDefinition; + $transitionDefinition->setPublic(false); + $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); + $container->setDefinition($transitionId, $transitionDefinition); + $transitions[] = new Reference($transitionId); + if (isset($transition['guard'])) { + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument(new Reference($transitionId)); + $configuration->addArgument($transition['guard']); + $configuration->setPublic(false); + $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); + $guardsConfiguration[$eventName][] = $configuration; + } if ($transition['metadata']) { $transitionsMetadataDefinition->addMethodCall('attach', array( $transitionDefinition, @@ -582,7 +608,6 @@ class FrameworkExtension extends Extension } // Create Workflow - $workflowId = sprintf('%s.%s', $type, $name); $workflowDefinition = new ChildDefinition(sprintf('%s.abstract', $type)); $workflowDefinition->replaceArgument(0, new Reference(sprintf('%s.definition', $workflowId))); if (isset($markingStoreDefinition)) { @@ -619,16 +644,7 @@ class FrameworkExtension extends Extension } // Add Guard Listener - $guard = new Definition(Workflow\EventListener\GuardListener::class); - $guard->setPrivate(true); - $configuration = array(); - foreach ($workflow['transitions'] as $config) { - $transitionName = $config['name']; - - if (!isset($config['guard'])) { - continue; - } - + if ($guardsConfiguration) { if (!class_exists(ExpressionLanguage::class)) { throw new LogicException('Cannot guard workflows as the ExpressionLanguage component is not installed. Try running "composer require symfony/expression-language".'); } @@ -637,13 +653,11 @@ class FrameworkExtension extends Extension throw new LogicException('Cannot guard workflows as the Security component is not installed. Try running "composer require symfony/security".'); } - $eventName = sprintf('workflow.%s.guard.%s', $name, $transitionName); - $guard->addTag('kernel.event_listener', array('event' => $eventName, 'method' => 'onTransition')); - $configuration[$eventName] = $config['guard']; - } - if ($configuration) { + $guard = new Definition(Workflow\EventListener\GuardListener::class); + $guard->setPrivate(true); + $guard->setArguments(array( - $configuration, + $guardsConfiguration, new Reference('workflow.security.expression_language'), new Reference('security.token_storage'), new Reference('security.authorization_checker'), @@ -651,6 +665,9 @@ class FrameworkExtension extends Extension new Reference('security.role_hierarchy'), new Reference('validator', ContainerInterface::NULL_ON_INVALID_REFERENCE), )); + foreach ($guardsConfiguration as $eventName => $config) { + $guard->addTag('kernel.event_listener', array('event' => $eventName, 'method' => 'onTransition')); + } $container->setDefinition(sprintf('%s.listener.guard', $workflowId), $guard); $container->setParameter('workflow.has_guard_listeners', true); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml index 8cdbbbd9fb..b36d7d42d6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml @@ -70,6 +70,11 @@ + + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index c99e543fa5..f2b57a31e7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -314,6 +314,7 @@ + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php new file mode 100644 index 0000000000..89c86339af --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php @@ -0,0 +1,51 @@ +loadFromExtension('framework', array( + 'workflows' => array( + 'article' => array( + 'type' => 'workflow', + 'marking_store' => array( + 'type' => 'multiple_state', + ), + 'supports' => array( + FrameworkExtensionTest::class, + ), + 'initial_place' => 'draft', + 'places' => array( + 'draft', + 'wait_for_journalist', + 'approved_by_journalist', + 'wait_for_spellchecker', + 'approved_by_spellchecker', + 'published', + ), + 'transitions' => array( + 'request_review' => array( + 'from' => 'draft', + 'to' => array('wait_for_journalist', 'wait_for_spellchecker'), + ), + 'journalist_approval' => array( + 'from' => 'wait_for_journalist', + 'to' => 'approved_by_journalist', + ), + 'spellchecker_approval' => array( + 'from' => 'wait_for_spellchecker', + 'to' => 'approved_by_spellchecker', + ), + 'publish' => array( + 'from' => array('approved_by_journalist', 'approved_by_spellchecker'), + 'to' => 'published', + 'guard' => '!!true', + ), + 'publish_editor_in_chief' => array( + 'name' => 'publish', + 'from' => 'draft', + 'to' => 'published', + 'guard' => '!!false', + ), + ), + ), + ), +)); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml new file mode 100644 index 0000000000..a5124d1fe7 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml @@ -0,0 +1,48 @@ + + + + + + + + a + a + + Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest + draft + wait_for_journalist + approved_by_journalist + wait_for_spellchecker + approved_by_spellchecker + published + + draft + wait_for_journalist + wait_for_spellchecker + + + wait_for_journalist + approved_by_journalist + + + wait_for_spellchecker + approved_by_spellchecker + + + approved_by_journalist + approved_by_spellchecker + published + !!true + + + draft + published + !!false + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml new file mode 100644 index 0000000000..458cb4ae1e --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml @@ -0,0 +1,35 @@ +framework: + workflows: + article: + type: workflow + marking_store: + type: multiple_state + supports: + - Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest + initial_place: draft + places: + - draft + - wait_for_journalist + - approved_by_journalist + - wait_for_spellchecker + - approved_by_spellchecker + - published + transitions: + request_review: + from: [draft] + to: [wait_for_journalist, wait_for_spellchecker] + journalist_approval: + from: [wait_for_journalist] + to: [approved_by_journalist] + spellchecker_approval: + from: [wait_for_spellchecker] + to: [approved_by_spellchecker] + publish: + from: [approved_by_journalist, approved_by_spellchecker] + to: [published] + guard: "!!true" + publish_editor_in_chief: + name: publish + from: [draft] + to: [published] + guard: "!!false" diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 0e16447cf5..6fdec21057 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -323,14 +323,84 @@ abstract class FrameworkExtensionTest extends TestCase $this->assertCount(5, $transitions); - $this->assertSame('request_review', $transitions[0]->getArgument(0)); - $this->assertSame('journalist_approval', $transitions[1]->getArgument(0)); - $this->assertSame('spellchecker_approval', $transitions[2]->getArgument(0)); - $this->assertSame('publish', $transitions[3]->getArgument(0)); - $this->assertSame('publish', $transitions[4]->getArgument(0)); + $this->assertSame('workflow.article.transition.0', (string) $transitions[0]); + $this->assertSame(array( + 'request_review', + array( + 'draft', + ), + array( + 'wait_for_journalist', 'wait_for_spellchecker', + ), + ), $container->getDefinition($transitions[0])->getArguments()); - $this->assertSame(array('approved_by_journalist', 'approved_by_spellchecker'), $transitions[3]->getArgument(1)); - $this->assertSame(array('draft'), $transitions[4]->getArgument(1)); + $this->assertSame('workflow.article.transition.1', (string) $transitions[1]); + $this->assertSame(array( + 'journalist_approval', + array( + 'wait_for_journalist', + ), + array( + 'approved_by_journalist', + ), + ), $container->getDefinition($transitions[1])->getArguments()); + + $this->assertSame('workflow.article.transition.2', (string) $transitions[2]); + $this->assertSame(array( + 'spellchecker_approval', + array( + 'wait_for_spellchecker', + ), + array( + 'approved_by_spellchecker', + ), + ), $container->getDefinition($transitions[2])->getArguments()); + + $this->assertSame('workflow.article.transition.3', (string) $transitions[3]); + $this->assertSame(array( + 'publish', + array( + 'approved_by_journalist', + 'approved_by_spellchecker', + ), + array( + 'published', + ), + ), $container->getDefinition($transitions[3])->getArguments()); + + $this->assertSame('workflow.article.transition.4', (string) $transitions[4]); + $this->assertSame(array( + 'publish', + array( + 'draft', + ), + array( + 'published', + ), + ), $container->getDefinition($transitions[4])->getArguments()); + } + + public function testGuardExpressions() + { + $container = $this->createContainerFromFile('workflow_with_guard_expression'); + + $this->assertTrue($container->hasDefinition('workflow.article.listener.guard'), 'Workflow guard listener is registered as a service'); + $this->assertTrue($container->hasParameter('workflow.has_guard_listeners'), 'Workflow guard listeners parameter exists'); + $this->assertTrue(true === $container->getParameter('workflow.has_guard_listeners'), 'Workflow guard listeners parameter is enabled'); + $guardDefinition = $container->getDefinition('workflow.article.listener.guard'); + $this->assertSame(array( + array( + 'event' => 'workflow.article.guard.publish', + 'method' => 'onTransition', + ), + ), $guardDefinition->getTag('kernel.event_listener')); + $guardsConfiguration = $guardDefinition->getArgument(0); + $this->assertTrue(1 === \count($guardsConfiguration), 'Workflow guard configuration contains one element per transition name'); + $transitionGuardExpressions = $guardsConfiguration['workflow.article.guard.publish']; + $this->assertSame('workflow.article.transition.3', (string) $transitionGuardExpressions[0]->getArgument(0)); + $this->assertSame('!!true', $transitionGuardExpressions[0]->getArgument(1)); + $this->assertSame('workflow.article.transition.4', (string) $transitionGuardExpressions[1]->getArgument(0)); + $this->assertSame('!!false', $transitionGuardExpressions[1]->getArgument(1)); } public function testWorkflowServicesCanBeEnabled() diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/config.html.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/config.html.twig index 3c4135db3c..aa0e452375 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/config.html.twig +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/config.html.twig @@ -88,7 +88,7 @@
Help - + Symfony Support Channels diff --git a/src/Symfony/Component/Config/Definition/ArrayNode.php b/src/Symfony/Component/Config/Definition/ArrayNode.php index e28084a40f..baebaa29f4 100644 --- a/src/Symfony/Component/Config/Definition/ArrayNode.php +++ b/src/Symfony/Component/Config/Definition/ArrayNode.php @@ -292,7 +292,10 @@ class ArrayNode extends BaseNode implements PrototypeNodeInterface $normalized = array(); foreach ($value as $name => $val) { if (isset($this->children[$name])) { - $normalized[$name] = $this->children[$name]->normalize($val); + try { + $normalized[$name] = $this->children[$name]->normalize($val); + } catch (UnsetKeyException $e) { + } unset($value[$name]); } elseif (!$this->removeExtraKeys) { $normalized[$name] = $val; diff --git a/src/Symfony/Component/Config/Definition/Builder/ExprBuilder.php b/src/Symfony/Component/Config/Definition/Builder/ExprBuilder.php index 5c070bee7c..7ba19515b8 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ExprBuilder.php +++ b/src/Symfony/Component/Config/Definition/Builder/ExprBuilder.php @@ -174,7 +174,7 @@ class ExprBuilder } /** - * Sets a closure marking the value as invalid at validation time. + * Sets a closure marking the value as invalid at processing time. * * if you want to add the value of the node in your message just use a %s placeholder. * @@ -192,7 +192,7 @@ class ExprBuilder } /** - * Sets a closure unsetting this key of the array at validation time. + * Sets a closure unsetting this key of the array at processing time. * * @return $this * diff --git a/src/Symfony/Component/Config/Tests/Definition/Builder/ArrayNodeDefinitionTest.php b/src/Symfony/Component/Config/Tests/Definition/Builder/ArrayNodeDefinitionTest.php index 8c269eb42c..0d4cad96bd 100644 --- a/src/Symfony/Component/Config/Tests/Definition/Builder/ArrayNodeDefinitionTest.php +++ b/src/Symfony/Component/Config/Tests/Definition/Builder/ArrayNodeDefinitionTest.php @@ -232,6 +232,25 @@ class ArrayNodeDefinitionTest extends TestCase $this->assertFalse($this->getField($node, 'normalizeKeys')); } + public function testUnsetChild() + { + $node = new ArrayNodeDefinition('root'); + $node + ->children() + ->scalarNode('value') + ->beforeNormalization() + ->ifTrue(function ($value) { + return empty($value); + }) + ->thenUnset() + ->end() + ->end() + ->end() + ; + + $this->assertSame(array(), $node->getNode()->normalize(array('value' => null))); + } + public function testPrototypeVariable() { $node = new ArrayNodeDefinition('root'); diff --git a/src/Symfony/Component/Console/Command/Command.php b/src/Symfony/Component/Console/Command/Command.php index 6050733bb7..0e23847379 100644 --- a/src/Symfony/Component/Console/Command/Command.php +++ b/src/Symfony/Component/Console/Command/Command.php @@ -379,11 +379,11 @@ class Command /** * Adds an option. * - * @param string $name The option name - * @param string|array $shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts - * @param int|null $mode The option mode: One of the VALUE_* constants - * @param string $description A description text - * @param string|string[]|bool|null $default The default value (must be null for self::VALUE_NONE) + * @param string $name The option name + * @param string|array $shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts + * @param int|null $mode The option mode: One of the VALUE_* constants + * @param string $description A description text + * @param string|string[]|int|bool|null $default The default value (must be null for self::VALUE_NONE) * * @throws InvalidArgumentException If option mode is invalid or incompatible * diff --git a/src/Symfony/Component/Form/Extension/Core/CoreExtension.php b/src/Symfony/Component/Form/Extension/Core/CoreExtension.php index 99578b4992..5b01e9e90a 100644 --- a/src/Symfony/Component/Form/Extension/Core/CoreExtension.php +++ b/src/Symfony/Component/Form/Extension/Core/CoreExtension.php @@ -16,8 +16,10 @@ use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator; use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface; use Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory; use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator; +use Symfony\Component\Form\Extension\Core\Type\TransformationFailureExtension; use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; +use Symfony\Component\Translation\TranslatorInterface; /** * Represents the main form extension, which loads the core functionality. @@ -28,11 +30,13 @@ class CoreExtension extends AbstractExtension { private $propertyAccessor; private $choiceListFactory; + private $translator; - public function __construct(PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null) + public function __construct(PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null, TranslatorInterface $translator = null) { $this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor(); $this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(new PropertyAccessDecorator(new DefaultChoiceListFactory(), $this->propertyAccessor)); + $this->translator = $translator; } protected function loadTypes() @@ -74,4 +78,11 @@ class CoreExtension extends AbstractExtension new Type\ColorType(), ); } + + protected function loadTypeExtensions() + { + return array( + new TransformationFailureExtension($this->translator), + ); + } } diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php index f9721e52b1..9e86310b22 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php +++ b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php @@ -73,16 +73,17 @@ class PropertyPathMapper implements DataMapperInterface // Write-back is disabled if the form is not synchronized (transformation failed), // if the form was not submitted and if the form is disabled (modification not allowed) if (null !== $propertyPath && $config->getMapped() && $form->isSubmitted() && $form->isSynchronized() && !$form->isDisabled()) { - // If the field is of type DateTime and the data is the same skip the update to + $propertyValue = $form->getData(); + // If the field is of type DateTimeInterface and the data is the same skip the update to // keep the original object hash - if ($form->getData() instanceof \DateTime && $form->getData() == $this->propertyAccessor->getValue($data, $propertyPath)) { + if ($propertyValue instanceof \DateTimeInterface && $propertyValue == $this->propertyAccessor->getValue($data, $propertyPath)) { continue; } // If the data is identical to the value in $data, we are // dealing with a reference - if (!\is_object($data) || !$config->getByReference() || $form->getData() !== $this->propertyAccessor->getValue($data, $propertyPath)) { - $this->propertyAccessor->setValue($data, $propertyPath, $form->getData()); + if (!\is_object($data) || !$config->getByReference() || $propertyValue !== $this->propertyAccessor->getValue($data, $propertyPath)) { + $this->propertyAccessor->setValue($data, $propertyPath, $propertyValue); } } } diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/TransformationFailureListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/TransformationFailureListener.php new file mode 100644 index 0000000000..f46eb499e0 --- /dev/null +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/TransformationFailureListener.php @@ -0,0 +1,64 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Extension\Core\EventListener; + +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\Form\FormError; +use Symfony\Component\Form\FormEvent; +use Symfony\Component\Form\FormEvents; +use Symfony\Component\Translation\TranslatorInterface; + +/** + * @author Christian Flothmann + */ +class TransformationFailureListener implements EventSubscriberInterface +{ + private $translator; + + public function __construct(TranslatorInterface $translator = null) + { + $this->translator = $translator; + } + + public static function getSubscribedEvents() + { + return array( + FormEvents::POST_SUBMIT => array('convertTransformationFailureToFormError', -1024), + ); + } + + public function convertTransformationFailureToFormError(FormEvent $event) + { + $form = $event->getForm(); + + if (null === $form->getTransformationFailure() || !$form->isValid()) { + return; + } + + foreach ($form as $child) { + if (!$child->isSynchronized()) { + return; + } + } + + $clientDataAsString = is_scalar($form->getViewData()) ? (string) $form->getViewData() : \gettype($form->getViewData()); + $messageTemplate = 'The value {{ value }} is not valid.'; + + if (null !== $this->translator) { + $message = $this->translator->trans($messageTemplate, array('{{ value }}' => $clientDataAsString)); + } else { + $message = strtr($messageTemplate, array('{{ value }}' => $clientDataAsString)); + } + + $form->addError(new FormError($message, $messageTemplate, array('{{ value }}' => $clientDataAsString), null, $form->getTransformationFailure())); + } +} diff --git a/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php b/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php index c0a5e6ab1a..653307e081 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php @@ -91,6 +91,9 @@ class DateTimeType extends AbstractType )); } } else { + // when the form is compound the entries of the array are ignored in favor of children data + // so we need to handle the cascade setting here + $emptyData = $builder->getEmptyData() ?: array(); // Only pass a subset of the options to children $dateOptions = array_intersect_key($options, array_flip(array( 'years', @@ -105,6 +108,10 @@ class DateTimeType extends AbstractType 'invalid_message_parameters', ))); + if (isset($emptyData['date'])) { + $dateOptions['empty_data'] = $emptyData['date']; + } + $timeOptions = array_intersect_key($options, array_flip(array( 'hours', 'minutes', @@ -120,6 +127,10 @@ class DateTimeType extends AbstractType 'invalid_message_parameters', ))); + if (isset($emptyData['time'])) { + $timeOptions['empty_data'] = $emptyData['time']; + } + if (false === $options['label']) { $dateOptions['label'] = false; $timeOptions['label'] = false; @@ -237,6 +248,9 @@ class DateTimeType extends AbstractType 'compound' => $compound, 'date_label' => null, 'time_label' => null, + 'empty_data' => function (Options $options) { + return $options['compound'] ? array() : ''; + }, )); // Don't add some defaults in order to preserve the defaults diff --git a/src/Symfony/Component/Form/Extension/Core/Type/DateType.php b/src/Symfony/Component/Form/Extension/Core/Type/DateType.php index 6d6eb17afb..e8519746f7 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/DateType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/DateType.php @@ -76,7 +76,21 @@ class DateType extends AbstractType $yearOptions = $monthOptions = $dayOptions = array( 'error_bubbling' => true, + 'empty_data' => '', ); + // when the form is compound the entries of the array are ignored in favor of children data + // so we need to handle the cascade setting here + $emptyData = $builder->getEmptyData() ?: array(); + + if (isset($emptyData['year'])) { + $yearOptions['empty_data'] = $emptyData['year']; + } + if (isset($emptyData['month'])) { + $monthOptions['empty_data'] = $emptyData['month']; + } + if (isset($emptyData['day'])) { + $dayOptions['empty_data'] = $emptyData['day']; + } if (isset($options['invalid_message'])) { $dayOptions['invalid_message'] = $options['invalid_message']; @@ -265,6 +279,9 @@ class DateType extends AbstractType // this option. 'data_class' => null, 'compound' => $compound, + 'empty_data' => function (Options $options) { + return $options['compound'] ? array() : ''; + }, 'choice_translation_domain' => false, )); diff --git a/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php b/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php index 4fd8866c99..ec063985f7 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php @@ -71,7 +71,15 @@ class TimeType extends AbstractType } else { $hourOptions = $minuteOptions = $secondOptions = array( 'error_bubbling' => true, + 'empty_data' => '', ); + // when the form is compound the entries of the array are ignored in favor of children data + // so we need to handle the cascade setting here + $emptyData = $builder->getEmptyData() ?: array(); + + if (isset($emptyData['hour'])) { + $hourOptions['empty_data'] = $emptyData['hour']; + } if (isset($options['invalid_message'])) { $hourOptions['invalid_message'] = $options['invalid_message']; @@ -136,10 +144,16 @@ class TimeType extends AbstractType $builder->add('hour', self::$widgets[$options['widget']], $hourOptions); if ($options['with_minutes']) { + if (isset($emptyData['minute'])) { + $minuteOptions['empty_data'] = $emptyData['minute']; + } $builder->add('minute', self::$widgets[$options['widget']], $minuteOptions); } if ($options['with_seconds']) { + if (isset($emptyData['second'])) { + $secondOptions['empty_data'] = $emptyData['second']; + } $builder->add('second', self::$widgets[$options['widget']], $secondOptions); } @@ -258,6 +272,9 @@ class TimeType extends AbstractType // representation is not \DateTime, but an array, we need to unset // this option. 'data_class' => null, + 'empty_data' => function (Options $options) { + return $options['compound'] ? array() : ''; + }, 'compound' => $compound, 'choice_translation_domain' => false, )); diff --git a/src/Symfony/Component/Form/Extension/Core/Type/TransformationFailureExtension.php b/src/Symfony/Component/Form/Extension/Core/Type/TransformationFailureExtension.php new file mode 100644 index 0000000000..98875594d6 --- /dev/null +++ b/src/Symfony/Component/Form/Extension/Core/Type/TransformationFailureExtension.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Extension\Core\Type; + +use Symfony\Component\Form\AbstractTypeExtension; +use Symfony\Component\Form\Extension\Core\EventListener\TransformationFailureListener; +use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\Translation\TranslatorInterface; + +/** + * @author Christian Flothmann + */ +class TransformationFailureExtension extends AbstractTypeExtension +{ + private $translator; + + public function __construct(TranslatorInterface $translator = null) + { + $this->translator = $translator; + } + + public function buildForm(FormBuilderInterface $builder, array $options) + { + if (!isset($options['invalid_message']) && !isset($options['invalid_message_parameters'])) { + $builder->addEventSubscriber(new TransformationFailureListener($this->translator)); + } + } + + public function getExtendedType() + { + return 'Symfony\Component\Form\Extension\Core\Type\FormType'; + } +} diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 181cc65af3..848025dff3 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -31,16 +31,18 @@ use Symfony\Component\PropertyAccess\PropertyPath; * * (1) the "model" format required by the form's object * (2) the "normalized" format for internal processing - * (3) the "view" format used for display + * (3) the "view" format used for display simple fields + * or map children model data for compound fields * * A date field, for example, may store a date as "Y-m-d" string (1) in the * object. To facilitate processing in the field, this value is normalized * to a DateTime object (2). In the HTML representation of your form, a - * localized string (3) is presented to and modified by the user. + * localized string (3) may be presented to and modified by the user, or it could be an array of values + * to be mapped to choices fields. * * In most cases, format (1) and format (2) will be the same. For example, * a checkbox field uses a Boolean value for both internal processing and - * storage in the object. In these cases you simply need to set a value + * storage in the object. In these cases you simply need to set a view * transformer to convert between formats (2) and (3). You can do this by * calling addViewTransformer(). * @@ -48,7 +50,7 @@ use Symfony\Component\PropertyAccess\PropertyPath; * demonstrate this, let's extend our above date field to store the value * either as "Y-m-d" string or as timestamp. Internally we still want to * use a DateTime object for processing. To convert the data from string/integer - * to DateTime you can set a normalization transformer by calling + * to DateTime you can set a model transformer by calling * addModelTransformer(). The normalized data is then converted to the displayed * data as described before. * @@ -217,7 +219,7 @@ class Form implements \IteratorAggregate, FormInterface, ClearableErrorsInterfac } if (null === $this->getName() || '' === $this->getName()) { - return; + return null; } $parent = $this->parent; @@ -340,8 +342,8 @@ class Form implements \IteratorAggregate, FormInterface, ClearableErrorsInterfac $modelData = $event->getData(); } - // Treat data as strings unless a value transformer exists - if (!$this->config->getViewTransformers() && !$this->config->getModelTransformers() && is_scalar($modelData)) { + // Treat data as strings unless a transformer exists + if (is_scalar($modelData) && !$this->config->getViewTransformers() && !$this->config->getModelTransformers()) { $modelData = (string) $modelData; } @@ -1035,7 +1037,7 @@ class Form implements \IteratorAggregate, FormInterface, ClearableErrorsInterfac } /** - * Normalizes the value if a normalization transformer is set. + * Normalizes the value if a model transformer is set. * * @param mixed $value The value to transform * @@ -1057,7 +1059,7 @@ class Form implements \IteratorAggregate, FormInterface, ClearableErrorsInterfac } /** - * Reverse transforms a value if a normalization transformer is set. + * Reverse transforms a value if a model transformer is set. * * @param string $value The value to reverse transform * @@ -1081,7 +1083,7 @@ class Form implements \IteratorAggregate, FormInterface, ClearableErrorsInterfac } /** - * Transforms the value if a value transformer is set. + * Transforms the value if a view transformer is set. * * @param mixed $value The value to transform * @@ -1112,7 +1114,7 @@ class Form implements \IteratorAggregate, FormInterface, ClearableErrorsInterfac } /** - * Reverse transforms a value if a value transformer is set. + * Reverse transforms a value if a view transformer is set. * * @param string $value The value to reverse transform * diff --git a/src/Symfony/Component/Form/NativeRequestHandler.php b/src/Symfony/Component/Form/NativeRequestHandler.php index ccebdab6d8..94210d51e8 100644 --- a/src/Symfony/Component/Form/NativeRequestHandler.php +++ b/src/Symfony/Component/Form/NativeRequestHandler.php @@ -15,7 +15,7 @@ use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\Form\Util\ServerParams; /** - * A request handler using PHP's super globals $_GET, $_POST and $_SERVER. + * A request handler using PHP super globals $_GET, $_POST and $_SERVER. * * @author Bernhard Schussek */ @@ -213,7 +213,7 @@ class NativeRequestHandler implements RequestHandlerInterface if (self::$fileKeys === $keys) { if (UPLOAD_ERR_NO_FILE === $data['error']) { - return; + return null; } return $data; diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/CoreExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/CoreExtensionTest.php new file mode 100644 index 0000000000..ff85149e21 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Extension/Core/CoreExtensionTest.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Extension\Core; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Form\Extension\Core\CoreExtension; +use Symfony\Component\Form\FormFactoryBuilder; + +class CoreExtensionTest extends TestCase +{ + public function testTransformationFailuresAreConvertedIntoFormErrors() + { + $formFactoryBuilder = new FormFactoryBuilder(); + $formFactory = $formFactoryBuilder->addExtension(new CoreExtension()) + ->getFormFactory(); + + $form = $formFactory->createBuilder() + ->add('foo', 'Symfony\Component\Form\Extension\Core\Type\DateType') + ->getForm(); + $form->submit('foo'); + + $this->assertFalse($form->isValid()); + } +} diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index bcc5933a9f..da206ba857 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -353,4 +353,39 @@ class PropertyPathMapperTest extends TestCase $this->mapper->mapFormsToData(array($form), $car); } + + /** + * @dataProvider provideDate + */ + public function testMapFormsToDataDoesNotChangeEqualDateTimeInstance($date) + { + $article = array(); + $publishedAt = $date; + $article['publishedAt'] = clone $publishedAt; + $propertyPath = $this->getPropertyPath('[publishedAt]'); + + $this->propertyAccessor->expects($this->once()) + ->method('getValue') + ->willReturn($article['publishedAt']) + ; + $this->propertyAccessor->expects($this->never()) + ->method('setValue') + ; + + $config = new FormConfigBuilder('publishedAt', \get_class($publishedAt), $this->dispatcher); + $config->setByReference(false); + $config->setPropertyPath($propertyPath); + $config->setData($publishedAt); + $form = $this->getForm($config); + + $this->mapper->mapFormsToData(array($form), $article); + } + + public function provideDate() + { + return array( + array(new \DateTime()), + array(new \DateTimeImmutable()), + ); + } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTimeTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTimeTypeTest.php index d7e48e0712..440fb822bd 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTimeTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTimeTypeTest.php @@ -644,4 +644,31 @@ class DateTimeTypeTest extends BaseTypeTest $this->assertSame($expectedData, $form->getNormData()); $this->assertSame($expectedData, $form->getData()); } + + /** + * @dataProvider provideEmptyData + */ + public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedData) + { + $form = $this->factory->create(static::TESTED_TYPE, null, array( + 'widget' => $widget, + 'empty_data' => $emptyData, + )); + $form->submit(null); + + $this->assertSame($emptyData, $form->getViewData()); + $this->assertEquals($expectedData, $form->getNormData()); + $this->assertEquals($expectedData, $form->getData()); + } + + public function provideEmptyData() + { + $expectedData = \DateTime::createFromFormat('Y-m-d H:i', '2018-11-11 21:23'); + + return array( + 'Simple field' => array('single_text', '2018-11-11T21:23:00', $expectedData), + 'Compound text field' => array('text', array('date' => array('year' => '2018', 'month' => '11', 'day' => '11'), 'time' => array('hour' => '21', 'minute' => '23')), $expectedData), + 'Compound choice field' => array('choice', array('date' => array('year' => '2018', 'month' => '11', 'day' => '11'), 'time' => array('hour' => '21', 'minute' => '23')), $expectedData), + ); + } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php index 94ae1a3a41..a19ab5bffc 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php @@ -1005,25 +1005,36 @@ class DateTypeTest extends BaseTypeTest )); $form->submit(null); - // view transformer write back empty strings in the view data + // view transformer writes back empty strings in the view data $this->assertSame(array('year' => '', 'month' => '', 'day' => ''), $form->getViewData()); $this->assertSame($expectedData, $form->getNormData()); $this->assertSame($expectedData, $form->getData()); } - public function testSingleTextSubmitNullUsesDefaultEmptyData() + /** + * @dataProvider provideEmptyData + */ + public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedData) { - $emptyData = '2018-11-11'; $form = $this->factory->create(static::TESTED_TYPE, null, array( - 'widget' => 'single_text', + 'widget' => $widget, 'empty_data' => $emptyData, )); $form->submit(null); - $date = new \DateTime($emptyData); - $this->assertSame($emptyData, $form->getViewData()); - $this->assertEquals($date, $form->getNormData()); - $this->assertEquals($date, $form->getData()); + $this->assertEquals($expectedData, $form->getNormData()); + $this->assertEquals($expectedData, $form->getData()); + } + + public function provideEmptyData() + { + $expectedData = \DateTime::createFromFormat('Y-m-d H:i:s', '2018-11-11 00:00:00'); + + return array( + 'Simple field' => array('single_text', '2018-11-11', $expectedData), + 'Compound text fields' => array('text', array('year' => '2018', 'month' => '11', 'day' => '11'), $expectedData), + 'Compound choice fields' => array('choice', array('year' => '2018', 'month' => '11', 'day' => '11'), $expectedData), + ); } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/TimeTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/TimeTypeTest.php index 03bcc3632a..bedc93212d 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/TimeTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/TimeTypeTest.php @@ -798,9 +798,36 @@ class TimeTypeTest extends BaseTypeTest )); $form->submit(null); - // view transformer write back empty strings in the view data + // view transformer writes back empty strings in the view data $this->assertSame(array('hour' => '', 'minute' => ''), $form->getViewData()); $this->assertSame($expectedData, $form->getNormData()); $this->assertSame($expectedData, $form->getData()); } + + /** + * @dataProvider provideEmptyData + */ + public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedData) + { + $form = $this->factory->create(static::TESTED_TYPE, null, array( + 'widget' => $widget, + 'empty_data' => $emptyData, + )); + $form->submit(null); + + $this->assertSame($emptyData, $form->getViewData()); + $this->assertEquals($expectedData, $form->getNormData()); + $this->assertEquals($expectedData, $form->getData()); + } + + public function provideEmptyData() + { + $expectedData = \DateTime::createFromFormat('Y-m-d H:i', '1970-01-01 21:23'); + + return array( + 'Simple field' => array('single_text', '21:23', $expectedData), + 'Compound text field' => array('text', array('hour' => '21', 'minute' => '23'), $expectedData), + 'Compound choice field' => array('choice', array('hour' => '21', 'minute' => '23'), $expectedData), + ); + } } diff --git a/src/Symfony/Component/Form/Util/FormUtil.php b/src/Symfony/Component/Form/Util/FormUtil.php index 0862179f54..53053f9d5b 100644 --- a/src/Symfony/Component/Form/Util/FormUtil.php +++ b/src/Symfony/Component/Form/Util/FormUtil.php @@ -27,7 +27,7 @@ class FormUtil * Returns whether the given data is empty. * * This logic is reused multiple times throughout the processing of - * a form and needs to be consistent. PHP's keyword `empty` cannot + * a form and needs to be consistent. PHP keyword `empty` cannot * be used as it also considers 0 and "0" to be empty. * * @param mixed $data diff --git a/src/Symfony/Component/Form/Util/OrderedHashMap.php b/src/Symfony/Component/Form/Util/OrderedHashMap.php index 6a97559850..26e45a4622 100644 --- a/src/Symfony/Component/Form/Util/OrderedHashMap.php +++ b/src/Symfony/Component/Form/Util/OrderedHashMap.php @@ -128,7 +128,7 @@ class OrderedHashMap implements \ArrayAccess, \IteratorAggregate, \Countable $key = array() === $this->orderedKeys // If the array is empty, use 0 as key ? 0 - // Imitate PHP's behavior of generating a key that equals + // Imitate PHP behavior of generating a key that equals // the highest existing integer key + 1 : 1 + (int) max($this->orderedKeys); } diff --git a/src/Symfony/Component/Form/Util/OrderedHashMapIterator.php b/src/Symfony/Component/Form/Util/OrderedHashMapIterator.php index 3de636392d..93a7caa58d 100644 --- a/src/Symfony/Component/Form/Util/OrderedHashMapIterator.php +++ b/src/Symfony/Component/Form/Util/OrderedHashMapIterator.php @@ -56,8 +56,6 @@ class OrderedHashMapIterator implements \Iterator private $current; /** - * Creates a new iterator. - * * @param array $elements The elements of the map, indexed by their * keys * @param array $orderedKeys The keys of the map in the order in which @@ -84,7 +82,7 @@ class OrderedHashMapIterator implements \Iterator */ public function __destruct() { - // Use array_splice() instead of isset() to prevent holes in the + // Use array_splice() instead of unset() to prevent holes in the // array indices, which would break the initialization of $cursorId array_splice($this->managedCursors, $this->cursorId, 1); } diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md index 8ba132f6c4..b96b4aefa1 100644 --- a/src/Symfony/Component/HttpKernel/CHANGELOG.md +++ b/src/Symfony/Component/HttpKernel/CHANGELOG.md @@ -15,7 +15,7 @@ CHANGELOG * added orphaned events support to `EventDataCollector` * `ExceptionListener` now logs exceptions at priority `0` (previously logged at `-128`) - * Deprecated `service:action` syntax with a single colon to reference controllers. Use `service::method` instead. + * Added support for using `service::method` to reference controllers, making it consistent with other cases. It is recommended over the `service:action` syntax with a single colon, which will be deprecated in the future. * Added the ability to profile individual argument value resolvers via the `Symfony\Component\HttpKernel\Controller\ArgumentResolver\TraceableValueResolver` diff --git a/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php index ed515d247c..4f80921cf5 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php @@ -36,7 +36,7 @@ class ContainerControllerResolver extends ControllerResolver { if (1 === substr_count($controller, ':')) { $controller = str_replace(':', '::', $controller); - @trigger_error(sprintf('Referencing controllers with a single colon is deprecated since Symfony 4.1. Use %s instead.', $controller), E_USER_DEPRECATED); + // TODO deprecate this in 5.1 } return parent::createController($controller); diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php index 57414d0012..1a144eee49 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php @@ -19,10 +19,6 @@ use Symfony\Component\HttpKernel\Controller\ContainerControllerResolver; class ContainerControllerResolverTest extends ControllerResolverTest { - /** - * @group legacy - * @expectedDeprecation Referencing controllers with a single colon is deprecated since Symfony 4.1. Use foo::action instead. - */ public function testGetControllerServiceWithSingleColon() { $service = new ControllerTestService('foo'); diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 3aa4a32025..0e17fdf7aa 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -607,16 +607,6 @@ class PropertyAccessor implements PropertyAccessorInterface $camelized = $this->camelize($property); $singulars = (array) Inflector::singularize($camelized); - if (\is_array($value) || $value instanceof \Traversable) { - $methods = $this->findAdderAndRemover($reflClass, $singulars); - - if (null !== $methods) { - $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER; - $access[self::ACCESS_ADDER] = $methods[0]; - $access[self::ACCESS_REMOVER] = $methods[1]; - } - } - if (!isset($access[self::ACCESS_TYPE])) { $setter = 'set'.$camelized; $getsetter = lcfirst($camelized); // jQuery style, e.g. read: last(), write: last($item) @@ -638,16 +628,22 @@ class PropertyAccessor implements PropertyAccessorInterface $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC; $access[self::ACCESS_NAME] = $setter; } elseif (null !== $methods = $this->findAdderAndRemover($reflClass, $singulars)) { - $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND; - $access[self::ACCESS_NAME] = sprintf( - 'The property "%s" in class "%s" can be defined with the methods "%s()" but '. - 'the new value must be an array or an instance of \Traversable, '. - '"%s" given.', - $property, - $reflClass->name, - implode('()", "', $methods), - \is_object($value) ? \get_class($value) : \gettype($value) - ); + if (\is_array($value) || $value instanceof \Traversable) { + $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER; + $access[self::ACCESS_ADDER] = $methods[0]; + $access[self::ACCESS_REMOVER] = $methods[1]; + } else { + $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND; + $access[self::ACCESS_NAME] = sprintf( + 'The property "%s" in class "%s" can be defined with the methods "%s()" but '. + 'the new value must be an array or an instance of \Traversable, '. + '"%s" given.', + $property, + $reflClass->name, + implode('()", "', $methods), + \is_object($value) ? \get_class($value) : \gettype($value) + ); + } } else { $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND; $access[self::ACCESS_NAME] = sprintf( diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestSingularAndPluralProps.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestSingularAndPluralProps.php new file mode 100644 index 0000000000..db17f3f612 --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestSingularAndPluralProps.php @@ -0,0 +1,65 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\PropertyAccess\Tests\Fixtures; + +/** + * Notice we don't have getter/setter for emails + * because we count on adder/remover. + */ +class TestSingularAndPluralProps +{ + /** @var string|null */ + private $email; + + /** @var array */ + private $emails = array(); + + /** + * @return string|null + */ + public function getEmail() + { + return $this->email; + } + + /** + * @param string|null $email + */ + public function setEmail($email) + { + $this->email = $email; + } + + /** + * @return array + */ + public function getEmails() + { + return $this->emails; + } + + /** + * @param string $email + */ + public function addEmail($email) + { + $this->emails[] = $email; + } + + /** + * @param string $email + */ + public function removeEmail($email) + { + $this->emails = array_diff($this->emails, array($email)); + } +} diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index f718808c12..3f10976816 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -22,6 +22,7 @@ use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicCall; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicGet; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassSetValue; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassTypeErrorInsideCall; +use Symfony\Component\PropertyAccess\Tests\Fixtures\TestSingularAndPluralProps; use Symfony\Component\PropertyAccess\Tests\Fixtures\Ticket5775Object; use Symfony\Component\PropertyAccess\Tests\Fixtures\TypeHinted; @@ -675,4 +676,26 @@ class PropertyAccessorTest extends TestCase $this->propertyAccessor->setValue($object, 'name', 'foo'); } + + public function testWriteToSingularPropertyWhilePluralOneExists() + { + $object = new TestSingularAndPluralProps(); + + $this->propertyAccessor->isWritable($object, 'email'); //cache access info + $this->propertyAccessor->setValue($object, 'email', 'test@email.com'); + + self::assertEquals('test@email.com', $object->getEmail()); + self::assertEmpty($object->getEmails()); + } + + public function testWriteToPluralPropertyWhileSingularOneExists() + { + $object = new TestSingularAndPluralProps(); + + $this->propertyAccessor->isWritable($object, 'emails'); //cache access info + $this->propertyAccessor->setValue($object, 'emails', array('test@email.com')); + + self::assertEquals(array('test@email.com'), $object->getEmails()); + self::assertNull($object->getEmail()); + } } diff --git a/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php b/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php index 12dd3f28fa..6f08a0f395 100644 --- a/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php +++ b/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php @@ -113,7 +113,7 @@ EOF; ?? $this->context->getParameter('_locale') ?: $this->defaultLocale; - if (null !== $locale) { + if (null !== $locale && null !== $name) { do { if ((self::$declaredRoutes[$name.'.'.$locale][1]['_canonical_route'] ?? null) === $name) { unset($parameters['_locale']); diff --git a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php new file mode 100644 index 0000000000..09ab15086b --- /dev/null +++ b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php @@ -0,0 +1,40 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\EventListener; + +use Symfony\Component\Workflow\Transition; + +class GuardExpression +{ + private $transition; + + private $expression; + + /** + * @param string $expression + */ + public function __construct(Transition $transition, $expression) + { + $this->transition = $transition; + $this->expression = $expression; + } + + public function getTransition() + { + return $this->transition; + } + + public function getExpression() + { + return $this->expression; + } +} diff --git a/src/Symfony/Component/Workflow/EventListener/GuardListener.php b/src/Symfony/Component/Workflow/EventListener/GuardListener.php index 912dc5dada..4f1c229e51 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardListener.php +++ b/src/Symfony/Component/Workflow/EventListener/GuardListener.php @@ -50,8 +50,21 @@ class GuardListener return; } - $expression = $this->configuration[$eventName]; + $eventConfiguration = (array) $this->configuration[$eventName]; + foreach ($eventConfiguration as $guard) { + if ($guard instanceof GuardExpression) { + if ($guard->getTransition() !== $event->getTransition()) { + continue; + } + $this->validateGuardExpression($event, $guard->getExpression()); + } else { + $this->validateGuardExpression($event, $guard); + } + } + } + private function validateGuardExpression(GuardEvent $event, string $expression) + { if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) { $blocker = TransitionBlocker::createBlockedByExpressionGuardListener($expression); $event->addTransitionBlocker($blocker); diff --git a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php index f0cc707220..c898828138 100644 --- a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php +++ b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php @@ -11,6 +11,7 @@ use Symfony\Component\Security\Core\Role\Role; use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\EventListener\ExpressionLanguage; +use Symfony\Component\Workflow\EventListener\GuardExpression; use Symfony\Component\Workflow\EventListener\GuardListener; use Symfony\Component\Workflow\Marking; use Symfony\Component\Workflow\Transition; @@ -21,12 +22,17 @@ class GuardListenerTest extends TestCase private $authenticationChecker; private $validator; private $listener; + private $configuration; protected function setUp() { - $configuration = array( + $this->configuration = array( 'test_is_granted' => 'is_granted("something")', 'test_is_valid' => 'is_valid(subject)', + 'test_expression' => array( + new GuardExpression(new Transition('name', 'from', 'to'), '!is_valid(subject)'), + new GuardExpression(new Transition('name', 'from', 'to'), 'is_valid(subject)'), + ), ); $expressionLanguage = new ExpressionLanguage(); $token = $this->getMockBuilder(TokenInterface::class)->getMock(); @@ -36,7 +42,7 @@ class GuardListenerTest extends TestCase $this->authenticationChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); $trustResolver = $this->getMockBuilder(AuthenticationTrustResolverInterface::class)->getMock(); $this->validator = $this->getMockBuilder(ValidatorInterface::class)->getMock(); - $this->listener = new GuardListener($configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator); + $this->listener = new GuardListener($this->configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator); } protected function tearDown() @@ -97,11 +103,38 @@ class GuardListenerTest extends TestCase $this->assertFalse($event->isBlocked()); } - private function createEvent() + public function testWithGuardExpressionWithNotSupportedTransition() + { + $event = $this->createEvent(); + $this->configureValidator(false); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testWithGuardExpressionWithSupportedTransition() + { + $event = $this->createEvent($this->configuration['test_expression'][1]->getTransition()); + $this->configureValidator(true, true); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testGuardExpressionBlocks() + { + $event = $this->createEvent($this->configuration['test_expression'][1]->getTransition()); + $this->configureValidator(true, false); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertTrue($event->isBlocked()); + } + + private function createEvent(Transition $transition = null) { $subject = new \stdClass(); $subject->marking = new Marking(); - $transition = new Transition('name', 'from', 'to'); + $transition = $transition ?: new Transition('name', 'from', 'to'); $workflow = $this->getMockBuilder(WorkflowInterface::class)->getMock(); diff --git a/src/Symfony/Component/Workflow/Tests/StateMachineTest.php b/src/Symfony/Component/Workflow/Tests/StateMachineTest.php index 5ece3c36d0..7f76c4a70b 100644 --- a/src/Symfony/Component/Workflow/Tests/StateMachineTest.php +++ b/src/Symfony/Component/Workflow/Tests/StateMachineTest.php @@ -3,7 +3,10 @@ namespace Symfony\Component\Workflow\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\StateMachine; +use Symfony\Component\Workflow\TransitionBlocker; class StateMachineTest extends TestCase { @@ -38,4 +41,70 @@ class StateMachineTest extends TestCase $this->assertTrue($net->can($subject, 't2')); $this->assertTrue($net->can($subject, 't3')); } + + public function testBuildTransitionBlockerList() + { + $definition = $this->createComplexStateMachineDefinition(); + + $net = new StateMachine($definition); + $subject = new \stdClass(); + + $subject->marking = 'a'; + $this->assertTrue($net->buildTransitionBlockerList($subject, 't1')->isEmpty()); + $subject->marking = 'd'; + $this->assertTrue($net->buildTransitionBlockerList($subject, 't1')->isEmpty()); + + $subject->marking = 'b'; + $this->assertFalse($net->buildTransitionBlockerList($subject, 't1')->isEmpty()); + } + + public function testBuildTransitionBlockerListWithMultipleTransitions() + { + $definition = $this->createComplexStateMachineDefinition(); + + $net = new StateMachine($definition); + $subject = new \stdClass(); + + $subject->marking = 'b'; + $this->assertTrue($net->buildTransitionBlockerList($subject, 't2')->isEmpty()); + $this->assertTrue($net->buildTransitionBlockerList($subject, 't3')->isEmpty()); + } + + public function testBuildTransitionBlockerListReturnsExpectedReasonOnBranchMerge() + { + $definition = $this->createComplexStateMachineDefinition(); + + $dispatcher = new EventDispatcher(); + $net = new StateMachine($definition, null, $dispatcher); + + $dispatcher->addListener('workflow.guard', function (GuardEvent $event) { + $event->addTransitionBlocker(new TransitionBlocker(\sprintf('Transition blocker of place %s', $event->getTransition()->getFroms()[0]), 'blocker')); + }); + + $subject = new \stdClass(); + + // There may be multiple transitions with the same name. Make sure that transitions + // that are not enabled by the marking are evaluated. + // see https://github.com/symfony/symfony/issues/28432 + + // Test if when you are in place "a"trying transition "t1" then returned + // blocker list contains guard blocker instead blockedByMarking + $subject->marking = 'a'; + $transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1'); + $this->assertCount(1, $transitionBlockerList); + $blockers = iterator_to_array($transitionBlockerList); + + $this->assertSame('Transition blocker of place a', $blockers[0]->getMessage()); + $this->assertSame('blocker', $blockers[0]->getCode()); + + // Test if when you are in place "d" trying transition "t1" then + // returned blocker list contains guard blocker instead blockedByMarking + $subject->marking = 'd'; + $transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1'); + $this->assertCount(1, $transitionBlockerList); + $blockers = iterator_to_array($transitionBlockerList); + + $this->assertSame('Transition blocker of place d', $blockers[0]->getMessage()); + $this->assertSame('blocker', $blockers[0]->getCode()); + } } diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php index 87f4800248..75ceb1f108 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php +++ b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php @@ -195,6 +195,32 @@ class WorkflowTest extends TestCase $workflow->buildTransitionBlockerList($subject, '404 Not Found'); } + public function testBuildTransitionBlockerList() + { + $definition = $this->createComplexWorkflowDefinition(); + $subject = new \stdClass(); + $subject->marking = null; + $workflow = new Workflow($definition, new MultipleStateMarkingStore()); + + $this->assertTrue($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty()); + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty()); + + $subject->marking = array('b' => 1); + + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty()); + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty()); + + $subject->marking = array('b' => 1, 'c' => 1); + + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty()); + $this->assertTrue($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty()); + + $subject->marking = array('f' => 1); + + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't5')->isEmpty()); + $this->assertTrue($workflow->buildTransitionBlockerList($subject, 't6')->isEmpty()); + } + public function testBuildTransitionBlockerListReturnsReasonsProvidedByMarking() { $definition = $this->createComplexWorkflowDefinition(); diff --git a/src/Symfony/Component/Workflow/TransitionBlockerList.php b/src/Symfony/Component/Workflow/TransitionBlockerList.php index 34f9437cda..b7ef5a157c 100644 --- a/src/Symfony/Component/Workflow/TransitionBlockerList.php +++ b/src/Symfony/Component/Workflow/TransitionBlockerList.php @@ -37,6 +37,17 @@ final class TransitionBlockerList implements \IteratorAggregate, \Countable $this->blockers[] = $blocker; } + public function has(string $code): bool + { + foreach ($this->blockers as $blocker) { + if ($code === $blocker->getCode()) { + return true; + } + } + + return false; + } + public function clear(): void { $this->blockers = array(); diff --git a/src/Symfony/Component/Workflow/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index 21676e0fc8..11d82daa01 100644 --- a/src/Symfony/Component/Workflow/Workflow.php +++ b/src/Symfony/Component/Workflow/Workflow.php @@ -119,7 +119,15 @@ class Workflow implements WorkflowInterface $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); if ($transitionBlockerList->isEmpty()) { - continue; + return $transitionBlockerList; + } + + // We prefer to return transitions blocker by something else than + // marking. Because it means the marking was OK. Transitions are + // deterministic: it's not possible to have many transitions enabled + // at the same time that match the same marking with the same name + if (!$transitionBlockerList->has(TransitionBlocker::BLOCKED_BY_MARKING)) { + return $transitionBlockerList; } } diff --git a/src/Symfony/Component/Workflow/composer.json b/src/Symfony/Component/Workflow/composer.json index 17500cbb25..645c550c67 100644 --- a/src/Symfony/Component/Workflow/composer.json +++ b/src/Symfony/Component/Workflow/composer.json @@ -3,7 +3,7 @@ "type": "library", "description": "Symfony Workflow Component", "keywords": ["workflow", "petrinet", "place", "transition", "statemachine", "state"], - "homepage": "http://symfony.com", + "homepage": "https://symfony.com", "license": "MIT", "authors": [ { @@ -16,7 +16,7 @@ }, { "name": "Symfony Community", - "homepage": "http://symfony.com/contributors" + "homepage": "https://symfony.com/contributors" } ], "require": {