This PR was merged into the 2.3 branch.
Discussion
----------
[Form] Fixed ValidatorTypeGuesser to guess properties without constraints not to be required
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #6645
| License | MIT
| Doc PR | -
Consider the following entity:
```php
class Author
{
/**
* @Assert\NotBlank
*/
private $name;
private $age;
}
```
Right now, the "required" HTML attribute is set for both fields (since the default value of the "required" option is true). IMO this is wrong.
With this fix, the ValidatorTypeGuesser guesses `false` for the "required" option unless a NotNull/NotBlank constraint is present.
Commits
-------
fd77b09 [Form] Fixed ValidatorTypeGuesser to guess properties without constraints not to be required
This PR was merged into the 2.3 branch.
Discussion
----------
[Form][Validator] All index items after children are to be considered grand-children when resolving ViolationPath
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | unsure, see note below
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #11458
| License | MIT
| Doc PR | -
#### Possible BC Break
The old behavior had unit test cases specifically testing the case of a grand-children form. However, this behavior is not documented anywhere and the fix seems to have no adverse effects on form validation. `Symfony\Component\Form\FormInterface` implements `ArrayAccess`, therefore, semantically speaking, `children[direct_child].children[grand_children]` and `children[direct_child][grand_children]` are equivalent. `offsetGet` is expected to fetch an element from `children`. I do not see why both were not considered equivalent when resolving the ViolationPath.
This commit will indeed change how some errors are mapped. However since the old mapping is (in my opinion) a bug...
Commits
-------
c64a75f [Form][Validator] All index items after children are to be considered grand-children when resolving ViolationPath (fixes#11458)
* 2.2:
[Form] add support for Length and Range constraint in order to replace MaxLength, MinLength, Max and Min constraints in next release (2.3)
[Form] check the required output timezone against the actual timezone of the input datetime object, rather than the expected timezone supplied
* 2.2:
added additional tests to cover invalid argument exceptions in OutputFormatterStyle component
added a missing check for the provider key
[Validator] fixed wrong URL for XSD
[Validator] Fixed: $traverse and $deep is passed to the visitor from Validator::validate()
[Form] Fixed transform()/reverseTransform() to always throw TransformationFailedExceptions
[Form] Fixed: String validation groups are never interpreted as callbacks
if the repository method returns an array ensure that it's internal poin...
[Form] Improved multi-byte handling of NumberToLocalizedStringTransformer
Fix wrong method in findTaggedServiceIds(), add example to docblock.
Conflicts:
src/Symfony/Component/Form/Extension/Core/DataTransformer/ChoicesToBooleanArrayTransformer.php
src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php
* 2.1:
added additional tests to cover invalid argument exceptions in OutputFormatterStyle component
added a missing check for the provider key
[Validator] fixed wrong URL for XSD
[Form] Fixed transform()/reverseTransform() to always throw TransformationFailedExceptions
[Form] Fixed: String validation groups are never interpreted as callbacks
if the repository method returns an array ensure that it's internal poin...
Fix wrong method in findTaggedServiceIds(), add example to docblock.
Conflicts:
src/Symfony/Bridge/Doctrine/Form/DataTransformer/CollectionToArrayTransformer.php
src/Symfony/Component/Form/Extension/Core/DataTransformer/DataTransformerChain.php
src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/ArrayToPartsTransformerTest.php
src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/ChoiceToValueTransformerTest.php
src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/ChoicesToValuesTransformerTest.php
src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/DateTimeToArrayTransformerTest.php
src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/DateTimeToRfc3339TransformerTest.php
src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/IntegerToLocalizedStringTransformerTest.php
src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/ValueToDuplicatesTransformerTest.php
This PR was merged into the master branch.
Discussion
----------
[2.3] [Form] Renamed option "virtual" to "inherit_data" and improved handling of such forms
Bug fix: yes
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #5899, #5720, #5578
Todo: -
License of the code: MIT
Documentation PR: symfony/symfony-docs#2107
This PR renames the option "virtual" to "inherit_data" for more clarity (the old option is deprecated and usable until 2.3). It also fixes the behavior of forms having that option set.
Forms with that option set will now correctly return their parents' data from `getData()`, `getNormData()` and `getViewData()`. Furthermore, `getPropertyPath()` was fixed for forms that inherit their parent data.
Commits
-------
1290b80 [Form] Fixed the deprecation notes for the "virtual" option
ac2ca44 [Form] Moved parent data inheritance from data mappers to Form
8ea5e1a [Form] Renamed option "virtual" to "inherit_data"
This PR was merged into the master branch.
Commits
-------
6b1652e [PropertyAccess] Property path, small refactoring, read/writeProperty to read/write Property/Index.
1bae7b2 [PropertyAccess] Extracted PropertyAccess component out of Form
Discussion
----------
[PropertyAccess] Extracted PropertyAccess component out of Form
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
TODO: adapt DoctrineBundle/PropelBundle to pass the "property_accessor" service to EntityType/ModelType
Usage:
```php
$accessor = PropertyAccess::getPropertyAccessor();
// equivalent to $object->getFoo()->setBar('value')
$accessor->setValue($object, 'foo.bar', 'value');
// equivalent to $object->getFoo()->getBar()
$accessor->getValue($object, 'foo.bar');
// equivalent to $object->getFoo()['bar']
$accessor->getValue($object, 'foo[bar]');
// equivalent to $array['foo']->setBar('value')
$accessor->setValue($array, '[foo].bar', 'value');
// equivalent to $array['foo']['bar']
$accessor->getValue($array, '[foo][bar]');
```
Later on, once we have generation and caching of class-specific accessors, configuration will be something like this (consistent with the Form and Validator component):
```php
$accessor = PropertyAccess::getPropertyAccessorBuilder()
->setCacheDirectory(__DIR__ . '/cache')
->setCacheLifeTime(86400)
->enableMagicGetSet()
->enableMagicCall()
->getPropertyAccessor();
```
or
```php
$accessor = PropertyAccess::getPropertyAccessorBuilder()
->setCache($cache)
->getPropertyAccessor();
```
etc.
---------------------------------------------------------------------------
by Burgov at 2013-01-07T08:48:15Z
+1. I use this feature outside of the Form context a lot
---------------------------------------------------------------------------
by stof at 2013-01-07T08:49:34Z
The classes in the Form component should be kept for BC (and deprecated) for people using the feature
---------------------------------------------------------------------------
by michelsalib at 2013-01-07T10:02:19Z
YES YES YES 👍. Sorry for my enthusiasm, but I already copy pasted the PropertyPath class to some of my libraries to avoid linking to the whole Form component. I thus will be glad to officially use this component into my libraries via composer.
---------------------------------------------------------------------------
by norzechowicz at 2013-01-07T10:17:39Z
Same as @michelsalib to avoid linking full Form component I was using copied parts of code. Can't wait to use this component in my lib. 👍
---------------------------------------------------------------------------
by bschussek at 2013-01-07T10:43:41Z
I split away `getValue()` and `setValue()` from `PropertyPath` into a new class `ReflectionGraph`. The component is also named ReflectionGraph now.
---------------------------------------------------------------------------
by michelsalib at 2013-01-07T10:47:10Z
I am not found of the name. What do you intend to do in the component more than what PropertyPath does ?
---------------------------------------------------------------------------
by bschussek at 2013-01-07T10:58:59Z
@michelsalib A `PropertyPath` is simply a string like `foo.bar[baz]`. `getValue()` and `setValue()` interpret this path. There may be different interpretations for the same path, so these methods were split into a new class.
I chose the name `ReflectionGraph` because the functionality is very similar to `ReflectionProperty`.
```php
$reflProperty = new ReflectionProperty('Vendor/Class', 'property');
$reflProperty->setValue($object, 'foo');
$reflGraph = new ReflectionGraph();
$reflGraph->setValue($object, 'property.path', 'foo');
```
---------------------------------------------------------------------------
by michelsalib at 2013-01-07T11:00:42Z
What about naming it `Reflection`, maybe sometime we will want to add more reflection tools for classes, interfaces... ?
---------------------------------------------------------------------------
by bschussek at 2013-01-07T11:02:32Z
@michelsalib I doubt that we will do that. PHP's implementation is sufficient.
---------------------------------------------------------------------------
by vicb at 2013-01-07T11:03:57Z
> Backwards compatibility break: no
Really ?
---------------------------------------------------------------------------
by michelsalib at 2013-01-07T11:05:07Z
Well, that is just a suggestion. If I am the only one to oppose, I won't complain.
---------------------------------------------------------------------------
by bschussek at 2013-01-07T11:09:08Z
> Really ?
@vicb Would you please refrain from such meanginless comments in the future? I'm getting a bit tired of them. If you think that BC is broken somewhere, tell me where so that I can fix it.
---------------------------------------------------------------------------
by stof at 2013-01-07T11:09:43Z
@vicb There is no BC break as he kept deprecated classes for BC
---------------------------------------------------------------------------
by norzechowicz at 2013-01-07T11:13:12Z
@bschussek what do you think about some kind of factory for Reflection? This will prevent creating new Reflection objects each time you want to access properties values.
---------------------------------------------------------------------------
by vicb at 2013-01-07T11:18:47Z
@bschussek my point is that my comment is no more meaningless than closing #6453 because it will break BC.We could also keep BC by extending the classes in the new ns but in both cases BC will ultimately be broken (when the legacy classes are removed)
---------------------------------------------------------------------------
by vicb at 2013-01-07T12:23:45Z
@bschussek @stof I think that modifying the constructor signatures of `EntityChoiceList`, `FormType` are BC breaks (this is not an exhaustive list)
---------------------------------------------------------------------------
by bschussek at 2013-01-07T12:35:13Z
@vicb You are right. I added corresponding entries to the CHANGELOG and adapted the above description.
---------------------------------------------------------------------------
by vicb at 2013-01-08T13:39:13Z
@bschussek looking at this PR, I was wondering if an alternate syntax would make sense:
```php
<?php
$reflGraph = new ReflectionGraph($object);
// equivalent to $object->getFoo()->setBar('value')
$reflGraph['foo.bar'] = 'value';
// equivalent to $object->getFoo()->getBar()
$reflGraph['foo.bar'];
```
_Sorry for the off topic_
---------------------------------------------------------------------------
by vicb at 2013-01-08T13:49:46Z
The advantage of using such a `ReflectionGraph` factory is that it might be easier to return specialized reflection graphs, ie optimized instances (that would be cached).
---------------------------------------------------------------------------
by Toflar at 2013-01-08T14:49:54Z
I was also puzzled by the fact that there will be many `ReflectionGraph` instances although they don't have to. I'm with @vicb and I'd also vote for using the constructor to set the subject you're working on. Otherwise you'll repeat yourself over and over again by passing the subject - say `$object` - to `getValue()` or `setValue()`. If however you don't like the constructor thing then why do we have to have an instance of `ReflectionGraph` rather than just go for static methods and use `ReflectionGraph::getValue()` and `ReflectionGraph::setValue()`?
In my opinion there are a few methods that could be static anyway (especially some private ones) :)
But probably I misunderstood something as I'm just about to discover the SF components and don't have any experience working with them (so basically I just read the PR because of @bschussek's tweet :D)
Couldn't come up with any intuitive name for the component though :(
Generally when we talk about "getting" and "setting" values we call those things "mutators"...so `GraphMutator` might be more intuitive than the word `Reflection` :)
---------------------------------------------------------------------------
by Taluu at 2013-01-08T14:57:42Z
I like the last proposition made by @vicb (implementing `ArrayAccess` on `ReflectionGraph` - or whatever name will be chosen (`PathMutator` for example :D), and also specify which object should be worked on in the constructor rather than in each method).
Would this also be used in the `Validator` component ?
---------------------------------------------------------------------------
by stof at 2013-01-08T15:16:12Z
@Toflar A static ``ReflectionGraph::getValue()`` means you have a coupling to the implementation (as with any static call). The current implementation allows you to replace it with your own implementation as long as you implement the interface as it follows the DI pattern (as done in other places in Symfony).
@vicb The issue with ``$reflGraph = new ReflectionGraph($object);`` is that you cannot inject the ReflectionGraph anymore, as you need a new one each time. This would mean adding a ``ReflectionGraphFactory`` to be injected (and which could then be replaced by a factory using code generation). Using the constructor directly would not allow using a replacement based on code generation later. So the resulting code would more likely be
```php
$reflGraph = new ReflectionGraph();
$mutator = $reflGraph->getMutator($object);
// equivalent to $object->getFoo()->setBar('value')
$mutator['foo.bar'] = 'value';
// equivalent to $object->getFoo()->getBar()
$mutator['foo.bar'];
```
Btw, writing this, I find the naming Mutator suggested by @Taluu good when it concerns the setter, but quite weird when getting the value.
---------------------------------------------------------------------------
by Taluu at 2013-01-08T15:21:00Z
I was not the one to suggest though, it was @everzet. But then something like `PathAccessor`, as it is both a mutator and a getter ? I also like @stof suggestion, still in the idea of avoiding to have to put the object as an argument and also allowing to use an `ArrayAccess` interface..
---------------------------------------------------------------------------
by vicb at 2013-01-08T15:21:54Z
@stof your remark makes sense.
What about `Accessor`, the benefit being that it might well be the name of a coming PHP feature: https://wiki.php.net/rfc/propertygetsetsyntax-v1.2
---------------------------------------------------------------------------
by everzet at 2013-01-08T15:27:02Z
```php
$manager = new PropertyManager(new PropertyPath());
$num = $manager->getValue($object, 'foo.num');
$manager->setValue($object, 'foo.num', $num + 1);
$objectManager = new ObjectPropertyManager($object[, $manager]);
$num = $objectManager->getValue('foo.num');
$objectManager->setValue('foo.num', $num + 1);
$objectManager['foo.num'] += 1;
```
---------------------------------------------------------------------------
by bschussek at 2013-01-08T15:28:01Z
It might be me, but I don't like `ArrayAccess` to be misused for features like that. If I access a key in an array access structure, I expect it to be something like a collection, an associative array or a key value store. This class is neither.
Putting that aside, an accessor for a specific object might make sense, but I'm not sure about that yet.
```php
$reflObject->setValue('foo.bar', 'value');
$reflObject->getValue('foo.bar');
```
---------------------------------------------------------------------------
by stof at 2013-01-08T15:28:52Z
@vicb I would vote for PathAccessor then, as we are not doing simple accessor but accessors through a path in an object graph.
In my snippet above, we would then have a ReflectionGraph instance and a PathAccessor instance (``$mutator``).
Btw, I would also keep the methods ``setValue`` and ``getValue``. I find it more clear.
---------------------------------------------------------------------------
by bschussek at 2013-01-08T15:32:07Z
@stof But then we're rather left with the question of ReflectionGraph **vs.** PathAccessor. I don't think that the tiny interface difference (one global, one object-based) justifies the big naming difference.
---------------------------------------------------------------------------
by vicb at 2013-01-08T15:33:24Z
> This class is neither.
It might be `$pa['foo.bar[baz]'] = $pa['foo.bar']['baz'];` I don't know if it would help though.
---------------------------------------------------------------------------
by stof at 2013-01-08T15:35:51Z
@bschussek In my suggestion, ``ReflectionGraph`` is a factory for the PathAccessor objects. It is not accessing anymore itself (which would probably continue to cause issues to implement it with code generation). But the naming could indeed be changed to something else.
- Removed useless error handlers around FormEvent as the triggering has
been fixed in it.
- Enhanced the triggering of deprecation errors for places where the BC
method provide some user logic needing to be converted to a new way.
- Enhanced the deprecation messages to mention the replacement whenever
possible.
* 2.1:
fixed CS
fixed CS
[Security] fixed path info encoding (closes#6040, closes#5695)
[HttpFoundation] added some tests for the previous merge and removed dead code (closes#6037)
Improved Cache-Control header when no-cache is sent
removed unneeded comment
Fix to allow null values in labels array
fix date in changelog
removed the Travis icon (as this is not stable enough -- many false positive, closes#6186)
Revert "merged branch gajdaw/finder_splfileinfo_fpassthu (PR #4751)" (closes#6224)
Fixed a typo
Fixed: HeaderBag::parseCacheControl() not parsing quoted zero correctly
[Form] Fix const inside an anonymous function
[Config] Loader::import must return imported data
[DoctrineBridge] Fixed caching in DoctrineType when "choices" or "preferred_choices" is passed
[Form] Fixed the default value of "format" in DateType to DateType::DEFAULT_FORMAT if "widget" is not "single_text"
[HttpFoundation] fixed a small regression
Conflicts:
src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php