Replace increment type with a storage strategy #1352
Conversation
| @@ -64,7 +64,7 @@ | |||
| <xs:attribute name="id" type="xs:boolean" default="false" /> | |||
| <xs:attribute name="name" type="xs:NMTOKEN" /> | |||
| <xs:attribute name="type" type="xs:NMTOKEN" default="string" /> | |||
| <xs:attribute name="strategy" type="xs:NMTOKEN" default="pushAll" /> | |||
| <xs:attribute name="strategy" type="xs:NMTOKEN" default="set" /> | |||
There was a problem hiding this comment.
Ah ok, got it after looking further :) Won't it break collections though? (note: I'm n00b when it comes to XMLs and theiir stuff)
There was a problem hiding this comment.
This type only applies to scalar fields, the association types have a different mapping. Technically, this field was not used to begin with ;)
|
Like the approach and where it can go :) I'll elaborate on later today |
| * @param string $strategy | ||
| * @return MappingException | ||
| */ | ||
| public static function invalidStorageStrategy($className, $fieldName, $strategy) |
There was a problem hiding this comment.
I'd pass type here as well, optionally we could also throw in allowed strategies and propose correct one basing on Levenshtein distance? But it can be too much :D
👍 on this, end of |
alcaeus
force-pushed
the
deprecate-increment-type
branch
from
2d739bb to
9256261
Compare
|
|
||
| The ``increment`` strategy does not apply to collections but can be used for | ||
| ``int`` and ``float`` fields. When using the ``increment`` strategy, the field | ||
| value will be updated using an `$inc`_ operator. |
There was a problem hiding this comment.
I think we can write "the $inc operator" here.
alcaeus
force-pushed
the
deprecate-increment-type
branch
from
a0c8d7c to
37ca9a4
Compare
| Doctrine MongoDB ODM implements four different strategies for persisting changes | ||
| to collections of embedded documents or references. These strategies apply to | ||
| the following mapping types: | ||
| Doctrine MongoDB ODM implements seven different strategies for persisting changes |
There was a problem hiding this comment.
How about changing seven to several? I noticed "four" only lately and haven't got to changing it to "six", I suppose it'll be the same with "seven" when we add eighth one :D
This removes a bunch of magic strings that were used throughout.
This also forces all EmbedOne and ReferenceOne mappings to use the set strategy. All other strategies will throw an error.
This changes the collection strategy documentation to be more generic and describe all storage strategies, regardless of whether they apply to collections or not.
alcaeus
force-pushed
the
deprecate-increment-type
branch
from
37ca9a4 to
b24d6af
Compare
|
Merged manually in b1cbaf8 |
This PR deprecates the
incrementtype that can be used in ODM, replacing it instead with a more flexiblestrategyoption for fields.First of all, the
incrementtype was supposed to handle int and float values at the same time. There was no option to ensure a value read from the database was cast to an int (which could lead to unexpected floats appearing in the application if one wasn't careful).Furthermore, for collections there is a
strategyoption which controls how the elements in a collection are written to the database. This kind of write control is exactly what should be used for increment fields as well.At first glance, there are no BC breaks to this. When the
ClassMetadataInfoclass encounters a mapping with typeincrement, it will automatically set the strategy toincrement, regardless of any other strategy being set. The type is left unchanged to keep the current type cast behavior. Users are encouraged to switch their increment mappings to the new strategy field.Open tasks:
incrementtype@malarzm: I'd appreciate your guidance on the last two tasks since you've spent quite some time with collections ;)