[
https://issues.apache.org/jira/browse/FINERACT-1298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17266766#comment-17266766
]
Michael Vorburger edited comment on FINERACT-1298 at 1/17/21, 12:27 PM:
------------------------------------------------------------------------
On a positive note, it seems like we may not (necessarily) actually have to
replace all 276 uses of {{java.util.Date}}; it seems that Error Prone (2.5.1)
more specifically objects to the use of some particular known "bad" methods of
{{jjava.util.Date}}; for example, conversions using its {{toString()}}. So
this may less work than getting entirely rid of each and all last mention of
{{java.util.Date}} anymore.
But on a possibly slightly more "challenging" note, but perhaps also ;)
hopeful, solving this properly very likely leads into fixing a number of
local/server Time Zone related issues that the Fineract codebase clearly has.
For example, to replace the use of {{jjava.util.Date.toString()}} in line 379
of
{{org.apache.fineract.infrastructure.core.api.JsonCommand.isChangeInTimeParameterNamed(String,
Date, String)}}, inspired by
https://www.baeldung.com/java-date-to-localdate-and-localdatetime, we need a
{{ZoneId}}.. the current code is just hiding that (and that's what leads to
some of those problem), and using {{ZoneId.systemDefault()}} there, as in
{{time =
existingValue.toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime();}},
is wrong - IMHO. (But I haven't spent enough time to understand what would be
better.)
was (Author: vorburger):
On a positive note, it seems like we may not (necessarily) actually have to
replace all 276 uses of {{jjava.util.Date}}; it seems that Error Prone (2.5.1)
more specifically objects to the use of some particular known "bad" methods of
{{jjava.util.Date}}; for example, conversions using its {{toString()}}. So
this may less work.
On a possibly slightly more "challenging" note, but also hopeful, solving this
properly very likely leads into fixing a number of local/server Time Zone
related issues that the Fineract codebase clearly has. For example, to replace
the use of {{jjava.util.Date.toString()}} in line 379 of
{{org.apache.fineract.infrastructure.core.api.JsonCommand.isChangeInTimeParameterNamed(String,
Date, String)}}, inspired by
https://www.baeldung.com/java-date-to-localdate-and-localdatetime, we need a
{{ZoneId}}.. the current code is just hiding that (and that's what leads to
some of those problem), and using {{ZoneId.systemDefault()}} there, as in
{{time =
existingValue.toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime();}},
is wrong - IMHO. (But I haven't spent enough time to understand what would be
better.)
> Replace all 276 java.util.Date usages with java.time
> ----------------------------------------------------
>
> Key: FINERACT-1298
> URL: https://issues.apache.org/jira/browse/FINERACT-1298
> Project: Apache Fineract
> Issue Type: Improvement
> Components: System
> Reporter: Michael Vorburger
> Priority: Major
> Labels: tech-debt
>
> https://github.com/apache/fineract/pull/1574 led us to run into
> https://errorprone.info/bugpattern/JavaUtilDate.
> I'll see if we can FINERACT-1297 by inhibiting that check, for now - but
> replacing all of our currently 276 {{java.util.Date}} usages with
> {{java.time}} does seem a good idea, that I'm personally in support of. (The
> 276 are what a quick "grep" filtered by anything in ".java" shows.)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
