[FEATURE] versionInfo: Use UTC Timestamp

[RandomByte]

No description provided.

[matz3]

@RandomByte the issue is related to the cachebuster but the code changes are for the version info. So I'm a bit confused 😕

[RandomByte]

Removed the reference to https://github.com/SAP/ui5-builder/issues/347 as it is actually unrelated.

Still to be checked: Does the technical info dialog expect UTC or the user timezone?

[coveralls]

Coverage increased (+0.02%) to 90.178% when pulling 660a83e on version-info-utc-timestamp into 829134b on master.

[codeworrior]

For the technical info dialog, buildtimestamp is a string. It does not try to parse or interpret it.

[RandomByte]

In that case I guess it is an improvement to use the UTC time here. Should we append a "UTC" string to the end of it?

[codeworrior]

That we have to check against the runtime, first. I don't remember where the cache buster takes the timestamp from. But AFAIK some code makes assumptions about the format of the timestamp. A trailing 'UTC' or 'Z' might confuse such code.

Most likely, it is taken from a different location. And definitely, the ui5 tooling is not responsible yet for the cachebuster token of the CDN installation. But if the version.json is the source for the runtime I would keep the syntax of the timestamp aligned.

[RandomByte]

So this PR should be good to merge for the time being?

[codeworrior]

From my POV, yes.

[matz3]

For the technical info dialog, buildtimestamp is a string. It does not try to parse or interpret it.

That's not quite true, right?
But changing to UTC should be fine, although the timezone is still not clear. This should rather be solved in the runtime.

CTRL+SHIFT+ALT+S
1.70.1 (built at 2019-10-08T08:43) => via RegExp /w fallback to given string

CTRL+SHIFT+ALT+P
1.70.1 (built at 08.10.2019 08:43:00) => via DateFormat (yyyyMMdd-HHmmss)

[codeworrior]

Strange. I could swear I saw a simple setProperty. But now I see

  oViewModel.setProperty("/ProductTimestamp",
     this._generateLocalizedBuildDate(oVersionInfo.buildTimestamp));

which couldn't be any clearer :-(

[codeworrior]

To compensate my embarrassing fault above a bit, I tried to get a full picture now, but what I see so far doesn't look nice :-(

In the TechnicalInfo (CTRL-SHIFT-ALT P), method _generateLocalizedBuildDate does not only create a localized date, it also parses the original time stamp with a locale specific DateFormat instance. That is, it assumes a local timestamp, which will be wrong whenever server and client are not in the same TZ.

So what about the sources for the timestamps?

In the Maven build, the sap.ui.buildinfo.buildtime is filled from the maven.build.timestamp property. According to https://issues.apache.org/jira/browse/MNG-5452 , Maven since version 3.2.2 uses UTC for its timestamp. In older versions, it used local time of the Java VM. I've checked some of the existing release log files, they all used Maven 3.0.5 for the recent patch releases (which implies that timestamps are in local time of the server / Java VM).

For the sap-ui-version.json in our delivery, the same seems to apply. The Maven tooling creates a timestamp with local time (using standard Java means, no dependency to the Maven version, AFAICS).

[codeworrior]

For a full picture, the UI5 tooling is still missing (at least the placeholder replacement in Global.js, sap-ui-version.json is the topic of this PR), but that's your home-turf :-)

[tobiasso85]

For the technical info dialog, buildtimestamp is a string. It does not try to parse or interpret it.

That's not quite true, right?
But changing to UTC should be fine, although the timezone is still not clear. This should rather be solved in the runtime.

CTRL+SHIFT+ALT+S
1.70.1 (built at 2019-10-08T08:43) => via RegExp /w fallback to given string

CTRL+SHIFT+ALT+P
1.70.1 (built at 08.10.2019 08:43:00) => via DateFormat (yyyyMMdd-HHmmss)

Couldn't we use DateFormat.getDateInstance in both cases and pass the UTC: true option to it?
Since it was never correct/stable anyways (server creates with local date/client consumes with local date).

Then we could eave this change as it is.

Making it a "proper" UTC timestamp requires the TZ imho and is a different topic

[tobiasso85]

I think it makes sense changing it here to UTC and adapt the UI5 codebase accordingly.
Does anybody depend on this timestamp being a "local" one?