[discovery] Minor clean-ups for discovery (#1539)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
This commit is contained in:
Christoph Weitkamp 2020-07-09 23:17:33 +02:00 committed by GitHub
parent a142e6746e
commit 1166dc4677
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 88 additions and 94 deletions

View File

@ -137,7 +137,7 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
*/
@Override
public Set<ThingTypeUID> getSupportedThingTypes() {
return this.supportedThingTypes;
return supportedThingTypes;
}
/**
@ -148,7 +148,7 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
*/
@Override
public int getScanTimeout() {
return this.timeout;
return timeout;
}
@Override
@ -185,24 +185,19 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
scheduledStop = null;
}
this.scanListener = listener;
scanListener = listener;
// schedule an automatic call of stopScan when timeout is reached
if (getScanTimeout() > 0) {
Runnable runnable = new Runnable() {
@Override
public void run() {
try {
stopScan();
} catch (Exception e) {
logger.debug("Exception occurred during execution: {}", e.getMessage(), e);
}
scheduledStop = scheduler.schedule(() -> {
try {
stopScan();
} catch (Exception e) {
logger.debug("Exception occurred during execution: {}", e.getMessage(), e);
}
};
scheduledStop = scheduler.schedule(runnable, getScanTimeout(), TimeUnit.SECONDS);
}, getScanTimeout(), TimeUnit.SECONDS);
}
this.timestampOfLastScan = new Date().getTime();
timestampOfLastScan = new Date().getTime();
try {
startScan();
@ -258,14 +253,14 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
*/
protected void thingDiscovered(final DiscoveryResult discoveryResult) {
final DiscoveryResult discoveryResultNew;
if (this.i18nProvider != null && this.localeProvider != null) {
if (i18nProvider != null && localeProvider != null) {
Bundle bundle = FrameworkUtil.getBundle(this.getClass());
String defaultLabel = discoveryResult.getLabel();
String key = I18nUtil.stripConstantOr(defaultLabel, () -> inferKey(discoveryResult, "label"));
String label = this.i18nProvider.getText(bundle, key, defaultLabel, this.localeProvider.getLocale());
String label = i18nProvider.getText(bundle, key, defaultLabel, localeProvider.getLocale());
discoveryResultNew = DiscoveryResultBuilder.create(discoveryResult.getThingUID())
.withThingType(discoveryResult.getThingTypeUID()).withBridge(discoveryResult.getBridgeUID())
@ -378,10 +373,10 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
if (configProperties != null) {
Object property = configProperties.get(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY);
if (property != null) {
this.backgroundDiscoveryEnabled = getAutoDiscoveryEnabled(property);
backgroundDiscoveryEnabled = getAutoDiscoveryEnabled(property);
}
}
if (this.backgroundDiscoveryEnabled) {
if (backgroundDiscoveryEnabled) {
startBackgroundDiscovery();
logger.debug("Background discovery for discovery service '{}' enabled.", this.getClass().getName());
}
@ -403,15 +398,15 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
if (property != null) {
boolean enabled = getAutoDiscoveryEnabled(property);
if (this.backgroundDiscoveryEnabled && !enabled) {
if (backgroundDiscoveryEnabled && !enabled) {
stopBackgroundDiscovery();
logger.debug("Background discovery for discovery service '{}' disabled.",
this.getClass().getName());
} else if (!this.backgroundDiscoveryEnabled && enabled) {
} else if (!backgroundDiscoveryEnabled && enabled) {
startBackgroundDiscovery();
logger.debug("Background discovery for discovery service '{}' enabled.", this.getClass().getName());
}
this.backgroundDiscoveryEnabled = enabled;
backgroundDiscoveryEnabled = enabled;
}
}
}
@ -423,7 +418,7 @@ public abstract class AbstractDiscoveryService implements DiscoveryService {
* discovery is enabled at the time of component deactivation.
*/
protected void deactivate() {
if (this.backgroundDiscoveryEnabled) {
if (backgroundDiscoveryEnabled) {
stopBackgroundDiscovery();
}
}

View File

@ -58,14 +58,14 @@ public interface DiscoveryService {
* @return the list of Thing types which are supported by the discovery service
* (not null, could be empty)
*/
public Collection<ThingTypeUID> getSupportedThingTypes();
Collection<ThingTypeUID> getSupportedThingTypes();
/**
* Returns the amount of time in seconds after which an active scan ends.
*
* @return the scan timeout in seconds (>= 0).
*/
public int getScanTimeout();
int getScanTimeout();
/**
* Returns {@code true} if the background discovery mode is enabled, otherwise {@code false}.

View File

@ -96,7 +96,7 @@ public interface Inbox {
*
* @param flag the flag of the given thingUID result to be set (could be null)
*/
public void setFlag(ThingUID thingUID, @Nullable DiscoveryResultFlag flag);
void setFlag(ThingUID thingUID, @Nullable DiscoveryResultFlag flag);
/**
* Adds an {@link InboxListener} to the listeners' registry.

View File

@ -119,8 +119,8 @@ public class AutomaticInboxProcessor extends AbstractTypedEventSubscriber<ThingS
@Activate
protected void activate(@Nullable Map<String, @Nullable Object> properties) {
this.thingRegistry.addRegistryChangeListener(this);
this.inbox.addInboxListener(this);
thingRegistry.addRegistryChangeListener(this);
inbox.addInboxListener(this);
modified(properties);
}
@ -138,8 +138,8 @@ public class AutomaticInboxProcessor extends AbstractTypedEventSubscriber<ThingS
@Deactivate
protected void deactivate() {
this.inbox.removeInboxListener(this);
this.thingRegistry.removeRegistryChangeListener(this);
inbox.removeInboxListener(this);
thingRegistry.removeRegistryChangeListener(this);
}
@Override
@ -160,7 +160,7 @@ public class AutomaticInboxProcessor extends AbstractTypedEventSubscriber<ThingS
.filter(t -> Objects.equals(value, getRepresentationPropertyValueForThing(t)))
.filter(t -> Objects.equals(t.getThingTypeUID(), result.getThingTypeUID())).findFirst();
if (thing.isPresent()) {
logger.debug("Auto-ignoring the inbox entry for the representation value {}", value);
logger.debug("Auto-ignoring the inbox entry for the representation value '{}'.", value);
inbox.setFlag(result.getThingUID(), DiscoveryResultFlag.IGNORED);
}
}
@ -216,7 +216,8 @@ public class AutomaticInboxProcessor extends AbstractTypedEventSubscriber<ThingS
List<DiscoveryResult> results = inbox.stream().filter(withRepresentationPropertyValue(representationValue))
.filter(forThingTypeUID(thingtypeUID)).collect(Collectors.toList());
if (results.size() == 1) {
logger.debug("Auto-ignoring the inbox entry for the representation value {}", representationValue);
logger.debug("Auto-ignoring the inbox entry for the representation value '{}'.", representationValue);
inbox.setFlag(results.get(0).getThingUID(), DiscoveryResultFlag.IGNORED);
}
}
@ -254,7 +255,7 @@ public class AutomaticInboxProcessor extends AbstractTypedEventSubscriber<ThingS
.filter(forThingTypeUID(thingtypeUID)).filter(withFlag(DiscoveryResultFlag.IGNORED))
.collect(Collectors.toList());
if (results.size() == 1) {
logger.debug("Removing the ignored result from the inbox for the representation value {}",
logger.debug("Removing the ignored result from the inbox for the representation value '{}'.",
representationValue);
inbox.remove(results.get(0).getThingUID());
}

View File

@ -149,8 +149,8 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
for (final DiscoveryService discoveryService : discoveryServicesAll) {
removeDiscoveryServiceActivated(discoveryService);
}
this.listeners.clear();
this.cachedResults.clear();
listeners.clear();
cachedResults.clear();
}
@Override
@ -184,7 +184,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
results.forEach(result -> listener.thingDiscovered(service, result));
});
}
this.listeners.add(listener);
listeners.add(listener);
}
@Override
@ -224,7 +224,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
@Override
public List<ThingTypeUID> getSupportedThingTypes() {
List<ThingTypeUID> thingTypeUIDs = new ArrayList<>();
for (DiscoveryService discoveryService : this.discoveryServices) {
for (DiscoveryService discoveryService : discoveryServices) {
thingTypeUIDs.addAll(discoveryService.getSupportedThingTypes());
}
return thingTypeUIDs;
@ -233,7 +233,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
@Override
public List<String> getSupportedBindings() {
List<String> bindings = new ArrayList<>();
for (DiscoveryService discoveryService : this.discoveryServices) {
for (DiscoveryService discoveryService : discoveryServices) {
Collection<ThingTypeUID> supportedThingTypes = discoveryService.getSupportedThingTypes();
for (ThingTypeUID thingTypeUID : supportedThingTypes) {
bindings.add(thingTypeUID.getBindingId());
@ -244,7 +244,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
@Override
public synchronized void removeDiscoveryListener(DiscoveryListener listener) throws IllegalStateException {
this.listeners.remove(listener);
listeners.remove(listener);
}
@Override
@ -252,7 +252,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
synchronized (cachedResults) {
cachedResults.computeIfAbsent(source, unused -> new HashSet<>()).add(result);
}
for (final DiscoveryListener listener : this.listeners) {
for (final DiscoveryListener listener : listeners) {
try {
AccessController.doPrivileged(new PrivilegedAction<@Nullable Void>() {
@Override
@ -262,7 +262,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
}
});
} catch (Exception ex) {
logger.error("Cannot notify the DiscoveryListener {} on Thing discovered event!",
logger.error("Cannot notify the DiscoveryListener '{}' on Thing discovered event!",
listener.getClass().getName(), ex);
}
}
@ -278,7 +278,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
}
}
}
for (final DiscoveryListener listener : this.listeners) {
for (final DiscoveryListener listener : listeners) {
try {
AccessController.doPrivileged(new PrivilegedAction<@Nullable Void>() {
@Override
@ -298,7 +298,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
public @Nullable Collection<ThingUID> removeOlderResults(final DiscoveryService source, final long timestamp,
final @Nullable Collection<ThingTypeUID> thingTypeUIDs, @Nullable ThingUID bridgeUID) {
Set<ThingUID> removedResults = new HashSet<>();
for (final DiscoveryListener listener : this.listeners) {
for (final DiscoveryListener listener : listeners) {
try {
Collection<ThingUID> olderResults = AccessController
.doPrivileged(new PrivilegedAction<@Nullable Collection<ThingUID>>() {
@ -422,7 +422,7 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
private void addDiscoveryServiceActivated(final DiscoveryService discoveryService) {
discoveryService.addDiscoveryListener(this);
this.discoveryServices.add(discoveryService);
discoveryServices.add(discoveryService);
}
protected void removeDiscoveryService(DiscoveryService discoveryService) {
@ -433,22 +433,20 @@ public final class DiscoveryServiceRegistryImpl implements DiscoveryServiceRegis
}
private void removeDiscoveryServiceActivated(DiscoveryService discoveryService) {
this.discoveryServices.remove(discoveryService);
discoveryServices.remove(discoveryService);
discoveryService.removeDiscoveryListener(this);
synchronized (cachedResults) {
this.cachedResults.remove(discoveryService);
cachedResults.remove(discoveryService);
}
}
private int getMaxScanTimeout(Set<DiscoveryService> discoveryServices) {
int result = 0;
for (DiscoveryService discoveryService : discoveryServices) {
if (discoveryService.getScanTimeout() > result) {
result = discoveryService.getScanTimeout();
}
}
return result;
}

View File

@ -94,9 +94,9 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
// Internal enumeration to identify the correct type of the event to be fired.
private enum EventType {
added,
removed,
updated
ADDED,
REMOVED,
UPDATED
}
private class TimeToLiveCheckingThread implements Runnable {
@ -112,7 +112,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
long now = new Date().getTime();
for (DiscoveryResult result : inbox.getAll()) {
if (isResultExpired(result, now)) {
logger.debug("Inbox entry for thing {} is expired and will be removed", result.getThingUID());
logger.debug("Inbox entry for thing '{}' is expired and will be removed.", result.getThingUID());
remove(result.getThingUID());
}
}
@ -158,18 +158,18 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
@Activate
protected void activate() {
this.discoveryServiceRegistry.addDiscoveryListener(this);
this.thingRegistry.addRegistryChangeListener(this);
this.timeToLiveChecker = ThreadPoolManager.getScheduledPool("discovery")
discoveryServiceRegistry.addDiscoveryListener(this);
thingRegistry.addRegistryChangeListener(this);
timeToLiveChecker = ThreadPoolManager.getScheduledPool("discovery")
.scheduleWithFixedDelay(new TimeToLiveCheckingThread(this), 0, 30, TimeUnit.SECONDS);
}
@Deactivate
protected void deactivate() {
this.thingRegistry.removeRegistryChangeListener(this);
this.discoveryServiceRegistry.removeDiscoveryListener(this);
this.listeners.clear();
this.timeToLiveChecker.cancel(true);
thingRegistry.removeRegistryChangeListener(this);
discoveryServiceRegistry.removeDiscoveryListener(this);
listeners.clear();
timeToLiveChecker.cancel(true);
}
@Override
@ -188,9 +188,9 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
final Configuration config = new Configuration(configParams);
ThingTypeUID thingTypeUID = result.getThingTypeUID();
Thing newThing = ThingFactory.createThing(thingUID, config, properties, result.getBridgeUID(), thingTypeUID,
this.thingHandlerFactories);
thingHandlerFactories);
if (newThing == null) {
logger.warn("Cannot create thing. No binding found that supports creating a thing" + " of type {}.",
logger.warn("Cannot create thing. No binding found that supports creating a thing of type {}.",
thingTypeUID);
return null;
}
@ -211,14 +211,14 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
final DiscoveryResult result = discoveryResult;
ThingUID thingUID = result.getThingUID();
Thing thing = this.thingRegistry.get(thingUID);
Thing thing = thingRegistry.get(thingUID);
if (thing == null) {
DiscoveryResult inboxResult = get(thingUID);
if (inboxResult == null) {
discoveryResultStorage.put(result.getThingUID().toString(), result);
notifyListeners(result, EventType.added);
notifyListeners(result, EventType.ADDED);
logger.info("Added new thing '{}' to inbox.", thingUID);
return true;
} else {
@ -226,7 +226,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
DiscoveryResultImpl resultImpl = (DiscoveryResultImpl) inboxResult;
resultImpl.synchronize(result);
discoveryResultStorage.put(result.getThingUID().toString(), resultImpl);
notifyListeners(resultImpl, EventType.updated);
notifyListeners(resultImpl, EventType.UPDATED);
logger.debug("Updated discovery result for '{}'.", thingUID);
return true;
} else {
@ -235,15 +235,16 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
}
}
} else {
logger.debug("Discovery result with thing '{}' not added as inbox entry."
+ " It is already present as thing in the ThingRegistry.", thingUID);
logger.debug(
"Discovery result with thing '{}' not added as inbox entry. It is already present as thing in the ThingRegistry.",
thingUID);
boolean updated = synchronizeConfiguration(result.getThingTypeUID(), result.getProperties(),
thing.getConfiguration());
if (updated) {
logger.debug("The configuration for thing '{}' is updated...", thingUID);
this.managedThingProvider.update(thing);
managedThingProvider.update(thing);
}
}
@ -300,7 +301,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
@Override
public void addInboxListener(@Nullable InboxListener listener) throws IllegalStateException {
if (listener != null) {
this.listeners.add(listener);
listeners.add(listener);
}
}
@ -338,8 +339,8 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
removeResultsForBridge(thingUID);
}
resultDiscovererMap.remove(discoveryResult);
this.discoveryResultStorage.remove(thingUID.toString());
notifyListeners(discoveryResult, EventType.removed);
discoveryResultStorage.remove(thingUID.toString());
notifyListeners(discoveryResult, EventType.REMOVED);
return true;
}
}
@ -349,7 +350,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
@Override
public void removeInboxListener(@Nullable InboxListener listener) throws IllegalStateException {
if (listener != null) {
this.listeners.remove(listener);
listeners.remove(listener);
}
}
@ -378,7 +379,8 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
if (bridgeUID == null || bridgeUID.equals(discoveryResult.getBridgeUID())) {
removedThings.add(thingUID);
remove(thingUID);
logger.debug("Removed {} from inbox because it was older than {}", thingUID, new Date(timestamp));
logger.debug("Removed thing '{}' from inbox because it was older than {}.", thingUID,
new Date(timestamp));
}
}
}
@ -389,7 +391,8 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
public void added(Thing thing) {
if (remove(thing.getUID())) {
logger.debug(
"Discovery result removed from inbox, because it was added as a Thing" + " to the ThingRegistry.");
"Discovery result for thing '{}' removed from inbox, because it was added as a Thing to the ThingRegistry.",
thing.getUID());
}
}
@ -414,7 +417,7 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
DiscoveryResultImpl resultImpl = (DiscoveryResultImpl) result;
resultImpl.setFlag((flag == null) ? DiscoveryResultFlag.NEW : flag);
discoveryResultStorage.put(resultImpl.getThingUID().toString(), resultImpl);
notifyListeners(resultImpl, EventType.updated);
notifyListeners(resultImpl, EventType.UPDATED);
} else {
logger.warn("Cannot set flag for result of instance type '{}'", result.getClass().getName());
}
@ -434,35 +437,32 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
if (thingUID != null) {
return discoveryResultStorage.get(thingUID.toString());
}
return null;
}
private void notifyListeners(DiscoveryResult result, EventType type) {
for (InboxListener listener : this.listeners) {
for (InboxListener listener : listeners) {
try {
switch (type) {
case added:
case ADDED:
listener.thingAdded(this, result);
break;
case removed:
case REMOVED:
listener.thingRemoved(this, result);
break;
case updated:
case UPDATED:
listener.thingUpdated(this, result);
break;
}
} catch (Exception ex) {
String errorMessage = String.format("Cannot notify the InboxListener '%s' about a Thing %s event!",
listener.getClass().getName(), type.name());
logger.error(errorMessage, ex);
logger.error("Cannot notify the InboxListener '{}' about a Thing {} event!",
listener.getClass().getName(), type.name(), ex);
}
}
// in case of EventType added/updated the listeners might have modified the result in the discoveryResultStorage
final DiscoveryResult resultForEvent;
if (type == EventType.removed) {
if (type == EventType.REMOVED) {
resultForEvent = result;
} else {
resultForEvent = get(result.getThingUID());
@ -477,13 +477,13 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
if (eventPublisher != null) {
try {
switch (eventType) {
case added:
case ADDED:
eventPublisher.post(InboxEventFactory.createAddedEvent(result));
break;
case removed:
case REMOVED:
eventPublisher.post(InboxEventFactory.createRemovedEvent(result));
break;
case updated:
case UPDATED:
eventPublisher.post(InboxEventFactory.createUpdatedEvent(result));
break;
default:
@ -506,8 +506,8 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
for (ThingUID thingUID : getResultsForBridge(bridgeUID)) {
DiscoveryResult discoveryResult = get(thingUID);
if (discoveryResult != null) {
this.discoveryResultStorage.remove(thingUID.toString());
notifyListeners(discoveryResult, EventType.removed);
discoveryResultStorage.remove(thingUID.toString());
notifyListeners(discoveryResult, EventType.REMOVED);
}
}
}
@ -580,8 +580,8 @@ public final class PersistentInbox implements Inbox, DiscoveryListener, ThingReg
}
void setTimeToLiveCheckingInterval(int interval) {
this.timeToLiveChecker.cancel(true);
this.timeToLiveChecker = ThreadPoolManager.getScheduledPool("discovery")
timeToLiveChecker.cancel(true);
timeToLiveChecker = ThreadPoolManager.getScheduledPool("discovery")
.scheduleWithFixedDelay(new TimeToLiveCheckingThread(this), 0, interval, TimeUnit.SECONDS);
}

View File

@ -118,7 +118,7 @@ public class AutomaticInboxProcessorTest {
private ManagedThingProvider thingProvider;
@Before
public void setUp() throws Exception {
public void setUp() {
initMocks(this);
when(thing.getConfiguration()).thenReturn(CONFIG);

View File

@ -121,7 +121,7 @@ public class PersistentInboxTest {
}
@Test
public void testConfigUpdateNormalizationWithConfigDescription() throws Exception {
public void testConfigUpdateNormalizationWithConfigDescription() throws URISyntaxException {
Map<String, Object> props = new HashMap<>();
props.put("foo", "1");
Configuration config = new Configuration(props);
@ -139,7 +139,7 @@ public class PersistentInboxTest {
}
@Test
public void testApproveNormalization() throws Exception {
public void testApproveNormalization() throws URISyntaxException {
DiscoveryResult result = DiscoveryResultBuilder.create(THING_UID).withProperty("foo", 3).build();
configureConfigDescriptionRegistryMock("foo", Type.TEXT);
when(storage.getValues()).thenReturn(Collections.singletonList(result));