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

Added methods to update last-modified timestamp to DB interface #598

Merged
merged 1 commit into from May 24, 2018

Conversation

Projects
None yet
3 participants
@fangel
Member

fangel commented May 17, 2018

This moves the responsibility for updating the last-modified timestamp away from the update-metadata functionality,
and makes it into an explicit action performed by the database-operations event-listener

This is in reference to #530, where the behaviour between the Doctrine and Mongo implementation were different.

);
$now = $this->adapter->setLastModifiedNow($user, $imageIdentifier);
$this->assertEquals(time(), $now->getTimestamp(), 'Returned timestamp should be around now', 1);

This comment has been minimized.

@matslindh

matslindh May 22, 2018

Contributor

This can break if the test is run at exactly the right time (where the clock changes between the two statements). I think $time = time() before the call to setLastModifiedNow and then assertThat() with assertGreaterThanOrEqual($time) and assertLessThanOrEqual($time + 1) would at least give us a bit more leeway (.. if it takes more than a second to set the value, we might have other issues such as a table lock).

This comment has been minimized.

@fangel

fangel May 22, 2018

Member

The 4'th parameter to the assert (1) is actually an allowed delta-parameter. So it already allows for clock-skews of up to one second between the call to setLastModifiedNow and the assert...

This comment has been minimized.

@matslindh

matslindh May 22, 2018

Contributor

Ah, correct. I saw that and since the documentation for phpunit didn't say anything about it I assumed it had been deprecated (and we have a issue for upgrading to phpunit 6.4/6.5). I'm +0 for changing it then.

This comment has been minimized.

@fangel

fangel May 23, 2018

Member

It's listed in the manual for PHPUnit 6.5 - see https://phpunit.de/manual/6.5/en/appendixes.assertions.html#appendixes.assertions.assertEquals under example A.17 about working with floats. And it works for ints as well, even if the $delta-parameter is only mentioned wrt floats.

This comment has been minimized.

@matslindh

matslindh May 23, 2018

Contributor

There it is! I got confused by having the signatures documented separately. No change needed. :-)

Added methods to update last-modified timestamp to DB interface
This moves the responsibility for updating the last-modified timestamp away from the update-metadata functionality,
and makes it into an explicit action performed by the database-operations event-listener

This is in reference to #530, where the behaviour between the Doctrine and Mongo implementation were different.
@christeredvartsen

This comment has been minimized.

Member

christeredvartsen commented May 24, 2018

Looks good. Care to update the Changelog as well with relevant information for the next release?

@christeredvartsen christeredvartsen merged commit 1a0a968 into imbo:develop May 24, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment