Repository: ambari Updated Branches: refs/heads/trunk ad1ce5097 -> 2eab61c82
AMBARI-5987 - VIews: On View Instance create, should validate that req props are provided Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/2eab61c8 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/2eab61c8 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/2eab61c8 Branch: refs/heads/trunk Commit: 2eab61c824d5f5ea71330e0071b76f89950afa5f Parents: ad1ce50 Author: tbeerbower <[email protected]> Authored: Mon Jun 2 12:28:03 2014 -0400 Committer: tbeerbower <[email protected]> Committed: Mon Jun 2 13:06:48 2014 -0400 ---------------------------------------------------------------------- .../server/orm/entities/ViewInstanceEntity.java | 29 +++ .../apache/ambari/server/view/ViewRegistry.java | 38 ++- .../orm/entities/ViewInstanceEntityTest.java | 88 +++++++ .../ambari/server/view/ViewRegistryTest.java | 235 ++++++++++++++++++- 4 files changed, 379 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/2eab61c8/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java index 6456492..a4900e6 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java @@ -43,6 +43,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Set; /** * Represents an instance of a View. @@ -510,6 +511,34 @@ public class ViewInstanceEntity implements ViewInstanceDefinition { return userNameProvider.getUsername(); } + /** + * Validate the state of the instance. + * + * @param viewEntity the view entity to which this instance will be bound + * + * @throws IllegalStateException if the instance is not in a valid state + */ + public void validate(ViewEntity viewEntity) throws IllegalStateException { + + // make sure that there is an instance property value defined + // for each required view parameter + Set<String> requiredParamterNames = new HashSet<String>(); + for (ViewParameterEntity parameter : viewEntity.getParameters()) { + if (parameter.isRequired()) { + requiredParamterNames.add(parameter.getName()); + } + } + for (ViewInstancePropertyEntity property : getProperties()) { + requiredParamterNames.remove(property.getName()); + } + + if (!requiredParamterNames.isEmpty()) { + throw new IllegalStateException("No property values exist for the required parameters " + + requiredParamterNames); + } + } + + // ----- helper methods ---------------------------------------------------- // get the current user name http://git-wip-us.apache.org/repos/asf/ambari/blob/2eab61c8/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java b/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java index 2a7f334..966f50a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java @@ -337,7 +337,12 @@ public class ViewRegistry { Set<ViewInstanceEntity> instanceDefinitions = new HashSet<ViewInstanceEntity>(); for (InstanceConfig instanceConfig : viewConfig.getInstances()) { - instanceDefinitions.add(createViewInstanceDefinition(viewDefinition, instanceConfig)); + try { + instanceDefinitions.add(createViewInstanceDefinition(viewDefinition, instanceConfig)); + } catch (Exception e) { + LOG.error("Caught exception adding view instance for view " + + viewDefinition.getViewName(), e); + } } // ensure that the view entity matches the db instanceDefinitions.addAll(persistView(viewDefinition)); @@ -366,11 +371,16 @@ public class ViewRegistry { } /** - * Install a view instance for the view with the given view name. + * Install the given view instance with its associated view. * * @param instanceEntity the view instance entity + * + * @throws IllegalStateException if the given instance is not in a valid state + * @throws IllegalArgumentException if the view associated with the given instance + * does not exist */ - public void installViewInstance(ViewInstanceEntity instanceEntity){ + public void installViewInstance(ViewInstanceEntity instanceEntity) + throws IllegalStateException, IllegalArgumentException { ViewEntity viewEntity = getDefinition(instanceEntity.getViewName()); if (viewEntity != null) { @@ -383,15 +393,18 @@ public class ViewRegistry { LOG.debug("Creating view instance " + viewName + "/" + version + "/" + instanceName); } + instanceEntity.validate(viewEntity); instanceDAO.create(instanceEntity); try { // bind the view instance to a view bindViewInstance(viewEntity, instanceEntity); - // update the registry - addInstanceDefinition(viewEntity, instanceEntity); - } catch (ClassNotFoundException e) { - LOG.error("Caught exception installing view instance.", e); + } catch (Exception e) { + String message = "Caught exception installing view instance."; + LOG.error(message, e); + throw new IllegalStateException(message, e); } + // update the registry + addInstanceDefinition(viewEntity, instanceEntity); } } else { String message = "Attempt to install an instance for an unknown view " + @@ -406,8 +419,11 @@ public class ViewRegistry { * Update a view instance for the view with the given view name. * * @param instanceEntity the view instance entity + * + * @throws IllegalStateException if the given instance is not in a valid state */ - public void updateViewInstance(ViewInstanceEntity instanceEntity) { + public void updateViewInstance(ViewInstanceEntity instanceEntity) + throws IllegalStateException { ViewEntity viewEntity = getDefinition(instanceEntity.getViewName()); if (viewEntity != null) { @@ -426,6 +442,7 @@ public class ViewRegistry { entity.setProperties(instanceEntity.getProperties()); entity.setData(instanceEntity.getData()); + instanceEntity.validate(viewEntity); instanceDAO.merge(entity); } } @@ -635,20 +652,21 @@ public class ViewRegistry { // create a new view instance definition protected ViewInstanceEntity createViewInstanceDefinition(ViewEntity viewDefinition, InstanceConfig instanceConfig) - throws ClassNotFoundException { + throws ClassNotFoundException, IllegalStateException { ViewInstanceEntity viewInstanceDefinition = new ViewInstanceEntity(viewDefinition, instanceConfig); for (PropertyConfig propertyConfig : instanceConfig.getProperties()) { viewInstanceDefinition.putProperty(propertyConfig.getKey(), propertyConfig.getValue()); } + viewInstanceDefinition.validate(viewDefinition); bindViewInstance(viewDefinition, viewInstanceDefinition); return viewInstanceDefinition; } // bind a view instance definition to the given view definition - private void bindViewInstance(ViewEntity viewDefinition, + protected void bindViewInstance(ViewEntity viewDefinition, ViewInstanceEntity viewInstanceDefinition) throws ClassNotFoundException { viewInstanceDefinition.setViewEntity(viewDefinition); http://git-wip-us.apache.org/repos/asf/ambari/blob/2eab61c8/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java index 7b4fcff..a6653cc 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java @@ -18,14 +18,19 @@ package org.apache.ambari.server.orm.entities; +import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.controller.spi.Resource; +import org.apache.ambari.server.view.ViewRegistryTest; import org.apache.ambari.server.view.configuration.InstanceConfig; import org.apache.ambari.server.view.configuration.InstanceConfigTest; +import org.apache.ambari.server.view.configuration.ViewConfig; +import org.apache.ambari.server.view.configuration.ViewConfigTest; import org.apache.ambari.view.ResourceProvider; import org.junit.Assert; import org.junit.Test; import java.util.Map; +import java.util.Properties; import static org.easymock.EasyMock.createNiceMock; @@ -53,6 +58,54 @@ public class ViewInstanceEntityTest { " </instance>\n" + "</view>"; + private static String xml_valid_instance = "<view>\n" + + " <name>MY_VIEW</name>\n" + + " <label>My View!</label>\n" + + " <version>1.0.0</version>\n" + + " <parameter>\n" + + " <name>p1</name>\n" + + " <description>Parameter 1.</description>\n" + + " <required>true</required>\n" + + " </parameter>\n" + + " <parameter>\n" + + " <name>p2</name>\n" + + " <description>Parameter 2.</description>\n" + + " <required>false</required>\n" + + " </parameter>\n" + + " <instance>\n" + + " <name>INSTANCE1</name>\n" + + " <label>My Instance 1!</label>\n" + + " <property>\n" + + " <key>p1</key>\n" + + " <value>v1-1</value>\n" + + " </property>\n" + + " <property>\n" + + " <key>p2</key>\n" + + " <value>v2-1</value>\n" + + " </property>\n" + + " </instance>\n" + + "</view>"; + + private static String xml_invalid_instance = "<view>\n" + + " <name>MY_VIEW</name>\n" + + " <label>My View!</label>\n" + + " <version>1.0.0</version>\n" + + " <parameter>\n" + + " <name>p1</name>\n" + + " <description>Parameter 1.</description>\n" + + " <required>true</required>\n" + + " </parameter>\n" + + " <parameter>\n" + + " <name>p2</name>\n" + + " <description>Parameter 2.</description>\n" + + " <required>false</required>\n" + + " </parameter>\n" + + " <instance>\n" + + " <name>INSTANCE1</name>\n" + + " <label>My Instance 1!</label>\n" + + " </instance>\n" + + "</view>"; + @Test public void testGetViewEntity() throws Exception { InstanceConfig instanceConfig = InstanceConfigTest.getInstanceConfigs().get(0); @@ -220,6 +273,41 @@ public class ViewInstanceEntityTest { return new ViewInstanceEntity(viewDefinition, instanceConfig); } + @Test + public void testValidate() throws Exception { + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + + ViewConfig config = ViewConfigTest.getConfig(xml_valid_instance); + ViewEntity viewEntity = ViewRegistryTest.getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + ViewInstanceEntity viewInstanceEntity = ViewRegistryTest.getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + + viewInstanceEntity.validate(viewEntity); + } + + @Test + public void testValidate_fail() throws Exception { + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + + ViewConfig config = ViewConfigTest.getConfig(xml_invalid_instance); + ViewEntity viewEntity = ViewRegistryTest.getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + ViewInstanceEntity viewInstanceEntity = ViewRegistryTest.getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + + try { + viewInstanceEntity.validate(viewEntity); + Assert.fail("Expected an IllegalStateException"); + } catch (IllegalStateException e) { + // expected + } + } + public static ViewInstanceEntity getViewInstanceEntity(ViewInstanceEntity.UserNameProvider userNameProvider) throws Exception { ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(); http://git-wip-us.apache.org/repos/asf/ambari/blob/2eab61c8/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java b/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java index fc493ab..a980696 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java @@ -31,9 +31,11 @@ import org.apache.ambari.server.orm.entities.ViewInstanceEntity; import org.apache.ambari.server.orm.entities.ViewInstanceEntityTest; import org.apache.ambari.server.view.configuration.InstanceConfig; import org.apache.ambari.server.view.configuration.InstanceConfigTest; +import org.apache.ambari.server.view.configuration.PropertyConfig; import org.apache.ambari.server.view.configuration.ResourceConfig; import org.apache.ambari.server.view.configuration.ResourceConfigTest; import org.apache.ambari.server.view.configuration.ViewConfig; +import org.apache.ambari.server.view.configuration.ViewConfigTest; import org.apache.ambari.server.view.events.EventImpl; import org.apache.ambari.server.view.events.EventImplTest; import org.apache.ambari.view.events.Event; @@ -57,6 +59,7 @@ import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.Map; +import java.util.Properties; import java.util.Set; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -86,6 +89,54 @@ public class ViewRegistryTest { " <version>2.0.0</version>\n" + "</view>"; + private static String xml_valid_instance = "<view>\n" + + " <name>MY_VIEW</name>\n" + + " <label>My View!</label>\n" + + " <version>1.0.0</version>\n" + + " <parameter>\n" + + " <name>p1</name>\n" + + " <description>Parameter 1.</description>\n" + + " <required>true</required>\n" + + " </parameter>\n" + + " <parameter>\n" + + " <name>p2</name>\n" + + " <description>Parameter 2.</description>\n" + + " <required>false</required>\n" + + " </parameter>\n" + + " <instance>\n" + + " <name>INSTANCE1</name>\n" + + " <label>My Instance 1!</label>\n" + + " <property>\n" + + " <key>p1</key>\n" + + " <value>v1-1</value>\n" + + " </property>\n" + + " <property>\n" + + " <key>p2</key>\n" + + " <value>v2-1</value>\n" + + " </property>\n" + + " </instance>\n" + + "</view>"; + + private static String xml_invalid_instance = "<view>\n" + + " <name>MY_VIEW</name>\n" + + " <label>My View!</label>\n" + + " <version>1.0.0</version>\n" + + " <parameter>\n" + + " <name>p1</name>\n" + + " <description>Parameter 1.</description>\n" + + " <required>true</required>\n" + + " </parameter>\n" + + " <parameter>\n" + + " <name>p2</name>\n" + + " <description>Parameter 2.</description>\n" + + " <required>false</required>\n" + + " </parameter>\n" + + " <instance>\n" + + " <name>INSTANCE1</name>\n" + + " <label>My Instance 1!</label>\n" + + " </instance>\n" + + "</view>"; + @Test public void testReadViewArchives() throws Exception { Configuration configuration = createNiceMock(Configuration.class); @@ -402,6 +453,180 @@ public class ViewRegistryTest { } @Test + public void testInstallViewInstance() throws Exception { + + ViewDAO viewDAO = createNiceMock(ViewDAO.class); + ViewInstanceDAO viewInstanceDAO = createNiceMock(ViewInstanceDAO.class); + + ViewRegistry.init(viewDAO, viewInstanceDAO); + + ViewRegistry registry = ViewRegistry.getInstance(); + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + + ViewConfig config = ViewConfigTest.getConfig(xml_valid_instance); + ViewEntity viewEntity = getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + + viewInstanceDAO.create(viewInstanceEntity); + + replay(viewDAO, viewInstanceDAO); + + registry.addDefinition(viewEntity); + registry.installViewInstance(viewInstanceEntity); + + Collection<ViewInstanceEntity> viewInstanceDefinitions = registry.getInstanceDefinitions(viewEntity); + + Assert.assertEquals(1, viewInstanceDefinitions.size()); + + Assert.assertEquals(viewInstanceEntity, viewInstanceDefinitions.iterator().next()); + + verify(viewDAO, viewInstanceDAO); + } + + @Test + public void testInstallViewInstance_invalid() throws Exception { + + ViewDAO viewDAO = createNiceMock(ViewDAO.class); + ViewInstanceDAO viewInstanceDAO = createNiceMock(ViewInstanceDAO.class); + + ViewRegistry.init(viewDAO, viewInstanceDAO); + + ViewRegistry registry = ViewRegistry.getInstance(); + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + + ViewConfig config = ViewConfigTest.getConfig(xml_invalid_instance); + ViewEntity viewEntity = getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + + replay(viewDAO, viewInstanceDAO); + + registry.addDefinition(viewEntity); + try { + registry.installViewInstance(viewInstanceEntity); + Assert.fail("expected an IllegalStateException"); + } catch (IllegalStateException e) { + // expected + } + verify(viewDAO, viewInstanceDAO); + } + + @Test + public void testInstallViewInstance_unknownView() throws Exception { + + ViewDAO viewDAO = createNiceMock(ViewDAO.class); + ViewInstanceDAO viewInstanceDAO = createNiceMock(ViewInstanceDAO.class); + + ViewRegistry.init(viewDAO, viewInstanceDAO); + + ViewRegistry registry = ViewRegistry.getInstance(); + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + + ViewConfig config = ViewConfigTest.getConfig(xml_valid_instance); + ViewEntity viewEntity = getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + viewInstanceEntity.setViewName("BOGUS_VIEW"); + + replay(viewDAO, viewInstanceDAO); + + registry.addDefinition(viewEntity); + try { + registry.installViewInstance(viewInstanceEntity); + Assert.fail("expected an IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // expected + } + verify(viewDAO, viewInstanceDAO); + } + + @Test + public void testUpdateViewInstance() throws Exception { + + ViewDAO viewDAO = createNiceMock(ViewDAO.class); + ViewInstanceDAO viewInstanceDAO = createNiceMock(ViewInstanceDAO.class); + + ViewRegistry.init(viewDAO, viewInstanceDAO); + + ViewRegistry registry = ViewRegistry.getInstance(); + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + + ViewConfig config = ViewConfigTest.getConfig(xml_valid_instance); + ViewEntity viewEntity = getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + + viewInstanceDAO.create(viewInstanceEntity); + expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(viewInstanceEntity); + + replay(viewDAO, viewInstanceDAO); + + registry.addDefinition(viewEntity); + registry.installViewInstance(viewInstanceEntity); + + registry.updateViewInstance(updateInstance); + + Collection<ViewInstanceEntity> viewInstanceDefinitions = registry.getInstanceDefinitions(viewEntity); + + Assert.assertEquals(1, viewInstanceDefinitions.size()); + + Assert.assertEquals(viewInstanceEntity, viewInstanceDefinitions.iterator().next()); + + verify(viewDAO, viewInstanceDAO); + } + + @Test + public void testUpdateViewInstance_invalid() throws Exception { + + ViewDAO viewDAO = createNiceMock(ViewDAO.class); + ViewInstanceDAO viewInstanceDAO = createNiceMock(ViewInstanceDAO.class); + + ViewRegistry.init(viewDAO, viewInstanceDAO); + + ViewRegistry registry = ViewRegistry.getInstance(); + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + + ViewConfig config = ViewConfigTest.getConfig(xml_valid_instance); + ViewConfig invalidConfig = ViewConfigTest.getConfig(xml_invalid_instance); + ViewEntity viewEntity = getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, invalidConfig.getInstances().get(0)); + + viewInstanceDAO.create(viewInstanceEntity); + + replay(viewDAO, viewInstanceDAO); + + registry.addDefinition(viewEntity); + registry.installViewInstance(viewInstanceEntity); + + try { + registry.updateViewInstance(updateInstance); + Assert.fail("expected an IllegalStateException"); + } catch (IllegalStateException e) { + // expected + } + verify(viewDAO, viewInstanceDAO); + } + + @Test public void testRemoveInstanceData() throws Exception { ViewDAO viewDAO = createNiceMock(ViewDAO.class); @@ -501,6 +726,14 @@ public class ViewRegistryTest { public static ViewInstanceEntity getViewInstanceEntity(ViewEntity viewDefinition, InstanceConfig instanceConfig) throws Exception { ViewRegistry registry = ViewRegistry.getInstance(); - return registry.createViewInstanceDefinition(viewDefinition, instanceConfig); + ViewInstanceEntity viewInstanceDefinition = + new ViewInstanceEntity(viewDefinition, instanceConfig); + + for (PropertyConfig propertyConfig : instanceConfig.getProperties()) { + viewInstanceDefinition.putProperty(propertyConfig.getKey(), propertyConfig.getValue()); + } + + registry.bindViewInstance(viewDefinition, viewInstanceDefinition); + return viewInstanceDefinition; } }
