Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

core/ConfigUtil: Do not normalize objectClass, component.name, component.id configuration values #4109

Merged
merged 1 commit into from
Aug 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

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)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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));
Copy link
Contributor

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 your convertedConfiguration if it is not an OSGI parameter, is that by intention or is there an

else {
    convertedConfiguration.put(name, value);
}

missing?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

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!

}
}
return convertedConfiguration;
}
Expand Down Expand Up @@ -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--) {
Expand All @@ -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));
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
}
Expand Down