Improve coherent attribute updater

src/main/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/impl/FileFilter.java

@@ -32,7 +32,6 @@ public class FileFilter {
             return false;
         }
 
-        ResultStatistic.getInstance().total();
         if (!hasMatchingPattern(pathName)
                 || !isNewer(pathName)
                 || isExcluded(pathName, new HashSet<>(excluded))) {

src/main/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/impl/processors/AttributeChangeProcessor.java

@@ -8,13 +8,13 @@ import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 
-public class AttributeProcessor {
+public class AttributeChangeProcessor {
     private final SubtitleTrackComparator subtitleTrackComparator;
     private final Set<String> commentaryKeywords;
     private final Set<String> hearingImpairedKeywords;
     private final Set<String> forcedKeywords;
 
-    public AttributeProcessor(String[] preferredSubtitles, Set<String> forcedKeywords, Set<String> commentaryKeywords, Set<String> hearingImpairedKeywords) {
+    public AttributeChangeProcessor(String[] preferredSubtitles, Set<String> forcedKeywords, Set<String> commentaryKeywords, Set<String> hearingImpairedKeywords) {
         this.subtitleTrackComparator = new SubtitleTrackComparator(preferredSubtitles);
         this.commentaryKeywords = commentaryKeywords;
         this.hearingImpairedKeywords = hearingImpairedKeywords;

src/main/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/impl/processors/AttributeUpdater.java

@@ -22,7 +22,7 @@ public abstract class AttributeUpdater {
 
     protected final InputConfig config;
     protected final FileProcessor fileProcessor;
-    protected final AttributeProcessor attributeProcessor;
+    protected final AttributeChangeProcessor attributeChangeProcessor;
     protected final ResultStatistic statistic = ResultStatistic.getInstance();
 
     private final ExecutorService executor;
@@ -30,7 +30,7 @@ public abstract class AttributeUpdater {
     public AttributeUpdater(InputConfig config, FileProcessor fileProcessor) {
         this.config = config;
         this.fileProcessor = fileProcessor;
-        this.attributeProcessor = new AttributeProcessor(config.getPreferredSubtitles().toArray(new String[0]), config.getForcedKeywords(), config.getCommentaryKeywords(), config.getHearingImpaired());
+        this.attributeChangeProcessor = new AttributeChangeProcessor(config.getPreferredSubtitles().toArray(new String[0]), config.getForcedKeywords(), config.getCommentaryKeywords(), config.getHearingImpaired());
         this.executor = Executors.newFixedThreadPool(config.getThreads());
     }
 

src/main/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/impl/processors/CoherentAttributeUpdater.java

@@ -34,47 +34,66 @@ public class CoherentAttributeUpdater extends SingleFileAttributeUpdater {
         List<File> files = fileProcessor.loadFiles(rootDir.getPath());
         Set<FileInfo> matchedFiles = new HashSet<>(files.size() * 2);
 
-        AttributeConfig matchedConfig = null;
         for (AttributeConfig config: config.getAttributeConfig()) {
-            for (File file: files)  {
+            AttributeConfig matchedConfig = findMatch(config, matchedFiles, files);
-                FileInfo fileInfo = fileProcessor.readAttributes(file);
-                fileInfo.resetChanges();
-                fileInfo.setMatchedConfig(null);
 
-                if (fileInfo.getTracks().isEmpty()) {
+            if (matchedConfig == null) continue;
-                    log.warn("No attributes found for file {}", file);
+            if (matchedFiles.size() != files.size()) {
-                    statistic.failure();
+                log.warn("Skip applying changes: Found coherent match, but matched count is different than file count (matched: {}, files: {}, dir: {})",
-                    break;
+                        matchedFiles.size(), files.size(), rootDir.getPath());
-                }
-
-                attributeProcessor.findDefaultMatchAndApplyChanges(fileInfo, config);
-
-                if (matchedConfig == null) matchedConfig = fileInfo.getMatchedConfig();
-                matchedFiles.add(fileInfo);
-                if (matchedConfig != fileInfo.getMatchedConfig()) {
-                    matchedConfig = null;
-                    break;
-                }
             }
 
-            if (matchedConfig != null) break;
-        }
-
-        if (matchedConfig != null) {
             matchedFiles.forEach(fileInfo -> {
-                attributeProcessor.findForcedTracksAndApplyChanges(fileInfo, config.isOverwriteForced());
+                attributeChangeProcessor.findForcedTracksAndApplyChanges(fileInfo, this.config.isOverwriteForced());
-                attributeProcessor.findCommentaryTracksAndApplyChanges(fileInfo);
+                attributeChangeProcessor.findCommentaryTracksAndApplyChanges(fileInfo);
-                attributeProcessor.findHearingImpairedTracksAndApplyChanges(fileInfo);
+                attributeChangeProcessor.findHearingImpairedTracksAndApplyChanges(fileInfo);
 
                 checkStatusAndUpdate(fileInfo);
             });
-        } else {
+            return; // match was found and process must be stopped
-            log.info("No coherent match found, trying to find coherent match in child directories: {}", rootDir.getPath());
-            matchedFiles.forEach(fileInfo -> {
-                fileInfo.resetChanges();
-                fileInfo.setMatchedConfig(null);
-            });
-            for (File dir: fileProcessor.loadDirectory(rootDir.getPath(), 1)) this.process(dir);
         }
+
+        // Couldn't match any config at current level. Resetting changes and trying to one level deeper
+        matchedFiles.forEach(fileInfo -> {
+            fileInfo.resetChanges();
+            fileInfo.setMatchedConfig(null);
+        });
+
+        if (config.isForceCoherent()) {
+            log.info("No coherent match found, aborting: {}", rootDir.getPath());
+            statistic.increaseNoSuitableConfigFoundBy(files.size()); // TODO: should matchedFiles count as already fit config?
+            return;
+        }
+
+        log.info("No coherent match found, trying to find coherent match in child directories: {}", rootDir.getPath());
+        for (File dir: fileProcessor.loadDirectory(rootDir.getPath(), 1)) this.process(dir);
+    }
+
+    private AttributeConfig findMatch(AttributeConfig config, Set<FileInfo> matchedFiles, List<File> files) {
+        AttributeConfig matchedConfig = null;
+        matchedFiles.clear();
+
+        for (File file: files)  {
+            FileInfo fileInfo = fileProcessor.readAttributes(file);
+            fileInfo.resetChanges();
+            fileInfo.setMatchedConfig(null);
+
+            if (fileInfo.getTracks().isEmpty()) {
+                log.warn("No attributes found for file {}", file);
+                statistic.failure();
+                break;
+            }
+
+            attributeChangeProcessor.findDefaultMatchAndApplyChanges(fileInfo, config);
+
+            if (matchedConfig == null) matchedConfig = fileInfo.getMatchedConfig();
+            if (matchedConfig == null || matchedConfig != fileInfo.getMatchedConfig()) {
+                matchedConfig = null;
+                break;
+            }
+            matchedFiles.add(fileInfo);
+        }
+
+        return matchedConfig;
     }
 }

src/main/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/impl/processors/SingleFileAttributeUpdater.java

@@ -29,10 +29,10 @@ public class SingleFileAttributeUpdater extends AttributeUpdater {
             return;
         }
 
-        attributeProcessor.findDefaultMatchAndApplyChanges(fileInfo, config.getAttributeConfig());
+        attributeChangeProcessor.findDefaultMatchAndApplyChanges(fileInfo, config.getAttributeConfig());
-        attributeProcessor.findForcedTracksAndApplyChanges(fileInfo, config.isOverwriteForced());
+        attributeChangeProcessor.findForcedTracksAndApplyChanges(fileInfo, config.isOverwriteForced());
-        attributeProcessor.findCommentaryTracksAndApplyChanges(fileInfo);
+        attributeChangeProcessor.findCommentaryTracksAndApplyChanges(fileInfo);
-        attributeProcessor.findHearingImpairedTracksAndApplyChanges(fileInfo);
+        attributeChangeProcessor.findHearingImpairedTracksAndApplyChanges(fileInfo);
 
         checkStatusAndUpdate(fileInfo);
     }

src/main/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/model/InputConfig.java

@@ -46,9 +46,9 @@ public class InputConfig implements CommandLine.IVersionProvider {
     private int threads;
 
     @Min(0)
-    @Option(names = {"-c", "--coherent"}, description = "try to match all files in dir of depth with the same attribute config")
+    @Option(names = {"-c", "--coherent"}, description = "try to match all files in dir of depth with the same attribute config. Attempting increasing deeper levels until match is found (worst case applying config on single file basis)")
     private Integer coherent;
-    @Option(names = {"-cf", "--force-coherent"}, description = "changes are only applied if it's a coherent match")
+    @Option(names = {"-cf", "--force-coherent"}, description = "only applies changes if a coherent match was found for the specifically entered depth")
     private boolean forceCoherent;
 
     @Option(names = {"-n", "--only-new-file"}, description = "sets filter-date to last successful execution (overwrites input of filter-date)")

src/main/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/model/ResultStatistic.java

@@ -17,7 +17,6 @@ public class ResultStatistic {
             "└─ Failed: %s%n" +
             "Runtime: %s";
     private static ResultStatistic instance;
-    private int total = 0;
     private int excluded = 0;
 
     private int shouldChange = 0;
@@ -43,12 +42,8 @@ public class ResultStatistic {
         return instance;
     }
 
-    public void increaseTotalBy(int amount) {
+    public int total() {
-        total += amount;
+        return shouldChange + noSuitableConfigFound + alreadyFits + failed;
-    }
-
-    public synchronized void total() {
-        total++;
     }
 
     public void increaseExcludedBy(int amount) {
@@ -75,6 +70,10 @@ public class ResultStatistic {
         noSuitableConfigFound++;
     }
 
+    public synchronized void increaseNoSuitableConfigFoundBy(int amount) {
+        noSuitableConfigFound += amount;
+    }
+
     public synchronized void alreadyFits() {
         alreadyFits++;
     }
@@ -114,13 +113,13 @@ public class ResultStatistic {
     }
 
     public String prettyPrint() {
-        return String.format(result, total, excluded, shouldChange, failedChanging, successfullyChanged,
+        return String.format(result, total(), excluded, shouldChange, failedChanging, successfullyChanged,
                 noSuitableConfigFound, alreadyFits, failed, formatTimer());
     }
 
     @Override
     public String toString() {
-        return "ResultStatistic: " + "total=" + total +
+        return "ResultStatistic: " + "total=" + total() +
                 ", excluded=" + excluded +
                 ", shouldChange=" + shouldChange +
                 " (failedChanging=" + failedChanging +
@@ -130,4 +129,5 @@ public class ResultStatistic {
                 ", failed=" + failed +
                 ", runtime=" + formatTimer();
     }
+
 }

src/test/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/impl/FileFilterTest.java

@@ -73,6 +73,7 @@ class FileFilterTest {
 
             assertEquals(expectedHit, fileFilter.accept(file, new HashSet<>(args)), "File is accepted");
             assertEquals(total, ResultStatistic.getInstance().getTotal(), "Total files");
+            assertEquals(total, ResultStatistic.getInstance().total(), "Total files");
             assertEquals(excluded, ResultStatistic.getInstance().getExcluded(), "Excluded files");
         }
     }

src/test/java/at/pcgamingfreaks/mkvaudiosubtitlechanger/impl/processors/AttributeChangeProcessorTest.java

@@ -16,7 +16,7 @@ import static at.pcgamingfreaks.mkvaudiosubtitlechanger.util.FileInfoTestUtil.*;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-class AttributeProcessorTest {
+class AttributeChangeProcessorTest {
 
     private static Stream<Arguments> attributeConfigMatching() {
         return Stream.of(
@@ -91,11 +91,11 @@ class AttributeProcessorTest {
     @ParameterizedTest
     @MethodSource("attributeConfigMatching")
     void findDefaultMatchAndApplyChanges(List<TrackAttributes> tracks, AttributeConfig[] config, String expectedConfig, Map<TrackAttributes, Boolean> changes) {
-        AttributeProcessor attributeProcessor = new AttributeProcessor(new String[]{}, Set.of("forced"), Set.of("commentary"), Set.of("SDH"));
+        AttributeChangeProcessor attributeChangeProcessor = new AttributeChangeProcessor(new String[]{}, Set.of("forced"), Set.of("commentary"), Set.of("SDH"));
 
         FileInfo fileInfo = new FileInfo(null);
         fileInfo.addTracks(tracks);
-        attributeProcessor.findDefaultMatchAndApplyChanges(fileInfo, config);
+        attributeChangeProcessor.findDefaultMatchAndApplyChanges(fileInfo, config);
         assertEquals(expectedConfig, fileInfo.getMatchedConfig() != null  ? fileInfo.getMatchedConfig().toStringShort() : fileInfo.getMatchedConfig());
         assertEquals(changes.size(), fileInfo.getChanges().getDefaultTrack().size());
         changes.forEach((key, value) -> {
@@ -137,15 +137,15 @@ class AttributeProcessorTest {
     @ParameterizedTest
     @MethodSource("filterForPossibleDefaults")
     void filterForPossibleDefaults(List<TrackAttributes> tracks, Set<TrackAttributes> expected) throws InvocationTargetException, IllegalAccessException {
-        AttributeProcessor attributeProcessor = new AttributeProcessor(new String[]{}, Set.of("forced"), Set.of("commentary"), Set.of("SDH"));
+        AttributeChangeProcessor attributeChangeProcessor = new AttributeChangeProcessor(new String[]{}, Set.of("forced"), Set.of("commentary"), Set.of("SDH"));
-        Optional<Method> method = Arrays.stream(AttributeProcessor.class.getDeclaredMethods())
+        Optional<Method> method = Arrays.stream(AttributeChangeProcessor.class.getDeclaredMethods())
                 .filter(m -> m.getName().equals("filterForPossibleDefaults"))
                 .findFirst();
 
         assertTrue(method.isPresent());
         Method underTest = method.get();
         underTest.setAccessible(true);
-        List<TrackAttributes> result = (List<TrackAttributes>) underTest.invoke(attributeProcessor, tracks);
+        List<TrackAttributes> result = (List<TrackAttributes>) underTest.invoke(attributeChangeProcessor, tracks);
         assertEquals(expected.size(), result.size());
         for (TrackAttributes track : result) {
             assertTrue(expected.contains(track));
@@ -154,6 +154,18 @@ class AttributeProcessorTest {
 
     private static Stream<Arguments> findForcedTracksAndApplyChanges() {
         return Stream.of(
+                Arguments.of(List.of(),
+                        Set.of("song & signs"), false,
+                        Map.ofEntries()
+                ),
+                Arguments.of(List.of(withName(SUB_GER, "song & signs"), SUB_GER),
+                        Set.of("song & signs"), false,
+                        Map.ofEntries(on(withName(SUB_GER, "song & signs")))
+                ),
+                Arguments.of(List.of(withName(SUB_GER, "song & signs"), withName(SUB_GER, "")),
+                        Set.of("song & signs"), false,
+                        Map.ofEntries(on(withName(SUB_GER, "song & signs")))
+                ),
                 Arguments.of(List.of(withName(SUB_GER, "song & signs"), withName(SUB_GER, null)),
                         Set.of("song & signs"), false,
                         Map.ofEntries(on(withName(SUB_GER, "song & signs")))
@@ -176,11 +188,11 @@ class AttributeProcessorTest {
     @ParameterizedTest
     @MethodSource("findForcedTracksAndApplyChanges")
     void findForcedTracksAndApplyChanges(List<TrackAttributes> tracks, Set<String> keywords, boolean overwrite, Map<TrackAttributes, Boolean> changes) {
-        AttributeProcessor attributeProcessor = new AttributeProcessor(new String[]{}, keywords, Set.of(), Set.of());
+        AttributeChangeProcessor attributeChangeProcessor = new AttributeChangeProcessor(new String[]{}, keywords, Set.of(), Set.of());
 
         FileInfo fileInfo = new FileInfo(null);
         fileInfo.addTracks(tracks);
-        attributeProcessor.findForcedTracksAndApplyChanges(fileInfo, overwrite);
+        attributeChangeProcessor.findForcedTracksAndApplyChanges(fileInfo, overwrite);
 
         assertEquals(changes.size(), fileInfo.getChanges().getForcedTrack().size());
         changes.forEach((key, value) -> {
@@ -213,11 +225,11 @@ class AttributeProcessorTest {
     @ParameterizedTest
     @MethodSource("findCommentaryTracksAndApplyChanges")
     void findCommentaryTracksAndApplyChanges(List<TrackAttributes> tracks, Set<String> keywords, Map<TrackAttributes, Boolean> changes) {
-        AttributeProcessor attributeProcessor = new AttributeProcessor(new String[]{}, Set.of(), keywords, Set.of());
+        AttributeChangeProcessor attributeChangeProcessor = new AttributeChangeProcessor(new String[]{}, Set.of(), keywords, Set.of());
 
         FileInfo fileInfo = new FileInfo(null);
         fileInfo.addTracks(tracks);
-        attributeProcessor.findCommentaryTracksAndApplyChanges(fileInfo);
+        attributeChangeProcessor.findCommentaryTracksAndApplyChanges(fileInfo);
 
         assertEquals(changes.size(), fileInfo.getChanges().getCommentaryTrack().size());
         changes.forEach((key, value) -> {
@@ -250,11 +262,11 @@ class AttributeProcessorTest {
     @ParameterizedTest
     @MethodSource("findCommentaryTracksAndApplyChanges")
     void findHearingImpairedTracksAndApplyChanges(List<TrackAttributes> tracks, Set<String> keywords, Map<TrackAttributes, Boolean> changes) {
-        AttributeProcessor attributeProcessor =  new AttributeProcessor(new String[]{}, Set.of(), Set.of(), keywords);
+        AttributeChangeProcessor attributeChangeProcessor =  new AttributeChangeProcessor(new String[]{}, Set.of(), Set.of(), keywords);
 
         FileInfo fileInfo = new FileInfo(null);
         fileInfo.addTracks(tracks);
-        attributeProcessor.findHearingImpairedTracksAndApplyChanges(fileInfo);
+        attributeChangeProcessor.findHearingImpairedTracksAndApplyChanges(fileInfo);
 
         assertEquals(changes.size(), fileInfo.getChanges().getHearingImpairedTrack().size());
         changes.forEach((key, value) -> {