demus/openhab-core
1
0
Fork 0
mirror of https://github.com/danieldemus/openhab-core.git synced 2025-01-10 13:21:53 +01:00
Code Issues 1 Packages Projects Releases Wiki Activity

[discovery] Fix warnings (#4065)

Browse Source 
* Remove unused imports and reference explicit namespaces in Javadoc links
* Fix potential null pointer access
* Remove redundant null checks and dead code
* Suppress deprecation warnings in tests

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
This commit is contained in:
Jacob Laursen 2024-01-27 18:14:43 +01:00 committed by GitHub
parent a9571228d2
commit 5b9fee7d36
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 49 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -173,9 +173,10 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
// we first stop any currently running scan and its scheduled stop
// call
stopScan();
ScheduledFuture<?> scheduledStop = this.scheduledStop;
if (scheduledStop != null) {
scheduledStop.cancel(false);
scheduledStop = null;
this.scheduledStop = null;
}
scanListener = listener;
@ -195,9 +196,10 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
try {
startScan();
} catch (Exception ex) {
scheduledStop = this.scheduledStop;
if (scheduledStop != null) {
scheduledStop.cancel(false);
scheduledStop = null;
this.scheduledStop = null;
}
scanListener = null;
throw ex;
@ -208,9 +210,10 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
@Override
public synchronized void abortScan() {
synchronized (this) {
ScheduledFuture<?> scheduledStop = this.scheduledStop;
if (scheduledStop != null) {
scheduledStop.cancel(false);
scheduledStop = null;
this.scheduledStop = null;
}
final ScanListener scanListener = this.scanListener;
if (scanListener != null) {
@ -233,9 +236,10 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
* This method cleans up after a scan, i.e. it removes listeners and other required operations.
*/
protected synchronized void stopScan() {
ScanListener scanListener = this.scanListener;
if (scanListener != null) {
scanListener.onFinished();
scanListener = null;
this.scanListener = null;
}
}
@ -471,8 +475,8 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
if (parts.length == 1) {
this.args = null;
} else {
this.args = Arrays.stream(parts[1].replaceAll("\\[|\\]|\"", "").split(","))
.filter(s -> s != null && !s.isBlank()).map(String::trim).toArray(Object[]::new);
this.args = Arrays.stream(parts[1].replaceAll("\\[|\\]|\"", "").split(",")).filter(s -> !s.isBlank())
.map(String::trim).toArray(Object[]::new);
}
}
}

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

@ -17,8 +17,6 @@ import java.util.Map;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.thing.Bridge;
import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.ThingUID;
@ -57,8 +55,8 @@ public interface DiscoveryResult {
/**
* Returns the unique {@code Thing} type ID of this result object.
* <p>
* A {@code Thing} type ID could be a product number which identifies the same type of {@link Thing}s. It's usually
* <i>not</i> a serial number.
* A {@code Thing} type ID could be a product number which identifies the same type of
* {@link org.openhab.core.thing.Thing}s. It's usually <i>not</i> a serial number.
*
* @return the unique Thing type
*/
@ -115,7 +113,7 @@ public interface DiscoveryResult {
String getLabel();
/**
* Returns the unique {@link Bridge} ID of this result object.
* Returns the unique {@link org.openhab.core.thing.Bridge} ID of this result object.
*
* @return the unique Bridge ID
*/

6
bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/inbox/Inbox.java
View File

@ -20,14 +20,12 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.config.discovery.DiscoveryResult;
import org.openhab.core.config.discovery.DiscoveryResultFlag;
import org.openhab.core.config.discovery.DiscoveryService;
import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingRegistry;
import org.openhab.core.thing.ThingUID;
/**
* The {@link Inbox} is a service interface providing a container for discovered {@code Thing}s
* (e.g. found by a {@link DiscoveryService}) as {@link DiscoveryResult}s.
* (e.g. found by a {@link org.openhab.core.config.discovery.DiscoveryService}) as {@link DiscoveryResult}s.
* <p>
* A {@link DiscoveryResult} entry in this container is not a full configured {@code Thing} and therefore no
* {@code Thing} exists for it. A {@link DiscoveryResult} can be marked to be ignored, so that a specific {@code Thing}
@ -126,7 +124,7 @@ public interface Inbox {
void removeInboxListener(@Nullable InboxListener listener);
/**
* Creates new {@link Thing} and adds it to the {@link ThingRegistry}.
* Creates new {@link Thing} and adds it to the {@link org.openhab.core.thing.ThingRegistry}.
*
* @param thingUID the UID of the Thing
* @param label the label of the Thing

9
bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/inbox/InboxAutoApprovePredicate.java
View File

@ -16,15 +16,14 @@ import java.util.function.Predicate;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.core.config.discovery.DiscoveryResult;
import org.openhab.core.config.discovery.internal.AutomaticInboxProcessor;
import org.osgi.service.component.annotations.Component;
/**
* {@link Component}s implementing this interface participate in the {@link AutomaticInboxProcessor}'s decision whether
* to automatically approve an inbox result or not.
* {@link org.osgi.service.component.annotations.Component}s implementing this interface participate in the
* {@link org.openhab.core.config.discovery.internal.AutomaticInboxProcessor}'s
* decision whether to automatically approve an inbox result or not.
* <p/>
* If this {@link Predicate} returns <code>true</code> the {@link DiscoveryResult} will be automatically approved by the
* {@link AutomaticInboxProcessor}.
* {@link org.openhab.core.config.discovery.internal.AutomaticInboxProcessor}.
* <p/>
* Note that if this {@link Predicate} returns <code>false</code> the {@link DiscoveryResult} might still be
* automatically approved (e.g., because another such {@link Predicate} returned <code>true</code>) - i.e., it is not

2
bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/inbox/InboxPredicates.java
View File

@ -47,7 +47,7 @@ public class InboxPredicates {
public static Predicate<DiscoveryResult> withProperty(@Nullable String propertyName, String propertyValue) {
return r -> r.getProperties().containsKey(propertyName)
&& r.getProperties().get(propertyName).equals(propertyValue);
&& propertyValue.equals(r.getProperties().get(propertyName));
}
public static Predicate<DiscoveryResult> withRepresentationProperty(@Nullable String propertyName) {

6
bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/inbox/events/AbstractInboxEvent.java
View File

@ -14,12 +14,12 @@ package org.openhab.core.config.discovery.inbox.events;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.core.config.discovery.dto.DiscoveryResultDTO;
import org.openhab.core.config.discovery.inbox.Inbox;
import org.openhab.core.events.AbstractEvent;
/**
* Abstract implementation of an inbox event which will be posted by the {@link Inbox} for added, removed
* and updated discovery results.
* Abstract implementation of an inbox event which will be posted by the
* {@link org.openhab.core.config.discovery.inbox.Inbox}
* for added, removed and updated discovery results.
*
* @author Stefan Bußweiler - Initial contribution
*/

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

@ -22,9 +22,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.common.AbstractUID;
import org.openhab.core.config.discovery.DiscoveryResult;
import org.openhab.core.config.discovery.DiscoveryResultBuilder;
import org.openhab.core.config.discovery.DiscoveryResultFlag;
import org.openhab.core.thing.Bridge;
import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.ThingUID;
@ -59,22 +57,19 @@ public class DiscoveryResultImpl implements DiscoveryResult {
* @param thingUID the {@link ThingUID} to be set. If a {@code Thing} disappears and is discovered again, the same
* {@code Thing} ID must be created. A typical {@code Thing} ID could be the serial number. It's usually
* <i>not</i> a product name.
* @param bridgeUID the unique {@link Bridge} ID to be set
* @param bridgeUID the unique {@link org.openhab.core.thing.Bridge} ID to be set
* @param properties the properties to be set
* @param representationProperty the representationProperty to be set
* @param label the human readable label to set
* @param timeToLive time to live in seconds
*
* @throws IllegalArgumentException if the {@link ThingUID} is null or the time to live is less than 1
* @deprecated use {@link DiscoveryResultBuilder} instead.
* @deprecated use {@link org.openhab.core.config.discovery.DiscoveryResultBuilder} instead.
*/
@Deprecated
public DiscoveryResultImpl(@Nullable ThingTypeUID thingTypeUID, ThingUID thingUID, @Nullable ThingUID bridgeUID,
@Nullable Map<String, Object> properties, @Nullable String representationProperty, @Nullable String label,
long timeToLive) throws IllegalArgumentException {
if (thingUID == null) {
throw new IllegalArgumentException("The thing UID must not be null!");
}
if (timeToLive < 1 && timeToLive != TTL_UNLIMITED) {
throw new IllegalArgumentException("The ttl must not be 0 or negative!");
}

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

@ -19,6 +19,7 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CopyOnWriteArraySet;
@ -84,6 +85,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
logger.debug("Finished {} of {} discovery services.", finishedDiscoveryServices,
numberOfDiscoveryServices);
if (!errorOccurred && finishedDiscoveryServices == numberOfDiscoveryServices) {
ScanListener listener = this.listener;
if (listener != null) {
listener.onFinished();
}
@ -95,6 +97,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
public void onErrorOccurred(@Nullable Exception exception) {
synchronized (this) {
if (!errorOccurred) {
ScanListener listener = this.listener;
if (listener != null) {
listener.onErrorOccurred(exception);
}
@ -115,6 +118,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
synchronized (this) {
numberOfDiscoveryServices--;
if (!errorOccurred && finishedDiscoveryServices == numberOfDiscoveryServices) {
ScanListener listener = this.listener;
if (listener != null) {
listener.onFinished();
}
@ -247,7 +251,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
@Override
public synchronized void thingDiscovered(final DiscoveryService source, final DiscoveryResult result) {
synchronized (cachedResults) {
cachedResults.computeIfAbsent(source, unused -> new HashSet<>()).add(result);
Objects.requireNonNull(cachedResults.computeIfAbsent(source, unused -> new HashSet<>())).add(result);
}
for (final DiscoveryListener listener : listeners) {
try {

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

@ -185,9 +185,6 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
@Override
public @Nullable Thing approve(ThingUID thingUID, @Nullable String label, @Nullable String newThingId) {
if (thingUID == null) {
throw new IllegalArgumentException("Thing UID must not be null");
}
List<DiscoveryResult> results = stream().filter(forThingUID(thingUID)).toList();
if (results.isEmpty()) {
throw new IllegalArgumentException("No Thing with UID " + thingUID.getAsString() + " in inbox");
@ -373,22 +370,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
@Override
public Stream<DiscoveryResult> stream() {
final Storage<DiscoveryResult> discoveryResultStorage = this.discoveryResultStorage;
if (discoveryResultStorage == null) {
final ScheduledFuture<?> timeToLiveChecker = this.timeToLiveChecker;
logger.error("The OSGi lifecycle has been violated (storage: {}, ttl checker cancelled: {}).",
this.discoveryResultStorage == null ? "null" : this.discoveryResultStorage,
timeToLiveChecker == null ? "null" : timeToLiveChecker.isCancelled());
return Stream.empty();
}
final Collection<@Nullable DiscoveryResult> values = discoveryResultStorage.getValues();
if (values == null) {
logger.warn(
"The storage service violates the nullness requirements (get values must not return null) (storage class: {}).",
discoveryResultStorage.getClass());
return Stream.empty();
}
return (Stream<DiscoveryResult>) values.stream().filter(Objects::nonNull);
return (Stream<DiscoveryResult>) discoveryResultStorage.getValues().stream().filter(Objects::nonNull);
}
@Override
@ -497,10 +479,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
* null, if no discovery result could be found
*/
private @Nullable DiscoveryResult get(ThingUID thingUID) {
if (thingUID != null) {
return discoveryResultStorage.get(thingUID.toString());
}
return null;
return discoveryResultStorage.get(thingUID.toString());
}
private void notifyListeners(DiscoveryResult result, EventType type) {
@ -537,6 +516,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
}
private void postEvent(DiscoveryResult result, EventType eventType) {
EventPublisher eventPublisher = this.eventPublisher;
if (eventPublisher != null) {
try {
switch (eventType) {
@ -575,7 +555,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
private List<ThingUID> getResultsForBridge(ThingUID bridgeUID) {
List<ThingUID> thingsForBridge = new ArrayList<>();
for (DiscoveryResult result : discoveryResultStorage.getValues()) {
if (bridgeUID.equals(result.getBridgeUID())) {
if (result != null && bridgeUID.equals(result.getBridgeUID())) {
thingsForBridge.add(result.getThingUID());
}
}
@ -609,10 +589,8 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
private Set<String> getConfigDescParamNames(List<ConfigDescriptionParameter> configDescParams) {
Set<String> paramNames = new HashSet<>();
if (configDescParams != null) {
for (ConfigDescriptionParameter param : configDescParams) {
paramNames.add(param.getName());
}
for (ConfigDescriptionParameter param : configDescParams) {
paramNames.add(param.getName());
}
return paramNames;
}

11
bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/internal/DiscoveryResultImplTest.java
View File

@ -18,14 +18,14 @@ import java.util.Map;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.Test;
import org.openhab.core.config.discovery.DiscoveryResult;
import org.openhab.core.config.discovery.DiscoveryResultFlag;
import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.ThingUID;
/**
* The {@link DiscoveryResultImplTest} checks if any invalid input parameters
* and the synchronization of {@link DiscoveryResult}s work in a correct way.
* and the synchronization of {@link org.openhab.core.config.discovery.DiscoveryResult}s
* work in a correct way.
*
* @author Michael Grammling - Initial contribution
* @author Thomas Höfer - Added representation
@ -36,12 +36,14 @@ public class DiscoveryResultImplTest {
private static final int DEFAULT_TTL = 60;
@SuppressWarnings("deprecation")
@Test
public void testInvalidConstructorForThingType() {
assertThrows(IllegalArgumentException.class,
() -> new DiscoveryResultImpl(null, new ThingUID("aa"), null, null, null, null, DEFAULT_TTL));
}
@SuppressWarnings("deprecation")
@Test
public void testInvalidConstructorForTTL() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
@ -49,6 +51,7 @@ public class DiscoveryResultImplTest {
new ThingUID(thingTypeUID, "thingId"), null, null, null, null, -2));
}
@SuppressWarnings("deprecation")
@Test
public void testValidConstructor() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
@ -66,6 +69,7 @@ public class DiscoveryResultImplTest {
assertNull(discoveryResult.getRepresentationProperty());
}
@SuppressWarnings("deprecation")
@Test
public void testInvalidSynchronize() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
@ -85,6 +89,7 @@ public class DiscoveryResultImplTest {
assertEquals(DiscoveryResultFlag.IGNORED, discoveryResult.getFlag());
}
@SuppressWarnings("deprecation")
@Test
public void testIrrelevantSynchronize() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
@ -107,6 +112,7 @@ public class DiscoveryResultImplTest {
assertEquals(DiscoveryResultFlag.IGNORED, discoveryResult.getFlag());
}
@SuppressWarnings("deprecation")
@Test
public void testSynchronize() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
@ -135,6 +141,7 @@ public class DiscoveryResultImplTest {
assertEquals(DiscoveryResultFlag.IGNORED, discoveryResult.getFlag());
}
@SuppressWarnings("deprecation")
@Test
public void testThingTypeCompatibility() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");

4
bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/internal/PersistentInboxTest.java
View File

@ -150,6 +150,8 @@ public class PersistentInboxTest {
inbox.activate();
inbox.approve(THING_UID, "Test", null);
Thing lastAddedThing = this.lastAddedThing;
assertNotNull(lastAddedThing);
assertEquals(THING_UID, lastAddedThing.getUID());
assertInstanceOf(String.class, lastAddedThing.getConfiguration().get("foo"));
assertEquals("3", lastAddedThing.getConfiguration().get("foo"));
@ -164,6 +166,8 @@ public class PersistentInboxTest {
inbox.activate();
inbox.approve(THING_UID, "Test", THING_OTHER_ID);
Thing lastAddedThing = this.lastAddedThing;
assertNotNull(lastAddedThing);
assertEquals(THING_OTHER_UID, lastAddedThing.getUID());
assertInstanceOf(String.class, lastAddedThing.getConfiguration().get("foo"));
assertEquals("3", lastAddedThing.getConfiguration().get("foo"));