Crell / Serde

Robust Serde (serialization/deserialization) library for PHP 8.
Other
296 stars 14 forks source link

Import/Export unix timestamps #51

Closed withinboredom closed 5 months ago

withinboredom commented 9 months ago

Description

Allows for the importing and exporting of UNIX timestamps

Motivation and context

closes #50

How has this been tested?

Ran on a dev-deployment of an API parsing lots of variations of datetimes.

Screenshots (if appropriate)

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

Please, please, please, don't send your pull request until all of the boxes are ticked. Once your pull request is created, it will trigger a build on our continuous integration server to make sure your tests and code style pass.

If you're unsure about any of these, don't hesitate to ask. We're here to help!

Crell commented 9 months ago

Thanks. This does look like the right direction. The test needs to be fleshed out a bit more (the timestamp needs to actually be provided in the test method, not just added to the object), and the @todo regarding checking the format should be resolved. Either let's do it, or let's say we don't need to. (I don't believe we're enforcing other formats on import, so I'm fine with not doing so here.)

photz-insify commented 8 months ago

The format specifier can also be U.v or U.u:

php > var_dump(\DateTimeImmutable::createfromFormat('U.v', '1707737662.500'));
php shell code:1:
class DateTimeImmutable#1 (3) {
  public $date =>
  string(26) "2024-02-12 11:34:22.500000"
  public $timezone_type =>
  int(1)
  public $timezone =>
  string(6) "+00:00"
}
php > var_dump(\DateTimeImmutable::createfromFormat('U.u', '1707737662.000900'));
php shell code:1:
class DateTimeImmutable#1 (3) {
  public $date =>
  string(26) "2024-02-12 11:34:22.000900"
  public $timezone_type =>
  int(1)
  public $timezone =>
  string(6) "+00:00"
}
Crell commented 8 months ago

@withinboredom Are you going to follow up here?

withinboredom commented 8 months ago

Thanks for the ping @Crell.

Are you going to follow up here?

Hmmm. I have the code on my computer at home; I just hadn't pushed it, apparently. I thought I did. I'll take care of that this evening or tomorrow morning.

withinboredom commented 8 months ago

Props to @photz-insify to give me the idea of a generic unix-time-exporter vs. trying to deal with various integer formats. This is much cleaner than the original.

Crell commented 8 months ago

Thinking on it, I agree, this is a great use case for a dedicated type field.

derickr commented 7 months ago

It can't.

Crell commented 6 months ago
$seconds = $value->getTimestamp();
if ($seconds > PHP_MAX_INT/$multiplier + 1) { fail}
//and with MIN_INT.

if (is microseconds) {
  $serializedVal = $seconds * $multiplier + $value->format('u');
} else if (is milliseconds) {
  $serializedVal = $seconds * $multiplier + $value->Format('u')/1000;
}

Talked to Derick at phpTek. We need something like this to avoid the rounding issues above. This also means that the range for serializing to milliseconds or microseconds is limited to a smaller range. It should cover most of current usage eras, but far-future values won't serialize. (Note: The above code is pseudocode at best, I probably got some API call wrong, the structure could be cleaner, etc. This is just to communicate the idea.)

Also, we're just not going to deal with nanoseconds. I don't think there are enough use cases to justify figuring out how to deal with Javascript's reduced range, which would result in data loss.

If you don't want to continue on it, I'll try to take this over sometime soon.

Thanks, @derickr!

withinboredom commented 6 months ago

This also means that the range for serializing to milliseconds or microseconds is limited to a smaller range. It should cover most of current usage eras, but far-future values won't serialize.

Sounds fine to me!

Also, we're just not going to deal with nanoseconds.

Heh, that was my original approach, I like it.

If you don't want to continue on it, I'll try to take this over sometime soon.

I'll tackle this later this week unless you beat me to it!

Crell commented 5 months ago

Needed to rebase to avoid conflicts. But this should take care of it. Also, looks like PHPStan got updated and is now pickier. How nice.

Crell commented 5 months ago

All set. Thank you, @withinboredom!