Skip to content

Commit

Permalink
Reivew comment: use TemporalAmount
Browse files Browse the repository at this point in the history 
Signed-off-by: Laurent Garnier <lg.hc@^free.fr>
  • Loading branch information
@lolodomo
lolodomo committed Nov 18, 2023
1 parent 4b0c00a commit ebe2964
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 128 deletions.
39 changes: 17 additions & 22 deletions ...es/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/chart/ChartServlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAmount;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -99,7 +100,7 @@ public class ChartServlet extends HttpServlet {
// The URI of this servlet
public static final String SERVLET_PATH = "/chart";

private static final Duration DEFAULT_DURATION = Duration.ofDays(1);
private static final Duration DEFAULT_PERIOD = Duration.ofDays(1);

protected static final Map<String, ChartProvider> CHART_PROVIDERS = new ConcurrentHashMap<>();

Expand Down Expand Up @@ -223,8 +224,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) throws Ser
}

// Read out the parameter period, begin and end and save them.
Period period = convertToPeriod(periodParam, Period.ZERO);
Duration duration = convertToDuration(periodParam, DEFAULT_DURATION);
TemporalAmount period = convertToTemporalAmount(periodParam, DEFAULT_PERIOD);
ZonedDateTime timeBegin = null;
ZonedDateTime timeEnd = null;

Expand All @@ -251,13 +251,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) throws Ser
// Set begin and end time and check legality.
if (timeBegin == null && timeEnd == null) {
timeEnd = ZonedDateTime.now(timeZoneProvider.getTimeZone());
timeBegin = timeEnd.minus(!period.isZero() ? period : duration);
timeBegin = timeEnd.minus(period);
logger.debug("No begin or end is specified, use now as end and now-period as begin.");
} else if (timeEnd == null) {
timeEnd = timeBegin.plus(!period.isZero() ? period : duration);
timeEnd = timeBegin.plus(period);
logger.debug("No end is specified, use begin + period as end.");
} else if (timeBegin == null) {
timeBegin = timeEnd.minus(!period.isZero() ? period : duration);
timeBegin = timeEnd.minus(period);
logger.debug("No begin is specified, use end-period as begin");
} else if (timeEnd.isBefore(timeBegin)) {
res.sendError(HttpServletResponse.SC_BAD_REQUEST, "The end is before the begin.");
Expand Down Expand Up @@ -351,30 +351,25 @@ public void init(@Nullable ServletConfig config) throws ServletException {
public void destroy() {
}

public static Period convertToPeriod(@Nullable String periodParam, Period defaultPeriod) {
Period period = defaultPeriod;
public static TemporalAmount convertToTemporalAmount(@Nullable String periodParam, TemporalAmount defaultPeriod) {
TemporalAmount period = defaultPeriod;
String convertedPeriod = convertPeriodToISO8601(periodParam);
if (convertedPeriod != null) {
boolean failed = false;
try {
period = Period.parse(convertedPeriod);
} catch (DateTimeParseException e) {
// Ignored
failed = true;
}
}
return period;
}

public static Duration convertToDuration(@Nullable String periodParam, Duration defaultDuration) {
Duration duration = defaultDuration;
String convertedPeriod = convertPeriodToISO8601(periodParam);
if (convertedPeriod != null) {
try {
duration = Duration.parse(convertedPeriod);
} catch (DateTimeParseException e) {
// Ignored
if (failed) {
try {
period = Duration.parse(convertedPeriod);
} catch (DateTimeParseException e) {
// Ignored
}
}
}
return duration;
return period;
}

private static @Nullable String convertPeriodToISO8601(@Nullable String period) {
Expand Down
165 changes: 59 additions & 106 deletions ...core.ui/src/test/java/org/openhab/core/ui/internal/chart/ChartServletPeriodParamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
*/
package org.openhab.core.ui.internal.chart;

import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.time.Duration;
import java.time.Period;
import java.time.temporal.ChronoUnit;
import java.time.temporal.TemporalAmount;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.Test;
Expand All @@ -27,128 +28,80 @@
public class ChartServletPeriodParamTest {

@Test
public void convertToPeriodFromNull() {
Period period = ChartServlet.convertToPeriod(null, Period.ZERO);
assertTrue(period.isZero());
public void convertToTemporalAmountFromNull() {
TemporalAmount period = ChartServlet.convertToTemporalAmount(null, Duration.ZERO);
assertEquals(0, period.get(ChronoUnit.SECONDS));
}

@Test
public void convertToPeriodFromHours() {
Period period = ChartServlet.convertToPeriod("2h", Period.ZERO);
assertTrue(period.isZero());
}

@Test
public void convertToPeriodFromDays() {
Period period = ChartServlet.convertToPeriod("D", Period.ZERO);
assertEquals(1, period.getDays());
assertEquals(0, period.getMonths());
assertEquals(0, period.getYears());

period = ChartServlet.convertToPeriod("4D", Period.ZERO);
assertEquals(4, period.getDays());
assertEquals(0, period.getMonths());
assertEquals(0, period.getYears());
}

@Test
public void convertToPeriodFromWeeks() {
Period period = ChartServlet.convertToPeriod("W", Period.ZERO);
assertEquals(7, period.getDays());
assertEquals(0, period.getMonths());
assertEquals(0, period.getYears());

period = ChartServlet.convertToPeriod("2W", Period.ZERO);
assertEquals(14, period.getDays());
assertEquals(0, period.getMonths());
assertEquals(0, period.getYears());
}
public void convertToTemporalAmountFromHours() {
TemporalAmount period = ChartServlet.convertToTemporalAmount("h", Duration.ZERO);
assertEquals(1 * 60 * 60, period.get(ChronoUnit.SECONDS));

@Test
public void convertToPeriodFromMonths() {
Period period = ChartServlet.convertToPeriod("M", Period.ZERO);
assertEquals(0, period.getDays());
assertEquals(1, period.getMonths());
assertEquals(0, period.getYears());

period = ChartServlet.convertToPeriod("3M", Period.ZERO);
assertEquals(0, period.getDays());
assertEquals(3, period.getMonths());
assertEquals(0, period.getYears());
period = ChartServlet.convertToTemporalAmount("12h", Duration.ZERO);
assertEquals(12 * 60 * 60, period.get(ChronoUnit.SECONDS));
}

@Test
public void convertToPeriodFromYears() {
Period period = ChartServlet.convertToPeriod("Y", Period.ZERO);
assertEquals(0, period.getDays());
assertEquals(0, period.getMonths());
assertEquals(1, period.getYears());

period = ChartServlet.convertToPeriod("2Y", Period.ZERO);
assertEquals(0, period.getDays());
assertEquals(0, period.getMonths());
assertEquals(2, period.getYears());
public void convertToTemporalAmountFromDays() {
TemporalAmount period = ChartServlet.convertToTemporalAmount("D", Duration.ZERO);
assertEquals(1, period.get(ChronoUnit.DAYS));
assertEquals(0, period.get(ChronoUnit.MONTHS));
assertEquals(0, period.get(ChronoUnit.YEARS));

period = ChartServlet.convertToTemporalAmount("4D", Duration.ZERO);
assertEquals(4, period.get(ChronoUnit.DAYS));
assertEquals(0, period.get(ChronoUnit.MONTHS));
assertEquals(0, period.get(ChronoUnit.YEARS));
}

@Test
public void convertToPeriodFromISO8601() {
Period period = ChartServlet.convertToPeriod("P2Y3M4D", Period.ZERO);
assertEquals(4, period.getDays());
assertEquals(3, period.getMonths());
assertEquals(2, period.getYears());

period = ChartServlet.convertToPeriod("P1DT12H30M15S", Period.ZERO);
assertTrue(period.isZero());
public void convertToTemporalAmountFromWeeks() {
TemporalAmount period = ChartServlet.convertToTemporalAmount("W", Duration.ZERO);
assertEquals(7, period.get(ChronoUnit.DAYS));
assertEquals(0, period.get(ChronoUnit.MONTHS));
assertEquals(0, period.get(ChronoUnit.YEARS));

period = ChartServlet.convertToTemporalAmount("2W", Duration.ZERO);
assertEquals(14, period.get(ChronoUnit.DAYS));
assertEquals(0, period.get(ChronoUnit.MONTHS));
assertEquals(0, period.get(ChronoUnit.YEARS));
}

@Test
public void convertToDurationFromNull() {
Duration duration = ChartServlet.convertToDuration(null, Duration.ZERO);
assertTrue(duration.isZero());
public void convertToTemporalAmountFromMonths() {
TemporalAmount period = ChartServlet.convertToTemporalAmount("M", Duration.ZERO);
assertEquals(0, period.get(ChronoUnit.DAYS));
assertEquals(1, period.get(ChronoUnit.MONTHS));
assertEquals(0, period.get(ChronoUnit.YEARS));

period = ChartServlet.convertToTemporalAmount("3M", Duration.ZERO);
assertEquals(0, period.get(ChronoUnit.DAYS));
assertEquals(3, period.get(ChronoUnit.MONTHS));
assertEquals(0, period.get(ChronoUnit.YEARS));
}

@Test
public void convertToDurationFromHours() {
Duration duration = ChartServlet.convertToDuration("h", Duration.ZERO);
assertEquals(1 * 60 * 60, duration.getSeconds());

duration = ChartServlet.convertToDuration("12h", Duration.ZERO);
assertEquals(12 * 60 * 60, duration.getSeconds());
public void convertToTemporalAmountFromYears() {
TemporalAmount period = ChartServlet.convertToTemporalAmount("Y", Duration.ZERO);
assertEquals(0, period.get(ChronoUnit.DAYS));
assertEquals(0, period.get(ChronoUnit.MONTHS));
assertEquals(1, period.get(ChronoUnit.YEARS));

period = ChartServlet.convertToTemporalAmount("2Y", Duration.ZERO);
assertEquals(0, period.get(ChronoUnit.DAYS));
assertEquals(0, period.get(ChronoUnit.MONTHS));
assertEquals(2, period.get(ChronoUnit.YEARS));
}

@Test
public void convertToDurationFromDays() {
Duration duration = ChartServlet.convertToDuration("D", Duration.ZERO);
assertEquals(24 * 60 * 60, duration.getSeconds());

duration = ChartServlet.convertToDuration("2D", Duration.ZERO);
assertEquals(48 * 60 * 60, duration.getSeconds());
}

@Test
public void convertToDurationFromWeeks() {
Duration duration = ChartServlet.convertToDuration("2W", Duration.ZERO);
assertTrue(duration.isZero());
}

@Test
public void convertToDurationFromMonths() {
Duration duration = ChartServlet.convertToDuration("3M", Duration.ZERO);
assertTrue(duration.isZero());
}

@Test
public void convertToDurationFromYears() {
Duration duration = ChartServlet.convertToDuration("2Y", Duration.ZERO);
assertTrue(duration.isZero());
}

@Test
public void convertToDurationFromISO8601() {
Duration duration = ChartServlet.convertToDuration("P1DT12H30M15S", Duration.ZERO);
assertEquals(36 * 60 * 60 + 30 * 60 + 15, duration.getSeconds());

duration = ChartServlet.convertToDuration("P2Y3M4D", Duration.ZERO);
assertTrue(duration.isZero());
public void convertToTemporalAmountFromISO8601() {
TemporalAmount period = ChartServlet.convertToTemporalAmount("P2Y3M4D", Duration.ZERO);
assertEquals(4, period.get(ChronoUnit.DAYS));
assertEquals(3, period.get(ChronoUnit.MONTHS));
assertEquals(2, period.get(ChronoUnit.YEARS));

period = ChartServlet.convertToTemporalAmount("P1DT12H30M15S", Duration.ZERO);
assertEquals(36 * 60 * 60 + 30 * 60 + 15, period.get(ChronoUnit.SECONDS));
}
}

0 comments on commit ebe2964

Please sign in to comment.