Hi Kevin,
Appreciate you taking the time to work on this; my thoughts within...


On 27/03/2011 16:39, Kevin Meyer - KMZ wrote:
I'm looking at defining an "application" timezone, but I'm not sure
where the best place is to define it.

That does seem like a useful concept.

org.apache.isis.runtimes.dflt.runtime.context.IsisContext seems like a
good start. The IsisSession has a getUserProfile, which has a
Localisation interface (which exposes a getTimeZone() method), but
this would possibly be the place to keep a per-user timezone, not an
application-level timezone [is this an artificial distinction?].

I think we *should* distinguish the two.

As per the previous thread, I think that there are three clocks:

- user timezone
- app timezone (where the appserver runs)
- db timezone.

However, as your other thread said, java.sql.Date is meant to be a UTC date. So that simplifies things somewhat.

- user timezone
- app timezone (where the appserver runs)
- db timezone = UTC

In addition, though, I think we could reasonably say (define) that the app timezone = UTC always also. This is, effectively, what Isis currently does. That would reduce matters down to:

- user timezone
- app/db timezone = UTC



My goal is replace all occurrences of:
        "cal.setTimeZone(UTC_TIME_ZONE);"
with a configurable
        "cal.setTimeZone(IsisContext.getTimeZone());"

Likewise, the datastore implementations could provide their own
getTimeZone() method, which defines the datastore's internal
timezone.
Comments? Suggestions?

I'm not in favour of this, because "cal" could occur in many cases, some of which may be in the app layer, some of which may logically be in the UI layer. Moreover, some of these "cal" occur in the applib, which MUST NOT reference the runtime (IsisContext).



This has an impact on all tests (for example), as it introduces a new
dependency into those classes that currently just create a UTC
timezone.

Currently affected ( a quick search for etc/UTC):
./applib/src/main/java/org/apache/isis/applib/fixtures/FixtureClock.java
./applib/src/main/java/org/apache/isis/applib/value/Date.java
./applib/src/main/java/org/apache/isis/applib/value/Time.java
./applib/src/test/java/org/apache/isis/applib/value/TestClock.java
[snip]

As noted above, these applib classes MUST NOT depend on IsisContext.

~~~
As - again - you did note in your other thread, what's required is some sort of translation layer between the user timezone and UTC, and - again as you point out - org.apache.isis.adapters.Localization interface has this. This was something that Rob introduced about a month or go.

I've just taken a look at how this interface is used it, it would seem to be used by ValueSemanticsProvider's Parser#displayTitleOf. In other words, this *is* the translation layer that you speak of (a Parser is used to render every value of every property of every object).

~~~
Putting all the above together:

- let's agree that app server time = DB server time = UTC
- the user timezone is as defined by Localization
- the UI layer - by way of Parser interface - always (or should always) render the time with respect to the user's localization
- we therefore shouldn't need any changes to the codebase.

Where we currently have failing tests, it's probably because they are evaluating the date/time with respect to the timezone in which they are running, rather than UTC. In other words, the tests are at fault.

~~~
Now, the above analysis may well be wrong, I haven't thought about this as much as you, but that's my initial stab on the matter.

Let's keep this thread open until we get to a design that we are all happy with and understand.

Cheers
Dan



Reply via email to