Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[discovery] Fix warnings #4065

Merged
merged 4 commits into from
Jan 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,10 @@ public synchronized void startScan(@Nullable ScanListener listener) {
// 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;
Expand All @@ -195,9 +196,10 @@ public synchronized void startScan(@Nullable ScanListener listener) {
try {
startScan();
} catch (Exception ex) {
scheduledStop = this.scheduledStop;
if (scheduledStop != null) {
scheduledStop.cancel(false);
scheduledStop = null;
this.scheduledStop = null;
}
scanListener = null;
throw ex;
Expand All @@ -208,9 +210,10 @@ public synchronized void startScan(@Nullable ScanListener listener) {
@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) {
Expand All @@ -233,9 +236,10 @@ public synchronized void abortScan() {
* 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;
}
}

Expand Down Expand Up @@ -471,8 +475,8 @@ private ParsedKey(String label) {
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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

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;

Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@
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}
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,14 @@

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static Predicate<DiscoveryResult> withFlag(DiscoveryResultFlag flag) {

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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@

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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@
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;

Expand Down Expand Up @@ -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!");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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;
Expand Down Expand Up @@ -84,6 +85,7 @@ public synchronized void onFinished() {
logger.debug("Finished {} of {} discovery services.", finishedDiscoveryServices,
numberOfDiscoveryServices);
if (!errorOccurred && finishedDiscoveryServices == numberOfDiscoveryServices) {
ScanListener listener = this.listener;
if (listener != null) {
listener.onFinished();
}
Expand All @@ -95,6 +97,7 @@ public synchronized void onFinished() {
public void onErrorOccurred(@Nullable Exception exception) {
synchronized (this) {
if (!errorOccurred) {
ScanListener listener = this.listener;
if (listener != null) {
listener.onErrorOccurred(exception);
}
Expand All @@ -115,6 +118,7 @@ public void reduceNumberOfDiscoveryServices() {
synchronized (this) {
numberOfDiscoveryServices--;
if (!errorOccurred && finishedDiscoveryServices == numberOfDiscoveryServices) {
ScanListener listener = this.listener;
if (listener != null) {
listener.onFinished();
}
Expand Down Expand Up @@ -247,7 +251,7 @@ public synchronized void removeDiscoveryListener(DiscoveryListener listener) thr
@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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,6 @@ protected void deactivate() {

@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");
Expand Down Expand Up @@ -373,22 +370,7 @@ public List<DiscoveryResult> getAll() {

@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
Expand Down Expand Up @@ -497,10 +479,7 @@ public void setFlag(ThingUID thingUID, @Nullable DiscoveryResultFlag flag) {
* 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) {
Expand Down Expand Up @@ -537,6 +516,7 @@ private void notifyListeners(DiscoveryResult result, EventType type) {
}

private void postEvent(DiscoveryResult result, EventType eventType) {
EventPublisher eventPublisher = this.eventPublisher;
if (eventPublisher != null) {
try {
switch (eventType) {
Expand Down Expand Up @@ -575,7 +555,7 @@ private void removeResultsForBridge(ThingUID bridgeUID) {
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());
}
}
Expand Down Expand Up @@ -609,10 +589,8 @@ private void getPropsAndConfigParams(final DiscoveryResult discoveryResult, fina

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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@

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
Expand All @@ -36,19 +36,22 @@ 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");
assertThrows(IllegalArgumentException.class, () -> new DiscoveryResultImpl(thingTypeUID,
new ThingUID(thingTypeUID, "thingId"), null, null, null, null, -2));
}

@SuppressWarnings("deprecation")
@Test
public void testValidConstructor() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
Expand All @@ -66,6 +69,7 @@ public void testValidConstructor() {
assertNull(discoveryResult.getRepresentationProperty());
}

@SuppressWarnings("deprecation")
@Test
public void testInvalidSynchronize() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
Expand All @@ -85,6 +89,7 @@ public void testInvalidSynchronize() {
assertEquals(DiscoveryResultFlag.IGNORED, discoveryResult.getFlag());
}

@SuppressWarnings("deprecation")
@Test
public void testIrrelevantSynchronize() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
Expand All @@ -107,6 +112,7 @@ public void testIrrelevantSynchronize() {
assertEquals(DiscoveryResultFlag.IGNORED, discoveryResult.getFlag());
}

@SuppressWarnings("deprecation")
@Test
public void testSynchronize() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
Expand Down Expand Up @@ -135,6 +141,7 @@ public void testSynchronize() {
assertEquals(DiscoveryResultFlag.IGNORED, discoveryResult.getFlag());
}

@SuppressWarnings("deprecation")
@Test
public void testThingTypeCompatibility() {
ThingTypeUID thingTypeUID = new ThingTypeUID("bindingId", "thingType");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ public void testApproveNormalization() {
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"));
Expand All @@ -164,6 +166,8 @@ public void testApproveWithThingId() {
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"));
Expand Down