You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ad...@apache.org on 2018/11/21 16:34:54 UTC
[ambari] branch trunk updated: AMBARI-24934. Accept legacy JSON
configuration in Add Service request (#2640)
This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push:
new e62cc46 AMBARI-24934. Accept legacy JSON configuration in Add Service request (#2640)
e62cc46 is described below
commit e62cc462fb5d2ac0f2c90912d044db352c477c74
Author: Doroszlai, Attila <64...@users.noreply.github.com>
AuthorDate: Wed Nov 21 17:34:49 2018 +0100
AMBARI-24934. Accept legacy JSON configuration in Add Service request (#2640)
---
.../ambari/server/topology/ConfigurableHelper.java | 51 ++++++++++++----------
.../ambari/server/topology/ConfigurableTest.java | 42 ++++++++++--------
.../resources/add_service_api/configurable3.json | 10 +++++
3 files changed, 63 insertions(+), 40 deletions(-)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurableHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurableHelper.java
index 53cef2f..572f12d 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurableHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurableHelper.java
@@ -44,7 +44,7 @@ import com.google.common.collect.Sets;
*/
public class ConfigurableHelper {
- static final ImmutableSet<String> PERMITTED_CONFIG_FIELDS = ImmutableSet.of(PROPERTIES_PROPERTY_ID, PROPERTIES_ATTRIBUTES_PROPERTY_ID);
+ private static final ImmutableSet<String> PERMITTED_CONFIG_FIELDS = ImmutableSet.of(PROPERTIES_PROPERTY_ID, PROPERTIES_ATTRIBUTES_PROPERTY_ID);
/**
* Parses configuration maps The configs can be in fully structured JSON, e.g.
@@ -65,8 +65,6 @@ public class ConfigurableHelper {
* }]
* </code>
* In the latter case it calls {@link ConfigurationFactory#getConfiguration(Collection)}
- * @param configs
- * @return
*/
public static Configuration parseConfigs(@Nullable Collection<? extends Map<String, ?>> configs) {
Configuration configuration;
@@ -74,7 +72,7 @@ public class ConfigurableHelper {
if (null == configs || configs.isEmpty()) {
configuration = Configuration.newEmpty();
}
- else if (!configs.isEmpty() && configs.iterator().next().keySet().iterator().next().contains("/")) {
+ else if (configs.iterator().next().keySet().iterator().next().contains("/")) {
// Configuration has keys with slashes like "zk.cfg/properties/dataDir" means it is coming through
// the resource framework and must be parsed with configuration factories
checkFlattenedConfig(configs);
@@ -96,24 +94,31 @@ public class ConfigurableHelper {
checkArgument(configEntry.getValue() instanceof Map,
"The value for %s must be a JSON object (found: %s)", configName, getClassName(configEntry.getValue()));
- Map<String, Object> configData = (Map<String, Object>) configEntry.getValue();
+ Map<String, ?> configData = (Map<String, ?>) configEntry.getValue();
Set<String> extraKeys = Sets.difference(configData.keySet(), PERMITTED_CONFIG_FIELDS);
- checkArgument(extraKeys.isEmpty(), "Invalid fields in %s configuration: %s", configName, extraKeys);
+ boolean legacy = extraKeys.size() == configData.keySet().size();
+ checkArgument(legacy || extraKeys.isEmpty(), "Invalid fields in %s configuration: %s", configName, extraKeys);
- if (configData.containsKey(PROPERTIES_PROPERTY_ID)) {
- checkMap(PROPERTIES_PROPERTY_ID, configData.get(PROPERTIES_PROPERTY_ID), String.class);
-
- Map<String, String> properties = (Map<String, String>)configData.get(PROPERTIES_PROPERTY_ID);
+ if (legacy) {
+ checkMap("don't care", configData, String.class);
+ Map<String, String> properties = (Map<String, String>)configData;
allProperties.put(configName, properties);
- }
- if (configData.containsKey(PROPERTIES_ATTRIBUTES_PROPERTY_ID)) {
- checkMap(PROPERTIES_ATTRIBUTES_PROPERTY_ID, configData.get(PROPERTIES_ATTRIBUTES_PROPERTY_ID), Map.class);
- Map<String, Map<String, String>> attributes =
- (Map<String, Map<String, String>>)configData.get(PROPERTIES_ATTRIBUTES_PROPERTY_ID);
- attributes.entrySet().forEach( entry -> checkMap(entry.getKey(), entry.getValue(), String.class));
-
- allAttributes.put(configName, attributes);
+ } else {
+ if (configData.containsKey(PROPERTIES_PROPERTY_ID)) {
+ checkMap(PROPERTIES_PROPERTY_ID, configData.get(PROPERTIES_PROPERTY_ID), String.class);
+
+ Map<String, String> properties = (Map<String, String>) configData.get(PROPERTIES_PROPERTY_ID);
+ allProperties.put(configName, properties);
+ }
+ if (configData.containsKey(PROPERTIES_ATTRIBUTES_PROPERTY_ID)) {
+ checkMap(PROPERTIES_ATTRIBUTES_PROPERTY_ID, configData.get(PROPERTIES_ATTRIBUTES_PROPERTY_ID), Map.class);
+ Map<String, Map<String, String>> attributes =
+ (Map<String, Map<String, String>>) configData.get(PROPERTIES_ATTRIBUTES_PROPERTY_ID);
+ attributes.forEach((key, value) -> checkMap(key, value, String.class));
+
+ allAttributes.put(configName, attributes);
+ }
}
});
@@ -131,7 +136,7 @@ public class ConfigurableHelper {
Collection<Map<String, Map<String, ?>>> configurations = new ArrayList<>();
Set<String> allConfigTypes = Sets.union(configuration.getProperties().keySet(), configuration.getAttributes().keySet());
for (String configType: allConfigTypes) {
- Map<String, Map<String, ? extends Object>> configData = new HashMap<>();
+ Map<String, Map<String, ?>> configData = new HashMap<>();
if (configuration.getProperties().containsKey(configType)) {
configData.put(PROPERTIES_PROPERTY_ID, configuration.getProperties().get(configType));
}
@@ -167,10 +172,10 @@ public class ConfigurableHelper {
private static void checkMap(String fieldName, Object mapObj, Class<?> valueType) {
checkArgument(mapObj instanceof Map, "'%s' needs to be a JSON object. Found: %s", fieldName, getClassName(mapObj));
Map<?, ?> map = (Map<?, ?>)mapObj;
- map.forEach( (__, value) -> {
- checkArgument(valueType.isInstance(value), "Expected %s as value type, found %s, type: %s",
- valueType.getName(), value, getClassName(value));
- });
+ map.forEach( (__, value) ->
+ checkArgument(valueType.isInstance(value),
+ "Expected %s as value type, found %s, type: %s",
+ valueType.getName(), value, getClassName(value)));
}
private static String getClassName(Object object) {
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurableTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurableTest.java
index 4bbc0c3..752e3af 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurableTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurableTest.java
@@ -28,7 +28,6 @@ import java.util.List;
import java.util.regex.Pattern;
import org.apache.commons.lang3.exception.ExceptionUtils;
-import org.junit.Before;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -42,25 +41,18 @@ import com.google.common.io.Resources;
public class ConfigurableTest {
public static final String JSON_LOCATION = "add_service_api/configurable.json";
public static final String JSON_LOCATION2 = "add_service_api/configurable2.json";
+ public static final String JSON_LOCATION3 = "add_service_api/configurable3.json";
public static final String INVALID_CONFIGS_LOCATION = "add_service_api/invalid_configurables.txt";
- private TestConfigurable configurable;
- private ObjectMapper mapper;
-
- Logger LOG = LoggerFactory.getLogger(ConfigurableTest.class);
-
- @Before
- public void setUp() throws Exception {
- mapper = new ObjectMapper();
- URL url = Resources.getResource(JSON_LOCATION);
- configurable = mapper.readValue(url, TestConfigurable.class);
- }
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigurableTest.class);
/**
* Parse normal JSON configuration
*/
@Test
public void testParseConfigurable() throws Exception {
+ TestConfigurable configurable = new ObjectMapper().readValue(Resources.getResource(JSON_LOCATION), TestConfigurable.class);
+
assertEquals(ImmutableMap.of("zoo.cfg", ImmutableMap.of("dataDir", "/zookeeper1")),
configurable.getConfiguration().getProperties());
assertEquals(
@@ -83,7 +75,7 @@ public class ConfigurableTest {
for (String config: invalidConfigs) {
LOG.info("Invalid config to parse:\n{}", config);
try {
- mapper.readValue(config, TestConfigurable.class);
+ new ObjectMapper().readValue(config, TestConfigurable.class);
fail("Expected " + JsonProcessingException.class.getSimpleName() + " while processing config:\n" + config);
}
catch (JsonProcessingException ex) {
@@ -102,7 +94,9 @@ public class ConfigurableTest {
* Deserialize normal JSON configuration
*/
@Test
- public void testSerializaDeserialize() throws Exception {
+ public void testSerializeDeserialize() throws Exception {
+ ObjectMapper mapper = new ObjectMapper();
+ TestConfigurable configurable = mapper.readValue(Resources.getResource(JSON_LOCATION), TestConfigurable.class);
String persisted = mapper.writeValueAsString(configurable);
Configurable restored = mapper.readValue(persisted, TestConfigurable.class);
assertEquals(configurable.getConfiguration().getProperties(), restored.getConfiguration().getProperties());
@@ -113,10 +107,9 @@ public class ConfigurableTest {
* Parse flattened configuration
*/
@Test
- public void testParseConfigurableFromResoueceManager() throws Exception{
- mapper = new ObjectMapper();
+ public void testParseConfigurableFromResourceManager() throws Exception{
URL url = Resources.getResource(JSON_LOCATION2);
- configurable = new ObjectMapper().readValue(url, TestConfigurable.class);
+ TestConfigurable configurable = new ObjectMapper().readValue(url, TestConfigurable.class);
assertEquals(ImmutableMap.of("zoo.cfg", ImmutableMap.of("dataDir", "/zookeeper1")),
configurable.getConfiguration().getProperties());
@@ -127,6 +120,21 @@ public class ConfigurableTest {
configurable.getConfiguration().getAttributes());
}
+ /**
+ * Parse legacy configuration
+ */
+ @Test
+ public void testParseLegacyConfigurable() throws Exception {
+ URL url = Resources.getResource(JSON_LOCATION3);
+ TestConfigurable configurable = new ObjectMapper().readValue(url, TestConfigurable.class);
+
+ assertEquals(ImmutableMap.of("cluster-env", ImmutableMap.of(
+ "dataDir", "/zookeeper1",
+ "custom-property", "true"
+ )),
+ configurable.getConfiguration().getProperties());
+ }
+
static class TestConfigurable implements Configurable {
Configuration configuration;
diff --git a/ambari-server/src/test/resources/add_service_api/configurable3.json b/ambari-server/src/test/resources/add_service_api/configurable3.json
new file mode 100644
index 0000000..4fc7a68
--- /dev/null
+++ b/ambari-server/src/test/resources/add_service_api/configurable3.json
@@ -0,0 +1,10 @@
+{
+ "configurations" : [
+ {
+ "cluster-env" : {
+ "dataDir" : "/zookeeper1",
+ "custom-property": "true"
+ }
+ }
+ ]
+}