#95 Now using one instance of Git & one instance of Repository

jimbethancourt · jimbethancourt · commit 20018e56538b · 2024-09-23T07:07:03.000-05:00
diff --git a/change-proneness-ranker/src/main/java/org/hjug/git/ChangePronenessRanker.java b/change-proneness-ranker/src/main/java/org/hjug/git/ChangePronenessRanker.java
@@ -4,18 +4,17 @@
 import java.util.*;
 import lombok.extern.slf4j.Slf4j;
 import org.eclipse.jgit.api.errors.GitAPIException;
-import org.eclipse.jgit.lib.Repository;
 
 @Slf4j
 public class ChangePronenessRanker {
 
     private final TreeMap<Integer, Integer> changeCountsByTimeStamps = new TreeMap<>();
     private final Map<String, ScmLogInfo> cachedScmLogInfos = new HashMap<>();
 
-    public ChangePronenessRanker(Repository repository, GitLogReader repositoryLogReader) {
+    public ChangePronenessRanker(GitLogReader repositoryLogReader) {
         try {
             log.info("Capturing change count based on commit timestamps");
-            changeCountsByTimeStamps.putAll(repositoryLogReader.captureChangeCountByCommitTimestamp(repository));
+            changeCountsByTimeStamps.putAll(repositoryLogReader.captureChangeCountByCommitTimestamp());
         } catch (IOException | GitAPIException e) {
             log.error("Error reading from repository: {}", e.getMessage());
         }
diff --git a/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java b/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java
@@ -15,10 +15,39 @@
 import org.eclipse.jgit.util.io.NullOutputStream;
 
 @Slf4j
-public class GitLogReader {
+public class GitLogReader implements AutoCloseable {
 
     static final String JAVA_FILE_TYPE = ".java";
 
+    private Repository gitRepository;
+
+    private Git git;
+
+    public GitLogReader() {}
+
+    public GitLogReader(File basedir) throws IOException {
+        FileRepositoryBuilder repositoryBuilder = new FileRepositoryBuilder().findGitDir(basedir);
+        String gitIndexFileEnvVariable = System.getenv("GIT_INDEX_FILE");
+        if (Objects.nonNull(gitIndexFileEnvVariable)
+                && !gitIndexFileEnvVariable.trim().isEmpty()) {
+            log.debug("Setting Index File based on Env Variable GIT_INDEX_FILE {}", gitIndexFileEnvVariable);
+            repositoryBuilder = repositoryBuilder.setIndexFile(new File(gitIndexFileEnvVariable));
+        }
+
+        git = Git.open(repositoryBuilder.getGitDir());
+        gitRepository = git.getRepository();
+    }
+
+    GitLogReader(Git git) {
+        this.git = git;
+        gitRepository = git.getRepository();
+    }
+
+    @Override
+    public void close() throws Exception {
+        git.close();
+    }
+
     // Based on
     // https://github.com/Cosium/git-code-format-maven-plugin/blob/master/src/main/java/com/cosium/code/format/AbstractMavenGitCodeFormatMojo.java
     // MIT License
@@ -81,14 +110,12 @@ public Map<String, ByteArrayOutputStream> listRepositoryContentsAtHEAD(Repositor
      * Returns the number of commits and earliest commit for a given path
      * TODO: Move to a different class???
      *
-     * @param repository
      * @param path
      * @return a LogInfo object
      * @throws GitAPIException
      */
-    public ScmLogInfo fileLog(Repository repository, String path) throws GitAPIException, IOException {
-        Git git = new Git(repository);
-        ObjectId branchId = repository.resolve("HEAD");
+    public ScmLogInfo fileLog(String path) throws GitAPIException, IOException {
+        ObjectId branchId = gitRepository.resolve("HEAD");
         Iterable<RevCommit> revCommits = git.log().add(branchId).addPath(path).call();
 
         int commitCount = 0;
@@ -101,8 +128,7 @@ public ScmLogInfo fileLog(Repository repository, String path) throws GitAPIExcep
         }
 
         // based on https://stackoverflow.com/a/59274329/346247
-        int mostRecentCommit = new Git(repository)
-                .log()
+        int mostRecentCommit = git.log()
                 .add(branchId)
                 .addPath(path)
                 .setMaxCount(1)
@@ -115,49 +141,47 @@ public ScmLogInfo fileLog(Repository repository, String path) throws GitAPIExcep
     }
 
     // based on https://stackoverflow.com/questions/27361538/how-to-show-changes-between-commits-with-jgit
-    public TreeMap<Integer, Integer> captureChangeCountByCommitTimestamp(Repository repository)
-            throws IOException, GitAPIException {
+    public TreeMap<Integer, Integer> captureChangeCountByCommitTimestamp() throws IOException, GitAPIException {
 
         TreeMap<Integer, Integer> changesByCommitTimestamp = new TreeMap<>();
 
-        try (Git git = new Git(repository)) {
-            ObjectId branchId = repository.resolve("HEAD");
-            Iterable<RevCommit> commits = git.log().add(branchId).call();
+        ObjectId branchId = gitRepository.resolve("HEAD");
+        Iterable<RevCommit> commits = git.log().add(branchId).call();
 
-            RevCommit newCommit = null;
+        RevCommit newCommit = null;
 
-            for (Iterator<RevCommit> iterator = commits.iterator(); iterator.hasNext(); ) {
-                RevCommit oldCommit = iterator.next();
-
-                int count = 0;
-                if (null == newCommit) {
-                    newCommit = oldCommit;
-                    continue;
-                }
+        for (Iterator<RevCommit> iterator = commits.iterator(); iterator.hasNext(); ) {
+            RevCommit oldCommit = iterator.next();
 
-                for (DiffEntry entry : getDiffEntries(git, newCommit, oldCommit)) {
-                    if (entry.getNewPath().endsWith(JAVA_FILE_TYPE)
-                            || entry.getOldPath().endsWith(JAVA_FILE_TYPE)) {
-                        count++;
-                    }
-                }
+            int count = 0;
+            if (null == newCommit) {
+                newCommit = oldCommit;
+                continue;
+            }
 
-                if (count > 0) {
-                    changesByCommitTimestamp.put(newCommit.getCommitTime(), count);
+            for (DiffEntry entry : getDiffEntries(newCommit, oldCommit)) {
+                if (entry.getNewPath().endsWith(JAVA_FILE_TYPE)
+                        || entry.getOldPath().endsWith(JAVA_FILE_TYPE)) {
+                    count++;
                 }
+            }
 
-                // Handle first / initial commit
-                if (!iterator.hasNext()) {
-                    changesByCommitTimestamp.putAll(walkFirstCommit(repository, oldCommit));
-                }
+            if (count > 0) {
+                changesByCommitTimestamp.put(newCommit.getCommitTime(), count);
+            }
 
-                newCommit = oldCommit;
+            // Handle first / initial commit
+            if (!iterator.hasNext()) {
+                changesByCommitTimestamp.putAll(walkFirstCommit(oldCommit));
             }
+
+            newCommit = oldCommit;
         }
+
         return changesByCommitTimestamp;
     }
 
-    private List<DiffEntry> getDiffEntries(Git git, RevCommit newCommit, RevCommit oldCommit) throws IOException {
+    private List<DiffEntry> getDiffEntries(RevCommit newCommit, RevCommit oldCommit) throws IOException {
         CanonicalTreeParser oldTreeIter = new CanonicalTreeParser();
         CanonicalTreeParser newTreeIter = new CanonicalTreeParser();
         try (ObjectReader reader = git.getRepository().newObjectReader()) {
@@ -172,11 +196,11 @@ private List<DiffEntry> getDiffEntries(Git git, RevCommit newCommit, RevCommit o
         return df.scan(oldTreeIter, newTreeIter);
     }
 
-    Map<Integer, Integer> walkFirstCommit(Repository repository, RevCommit firstCommit) throws IOException {
+    Map<Integer, Integer> walkFirstCommit(RevCommit firstCommit) throws IOException {
         Map<Integer, Integer> changesByCommitTimestamp = new TreeMap<>();
         int firstCommitCount = 0;
         ObjectId treeId = firstCommit.getTree();
-        try (TreeWalk treeWalk = new TreeWalk(repository)) {
+        try (TreeWalk treeWalk = new TreeWalk(gitRepository)) {
             treeWalk.setRecursive(false);
             treeWalk.reset(treeId);
             while (treeWalk.next()) {
diff --git a/change-proneness-ranker/src/test/java/org/hjug/git/ChangePronenessRankerTest.java b/change-proneness-ranker/src/test/java/org/hjug/git/ChangePronenessRankerTest.java
@@ -1,6 +1,5 @@
 package org.hjug.git;
 
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -31,9 +30,9 @@ void testChangePronenessCalculation() throws IOException, GitAPIException {
         commitsWithChangeCounts.put(scmLogInfo.getEarliestCommit() + 5 * 60, 3);
         commitsWithChangeCounts.put(scmLogInfo.getEarliestCommit() + 10 * 60, 3);
 
-        when(repositoryLogReader.captureChangeCountByCommitTimestamp(any())).thenReturn(commitsWithChangeCounts);
+        when(repositoryLogReader.captureChangeCountByCommitTimestamp()).thenReturn(commitsWithChangeCounts);
 
-        changePronenessRanker = new ChangePronenessRanker(null, repositoryLogReader);
+        changePronenessRanker = new ChangePronenessRanker(repositoryLogReader);
         List<ScmLogInfo> scmLogInfos = new ArrayList<>();
         scmLogInfos.add(scmLogInfo);
         changePronenessRanker.rankChangeProneness(scmLogInfos);
@@ -57,8 +56,8 @@ void testRankChangeProneness() throws IOException, GitAPIException {
         commitsWithChangeCounts.put(scmLogInfo2.getEarliestCommit() + 5 * 60, 5);
         commitsWithChangeCounts.put(scmLogInfo2.getEarliestCommit() + 10 * 60, 5);
 
-        when(repositoryLogReader.captureChangeCountByCommitTimestamp(any())).thenReturn(commitsWithChangeCounts);
-        changePronenessRanker = new ChangePronenessRanker(null, repositoryLogReader);
+        when(repositoryLogReader.captureChangeCountByCommitTimestamp()).thenReturn(commitsWithChangeCounts);
+        changePronenessRanker = new ChangePronenessRanker(repositoryLogReader);
 
         List<ScmLogInfo> scmLogInfos = new ArrayList<>();
         scmLogInfos.add(scmLogInfo);
diff --git a/change-proneness-ranker/src/test/java/org/hjug/git/GitLogReaderTest.java b/change-proneness-ranker/src/test/java/org/hjug/git/GitLogReaderTest.java
@@ -40,7 +40,7 @@ void testFileLog() throws IOException, GitAPIException, InterruptedException {
         // This path works when referencing the full Tobago repository
         // String filePath = "tobago-core/src/main/java/org/apache/myfaces/tobago/facelets/AttributeHandler.java";
 
-        GitLogReader gitLogReader = new GitLogReader();
+        GitLogReader gitLogReader = new GitLogReader(git);
 
         String attributeHandler = "AttributeHandler.java";
         InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(attributeHandler);
@@ -59,7 +59,7 @@ void testFileLog() throws IOException, GitAPIException, InterruptedException {
         git.add().addFilepattern(".").call();
         RevCommit secondCommit = git.commit().setMessage("message").call();
 
-        ScmLogInfo scmLogInfo = gitLogReader.fileLog(repository, attributeHandler);
+        ScmLogInfo scmLogInfo = gitLogReader.fileLog(attributeHandler);
 
         Assertions.assertEquals(2, scmLogInfo.getCommitCount());
         Assertions.assertEquals(firstCommit.getCommitTime(), scmLogInfo.getEarliestCommit());
@@ -68,23 +68,23 @@ void testFileLog() throws IOException, GitAPIException, InterruptedException {
 
     @Test
     void testWalkFirstCommit() throws IOException, GitAPIException {
-        GitLogReader gitLogReader = new GitLogReader();
+        GitLogReader gitLogReader = new GitLogReader(git);
 
         String attributeHandler = "AttributeHandler.java";
         InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(attributeHandler);
         writeFile(attributeHandler, convertInputStreamToString(resourceAsStream));
         git.add().addFilepattern(".").call();
         RevCommit commit = git.commit().setMessage("message").call();
 
-        Map<Integer, Integer> result = gitLogReader.walkFirstCommit(repository, commit);
+        Map<Integer, Integer> result = gitLogReader.walkFirstCommit(commit);
 
         Assertions.assertTrue(result.containsKey(commit.getCommitTime()));
         Assertions.assertEquals(1, result.get(commit.getCommitTime()).intValue());
     }
 
     @Test
     void testCaptureChangCountByCommitTimestamp() throws Exception {
-        GitLogReader gitLogReader = new GitLogReader();
+        GitLogReader gitLogReader = new GitLogReader(git);
 
         String attributeHandler = "AttributeHandler.java";
         InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(attributeHandler);
@@ -106,7 +106,7 @@ void testCaptureChangCountByCommitTimestamp() throws Exception {
         git.add().addFilepattern(".").call();
         RevCommit secondCommit = git.commit().setMessage("message").call();
 
-        Map<Integer, Integer> commitCounts = gitLogReader.captureChangeCountByCommitTimestamp(repository);
+        Map<Integer, Integer> commitCounts = gitLogReader.captureChangeCountByCommitTimestamp();
 
         Assertions.assertEquals(1, commitCounts.get(firstCommit.getCommitTime()).intValue());
         Assertions.assertEquals(
diff --git a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java
@@ -18,7 +18,6 @@
 import net.sourceforge.pmd.*;
 import net.sourceforge.pmd.lang.LanguageRegistry;
 import org.eclipse.jgit.api.errors.GitAPIException;
-import org.eclipse.jgit.lib.Repository;
 import org.hjug.cycledetector.CircularReferenceChecker;
 import org.hjug.git.ChangePronenessRanker;
 import org.hjug.git.GitLogReader;
@@ -33,14 +32,14 @@
 import org.jgrapht.graph.DefaultWeightedEdge;
 
 @Slf4j
-public class CostBenefitCalculator {
+public class CostBenefitCalculator implements AutoCloseable {
 
     private final Map<String, AsSubgraph<String, DefaultWeightedEdge>> renderedSubGraphs = new HashMap<>();
 
     private Report report;
     private String repositoryPath;
-    private final GitLogReader gitLogReader = new GitLogReader();
-    private Repository repository = null;
+    private GitLogReader gitLogReader;
+
     private final ChangePronenessRanker changePronenessRanker;
     private final JavaProjectParser javaProjectParser = new JavaProjectParser();
 
@@ -52,16 +51,22 @@ public CostBenefitCalculator(String repositoryPath) {
 
         log.info("Initiating Cost Benefit calculation");
         try {
-            repository = gitLogReader.gitRepository(new File(repositoryPath));
-            for (String file :
-                    gitLogReader.listRepositoryContentsAtHEAD(repository).keySet()) {
-                log.info("Files at HEAD: {}", file);
-            }
+            gitLogReader = new GitLogReader(new File(repositoryPath));
+            //            repository = gitLogReader.gitRepository(new File(repositoryPath));
+            //            for (String file :
+            //                    gitLogReader.listRepositoryContentsAtHEAD(repository).keySet()) {
+            //                log.info("Files at HEAD: {}", file);
+            //            }
         } catch (IOException e) {
             log.error("Failure to access Git repository", e);
         }
 
-        changePronenessRanker = new ChangePronenessRanker(repository, gitLogReader);
+        changePronenessRanker = new ChangePronenessRanker(gitLogReader);
+    }
+
+    @Override
+    public void close() throws Exception {
+        gitLogReader.close();
     }
 
     public List<RankedCycle> runCycleAnalysis(String outputDirectoryPath, boolean renderImages) {
@@ -148,7 +153,7 @@ public List<RankedCycle> runCycleAnalysis(String outputDirectoryPath, boolean re
 
     private boolean isDuplicateSubGraph(AsSubgraph<String, DefaultWeightedEdge> subGraph, String vertex) {
         if (!renderedSubGraphs.isEmpty()) {
-            for (AsSubgraph<String, DefaultWeightedEdge>  renderedSubGraph : renderedSubGraphs.values()) {
+            for (AsSubgraph<String, DefaultWeightedEdge> renderedSubGraph : renderedSubGraphs.values()) {
                 if (renderedSubGraph.vertexSet().size() == subGraph.vertexSet().size()
                         && renderedSubGraph.edgeSet().size()
                                 == subGraph.edgeSet().size()
@@ -237,7 +242,7 @@ <T extends Disharmony> List<ScmLogInfo> getRankedChangeProneness(List<T> disharm
             String path = disharmony.getFileName();
             ScmLogInfo scmLogInfo = null;
             try {
-                scmLogInfo = gitLogReader.fileLog(repository, path);
+                scmLogInfo = gitLogReader.fileLog(path);
                 log.info("Successfully fetched scmLogInfo for {}", scmLogInfo.getPath());
             } catch (GitAPIException | IOException e) {
                 log.error("Error reading Git repository contents.", e);
diff --git a/refactor-first-maven-plugin/src/main/java/org/hjug/mavenreport/RefactorFirstMavenReport.java b/refactor-first-maven-plugin/src/main/java/org/hjug/mavenreport/RefactorFirstMavenReport.java
@@ -22,6 +22,7 @@
 import org.apache.maven.reporting.AbstractMavenReport;
 import org.apache.maven.reporting.MavenReportException;
 import org.hjug.cbc.CostBenefitCalculator;
+import org.hjug.cbc.RankedCycle;
 import org.hjug.cbc.RankedDisharmony;
 import org.hjug.gdg.GraphDataGenerator;
 import org.hjug.git.GitLogReader;
@@ -209,16 +210,16 @@ public void executeReport(Locale locale) throws MavenReportException {
             return;
         }
 
-        CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir);
-        try {
+        List<RankedDisharmony> rankedGodClassDisharmonies;
+        List<RankedDisharmony> rankedCBODisharmonies;
+        try (CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir)) {
             costBenefitCalculator.runPmdAnalysis();
-        } catch (IOException e) {
-            log.error("Error running PMD analysis.");
+            rankedGodClassDisharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues();
+            rankedCBODisharmonies = costBenefitCalculator.calculateCBOCostBenefitValues();
+        } catch (Exception e) {
+            log.error("Error running analysis.");
             throw new RuntimeException(e);
         }
-        List<RankedDisharmony> rankedGodClassDisharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues();
-
-        List<RankedDisharmony> rankedCBODisharmonies = costBenefitCalculator.calculateCBOCostBenefitValues();
 
         if (rankedGodClassDisharmonies.isEmpty() && rankedCBODisharmonies.isEmpty()) {
             mainSink.text("Contratulations!  " + projectName + " " + projectVersion
diff --git a/report/src/main/java/org/hjug/refactorfirst/report/CsvReport.java b/report/src/main/java/org/hjug/refactorfirst/report/CsvReport.java
@@ -11,6 +11,7 @@
 import java.util.*;
 import lombok.extern.slf4j.Slf4j;
 import org.hjug.cbc.CostBenefitCalculator;
+import org.hjug.cbc.RankedCycle;
 import org.hjug.cbc.RankedDisharmony;
 import org.hjug.git.GitLogReader;
 
@@ -83,14 +84,14 @@ public void execute(
         }
 
         // actual calcualte
-        CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir);
-        try {
+        List<RankedDisharmony> rankedDisharmonies;
+        try (CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir)) {
             costBenefitCalculator.runPmdAnalysis();
-        } catch (IOException e) {
-            log.error("Error running PMD analysis.");
+            rankedDisharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues();
+        } catch (Exception e) {
+            log.error("Error running analysis.");
             throw new RuntimeException(e);
         }
-        List<RankedDisharmony> rankedDisharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues();
 
         rankedDisharmonies.sort(Comparator.comparing(RankedDisharmony::getPriority));
 
diff --git a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java
