Code review n°4

Signed-off-by: clinique <gael@lhopital.org>
openhab · May 25, 2024 · 71607df · 71607df
1 parent 843b9c4
commit 71607df
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 39 deletions.
diff --git a/...ng.ephemeris/src/main/java/org/openhab/binding/ephemeris/internal/EphemerisException.java b/...ng.ephemeris/src/main/java/org/openhab/binding/ephemeris/internal/EphemerisException.java
@@ -0,0 +1,36 @@
+/**
+ * Copyright (c) 2010-2024 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.ephemeris.internal;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.openhab.core.thing.ThingStatusDetail;
+
+/**
+ * Exception raised by Ephemeris Handlers
+ *
+ * @author Gaël L'hopital - Initial contribution
+ */
+@NonNullByDefault
+public class EphemerisException extends Exception {
+    private static final long serialVersionUID = -8813754360966576513L;
+    private final ThingStatusDetail statusDetail;
+
+    public EphemerisException(String message, ThingStatusDetail statusDetail) {
+        super(message);
+        this.statusDetail = statusDetail;
+    }
+
+    public ThingStatusDetail getStatusDetail() {
+        return statusDetail;
+    }
+}
diff --git a/...is/src/main/java/org/openhab/binding/ephemeris/internal/handler/BaseEphemerisHandler.java b/...is/src/main/java/org/openhab/binding/ephemeris/internal/handler/BaseEphemerisHandler.java
@@ -20,12 +20,11 @@
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.ephemeris.internal.EphemerisException;
 import org.openhab.core.ephemeris.EphemerisManager;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
-import org.openhab.core.thing.ThingStatusDetail;
 import org.openhab.core.thing.binding.BaseThingHandler;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.RefreshType;
@@ -72,20 +71,21 @@ private void updateData() {
         ZonedDateTime now = ZonedDateTime.now().withZoneSameLocal(zoneId);
 
         logger.debug("Updating {} channels", getThing().getUID());
-        String error = internalUpdate(now.truncatedTo(ChronoUnit.DAYS));
-        if (error == null) {
+        try {
+            internalUpdate(now.truncatedTo(ChronoUnit.DAYS));
+
             updateStatus(ThingStatus.ONLINE);
             ZonedDateTime nextUpdate = now.plusDays(1).withHour(REFRESH_FIRST_HOUR_OF_DAY)
                     .withMinute(REFRESH_FIRST_MINUTE_OF_DAY).truncatedTo(ChronoUnit.MINUTES);
             long delay = ChronoUnit.MINUTES.between(now, nextUpdate);
             logger.debug("Scheduling next {} update in {} minutes", getThing().getUID(), delay);
             refreshJob = Optional.of(scheduler.schedule(this::updateData, delay, TimeUnit.MINUTES));
-        } else {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.NONE, error);
+        } catch (EphemerisException e) {
+            updateStatus(ThingStatus.OFFLINE, e.getStatusDetail(), e.getMessage());
         }
     }
 
-    protected abstract @Nullable String internalUpdate(ZonedDateTime today);
+    protected abstract void internalUpdate(ZonedDateTime today) throws EphemerisException;
 
     @Override
     public void handleCommand(ChannelUID channelUID, Command command) {

diff --git a/...ephemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/CustomHandler.java b/...ephemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/CustomHandler.java
@@ -22,6 +22,7 @@
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.ephemeris.internal.EphemerisException;
 import org.openhab.binding.ephemeris.internal.configuration.FileConfiguration;
 import org.openhab.core.ephemeris.EphemerisManager;
 import org.openhab.core.library.types.OnOffType;
@@ -56,30 +57,25 @@ public void initialize() {
     }
 
     @Override
-    protected @Nullable String internalUpdate(ZonedDateTime today) {
+    protected void internalUpdate(ZonedDateTime today) throws EphemerisException {
         String event = getEvent(today);
         updateState(CHANNEL_EVENT_TODAY, OnOffType.from(event != null));
 
         event = getEvent(today.plusDays(1));
         updateState(CHANNEL_EVENT_TOMORROW, OnOffType.from(event != null));
 
-        // Thing could have been set to OFFLINE when calling getEvent
-        if (!thing.getStatus().equals(ThingStatus.ONLINE)) {
-            return null;
-        }
-        return super.internalUpdate(today);
+        super.internalUpdate(today);
     }
 
     @Override
-    protected @Nullable String getEvent(ZonedDateTime day) {
+    protected @Nullable String getEvent(ZonedDateTime day) throws EphemerisException {
         String path = definitionFile.get().getAbsolutePath();
         try {
             return ephemeris.getBankHolidayName(day, path);
         } catch (IllegalStateException e) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.NONE, "Incorrect syntax");
+            throw new EphemerisException("Incorrect syntax", ThingStatusDetail.NONE);
         } catch (FileNotFoundException e) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "File is absent: " + path);
+            throw new EphemerisException("File is absent: " + path, ThingStatusDetail.CONFIGURATION_ERROR);
         }
-        return null;
     }
 }
diff --git a/...ephemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/DaysetHandler.java b/...ephemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/DaysetHandler.java
@@ -18,7 +18,7 @@
 import java.time.ZonedDateTime;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.ephemeris.internal.EphemerisException;
 import org.openhab.binding.ephemeris.internal.configuration.DaysetConfiguration;
 import org.openhab.core.ephemeris.EphemerisManager;
 import org.openhab.core.library.types.OnOffType;
@@ -45,9 +45,8 @@ public void initialize() {
     }
 
     @Override
-    protected @Nullable String internalUpdate(ZonedDateTime today) {
+    protected void internalUpdate(ZonedDateTime today) throws EphemerisException {
         updateState(CHANNEL_TODAY, OnOffType.from(ephemeris.isInDayset(dayset, today)));
         updateState(CHANNEL_TOMORROW, OnOffType.from(ephemeris.isInDayset(dayset, today.plusDays(1))));
-        return null;
     }
 }
diff --git a/...phemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/HolidayHandler.java b/...phemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/HolidayHandler.java
@@ -16,11 +16,13 @@
 
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.ephemeris.internal.EphemerisException;
 import org.openhab.binding.ephemeris.internal.providers.EphemerisDescriptionProvider;
 import org.openhab.core.ephemeris.EphemerisManager;
 import org.openhab.core.library.types.OnOffType;
@@ -40,27 +42,29 @@ public HolidayHandler(Thing thing, EphemerisManager ephemerisManager, ZoneId zon
             EphemerisDescriptionProvider descriptionProvider) {
         super(thing, ephemerisManager, zoneId);
 
-        // Search all holidays in the coming year
-        List<StateOption> events = new ArrayList<>();
+        // Search all holidays in the coming year, using a map to avoid duplicates
+        Map<String, StateOption> events = new HashMap<>();
         ZonedDateTime now = ZonedDateTime.now();
-        for (int offset = 0; offset < 366; offset++) {
-            String nextEvent = getEvent(now.plusDays(offset));
-            if (nextEvent != null) {
-                String description = ephemeris.getHolidayDescription(nextEvent);
-                events.add(new StateOption(nextEvent, description == null ? nextEvent : description));
+        // Scans 13 monthes to be sure to catch mobile holidays
+        for (int offset = 0; offset < 398; offset++) {
+            String event = getEvent(now.plusDays(offset));
+            if (event != null) {
+                String description = ephemeris.getHolidayDescription(event);
+                events.put(event, new StateOption(event, description == null ? event : description));
             }
         }
 
         // Set descriptions for these events
-        descriptionProvider.setStateOptions(new ChannelUID(thing.getUID(), CHANNEL_CURRENT_EVENT), events);
-        descriptionProvider.setStateOptions(new ChannelUID(thing.getUID(), CHANNEL_NEXT_EVENT), events);
+        List<StateOption> stateOptions = events.values().stream().toList();
+        descriptionProvider.setStateOptions(new ChannelUID(thing.getUID(), CHANNEL_CURRENT_EVENT), stateOptions);
+        descriptionProvider.setStateOptions(new ChannelUID(thing.getUID(), CHANNEL_NEXT_EVENT), stateOptions);
     }
 
     @Override
-    protected @Nullable String internalUpdate(ZonedDateTime today) {
+    protected void internalUpdate(ZonedDateTime today) throws EphemerisException {
         updateState(CHANNEL_HOLIDAY_TODAY, OnOffType.from(getEvent(today) != null));
         updateState(CHANNEL_HOLIDAY_TOMORROW, OnOffType.from(getEvent(today.plusDays(1)) != null));
-        return super.internalUpdate(today);
+        super.internalUpdate(today);
     }
 
     @Override

diff --git a/...hemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/JollydayHandler.java b/...hemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/JollydayHandler.java
@@ -20,6 +20,7 @@
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.ephemeris.internal.EphemerisException;
 import org.openhab.core.ephemeris.EphemerisManager;
 import org.openhab.core.library.types.DateTimeType;
 import org.openhab.core.library.types.QuantityType;
@@ -42,7 +43,7 @@ public JollydayHandler(Thing thing, EphemerisManager ephemerisManager, ZoneId zo
     }
 
     @Override
-    protected @Nullable String internalUpdate(ZonedDateTime today) {
+    protected void internalUpdate(ZonedDateTime today) throws EphemerisException {
         String todayEvent = getEvent(today);
         updateState(CHANNEL_CURRENT_EVENT, toStringType(todayEvent));
 
@@ -55,12 +56,13 @@ public JollydayHandler(Thing thing, EphemerisManager ephemerisManager, ZoneId zo
         }
 
         updateState(CHANNEL_NEXT_EVENT, toStringType(nextEvent));
-        updateState(CHANNEL_NEXT_REMAINING, new QuantityType<>(Duration.between(today, nextDay).toDays(), Units.DAY));
+        updateState(CHANNEL_NEXT_REMAINING,
+                nextEvent != null ? new QuantityType<>(Duration.between(today, nextDay).toDays(), Units.DAY)
+                        : UnDefType.UNDEF);
         updateState(CHANNEL_NEXT_START, nextEvent != null ? new DateTimeType(nextDay) : UnDefType.UNDEF);
-        return null;
     }
 
-    protected abstract @Nullable String getEvent(ZonedDateTime day);
+    protected abstract @Nullable String getEvent(ZonedDateTime day) throws EphemerisException;
 
     protected State toStringType(@Nullable String event) {
         return event == null || event.isEmpty() ? UnDefType.NULL : new StringType(event);

diff --git a/...phemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/WeekendHandler.java b/...phemeris/src/main/java/org/openhab/binding/ephemeris/internal/handler/WeekendHandler.java
@@ -18,7 +18,7 @@
 import java.time.ZonedDateTime;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.ephemeris.internal.EphemerisException;
 import org.openhab.core.ephemeris.EphemerisManager;
 import org.openhab.core.library.types.OnOffType;
 import org.openhab.core.thing.Thing;
@@ -36,9 +36,8 @@ public WeekendHandler(Thing thing, EphemerisManager ephemerisManager, ZoneId zon
     }
 
     @Override
-    protected @Nullable String internalUpdate(ZonedDateTime today) {
+    protected void internalUpdate(ZonedDateTime today) throws EphemerisException {
         updateState(CHANNEL_TODAY, OnOffType.from(ephemeris.isWeekend(today)));
         updateState(CHANNEL_TOMORROW, OnOffType.from(ephemeris.isWeekend(today.plusDays(1))));
-        return null;
     }
 }