AMBARI-5871 - Views: Can't install new instance after deploying view instance

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

@@ -275,6 +275,8 @@ public class ViewRegistry {
    *
    * @param configuration  Ambari configuration
    *
+   * @return the set of view instance definitions read from the archives
+   *
    * @throws SystemException if the view archives can not be successfully read
    */
   public Set<ViewInstanceEntity> readViewArchives(Configuration configuration)
@@ -283,7 +285,7 @@ public class ViewRegistry {
     try {
       File viewDir = configuration.getViewsDir();
 
-      Set<ViewInstanceEntity> instanceDefinitions = new HashSet<ViewInstanceEntity>();
+      Set<ViewInstanceEntity> allInstanceDefinitions = new HashSet<ViewInstanceEntity>();
 
       String extractedArchivesPath = viewDir.getAbsolutePath() +
           File.separator + EXTRACTED_ARCHIVES_DIR;
@@ -303,28 +305,34 @@ public class ViewRegistry {
                 // extract the archive and get the class loader
                 ClassLoader cl = extractViewArchive(archiveFile, helper.getFile(archivePath));
 
-                ViewEntity viewDefinition = installView(viewConfig, configuration, cl, archivePath);
+                ViewEntity viewDefinition = createViewDefinition(viewConfig, configuration, cl, archivePath);
+
+                Set<ViewInstanceEntity> instanceDefinitions = new HashSet<ViewInstanceEntity>();
 
                 for (InstanceConfig instanceConfig : viewConfig.getInstances()) {
-                  ViewInstanceEntity viewInstanceDefinition =
-                      new ViewInstanceEntity(viewDefinition, instanceConfig);
+                  instanceDefinitions.add(createViewInstanceDefinition(viewDefinition, instanceConfig));
+                }
+                // ensure that the view entity matches the db
+                instanceDefinitions.addAll(persistView(viewDefinition));
 
-                  for (PropertyConfig propertyConfig : instanceConfig.getProperties()) {
-                    viewInstanceDefinition.putProperty(propertyConfig.getKey(), propertyConfig.getValue());
-                  }
+                // update the registry with the view
+                addDefinition(viewDefinition);
 
-                  installViewInstance(viewDefinition, viewInstanceDefinition);
-                  instanceDefinitions.add(viewInstanceDefinition);
+                // update the registry with the view instances
+                for (ViewInstanceEntity instanceEntity : instanceDefinitions) {
+                  addInstanceDefinition(viewDefinition, instanceEntity);
                 }
+
+                allInstanceDefinitions.addAll(instanceDefinitions);
               } catch (Exception e) {
                 LOG.error("Caught exception loading view from " + archiveFile.getAbsolutePath(), e);
               }
             }
           }
-          instanceDefinitions.addAll(persistViews());
+          removeUndeployedViews();
         }
       }
-      return instanceDefinitions;
+      return allInstanceDefinitions;
     } catch (Exception e) {
       throw new SystemException("Caught exception reading view archives.", e);
     }
@@ -350,11 +358,20 @@ public class ViewRegistry {
         }
         instanceDAO.create(instanceEntity);
         try {
-          installViewInstance(viewEntity, instanceEntity);
+          // 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);
         }
       }
+    } else {
+      String message = "Attempt to install an instance for an unknown view " +
+          instanceEntity.getViewName() + ".";
+
+      LOG.error(message);
+      throw new IllegalArgumentException(message);
     }
   }
 
@@ -478,9 +495,9 @@ public class ViewRegistry {
     return viewDefinitions.get(viewName);
   }
 
-  // install a new view definition
-  private ViewEntity installView(ViewConfig viewConfig, Configuration ambariConfig,
-                                            ClassLoader cl, String archivePath)
+  // create a new view definition
+  private ViewEntity createViewDefinition(ViewConfig viewConfig, Configuration ambariConfig,
+                                          ClassLoader cl, String archivePath)
       throws ClassNotFoundException, IntrospectionException {
 
     ViewEntity viewDefinition = new ViewEntity(viewConfig, ambariConfig, cl, archivePath);
@@ -545,12 +562,25 @@ public class ViewRegistry {
       }
       viewDefinition.setResources(resources);
     }
-    addDefinition(viewDefinition);
     return viewDefinition;
   }
 
-  // install a view instance definition
-  private void installViewInstance(ViewEntity viewDefinition,
+  // create a new view instance definition
+  private ViewInstanceEntity createViewInstanceDefinition(ViewEntity viewDefinition, InstanceConfig instanceConfig)
+      throws ClassNotFoundException {
+    ViewInstanceEntity viewInstanceDefinition =
+        new ViewInstanceEntity(viewDefinition, instanceConfig);
+
+    for (PropertyConfig propertyConfig : instanceConfig.getProperties()) {
+      viewInstanceDefinition.putProperty(propertyConfig.getKey(), propertyConfig.getValue());
+    }
+
+    bindViewInstance(viewDefinition, viewInstanceDefinition);
+    return viewInstanceDefinition;
+  }
+
+  // bind a view instance definition to the given view definition
+  private void bindViewInstance(ViewEntity viewDefinition,
                                    ViewInstanceEntity viewInstanceDefinition)
       throws ClassNotFoundException {
     viewInstanceDefinition.setViewEntity(viewDefinition);
@@ -586,7 +616,6 @@ public class ViewRegistry {
     setPersistenceEntities(viewInstanceDefinition);
 
     viewDefinition.addInstanceDefinition(viewInstanceDefinition);
-    addInstanceDefinition(viewDefinition, viewInstanceDefinition);
   }
 
   // Set the entities defined in the view persistence element for the given view instance
@@ -646,46 +675,50 @@ public class ViewRegistry {
     return viewInstanceInjector.getInstance(clazz);
   }
 
-  // make sure that the views in the ambari db match the registry
-  private Set<ViewInstanceEntity> persistViews() throws ClassNotFoundException {
-
-    Set<ViewInstanceEntity> instanceDefinitions = new HashSet<ViewInstanceEntity>();
-    Set<String>             persistedViews      = new HashSet<String>();
-
+  // remove undeployed views from the ambari db
+  private void removeUndeployedViews() {
     for (ViewEntity viewEntity : viewDAO.findAll()) {
       String name = viewEntity.getName();
       if (!ViewRegistry.getInstance().viewDefinitions.containsKey(name)) {
-        viewDAO.remove(viewEntity);
-      } else {
-        persistedViews.add(name);
-
-        ViewEntity viewDefinition = ViewRegistry.getInstance().viewDefinitions.get(name);
-
-        for (ViewInstanceEntity viewInstanceEntity : viewEntity.getInstances()){
-          ViewInstanceEntity instanceEntity = viewDefinition.getInstanceDefinition(viewInstanceEntity.getName());
-          if (instanceEntity == null) {
-            viewInstanceEntity.setViewEntity(viewDefinition);
-            installViewInstance(viewDefinition, viewInstanceEntity);
-            instanceDefinitions.add(viewInstanceEntity);
-          } else {
-            // apply overrides to the in-memory view instance entities
-            instanceEntity.setData(viewInstanceEntity.getData());
-            instanceEntity.setProperties(viewInstanceEntity.getProperties());
-            instanceEntity.setEntities(viewInstanceEntity.getEntities());
-          }
+        try {
+          viewDAO.remove(viewEntity);
+        } catch (Exception e) {
+          LOG.error("Caught exception undeploying view " + viewEntity.getName(), e);
         }
       }
     }
+  }
 
-    // persist new views
-    for (ViewEntity definition : viewDefinitions.values() ) {
-      String viewName = definition.getName();
+  // persist the given view
+  private Set<ViewInstanceEntity> persistView(ViewEntity viewDefinition)
+      throws ClassNotFoundException {
 
-      if (!persistedViews.contains(viewName)) {
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Creating View " + viewName + ".");
+    Set<ViewInstanceEntity> instanceDefinitions = new HashSet<ViewInstanceEntity>();
+
+    String viewName = viewDefinition.getName();
+
+    ViewEntity viewEntity = viewDAO.findByName(viewName);
+
+    if (viewEntity == null) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Creating View " + viewName + ".");
+      }
+      viewDAO.create(viewDefinition);
+    } 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.setData(viewInstanceEntity.getData());
+          instanceDefinition.setProperties(viewInstanceEntity.getProperties());
+          instanceDefinition.setEntities(viewInstanceEntity.getEntities());
         }
-        viewDAO.create(definition);
       }
     }
     return instanceDefinitions;

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

@@ -61,6 +61,7 @@ import static org.easymock.EasyMock.capture;
 import static org.easymock.EasyMock.createMock;
 import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
 
@@ -156,9 +157,11 @@ public class ViewRegistryTest {
 
     Capture<ViewEntity> captureViewEntity = new Capture<ViewEntity>();
 
-    expect(vDAO.findAll()).andReturn(Collections.<ViewEntity>emptyList());
+    expect(vDAO.findByName("MY_VIEW{1.0.0}")).andReturn(null);
     vDAO.create(capture(captureViewEntity));
 
+    expect(vDAO.findAll()).andReturn(Collections.<ViewEntity>emptyList());
+
     // replay mocks
     replay(configuration, viewDir, extractedArchiveDir, viewArchive, archiveDir, entryFile, classesDir,
         libDir, fileEntry, viewJarFile, enumeration, jarEntry, is, fos, vDAO);
@@ -176,6 +179,115 @@ public class ViewRegistryTest {
         libDir, fileEntry, viewJarFile, enumeration, jarEntry, is, fos, vDAO);
   }
 
+  @Test
+  public void testReadViewArchives_exception() throws Exception {
+    Configuration configuration = createNiceMock(Configuration.class);
+    File viewDir = createNiceMock(File.class);
+    File extractedArchiveDir = createNiceMock(File.class);
+    File viewArchive = createNiceMock(File.class);
+    File archiveDir = createNiceMock(File.class);
+    File entryFile  = createNiceMock(File.class);
+    File classesDir = createNiceMock(File.class);
+    File libDir = createNiceMock(File.class);
+    File fileEntry = createNiceMock(File.class);
+
+    JarFile viewJarFile = createNiceMock(JarFile.class);
+    Enumeration<JarEntry> enumeration = createMock(Enumeration.class);
+    JarEntry jarEntry = createNiceMock(JarEntry.class);
+    InputStream is = createMock(InputStream.class);
+    FileOutputStream fos = createMock(FileOutputStream.class);
+
+    ViewDAO vDAO = createMock(ViewDAO.class);
+
+    ViewRegistry.setViewDAO(vDAO);
+
+    ViewEntity viewDefinition = ViewEntityTest.getViewEntity();
+
+    Map<File, ViewConfig> viewConfigs =
+        Collections.singletonMap(viewArchive, viewDefinition.getConfiguration());
+
+    Map<String, File> files = new HashMap<String, File>();
+
+    files.put("/var/lib/ambari-server/resources/views/work", extractedArchiveDir);
+    files.put("/var/lib/ambari-server/resources/views/work/MY_VIEW{1.0.0}", archiveDir);
+    files.put("/var/lib/ambari-server/resources/views/work/MY_VIEW{1.0.0}/view.xml", entryFile);
+    files.put("/var/lib/ambari-server/resources/views/work/MY_VIEW{1.0.0}/WEB-INF/classes", classesDir);
+    files.put("/var/lib/ambari-server/resources/views/work/MY_VIEW{1.0.0}/WEB-INF/lib", libDir);
+
+    Map<File, FileOutputStream> outputStreams = new HashMap<File, FileOutputStream>();
+    outputStreams.put(entryFile, fos);
+
+    Map<File, JarFile> jarFiles = new HashMap<File, JarFile>();
+    jarFiles.put(viewArchive, viewJarFile);
+
+    // set expectations
+    expect(configuration.getViewsDir()).andReturn(viewDir);
+    expect(viewDir.getAbsolutePath()).andReturn("/var/lib/ambari-server/resources/views");
+
+    expect(viewDir.listFiles()).andReturn(new File[]{viewArchive});
+
+    expect(viewArchive.isDirectory()).andReturn(false);
+
+    expect(archiveDir.exists()).andReturn(false);
+    expect(archiveDir.getAbsolutePath()).andReturn(
+        "/var/lib/ambari-server/resources/views/work/MY_VIEW{1.0.0}").anyTimes();
+    expect(archiveDir.mkdir()).andReturn(true);
+    expect(archiveDir.toURI()).andReturn(new URI("file:./"));
+
+    expect(viewJarFile.entries()).andReturn(enumeration);
+    expect(viewJarFile.getInputStream(jarEntry)).andReturn(is);
+
+    expect(enumeration.hasMoreElements()).andReturn(true);
+    expect(enumeration.hasMoreElements()).andReturn(false);
+    expect(enumeration.nextElement()).andReturn(jarEntry);
+
+    expect(jarEntry.getName()).andReturn("view.xml");
+    expect(jarEntry.isDirectory()).andReturn(false);
+
+    expect(is.available()).andReturn(1);
+    expect(is.available()).andReturn(0);
+
+    expect(is.read()).andReturn(10);
+    fos.write(10);
+
+    fos.close();
+    is.close();
+
+    expect(extractedArchiveDir.exists()).andReturn(false);
+    expect(extractedArchiveDir.mkdir()).andReturn(true);
+
+    expect(classesDir.exists()).andReturn(true);
+    expect(classesDir.toURI()).andReturn(new URI("file:./"));
+
+    expect(libDir.exists()).andReturn(true);
+
+    expect(libDir.listFiles()).andReturn(new File[]{fileEntry});
+    expect(fileEntry.toURI()).andReturn(new URI("file:./"));
+
+    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.findAll()).andReturn(Collections.<ViewEntity>emptyList());
+
+    // replay mocks
+    replay(configuration, viewDir, extractedArchiveDir, viewArchive, archiveDir, entryFile, classesDir,
+        libDir, fileEntry, viewJarFile, enumeration, jarEntry, is, fos, vDAO);
+
+    ViewRegistry registry = ViewRegistry.getInstance();
+    registry.setHelper(new TestViewRegistryHelper(viewConfigs, files, outputStreams, jarFiles));
+
+    Set<ViewInstanceEntity> instanceEntities = registry.readViewArchives(configuration);
+
+    Assert.assertEquals(0, instanceEntities.size());
+
+    // verify mocks
+    verify(configuration, viewDir, extractedArchiveDir, viewArchive, archiveDir, entryFile, classesDir,
+        libDir, fileEntry, viewJarFile, enumeration, jarEntry, is, fos, vDAO);
+  }
+
   @Test
   public void testAddGetDefinitions() throws Exception {
     ViewEntity viewDefinition = ViewEntityTest.getViewEntity();