[icalendar] Fix internal calculation for retrieving events for comman…

…d tags (#11178) * [icalendar] Fixes #11084: Different method for retrieving events Replaced retrieval of events for CommandTags by another, already implemented method, fixing wrong behaviour in case of moved or removed events. Also updated dependencies to get this binding resolvable again. Signed-Off-By: Michael Wodniok <michi@noorganization.org>
openhab · Sep 11, 2021 · 9628538 · 9628538
1 parent 7efc3e9
commit 9628538
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 58 deletions.
diff --git a/bundles/org.openhab.binding.icalendar/pom.xml b/bundles/org.openhab.binding.icalendar/pom.xml
@@ -17,12 +17,12 @@
     <dependency>
       <groupId>net.sf.biweekly</groupId>
       <artifactId>biweekly</artifactId>
-      <version>0.6.4</version>
+      <version>0.6.6</version>
       <scope>compile</scope>
       <exclusions>
         <exclusion>
           <groupId>com.fasterxml.jackson.core</groupId>
-          <artifactId>*</artifactId>
+          <artifactId>jackson-databind</artifactId>
         </exclusion>
       </exclusions>
     </dependency>
@@ -39,12 +39,6 @@
       <version>${jackson.version}</version>
       <scope>compile</scope>
     </dependency>
-    <dependency>
-      <groupId>com.fasterxml.jackson.core</groupId>
-      <artifactId>jackson-annotations</artifactId>
-      <version>${jackson.version}</version>
-      <scope>compile</scope>
-    </dependency>
     <dependency>
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-databind</artifactId>

diff --git a/...c/main/java/org/openhab/binding/icalendar/internal/logic/BiweeklyPresentableCalendar.java b/...c/main/java/org/openhab/binding/icalendar/internal/logic/BiweeklyPresentableCalendar.java
@@ -88,55 +88,14 @@ class BiweeklyPresentableCalendar extends AbstractPresentableCalendar {
 
     @Override
     public List<Event> getJustBegunEvents(Instant frameBegin, Instant frameEnd) {
-        final List<Event> eventList = new ArrayList<>();
-        // process all the events in the iCalendar
-        for (final VEvent event : usedCalendar.getEvents()) {
-            // iterate over all begin dates
-            final DateIterator begDates = getRecurredEventDateIterator(event);
-            while (begDates.hasNext()) {
-                final Instant begInst = begDates.next().toInstant();
-                if (begInst.isBefore(frameBegin)) {
-                    continue;
-                } else if (begInst.isAfter(frameEnd)) {
-                    break;
-                }
-                // fall through => means we are within the time frame
-                Duration duration = getEventLength(event);
-                if (duration == null) {
-                    duration = Duration.ofMinutes(1);
-                }
-                eventList.add(new VEventWPeriod(event, begInst, begInst.plus(duration)).toEvent());
-                break;
-            }
-        }
-        return eventList;
+        return this.getVEventWPeriodsBetween(frameBegin, frameEnd, 0).stream().map(e -> e.toEvent())
+                .collect(Collectors.toList());
     }
 
     @Override
     public List<Event> getJustEndedEvents(Instant frameBegin, Instant frameEnd) {
-        final List<Event> eventList = new ArrayList<>();
-        // process all the events in the iCalendar
-        for (final VEvent event : usedCalendar.getEvents()) {
-            final Duration duration = getEventLength(event);
-            if (duration == null) {
-                continue;
-            }
-            // iterate over all begin dates
-            final DateIterator begDates = getRecurredEventDateIterator(event);
-            while (begDates.hasNext()) {
-                final Instant begInst = begDates.next().toInstant();
-                final Instant endInst = begInst.plus(duration);
-                if (endInst.isBefore(frameBegin)) {
-                    continue;
-                } else if (endInst.isAfter(frameEnd)) {
-                    break;
-                }
-                // fall through => means we are within the time frame
-                eventList.add(new VEventWPeriod(event, begInst, endInst).toEvent());
-                break;
-            }
-        }
-        return eventList;
+        return this.getVEventWPeriodsBetween(frameBegin, frameEnd, 0, true).stream().map(e -> e.toEvent())
+                .collect(Collectors.toList());
     }
 
     @Override
@@ -247,23 +206,43 @@ public List<Event> getFilteredEventsBetween(Instant begin, Instant end, @Nullabl
      * @return All events which begin in the time frame.
      */
     private List<VEventWPeriod> getVEventWPeriodsBetween(Instant frameBegin, Instant frameEnd, int maximumPerSeries) {
+        return this.getVEventWPeriodsBetween(frameBegin, frameEnd, maximumPerSeries, false);
+    }
+
+    /**
+     * Finds events which begin in the given frame by end time and date
+     *
+     * @param frameBegin Begin of the frame where to search events.
+     * @param frameEnd End of the time frame where to search events. The Instant is inclusive when searchByEnd is true.
+     * @param maximumPerSeries Limit the results per series. Set to 0 for no limit.
+     * @param searchByEnd Whether to search by begin of the event or by end.
+     * @return All events which begin in the time frame.
+     */
+    private List<VEventWPeriod> getVEventWPeriodsBetween(Instant frameBegin, Instant frameEnd, int maximumPerSeries,
+            boolean searchByEnd) {
         final List<VEvent> positiveEvents = new ArrayList<>();
         final List<VEvent> negativeEvents = new ArrayList<>();
         classifyEvents(positiveEvents, negativeEvents);
 
         final List<VEventWPeriod> eventList = new ArrayList<>();
         for (final VEvent positiveEvent : positiveEvents) {
             final DateIterator positiveBeginDates = getRecurredEventDateIterator(positiveEvent);
-            positiveBeginDates.advanceTo(Date.from(frameBegin));
+            Duration duration = getEventLength(positiveEvent);
+            if (duration == null) {
+                duration = Duration.ZERO;
+            }
+            positiveBeginDates.advanceTo(Date.from(frameBegin.minus(searchByEnd ? duration : Duration.ZERO)));
             int foundInSeries = 0;
             while (positiveBeginDates.hasNext()) {
                 final Instant begInst = positiveBeginDates.next().toInstant();
-                if (begInst.isAfter(frameEnd) || begInst.equals(frameEnd)) {
+                if ((!searchByEnd && (begInst.isAfter(frameEnd) || begInst.equals(frameEnd)))
+                        || (searchByEnd && begInst.plus(duration).isAfter(frameEnd))) {
                     break;
                 }
-                Duration duration = getEventLength(positiveEvent);
-                if (duration == null) {
-                    duration = Duration.ZERO;
+                // biweekly is not as precise as java.time. An exact check is required.
+                if ((!searchByEnd && begInst.isBefore(frameBegin))
+                        || (searchByEnd && begInst.plus(duration).isBefore(frameBegin))) {
+                    continue;
                 }
 
                 final VEventWPeriod resultingVEWP = new VEventWPeriod(positiveEvent, begInst, begInst.plus(duration));

diff --git a/...st/java/org/openhab/binding/icalendar/internal/logic/BiweeklyPresentableCalendarTest.java b/...st/java/org/openhab/binding/icalendar/internal/logic/BiweeklyPresentableCalendarTest.java
@@ -49,6 +49,7 @@ public class BiweeklyPresentableCalendarTest {
     private AbstractPresentableCalendar calendar3;
     private AbstractPresentableCalendar calendar_issue9647;
     private AbstractPresentableCalendar calendar_issue10808;
+    private AbstractPresentableCalendar calendar_issue11084;
 
     @BeforeEach
     public void setUp() throws IOException, CalendarException {
@@ -59,6 +60,8 @@ public void setUp() throws IOException, CalendarException {
                 new FileInputStream("src/test/resources/test-issue9647.ics"));
         calendar_issue10808 = new BiweeklyPresentableCalendar(
                 new FileInputStream("src/test/resources/test-issue10808.ics"));
+        calendar_issue11084 = new BiweeklyPresentableCalendar(
+                new FileInputStream("src/test/resources/test-issue11084.ics"));
     }
 
     /**
@@ -132,6 +135,13 @@ public void testGetCurrentEvent() {
         Event currentEvent4 = calendar_issue10808.getCurrentEvent(Instant.parse("2021-06-05T17:18:05Z"));
         assertNotNull(currentEvent4);
         assertTrue("Test event 1".contentEquals(currentEvent4.title));
+
+        Event currentEvent5 = calendar_issue11084.getCurrentEvent(Instant.parse("2021-08-16T16:30:05Z"));
+        assertNull(currentEvent5);
+
+        Event currentEvent6 = calendar_issue11084.getCurrentEvent(Instant.parse("2021-08-16T16:45:05Z"));
+        assertNotNull(currentEvent6);
+        assertTrue("TEST_REPEATING_EVENT_3".contentEquals(currentEvent6.title));
     }
 
     /**
@@ -563,6 +573,17 @@ public void testCommandTagCode() {
         cmd7 = cmdTags.get(7).getCommand();
         assertNotNull(cmd7);
         assertEquals(DecimalType.class, cmd7.getClass());
+
+        // issue 11084: Command tags from moved events are also executed
+        List<Event> events2 = calendar_issue11084.getJustBegunEvents(Instant.parse("2021-08-16T16:29:55Z"),
+                Instant.parse("2021-08-16T17:00:05Z"));
+        assertEquals(1, events2.size());
+        assertEquals(Instant.parse("2021-08-16T16:45:00Z"), events2.get(0).start);
+
+        List<Event> events3 = calendar_issue11084.getJustEndedEvents(Instant.parse("2021-08-16T16:29:55Z"),
+                Instant.parse("2021-08-16T17:00:05Z"));
+        assertEquals(1, events3.size());
+        assertEquals(Instant.parse("2021-08-16T17:00:00Z"), events3.get(0).end);
     }
 
     @SuppressWarnings("null")
@@ -621,5 +642,9 @@ public void testGetFilteredEventsBetween() {
                 LocalDate.parse("2021-01-04").atStartOfDay(ZoneId.systemDefault()).toInstant(),
                 LocalDate.parse("2021-01-05").atStartOfDay(ZoneId.systemDefault()).toInstant(), null, 3);
         assertArrayEquals(expectedFilteredEvents8, realFilteredEvents8.toArray(new Event[] {}));
+
+        List<Event> realFilteredEvents9 = calendar_issue11084.getFilteredEventsBetween(
+                Instant.parse("2021-08-16T16:45:00.123456Z"), Instant.parse("2021-08-16T16:46:00.768643Z"), null, 3);
+        assertEquals(0, realFilteredEvents9.size());
     }
 }
diff --git a/bundles/org.openhab.binding.icalendar/src/test/resources/test-issue11084.ics b/bundles/org.openhab.binding.icalendar/src/test/resources/test-issue11084.ics
@@ -0,0 +1,56 @@
+BEGIN:VCALENDAR
+PRODID:-//Google Inc//Google Calendar 70.9054//EN
+VERSION:2.0
+CALSCALE:GREGORIAN
+METHOD:PUBLISH
+X-WR-CALNAME:ohtest
+X-WR-TIMEZONE:UTC
+BEGIN:VTIMEZONE
+TZID:Europe/Brussels
+X-LIC-LOCATION:Europe/Brussels
+BEGIN:DAYLIGHT
+TZOFFSETFROM:+0100
+TZOFFSETTO:+0200
+TZNAME:CEST
+DTSTART:19700329T020000
+RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU
+END:DAYLIGHT
+BEGIN:STANDARD
+TZOFFSETFROM:+0200
+TZOFFSETTO:+0100
+TZNAME:CET
+DTSTART:19701025T030000
+RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU
+END:STANDARD
+END:VTIMEZONE
+BEGIN:VEVENT
+DTSTART;TZID=Europe/Brussels:20210816T184500
+DTEND;TZID=Europe/Brussels:20210816T190000
+DTSTAMP:20210816T174418Z
+UID:pseudo7346893o7r8328zheh@google.com
+RECURRENCE-ID;TZID=Europe/Brussels:20210816T183000
+CREATED:20210816T161602Z
+DESCRIPTION:BEGIN:E_Test_Cal:ON
+LAST-MODIFIED:20210816T162009Z
+LOCATION:
+SEQUENCE:1
+STATUS:CONFIRMED
+SUMMARY:TEST_REPEATING_EVENT_3
+TRANSP:OPAQUE
+END:VEVENT
+BEGIN:VEVENT
+DTSTART;TZID=Europe/Brussels:20210816T183000
+DTEND;TZID=Europe/Brussels:20210816T184500
+RRULE:FREQ=DAILY
+DTSTAMP:20210816T174418Z
+UID:pseudo7346893o7r8328zheh@google.com
+CREATED:20210816T161602Z
+DESCRIPTION:BEGIN:E_Test_Cal:ON
+LAST-MODIFIED:20210816T162009Z
+LOCATION:
+SEQUENCE:0
+STATUS:CONFIRMED
+SUMMARY:TEST_REPEATING_EVENT_3
+TRANSP:OPAQUE
+END:VEVENT
+END:VCALENDAR