This repository has been archived by the owner on May 7, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 786
core/ConfigUtil: Do not normalize objectClass, component.name, component.id configuration values #4109
Merged
Merged
core/ConfigUtil: Do not normalize objectClass, component.name, component.id configuration values #4109
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
45 changes: 0 additions & 45 deletions
45
....config.core.test/src/test/groovy/org/eclipse/smarthome/config/core/ConfigUtilTest.groovy
This file was deleted.
Oops, something went wrong.
57 changes: 57 additions & 0 deletions
57
...home.config.core.test/src/test/java/org/eclipse/smarthome/config/core/ConfigUtilTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
/** | ||
* Copyright (c) 2014-2017 by the respective copyright holders. | ||
* All rights reserved. This program and the accompanying materials | ||
* are made available under the terms of the Eclipse Public License v1.0 | ||
* which accompanies this distribution, and is available at | ||
* http://www.eclipse.org/legal/epl-v10.html | ||
*/ | ||
package org.eclipse.smarthome.config.core; | ||
|
||
import static org.hamcrest.CoreMatchers.*; | ||
import static org.junit.Assert.assertThat; | ||
|
||
import java.math.BigDecimal; | ||
import java.net.*; | ||
import java.util.*; | ||
|
||
import org.eclipse.smarthome.config.core.ConfigDescriptionParameter.Type; | ||
import org.junit.Test; | ||
|
||
/** | ||
* | ||
* @author Simon Kaufmann - initial contribution and API. | ||
* | ||
*/ | ||
class ConfigUtilTest { | ||
private Map<String, Object> m(String a, Object b) { | ||
Map<String, Object> m = new HashMap<>(); | ||
m.put(a, b); | ||
return m; | ||
} | ||
|
||
private static class L<T> extends ArrayList<T> { | ||
L(T... args) { | ||
for (T arg : args) { | ||
add(arg); | ||
} | ||
} | ||
}; | ||
|
||
@Test | ||
public void firstDesciptionWinsForNormalization() throws URISyntaxException { | ||
ConfigDescription configDescriptionInteger = new ConfigDescription(new URI("thing:fooThing"), | ||
new L<>(new ConfigDescriptionParameter("foo", Type.INTEGER))); | ||
|
||
ConfigDescription configDescriptionString = new ConfigDescription(new URI("thingType:fooThing"), | ||
new L<>(new ConfigDescriptionParameter("foo", Type.TEXT))); | ||
|
||
assertThat(ConfigUtil.normalizeTypes(m("foo", "1"), new L<>(configDescriptionInteger)).get("foo"), | ||
is(instanceOf(BigDecimal.class))); | ||
assertThat(ConfigUtil.normalizeTypes(m("foo", "1"), new L<>(configDescriptionString)).get("foo"), | ||
is(instanceOf(String.class))); | ||
assertThat(ConfigUtil.normalizeTypes(m("foo", "1"), new L<>(configDescriptionInteger, configDescriptionString)) | ||
.get("foo"), is(instanceOf(BigDecimal.class))); | ||
assertThat(ConfigUtil.normalizeTypes(m("foo", "1"), new L<>(configDescriptionString, configDescriptionInteger)) | ||
.get("foo"), is(instanceOf(String.class))); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,14 @@ | |
* @author Thomas Höfer - Minor changes for type normalization based on config description | ||
*/ | ||
public class ConfigUtil { | ||
/** | ||
* We do not want to handle or try to normalize OSGi provided configuration parameters | ||
* | ||
* @param name The configuration parameter name | ||
*/ | ||
private static boolean isOSGiConfigParameter(String name) { | ||
return name.equals("objectClass") || name.equals("component.name") || name.equals("component.id"); | ||
} | ||
|
||
/** | ||
* Normalizes the types to the ones allowed for configurations. | ||
|
@@ -35,11 +43,13 @@ public class ConfigUtil { | |
* @return normalized configuration | ||
*/ | ||
public static Map<String, Object> normalizeTypes(Map<String, Object> configuration) { | ||
Map<String, Object> convertedConfiguration = new HashMap<String, Object>(configuration.size()); | ||
Map<String, Object> convertedConfiguration = new HashMap<>(configuration.size()); | ||
for (Entry<String, Object> parameter : configuration.entrySet()) { | ||
String name = parameter.getKey(); | ||
Object value = parameter.getValue(); | ||
convertedConfiguration.put(name, normalizeType(value)); | ||
if (!isOSGiConfigParameter(name)) { | ||
convertedConfiguration.put(name, normalizeType(value)); | ||
} | ||
} | ||
return convertedConfiguration; | ||
} | ||
|
@@ -129,7 +139,7 @@ public static Map<String, Object> normalizeTypes(Map<String, Object> configurati | |
return null; | ||
} | ||
|
||
Map<String, Object> convertedConfiguration = new HashMap<String, Object>(configuration.size()); | ||
Map<String, Object> convertedConfiguration = new HashMap<>(); | ||
|
||
Map<String, ConfigDescriptionParameter> configParams = new HashMap<>(); | ||
for (int i = configDescriptions.size() - 1; i >= 0; i--) { | ||
|
@@ -138,8 +148,10 @@ public static Map<String, Object> normalizeTypes(Map<String, Object> configurati | |
for (Entry<String, ?> parameter : configuration.entrySet()) { | ||
String name = parameter.getKey(); | ||
Object value = parameter.getValue(); | ||
ConfigDescriptionParameter configDescriptionParameter = configParams.get(name); | ||
convertedConfiguration.put(name, normalizeType(value, configDescriptionParameter)); | ||
if (!isOSGiConfigParameter(name)) { | ||
ConfigDescriptionParameter configDescriptionParameter = configParams.get(name); | ||
convertedConfiguration.put(name, normalizeType(value, configDescriptionParameter)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, don't you want to add an (un-normalized) OSGI parameter to your return map? |
||
} | ||
} | ||
return convertedConfiguration; | ||
} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only add the
value
to yourconvertedConfiguration
if it is not an OSGI parameter, is that by intention or is there anmissing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's the entire purpose of this PR, to ignore the OSGi parameters completely. They contain either dots so cannot be matched with a variable name or are complex objects, so cannot be marshaled to values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 - these values don't make any sense for the service itself. It knows its name and pid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah Ok, then the name of this PR was a bit misleading for me, because it soudned like you only want to omit the "normalization" and not the entire parameters. thanks for the clarification!