[#865] AuthorConfig: further improve the branch field (#878)

In the author-config.csv, branch is an optional field that will automatically uses the default branch in the repo field. However, this can be improved such that if the branch field is not filled, we can bind that AuthorConfig to all the RepoConfig that has the same repo's location irregardless of branch. This can help users to easily setup config files to analyse multiple branches of a repo. Let's bind the AuthorConfig to all the RepoConfig that has the same repo's location irregardless of branch if the branch field is not filled.
reposense · Dec 17, 2019 · d671041 · d671041
1 parent a2adb59
commit d671041
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 43 deletions.
diff --git a/docs/UserGuide.md b/docs/UserGuide.md
@@ -340,7 +340,7 @@ Optionally, you can use a `author-config.csv` (which should be in the same direc
 Column Name | Explanation
 ----------- | -----------
 [Optional] Repository's Location | Same as `repo-config.csv`. Default: all the repos in `repo-config.csv`
-[Optional] Branch | The branch to analyze for this author e.g., `master`. Default: the default branch of the repo
+[Optional] Branch | The branch to analyze for this author e.g., `master`. Default: the author will be bound to all the repos in `repo-config.csv` that has the same repo's location, irregardless of branch
 Author's GitHub ID | GitHub username of the target author e.g., `JohnDoe`
 [Optional] Author's Emails<sup>*</sup> | Associated Github emails of the author. This can be found in your [GitHub settings](https://github.com/settings/emails).
 [Optional] Author's Display Name | The name to display for the author. Default: author's GitHub username.

diff --git a/src/main/java/reposense/model/Author.java b/src/main/java/reposense/model/Author.java
@@ -135,12 +135,16 @@ public PathMatcher getIgnoreGlobMatcher() {
     }
 
     /**
-     * Validates and adds a list of ignore glob into the {@code Author} class instance variable,
+     * Validates and adds a list of ignore glob into the {@code Author} class instance variable without duplicates
      * and updates the ignore glob matcher.
      */
-    public void appendIgnoreGlobList(List<String> ignoreGlobList) {
+    public void importIgnoreGlobList(List<String> ignoreGlobList) {
         validateIgnoreGlobs(ignoreGlobList);
-        this.ignoreGlobList.addAll(ignoreGlobList);
+        ignoreGlobList.forEach(ignoreGlob -> {
+            if (!this.ignoreGlobList.contains(ignoreGlob)) {
+                this.ignoreGlobList.add(ignoreGlob);
+            }
+        });
         updateIgnoreGlobMatcher();
     }
 

diff --git a/src/main/java/reposense/model/AuthorConfiguration.java b/src/main/java/reposense/model/AuthorConfiguration.java
@@ -47,7 +47,7 @@ public void update(StandaloneConfig standaloneConfig, List<String> ignoreGlobLis
 
         for (StandaloneAuthor sa : standaloneConfig.getAuthors()) {
             Author author = new Author(sa);
-            author.appendIgnoreGlobList(ignoreGlobList);
+            author.importIgnoreGlobList(ignoreGlobList);
 
             newAuthorList.add(author);
             newAuthorDisplayNameMap.put(author, author.getDisplayName());
@@ -120,7 +120,7 @@ private void setAuthorDetails(Author author) {
      * Propagates {@code ignoreGlobList} to {@code author}.
      */
     public static void propagateIgnoreGlobList(Author author, List<String> ignoreGlobList) {
-        author.appendIgnoreGlobList(ignoreGlobList);
+        author.importIgnoreGlobList(ignoreGlobList);
     }
 
     /**

diff --git a/src/main/java/reposense/model/RepoConfiguration.java b/src/main/java/reposense/model/RepoConfiguration.java
@@ -90,30 +90,40 @@ public static void setDatesToRepoConfigs(List<RepoConfiguration> configs, Date s
     public static void merge(List<RepoConfiguration> repoConfigs, List<AuthorConfiguration> authorConfigs) {
         for (AuthorConfiguration authorConfig : authorConfigs) {
             if (authorConfig.getLocation().isEmpty()) {
+                for (RepoConfiguration repoConfig : repoConfigs) {
+                    repoConfig.addAuthors(authorConfig.getAuthorList());
+                }
                 continue;
             }
 
-            RepoConfiguration matchingRepoConfig = getMatchingRepoConfig(repoConfigs, authorConfig);
+            List<RepoConfiguration> locationMatchingRepoConfigs =
+                    getMatchingRepoConfigsByLocation(repoConfigs, authorConfig.getLocation());
 
-            if (matchingRepoConfig == null) {
-                String branchInfo = authorConfig.isDefaultBranch()
-                        ? ""
-                        : String.format(" (branch %s)", authorConfig.getBranch());
+            if (locationMatchingRepoConfigs.isEmpty()) {
                 logger.warning(String.format(
-                        "Repository %s%s is not found in repo-config.csv.",
-                        authorConfig.getLocation(), branchInfo));
+                        "Repository %s is not found in repo-config.csv.",
+                        authorConfig.getLocation()));
+                continue;
+            }
+            if (authorConfig.isDefaultBranch()) {
+                locationMatchingRepoConfigs.forEach(matchingRepoConfig -> {
+                    matchingRepoConfig.addAuthors(authorConfig.getAuthorList());
+                });
                 continue;
             }
 
-            matchingRepoConfig.setAuthorConfiguration(authorConfig);
-        }
+            RepoConfiguration branchMatchingRepoConfig = getMatchingRepoConfig(repoConfigs, authorConfig);
 
-        for (AuthorConfiguration authorConfig : authorConfigs) {
-            if (authorConfig.getLocation().isEmpty()) {
-                for (RepoConfiguration repoConfig : repoConfigs) {
-                    repoConfig.addAuthors(authorConfig.getAuthorList());
+            if (branchMatchingRepoConfig == null) {
+                if (!authorConfig.isDefaultBranch()) {
+                    logger.warning(String.format(
+                            "Repository %s (branch %s) is not found in repo-config.csv.",
+                            authorConfig.getLocation(), authorConfig.getBranch()));
                 }
+                continue;
             }
+
+            branchMatchingRepoConfig.addAuthors(authorConfig.getAuthorList());
         }
     }
 
@@ -127,7 +137,7 @@ public static void setGroupConfigsToRepos(List<RepoConfiguration> repoConfigs,
                 continue;
             }
 
-            List<RepoConfiguration> matchingRepoConfigs = getMatchingRepoConfigsByRepoLocation(repoConfigs,
+            List<RepoConfiguration> matchingRepoConfigs = getMatchingRepoConfigsByLocation(repoConfigs,
                     groupConfig.getLocation());
             if (matchingRepoConfigs.isEmpty()) {
                 logger.warning(String.format(
@@ -146,6 +156,9 @@ public static void setGroupConfigsToRepos(List<RepoConfiguration> repoConfigs,
      */
     private static RepoConfiguration getMatchingRepoConfig(
             List<RepoConfiguration> repoConfigs, AuthorConfiguration authorConfig) {
+        if (authorConfig.isDefaultBranch()) {
+            return null;
+        }
         for (RepoConfiguration repoConfig : repoConfigs) {
             if (repoConfig.getLocation().equals(authorConfig.getLocation())
                     && repoConfig.getBranch().equals(authorConfig.getBranch())) {
@@ -158,7 +171,7 @@ private static RepoConfiguration getMatchingRepoConfig(
     /**
      * Returns a list of {@link RepoConfiguration} where the {@link RepoLocation} matches {@code targetRepoLocation}.
      */
-    private static List<RepoConfiguration> getMatchingRepoConfigsByRepoLocation(
+    private static List<RepoConfiguration> getMatchingRepoConfigsByLocation(
             List<RepoConfiguration> configs, RepoLocation targetRepoLocation) {
         return configs.stream().filter(config -> config.getLocation().equals(targetRepoLocation))
                 .collect(Collectors.toList());

diff --git a/src/test/java/reposense/model/AuthorTest.java b/src/test/java/reposense/model/AuthorTest.java
@@ -79,7 +79,7 @@ public void appendIgnoreGlobList_validGlobRegex_success() {
         ignoreGlobList.addAll(Arrays.asList(moreIgnoreGlobs));
 
         author.setIgnoreGlobList(Arrays.asList(ignoreGlobs));
-        author.appendIgnoreGlobList(Arrays.asList(moreIgnoreGlobs));
+        author.importIgnoreGlobList(Arrays.asList(moreIgnoreGlobs));
 
         Assert.assertEquals(4, author.getIgnoreGlobList().size());
         Assert.assertTrue(author.getIgnoreGlobList().containsAll(ignoreGlobList));
@@ -92,6 +92,6 @@ public void appendIgnoreGlobList_appendOrOperator_throwIllegalArgumentException(
         Author author = new Author("Tester");
         String[] ignoreGlobs = new String[] {"**[!(.md)] | rm -rf /", "C:\\Program Files\\**"};
 
-        author.appendIgnoreGlobList(Arrays.asList(ignoreGlobs));
+        author.importIgnoreGlobList(Arrays.asList(ignoreGlobs));
     }
 }
diff --git a/src/test/java/reposense/parser/CsvParserTest.java b/src/test/java/reposense/parser/CsvParserTest.java
@@ -66,7 +66,8 @@ public class CsvParserTest {
             .getResource("CsvParserTest/groupconfig_invalidLocation_test.csv").getFile()).toPath();
 
     private static final String TEST_REPO_BETA_LOCATION = "https://github.com/reposense/testrepo-Beta.git";
-    private static final String TEST_REPO_BETA_BRANCH = "master";
+    private static final String TEST_REPO_BETA_MASTER_BRANCH = "master";
+    private static final String TEST_REPO_BETA_ADD_CONFIG_JSON_BRANCH = "add-config-json";
     private static final List<FileType> TEST_REPO_BETA_GROUPS = Arrays.asList(
             new FileType("Code", Arrays.asList("**/*.java", "**/*.py")),
             new FileType("Docs", Collections.singletonList("docs/**")));
@@ -125,6 +126,7 @@ public class CsvParserTest {
 
     private static final List<String> REPO_LEVEL_GLOB_LIST = Arrays.asList("collated**");
     private static final List<String> FIRST_AUTHOR_GLOB_LIST = Arrays.asList("**.java", "collated**");
+    private static final List<String> SECOND_AUTHOR_GLOB_LIST = Arrays.asList("**.doc", "collated**");
     private static final List<String> FIRST_AUTHOR_EMAIL_LIST =
             Arrays.asList("[email protected]", "[email protected]", "[email protected]");
 
@@ -138,7 +140,7 @@ public void repoConfig_noSpecialCharacter_success() throws IOException, InvalidL
         RepoConfiguration config = configs.get(0);
 
         Assert.assertEquals(new RepoLocation(TEST_REPO_BETA_LOCATION), config.getLocation());
-        Assert.assertEquals(TEST_REPO_BETA_BRANCH, config.getBranch());
+        Assert.assertEquals(TEST_REPO_BETA_MASTER_BRANCH, config.getBranch());
 
         Assert.assertEquals(TEST_REPO_BETA_CONFIG_FORMATS, config.getFileTypeManager().getFormats());
 
@@ -163,7 +165,7 @@ public void authorConfig_noSpecialCharacter_success() throws IOException, Invali
         AuthorConfiguration config = configs.get(0);
 
         Assert.assertEquals(new RepoLocation(TEST_REPO_BETA_LOCATION), config.getLocation());
-        Assert.assertEquals(TEST_REPO_BETA_BRANCH, config.getBranch());
+        Assert.assertEquals(TEST_REPO_BETA_MASTER_BRANCH, config.getBranch());
 
         Assert.assertEquals(AUTHOR_CONFIG_NO_SPECIAL_CHARACTER_AUTHORS, config.getAuthorList());
     }
@@ -198,7 +200,7 @@ public void authorConfig_specialCharacter_success() throws IOException, InvalidL
         AuthorConfiguration config = configs.get(0);
 
         Assert.assertEquals(new RepoLocation(TEST_REPO_BETA_LOCATION), config.getLocation());
-        Assert.assertEquals(TEST_REPO_BETA_BRANCH, config.getBranch());
+        Assert.assertEquals(TEST_REPO_BETA_MASTER_BRANCH, config.getBranch());
 
         Assert.assertEquals(AUTHOR_CONFIG_SPECIAL_CHARACTER_AUTHORS, config.getAuthorList());
     }
@@ -278,7 +280,7 @@ public void parse_multipleColumnsWithCommasAndDoubleQuotes_success() throws IOEx
         AuthorConfiguration config = configs.get(0);
 
         Assert.assertEquals(new RepoLocation(TEST_REPO_BETA_LOCATION), config.getLocation());
-        Assert.assertEquals(TEST_REPO_BETA_BRANCH, config.getBranch());
+        Assert.assertEquals(TEST_REPO_BETA_MASTER_BRANCH, config.getBranch());
         Assert.assertEquals(AUTHOR_DISPLAY_NAME_COMMAS_AND_DOUBLE_QUOTES_MAP, config.getAuthorDisplayNameMap());
 
         Assert.assertEquals(AUTHOR_ALIAS_COMMAS_AND_DOUBLE_QUOTES_MAP.size(), config.getAuthorList().size());
@@ -290,20 +292,27 @@ public void parse_multipleColumnsWithCommasAndDoubleQuotes_success() throws IOEx
     @Test
     public void merge_twoRepoConfigs_success() throws ParseException, IOException, HelpScreenException {
         FIRST_AUTHOR.setIgnoreGlobList(FIRST_AUTHOR_GLOB_LIST);
-        SECOND_AUTHOR.setIgnoreGlobList(REPO_LEVEL_GLOB_LIST);
+        SECOND_AUTHOR.setIgnoreGlobList(SECOND_AUTHOR_GLOB_LIST);
         SECOND_AUTHOR.setAuthorAliases(SECOND_AUTHOR_ALIASES);
 
         List<Author> expectedAuthors = new ArrayList<>();
         expectedAuthors.add(FIRST_AUTHOR);
         expectedAuthors.add(SECOND_AUTHOR);
 
-        RepoConfiguration expectedConfig = new RepoConfiguration(new RepoLocation(TEST_REPO_BETA_LOCATION),
-                TEST_REPO_BETA_BRANCH);
-        expectedConfig.setAuthorList(expectedAuthors);
-        expectedConfig.setAuthorDisplayName(FIRST_AUTHOR, "Nbr");
-        expectedConfig.setAuthorDisplayName(SECOND_AUTHOR, "Zac");
-        expectedConfig.addAuthorEmailsAndAliasesMapEntry(SECOND_AUTHOR,  Arrays.asList("Zachary Tang"));
-        expectedConfig.setIgnoreGlobList(REPO_LEVEL_GLOB_LIST);
+        RepoConfiguration firstRepo = new RepoConfiguration(new RepoLocation(TEST_REPO_BETA_LOCATION),
+                TEST_REPO_BETA_MASTER_BRANCH);
+        firstRepo.setAuthorList(expectedAuthors);
+        firstRepo.setAuthorDisplayName(FIRST_AUTHOR, "Nbr");
+        firstRepo.setAuthorDisplayName(SECOND_AUTHOR, "Zac");
+        firstRepo.addAuthorEmailsAndAliasesMapEntry(SECOND_AUTHOR,  Arrays.asList("Zachary Tang"));
+        firstRepo.setIgnoreGlobList(REPO_LEVEL_GLOB_LIST);
+
+        RepoConfiguration secondRepo = new RepoConfiguration(new RepoLocation(TEST_REPO_BETA_LOCATION),
+                TEST_REPO_BETA_ADD_CONFIG_JSON_BRANCH);
+        secondRepo.setAuthorList(Arrays.asList(SECOND_AUTHOR));
+        secondRepo.setAuthorDisplayName(SECOND_AUTHOR, "Zac");
+        secondRepo.addAuthorEmailsAndAliasesMapEntry(SECOND_AUTHOR,  Arrays.asList("Zachary Tang"));
+        secondRepo.setIgnoreGlobList(REPO_LEVEL_GLOB_LIST);
 
         String input = new InputBuilder().addConfig(TEST_CONFIG_FOLDER).build();
         CliArguments cliArguments = ArgsParser.parse(translateCommandline(input));
@@ -314,8 +323,9 @@ public void merge_twoRepoConfigs_success() throws ParseException, IOException, H
                 new AuthorConfigCsvParser(((ConfigCliArguments) cliArguments).getAuthorConfigFilePath()).parse();
         RepoConfiguration.merge(actualConfigs, authorConfigs);
 
-        Assert.assertEquals(1, actualConfigs.size());
-        TestUtil.compareRepoConfig(expectedConfig, actualConfigs.get(0));
+        Assert.assertEquals(2, actualConfigs.size());
+        TestUtil.compareRepoConfig(firstRepo, actualConfigs.get(0));
+        TestUtil.compareRepoConfig(secondRepo, actualConfigs.get(1));
     }
 
     @Test
@@ -325,14 +335,14 @@ public void merge_emptyLocation_success() throws ParseException, IOException, He
         SECOND_AUTHOR.setAuthorAliases(SECOND_AUTHOR_ALIASES);
 
         List<Author> expectedBetaAuthors = new ArrayList<>();
-        expectedBetaAuthors.add(SECOND_AUTHOR);
         expectedBetaAuthors.add(FIRST_AUTHOR);
+        expectedBetaAuthors.add(SECOND_AUTHOR);
 
         List<Author> expectedDeltaAuthors = new ArrayList<>();
         expectedDeltaAuthors.add(FIRST_AUTHOR);
 
         RepoConfiguration expectedBetaConfig =
-                new RepoConfiguration(new RepoLocation(TEST_REPO_BETA_LOCATION), TEST_REPO_BETA_BRANCH);
+                new RepoConfiguration(new RepoLocation(TEST_REPO_BETA_LOCATION), TEST_REPO_BETA_MASTER_BRANCH);
         expectedBetaConfig.setAuthorList(expectedBetaAuthors);
         expectedBetaConfig.setAuthorDisplayName(FIRST_AUTHOR, "Nbr");
         expectedBetaConfig.setAuthorDisplayName(SECOND_AUTHOR, "Zac");
@@ -393,7 +403,7 @@ public void repoConfig_overrideKeyword_success() throws ParseException, IOExcept
 
         Assert.assertEquals(1, configs.size());
         Assert.assertEquals(new RepoLocation(TEST_REPO_BETA_LOCATION), config.getLocation());
-        Assert.assertEquals(TEST_REPO_BETA_BRANCH, config.getBranch());
+        Assert.assertEquals(TEST_REPO_BETA_MASTER_BRANCH, config.getBranch());
         Assert.assertEquals(TEST_REPO_BETA_CONFIG_FORMATS, config.getFileTypeManager().getFormats());
         Assert.assertFalse(config.isStandaloneConfigIgnored());
         Assert.assertEquals(CommitHash.convertStringsToCommits(TEST_REPO_BETA_CONFIG_IGNORED_COMMITS),
@@ -415,7 +425,7 @@ public void repoConfig_redundantLines_success() throws ParseException, IOExcepti
         RepoConfiguration deltaConfig = configs.get(2);
 
         Assert.assertEquals(new RepoLocation(TEST_REPO_BETA_LOCATION), betaConfig.getLocation());
-        Assert.assertEquals(TEST_REPO_BETA_BRANCH, betaConfig.getBranch());
+        Assert.assertEquals(TEST_REPO_BETA_MASTER_BRANCH, betaConfig.getBranch());
         Assert.assertEquals(new RepoLocation(TEST_REPO_CHARLIE_LOCATION), charlieConfig.getLocation());
         Assert.assertEquals(TEST_REPO_CHARLIE_BRANCH, charlieConfig.getBranch());
         Assert.assertEquals(new RepoLocation(TEST_REPO_DELTA_LOCATION), deltaConfig.getLocation());

diff --git a/src/test/resources/repoconfig_merge_test/author-config.csv b/src/test/resources/repoconfig_merge_test/author-config.csv
@@ -1,3 +1,3 @@
 Repository's Location,Branch,Author's GitHub ID,Author's Email,Author's Display Name,Author's Git Author Name,Ignore Glob List
 https://github.com/reposense/testrepo-Beta.git,master,nbriannl,,Nbr,,**.java
-https://github.com/reposense/testrepo-Beta.git,master,zacharytang,,Zac,Zachary Tang,
+https://github.com/reposense/testrepo-Beta.git,,zacharytang,,Zac,Zachary Tang,**.doc
diff --git a/src/test/resources/repoconfig_merge_test/repo-config.csv b/src/test/resources/repoconfig_merge_test/repo-config.csv
@@ -1,2 +1,3 @@
 Repository's Location,Branch,File formats,Ignore Glob List,Ignore standalone config,Ignore Commits List
 https://github.com/reposense/testrepo-Beta.git,master,,collated**,,
+https://github.com/reposense/testrepo-Beta.git,add-config-json,,collated**,,