Deprecate string default expressions #12279
Conversation
|
TODO:
|
greg0ire
force-pushed
the
deprecate-conversion
branch
3 times, most recently
from
07875f4 to
73a95a8
Compare
greg0ire
force-pushed
the
deprecate-conversion
branch
from
17a68cb to
5630107
Compare
greg0ire
force-pushed
the
deprecate-conversion
branch
10 times, most recently
from
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.
greg0ire
force-pushed
the
deprecate-conversion
branch
from
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.
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.
greg0ire
force-pushed
the
deprecate-conversion
branch
from
9ccca55 to
f312fcb
Compare
2 participants
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.