From 1d1cd673ed042a33adfffe081a0d821a1c759d42 Mon Sep 17 00:00:00 2001 From: Ben Fortuna Date: Fri, 28 Dec 2018 14:21:59 +1100 Subject: [PATCH] Improved string representation of temporal amount --- .../java/net/fortuna/ical4j/model/Period.java | 2 +- .../ical4j/model/TemporalAmountAdapter.java | 139 +++++++++++++----- .../ical4j/model/property/Duration.java | 4 +- .../ical4j/model/property/Trigger.java | 4 +- .../model/TemporalAmountAdapterTest.groovy | 104 ++++++++++++- 5 files changed, 212 insertions(+), 41 deletions(-) diff --git a/src/main/java/net/fortuna/ical4j/model/Period.java b/src/main/java/net/fortuna/ical4j/model/Period.java index 37d02d2e4..f87e79c47 100644 --- a/src/main/java/net/fortuna/ical4j/model/Period.java +++ b/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()); } /** diff --git a/src/main/java/net/fortuna/ical4j/model/TemporalAmountAdapter.java b/src/main/java/net/fortuna/ical4j/model/TemporalAmountAdapter.java index 6d21a53f5..ec9adea6b 100644 --- a/src/main/java/net/fortuna/ical4j/model/TemporalAmountAdapter.java +++ b/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()))); } } diff --git a/src/main/java/net/fortuna/ical4j/model/property/Duration.java b/src/main/java/net/fortuna/ical4j/model/property/Duration.java index b100cefcf..282ce9c56 100644 --- a/src/main/java/net/fortuna/ical4j/model/property/Duration.java +++ b/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()); } /** diff --git a/src/main/java/net/fortuna/ical4j/model/property/Trigger.java b/src/main/java/net/fortuna/ical4j/model/property/Trigger.java index 48a5d88ba..e10979cea 100644 --- a/src/main/java/net/fortuna/ical4j/model/property/Trigger.java +++ b/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()); } /** diff --git a/src/test/groovy/net/fortuna/ical4j/model/TemporalAmountAdapterTest.groovy b/src/test/groovy/net/fortuna/ical4j/model/TemporalAmountAdapterTest.groovy index e9400d45e..af7063268 100644 --- a/src/test/groovy/net/fortuna/ical4j/model/TemporalAmountAdapterTest.groovy +++ b/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 + } + }