PersonIdent: Use java.time instead of older Date and milliseconds

From errorprone: Date has a bad API that leads to bugs; prefer
java.time.Instant or LocalDate.

Replace the long with milliseconds and int with minutes offset with an
Instant and a ZoneOffset. Create new constructors and deprecate
variants with Date, milliseconds and minute offsets.

When comparing instances of PersonIdent truncate the timestamp precision
to 1 second since git commit timestamps are persisted with 1 second
precision [1].

[1] https://git-scm.com/docs/git-commit#Documentation/git-commit.txt-Gitinternalformat

Change-Id: Id4ba1f108e1ba0bfcdd87ba37c67e2d3cc7d254f

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/PersonIdentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/PersonIdentTest.java
index 97da1757e05fe5db9ad47da5727bc59afc67de5c..943a68b82ceed12cc5d81865b875898c6109361e 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/PersonIdentTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/PersonIdentTest.java
@@ -55,7 +55,8 @@ public class PersonIdentTest {
                                p.getWhenAsInstant());
                assertEquals("A U Thor <author@example.com> 1142878501 -0500",
                                p.toExternalString());
-               assertEquals(ZoneId.of("GMT-05:00"), p.getZoneId());
+               assertEquals(ZoneId.of("GMT-05:00").getRules().getOffset(
+                               Instant.ofEpochMilli(1142878501000L)), p.getZoneOffset());
        }
 
        @Test
@@ -69,7 +70,8 @@ public class PersonIdentTest {
                                p.getWhenAsInstant());
                assertEquals("A U Thor <author@example.com> 1142878501 +0530",
                                p.toExternalString());
-               assertEquals(ZoneId.of("GMT+05:30"), p.getZoneId());
+               assertEquals(ZoneId.of("GMT+05:30").getRules().getOffset(
+                               Instant.ofEpochMilli(1142878501000L)), p.getZoneOffset());
        }
 
        @SuppressWarnings("unused")

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
index 3ba055aae80f7240bcbc55d7c111a490686c3014..a59dde7083a37c5f9b8745b21944ca4574fa5bd1 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
@@ -13,9 +13,11 @@
 package org.eclipse.jgit.lib;
 
 import java.io.Serializable;
-import java.text.SimpleDateFormat;
 import java.time.Instant;
 import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
 import java.util.Date;
 import java.util.Locale;
 import java.util.TimeZone;
@@ -48,6 +50,17 @@ public class PersonIdent implements Serializable {
                return TimeZone.getTimeZone(tzId.toString());
        }
 
+       /**
+        * Translate a minutes offset into a ZoneId
+        *
+        * @param tzOffset as minutes east of UTC
+        * @return a ZoneId  for this offset
+        * @since 7.1
+        */
+       public static ZoneId getZoneId(int tzOffset) {
+               return ZoneOffset.ofHoursMinutes(tzOffset / 60, tzOffset % 60);
+       }
+
        /**
         * Format a timezone offset.
         *
@@ -121,13 +134,17 @@ public class PersonIdent implements Serializable {
                }
        }
 
+       // Write offsets as [+-]HHMM
+       private static final DateTimeFormatter OFFSET_FORMATTER = DateTimeFormatter
+                       .ofPattern("Z", Locale.US); //$NON-NLS-1$
+
        private final String name;
 
        private final String emailAddress;
 
-       private final long when;
+       private final Instant when;
 
-       private final int tzOffset;
+       private final ZoneId tzOffset;
 
        /**
         * Creates new PersonIdent from config info in repository, with current time.
@@ -160,7 +177,7 @@ public class PersonIdent implements Serializable {
         *            a {@link java.lang.String} object.
         */
        public PersonIdent(String aName, String aEmailAddress) {
-               this(aName, aEmailAddress, SystemReader.getInstance().getCurrentTime());
+               this(aName, aEmailAddress, SystemReader.getInstance().now());
        }
 
        /**
@@ -177,7 +194,7 @@ public class PersonIdent implements Serializable {
         */
        public PersonIdent(String aName, String aEmailAddress,
                        ProposedTimestamp when) {
-               this(aName, aEmailAddress, when.millis());
+               this(aName, aEmailAddress, when.instant());
        }
 
        /**
@@ -189,8 +206,25 @@ public class PersonIdent implements Serializable {
         *            local time
         * @param tz
         *            time zone
+        * @deprecated Use {@link #PersonIdent(PersonIdent, Instant, ZoneId)} instead.
         */
+       @Deprecated(since = "7.1")
        public PersonIdent(PersonIdent pi, Date when, TimeZone tz) {
+               this(pi.getName(), pi.getEmailAddress(), when.toInstant(), tz.toZoneId());
+       }
+
+       /**
+        * Copy a PersonIdent, but alter the clone's time stamp
+        *
+        * @param pi
+        *            original {@link org.eclipse.jgit.lib.PersonIdent}
+        * @param when
+        *            local time
+        * @param tz
+        *            time zone offset
+        * @since 7.1
+        */
+       public PersonIdent(PersonIdent pi, Instant when, ZoneId tz) {
                this(pi.getName(), pi.getEmailAddress(), when, tz);
        }
 
@@ -202,9 +236,11 @@ public class PersonIdent implements Serializable {
         *            original {@link org.eclipse.jgit.lib.PersonIdent}
         * @param aWhen
         *            local time
+        * @deprecated Use the variant with an Instant instead
         */
+       @Deprecated(since = "7.1")
        public PersonIdent(PersonIdent pi, Date aWhen) {
-               this(pi.getName(), pi.getEmailAddress(), aWhen.getTime(), pi.tzOffset);
+               this(pi.getName(), pi.getEmailAddress(), aWhen.toInstant());
        }
 
        /**
@@ -218,7 +254,7 @@ public class PersonIdent implements Serializable {
         * @since 6.1
         */
        public PersonIdent(PersonIdent pi, Instant aWhen) {
-               this(pi.getName(), pi.getEmailAddress(), aWhen.toEpochMilli(), pi.tzOffset);
+               this(pi.getName(), pi.getEmailAddress(), aWhen, pi.tzOffset);
        }
 
        /**
@@ -230,11 +266,12 @@ public class PersonIdent implements Serializable {
         *            local time stamp
         * @param aTZ
         *            time zone
+        * @deprecated Use the variant with Instant and ZoneId instead
         */
+       @Deprecated(since = "7.1")
        public PersonIdent(final String aName, final String aEmailAddress,
                        final Date aWhen, final TimeZone aTZ) {
-               this(aName, aEmailAddress, aWhen.getTime(), aTZ.getOffset(aWhen
-                               .getTime()) / (60 * 1000));
+               this(aName, aEmailAddress, aWhen.toInstant(), aTZ.toZoneId());
        }
 
        /**
@@ -252,10 +289,16 @@ public class PersonIdent implements Serializable {
         */
        public PersonIdent(final String aName, String aEmailAddress, Instant aWhen,
                        ZoneId zoneId) {
-               this(aName, aEmailAddress, aWhen.toEpochMilli(),
-                               TimeZone.getTimeZone(zoneId)
-                                               .getOffset(aWhen
-                               .toEpochMilli()) / (60 * 1000));
+               if (aName == null)
+                       throw new IllegalArgumentException(
+                                       JGitText.get().personIdentNameNonNull);
+               if (aEmailAddress == null)
+                       throw new IllegalArgumentException(
+                                       JGitText.get().personIdentEmailNonNull);
+               name = aName;
+               emailAddress = aEmailAddress;
+               when = aWhen;
+               tzOffset = zoneId;
        }
 
        /**
@@ -267,15 +310,18 @@ public class PersonIdent implements Serializable {
         *            local time stamp
         * @param aTZ
         *            time zone
+        * @deprecated Use the variant with Instant and ZoneId instead
         */
+       @Deprecated(since = "7.1")
        public PersonIdent(PersonIdent pi, long aWhen, int aTZ) {
-               this(pi.getName(), pi.getEmailAddress(), aWhen, aTZ);
+               this(pi.getName(), pi.getEmailAddress(), Instant.ofEpochMilli(aWhen),
+                               getZoneId(aTZ));
        }
 
        private PersonIdent(final String aName, final String aEmailAddress,
-                       long when) {
+                       Instant when) {
                this(aName, aEmailAddress, when, SystemReader.getInstance()
-                               .getTimezone(when));
+                               .getTimeZoneAt(when));
        }
 
        private PersonIdent(UserConfig config) {
@@ -298,19 +344,12 @@ public class PersonIdent implements Serializable {
         *            local time stamp
         * @param aTZ
         *            time zone
+        * @deprecated Use  the variant with Instant and ZoneId instead
         */
+       @Deprecated(since = "7.1")
        public PersonIdent(final String aName, final String aEmailAddress,
                        final long aWhen, final int aTZ) {
-               if (aName == null)
-                       throw new IllegalArgumentException(
-                                       JGitText.get().personIdentNameNonNull);
-               if (aEmailAddress == null)
-                       throw new IllegalArgumentException(
-                                       JGitText.get().personIdentEmailNonNull);
-               name = aName;
-               emailAddress = aEmailAddress;
-               when = aWhen;
-               tzOffset = aTZ;
+               this(aName, aEmailAddress, Instant.ofEpochMilli(aWhen), getZoneId(aTZ));
        }
 
        /**
@@ -335,9 +374,12 @@ public class PersonIdent implements Serializable {
         * Get timestamp
         *
         * @return timestamp
+        *
+        * @deprecated Use getWhenAsInstant instead
         */
+       @Deprecated(since = "7.1")
        public Date getWhen() {
-               return new Date(when);
+               return Date.from(when);
        }
 
        /**
@@ -347,26 +389,39 @@ public class PersonIdent implements Serializable {
         * @since 6.1
         */
        public Instant getWhenAsInstant() {
-               return Instant.ofEpochMilli(when);
+               return when;
        }
 
        /**
         * Get this person's declared time zone
         *
         * @return this person's declared time zone; null if time zone is unknown.
+        *
+        * @deprecated Use getZoneId instead
         */
+       @Deprecated(since = "7.1")
        public TimeZone getTimeZone() {
-               return getTimeZone(tzOffset);
+               return TimeZone.getTimeZone(tzOffset);
        }
 
        /**
         * Get the time zone id
         *
         * @return the time zone id
-        * @since 6.1
+        * @since 7.1
         */
        public ZoneId getZoneId() {
-               return getTimeZone().toZoneId();
+               return tzOffset;
+       }
+
+       /**
+        * Return the offset in this timezone at the specific time
+        *
+        * @return the offset
+        * @since 7.1
+        */
+       public ZoneOffset getZoneOffset() {
+               return tzOffset.getRules().getOffset(when);
        }
 
        /**
@@ -374,9 +429,11 @@ public class PersonIdent implements Serializable {
         *
         * @return this person's declared time zone as minutes east of UTC. If the
         *         timezone is to the west of UTC it is negative.
+        * @deprecated Use {@link #getZoneOffset()} and read minutes from there
         */
+       @Deprecated(since = "7.1")
        public int getTimeZoneOffset() {
-               return tzOffset;
+               return getZoneOffset().getTotalSeconds() / 60;
        }
 
        /**
@@ -388,7 +445,7 @@ public class PersonIdent implements Serializable {
        public int hashCode() {
                int hc = getEmailAddress().hashCode();
                hc *= 31;
-               hc += (int) (when / 1000L);
+               hc += when.hashCode();
                return hc;
        }
 
@@ -398,7 +455,9 @@ public class PersonIdent implements Serializable {
                        final PersonIdent p = (PersonIdent) o;
                        return getName().equals(p.getName())
                                        && getEmailAddress().equals(p.getEmailAddress())
-                                       && when / 1000L == p.when / 1000L;
+                                       // commmit timestamps are stored with 1 second precision
+                                       && when.truncatedTo(ChronoUnit.SECONDS)
+                                                       .equals(p.when.truncatedTo(ChronoUnit.SECONDS));
                }
                return false;
        }
@@ -414,9 +473,9 @@ public class PersonIdent implements Serializable {
                r.append(" <"); //$NON-NLS-1$
                appendSanitized(r, getEmailAddress());
                r.append("> "); //$NON-NLS-1$
-               r.append(when / 1000);
+               r.append(when.toEpochMilli() / 1000);
                r.append(' ');
-               appendTimezone(r, tzOffset);
+               r.append(OFFSET_FORMATTER.format(getZoneOffset()));
                return r.toString();
        }
 
@@ -424,18 +483,16 @@ public class PersonIdent implements Serializable {
        @SuppressWarnings("nls")
        public String toString() {
                final StringBuilder r = new StringBuilder();
-               final SimpleDateFormat dtfmt;
-               dtfmt = new SimpleDateFormat("EEE MMM d HH:mm:ss yyyy Z", Locale.US);
-               dtfmt.setTimeZone(getTimeZone());
-
+               DateTimeFormatter dtfmt = DateTimeFormatter
+                               .ofPattern("EEE MMM d HH:mm:ss yyyy Z", Locale.US) //$NON-NLS-1$
+                               .withZone(tzOffset);
                r.append("PersonIdent[");
                r.append(getName());
                r.append(", ");
                r.append(getEmailAddress());
                r.append(", ");
-               r.append(dtfmt.format(Long.valueOf(when)));
+               r.append(dtfmt.format(when));
                r.append("]");
-
                return r.toString();
        }
 }