AMBARI-6019 - Views : JPA Exception installing view

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

@@ -345,7 +345,7 @@ public class ViewRegistry {
                   }
                 }
                 // ensure that the view entity matches the db
-                instanceDefinitions.addAll(persistView(viewDefinition));
+                syncView(viewDefinition, instanceDefinitions);
 
                 // update the registry with the view
                 addDefinition(viewDefinition);
@@ -394,7 +394,7 @@ public class ViewRegistry {
               version + "/" + instanceName);
         }
         instanceEntity.validate(viewEntity);
-        instanceDAO.create(instanceEntity);
+        instanceDAO.merge(instanceEntity);
         try {
           // bind the view instance to a view
           bindViewInstance(viewEntity, instanceEntity);
@@ -790,42 +790,60 @@ public class ViewRegistry {
     }
   }
 
-  // persist the given view
-  private Set<ViewInstanceEntity> persistView(ViewEntity viewDefinition)
-      throws ClassNotFoundException {
-
-    Set<ViewInstanceEntity> instanceDefinitions = new HashSet<ViewInstanceEntity>();
+  // sync the given view with the db
+  private void syncView(ViewEntity view,
+                        Set<ViewInstanceEntity> instanceDefinitions)
+      throws Exception {
 
-    String viewName = viewDefinition.getName();
+    String viewName = view.getName();
 
-    ViewEntity viewEntity = viewDAO.findByName(viewName);
+    ViewEntity persistedView = viewDAO.findByName(viewName);
 
-    if (viewEntity == null) {
+    // if the view is not yet persisted ...
+    if (persistedView == null) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Creating View " + viewName + ".");
       }
-      viewDAO.create(viewDefinition);
+      // ... merge it
+      viewDAO.merge(view);
     } else {
-      for (ViewInstanceEntity viewInstanceEntity : viewEntity.getInstances()){
-        ViewInstanceEntity instanceDefinition =
-            viewDefinition.getInstanceDefinition(viewInstanceEntity.getName());
-
-        if (instanceDefinition == null) {
-          viewInstanceEntity.setViewEntity(viewDefinition);
-          bindViewInstance(viewDefinition, viewInstanceEntity);
-          instanceDefinitions.add(viewInstanceEntity);
-        } else {
-          // apply overrides to the in-memory view instance entities
-          instanceDefinition.setLabel(viewInstanceEntity.getLabel());
-          instanceDefinition.setDescription(viewInstanceEntity.getDescription());
-          instanceDefinition.setVisible(viewInstanceEntity.isVisible());
-          instanceDefinition.setData(viewInstanceEntity.getData());
-          instanceDefinition.setProperties(viewInstanceEntity.getProperties());
-          instanceDefinition.setEntities(viewInstanceEntity.getEntities());
+      Map<String, ViewInstanceEntity> instanceEntityMap = new HashMap<String, ViewInstanceEntity>();
+      for( ViewInstanceEntity instance : view.getInstances()) {
+        instanceEntityMap.put(instance.getName(), instance);
+      }
+
+      // 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();
+
+        ViewInstanceEntity instance =
+            view.getInstanceDefinition(instanceName);
+
+        instanceEntityMap.remove(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());
+      }
+
+      // 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);
       }
     }
-    return instanceDefinitions;
   }
 
   // ensure that the extracted view archive directory exists

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

@@ -225,7 +225,7 @@ public class ViewRegistryTest {
     Capture<ViewEntity> captureViewEntity = new Capture<ViewEntity>();
 
     expect(vDAO.findByName("MY_VIEW{1.0.0}")).andReturn(null);
-    vDAO.create(capture(captureViewEntity));
+    expect(vDAO.merge(capture(captureViewEntity))).andReturn(null);
 
     expect(vDAO.findAll()).andReturn(Collections.<ViewEntity>emptyList());
 
@@ -334,8 +334,7 @@ public class ViewRegistryTest {
     Capture<ViewEntity> captureViewEntity = new Capture<ViewEntity>();
 
     expect(vDAO.findByName("MY_VIEW{1.0.0}")).andReturn(null);
-    vDAO.create(capture(captureViewEntity));
-    expectLastCall().andThrow(new IllegalArgumentException("Expected exception."));
+    expect(vDAO.merge(capture(captureViewEntity))).andThrow(new IllegalArgumentException("Expected exception."));
 
     expect(vDAO.findAll()).andReturn(Collections.<ViewEntity>emptyList());
 
@@ -471,7 +470,7 @@ public class ViewRegistryTest {
     ViewEntity viewEntity = getViewEntity(config, ambariConfig, getClass().getClassLoader(), "");
     ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0));
 
-    viewInstanceDAO.create(viewInstanceEntity);
+    expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(null);
 
     replay(viewDAO, viewInstanceDAO);
 
@@ -570,7 +569,7 @@ public class ViewRegistryTest {
     ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0));
     ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, config.getInstances().get(0));
 
-    viewInstanceDAO.create(viewInstanceEntity);
+    expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(null);
     expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(viewInstanceEntity);
 
     replay(viewDAO, viewInstanceDAO);
@@ -610,7 +609,7 @@ public class ViewRegistryTest {
     ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0));
     ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, invalidConfig.getInstances().get(0));
 
-    viewInstanceDAO.create(viewInstanceEntity);
+    expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(null);
 
     replay(viewDAO, viewInstanceDAO);