From 99aadd0a0d33b5a9b5622e88e5d2a0e919d80b7c Mon Sep 17 00:00:00 2001 From: David Bilge Date: Wed, 14 Aug 2024 21:29:51 +0200 Subject: [PATCH] GH-31 Refactor git provider strategies, extracting common filtering code --- .../modulith/DiffStrategy.java | 14 ++------- .../springframework/modulith/FileChange.java | 4 +++ .../modulith/GitProviderStrategy.java | 2 +- .../springframework/modulith/JGitSupport.java | 18 +++++++++++ .../modulith/ModulithExecutionExtension.java | 30 ++++++++++++++----- .../modulith/UncommitedChangesStrategy.java | 15 ++++------ .../modulith/UnpushedChangesStrategy.java | 17 +++-------- 7 files changed, 57 insertions(+), 43 deletions(-) create mode 100644 spring-modulith-junit/src/main/java/org/springframework/modulith/FileChange.java create mode 100644 spring-modulith-junit/src/main/java/org/springframework/modulith/JGitSupport.java 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 index 43805fa1..cfc3d26f 100644 --- 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 getModifiedFiles(PropertyResolver propertyResolver) throws IOException { + public Set 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 getModifiedFiles(PropertyResolver propertyResolver) throws IO diffFormatter.setRepository(git.getRepository()); List 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 new file mode 100644 index 00000000..129afcb5 --- /dev/null +++ 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 index e096a6d5..a9f8535f 100644 --- 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 getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException; + Set 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 new file mode 100644 index 00000000..f6f2ea5f --- /dev/null +++ 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 convertDiffEntriesToFileChanges(Collection 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/spring-modulith-junit/src/main/java/org/springframework/modulith/ModulithExecutionExtension.java b/spring-modulith-junit/src/main/java/org/springframework/modulith/ModulithExecutionExtension.java index f6989005..e8f5d573 100644 --- a/spring-modulith-junit/src/main/java/org/springframework/modulith/ModulithExecutionExtension.java +++ b/spring-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> set = new HashSet<>(); + Set> changedClasses = new HashSet<>(); try { - for (String file : strategy.getModifiedFiles(applicationContext.getEnvironment())) { + Set modifiedFiles = strategy.getModifiedFiles(applicationContext.getEnvironment()); + + Set 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/spring-modulith-junit/src/main/java/org/springframework/modulith/UncommitedChangesStrategy.java b/spring-modulith-junit/src/main/java/org/springframework/modulith/UncommitedChangesStrategy.java index 6dd45481..4bb99be1 100644 --- a/spring-modulith-junit/src/main/java/org/springframework/modulith/UncommitedChangesStrategy.java +++ b/spring-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 getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException { + public Set getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException { try (var gitDir = new FileRepositoryBuilder().findGitDir().build()) { Git git = new Git(gitDir); Status status = git.status().call(); - Set 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/spring-modulith-junit/src/main/java/org/springframework/modulith/UnpushedChangesStrategy.java b/spring-modulith-junit/src/main/java/org/springframework/modulith/UnpushedChangesStrategy.java index 6aabfd62..eab442da 100644 --- a/spring-modulith-junit/src/main/java/org/springframework/modulith/UnpushedChangesStrategy.java +++ b/spring-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 getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException { + public Set getModifiedFiles(PropertyResolver propertyResolver) throws IOException, GitAPIException { try (var gitDir = new FileRepositoryBuilder().findGitDir().build()) { Git git = new Git(gitDir); - List 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 diffEntries = git.diff().call(); + + return JGitSupport.convertDiffEntriesToFileChanges(diffEntries); } }