Improved string representation of temporal amount

src/main/java/net/fortuna/ical4j/model/Period.java

@@ -100,7 +100,7 @@ public Period(final DateTime start, final DateTime end) {
      */
     @Deprecated
     public Period(final DateTime start, final Dur duration) {
-        this(start, TemporalAmountAdapter.from(duration));
+        this(start, TemporalAmountAdapter.from(duration).getDuration());
     }
 
     /**

src/main/java/net/fortuna/ical4j/model/TemporalAmountAdapter.java

@@ -1,6 +1,7 @@
 package net.fortuna.ical4j.model;
 
 import java.time.Duration;
+import java.time.Instant;
 import java.time.Period;
 import java.time.temporal.TemporalAmount;
 import java.util.Date;
@@ -26,32 +27,90 @@ public String toString() {
         if (Duration.ZERO.equals(duration) || Period.ZERO.equals(duration)) {
             retVal = duration.toString();
         } else if (duration instanceof Period) {
-            Period period = (Period) duration;
-            if (period.getYears() > 0) {
-                int weeks = Math.abs(period.getYears()) * 52;
-                if (period.getYears() < 0) {
-                    weeks = -weeks;
-                }
-                retVal = String.format("P%dW", weeks);
-            } else if (period.getMonths() > 0) {
-                int weeks = Math.abs(period.getMonths()) * 4;
-                if (period.getMonths() < 0) {
-                    weeks = -weeks;
-                }
-                retVal = String.format("P%dW", weeks);
-            } else if (period.getDays() % 7 == 0) {
-                int weeks = Math.abs(period.getDays()) / 7;
-                if (period.getDays() < 0) {
-                    weeks = -weeks;
+            retVal = periodToString(((Period) duration).normalized());
+        } else {
+            retVal = durationToString((Duration) duration);
+        }
+        return  retVal;
+    }
+
+    /**
+     * As the {@link Period} implementation doesn't support string representation in weeks, but does support
+     * years and months, we need to generate a string that converts years, months and days to weeks.
+     *
+     * @param period a period instance
+     * @return a string representation of the period that is compliant with the RFC5545 specification.
+     */
+    private String periodToString(Period period) {
+        String retVal;
+        if (period.getYears() > 0) {
+            int weeks = Math.abs(period.getYears()) * 52;
+            if (period.getYears() < 0) {
+                weeks = -weeks;
+            }
+            retVal = String.format("P%dW", weeks);
+        } else if (period.getMonths() > 0) {
+            int weeks = Math.abs(period.getMonths()) * 4;
+            if (period.getMonths() < 0) {
+                weeks = -weeks;
+            }
+            retVal = String.format("P%dW", weeks);
+        } else if (period.getDays() % 7 == 0) {
+            int weeks = Math.abs(period.getDays()) / 7;
+            if (period.getDays() < 0) {
+                weeks = -weeks;
+            }
+            retVal = String.format("P%dW", weeks);
+        } else {
+            retVal = period.toString();
+        }
+        return retVal;
+    }
+
+    /**
+     * As the {@link Duration} implementation doesn't support string representation in days (to avoid
+     * confusion with {@link Period}), we need to generate a string that does support days.
+     *
+     * @param duration a duration instance
+     * @return a string representation of the duration that is compliant with the RFC5545 specification.
+     */
+    private String durationToString(Duration duration) {
+        String retVal = null;
+        Duration absDuration = duration.abs();
+        int days = 0;
+        if (absDuration.getSeconds() != 0) {
+            days = (int) absDuration.getSeconds() / (24 * 60 * 60);
+        }
+
+        if (days != 0) {
+            Duration durationMinusDays = absDuration.minusDays(days);
+            if (durationMinusDays.getSeconds() != 0) {
+                int hours = (int) durationMinusDays.getSeconds() / (60 * 60);
+                int minutes = (int) durationMinusDays.minusHours(hours).getSeconds() / 60;
+                int seconds = (int) durationMinusDays.minusHours(hours).minusMinutes(minutes).getSeconds();
+                if (hours > 0) {
+                    if (seconds > 0) {
+                        retVal = String.format("P%dDT%dH%dM%dS", days, hours, minutes, seconds);
+                    } else {
+                        retVal = String.format("P%dDT%dH", days, hours);
+                    }
+                } else if (minutes > 0) {
+                    retVal = String.format("P%dDT%dM", days, minutes);
+                } else if (seconds > 0) {
+                    retVal = String.format("P%dDT%dS", days, seconds);
                 }
-                retVal = String.format("P%dW", weeks);
             } else {
-                retVal = duration.toString();
+                retVal = String.format("P%dD", days);
             }
         } else {
-            retVal = duration.toString();
+            retVal = absDuration.toString();
+        }
+
+        if (duration.isNegative()) {
+            return "-" + retVal;
+        } else {
+            return retVal;
         }
-        return  retVal;
     }
 
     public static TemporalAmountAdapter parse(String value) {
@@ -75,21 +134,33 @@ public static TemporalAmountAdapter fromDateRange(Date start, Date end) {
         return new TemporalAmountAdapter(duration);
     }
 
-    public static TemporalAmount from(Dur duration) {
-        if (duration.getWeeks() > 0) {
-            Period p = Period.ofWeeks(duration.getWeeks());
-            if (duration.isNegative()) {
+    public static TemporalAmountAdapter from(Dur dur) {
+        TemporalAmount duration;
+        if (dur.getWeeks() > 0) {
+            Period p = Period.ofWeeks(dur.getWeeks());
+            if (dur.isNegative()) {
                 p = p.negated();
             }
-            return p;
-        }
-        Duration d = Duration.ofDays(duration.getDays())
-                .plusHours(duration.getHours())
-                .plusMinutes(duration.getMinutes())
-                .plusSeconds(duration.getSeconds());
-        if (duration.isNegative()) {
-            d = d.negated();
+            duration = p;
+        } else {
+            Duration d = Duration.ofDays(dur.getDays())
+                    .plusHours(dur.getHours())
+                    .plusMinutes(dur.getMinutes())
+                    .plusSeconds(dur.getSeconds());
+            if (dur.isNegative()) {
+                d = d.negated();
+            }
+            duration = d;
         }
-        return d;
+        return new TemporalAmountAdapter(duration);
+    }
+
+    /**
+     * Returns a date representing the end of this duration from the specified start date.
+     * @param start the date to start the duration
+     * @return the end of the duration as a date
+     */
+    public final Date getTime(final Date start) {
+        return Date.from(Instant.from(duration.addTo(start.toInstant())));
     }
 }

src/main/java/net/fortuna/ical4j/model/property/Duration.java

@@ -132,7 +132,7 @@ public Duration(final ParameterList aList, final String aValue) {
      */
     @Deprecated
     public Duration(final Dur duration) {
-        this(TemporalAmountAdapter.from(duration));
+        this(TemporalAmountAdapter.from(duration).getDuration());
     }
 
     /**
@@ -149,7 +149,7 @@ public Duration(final TemporalAmount duration) {
      */
     @Deprecated
     public Duration(final ParameterList aList, final Dur duration) {
-        this(aList, TemporalAmountAdapter.from(duration));
+        this(aList, TemporalAmountAdapter.from(duration).getDuration());
     }
 
     /**

src/main/java/net/fortuna/ical4j/model/property/Trigger.java

@@ -170,7 +170,7 @@ public Trigger(final ParameterList aList, final String aValue) {
      */
     @Deprecated
     public Trigger(final Dur duration) {
-        this(TemporalAmountAdapter.from(duration));
+        this(TemporalAmountAdapter.from(duration).getDuration());
     }
 
     /**
@@ -187,7 +187,7 @@ public Trigger(final TemporalAmount duration) {
      */
     @Deprecated
     public Trigger(final ParameterList aList, final Dur duration) {
-        this(aList, TemporalAmountAdapter.from(duration));
+        this(aList, TemporalAmountAdapter.from(duration).getDuration());
     }
 
     /**

src/test/groovy/net/fortuna/ical4j/model/TemporalAmountAdapterTest.groovy

@@ -1,6 +1,7 @@
 package net.fortuna.ical4j.model
 
 import spock.lang.Specification
+import spock.lang.Unroll
 
 import java.time.Duration
 
@@ -13,18 +14,20 @@ class TemporalAmountAdapterTest extends Specification {
         where:
         duration                    | expectedValue
         Duration.ofHours(4)         | "PT4H"
-        Duration.ofDays(12)         | "PT288H"
+        Duration.ofHours(-4)         | "-PT4H"
+        Duration.ofDays(12)         | "P12D"
         java.time.Period.ofDays(12) | "P12D"
         java.time.Period.ofWeeks(7) | "P7W"
         java.time.Period.ofDays(365) | "P365D"
         java.time.Period.ofDays(364) | "P52W"
         java.time.Period.ofYears(1) | "P52W"
         java.time.Period.ofMonths(6) | "P24W"
+        Duration.ofDays(15).plusHours(5).plusSeconds(20)    | 'P15DT5H0M20S'
     }
 
     def 'verify temporalamount creation'() {
         expect:
-        TemporalAmountAdapter.from(duration) == expectedTemporalAmount
+        TemporalAmountAdapter.from(duration).duration == expectedTemporalAmount
 
         where:
         duration       | expectedTemporalAmount
@@ -33,4 +36,101 @@ class TemporalAmountAdapterTest extends Specification {
         new Dur(5)  | java.time.Period.ofWeeks(5)
         new Dur(-9)  | java.time.Period.ofWeeks(-9)
     }
+
+    @Unroll
+    def 'validate string representation: #dur'() {
+        expect: 'derived string representation equals expected'
+        dur.toString() == expectedString
+
+        where:
+        dur						| expectedString
+        TemporalAmountAdapter.from(new Dur(33))				| 'P33W'
+        TemporalAmountAdapter.from(new Dur('-P2D'))			| '-P2D'
+        TemporalAmountAdapter.from(new Dur(-2, 0, 0, 0))	| '-P2D'
+    }
+
+    @Unroll
+    def 'verify duration plus time operations: #duration'() {
+        expect: 'derived end time value equals expected'
+        TemporalAmountAdapter.from(new Dur(duration)).getTime(new DateTime(start)) == new DateTime(expectedEnd)
+
+        where:
+        duration	| start				| expectedEnd
+        'P1D' | '20110326T110000' | '20110327T110000'
+    }
+
+    @Unroll
+    def 'verify duration plus time operations in different timezones: #duration'() {
+        setup: 'initialise timezone registry'
+        def tzRegistry = TimeZoneRegistryFactory.instance.createRegistry()
+
+        expect: 'derived end time value equals expected'
+        def tz = tzRegistry.getTimeZone(timezone)
+        TemporalAmountAdapter.from(new Dur(duration)).getTime(new DateTime(start, tz)) == new DateTime(expectedEnd, tz)
+
+        where:
+        duration	| timezone					| start				| expectedEnd
+        'P1D' | 'America/Los_Angeles' | '20110326T110000' | '20110327T110000'
+    }
+
+    @Unroll
+    def 'verify duration plus time operations in different timezones with overridden platform default: #duration'() {
+        setup: 'override platform default timezone'
+        def originalPlatformTz = TimeZone.default
+        TimeZone.default = TimeZone.getTimeZone('Europe/Paris')
+
+        and: 'initialise timezone registry'
+        def tzRegistry = TimeZoneRegistryFactory.instance.createRegistry()
+
+        expect: 'derived end time value equals expected'
+        def tz = tzRegistry.getTimeZone(timezone)
+        TemporalAmountAdapter.from(new Dur(duration)).getTime(new DateTime(start, tz)) == new DateTime(expectedEnd, tz)
+
+        cleanup: 'restore platform default timezone'
+        TimeZone.default = originalPlatformTz
+
+        where:
+        duration	| timezone					| start				| expectedEnd
+        'P1D' | 'America/Los_Angeles' | '20110326T110000' | '20110327T110000'
+    }
+
+    @Unroll
+    def 'verify duration plus date operations: #duration'() {
+        expect: 'derived end date value equals expected'
+        TemporalAmountAdapter.from(new Dur(duration)).getTime(new Date(start)) == new Date(expectedEnd)
+
+        where:
+        duration	| start				| expectedEnd
+        'P1D' | '20110312' | '20110313'
+        'P1D' | '20110313' | '20110314'
+    }
+
+    @Unroll
+    def 'verify duration plus date operations with overriden platform default timezone: #duration'() {
+        setup: 'override platform default timezone'
+        def originalPlatformTz = TimeZone.default
+        TimeZone.default = TimeZone.getTimeZone('America/New_York')
+
+        expect: 'derived end date value equals expected'
+        TemporalAmountAdapter.from(new Dur(duration)).getTime(new Date(start)) == new Date(expectedEnd)
+
+        cleanup: 'restore platform default timezone'
+        TimeZone.default = originalPlatformTz
+
+        where:
+        duration	| start				| expectedEnd
+        'P1D' | '20110312' | '20110313'
+        'P1D' | '20110313' | '20110314'
+    }
+
+    def 'extension module test: plus'() {
+        expect:
+        TemporalAmountAdapter.from(new Dur('P1D')).duration + TemporalAmountAdapter.from(new Dur('P1D')).duration == TemporalAmountAdapter.from(new Dur('P2D')).duration
+    }
+
+    def 'extension module test: negative'() {
+        expect:
+        -TemporalAmountAdapter.from(new Dur('P1D')).duration == TemporalAmountAdapter.from(new Dur('-P1D')).duration
+    }
+
 }