[
https://issues.apache.org/jira/browse/FINERACT-826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17082179#comment-17082179
]
Michael Vorburger commented on FINERACT-826:
--------------------------------------------
[https://github.com/apache/fineract/pull/755] :
{quote}I also want to check if very tenant is guaranteed to always have a TZ.
@vorburger seems it falls dependent on the OS / JVM TZ if tenant TZ is
null.{quote}
if it's not mandatory (`NON NULL`) in the DB, then you can't be sure, right? It
would break backwards compatibility for any existing installations out there,
agreed? The only thing we (you) could do if you want to fix this is to make a
Flyway migration script that sets it to required? Then anyone upgrading would
fail, and would be forced to set a TZ for each of their Tenants, if they don't
already have one. Then you could make this code assume there always is one, and
throw a (clear... include reference to a JIRA!) exception.
> org.apache.fineract.infrastructure.core.service.DateUtils has un-used line,
> and should use java.time instead of Joda API
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: FINERACT-826
> URL: https://issues.apache.org/jira/browse/FINERACT-826
> Project: Apache Fineract
> Issue Type: Bug
> Affects Versions: 1.4.0
> Reporter: Michael Vorburger
> Assignee: Michael Vorburger
> Priority: Major
> Fix For: 1.4.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> [http://mail-archives.apache.org/mod_mbox/fineract-dev/201907.mbox/%3ccalix4ib1d32zskzn87kureosbwscjmo9dhgnoqbwydyui-c...@mail.gmail.com%3e,]
> and the original previous and the follow up replies on that thread Subject
> "Questions about my approach to find problems with tenant time zones, and
> about the instance values of SQL migration files.", related to FINERACT-723?
> I think, point out that
> {{org.apache.fineract.infrastructure.core.service.DateUtils}} could do with a
> closer look and fixes, specifically:
> {quote}That DateUtils class is interesting.. do we really need to mix the old
> java.util.Date API and the org.joda.time Time API ? FYI Joda Time has been
> integrated into Java 8 as java.time. So if someone was up for taking up work
> related to cleaning that up, that could be useful IMHO (so converge
> everything to java.time, and eventually remove the Joda dependency, and
> introduce an enforced check via Checkstle or SpotBugs to forbid
> java.util.Date).
>
> The null management in DateUtils is pretty interesting as well. So is this
> per tenant TZ guaranteed to always be in the DB? What does the DB schema say
> - required, never NULL? Does the Java code ensure it's never null? I'm asking
> these questions because something like that getLocalDateTimeOfTenant(),
> despite its name, seems to be actually be written to EITHER give us a "local"
> time based on the tenant, or, if that's null, it just falls back to what l
> suspect (not verified) is dependent on the OS / JVM TZ - that's scary.
> Perhaps it's not a problem in reality, if every tenant is guaranteed to
> always have a TZ, but if someone could double check this, and if so remove
> those null check and system dependent fallbacks, then IMHO that would make
> that utility code clearer.
>
> BTW line 45 in DateUtils seems completely useless, agreed? Would you like to
> raise a PR proposing to ditch that?
> {quote}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
