refresh group lists in project message handling

vladak · vladak · commit 97bbd7e22e5f · 2017-09-14T20:49:06.000+02:00
fixes #1769
diff --git a/src/org/opensolaris/opengrok/configuration/RuntimeEnvironment.java b/src/org/opensolaris/opengrok/configuration/RuntimeEnvironment.java
@@ -1285,8 +1285,10 @@ private void generateProjectRepositoriesMap() throws IOException {
 
     /**
      * Classifies projects and puts them in their groups.
+     * @param groups groups to update
+     * @param projects projects to classify
      */
-    private void populateGroups(Set<Group> groups, Set<Project> projects) {
+    public void populateGroups(Set<Group> groups, Set<Project> projects) {
         if (projects == null || groups == null) {
             return;
         }
@@ -1326,7 +1328,7 @@ public void setConfiguration(Configuration configuration, List<String> subFileLi
         } catch (IOException ex) {
             LOGGER.log(Level.SEVERE, "Cannot generate project - repository map", ex);
         }
-        populateGroups(getGroups(), new TreeSet<Project>(getProjects().values()));
+        populateGroups(getGroups(), new TreeSet<>(getProjects().values()));
         if (subFileList != null) {
             HistoryGuru.getInstance().invalidateRepositories(
                 configuration.getRepositories(), subFileList);
diff --git a/src/org/opensolaris/opengrok/configuration/messages/ProjectMessage.java b/src/org/opensolaris/opengrok/configuration/messages/ProjectMessage.java
@@ -28,10 +28,13 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collector;
 import java.util.stream.Collectors;
+import org.opensolaris.opengrok.configuration.Group;
 import org.opensolaris.opengrok.configuration.Project;
 import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
 import org.opensolaris.opengrok.history.HistoryGuru;
@@ -41,6 +44,7 @@
 import org.opensolaris.opengrok.index.IndexDatabase;
 import org.opensolaris.opengrok.logger.LoggerFactory;
 import org.opensolaris.opengrok.util.IOUtils;
+import org.opensolaris.opengrok.web.ProjectHelper;
 
 
 /**
@@ -123,11 +127,17 @@ protected byte[] applyMessage(RuntimeEnvironment env) throws Exception {
                         // Note that the project is inactive in the UI until it is indexed.
                         // See {@code isIndexed()}
                         env.getProjects().put(projectName, project);
+
+                        Set<Project> projectSet = new TreeSet<>(); 
+                        projectSet.add(project);
+                        env.populateGroups(env.getGroups(), projectSet);
                     } else {
                         Project project = env.getProjects().get(projectName);
                         Map<Project, List<RepositoryInfo>> map = env.getProjectRepositoriesMap();
 
-                        // Refresh the list of repositories.
+                        // Refresh the list of repositories of this project.
+                        // This is the goal of this action: if an existing project
+                        // is re-added, this means its list of repositories has changed.
                         List<RepositoryInfo> repos = getRepositoriesInDir(env, projDir);
                         List<RepositoryInfo> allrepos = env.getRepositories();
                         synchronized (allrepos) {
@@ -156,7 +166,13 @@ protected byte[] applyMessage(RuntimeEnvironment env) throws Exception {
                         throw new Exception("cannot get project \"" + projectName + "\"");
                     }
 
-                    LOGGER.log(Level.INFO, "deleting data for project " + projectName);
+                    LOGGER.log(Level.INFO, "deleting configuration for project " + projectName);
+
+                    // Remove the project from its groups.
+                    for (Group group : proj.getGroups()) {
+                        group.getRepositories().remove(proj);
+                        group.getProjects().remove(proj);
+                    }
 
                     // Now remove the repositories associated with this project.
                     List<RepositoryInfo> repos = env.getProjectRepositoriesMap().get(proj);
@@ -169,6 +185,7 @@ protected byte[] applyMessage(RuntimeEnvironment env) throws Exception {
                     env.refreshSearcherManagerMap();
 
                     // Lastly, remove data associated with the project.
+                    LOGGER.log(Level.INFO, "deleting data for project " + projectName);
                     for (String dirName: new String[]{
                         IndexDatabase.INDEX_DIR, IndexDatabase.XREF_DIR}) {
 
diff --git a/src/org/opensolaris/opengrok/web/ProjectHelper.java b/src/org/opensolaris/opengrok/web/ProjectHelper.java
@@ -128,10 +128,10 @@ public List<RepositoryInfo> getRepositoryInfo(Project p) {
     private void populateGroups() {
         groups.addAll(cfg.getEnv().getGroups());
         for (Project project : cfg.getEnv().getProjectList()) {
-            // filterProjects only groups which match project's description
+            // filterProjects() only adds groups which match project's name.
             Set<Group> copy = Group.matching(project, groups);
 
-            // if no group matches the project, add it to not-grouped projects
+            // If no group matches the project, add it to not-grouped projects.
             if (copy.isEmpty()) {
                 if (cfg.getEnv().getProjectRepositoriesMap().get(project) == null) {
                     ungroupedProjects.add(project);
diff --git a/test/org/opensolaris/opengrok/configuration/messages/ProjectMessageTest.java b/test/org/opensolaris/opengrok/configuration/messages/ProjectMessageTest.java
@@ -26,14 +26,17 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
 import org.junit.After;
 import org.junit.Assert;
 import static org.junit.Assert.assertTrue;
 import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
+import org.opensolaris.opengrok.configuration.Group;
 import org.opensolaris.opengrok.configuration.Project;
 import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
 import org.opensolaris.opengrok.history.GitRepository;
@@ -131,12 +134,24 @@ public void testValidate() {
 
     @Test
     public void testAdd() throws Exception {
-        // Add one project.
+        Assert.assertTrue(env.getRepositories().isEmpty());
+        Assert.assertTrue(env.getProjects().isEmpty());
+
+        // Add a group matching the project to be added.
+        String groupName = "mercurialgroup";
+        Group group = new Group(groupName, "mercurial.*");
+        env.getGroups().add(group);
+        Assert.assertTrue(env.hasGroups());
+        Assert.assertEquals(1, env.getGroups().stream().
+                filter(g -> g.getName().equals(groupName)).
+                collect(Collectors.toSet()).size());
+        Assert.assertEquals(0, group.getRepositories().size());
+        Assert.assertEquals(0, group.getProjects().size());
+
+        // Prepare project addition.
         Message m = new ProjectMessage();
         m.setText("add");
         m.addTag("mercurial");
-        Assert.assertTrue(env.getRepositories().isEmpty());
-        Assert.assertTrue(env.getProjects().isEmpty());
 
         // Add a sub-repository.
         File mercurialRoot = new File(repository.getSourceRoot() + File.separator + "mercurial");
@@ -146,16 +161,24 @@ public void testAdd() throws Exception {
             "clone", mercurialRoot.getAbsolutePath(),
             mercurialRoot.getAbsolutePath() + File.separator + "usr" + File.separator + "closed");
 
+        // Add the project.
         env.setScanningDepth(3);
         m.apply(env);
+
+        // Check that the project was added properly.
         Assert.assertTrue(env.getProjects().containsKey("mercurial"));
         Assert.assertEquals(1, env.getProjects().size());
         Assert.assertEquals(2, env.getRepositories().size());
+        Assert.assertEquals(1, group.getRepositories().size());
+        Assert.assertEquals(0, group.getProjects().size());
+        Assert.assertEquals(1, group.getRepositories().stream().
+                filter(p -> p.getName().equals("mercurial")).
+                collect(Collectors.toSet()).size());
 
         // Add more projects and check that they have been added incrementally.
         // At the same time, it checks that multiple projects can be added
         // with single message.
-        m.setTags(new TreeSet<String>());
+        m.setTags(new TreeSet<>());
         m.addTag("git");
         m.addTag("svn");
         m.apply(env);
@@ -195,13 +218,31 @@ public void testRepositoryRefresh() throws Exception {
         Assert.assertEquals(1, env.getProjectRepositoriesMap().get(Project.getProject(mercurialRoot)).size());
     }
 
+    /**
+     * This test needs to perform indexing so that it can be verified that
+     * the delete message handling performs removal of the index data.
+     * @throws Exception 
+     */
     @Test
     public void testDelete() throws Exception {
+        String projectsToDelete[] = { "git", "svn" };
+
         env.setCtags(System.getProperty(ctagsProperty, "ctags"));
 
         assertTrue("No point in running indexer tests without valid ctags",
                 RuntimeEnvironment.getInstance().validateExuberantCtags());
 
+        // Add a group matching the project to be added.
+        String groupName = "gitgroup";
+        Group group = new Group(groupName, "git.*");
+        env.getGroups().add(group);
+        Assert.assertTrue(env.hasGroups());
+        Assert.assertEquals(1, env.getGroups().stream().
+                filter(g -> g.getName().equals(groupName)).
+                collect(Collectors.toSet()).size());
+        Assert.assertEquals(0, group.getRepositories().size());
+        Assert.assertEquals(0, group.getProjects().size());
+
         // Firstly add some projects.
         Message m = new ProjectMessage();
         m.setText("add");
@@ -216,6 +257,13 @@ public void testDelete() throws Exception {
         Assert.assertEquals(3, env.getRepositories().size());
         Assert.assertEquals(3, env.getProjectRepositoriesMap().size());
 
+        // Check the group was populated properly.
+        Assert.assertEquals(1, group.getRepositories().size());
+        Assert.assertEquals(0, group.getProjects().size());
+        Assert.assertEquals(1, group.getRepositories().stream().
+                filter(p -> p.getName().equals("git")).
+                collect(Collectors.toSet()).size());
+
         // Run the indexer (ala 'indexpart') so that data directory is populated.
         ArrayList<String> subFiles = new ArrayList<>();
         subFiles.add("/git");
@@ -247,23 +295,28 @@ public void testDelete() throws Exception {
         // Then remove multiple projects.
         m.setText("delete");
         m.setTags(new TreeSet<String>());
-        m.addTag("git");
-        m.addTag("svn");
+        for (String p : projectsToDelete) {
+            m.addTag(p);
+        }
         m.apply(env);
         Assert.assertEquals(1, env.getProjects().size());
         Assert.assertEquals(1, env.getRepositories().size());
         Assert.assertEquals(1, env.getProjectRepositoriesMap().size());
 
         // Test data removal.
         File dataRoot = env.getDataRootFile();
-        for (String projectName : new String[]{"git", "svn"}) {
+        for (String projectName : projectsToDelete) {
             for (String dirName : new String[]{"historycache",
                 IndexDatabase.XREF_DIR, IndexDatabase.INDEX_DIR}) {
                     File dir = new File(env.getDataRootFile(),
                         dirName + File.separator + projectName);
                     Assert.assertFalse(dir.exists());
             }
         }
+
+        // Check the group no longer contains the removed project.
+        Assert.assertEquals(0, group.getRepositories().size());
+        Assert.assertEquals(0, group.getProjects().size());
     }
 
     @Test
