Skip to content

Commit 478a7de

Browse files
lolodomololodomo
authored
YAML config: enhance hashCode() and add new unit tests (openhab#4941)
Reduce potential collisions in hashCode() Make identical a null list/set and an empty list/set Test hashCode in addition to equals in unit tests. Add new tests for YamlItemDTO (equals/hashCode) Related to openhab#4936 Signed-off-by: Laurent Garnier <lg.hc@free.fr>
1 parent f7e07bd commit 478a7de

10 files changed

Lines changed: 410 additions & 124 deletions

File tree

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,13 @@ public String getFunction() {
6868
return function != null ? function.toUpperCase() : DEFAULT_FUNCTION;
6969
}
7070

71+
public @NonNull List<@NonNull String> getParameters() {
72+
return parameters == null ? List.of() : parameters;
73+
}
74+
7175
@Override
7276
public int hashCode() {
73-
return Objects.hash(getBaseType(), getFunction());
77+
return Objects.hash(getBaseType(), getFunction(), getParameters());
7478
}
7579

7680
@Override
@@ -82,6 +86,6 @@ public boolean equals(@Nullable Object obj) {
8286
}
8387
YamlGroupDTO other = (YamlGroupDTO) obj;
8488
return Objects.equals(getBaseType(), other.getBaseType()) && Objects.equals(getFunction(), other.getFunction())
85-
&& YamlElementUtils.equalsListStrings(parameters, other.parameters);
89+
&& Objects.equals(getParameters(), other.getParameters());
8690
}
8791
}

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,18 @@ private void addToList(@Nullable List<@NonNull String> list, String value) {
250250
return YamlElementUtils.getItemTypeWithDimension(type, dimension);
251251
}
252252

253+
public @NonNull List<@NonNull String> getGroups() {
254+
return groups == null ? List.of() : groups;
255+
}
256+
257+
public @NonNull Set<@NonNull String> getTags() {
258+
return tags == null ? Set.of() : tags;
259+
}
260+
253261
@Override
254262
public int hashCode() {
255-
return Objects.hash(name, getType(), label, icon);
263+
return Objects.hash(name, getType(), group, label, icon, format, unit, autoupdate, getGroups(), getTags(),
264+
channel);
256265
}
257266

258267
@Override
@@ -267,9 +276,9 @@ public boolean equals(@Nullable Object obj) {
267276
&& Objects.equals(group, other.group) && Objects.equals(label, other.label)
268277
&& Objects.equals(icon, other.icon) && Objects.equals(format, other.format)
269278
&& Objects.equals(unit, other.unit) && Objects.equals(autoupdate, other.autoupdate)
270-
&& YamlElementUtils.equalsListStrings(groups, other.groups)
271-
&& YamlElementUtils.equalsSetStrings(tags, other.tags) && Objects.equals(channel, other.channel)
272-
&& equalsChannels(channels, other.channels) && equalsMetadata(metadata, other.metadata);
279+
&& Objects.equals(getGroups(), other.getGroups()) && Objects.equals(getTags(), other.getTags())
280+
&& Objects.equals(channel, other.channel) && equalsChannels(channels, other.channels)
281+
&& equalsMetadata(metadata, other.metadata);
273282
}
274283

275284
private boolean equalsChannels(@Nullable Map<@NonNull String, @NonNull Map<@NonNull String, @NonNull Object>> first,

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,7 @@ public void removedModel(String modelName, Collection<YamlItemDTO> elements) {
181181
if (baseItem != null) {
182182
GroupFunctionDTO groupFunctionDto = new GroupFunctionDTO();
183183
groupFunctionDto.name = groupDTO.getFunction();
184-
groupFunctionDto.params = groupDTO.parameters != null
185-
? groupDTO.parameters.toArray(new String[0])
186-
: new String[0];
184+
groupFunctionDto.params = groupDTO.getParameters().toArray(new String[0]);
187185
item = new GroupItem(name, baseItem, ItemDTOMapper.mapFunction(baseItem, groupFunctionDto));
188186
} else {
189187
item = new GroupItem(name);
@@ -203,15 +201,11 @@ public void removedModel(String modelName, Collection<YamlItemDTO> elements) {
203201
genericItem.setLabel(itemDTO.label);
204202
genericItem.setCategory(itemDTO.icon);
205203

206-
if (itemDTO.tags != null) {
207-
for (String tag : itemDTO.tags) {
208-
genericItem.addTag(tag);
209-
}
204+
for (String tag : itemDTO.getTags()) {
205+
genericItem.addTag(tag);
210206
}
211-
if (itemDTO.groups != null) {
212-
for (String groupName : itemDTO.groups) {
213-
genericItem.addGroupName(groupName);
214-
}
207+
for (String groupName : itemDTO.getGroups()) {
208+
genericItem.addGroupName(groupName);
215209
}
216210
}
217211

‎bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingDTO.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public boolean isBridge() {
144144

145145
@Override
146146
public int hashCode() {
147-
return Objects.hash(uid, bridge, label, location);
147+
return Objects.hash(uid, isBridge(), bridge, label, location);
148148
}
149149

150150
@Override

‎bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/util/YamlElementUtils.java‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,6 @@ private static boolean equalsConfigValue(Object first, @Nullable Object second)
5151
: first.equals(second);
5252
}
5353

54-
public static boolean equalsListStrings(@Nullable List<String> first, @Nullable List<String> second) {
55-
if (first != null && second != null) {
56-
return Arrays.equals(first.toArray(), second.toArray());
57-
} else {
58-
return first == null && second == null;
59-
}
60-
}
61-
62-
public static boolean equalsSetStrings(@Nullable Set<String> first, @Nullable Set<String> second) {
63-
if (first != null && second != null) {
64-
return first.size() != second.size() ? false : first.containsAll(second);
65-
} else {
66-
return first == null && second == null;
67-
}
68-
}
69-
7054
public static @Nullable String getAdjustedItemType(@Nullable String type) {
7155
return type == null ? null : StringUtils.capitalize(type);
7256
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,22 +100,27 @@ public void testEquals() throws IOException {
100100
gr1.type = "String";
101101
gr2.type = "String";
102102
assertTrue(gr1.equals(gr2));
103+
assertEquals(gr1.hashCode(), gr2.hashCode());
103104
gr1.type = "String";
104105
gr2.type = "Number";
105106
assertFalse(gr1.equals(gr2));
106107
gr1.type = "Number";
107108
gr2.type = "Number";
108109
assertTrue(gr1.equals(gr2));
110+
assertEquals(gr1.hashCode(), gr2.hashCode());
109111
gr2.type = "number";
110112
assertTrue(gr1.equals(gr2));
113+
assertEquals(gr1.hashCode(), gr2.hashCode());
111114
gr1.dimension = "Temperature";
112115
assertFalse(gr1.equals(gr2));
113116
gr2.dimension = "Humidity";
114117
assertFalse(gr1.equals(gr2));
115118
gr2.dimension = "Temperature";
116119
assertTrue(gr1.equals(gr2));
120+
assertEquals(gr1.hashCode(), gr2.hashCode());
117121
gr2.dimension = "temperature";
118122
assertTrue(gr1.equals(gr2));
123+
assertEquals(gr1.hashCode(), gr2.hashCode());
119124

120125
gr1.function = "or";
121126
gr2.function = null;
@@ -126,10 +131,13 @@ public void testEquals() throws IOException {
126131
gr1.function = "or";
127132
gr2.function = "or";
128133
assertTrue(gr1.equals(gr2));
134+
assertEquals(gr1.hashCode(), gr2.hashCode());
129135
gr2.function = "Or";
130136
assertTrue(gr1.equals(gr2));
137+
assertEquals(gr1.hashCode(), gr2.hashCode());
131138
gr2.function = "OR";
132139
assertTrue(gr1.equals(gr2));
140+
assertEquals(gr1.hashCode(), gr2.hashCode());
133141

134142
gr1.parameters = List.of("ON", "OFF");
135143
gr2.parameters = null;
@@ -143,5 +151,6 @@ public void testEquals() throws IOException {
143151
assertFalse(gr1.equals(gr2));
144152
gr1.parameters = List.of("ON", "OFF");
145153
assertTrue(gr1.equals(gr2));
154+
assertEquals(gr1.hashCode(), gr2.hashCode());
146155
}
147156
}

0 commit comments

Comments
 (0)