Deprecate string default expressions #12279
Conversation
|
TODO:
|
07875f4 to
73a95a8
Compare
17a68cb to
5630107
Compare
3cb990e to
11bd618
Compare
src/Mapping/Driver/XmlDriver.php
Outdated
| if ($nameAttribute === 'default' && is_string($value) && is_a($value, DefaultExpression::class, true)) { | ||
| $array['default'] = new $value(); |
There was a problem hiding this comment.
I don't know how widely the XML driver is used and how maintainable it needs to be, but this is a bit hacky. If the default value of a string field matches an expression class, the driver will generate an expression, which will be invalid. Effectively, this is the same magic that the deprecation in the DBAL attempts to eliminate.
I don't have a problem if it's done the way it's done now. Properly accommodating a new type requires a change in the XML schema (for example, like PHPUnit used to do).
There was a problem hiding this comment.
You're right, it's not great… I'll have to think more about this. I'm thinking that it might not make sense to allow having children tags for all option tags. So maybe options should allow both option and default-expression as a child? But then it creates the possibility to have default-expression alongside option name="default" 😬
Maybe the best would be option name="default" class="Doctrine\DBAL\Schema\DefaultExpression\CurrentTimestamp", I'm really not sure about this.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
@morozov I've implemented support for <object>.
There was a problem hiding this comment.
Thank you. This looks much better.
11bd618 to
9ccca55
Compare
| <xs:complexType name="option" mixed="true"> | ||
| <xs:choice minOccurs="0" maxOccurs="unbounded"> | ||
| <xs:element name="option" type="orm:option"/> | ||
| <xs:element name="object" type="orm:object"/> |
There was a problem hiding this comment.
Not sure if this is the right place to put the new element. Currently, I can provide multiple <object> elements for a single <option>, and the document will still be valid.
<options>
<option name="default">
<object class="Doctrine\DBAL\Schema\DefaultExpression\CurrentDate"/>
<object class="Doctrine\DBAL\Schema\DefaultExpression\CurrentTime"/>
</option>
</options>There was a problem hiding this comment.
I don't know much about XSD, but my LLM does so I had it prevent this possibility. It's implemented as a choice of 1 element between <object>, and an unbounded sequence of what was previously under the existing xs:choice.
9ccca55 to
f312fcb
Compare
|
What would happen if I used |
|
Ah perfect, thank you. |
|
Can we add a test though that makes sure we're deprecation-free on DBAL 3? Apart from that, the change looks good. 👍🏻 |
Right now, the ORM handles the conversion of strings that happen to be default expressions for date, time and datetime columns into the corresponding value objects. Let us allow users to specify these value objects directly, and deprecate relying on the aforementioned conversion.
f312fcb to
4016d6b
Compare
Right now, the ORM handles the conversion of strings that happen to be default expressions for date, time and datetime columns into the corresponding value objects.
Let us allow users to specify these value objects directly, and deprecate relying on the aforementioned conversion.