stoerr commented on a change in pull request #144:
URL:
https://github.com/apache/jackrabbit-filevault/pull/144#discussion_r646872076
##########
File path:
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackagePropertiesImpl.java
##########
@@ -224,21 +222,17 @@ public boolean requiresRestart() {
public Calendar getDateProperty(String name) {
Calendar result = null;
try {
- // TODO: add timezone if not there?
String p = getProperty(name);
if (p != null) {
- result = ISO8601.parse(p);
- if (result == null) {
- // Perhaps this is due to unusual timezone format +02 or
+0200 that aren't supported by ISO8601.parse
- // but sometimes produced by maven plugins, compare
https://issues.apache.org/jira/browse/JCRVLT-276
- Matcher splittedDate = TIMEZONE_FIX_PATTERN.matcher(p);
- if (splittedDate.matches()) {
- String tzminutes = splittedDate.group("tzMinutes");
- tzminutes = tzminutes != null ? tzminutes : "00";
- String reformattedDate =
splittedDate.group("dateUptoTzHours") + ":" + tzminutes;
- result = ISO8601.parse(reformattedDate);
- }
+ ZonedDateTime zonedDateTime;
+ try {
+ // supports date in format new
SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSXXX")
+ zonedDateTime = ZonedDateTime.parse(p,
DateTimeFormatter.ISO_OFFSET_DATE_TIME);
+ } catch (DateTimeParseException e) {
+ // support dates given out via new
SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); as well (used in
package-maven-plugin till version 1.0.3, compare with
https://issues.apache.org/jira/browse/JCRVLT-276)
+ zonedDateTime = ZonedDateTime.parse(p,
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSX"));
Review comment:
That undoubtedly looks much cleaner than the pattern replacement
approach. So, if that is backwards compatible to ISO8601 and does parse the
+0200 timezone format variant, then that's completely fine with me. Mind you -
the test I wrote did not intend to see whether it still parses all date
formatting variants ISO8601.parse understands, if there are some, since it was
still using ISO8601.parse so far. So you might want to check whether you need
to extend the test somewhat.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
