AMBARI-5987 - VIews: On View Instance create, should validate that req props are provided

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

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);

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();

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);
@@ -401,6 +452,180 @@ public class ViewRegistryTest {
     Assert.assertEquals(viewInstanceEntity, viewInstanceDefinitions.iterator().next());
   }
 
+  @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 {
 
@@ -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;
   }
 }