Skip to content

Commit

Permalink
remove default zeroing of time for DateAttribute
Browse files Browse the repository at this point in the history
this leads to wrong interpretation of dates (off-by-one) as the time is
changed. The native format used still contains the time information to
provide the timezone information for consumers. A short format like
'yyyy-MM-dd+00:00' is not being used even though php likes it other
parties interpreting that value might not like it. One example are api
consumers that get that native value or even elasticsearch which wants to
see something like "yyyy-MM-ddT+00:00". That would be parsed as military
Tango time though by php. That's why we sent the time info. Meh.
Suggestions are welcome. Applications can reformat according to their
needs on their system boundaries.

Future improvement suggestion: internally store the timezone of the given
values to have them available later on. This helps in migrating stuff when
eg.g the timezone db changes.

refs #28
  • Loading branch information
graste committed Oct 26, 2016
1 parent 253174a commit 5d0b053
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 55 deletions.
4 changes: 0 additions & 4 deletions src/Runtime/Attribute/Date/DateAttribute.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@
// preferred exchange format is FORMAT_ISO8601 ('Y-m-d\TH:i:s.uP')
class DateAttribute extends TimestampAttribute
{
const OPTION_DEFAULT_HOUR = DateRule::OPTION_DEFAULT_HOUR;
const OPTION_DEFAULT_MINUTE = DateRule::OPTION_DEFAULT_MINUTE;
const OPTION_DEFAULT_SECOND = DateRule::OPTION_DEFAULT_SECOND;

const FORMAT_NATIVE = TimestampAttribute::FORMAT_ISO8601_SIMPLE;

protected function buildValidationRules()
Expand Down
6 changes: 5 additions & 1 deletion src/Runtime/Attribute/Date/DateValueHolder.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class DateValueHolder extends TimestampValueHolder
* returned format MUST be acceptable as a new value on the valueholder
* to reconstitute it.
*
* This serializes with time information and timezone to allow correct
* interpretation when deserializing even though the time itself is not
* relevant.
*
* @return mixed value that can be used for serializing/deserializing
*/
public function toNative()
Expand All @@ -26,7 +30,7 @@ public function toNative()
return $this->getValue()->format(
$this->getAttribute()->getOption(
DateAttribute::OPTION_FORMAT_NATIVE,
DateAttribute::FORMAT_ISO8601_SIMPLE
DateAttribute::FORMAT_NATIVE
)
);
}
Expand Down
42 changes: 0 additions & 42 deletions src/Runtime/Validator/Rule/Type/DateRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,7 @@

namespace Trellis\Runtime\Validator\Rule\Type;

use Trellis\Runtime\Attribute\AttributeInterface;
use Trellis\Runtime\Entity\EntityInterface;

class DateRule extends TimestampRule
{
const OPTION_DEFAULT_HOUR = 'default_hour';
const OPTION_DEFAULT_MINUTE = 'default_minute';
const OPTION_DEFAULT_SECOND = 'default_second';

const FORMAT_NATIVE = self::FORMAT_ISO8601_SIMPLE;

protected function execute($value, EntityInterface $entity = null)
{
$success = parent::execute($value);

if (!$success) {
return false;
}

$null_value = $this->getOption(AttributeInterface::OPTION_NULL_VALUE, null);
if ($value === $null_value || $value === '') {
// accept empty values as valid when no mandatory handling happens in this rule
$this->setSanitizedValue($null_value);
return true;
}

$date = $this->getSanitizedValue();

// forget about microsecond precision
$date = $date->createFromFormat(
self::FORMAT_ISO8601_SIMPLE,
$date->format(self::FORMAT_ISO8601_SIMPLE)
);

// set time to 00:00:00
$date = $date->setTime(
$this->getOption(self::OPTION_DEFAULT_HOUR, 0),
$this->getOption(self::OPTION_DEFAULT_MINUTE, 0),
$this->getOption(self::OPTION_DEFAULT_SECOND, 0)
);

$this->setSanitizedValue($date);

return true;
}
}
81 changes: 73 additions & 8 deletions tests/Runtime/Attribute/Date/DateAttributeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,48 @@ public function testCreate()
$this->assertEquals($attribute->getName(), self::ATTR_NAME);
}

public function testIssue28()
{
$previous_tz = date_default_timezone_get();
date_default_timezone_set('Europe/Berlin');

$datetime = '2016-12-10';
$attribute = new DateAttribute(self::ATTR_NAME, Mockery::mock(EntityTypeInterface::CLASS));
$value = $attribute->createValueHolder();
$value->setValue($datetime);
$this->assertEquals(
'2016-12-09T23:00:00.000000+00:00',
$value->getValue()->format(DateAttribute::FORMAT_ISO8601)
);

$date = $value->getValue();
$date = $date->setTimezone(new \DateTimeZone('Europe/Berlin'));
$this->assertEquals('2016-12-10T00:00:00.000000+01:00', $date->format(DateAttribute::FORMAT_ISO8601));
$this->assertEquals('2016-12-10+01:00', $date->format(DateAttribute::FORMAT_ISO8601_DATE));
$this->assertEquals('2016-12-10', $date->format(DateAttribute::FORMAT_ISO8601_DATE_SIMPLE));

date_default_timezone_set($previous_tz);
}

public function testCreateValueAcceptsString()
{
$datetime = '2014-12-31T13:45:55.123+01:00';
$date_in_utc = '2014-12-31T00:00:00.000000+00:00';
$date_in_utc = '2014-12-31T12:45:55.123000+00:00';
$date_native = '2014-12-31T12:45:55+00:00';
$attribute = new DateAttribute(self::ATTR_NAME, Mockery::mock(EntityTypeInterface::CLASS));
$value = $attribute->createValueHolder();
$this->assertInstanceOf(DateValueHolder::CLASS, $value);
$this->assertNull($value->getValue());
$value->setValue($datetime);
$this->assertInstanceOf(DateTimeImmutable::CLASS, $value->getValue());
$this->assertEquals($date_in_utc, $value->getValue()->format(DateAttribute::FORMAT_ISO8601));
$this->assertEquals($date_native, $value->getValue()->format(DateAttribute::FORMAT_NATIVE));
}

public function testCreateValueWithDefaultValueAsString()
{
$datetime = '2014-12-29+01:00';
$datetime_in_utc = '2014-12-28T00:00:00.000000+00:00';
$datetime_in_utc = '2014-12-28T23:00:00.000000+00:00';
$attribute = new DateAttribute(
self::ATTR_NAME,
Mockery::mock(EntityTypeInterface::CLASS),
Expand All @@ -54,8 +79,8 @@ public function testCreateValueWithDefaultValueAsString()
public function testCreateValueWithDefaultValueAsStringWithoutDefaultTimezoneForcing()
{
$datetime = '2014-12-28T13:45:55.123+01:00';
$datetime_in_cet = '2014-12-28T00:00:00.000000+01:00';
$datetime_in_utc = '2014-12-28T00:00:00.000000+00:00';
$datetime_in_cet = '2014-12-28T13:45:55.123000+01:00';
$datetime_in_utc = '2014-12-28T12:45:55.123000+00:00';
$attribute = new DateAttribute(
self::ATTR_NAME,
Mockery::mock(EntityTypeInterface::CLASS),
Expand Down Expand Up @@ -164,7 +189,7 @@ public function testMaxConstraint()
public function testToNative()
{
$datetime = '2014-12-28+01:00';
$datetime_string = '2014-12-27T00:00:00+00:00';
$datetime_string = '2014-12-27T23:00:00+00:00';
$attribute = new DateAttribute(
self::ATTR_NAME,
Mockery::mock(EntityTypeInterface::CLASS),
Expand Down Expand Up @@ -200,10 +225,13 @@ public function testToNativeRoundtripWithDefaultValue()
$this->assertEquals($attribute->getDefaultValue(), $valueholder->getValue());
}

public function testToNativeRoundtrip()
public function testToNativeRoundtripWithExplicitTimezoneInput()
{
$previous_tz = date_default_timezone_get();
date_default_timezone_set('Europe/Berlin');

$date = '2014-12-28+01:00';
$date_as_native_string = '2014-12-27T00:00:00+00:00';
$date_as_native_string = '2014-12-27T23:00:00+00:00';

$attribute = new DateAttribute(self::ATTR_NAME, Mockery::mock(EntityTypeInterface::CLASS));
$valueholder = $attribute->createValueHolder();
Expand All @@ -220,10 +248,47 @@ public function testToNativeRoundtrip()
$dt_cet = $valueholder->getValue()->setTimeZone(new DateTimeZone('Europe/Berlin'));
$valueholder->setValue($dt_cet);

$this->assertNotEquals($attribute->getNullValue(), $valueholder->getValue());
$dtval = $valueholder->getValue();
$this->assertNotEquals($attribute->getNullValue(), $dtval);
$this->assertEquals($date_as_native_string, $valueholder->toNative());
$this->assertEquals('2014-12-27T23:00:00.000000+00:00', $dtval->format(DateAttribute::FORMAT_ISO8601));
$this->assertEquals('2014-12-27+00:00', $dtval->format(DateAttribute::FORMAT_ISO8601_DATE));
$this->assertEquals('2014-12-27', $dtval->format(DateAttribute::FORMAT_ISO8601_DATE_SIMPLE));

date_default_timezone_set($previous_tz);
}

public function testToNativeRoundtrip()
{
$previous_tz = date_default_timezone_get();
date_default_timezone_set('Europe/Berlin');

$date = '2014-12-28';
$date_as_native_string = '2014-12-27T23:00:00+00:00';

$attribute = new DateAttribute(self::ATTR_NAME, Mockery::mock(EntityTypeInterface::CLASS));
$valueholder = $attribute->createValueHolder();
$valueholder->setValue($date);

$this->assertNotEquals($attribute->getNullValue(), $valueholder->getValue());
$this->assertEquals($date_as_native_string, $valueholder->toNative());

$valueholder->setValue($valueholder->toNative());

$this->assertNotEquals($attribute->getNullValue(), $valueholder->getValue());
$this->assertEquals($date_as_native_string, $valueholder->toNative());

$valueholder->setValue($valueholder->toNative());

$dtval = $valueholder->getValue();
$this->assertNotEquals($attribute->getNullValue(), $dtval);
$this->assertEquals('2014-12-27T23:00:00+00:00', $valueholder->toNative());
$this->assertEquals('2014-12-27T23:00:00.000000+00:00', $dtval->format(DateAttribute::FORMAT_ISO8601));
$this->assertEquals('2014-12-27+00:00', $dtval->format(DateAttribute::FORMAT_ISO8601_DATE));
$this->assertEquals('2014-12-27', $dtval->format(DateAttribute::FORMAT_ISO8601_DATE_SIMPLE));

date_default_timezone_set($previous_tz);
}
public function testToNativeCustomFormat()
{
$date = '2014-12-28+01:00';
Expand Down

0 comments on commit 5d0b053

Please sign in to comment.