Skip to content

Commit dcadb51

Browse files
jimtngjimtng
committed
preserve standalone stateFormatter when applicable
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
1 parent 920190d commit dcadb51

2 files changed

Lines changed: 150 additions & 29 deletions

File tree

‎bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/items/fileconverter/YamlItemConverter.java‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ private YamlItemDTO buildItemDTO(Item item, List<Metadata> channelLinks, List<Me
114114
dto.label = item.getLabel();
115115
}
116116

117+
String defaultPattern = getDefaultStatePattern(item);
118+
if (stateFormatter != null && !stateFormatter.equals(defaultPattern)) {
119+
dto.format = stateFormatter;
120+
}
121+
117122
dto.type = item.getType();
118123
String mainType = ItemUtil.getMainItemType(item.getType());
119124
String dimension = ItemUtil.getItemTypeExtension(item.getType());
@@ -180,6 +185,8 @@ private YamlItemDTO buildItemDTO(Item item, List<Metadata> channelLinks, List<Me
180185
for (Metadata md : metadata) {
181186
String namespace = md.getUID().getNamespace();
182187
String value = md.getValue();
188+
String adoptedPattern = null;
189+
183190
if ("autoupdate".equals(namespace)) {
184191
// When autoupdate value is an empty string, treat it as not set since dto.autoupdate only accepts
185192
// true/false
@@ -199,27 +206,48 @@ private YamlItemDTO buildItemDTO(Item item, List<Metadata> channelLinks, List<Me
199206
continue;
200207
}
201208

202-
if ("stateDescription".equals(namespace) && (value == null || value.isBlank())) {
209+
if ("stateDescription".equals(namespace)) {
203210
Map<String, Object> config = md.getConfiguration();
211+
boolean isValueEmpty = value == null || value.isEmpty();
204212

205-
String defaultPattern = getDefaultStatePattern(item);
206-
if (config.isEmpty() && stateFormatter != null && !stateFormatter.equals(defaultPattern)) {
207-
dto.format = stateFormatter;
208-
continue;
213+
String pattern = config.get("pattern") instanceof String p && !p.isEmpty() ? p : null;
214+
boolean hasPattern = pattern != null;
215+
boolean hasOnlyPattern = hasPattern && config.size() == 1;
216+
217+
// Rule 1: Special early exit path when ONLY config.pattern is present
218+
if (hasOnlyPattern && isValueEmpty) {
219+
if (!pattern.equals(defaultPattern)) {
220+
dto.format = pattern;
221+
} else {
222+
dto.format = null;
223+
}
224+
continue; // Skip adding to dto.metadata
209225
}
210226

211-
if (config.get("pattern") instanceof String pattern && !pattern.isBlank() && config.size() == 1) {
212-
dto.format = pattern;
213-
continue;
227+
// Rule 2: Note if we need to adopt dto.format before clearing it
228+
if (!hasPattern) {
229+
adoptedPattern = dto.format;
214230
}
231+
232+
// Rule 3: Clear dto.format (stateDescription metadata takes precedence)
233+
dto.format = null;
234+
235+
// FALL THROUGH to common YamlMetadataDTO construction below...
215236
}
216237

238+
// --- Single shared YamlMetadataDTO creation block ---
217239
YamlMetadataDTO mdDto = new YamlMetadataDTO();
218240
mdDto.value = value;
219241
Map<String, Object> configuration = new LinkedHashMap<>();
220242
for (ConfigParameter param : getConfigurationParameters(md)) {
221243
configuration.put(param.name(), param.value());
222244
}
245+
246+
// Inject adopted pattern if Rule 2 was triggered
247+
if (adoptedPattern != null) {
248+
configuration.put("pattern", adoptedPattern);
249+
}
250+
223251
mdDto.config = configuration.isEmpty() ? null : configuration;
224252
metadataDto.put(namespace, mdDto);
225253
}

‎bundles/org.openhab.core.model.yaml/src/test/java/org/openhab/core/model/yaml/internal/items/fileconverter/YamlItemConverterTest.java‎

Lines changed: 114 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import org.eclipse.jdt.annotation.NonNullByDefault;
2626
import org.eclipse.jdt.annotation.Nullable;
27+
import org.junit.jupiter.api.Nested;
2728
import org.junit.jupiter.api.Test;
2829
import org.openhab.core.config.core.ConfigDescriptionRegistry;
2930
import org.openhab.core.items.Item;
@@ -99,28 +100,95 @@ public void testUnitMetadataEmptyStringStaysInShortForm() {
99100
assertNull(dto.metadata);
100101
}
101102

102-
@Test
103-
public void testStateDescriptionMetadataConvertedToShortForm() {
104-
Metadata stateDescriptionMetadata = new Metadata(new MetadataKey("stateDescription", "item_name"), "",
105-
Map.of("pattern", "%d"));
106-
YamlItemDTO dto = convertWithMetadata(stateDescriptionMetadata, "Number");
107-
assertEquals("%d", dto.format);
108-
assertNull(dto.metadata);
109-
}
103+
@Nested
104+
class StateDescriptionAndFormatTests {
110105

111-
@Test
112-
public void testStateDescriptionMetadataWithOtherConfigStaysInMetadata() {
113-
Metadata stateDescriptionMetadata = new Metadata(new MetadataKey("stateDescription", "item_name"), "",
114-
Map.of("pattern", "%d", "min", 0, "max", 100));
115-
YamlItemDTO dto = convertWithMetadata(stateDescriptionMetadata, "Number");
116-
assertNull(dto.format);
117-
assertNotNull(dto.metadata);
118-
YamlMetadataDTO stateDescDto = dto.metadata.get("stateDescription");
119-
assertNotNull(stateDescDto);
120-
assertEquals("", stateDescDto.getValue());
121-
assertEquals("%d", stateDescDto.config.get("pattern"));
122-
assertEquals(0, stateDescDto.config.get("min"));
123-
assertEquals(100, stateDescDto.config.get("max"));
106+
@Test
107+
public void testStateDescriptionMetadataConvertedToShortForm() {
108+
Metadata stateDescriptionMetadata = new Metadata(new MetadataKey("stateDescription", "item_name"), "",
109+
Map.of("pattern", "%d"));
110+
YamlItemDTO dto = convertWithMetadata(stateDescriptionMetadata, "Number");
111+
assertEquals("%d", dto.format);
112+
assertNull(dto.metadata);
113+
}
114+
115+
@Test
116+
public void testStateDescriptionMetadataWithOtherConfigStaysInMetadata() {
117+
Metadata stateDescriptionMetadata = new Metadata(new MetadataKey("stateDescription", "item_name"), "",
118+
Map.of("pattern", "%d", "min", 0, "max", 100));
119+
YamlItemDTO dto = convertWithMetadata(stateDescriptionMetadata, "Number");
120+
assertNull(dto.format);
121+
assertNotNull(dto.metadata);
122+
YamlMetadataDTO stateDescDto = dto.metadata.get("stateDescription");
123+
assertNotNull(stateDescDto);
124+
assertEquals("", stateDescDto.getValue());
125+
assertEquals("%d", stateDescDto.config.get("pattern"));
126+
assertEquals(0, stateDescDto.config.get("min"));
127+
assertEquals(100, stateDescDto.config.get("max"));
128+
}
129+
130+
@Test
131+
public void testStandaloneStateFormatterPreservedWithoutStateDescription() {
132+
// Test standalone stateFormatter (e.g., extracted from item label or file-format API)
133+
// when no stateDescription metadata exists.
134+
YamlItemDTO dto = convertWithStateFormatterAndMetadata("Number", "%.2f %s", List.of());
135+
assertEquals("%.2f %s", dto.format);
136+
assertNull(dto.metadata);
137+
}
138+
139+
@Test
140+
public void testStandaloneStateFormatterWithStateDescriptionWithoutPattern() {
141+
// Test when standalone stateFormatter is provided along with stateDescription metadata
142+
// that has min/max config but NO pattern.
143+
Metadata stateDescMeta = new Metadata(new MetadataKey("stateDescription", "item_name"), "",
144+
Map.of("min", 0, "max", 100));
145+
146+
YamlItemDTO dto = convertWithStateFormatterAndMetadata("Number", "%.1f °C", List.of(stateDescMeta));
147+
148+
// When stateDescription metadata exists with other params, dto.format should be cleared
149+
// because stateDescription takes precedence, but stateDescription config will adopt or keep format
150+
// parameters.
151+
assertNull(dto.format);
152+
assertNotNull(dto.metadata);
153+
YamlMetadataDTO stateDescDto = dto.metadata.get("stateDescription");
154+
assertNotNull(stateDescDto);
155+
assertEquals("", stateDescDto.getValue());
156+
assertEquals(0, stateDescDto.config.get("min"));
157+
assertEquals(100, stateDescDto.config.get("max"));
158+
// Verify adopted or preserved pattern in stateDescription config
159+
assertEquals("%.1f °C", stateDescDto.config.get("pattern"));
160+
}
161+
162+
@Test
163+
public void testStateDescriptionPatternOverridesStandaloneStateFormatter() {
164+
// Test when stateDescription metadata HAS a pattern and standalone stateFormatter is also provided.
165+
// The stateDescription pattern should override/take precedence.
166+
Metadata stateDescMeta = new Metadata(new MetadataKey("stateDescription", "item_name"), "",
167+
Map.of("pattern", "%d kWh"));
168+
169+
YamlItemDTO dto = convertWithStateFormatterAndMetadata("Number", "%.2f", List.of(stateDescMeta));
170+
171+
// Short-form format should reflect the stateDescription pattern (%d kWh),
172+
// not the standalone stateFormatter (%.2f)
173+
assertEquals("%d kWh", dto.format);
174+
assertNull(dto.metadata);
175+
}
176+
177+
@Test
178+
public void testStateFormatterDslToYamlRoundTripInteraction() {
179+
// Simulate DSL -> YAML conversion flow with a standalone stateFormatter and additional metadata
180+
Metadata expireMeta = new Metadata(new MetadataKey("expire", "item_name"), "5m", Map.of());
181+
Metadata unitMeta = new Metadata(new MetadataKey("unit", "item_name"), "°C", Map.of());
182+
183+
YamlItemDTO dto = convertWithStateFormatterAndMetadata("Number:Temperature", "%.1f %unit%",
184+
List.of(expireMeta, unitMeta));
185+
186+
// Standalone stateFormatter is converted to short-form format
187+
assertEquals("%.1f %unit%", dto.format);
188+
assertEquals("5m", dto.expire);
189+
assertEquals("°C", dto.unit);
190+
assertNull(dto.metadata);
191+
}
124192
}
125193

126194
private YamlItemDTO convertWithMetadata(Metadata metadata, String itemType) {
@@ -145,6 +213,31 @@ private YamlItemDTO convertWithMetadata(Metadata metadata, String itemType) {
145213
return (YamlItemDTO) elements.getFirst();
146214
}
147215

216+
private YamlItemDTO convertWithStateFormatterAndMetadata(String itemType, @Nullable String stateFormatter,
217+
List<Metadata> metadataList) {
218+
CapturingYamlModelRepository repository = new CapturingYamlModelRepository();
219+
YamlItemConverter converter = new YamlItemConverter(repository, mock(YamlItemProvider.class),
220+
mock(YamlMetadataProvider.class), mock(YamlChannelLinkProvider.class),
221+
mock(ConfigDescriptionRegistry.class));
222+
223+
Item item = mock(Item.class);
224+
when(item.getName()).thenReturn("item_name");
225+
when(item.getLabel()).thenReturn(null);
226+
when(item.getType()).thenReturn(itemType);
227+
when(item.getCategory()).thenReturn(null);
228+
when(item.getGroupNames()).thenReturn(List.of());
229+
when(item.getTags()).thenReturn(Set.of());
230+
231+
Map<String, String> stateFormatters = stateFormatter != null ? Map.of("item_name", stateFormatter) : Map.of();
232+
233+
converter.setItemsToBeSerialized("id", List.of(item), metadataList, stateFormatters, false);
234+
235+
List<YamlElement> elements = repository.getElements();
236+
assertEquals(1, elements.size());
237+
assertInstanceOf(YamlItemDTO.class, elements.getFirst());
238+
return (YamlItemDTO) elements.getFirst();
239+
}
240+
148241
private static class CapturingYamlModelRepository implements YamlModelRepository {
149242

150243
private List<YamlElement> elements = new ArrayList<>();

0 commit comments

Comments
 (0)