add precision option for datetime string dumping

[f0ster]

No description provided.

[pboling]

This looks like a great enhancement!
Is there a reasonable way to test this new option?

[andrykonchin]

Great, but the feature is missing specs. Please add specs for Dynamoid::Dumping module.

[f0ster]

@andrykonchin @pboling added specs

[andrykonchin]

I don't think we need these changes in type casting. Anything that is like date/time could be casted to DateTime. Type casting isn't connected with writing/reading attributes to/from DynamoDB.

[f0ster]

@andrykonchin Per the comment, the existing expected behaviour is to break for poorly parsed strings. https://github.com/Dynamoid/dynamoid/blob/master/spec/dynamoid/type_casting_spec.rb#L143

I wrote this work around to support precision, and also keep the same erroneous expectations. Parse is expected to throw an exception, where as constructing with iso8601 has less strict input expectations.

[andrykonchin]

I see. Well.

                 DateTime.parse(value)
                 DateTime.iso8601(value)

Here. DateTime.parse supports precision, doesn't it?

irb(main):003:0> s = DateTime.now.iso8601(2)
=> "2019-07-29T22:06:24.62+03:00"
irb(main):004:0> DateTime.parse(s).usec
=> 620000

Here

            if dt_has_precision
              dt.utc

completely ignores ApplicationTimeZone.utc_offset what's wrong. If passed string doesn't have timezone - ApplicationTimeZone.utc_offset should be used as default timezone.

I thinks precision support could be added directly in this line:

DateTime.new(dt.year, dt.mon, dt.mday, dt.hour, dt.min, dt.sec, offset)

seconds argument could have fraction so this line could be changed in this way:

DateTime.new(dt.year, dt.mon, dt.mday, dt.hour, dt.min, dt.sec + dt.sec_fraction, offset)

Look at the example:

irb(main):015:0> s = DateTime.now.iso8601(4)
=> "2019-07-29T22:24:20.8163+03:00"
irb(main):016:0> DateTime.parse(s).sec
=> 20
irb(main):018:0> DateTime.parse(s).sec_fraction
=> (8163/10000)

[andrykonchin]

I believe it should be tested as well. It's a new feature - storing milliseconds when use string format for datetime.

[f0ster]

I agree, I think this could be a clearer abstraction. FWIW, This was amended to make the current and additional spec coverage pass

[andrykonchin]

Excessive new lines detected

[andrykonchin]

I think i't completely OK to use to_f here but it's a bad practice to check equality of floating point numbers. Comparing milliseconds or string representation would be more correct.

The precision is 6 in the test so it's OK, but with precision 9 (with nanoseconds) this test could become flaky because MRI isn't accurate in preserving nanoseconds for Time/DateTime.

[andrykonchin]

@f0ster Did you have a chance to look through my comments?