spring-projectsGH-31 Refactor git provider strategies, extracting com…

…mon filtering code
lukasdo · Aug 14, 2024 · 99aadd0 · 99aadd0
1 parent 5c1e725
commit 99aadd0
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 43 deletions.
diff --git a/spring-modulith-junit/src/main/java/org/springframework/modulith/DiffStrategy.java b/spring-modulith-junit/src/main/java/org/springframework/modulith/DiffStrategy.java
@@ -5,7 +5,6 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.diff.DiffFormatter;
@@ -17,7 +16,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.core.env.PropertyResolver;
-import org.springframework.util.ClassUtils;
 
 /**
  * Implementation to get changes between HEAD and a complete or abbreviated SHA-1
@@ -26,7 +24,7 @@ public class DiffStrategy implements GitProviderStrategy {
 	private static final Logger log = LoggerFactory.getLogger(DiffStrategy.class);
 
 	@Override
-	public Set<String> getModifiedFiles(PropertyResolver propertyResolver) throws IOException {
+	public Set<FileChange> getModifiedFiles(PropertyResolver propertyResolver) throws IOException {
 		String commitIdToCompareTo = propertyResolver.getProperty(CONFIG_PROPERTY_PREFIX + ".reference-commit");
 
 		try (var gitDir = new FileRepositoryBuilder().findGitDir().build()) {
@@ -53,15 +51,7 @@ public Set<String> getModifiedFiles(PropertyResolver propertyResolver) throws IO
 			diffFormatter.setRepository(git.getRepository());
 			List<DiffEntry> entries = diffFormatter.scan(oldTreeIter, newTreeIter);
 
-			return entries.stream()
-				// Consider old path of file as well?
-				.map(DiffEntry::getNewPath)
-				.map(ClassUtils::convertResourcePathToClassName)
-				.filter(s -> s.contains(PACKAGE_PREFIX)) // DELETED will be filtered as new path will be /dev/null
-				.filter(s -> s.endsWith(CLASS_FILE_SUFFIX))
-				.map(s -> s.substring(s.lastIndexOf(PACKAGE_PREFIX) + PACKAGE_PREFIX.length() + 1,
-					s.length() - CLASS_FILE_SUFFIX.length()))
-				.collect(Collectors.toSet());
+			return JGitSupport.convertDiffEntriesToFileChanges(entries);
 		}
 	}
 }
diff --git a/spring-modulith-junit/src/main/java/org/springframework/modulith/FileChange.java b/spring-modulith-junit/src/main/java/org/springframework/modulith/FileChange.java
@@ -0,0 +1,4 @@
+package org.springframework.modulith;
+
+public record FileChange(String path) {
+}
diff --git a/spring-modulith-junit/src/main/java/org/springframework/modulith/GitProviderStrategy.java b/spring-modulith-junit/src/main/java/org/springframework/modulith/GitProviderStrategy.java
@@ -10,6 +10,6 @@ public interface GitProviderStrategy {
 	String CLASS_FILE_SUFFIX = ".java";
 	String PACKAGE_PREFIX = "src.main.java";
 
-	Set<String> getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException;
+	Set<FileChange> getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException;
 
 }
diff --git a/spring-modulith-junit/src/main/java/org/springframework/modulith/JGitSupport.java b/spring-modulith-junit/src/main/java/org/springframework/modulith/JGitSupport.java
@@ -0,0 +1,18 @@
+package org.springframework.modulith;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.eclipse.jgit.diff.DiffEntry;
+
+final class JGitSupport {
+	private JGitSupport() {}
+
+	static Set<FileChange> convertDiffEntriesToFileChanges(Collection<DiffEntry> diffEntries) {
+		return diffEntries.stream()
+			.flatMap(entry -> Stream.of(new FileChange(entry.getNewPath()), new FileChange(entry.getOldPath())))
+			.filter(change -> !change.path().equals("/dev/null"))
+			.collect(Collectors.toSet());
+	}
+}
diff --git a/...modulith-junit/src/main/java/org/springframework/modulith/ModulithExecutionExtension.java b/...modulith-junit/src/main/java/org/springframework/modulith/ModulithExecutionExtension.java
@@ -1,5 +1,7 @@
 package org.springframework.modulith;
 
+import java.util.stream.Collectors;
+
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.junit.jupiter.api.extension.ConditionEvaluationResult;
 import org.junit.jupiter.api.extension.ExecutionCondition;
@@ -19,6 +21,8 @@
 import java.util.ServiceLoader.Provider;
 import java.util.Set;
 
+import static org.springframework.modulith.GitProviderStrategy.CLASS_FILE_SUFFIX;
+import static org.springframework.modulith.GitProviderStrategy.PACKAGE_PREFIX;
 import static org.springframework.test.context.junit.jupiter.SpringExtension.getApplicationContext;
 
 
@@ -92,21 +96,33 @@ public void writeChangedFilesToStore(ExtensionContext context, ApplicationContex
 
         ExtensionContext.Store store = context.getRoot().getStore(ExtensionContext.Namespace.GLOBAL);
         store.getOrComputeIfAbsent(PROJECT_ID, s -> {
-            Set<Class<?>> set = new HashSet<>();
+            Set<Class<?>> changedClasses = new HashSet<>();
             try {
-                for (String file : strategy.getModifiedFiles(applicationContext.getEnvironment())) {
+                Set<FileChange> modifiedFiles = strategy.getModifiedFiles(applicationContext.getEnvironment());
+
+                Set<String> changedClassNames = modifiedFiles.stream()
+                        // Consider old path of file as well?
+                        .map(FileChange::path)
+                        .map(ClassUtils::convertResourcePathToClassName)
+                        .filter(path -> path.contains(PACKAGE_PREFIX)) // DELETED will be filtered as new path will be /dev/null
+                        .filter(path -> path.endsWith(CLASS_FILE_SUFFIX))
+                        .map(path -> path.substring(path.lastIndexOf(PACKAGE_PREFIX) + PACKAGE_PREFIX.length() + 1,
+                                path.length() - CLASS_FILE_SUFFIX.length()))
+                        .collect(Collectors.toSet());
+
+                for (String className : changedClassNames) {
                     try {
-                        Class<?> aClass = ClassUtils.forName(file, null);
-                        set.add(aClass);
+                        Class<?> aClass = ClassUtils.forName(className, null);
+                        changedClasses.add(aClass);
                     } catch (ClassNotFoundException e) {
-                        log.trace("ModulithExecutionExtension: Unable to find class for file {}", file);
+                        log.trace("ModulithExecutionExtension: Unable to find class \"{}\"", className);
                     }
                 }
-                return set;
+                return changedClasses;
             } catch (IOException | GitAPIException e) {
                 log.error("ModulithExecutionExtension: Unable to fetch changed files, executing all tests", e);
                 store.put(PROJECT_ERROR, e);
-                return set;
+                return changedClasses;
             }
         });
     }

diff --git a/...-modulith-junit/src/main/java/org/springframework/modulith/UncommitedChangesStrategy.java b/...-modulith-junit/src/main/java/org/springframework/modulith/UncommitedChangesStrategy.java
@@ -1,5 +1,6 @@
 package org.springframework.modulith;
 
+import com.tngtech.archunit.thirdparty.com.google.common.collect.Streams;
 import java.io.IOException;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -8,7 +9,6 @@
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
 import org.springframework.core.env.PropertyResolver;
-import org.springframework.util.ClassUtils;
 
 /**
  * Implementation to get latest local file changes.
@@ -19,18 +19,13 @@ public class UncommitedChangesStrategy implements GitProviderStrategy {
 
 
 	@Override
-	public Set<String> getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException {
+	public Set<FileChange> getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException {
 		try (var gitDir = new FileRepositoryBuilder().findGitDir().build()) {
 			Git git = new Git(gitDir);
 			Status status = git.status().call();
-			Set<String> modified = status.getUncommittedChanges();
-			//TODO:: Add untracked
-			return modified.stream()
-				.map(ClassUtils::convertResourcePathToClassName)
-				.filter(s -> s.contains(PACKAGE_PREFIX))
-				.filter(s -> s.endsWith(CLASS_FILE_SUFFIX))
-				.map(s -> s.substring(s.lastIndexOf(PACKAGE_PREFIX) + PACKAGE_PREFIX.length() + 1,
-					s.length() - CLASS_FILE_SUFFIX.length()))
+
+			return Streams.concat(status.getUncommittedChanges().stream(), status.getUntracked().stream())
+				.map(FileChange::new)
 				.collect(Collectors.toSet());
 		}
 	}

diff --git a/...ng-modulith-junit/src/main/java/org/springframework/modulith/UnpushedChangesStrategy.java b/...ng-modulith-junit/src/main/java/org/springframework/modulith/UnpushedChangesStrategy.java
@@ -3,32 +3,23 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
 import org.springframework.core.env.PropertyResolver;
-import org.springframework.util.ClassUtils;
 
 /**
  * Implementation to get unpushed changes
  */
 public class UnpushedChangesStrategy implements GitProviderStrategy {
 	@Override
-	public Set<String> getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException {
+	public Set<FileChange> getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException {
 		try (var gitDir = new FileRepositoryBuilder().findGitDir().build()) {
 			Git git = new Git(gitDir);
-			List<DiffEntry> call = git.diff().call();
-			return call.stream()
-				// Consider old path of file as well?
-				.map(DiffEntry::getNewPath)
-				.map(ClassUtils::convertResourcePathToClassName)
-				.filter(s -> s.contains(PACKAGE_PREFIX))
-				.filter(s -> s.endsWith(CLASS_FILE_SUFFIX))
-				.map(s -> s.substring(s.lastIndexOf(PACKAGE_PREFIX) + PACKAGE_PREFIX.length() + 1,
-					s.length() - CLASS_FILE_SUFFIX.length()))
-				.collect(Collectors.toSet());
+			List<DiffEntry> diffEntries = git.diff().call();
+
+			return JGitSupport.convertDiffEntriesToFileChanges(diffEntries);
 		}
 	}