AMBARI-6230 - Views: updates after POST to create cause exceptions

ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java

@@ -281,6 +281,24 @@ public class ViewInstanceEntity implements ViewInstanceDefinition {
 
   // ----- ViewInstanceEntity ------------------------------------------------
 
+  /**
+   * Get the view instance id.
+   *
+   * @return the instance id
+   */
+  public Long getViewInstanceId() {
+    return viewInstanceId;
+  }
+
+  /**
+   * Set the given view instance id.
+   *
+   * @param viewInstanceId  the instance id
+   */
+  public void setViewInstanceId(Long viewInstanceId) {
+    this.viewInstanceId = viewInstanceId;
+  }
+
   /**
    * Set the view name.
    *

ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java

@@ -395,6 +395,16 @@ public class ViewRegistry {
         }
         instanceEntity.validate(viewEntity);
         instanceDAO.merge(instanceEntity);
+
+        ViewInstanceEntity persistedInstance = instanceDAO.findByName(ViewEntity.getViewName(viewName, version), instanceName);
+        if (persistedInstance == null) {
+          String message = "Instance  " + instanceEntity.getViewName() + " can not be found.";
+
+          LOG.error(message);
+          throw new IllegalStateException(message);
+        }
+        instanceEntity.setViewInstanceId(persistedInstance.getViewInstanceId());
+
         try {
           // bind the view instance to a view
           bindViewInstance(viewEntity, instanceEntity);
@@ -806,43 +816,53 @@ public class ViewRegistry {
       }
       // ... merge it
       viewDAO.merge(view);
-    } else {
-      Map<String, ViewInstanceEntity> instanceEntityMap = new HashMap<String, ViewInstanceEntity>();
-      for( ViewInstanceEntity instance : view.getInstances()) {
-        instanceEntityMap.put(instance.getName(), instance);
+
+      persistedView = viewDAO.findByName(viewName);
+      if (persistedView == null) {
+        String message = "View  " + persistedView.getViewName() + " can not be found.";
+
+        LOG.error(message);
+        throw new IllegalStateException(message);
       }
+    }
 
-      // make sure that each instance of the view in the db is reflected in the given view
-      for (ViewInstanceEntity persistedInstance : persistedView.getInstances()){
-        String instanceName = persistedInstance.getName();
+    Map<String, ViewInstanceEntity> instanceEntityMap = new HashMap<String, ViewInstanceEntity>();
+    for( ViewInstanceEntity instance : view.getInstances()) {
+      instanceEntityMap.put(instance.getName(), instance);
+    }
 
-        ViewInstanceEntity instance =
-            view.getInstanceDefinition(instanceName);
+    // make sure that each instance of the view in the db is reflected in the given view
+    for (ViewInstanceEntity persistedInstance : persistedView.getInstances()){
+      String instanceName = persistedInstance.getName();
 
-        instanceEntityMap.remove(instanceName);
+      ViewInstanceEntity instance =
+          view.getInstanceDefinition(instanceName);
 
-        // if the persisted instance is not in the registry ...
-        if (instance == null) {
-          // ... create and add it
-          instance = new ViewInstanceEntity(view, instanceName);
-          bindViewInstance(view, instance);
-          instanceDefinitions.add(instance);
-        }
-        // apply the persisted overrides to the in-memory instance
-        instance.setLabel(persistedInstance.getLabel());
-        instance.setDescription(persistedInstance.getDescription());
-        instance.setVisible(persistedInstance.isVisible());
-        instance.setData(persistedInstance.getData());
-        instance.setProperties(persistedInstance.getProperties());
-        instance.setEntities(persistedInstance.getEntities());
-      }
+      instanceEntityMap.remove(instanceName);
 
-      // these instances appear in the archive but have been deleted
-      // from the db... remove them from the registry
-      for (ViewInstanceEntity instance : instanceEntityMap.values()) {
-        view.removeInstanceDefinition(instance.getName());
-        instanceDefinitions.remove(instance);
+      // if the persisted instance is not in the registry ...
+      if (instance == null) {
+        // ... create and add it
+        instance = new ViewInstanceEntity(view, instanceName);
+        bindViewInstance(view, instance);
+        instanceDefinitions.add(instance);
       }
+      instance.setViewInstanceId(persistedInstance.getViewInstanceId());
+
+      // apply the persisted overrides to the in-memory instance
+      instance.setLabel(persistedInstance.getLabel());
+      instance.setDescription(persistedInstance.getDescription());
+      instance.setVisible(persistedInstance.isVisible());
+      instance.setData(persistedInstance.getData());
+      instance.setProperties(persistedInstance.getProperties());
+      instance.setEntities(persistedInstance.getEntities());
+    }
+
+    // these instances appear in the archive but have been deleted
+    // from the db... remove them from the registry
+    for (ViewInstanceEntity instance : instanceEntityMap.values()) {
+      view.removeInstanceDefinition(instance.getName());
+      instanceDefinitions.remove(instance);
     }
   }
 

ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java

@@ -29,8 +29,10 @@ import org.apache.ambari.view.ResourceProvider;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
 
 import static org.easymock.EasyMock.createNiceMock;
 
@@ -135,6 +137,16 @@ public class ViewInstanceEntityTest {
     Assert.assertEquals(instanceConfig, viewInstanceDefinition.getConfiguration());
   }
 
+  @Test
+  public void testGetId() throws Exception {
+    ViewInstanceEntity viewInstanceDefinition = getViewInstanceEntity();
+
+    Assert.assertNull(viewInstanceDefinition.getViewInstanceId());
+
+    viewInstanceDefinition.setViewInstanceId(99L);
+    Assert.assertEquals(99L, (long) viewInstanceDefinition.getViewInstanceId());
+  }
+
   @Test
   public void testGetName() throws Exception {
     ViewInstanceEntity viewInstanceDefinition = getViewInstanceEntity();
@@ -352,6 +364,17 @@ public class ViewInstanceEntityTest {
     return new ViewInstanceEntity(viewDefinition, instanceConfig);
   }
 
+  public static Set<ViewInstanceEntity> getViewInstanceEntities(ViewEntity viewDefinition) throws Exception {
+    Set<ViewInstanceEntity> entities = new HashSet<ViewInstanceEntity>();
+
+    InstanceConfig instanceConfig = InstanceConfigTest.getInstanceConfigs().get(0);
+    entities.add(new ViewInstanceEntity(viewDefinition, instanceConfig));
+    instanceConfig = InstanceConfigTest.getInstanceConfigs().get(1);
+    entities.add(new ViewInstanceEntity(viewDefinition, instanceConfig));
+
+    return entities;
+  }
+
   @Test
   public void testValidate() throws Exception {
 

ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java

@@ -161,6 +161,8 @@ public class ViewRegistryTest {
 
     ViewEntity viewDefinition = ViewEntityTest.getViewEntity();
 
+    viewDefinition.setInstances(ViewInstanceEntityTest.getViewInstanceEntities(viewDefinition));
+
     Map<File, ViewConfig> viewConfigs =
         Collections.singletonMap(viewArchive, viewDefinition.getConfiguration());
 
@@ -226,6 +228,7 @@ public class ViewRegistryTest {
 
     expect(vDAO.findByName("MY_VIEW{1.0.0}")).andReturn(null);
     expect(vDAO.merge(capture(captureViewEntity))).andReturn(null);
+    expect(vDAO.findByName("MY_VIEW{1.0.0}")).andReturn(viewDefinition);
 
     expect(vDAO.findAll()).andReturn(Collections.<ViewEntity>emptyList());
 
@@ -471,6 +474,7 @@ public class ViewRegistryTest {
     ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0));
 
     expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(null);
+    expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity);
 
     replay(viewDAO, viewInstanceDAO);
 
@@ -571,6 +575,7 @@ public class ViewRegistryTest {
 
     expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(null);
     expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(viewInstanceEntity);
+    expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity);
 
     replay(viewDAO, viewInstanceDAO);
 
@@ -610,6 +615,7 @@ public class ViewRegistryTest {
     ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, invalidConfig.getInstances().get(0));
 
     expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(null);
+    expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity);
 
     replay(viewDAO, viewInstanceDAO);