From d22e14a1e47bba8c1a38345a4da03c8a87e4a172 Mon Sep 17 00:00:00 2001 From: lolodomo Date: Wed, 26 Jun 2024 21:25:13 +0200 Subject: [PATCH] Make input formatting for transformations consistent everywhere (#4203) Use item state formatter to format input of transformation, meaning using state.format(format) instead of String.format(format, state.toString()) This was already the case in sitemap API but not in other APIs used by Main UI. Make sure to call transformation even for NULL and UNDEF states. It was not the case in one API used by Main UI. When calling transformation and state is NULL or UNDEF, do not apply format to the input value and do not replace by "-". That means the transformation will be called with "NULL" or "UNDEF". Sitemap API was calling the transformation using a pattern containing "-". Fix #4101 Also related to discussion in openhab/openhab-addons#13777 Signed-off-by: Laurent Garnier --- .../rest/core/item/EnrichedItemDTOMapper.java | 57 +++++++----- .../internal/SseItemStatesEventBuilder.java | 58 ++++++++---- .../SseItemStatesEventBuilderTest.java | 88 ++++++++++++++++++- .../ui/internal/items/ItemUIRegistryImpl.java | 72 +++++++++------ .../items/ItemUIRegistryImplTest.java | 39 ++++++++ 5 files changed, 246 insertions(+), 68 deletions(-) diff --git a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java index e66966479..487083e09 100644 --- a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java +++ b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java @@ -18,6 +18,8 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.function.Predicate; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.ws.rs.core.UriBuilder; @@ -30,9 +32,11 @@ import org.openhab.core.items.dto.ItemDTOMapper; import org.openhab.core.library.items.NumberItem; import org.openhab.core.transform.TransformationException; import org.openhab.core.transform.TransformationHelper; +import org.openhab.core.transform.TransformationService; +import org.openhab.core.types.State; import org.openhab.core.types.StateDescription; import org.openhab.core.types.StateDescriptionFragmentBuilder; -import org.osgi.framework.FrameworkUtil; +import org.openhab.core.types.UnDefType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,6 +49,8 @@ import org.slf4j.LoggerFactory; @NonNullByDefault public class EnrichedItemDTOMapper { + private static final Pattern EXTRACT_TRANSFORM_FUNCTION_PATTERN = Pattern.compile("(.*?)\\((.*)\\):(.*)"); + private static final Logger LOGGER = LoggerFactory.getLogger(EnrichedItemDTOMapper.class); /** @@ -79,8 +85,8 @@ public class EnrichedItemDTOMapper { parents.add(item); } String state = item.getState().toFullString(); - String transformedState = considerTransformation(state, item, locale); - if (transformedState != null && transformedState.equals(state)) { + String transformedState = considerTransformation(item, locale); + if (state.equals(transformedState)) { transformedState = null; } StateDescription stateDescription = considerTransformation(item.getStateDescription(locale)); @@ -132,37 +138,48 @@ public class EnrichedItemDTOMapper { if (stateDescription != null) { String pattern = stateDescription.getPattern(); if (pattern != null) { - try { - return TransformationHelper.isTransform(pattern) - ? StateDescriptionFragmentBuilder.create(stateDescription).withPattern(pattern).build() - .toStateDescription() - : stateDescription; - } catch (NoClassDefFoundError ex) { - // TransformationHelper is optional dependency, so ignore if class not found - // return state description as it is without transformation - } + return TransformationHelper.isTransform(pattern) + ? StateDescriptionFragmentBuilder.create(stateDescription).withPattern(pattern).build() + .toStateDescription() + : stateDescription; } } return stateDescription; } - private static @Nullable String considerTransformation(String state, Item item, @Nullable Locale locale) { + private static @Nullable String considerTransformation(Item item, @Nullable Locale locale) { StateDescription stateDescription = item.getStateDescription(locale); if (stateDescription != null) { String pattern = stateDescription.getPattern(); - if (pattern != null) { + Matcher matcher; + if (pattern != null && (matcher = EXTRACT_TRANSFORM_FUNCTION_PATTERN.matcher(pattern)).find()) { + State state = item.getState(); try { - return TransformationHelper.transform( - FrameworkUtil.getBundle(EnrichedItemDTOMapper.class).getBundleContext(), pattern, state); - } catch (NoClassDefFoundError ex) { - // TransformationHelper is optional dependency, so ignore if class not found - // return state as it is without transformation + String type = matcher.group(1); + String function = matcher.group(2); + String value = matcher.group(3); + TransformationService transformation = TransformationHelper.getTransformationService(type); + if (transformation != null) { + String format = state instanceof UnDefType ? "%s" : value; + try { + return transformation.transform(function, state.format(format)); + } catch (IllegalArgumentException e) { + throw new TransformationException( + "Cannot format state '" + state + "' to format '" + format + "'", e); + } catch (RuntimeException e) { + throw new TransformationException("Transformation service of type '" + type + + "' threw an exception: " + e.getMessage(), e); + } + } else { + throw new TransformationException( + "Transformation service of type '" + type + "' is not available."); + } } catch (TransformationException e) { LOGGER.warn("Failed transforming the state '{}' on item '{}' with pattern '{}': {}", state, item.getName(), pattern, e.getMessage()); } } } - return state; + return null; } } diff --git a/bundles/org.openhab.core.io.rest.sse/src/main/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilder.java b/bundles/org.openhab.core.io.rest.sse/src/main/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilder.java index 184eb5b25..757197bdf 100644 --- a/bundles/org.openhab.core.io.rest.sse/src/main/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilder.java +++ b/bundles/org.openhab.core.io.rest.sse/src/main/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilder.java @@ -18,6 +18,8 @@ import java.util.IllegalFormatException; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.measure.Unit; import javax.ws.rs.core.MediaType; @@ -37,6 +39,7 @@ import org.openhab.core.library.types.QuantityType; import org.openhab.core.service.StartLevelService; import org.openhab.core.transform.TransformationException; import org.openhab.core.transform.TransformationHelper; +import org.openhab.core.transform.TransformationService; import org.openhab.core.types.State; import org.openhab.core.types.StateDescription; import org.openhab.core.types.StateOption; @@ -59,6 +62,8 @@ import org.slf4j.LoggerFactory; @NonNullByDefault public class SseItemStatesEventBuilder { + private static final Pattern EXTRACT_TRANSFORM_FUNCTION_PATTERN = Pattern.compile("(.*?)\\((.*)\\):(.*)"); + private final Logger logger = LoggerFactory.getLogger(SseItemStatesEventBuilder.class); private final ItemRegistry itemRegistry; @@ -113,22 +118,43 @@ public class SseItemStatesEventBuilder { State state = item.getState(); String displayState = state.toString(); - // NULL/UNDEF state is returned as "NULL"/"UNDEF" without considering anything else - if (!(state instanceof UnDefType)) { - if (stateDescription != null) { - boolean transformUsed = false; - boolean optionMatched = false; - String pattern = stateDescription.getPattern(); - // If there's a pattern, first check if it's a transformation - if (pattern != null && TransformationHelper.isTransform(pattern)) { - transformUsed = true; - try { - displayState = TransformationHelper.transform(pattern, state.toString()); - } catch (TransformationException e) { - logger.warn("Failed transforming the state '{}' on item '{}' with pattern '{}': {}", state, - item.getName(), pattern, e.getMessage()); + if (stateDescription != null) { + String pattern = stateDescription.getPattern(); + // First check if the pattern is a transformation + Matcher matcher; + if (pattern != null && (matcher = EXTRACT_TRANSFORM_FUNCTION_PATTERN.matcher(pattern)).find()) { + try { + String type = matcher.group(1); + String function = matcher.group(2); + String value = matcher.group(3); + TransformationService transformation = TransformationHelper.getTransformationService(type); + if (transformation != null) { + String format = state instanceof UnDefType ? "%s" : value; + try { + displayState = transformation.transform(function, state.format(format)); + if (displayState == null) { + displayState = state.toString(); + } + } catch (IllegalArgumentException e) { + throw new TransformationException( + "Cannot format state '" + state + "' to format '" + format + "'", e); + } catch (RuntimeException e) { + throw new TransformationException("Transformation service of type '" + type + + "' threw an exception: " + e.getMessage(), e); + } + } else { + throw new TransformationException( + "Transformation service of type '" + type + "' is not available."); } - } else if (!stateDescription.getOptions().isEmpty()) { + } catch (TransformationException e) { + logger.warn("Failed transforming the state '{}' on item '{}' with pattern '{}': {}", state, + item.getName(), pattern, e.getMessage()); + } + } + // If no transformation, NULL/UNDEF state is returned as "NULL"/"UNDEF" without considering anything else + else if (!(state instanceof UnDefType)) { + boolean optionMatched = false; + if (!stateDescription.getOptions().isEmpty()) { // Look for a state option with a value corresponding to the state for (StateOption option : stateDescription.getOptions()) { String label = option.getLabel(); @@ -146,7 +172,7 @@ public class SseItemStatesEventBuilder { } } } - if (pattern != null && !transformUsed && !optionMatched) { + if (pattern != null && !optionMatched) { // if it's not a transformation pattern and there is no matching state option, // then it must be a format string if (state instanceof QuantityType quantityState) { diff --git a/bundles/org.openhab.core.io.rest.sse/src/test/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilderTest.java b/bundles/org.openhab.core.io.rest.sse/src/test/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilderTest.java index fc4ee065b..0057551fc 100644 --- a/bundles/org.openhab.core.io.rest.sse/src/test/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilderTest.java +++ b/bundles/org.openhab.core.io.rest.sse/src/test/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilderTest.java @@ -30,6 +30,7 @@ import org.mockito.quality.Strictness; import org.openhab.core.io.rest.LocaleService; import org.openhab.core.items.Item; import org.openhab.core.items.ItemRegistry; +import org.openhab.core.library.types.DecimalType; import org.openhab.core.library.types.StringType; import org.openhab.core.service.StartLevelService; import org.openhab.core.transform.TransformationException; @@ -55,16 +56,23 @@ public class SseItemStatesEventBuilderTest { private static final String ITEM_NAME = "test"; private static final String ITEM_STATE_VALUE = "value"; private static final String ITEM_STATE_VALUE2 = "other"; + private static final int ITEM_STATE_VALUE3 = 16; private static final String ITEM_STATE_OPTION_LABEL = "The value"; private static final String PATTERN = "__ %s __"; - private static final String WRONG_PATTERN = "__ %d __"; + private static final String PATTERN2 = "__ %d __"; private static final String TRANSFORM_NAME = "TRANSFORM"; private static final String TRANSFORM_PATTERN = "Pattern"; private static final String TRANSFORM_FORMAT = "%s-1"; private static final String TRANSFORM_INPUT = String.format(TRANSFORM_FORMAT, ITEM_STATE_VALUE); - private static final String TRANSFORM_RESULT = "Result"; + private static final String TRANSFORM_INPUT2 = String.format(TRANSFORM_FORMAT, ITEM_STATE_VALUE2); + private static final String TRANSFORM_RESULT = "Result with string"; + private static final String TRANSFORM_FORMAT_NUMBER = "_%d_"; + private static final String TRANSFORM_INPUT3 = String.format(TRANSFORM_FORMAT_NUMBER, ITEM_STATE_VALUE3); + private static final String TRANSFORM_RESULT_NUMBER = "Result with number"; + private static final String TRANSFORM_RESULT_NULL = "State is NULL"; + private static final String TRANSFORM_RESULT_UNDEF = "State is UNDEF"; private @Mock @NonNullByDefault({}) ItemRegistry itemRegistryMock; private @Mock @NonNullByDefault({}) LocaleService localeServiceMock; @@ -85,6 +93,14 @@ public class SseItemStatesEventBuilderTest { public void init() throws TransformationException { Mockito.when(transformationServiceMock.transform(eq(TRANSFORM_PATTERN), eq(TRANSFORM_INPUT))) .thenAnswer(answer -> TRANSFORM_RESULT); + Mockito.when(transformationServiceMock.transform(eq(TRANSFORM_PATTERN), eq(TRANSFORM_INPUT2))) + .thenAnswer(answer -> null); + Mockito.when(transformationServiceMock.transform(eq(TRANSFORM_PATTERN), eq(TRANSFORM_INPUT3))) + .thenAnswer(answer -> TRANSFORM_RESULT_NUMBER); + Mockito.when(transformationServiceMock.transform(eq(TRANSFORM_PATTERN), eq("NULL"))) + .thenAnswer(answer -> TRANSFORM_RESULT_NULL); + Mockito.when(transformationServiceMock.transform(eq(TRANSFORM_PATTERN), eq("UNDEF"))) + .thenAnswer(answer -> TRANSFORM_RESULT_UNDEF); Mockito.when(serviceRefMock.getProperty(any())).thenReturn(TRANSFORM_NAME); @@ -139,7 +155,7 @@ public class SseItemStatesEventBuilderTest { @Test public void getDisplayStateWhenMatchingStateOptionAndWrongPattern() { - StateDescription stateDescription = StateDescriptionFragmentBuilder.create().withPattern(WRONG_PATTERN) + StateDescription stateDescription = StateDescriptionFragmentBuilder.create().withPattern(PATTERN2) .withOption(new StateOption(ITEM_STATE_VALUE, ITEM_STATE_OPTION_LABEL)).build().toStateDescription(); Mockito.when(itemMock.getStateDescription(eq(Locale.ENGLISH))).thenReturn(stateDescription); @@ -168,6 +184,14 @@ public class SseItemStatesEventBuilderTest { Mockito.when(itemMock.getState()).thenReturn(new StringType(ITEM_STATE_VALUE)); String result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); assertEquals(TRANSFORM_RESULT, result); + + StateDescription stateDescription2 = StateDescriptionFragmentBuilder.create() + .withPattern(TRANSFORM_NAME + "(" + TRANSFORM_PATTERN + "):" + TRANSFORM_FORMAT_NUMBER).build() + .toStateDescription(); + Mockito.when(itemMock.getStateDescription(eq(Locale.ENGLISH))).thenReturn(stateDescription2); + Mockito.when(itemMock.getState()).thenReturn(new DecimalType(ITEM_STATE_VALUE3)); + result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); + assertEquals(TRANSFORM_RESULT_NUMBER, result); } @Test @@ -179,6 +203,25 @@ public class SseItemStatesEventBuilderTest { Mockito.when(itemMock.getState()).thenReturn(new StringType(ITEM_STATE_VALUE)); String result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); assertEquals(TRANSFORM_RESULT, result); + + StateDescription stateDescription2 = StateDescriptionFragmentBuilder.create() + .withPattern(TRANSFORM_NAME + "(" + TRANSFORM_PATTERN + "):" + TRANSFORM_FORMAT_NUMBER).build() + .toStateDescription(); + Mockito.when(itemMock.getStateDescription(eq(Locale.ENGLISH))).thenReturn(stateDescription2); + Mockito.when(itemMock.getState()).thenReturn(new DecimalType(ITEM_STATE_VALUE3)); + result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); + assertEquals(TRANSFORM_RESULT_NUMBER, result); + } + + @Test + public void getDisplayStateWhenTransformReturningNull() { + StateDescription stateDescription = StateDescriptionFragmentBuilder.create() + .withPattern(TRANSFORM_NAME + "(" + TRANSFORM_PATTERN + "):" + TRANSFORM_FORMAT).build() + .toStateDescription(); + Mockito.when(itemMock.getStateDescription(eq(Locale.ENGLISH))).thenReturn(stateDescription); + Mockito.when(itemMock.getState()).thenReturn(new StringType(ITEM_STATE_VALUE2)); + String result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); + assertEquals(ITEM_STATE_VALUE2, result); } @Test @@ -201,6 +244,28 @@ public class SseItemStatesEventBuilderTest { assertEquals("NULL", result); } + @Test + public void getDisplayStateWhenTransformAndStateUndef() { + StateDescription stateDescription = StateDescriptionFragmentBuilder.create().withPattern(PATTERN) + .withPattern(TRANSFORM_NAME + "(" + TRANSFORM_PATTERN + "):" + TRANSFORM_FORMAT_NUMBER).build() + .toStateDescription(); + Mockito.when(itemMock.getStateDescription(eq(Locale.ENGLISH))).thenReturn(stateDescription); + Mockito.when(itemMock.getState()).thenReturn(UnDefType.UNDEF); + String result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); + assertEquals(TRANSFORM_RESULT_UNDEF, result); + } + + @Test + public void getDisplayStateWhenTransformAndStateNull() { + StateDescription stateDescription = StateDescriptionFragmentBuilder.create().withPattern(PATTERN) + .withPattern(TRANSFORM_NAME + "(" + TRANSFORM_PATTERN + "):" + TRANSFORM_FORMAT_NUMBER).build() + .toStateDescription(); + Mockito.when(itemMock.getStateDescription(eq(Locale.ENGLISH))).thenReturn(stateDescription); + Mockito.when(itemMock.getState()).thenReturn(UnDefType.NULL); + String result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); + assertEquals(TRANSFORM_RESULT_NULL, result); + } + @Test public void getDisplayStateWhenPatternProvided() { StateDescription stateDescription = StateDescriptionFragmentBuilder.create().withPattern(PATTERN).build() @@ -209,11 +274,18 @@ public class SseItemStatesEventBuilderTest { Mockito.when(itemMock.getState()).thenReturn(new StringType(ITEM_STATE_VALUE)); String result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); assertEquals(String.format(PATTERN, ITEM_STATE_VALUE), result); + + StateDescription stateDescription2 = StateDescriptionFragmentBuilder.create().withPattern(PATTERN2).build() + .toStateDescription(); + Mockito.when(itemMock.getStateDescription(eq(Locale.ENGLISH))).thenReturn(stateDescription2); + Mockito.when(itemMock.getState()).thenReturn(new DecimalType(ITEM_STATE_VALUE3)); + result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); + assertEquals(String.format(PATTERN2, ITEM_STATE_VALUE3), result); } @Test public void getDisplayStateWhenWrongPatternProvided() { - StateDescription stateDescription = StateDescriptionFragmentBuilder.create().withPattern(WRONG_PATTERN).build() + StateDescription stateDescription = StateDescriptionFragmentBuilder.create().withPattern(PATTERN2).build() .toStateDescription(); Mockito.when(itemMock.getStateDescription(eq(Locale.ENGLISH))).thenReturn(stateDescription); Mockito.when(itemMock.getState()).thenReturn(new StringType(ITEM_STATE_VALUE)); @@ -228,6 +300,10 @@ public class SseItemStatesEventBuilderTest { Mockito.when(itemMock.getState()).thenReturn(new StringType(ITEM_STATE_VALUE)); String result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); assertEquals(ITEM_STATE_VALUE, result); + + Mockito.when(itemMock.getState()).thenReturn(new DecimalType(ITEM_STATE_VALUE3)); + result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); + assertEquals(String.format("%d", ITEM_STATE_VALUE3), result); } @Test @@ -236,5 +312,9 @@ public class SseItemStatesEventBuilderTest { Mockito.when(itemMock.getState()).thenReturn(new StringType(ITEM_STATE_VALUE)); String result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); assertEquals(ITEM_STATE_VALUE, result); + + Mockito.when(itemMock.getState()).thenReturn(new DecimalType(ITEM_STATE_VALUE3)); + result = sseItemStatesEventBuilder.getDisplayState(itemMock, Locale.ENGLISH); + assertEquals(String.format("%d", ITEM_STATE_VALUE3), result); } } diff --git a/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java b/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java index 36f803ead..f5eccaf2b 100644 --- a/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java +++ b/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java @@ -86,7 +86,6 @@ import org.openhab.core.types.StateDescription; import org.openhab.core.types.StateOption; import org.openhab.core.types.UnDefType; import org.openhab.core.types.util.UnitUtils; -import org.openhab.core.ui.internal.UIActivator; import org.openhab.core.ui.items.ItemUIProvider; import org.openhab.core.ui.items.ItemUIRegistry; import org.osgi.framework.Constants; @@ -341,7 +340,7 @@ public class ItemUIRegistryImpl implements ItemUIRegistry { String itemName = w.getItem(); if (itemName == null || itemName.isBlank()) { - return transform(label, true, null); + return transform(label, true, null, null); } String labelMappedOption = null; @@ -373,7 +372,7 @@ public class ItemUIRegistryImpl implements ItemUIRegistry { state = item.getState(); if (formatPattern.contains("%d")) { - if (!(state instanceof Number)) { + if (!(state instanceof UnDefType) && !(state instanceof Number)) { // States which do not provide a Number will be converted to DecimalType. // e.g.: GroupItem can provide a count of items matching the active state // for some group functions. @@ -390,13 +389,25 @@ public class ItemUIRegistryImpl implements ItemUIRegistry { } boolean considerTransform = false; + String transformFailbackValue = null; if (formatPattern != null) { if (formatPattern.isEmpty()) { label = label.substring(0, label.indexOf("[")).trim(); } else { - if (state == null || state instanceof UnDefType) { + if (state == null) { formatPattern = formatUndefined(formatPattern); considerTransform = true; + } else if (state instanceof UnDefType) { + Matcher matcher = EXTRACT_TRANSFORM_FUNCTION_PATTERN.matcher(formatPattern); + if (matcher.find()) { + considerTransform = true; + String type = matcher.group(1); + String function = matcher.group(2); + formatPattern = type + "(" + function + "):" + state.toString(); + transformFailbackValue = "-"; + } else { + formatPattern = formatUndefined(formatPattern); + } } else { // if the channel contains options, we build a label with the mapped option value if (stateDescription != null) { @@ -457,9 +468,10 @@ public class ItemUIRegistryImpl implements ItemUIRegistry { if (matcher.find()) { considerTransform = true; String type = matcher.group(1); - String pattern = matcher.group(2); + String function = matcher.group(2); String value = matcher.group(3); - formatPattern = type + "(" + pattern + "):" + state.format(value); + formatPattern = type + "(" + function + "):" + state.format(value); + transformFailbackValue = state.toString(); } else { formatPattern = state.format(formatPattern); } @@ -478,7 +490,7 @@ public class ItemUIRegistryImpl implements ItemUIRegistry { } } - return transform(label, considerTransform, labelMappedOption); + return transform(label, considerTransform, transformFailbackValue, labelMappedOption); } @Override @@ -619,36 +631,40 @@ public class ItemUIRegistryImpl implements ItemUIRegistry { * If the value does not start with the call to a transformation service, * we return the label with the mapped option value if provided (not null). */ - private String transform(String label, boolean matchTransform, @Nullable String labelMappedOption) { + private String transform(String label, boolean matchTransform, @Nullable String transformFailbackValue, + @Nullable String labelMappedOption) { String ret = label; String formatPattern = getFormatPattern(label); if (formatPattern != null) { Matcher matcher = EXTRACT_TRANSFORM_FUNCTION_PATTERN.matcher(formatPattern); if (matchTransform && matcher.find()) { String type = matcher.group(1); - String pattern = matcher.group(2); + String function = matcher.group(2); String value = matcher.group(3); - TransformationService transformation = TransformationHelper - .getTransformationService(UIActivator.getContext(), type); - if (transformation != null) { - try { - String transformationResult = transformation.transform(pattern, value); - if (transformationResult != null) { - ret = insertInLabel(label, transformationResult); - } else { - logger.warn("transformation of type {} did not return a valid result", type); - ret = insertInLabel(label, UnDefType.NULL); + String failbackValue = transformFailbackValue != null ? transformFailbackValue : value; + try { + TransformationService transformation = TransformationHelper.getTransformationService(type); + if (transformation != null) { + try { + String transformationResult = transformation.transform(function, value); + if (transformationResult != null) { + ret = insertInLabel(label, transformationResult); + } else { + logger.warn("Transformation of type {} did not return a valid result", type); + ret = insertInLabel(label, failbackValue); + } + } catch (RuntimeException e) { + throw new TransformationException("Transformation service of type '" + type + + "' threw an exception: " + e.getMessage(), e); } - } catch (TransformationException e) { - logger.error("transformation throws exception [transformation={}, value={}]", transformation, - value, e); - ret = insertInLabel(label, value); + } else { + throw new TransformationException( + "Transformation service of type '" + type + "' is not available."); } - } else { - logger.warn( - "couldn't transform value in label because transformationService of type '{}' is unavailable", - type); - ret = insertInLabel(label, value); + } catch (TransformationException e) { + logger.warn("Failed transforming the value '{}' with pattern '{}': {}", value, formatPattern, + e.getMessage()); + ret = insertInLabel(label, failbackValue); } } else if (labelMappedOption != null) { ret = labelMappedOption; diff --git a/bundles/org.openhab.core.ui/src/test/java/org/openhab/core/ui/internal/items/ItemUIRegistryImplTest.java b/bundles/org.openhab.core.ui/src/test/java/org/openhab/core/ui/internal/items/ItemUIRegistryImplTest.java index 9470c347f..5cc7388ce 100644 --- a/bundles/org.openhab.core.ui/src/test/java/org/openhab/core/ui/internal/items/ItemUIRegistryImplTest.java +++ b/bundles/org.openhab.core.ui/src/test/java/org/openhab/core/ui/internal/items/ItemUIRegistryImplTest.java @@ -809,6 +809,45 @@ public class ItemUIRegistryImplTest { assertEquals("Memory [State]", label); } + @Test + public void getLabelFailingTransformation() throws ItemNotFoundException { + String testLabel = "Memory [FOO(echo %s):__%d__]"; + Widget w = mock(Widget.class); + Item item = mock(Item.class); + when(w.getLabel()).thenReturn(testLabel); + when(w.getItem()).thenReturn(ITEM_NAME); + when(registryMock.getItem(ITEM_NAME)).thenReturn(item); + when(item.getState()).thenReturn(new DecimalType(11)); + String label = uiRegistry.getLabel(w); + assertEquals("Memory [11]", label); + } + + @Test + public void getLabelFailingTransformationWithNullState() throws ItemNotFoundException { + String testLabel = "Memory [FOO(echo %s):__%d__]"; + Widget w = mock(Widget.class); + Item item = mock(Item.class); + when(w.getLabel()).thenReturn(testLabel); + when(w.getItem()).thenReturn(ITEM_NAME); + when(registryMock.getItem(ITEM_NAME)).thenReturn(item); + when(item.getState()).thenReturn(UnDefType.NULL); + String label = uiRegistry.getLabel(w); + assertEquals("Memory [-]", label); + } + + @Test + public void getLabelFailingTransformationWithUndefState() throws ItemNotFoundException { + String testLabel = "Memory [FOO(echo %s):__%d__]"; + Widget w = mock(Widget.class); + Item item = mock(Item.class); + when(w.getLabel()).thenReturn(testLabel); + when(w.getItem()).thenReturn(ITEM_NAME); + when(registryMock.getItem(ITEM_NAME)).thenReturn(item); + when(item.getState()).thenReturn(UnDefType.UNDEF); + String label = uiRegistry.getLabel(w); + assertEquals("Memory [-]", label); + } + @Test public void getLabelColorLabelWithDecimalValue() { String testLabel = "Label [%.3f]";