[config] Added nullness annotations and suppress 'ConfigStatusInfoEvent' if list of 'ConfigStatusMessage's is empty (#2680)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
pull/2668/head
Christoph Weitkamp 2022-01-14 11:47:52 +01:00 committed by GitHub
parent 029151d787
commit d693302190
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 99 additions and 137 deletions

View File

@ -12,12 +12,15 @@
*/
package org.openhab.core.config.core.status;
import org.eclipse.jdt.annotation.NonNullByDefault;
/**
* The {@link ConfigStatusCallback} interface is a callback interface to propagate a new configuration status for an
* entity.
*
* @author Thomas Höfer - Initial contribution
*/
@NonNullByDefault
public interface ConfigStatusCallback {
/**

View File

@ -20,6 +20,8 @@ import java.util.Objects;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.config.core.status.ConfigStatusMessage.Type;
/**
@ -28,6 +30,7 @@ import org.openhab.core.config.core.status.ConfigStatusMessage.Type;
*
* @author Thomas Höfer - Initial contribution
*/
@NonNullByDefault
public final class ConfigStatusInfo {
private final Collection<ConfigStatusMessage> configStatusMessages = new ArrayList<>();
@ -146,7 +149,7 @@ public final class ConfigStatusInfo {
}
@Override
public boolean equals(Object obj) {
public boolean equals(@Nullable Object obj) {
if (this == obj) {
return true;
}

View File

@ -15,6 +15,8 @@ package org.openhab.core.config.core.status;
import java.util.Arrays;
import java.util.Objects;
import org.eclipse.jdt.annotation.Nullable;
/**
* The {@link ConfigStatusMessage} is a domain object for a configuration status message. It contains the name
* of the configuration parameter, the {@link ConfigStatusMessage.Type} information, the internationalized message and
@ -74,19 +76,19 @@ public final class ConfigStatusMessage {
public final Type type;
/** The key for the message to be internalized. */
final transient String messageKey;
final transient @Nullable String messageKey;
/** The arguments to be injected into the internationalized message. */
final transient Object[] arguments;
final transient Object @Nullable [] arguments;
/** The corresponding internationalized status message. */
public final String message;
public final @Nullable String message;
/**
* The optional status code of the configuration status message; to be used if there are additional information to
* be provided.
*/
public final Integer statusCode;
public final @Nullable Integer statusCode;
/**
* Package protected default constructor to allow reflective instantiation.
@ -103,12 +105,7 @@ public final class ConfigStatusMessage {
* @param builder the configuration status message builder
*/
private ConfigStatusMessage(Builder builder) {
this.parameterName = builder.parameterName;
this.type = builder.type;
this.messageKey = builder.messageKey;
this.arguments = builder.arguments;
this.message = null;
this.statusCode = builder.statusCode;
this(builder.parameterName, builder.type, builder.messageKey, builder.arguments, null, builder.statusCode);
}
/**
@ -119,12 +116,12 @@ public final class ConfigStatusMessage {
* @param message the corresponding internationalized status message
* @param statusCode the optional status code
*/
ConfigStatusMessage(String parameterName, Type type, String message, Integer statusCode) {
ConfigStatusMessage(String parameterName, Type type, String message, @Nullable Integer statusCode) {
this(parameterName, type, null, null, message, statusCode);
}
private ConfigStatusMessage(String parameterName, Type type, String messageKey, Object[] arguments, String message,
Integer statusCode) {
private ConfigStatusMessage(String parameterName, Type type, @Nullable String messageKey,
Object @Nullable [] arguments, @Nullable String message, @Nullable Integer statusCode) {
this.parameterName = parameterName;
this.type = type;
this.messageKey = messageKey;
@ -144,11 +141,11 @@ public final class ConfigStatusMessage {
private final Type type;
private String messageKey;
private @Nullable String messageKey;
private Object[] arguments;
private Object @Nullable [] arguments;
private Integer statusCode;
private @Nullable Integer statusCode;
private Builder(String parameterName, Type type) {
Objects.requireNonNull(parameterName, "Parameter name must not be null.");
@ -198,6 +195,17 @@ public final class ConfigStatusMessage {
return new Builder(parameterName, Type.PENDING);
}
/**
* Adds the given message key suffix for the creation of {@link ConfigStatusMessage#messageKey}.
*
* @param messageKeySuffix the message key suffix to be added
* @return the updated builder
*/
public Builder withMessageKeySuffix(String messageKeySuffix) {
this.messageKey = CONFIG_STATUS_MSG_KEY_PREFIX + type.name().toLowerCase() + "." + messageKeySuffix;
return this;
}
/**
* Adds the given arguments (to be injected into the internationalized message) to the builder.
*
@ -220,17 +228,6 @@ public final class ConfigStatusMessage {
return this;
}
/**
* Adds the given message key suffix for the creation of {@link ConfigStatusMessage#messageKey}.
*
* @param messageKeySuffix the message key suffix to be added
* @return the updated builder
*/
public Builder withMessageKeySuffix(String messageKeySuffix) {
this.messageKey = CONFIG_STATUS_MSG_KEY_PREFIX + type.name().toLowerCase() + "." + messageKeySuffix;
return this;
}
/**
* Builds the new {@link ConfigStatusMessage} object.
*
@ -253,7 +250,7 @@ public final class ConfigStatusMessage {
}
@Override
public boolean equals(Object obj) {
public boolean equals(@Nullable Object obj) {
if (this == obj) {
return true;
}

View File

@ -18,6 +18,8 @@ import java.util.Locale;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ExecutorService;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.common.ThreadPoolManager;
import org.openhab.core.config.core.status.events.ConfigStatusInfoEvent;
import org.openhab.core.events.EventPublisher;
@ -25,6 +27,7 @@ import org.openhab.core.i18n.LocaleProvider;
import org.openhab.core.i18n.TranslationProvider;
import org.openhab.core.util.BundleResolver;
import org.osgi.framework.Bundle;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.component.annotations.ReferenceCardinality;
@ -41,20 +44,32 @@ import org.slf4j.LoggerFactory;
* @author Chris Jackson - Allow null messages
* @author Markus Rathgeb - Add locale provider support
*/
@Component(immediate = true, service = { ConfigStatusService.class })
@Component(immediate = true, service = ConfigStatusService.class)
@NonNullByDefault
public final class ConfigStatusService implements ConfigStatusCallback {
private final Logger logger = LoggerFactory.getLogger(ConfigStatusService.class);
private final List<ConfigStatusProvider> configStatusProviders = new CopyOnWriteArrayList<>();
private EventPublisher eventPublisher;
private LocaleProvider localeProvider;
private TranslationProvider translationProvider;
private BundleResolver bundleResolver;
private final EventPublisher eventPublisher;
private final LocaleProvider localeProvider;
private final TranslationProvider translationProvider;
private final BundleResolver bundleResolver;
private final List<ConfigStatusProvider> configStatusProviders = new CopyOnWriteArrayList<>();
private final ExecutorService executorService = ThreadPoolManager
.getPool(ConfigStatusService.class.getSimpleName());
@Activate
public ConfigStatusService(final @Reference EventPublisher eventPublisher, //
final @Reference LocaleProvider localeProvider, //
final @Reference TranslationProvider i18nProvider, //
final @Reference BundleResolver bundleResolver) {
this.eventPublisher = eventPublisher;
this.localeProvider = localeProvider;
this.translationProvider = i18nProvider;
this.bundleResolver = bundleResolver;
}
/**
* Retrieves the {@link ConfigStatusInfo} of the entity by using the registered
* {@link ConfigStatusProvider} that supports the given entity.
@ -67,7 +82,7 @@ public final class ConfigStatusService implements ConfigStatusCallback {
* supports the given entity
* @throws IllegalArgumentException if given entityId is null or empty
*/
public ConfigStatusInfo getConfigStatus(String entityId, final Locale locale) {
public @Nullable ConfigStatusInfo getConfigStatus(String entityId, final @Nullable Locale locale) {
if (entityId == null || entityId.isEmpty()) {
throw new IllegalArgumentException("EntityId must not be null or empty");
}
@ -76,7 +91,7 @@ public final class ConfigStatusService implements ConfigStatusCallback {
for (ConfigStatusProvider configStatusProvider : configStatusProviders) {
if (configStatusProvider.supportsEntity(entityId)) {
return getConfigStatus(configStatusProvider, entityId, loc);
return getConfigStatusInfo(configStatusProvider, entityId, loc);
}
}
@ -87,54 +102,44 @@ public final class ConfigStatusService implements ConfigStatusCallback {
@Override
public void configUpdated(final ConfigStatusSource configStatusSource) {
executorService.submit(new Runnable() {
@Override
public void run() {
final ConfigStatusInfo info = getConfigStatus(configStatusSource.entityId, null);
if (info != null) {
if (eventPublisher != null) {
eventPublisher.post(new ConfigStatusInfoEvent(configStatusSource.getTopic(), info));
} else {
logger.warn("EventPublisher not available. Cannot post new config status for entity {}",
configStatusSource.entityId);
}
}
executorService.submit(() -> {
final ConfigStatusInfo info = getConfigStatus(configStatusSource.entityId, null);
if (info != null) {
eventPublisher.post(new ConfigStatusInfoEvent(configStatusSource.getTopic(), info));
}
});
}
private ConfigStatusInfo getConfigStatus(ConfigStatusProvider configStatusProvider, String entityId,
Locale locale) {
Collection<ConfigStatusMessage> configStatus = configStatusProvider.getConfigStatus();
if (configStatus == null) {
private @Nullable ConfigStatusInfo getConfigStatusInfo(ConfigStatusProvider configStatusProvider, String entityId,
@Nullable Locale locale) {
Collection<ConfigStatusMessage> configStatusMessages = configStatusProvider.getConfigStatus();
if (configStatusMessages == null) {
logger.debug("Cannot provide config status for entity {} because its config status provider returned null.",
entityId);
return null;
}
Bundle bundle = bundleResolver.resolveBundle(configStatusProvider.getClass());
ConfigStatusInfo info = new ConfigStatusInfo();
for (ConfigStatusMessage configStatusMessage : configStatus) {
String message = null;
if (configStatusMessage.messageKey != null) {
message = translationProvider.getText(bundle, configStatusMessage.messageKey, null, locale,
configStatusMessage.arguments);
if (message == null) {
logger.warn(
"No translation found for key {} and config status provider {}. Will ignore the config status message.",
configStatusMessage.messageKey, configStatusProvider.getClass().getSimpleName());
continue;
if (!configStatusMessages.isEmpty()) {
ConfigStatusInfo info = new ConfigStatusInfo();
for (ConfigStatusMessage configStatusMessage : configStatusMessages) {
String message = null;
if (configStatusMessage.messageKey != null) {
message = translationProvider.getText(bundle, configStatusMessage.messageKey, null, locale,
configStatusMessage.arguments);
if (message == null) {
logger.warn(
"No translation found for key {} and config status provider {}. Will ignore the config status message.",
configStatusMessage.messageKey, configStatusProvider.getClass().getSimpleName());
continue;
}
}
info.add(new ConfigStatusMessage(configStatusMessage.parameterName, configStatusMessage.type, message,
configStatusMessage.statusCode));
}
info.add(new ConfigStatusMessage(configStatusMessage.parameterName, configStatusMessage.type, message,
configStatusMessage.statusCode));
return info;
}
return info;
return null;
}
@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
@ -147,40 +152,4 @@ public final class ConfigStatusService implements ConfigStatusCallback {
configStatusProvider.setConfigStatusCallback(null);
configStatusProviders.remove(configStatusProvider);
}
@Reference(policy = ReferencePolicy.DYNAMIC)
protected void setEventPublisher(EventPublisher eventPublisher) {
this.eventPublisher = eventPublisher;
}
protected void unsetEventPublisher(EventPublisher eventPublisher) {
this.eventPublisher = null;
}
@Reference
protected void setLocaleProvider(LocaleProvider localeProvider) {
this.localeProvider = localeProvider;
}
protected void unsetLocaleProvider(LocaleProvider localeProvider) {
this.localeProvider = null;
}
@Reference
protected void setTranslationProvider(TranslationProvider i18nProvider) {
this.translationProvider = i18nProvider;
}
protected void unsetTranslationProvider(TranslationProvider i18nProvider) {
this.translationProvider = null;
}
@Reference
protected void setBundleResolver(BundleResolver bundleResolver) {
this.bundleResolver = bundleResolver;
}
protected void unsetBundleResolver(BundleResolver bundleResolver) {
this.bundleResolver = bundleResolver;
}
}

View File

@ -14,6 +14,8 @@ package org.openhab.core.config.core.status.events;
import java.util.Set;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.config.core.status.ConfigStatusInfo;
import org.openhab.core.events.AbstractEventFactory;
import org.openhab.core.events.Event;
@ -26,7 +28,8 @@ import org.osgi.service.component.annotations.Component;
*
* @author Thomas Höfer - Initial contribution
*/
@Component(immediate = true, service = { EventFactory.class })
@Component(immediate = true, service = EventFactory.class)
@NonNullByDefault
public final class ConfigStatusEventFactory extends AbstractEventFactory {
private static final Set<String> SUPPORTED_EVENT_TYPES = Set.of(ConfigStatusInfoEvent.TYPE);
@ -48,7 +51,8 @@ public final class ConfigStatusEventFactory extends AbstractEventFactory {
}
@Override
protected Event createEventByType(String eventType, String topic, String payload, String source) throws Exception {
protected Event createEventByType(String eventType, String topic, String payload, @Nullable String source)
throws Exception {
if (ConfigStatusInfoEvent.TYPE.equals(eventType)) {
return createStatusInfoEvent(topic, payload);
}

View File

@ -12,6 +12,7 @@
*/
package org.openhab.core.config.core.status.events;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.core.config.core.status.ConfigStatusInfo;
import org.openhab.core.events.AbstractEvent;
@ -22,6 +23,7 @@ import com.google.gson.Gson;
*
* @author Thomas Höfer - Initial contribution
*/
@NonNullByDefault
public final class ConfigStatusInfoEvent extends AbstractEvent {
static final String TYPE = "ConfigStatusInfoEvent";

View File

@ -12,16 +12,13 @@
*/
package org.openhab.core.config.core.status;
import static java.util.Collections.emptySet;
import static java.util.stream.Collectors.toList;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.Test;
import org.openhab.core.config.core.status.ConfigStatusMessage.Type;
@ -31,6 +28,7 @@ import org.openhab.core.config.core.status.ConfigStatusMessage.Type;
* @author Thomas Höfer - Initial contribution
* @author Wouter Born - Migrate tests from Groovy to Java
*/
@NonNullByDefault
public class ConfigStatusInfoTest {
private static final String PARAM1 = "param1";
@ -60,8 +58,7 @@ public class ConfigStatusInfoTest {
private static final ConfigStatusMessage MSG6 = ConfigStatusMessage.Builder.pending(PARAM6)
.withMessageKeySuffix(ERROR2).withStatusCode(1).build();
private static final List<ConfigStatusMessage> ALL = Stream.of(MSG1, MSG2, MSG3, MSG4, MSG5, MSG6)
.collect(toList());
private static final List<ConfigStatusMessage> ALL = List.of(MSG1, MSG2, MSG3, MSG4, MSG5, MSG6);
@Test
public void assertCorrectConfigErrorHandlingForEmptyResultObject() {
@ -90,18 +87,6 @@ public class ConfigStatusInfoTest {
assertConfigStatusInfo(info);
}
@Test
public void assertNPEisThrownIfTypesAreNull() {
ConfigStatusInfo info = new ConfigStatusInfo();
assertThrows(NullPointerException.class, () -> info.getConfigStatusMessages(null, emptySet()));
}
@Test
public void assertNPEisThrownIfParameterNamesAreNull() {
ConfigStatusInfo info = new ConfigStatusInfo();
assertThrows(NullPointerException.class, () -> info.getConfigStatusMessages(emptySet(), null));
}
private void assertConfigStatusInfo(ConfigStatusInfo info) {
assertThat(info.getConfigStatusMessages().size(), is(ALL.size()));
assertThat(info.getConfigStatusMessages(), hasItems(MSG1, MSG2, MSG3, MSG4, MSG5, MSG6));

View File

@ -23,6 +23,8 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Locale;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.openhab.core.config.core.status.ConfigStatusMessage.Type;
@ -37,9 +39,10 @@ import org.openhab.core.util.BundleResolver;
*
* @author Thomas Höfer - Initial contribution
*/
@NonNullByDefault
public class ConfigStatusServiceTest extends JavaTest {
private ConfigStatusService configStatusService;
private @NonNullByDefault({}) ConfigStatusService configStatusService;
private static final String ENTITY_ID1 = "entity1";
private static final String ENTITY_ID2 = "entity2";
@ -104,21 +107,17 @@ public class ConfigStatusServiceTest extends JavaTest {
messagesEntity2En.add(buildConfigStatusMessage(PARAM3_MSG3.parameterName, PARAM3_MSG3.type,
MessageFormat.format(MSG3_EN, ARGS), PARAM3_MSG3.statusCode));
configStatusService = new ConfigStatusService();
LocaleProvider localeProvider = mock(LocaleProvider.class);
when(localeProvider.getLocale()).thenReturn(new Locale("en", "US"));
configStatusService.setLocaleProvider(localeProvider);
configStatusService.setEventPublisher(mock(EventPublisher.class));
configStatusService.setBundleResolver(mock(BundleResolver.class));
configStatusService.setTranslationProvider(getTranslationProvider());
configStatusService = new ConfigStatusService(mock(EventPublisher.class), localeProvider,
getTranslationProvider(), mock(BundleResolver.class));
configStatusService.addConfigStatusProvider(getConfigStatusProviderMock(ENTITY_ID1));
configStatusService.addConfigStatusProvider(getConfigStatusProviderMock(ENTITY_ID2));
}
private ConfigStatusMessage buildConfigStatusMessage(String parameterName, Type type, String msg,
Integer statusCode) {
@Nullable Integer statusCode) {
return new ConfigStatusMessage(parameterName, type, msg, statusCode);
}
@ -142,7 +141,7 @@ public class ConfigStatusServiceTest extends JavaTest {
assertConfigStatusInfo(info, new ConfigStatusInfo(expectedMessages));
}
private void assertConfigStatusInfo(ConfigStatusInfo actual, ConfigStatusInfo expected) {
private void assertConfigStatusInfo(@Nullable ConfigStatusInfo actual, ConfigStatusInfo expected) {
assertThat(actual, equalTo(expected));
}