Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DateImmutable type #2118

Merged
merged 2 commits into from
Nov 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/en/reference/annotations-reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ Alias of `@Index`_, with the ``unique`` option set by default.
--------

The annotated instance variable will be used to store version information for :ref:`optimistic locking <transactions_and_concurrency_optimistic_locking>`.
This is only compatible with ``int`` and ``date`` field types, and cannot be combined with `@Id`_.
This is only compatible with ``int``, ``date``, and ``date_immutable`` field types, and cannot be combined with `@Id`_.

.. code-block:: php

Expand Down
2 changes: 2 additions & 0 deletions docs/en/reference/basic-mapping.rst
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ Here is a quick overview of the built-in mapping types:
- ``collection``
- ``custom_id``
- ``date``
- ``date_immutable``
- ``file``
- ``float``
- ``hash``
Expand Down Expand Up @@ -180,6 +181,7 @@ This list explains some of the less obvious mapping types:
- ``bin_uuid``: string to MongoDB\BSON\Binary instance with a "uuid" type
- ``collection``: numerically indexed array to MongoDB array
- ``date``: DateTime to ``MongoDB\BSON\UTCDateTime``
- ``date_immutable``: DateTimeImmutable to ``MongoDB\BSON\UTCDateTime``
- ``hash``: associative array to MongoDB object
- ``id``: string to ObjectId by default, but other formats are possible
- ``timestamp``: string to ``MongoDB\BSON\Timestamp``
Expand Down
18 changes: 16 additions & 2 deletions docs/en/reference/transactions-and-concurrency.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Doctrine has integrated support for automatic optimistic locking
via a ``version`` field. Any document that should be
protected against concurrent modifications during long-running
business transactions gets a ``version`` field that is either a simple
number (mapping type: ``int``) or a date (mapping type: ``date``).
number (mapping type: ``int``) or a date (mapping type: ``date`` or ``date_immutable``).
When changes to the document are persisted,
the expected version and version increment are incorporated into the update criteria and modifiers, respectively.
If this results in no document being modified by the update (i.e. expected version did not match),
Expand Down Expand Up @@ -83,10 +83,24 @@ Alternatively, the ``date`` type may be used:

<field field-name="version" version="true" type="date" />

Or its immutable counterpart ``date_immutable``:

.. configuration-block::

.. code-block:: php

<?php
/** @Version @Field(type="date_immutable") */
private $version;

.. code-block:: xml

<field field-name="version" version="true" type="date_immutable" />

Choosing the Field Type
"""""""""""""""""""""""

When using the ``date`` type in a high-concurrency environment, it is still possible to create multiple documents
When using the date-based type in a high-concurrency environment, it is still possible to create multiple documents
with the same version and cause a conflict. This can be avoided by using the ``int`` type.

Usage
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ODM/MongoDB/LockException.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ public static function invalidLockFieldType(string $type) : self

public static function invalidVersionFieldType(string $type) : self
{
return new self('Invalid version field type ' . $type . '. Version field must be int or date.');
return new self('Invalid version field type ' . $type . '. Version field must be int, integer, date or date_immutable.');
}
}
2 changes: 1 addition & 1 deletion lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -1653,7 +1653,7 @@ public function isIdGeneratorNone() : bool
*/
public function setVersionMapping(array &$mapping) : void
{
if ($mapping['type'] !== 'int' && $mapping['type'] !== 'date') {
if (! in_array($mapping['type'], [Type::INT, Type::INTEGER, Type::DATE, Type::DATE_IMMUTABLE], true)) {
throw LockException::invalidVersionFieldType($mapping['type']);
}

Expand Down
19 changes: 10 additions & 9 deletions lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use BadMethodCallException;
use DateTime;
use DateTimeImmutable;
use Doctrine\Common\Persistence\Mapping\MappingException;
use Doctrine\ODM\MongoDB\DocumentManager;
use Doctrine\ODM\MongoDB\Hydrator\HydratorException;
Expand Down Expand Up @@ -211,11 +212,11 @@ public function executeInserts(array $options = []) : void
if ($this->class->isVersioned) {
$versionMapping = $this->class->fieldMappings[$this->class->versionField];
$nextVersion = null;
if ($versionMapping['type'] === 'int') {
if ($versionMapping['type'] === Type::INT || $versionMapping['type'] === Type::INTEGER) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we haven't used consts in the first place so I've used them everywhere and kept it as a separate commit

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should deprecate and remove the boolean and integer types to be consistent with PHP, which only allows bool and int. For now this works. 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #2124

$nextVersion = max(1, (int) $this->class->reflFields[$this->class->versionField]->getValue($document));
$this->class->reflFields[$this->class->versionField]->setValue($document, $nextVersion);
} elseif ($versionMapping['type'] === 'date') {
$nextVersionDateTime = new DateTime();
} elseif ($versionMapping['type'] === Type::DATE || $versionMapping['type'] === Type::DATE_IMMUTABLE) {
$nextVersionDateTime = $versionMapping['type'] === Type::DATE ? new DateTime() : new DateTimeImmutable();
$nextVersion = Type::convertPHPToDatabaseValue($nextVersionDateTime);
$this->class->reflFields[$this->class->versionField]->setValue($document, $nextVersionDateTime);
}
Expand Down Expand Up @@ -286,11 +287,11 @@ private function executeUpsert(object $document, array $options) : void
if ($this->class->isVersioned) {
$versionMapping = $this->class->fieldMappings[$this->class->versionField];
$nextVersion = null;
if ($versionMapping['type'] === 'int') {
if ($versionMapping['type'] === Type::INT || $versionMapping === Type::INTEGER) {
$nextVersion = max(1, (int) $this->class->reflFields[$this->class->versionField]->getValue($document));
$this->class->reflFields[$this->class->versionField]->setValue($document, $nextVersion);
} elseif ($versionMapping['type'] === 'date') {
$nextVersionDateTime = new DateTime();
} elseif ($versionMapping['type'] === Type::DATE || $versionMapping['type'] === Type::DATE_IMMUTABLE) {
$nextVersionDateTime = $versionMapping['type'] === Type::DATE ? new DateTime() : new DateTimeImmutable();
$nextVersion = Type::convertPHPToDatabaseValue($nextVersionDateTime);
$this->class->reflFields[$this->class->versionField]->setValue($document, $nextVersionDateTime);
}
Expand Down Expand Up @@ -371,12 +372,12 @@ public function update(object $document, array $options = []) : void
if ($this->class->isVersioned) {
$versionMapping = $this->class->fieldMappings[$this->class->versionField];
$currentVersion = $this->class->reflFields[$this->class->versionField]->getValue($document);
if ($versionMapping['type'] === 'int') {
if ($versionMapping['type'] === Type::INT || $versionMapping['type'] === Type::INTEGER) {
$nextVersion = $currentVersion + 1;
$update['$inc'][$versionMapping['name']] = 1;
$query[$versionMapping['name']] = $currentVersion;
} elseif ($versionMapping['type'] === 'date') {
$nextVersion = new DateTime();
} elseif ($versionMapping['type'] === Type::DATE || $versionMapping['type'] === Type::DATE_IMMUTABLE) {
$nextVersion = $versionMapping['type'] === Type::DATE ? new DateTime() : new DateTimeImmutable();
$update['$set'][$versionMapping['name']] = Type::convertPHPToDatabaseValue($nextVersion);
$query[$versionMapping['name']] = Type::convertPHPToDatabaseValue($currentVersion);
}
Expand Down
37 changes: 37 additions & 0 deletions lib/Doctrine/ODM/MongoDB/Types/DateImmutableType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Doctrine\ODM\MongoDB\Types;

use DateTime;
use DateTimeImmutable;
use DateTimeInterface;
use RuntimeException;
use function get_class;
use function sprintf;

class DateImmutableType extends DateType
{
/**
* @return DateTimeImmutable
*/
public static function getDateTime($value) : DateTimeInterface
alcaeus marked this conversation as resolved.
Show resolved Hide resolved
{
$datetime = parent::getDateTime($value);

if ($datetime instanceof DateTimeImmutable) {
return $datetime;
}

if ($datetime instanceof DateTime) {
return DateTimeImmutable::createFromMutable($datetime);
}

throw new RuntimeException(sprintf(
'%s::getDateTime has returned an unsupported implementation of DateTimeInterface: %s',
parent::class,
get_class($datetime)
));
}
}
2 changes: 1 addition & 1 deletion lib/Doctrine/ODM/MongoDB/Types/DateType.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class DateType extends Type
* Converts a value to a DateTime.
* Supports microseconds
*
* @param mixed $value \DateTimeInterface|\MongoDB\BSON\UTCDateTime|int|float
* @param mixed $value \DateTimeInterface|\MongoDB\BSON\UTCDateTime|int|float
*
* @throws InvalidArgumentException If $value is invalid.
*/
Expand Down
2 changes: 2 additions & 0 deletions lib/Doctrine/ODM/MongoDB/Types/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ abstract class Type
public const FLOAT = 'float';
public const STRING = 'string';
public const DATE = 'date';
public const DATE_IMMUTABLE = 'date_immutable';
public const KEY = 'key';
public const TIMESTAMP = 'timestamp';
public const BINDATA = 'bin';
Expand Down Expand Up @@ -62,6 +63,7 @@ abstract class Type
self::FLOAT => Types\FloatType::class,
self::STRING => Types\StringType::class,
self::DATE => Types\DateType::class,
self::DATE_IMMUTABLE => Types\DateImmutableType::class,
self::KEY => Types\KeyType::class,
self::TIMESTAMP => Types\TimestampType::class,
self::BINDATA => Types\BinDataType::class,
Expand Down
88 changes: 80 additions & 8 deletions tests/Doctrine/ODM/MongoDB/Tests/Functional/LockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ public function testMultipleFlushesDoIncrementalUpdates()
}
}

public function testLockTimestampSetsDefaultValue()
public function testLockDateSetsDefaultValue()
{
$test = new LockTimestamp();
$test = new LockDate();
$test->title = 'Testing';

$this->assertNull($test->version, 'Pre-Condition');
Expand All @@ -106,11 +106,33 @@ public function testLockTimestampSetsDefaultValue()
return $test;
}

public function testLockTimestampSetsDefaultValueOnUpsert()
public function testLockDateImmutableSetsDefaultValue()
{
$test = new LockDateImmutable();
$test->title = 'Testing';

$this->assertNull($test->version, 'Pre-Condition');

$this->dm->persist($test);
$this->dm->flush();

$date1 = $test->version;

$this->assertInstanceOf('DateTimeImmutable', $date1);

$test->title = 'changed';
$this->dm->flush();

$this->assertNotSame($date1, $test->version);

return $test;
}

public function testLockDateSetsDefaultValueOnUpsert()
{
$id = new ObjectId();

$test = new LockTimestamp();
$test = new LockDate();
$test->title = 'Testing';
$test->id = $id;

Expand All @@ -132,9 +154,52 @@ public function testLockTimestampSetsDefaultValueOnUpsert()
return $test;
}

public function testLockTimestampThrowsException()
public function testLockDateImmutableSetsDefaultValueOnUpsert()
{
$id = new ObjectId();

$test = new LockDateImmutable();
$test->title = 'Testing';
$test->id = $id;

$this->assertNull($test->version, 'Pre-Condition');

$this->dm->persist($test);
$this->dm->flush();

$date1 = $test->version;

$this->assertSame($id, $test->id);
$this->assertInstanceOf('DateTimeImmutable', $date1);

$test->title = 'changed';
$this->dm->flush();

$this->assertNotSame($date1, $test->version);

return $test;
}

public function testLockDateThrowsException()
{
$article = new LockTimestamp('Test LockInt');
$article = new LockDate('Test LockInt');
$this->dm->persist($article);
$this->dm->flush();

// Manually change the version so the next code will cause an exception
$this->dm->getDocumentCollection(get_class($article))->updateOne(['_id' => new ObjectId($article->id)], ['$set' => ['version' => new UTCDateTime(time() * 1000 + 600)]]);

// Now lets change a property and try and save it again
$article->title = 'ok';

$this->expectException(LockException::class);

$this->dm->flush();
}

public function testLockDateImmutableThrowsException()
{
$article = new LockDateImmutable('Test LockInt');
$this->dm->persist($article);
$this->dm->flush();

Expand Down Expand Up @@ -382,7 +447,7 @@ public function testInvalidLockDocument()
public function testInvalidVersionDocument()
{
$this->expectException(MongoDBException::class);
$this->expectExceptionMessage('Invalid version field type string. Version field must be int or date.');
$this->expectExceptionMessage('Invalid version field type string. Version field must be int, integer, date or date_immutable.');
$this->dm->getClassMetadata(InvalidVersionDocument::class);
}

Expand Down Expand Up @@ -468,12 +533,19 @@ class LockInt extends AbstractVersionBase
}

/** @ODM\Document */
class LockTimestamp extends AbstractVersionBase
class LockDate extends AbstractVersionBase
{
/** @ODM\Version @ODM\Field(type="date") */
public $version;
}

/** @ODM\Document */
class LockDateImmutable extends AbstractVersionBase
{
/** @ODM\Version @ODM\Field(type="date_immutable") */
public $version;
}

/** @ODM\Document */
class InvalidLockDocument
{
Expand Down
Loading