Refactor timestamps to Instant in discovery methods (#4866)

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractDiscoveryService.java

@@ -352,8 +352,22 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
      * as timestamp.
      *
      * @param timestamp timestamp, older results will be removed
+     * @deprecated Use {@link #removeOlderResults(Instant)} instead.
      */
+    @Deprecated(since = "5.0", forRemoval = true)
     protected void removeOlderResults(long timestamp) {
+        removeOlderResults(Instant.ofEpochMilli(timestamp));
+    }
+
+    /**
+     * Call to remove all results of all {@link #supportedThingTypes} that are
+     * older than the given timestamp. To remove all left over results after a
+     * full scan, this method could be called {@link #getTimestampOfLastScan()}
+     * as timestamp.
+     *
+     * @param timestamp timestamp, older results will be removed
+     */
+    protected void removeOlderResults(Instant timestamp) {
         removeOlderResults(timestamp, null, null);
     }
 
@@ -365,8 +379,23 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
      *
      * @param timestamp timestamp, older results will be removed
      * @param bridgeUID if not {@code null} only results of that bridge are being removed
+     * @deprecated Use {@link #removeOlderResults(Instant, ThingUID)} instead.
      */
+    @Deprecated(since = "5.0", forRemoval = true)
     protected void removeOlderResults(long timestamp, @Nullable ThingUID bridgeUID) {
+        removeOlderResults(Instant.ofEpochMilli(timestamp), null);
+    }
+
+    /**
+     * Call to remove all results of all {@link #supportedThingTypes} that are
+     * older than the given timestamp. To remove all left over results after a
+     * full scan, this method could be called {@link #getTimestampOfLastScan()}
+     * as timestamp.
+     *
+     * @param timestamp timestamp, older results will be removed
+     * @param bridgeUID if not {@code null} only results of that bridge are being removed
+     */
+    protected void removeOlderResults(Instant timestamp, @Nullable ThingUID bridgeUID) {
         removeOlderResults(timestamp, null, bridgeUID);
     }
 
@@ -381,9 +410,28 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
      *            {@link DiscoveryService#getSupportedThingTypes()} will be used
      *            instead
      * @param bridgeUID if not {@code null} only results of that bridge are being removed
+     * @deprecated Use {@link #removeOlderResults(Instant, Collection, ThingUID)} instead.
      */
+    @Deprecated(since = "5.0", forRemoval = true)
     protected void removeOlderResults(long timestamp, @Nullable Collection<ThingTypeUID> thingTypeUIDs,
             @Nullable ThingUID bridgeUID) {
+        removeOlderResults(Instant.ofEpochMilli(timestamp), thingTypeUIDs, bridgeUID);
+    }
+
+    /**
+     * Call to remove all results of the given types that are older than the
+     * given timestamp. To remove all left over results after a full scan, this
+     * method could be called {@link #getTimestampOfLastScan()} as timestamp.
+     *
+     * @param timestamp timestamp, older results will be removed
+     * @param thingTypeUIDs collection of {@code ThingType}s, only results of these
+     *            {@code ThingType}s will be removed; if {@code null} then
+     *            {@link DiscoveryService#getSupportedThingTypes()} will be used
+     *            instead
+     * @param bridgeUID if not {@code null} only results of that bridge are being removed
+     */
+    protected void removeOlderResults(Instant timestamp, @Nullable Collection<ThingTypeUID> thingTypeUIDs,
+            @Nullable ThingUID bridgeUID) {
         Collection<ThingUID> removedThings = null;
 
         Collection<ThingTypeUID> toBeRemoved = thingTypeUIDs != null ? thingTypeUIDs : getSupportedThingTypes();
@@ -484,10 +532,10 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
     /**
      * Get the timestamp of the last call of {@link #startScan()}.
      *
-     * @return timestamp as long
+     * @return timestamp as {@link Instant}
      */
-    protected long getTimestampOfLastScan() {
-        return Instant.MIN.equals(timestampOfLastScan) ? 0 : timestampOfLastScan.toEpochMilli();
+    protected Instant getTimestampOfLastScan() {
+        return timestampOfLastScan;
     }
 
     private String inferKey(DiscoveryResult discoveryResult, String lastSegment) {

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryListener.java

@@ -12,6 +12,7 @@
  */
 package org.openhab.core.config.discovery;
 
+import java.time.Instant;
 import java.util.Collection;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -72,6 +73,28 @@ public interface DiscoveryListener {
      * @return collection of thing UIDs of all removed things
      */
     @Nullable
+    default Collection<ThingUID> removeOlderResults(DiscoveryService source, Instant timestamp,
+            @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID) {
+        return removeOlderResults(source, timestamp.toEpochMilli(), thingTypeUIDs, bridgeUID);
+    }
+
+    /**
+     * Removes all results belonging to one of the given types that are older
+     * than the given timestamp.
+     *
+     * @param source the discovery service which is the source of this event (not
+     *            null)
+     * @param timestamp timestamp, all <b>older</b> results will be removed
+     * @param thingTypeUIDs collection of {@code ThingType}s, only results of these
+     *            {@code ThingType}s will be removed; if {@code null} then
+     *            {@link DiscoveryService#getSupportedThingTypes()} will be used
+     *            instead
+     * @param bridgeUID if not {@code null} only results of that bridge are being removed
+     * @return collection of thing UIDs of all removed things
+     * @deprecated Use {@link #removeOlderResults(DiscoveryService, Instant, Collection, ThingUID)} instead.
+     */
+    @Nullable
+    @Deprecated(since = "5.0", forRemoval = true)
     Collection<ThingUID> removeOlderResults(DiscoveryService source, long timestamp,
             @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID);
 }

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryResult.java

@@ -12,6 +12,7 @@
  */
 package org.openhab.core.config.discovery;
 
+import java.time.Instant;
 import java.util.List;
 import java.util.Map;
 
@@ -121,10 +122,19 @@ public interface DiscoveryResult {
     ThingUID getBridgeUID();
 
     /**
-     * Returns the timestamp of this result object.
+     * Returns the creation time of this result object.
+     *
+     * @return timestamp
+     */
+    Instant getCreationTime();
+
+    /**
+     * Returns the creation time of this result object.
      *
      * @return timestamp as long
+     * @deprecated Use {@link #getCreationTime} instead.
      */
+    @Deprecated(since = "5.0", forRemoval = true)
     long getTimestamp();
 
     /**

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryResultImpl.java

@@ -216,7 +216,12 @@ public class DiscoveryResultImpl implements DiscoveryResult {
 
     @Override
     public long getTimestamp() {
-        return Instant.MIN.equals(timestamp) ? 0 : timestamp.toEpochMilli();
+        return getCreationTime().toEpochMilli();
+    }
+
+    @Override
+    public Instant getCreationTime() {
+        return timestamp;
     }
 
     @Override

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java

@@ -12,6 +12,7 @@
  */
 package org.openhab.core.config.discovery.internal;
 
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -289,6 +290,12 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
     @Override
     public @Nullable Collection<ThingUID> removeOlderResults(final DiscoveryService source, final long timestamp,
             final @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID) {
+        return removeOlderResults(source, Instant.ofEpochMilli(timestamp), thingTypeUIDs, bridgeUID);
+    }
+
+    @Override
+    public @Nullable Collection<ThingUID> removeOlderResults(final DiscoveryService source, final Instant timestamp,
+            final @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID) {
         Set<ThingUID> removedResults = new HashSet<>();
         for (final DiscoveryListener listener : listeners) {
             try {

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/PersistentInbox.java

@@ -126,7 +126,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
             if (ttl == DiscoveryResult.TTL_UNLIMITED) {
                 return false;
             }
-            return Instant.ofEpochMilli(result.getTimestamp()).plusSeconds(ttl).isBefore(now);
+            return result.getCreationTime().plusSeconds(ttl).isBefore(now);
         }
     }
 
@@ -414,18 +414,23 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
     @Override
     public @Nullable Collection<ThingUID> removeOlderResults(DiscoveryService source, long timestamp,
             @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID) {
+        return removeOlderResults(source, Instant.ofEpochMilli(timestamp), thingTypeUIDs, bridgeUID);
+    }
+
+    @Override
+    public @Nullable Collection<ThingUID> removeOlderResults(DiscoveryService source, Instant timestamp,
+            @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID) {
         Set<ThingUID> removedThings = new HashSet<>();
         for (DiscoveryResult discoveryResult : getAll()) {
             Class<?> discoverer = resultDiscovererMap.get(discoveryResult);
             if (thingTypeUIDs != null && thingTypeUIDs.contains(discoveryResult.getThingTypeUID())
-                    && discoveryResult.getTimestamp() < timestamp
+                    && discoveryResult.getCreationTime().isBefore(timestamp)
                     && (discoverer == null || source.getClass() == discoverer)) {
                 ThingUID thingUID = discoveryResult.getThingUID();
                 if (bridgeUID == null || bridgeUID.equals(discoveryResult.getBridgeUID())) {
                     removedThings.add(thingUID);
                     remove(thingUID);
-                    logger.debug("Removed thing '{}' from inbox because it was older than {}.", thingUID,
-                            Instant.ofEpochMilli(timestamp));
+                    logger.debug("Removed thing '{}' from inbox because it was older than {}.", thingUID, timestamp);
                 }
             }
         }

bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/console/InboxConsoleCommandExtension.java

@@ -14,7 +14,6 @@ package org.openhab.core.config.discovery.internal.console;
 
 import static org.openhab.core.config.discovery.inbox.InboxPredicates.*;
 
-import java.time.Instant;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -199,7 +198,7 @@ public class InboxConsoleCommandExtension extends AbstractConsoleCommandExtensio
             ThingUID bridgeId = discoveryResult.getBridgeUID();
             Map<String, Object> properties = discoveryResult.getProperties();
             String representationProperty = discoveryResult.getRepresentationProperty();
-            String timestamp = Instant.ofEpochMilli(discoveryResult.getTimestamp()).toString();
+            String timestamp = discoveryResult.getCreationTime().toString();
             String timeToLive = discoveryResult.getTimeToLive() == DiscoveryResult.TTL_UNLIMITED ? "UNLIMITED"
                     : "" + discoveryResult.getTimeToLive();
             console.println(String.format(

bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/AbstractDiscoveryServiceTest.java

@@ -17,6 +17,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.collection.IsMapContaining.hasEntry;
 import static org.junit.jupiter.api.Assertions.*;
 
+import java.time.Instant;
 import java.util.Collection;
 import java.util.Locale;
 import java.util.Map;
@@ -193,6 +194,12 @@ public class AbstractDiscoveryServiceTest implements DiscoveryListener {
     public void thingRemoved(DiscoveryService source, ThingUID thingUID) {
     }
 
+    @Override
+    public @Nullable Collection<ThingUID> removeOlderResults(DiscoveryService source, Instant timestamp,
+            @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID) {
+        return null;
+    }
+
     @Override
     public @Nullable Collection<ThingUID> removeOlderResults(DiscoveryService source, long timestamp,
             @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID) {

itests/org.openhab.core.config.discovery.tests/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistryOSGiTest.java

@@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
 
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -212,8 +213,8 @@ public class DiscoveryServiceRegistryOSGiTest extends JavaOSGiTest {
         discoveryServiceMockForBinding1.removeOlderResults(discoveryServiceMockForBinding1.getTimestampOfLastScan());
 
         waitForAssert(() -> {
-            verify(discoveryListenerMock, times(1)).removeOlderResults(any(DiscoveryService.class), anyLong(), any(),
-                    any());
+            verify(discoveryListenerMock, times(1)).removeOlderResults(any(DiscoveryService.class), any(Instant.class),
+                    any(), any());
         });
         verifyNoMoreInteractions(discoveryListenerMock);
     }

itests/org.openhab.core.config.discovery.tests/src/main/java/org/openhab/core/config/discovery/internal/InboxOSGiTest.java

@@ -1019,7 +1019,7 @@ public class InboxOSGiTest extends JavaOSGiTest {
     @Test
     public void assertThatRemoveOlderResultsOnlyRemovesResultsFromTheSameDiscoveryService() {
         inbox.thingDiscovered(discoveryService1, testDiscoveryResult);
-        long now = Instant.now().toEpochMilli() + 1;
+        Instant now = Instant.now().plusMillis(1);
         assertThat(inbox.getAll().size(), is(1));
 
         // should not remove a result
@@ -1034,7 +1034,7 @@ public class InboxOSGiTest extends JavaOSGiTest {
     @Test
     public void assertThatRemoveOlderResultsRemovesResultsWithoutAsource() {
         inbox.add(testDiscoveryResult);
-        long now = Instant.now().toEpochMilli() + 1;
+        Instant now = Instant.now().plusMillis(1);
         assertThat(inbox.getAll().size(), is(1));
 
         // should remove a result