[ 
https://issues.apache.org/jira/browse/IO-689?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17200727#comment-17200727
 ] 

Uwe Schindler commented on IO-689:
----------------------------------

The general problem here: An instant is just Epoch seconds (look at source 
code, there are no timezones involved!), so why do you convert it to a zoned 
time at all (and back)? There is no reason to do this, you don't even have a 
test that shows that this is really needed. This is overhead, because it needs 
to access a synchronized global JVM setting (default zone), it also produces 
many useless objects and does calculations that don't change anything - which 
you verified in your test.

The roundtrip works in your example, because "atZone" includes the timezone 
into the returned object. I was missing that, sorry! If you would convert it to 
a LocalDateTime, then you have a problem - ZonedDatetime is OK!

With your Test you have already shown, that the Instant does not change, so 
please remove the useless conversion! That's all I want to say, the risk to 
change the epoch seconds is too high for nonsense. The method that's called at 
the very end is just {{isFileNewer(File,long)}} and that takes a epoch milli 
(that has no timezone). {{Instant}} is just a wrapper around that.

> All new FileUtils method with Java 8 time break on DST change
> -------------------------------------------------------------
>
>                 Key: IO-689
>                 URL: https://issues.apache.org/jira/browse/IO-689
>             Project: Commons IO
>          Issue Type: Bug
>          Components: Utilities
>    Affects Versions: 2.8.0
>            Reporter: Uwe Schindler
>            Priority: Major
>
> This commit in PR #124 breaks FileUtils with new Java 8 datetime APIs:
> https://github.com/apache/commons-io/pull/124/commits/2eb549873470844c88681e92c64631f796002020
> Because of this I had to put all of the methods to the list of blacklisted 
> APIs in Apache Lucene / Solr. The reason for this change is that now all 
> depend on local datetime, there's no way to pass an Instant with a UNIX 
> timestamp through the API without it being corrupted due to 
> forwards/backwards transformation.
> Background: During DST changes the same local date time exists two times (in 
> autumn, you have in most countries two time the 2:30am time, once before and 
> once after the DST change - because time is rolled back).
> With the above commit you first convert an Instant (which is by definition 
> just a UNIX timestamp and can be converted as-is to a long) to a local 
> Datetime and then back to an Instant. By this forward/backward transformation 
> you get off by an hour during that one hour in autumn, when DST switches back 
> to standard time.
> Please revert this commit and release a bugfix.
> There is no reason to convert an Instant to local and back, the arguments in 
> the PR are plain wrong. Instants are timezoneless and are identical to UNIX 
> timestamps.
> This commit breaks all new methods, because at the end all new methods go 
> through the "Instant" path which does the faulty transformation.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to