feat: clean up DirFilterEvents and adjust behavior for new packs

docs/additional/Migrations.md

@@ -77,4 +77,11 @@ Corresponds to version 4.0.0-4.3.1
 ## v10
 Corresponds to version 4.4.0
 
-- Stricter enforcement of legal entry names: instead of sanitization, unsupported names are logged and ignored
+- Stricter enforcement of legal entry names: instead of sanitization, unsupported names are logged and ignored
+
+## v11
+Corresponds to version 4.5.0
+
+- Directory RPOs in subdirectories are respected. Previously, only the innermost directory RPO would be used
+- Multiple replacements when resolving fallbacks for directory RPOs are prevented
+- DirFilterAdditive is now ignored as it is no longer necessary

src/main/java/io/gitlab/jfronny/respackopts/Respackopts.java

@@ -28,7 +28,7 @@ import java.util.concurrent.CompletableFuture;
 import java.util.regex.Pattern;
 
 public class Respackopts implements ModInitializer, SaveHook {
-    public static final Integer META_VERSION = 10;
+    public static final Integer META_VERSION = 11;
     public static final String FILE_EXTENSION = ".rpo";
     public static final Gson GSON = new GsonBuilder()
             .registerTypeAdapter(ConfigEnumEntry.class, new EnumEntrySerializer())

src/main/java/io/gitlab/jfronny/respackopts/filters/DirFilterEvents.java

@@ -2,12 +2,10 @@ package io.gitlab.jfronny.respackopts.filters;
 
 import io.gitlab.jfronny.libjf.ResourcePath;
 import io.gitlab.jfronny.libjf.data.manipulation.api.UserResourceEvents;
-import io.gitlab.jfronny.muscript.debug.ObjectGraphPrinter;
 import io.gitlab.jfronny.respackopts.Respackopts;
+import io.gitlab.jfronny.respackopts.filters.util.DirRpoResult;
 import io.gitlab.jfronny.respackopts.gson.AttachmentHolder;
-import io.gitlab.jfronny.respackopts.model.Condition;
 import io.gitlab.jfronny.respackopts.model.DirRpo;
-import io.gitlab.jfronny.respackopts.model.cache.CacheKey;
 import io.gitlab.jfronny.respackopts.model.cache.CachedPackState;
 import io.gitlab.jfronny.respackopts.model.enums.PackCapability;
 import io.gitlab.jfronny.respackopts.util.MetaCache;
@@ -15,7 +13,8 @@ import net.minecraft.resource.*;
 import net.minecraft.util.Identifier;
 
 import java.io.*;
-import java.util.Map;
+import java.util.LinkedList;
+import java.util.List;
 
 public enum DirFilterEvents implements UserResourceEvents.Open, UserResourceEvents.FindResource {
     INSTANCE;
@@ -29,14 +28,15 @@ public enum DirFilterEvents implements UserResourceEvents.Open, UserResourceEven
     public InputSupplier<InputStream> open(ResourceType type, Identifier id, InputSupplier<InputStream> previous, ResourcePack pack) {
         if (!MetaCache.hasCapability(pack, PackCapability.DirFilter)) return previous;
         String path = new ResourcePath(type, id).getName();
-        DirRpo rpo = findDirRpo(pack, path);
-        if (rpo != null && dirHidden(rpo, MetaCache.getKeyByPack(pack), path)) {
-            path = findReplacementDir(path, rpo);
-            if (path == null) return null;
-            ResourcePath rp = new ResourcePath(path);
-            return pack.open(rp.getType(), rp.getId());
-        }
-        return previous;
+        List<DirRpo> rpo = findDirRpos(pack, path);
+        //TODO use pattern matching for switch
+        DirRpoResult result = DirRpoResult.compute(path, rpo, MetaCache.getKeyByPack(pack));
+        if (result == DirRpoResult.ORIGINAL) return previous; // Using original file
+        if (result == DirRpoResult.IGNORE) return null; // No fallback
+        // Use fallback
+        DirRpoResult.Replacement replacement = (DirRpoResult.Replacement) result;
+        ResourcePath rp = new ResourcePath(replacement.toFallback(path));
+        return pack.open(rp.getType(), rp.getId());
     }
 
     @Override
@@ -44,81 +44,64 @@ public enum DirFilterEvents implements UserResourceEvents.Open, UserResourceEven
         // Warning: the Identifiers here DON'T CONTAIN THE TYPE!
         // Therefore, it needs to be added when calling a method that generates a ResourcePath!
         if (!MetaCache.hasCapability(pack, PackCapability.DirFilter)) return previous;
-        boolean dirFilterAdditive = MetaCache.hasCapability(pack, PackCapability.DirFilterAdditive);
         return (identifier, value) -> {
             String path = type.getDirectory() + "/" + identifier.getNamespace() + "/" + identifier.getPath();
-            DirRpo rpo = findDirRpo(pack, path);
-            if (rpo != null && dirHidden(rpo, MetaCache.getKeyByPack(pack), path)) {
-                path = findReplacementDir(path, rpo);
-                if (path != null && dirFilterAdditive) {
-                    String[] s = path.split("/", 3);
-                    if (s.length == 3) {
-                        ResourcePath rp = new ResourcePath(path);
-                        //TODO improve this impl (used for files that aren't at the original location
-                        pack.findResources(rp.getType(), rp.getId().getNamespace(), rp.getId().getPath(), (resource, resVal) -> {
-                            String p = type.getDirectory() + "/" + resource.getNamespace() + "/" + resource.getPath();
-                            p = p.replace(rpo.fallback, rpo.path + "/");
-                            previous.accept(new ResourcePath(p).getId(), resVal);
-                        });
-                    }
-                }
-            } else previous.accept(identifier, value);
+            //TODO use pattern matching for switch
+            DirRpoResult result = DirRpoResult.compute(path, findDirRpos(pack, path), MetaCache.getKeyByPack(pack));
+            if (result == DirRpoResult.ORIGINAL) { // Using original file
+                previous.accept(identifier, value);
+                return;
+            }
+            if (result == DirRpoResult.IGNORE) return; // No fallback
+            // New search for fallback path
+            DirRpoResult.Replacement replacement = (DirRpoResult.Replacement) result;
+            String newPath = replacement.toFallback(path);
+            if (newPath.split("/", 3).length == 3) {
+                ResourcePath rp = new ResourcePath(newPath);
+                pack.findResources(rp.getType(), rp.getId().getNamespace(), rp.getId().getPath(), (resource, resVal) -> {
+                    String fallbackPath = rp.getType().getDirectory() + "/" + resource.getNamespace() + "/" + resource.getPath();
+                    previous.accept(new ResourcePath(replacement.toOriginal(fallbackPath)).getId(), resVal);
+                });
+            } else Respackopts.LOGGER.error("Directory fallback path MUST be long enough to support representation as identifier (3 segments), but is too short: " + newPath);
         };
     }
 
-    private String findReplacementDir(String dir, DirRpo rpo) {
-        if (rpo.fallback == null) return null;
-        return dir.replace(rpo.path + "/", rpo.fallback);
-    }
-
-    private boolean dirHidden(DirRpo rpo, CacheKey key, String file) {
-        if (rpo.condition == null)
-            return false;
-        try {
-            return !rpo.condition.get(MetaCache.getScope(key));
-        } catch (Condition.ConditionException e) {
-            String res = "Could not evaluate condition for " + file + " (pack: " + key.packName() + ")";
-            try {
-                Respackopts.LOGGER.error(res + " with condition:\n" + ObjectGraphPrinter.printGraph(rpo.condition) + ")", e);
-            } catch (Throwable ex) {
-                Respackopts.LOGGER.error(res, e);
-            }
-        }
-        return false;
-    }
-
-    private DirRpo findDirRpo(ResourcePack pack, String name) {
-        CachedPackState state = MetaCache.getState(MetaCache.getKeyByPack(pack));
-        Map<String, DirRpo> drpReg = state.cachedDirRPOs();
+    /**
+     * Identify all directory RPOs relevant to the file at name (IE in its parent directories), from outermost to innermost
+     */
+    private List<DirRpo> findDirRpos(ResourcePack pack, String name) {
         int li = name.lastIndexOf('/');
-        if (li <= 0)
-            return null;
+        if (li <= 0) return List.of();
         name = name.substring(0, li);
-        if (drpReg.containsKey(name)) return drpReg.get(name);
-        DirRpo drp = findDirRpo(pack, name);
-        if (drp != null) {
-            drpReg.put(name, drp);
-            return drp;
+        CachedPackState state = MetaCache.getState(MetaCache.getKeyByPack(pack));
+        var cache = state.cachedDirRPOs();
+
+        // This is outside computeIfAbsent because it could cause modification of the map, which is unsupported within it
+        if (cache.containsKey(name)) return cache.get(name);
+        List<DirRpo> parentRPOs = findDirRpos(pack, name);
+        synchronized (cache) { // This is synchronized as multiple resources might be accessed at the same time, potentially causing a CME here
+            return cache.computeIfAbsent(name, parentName -> {
+                ResourcePath rp;
+                try {
+                    rp = new ResourcePath(parentName + "/" + Respackopts.FILE_EXTENSION);
+                } catch (Exception e) {
+                    return parentRPOs;
+                }
+                InputSupplier<InputStream> is = UserResourceEvents.disable(() -> pack.open(rp.getType(), rp.getId()));
+                if (is == null) return parentRPOs;
+                try (Reader w = new InputStreamReader(is.get())) {
+                    List<DirRpo> currentRPOs = new LinkedList<>(parentRPOs);
+                    DirRpo newRPO = AttachmentHolder.deserialize(state.metadata().version, w, DirRpo.class);
+                    newRPO.hydrate(parentName);
+                    if (newRPO.fallback != null && !newRPO.fallback.endsWith("/"))
+                        newRPO.fallback += "/";
+                    currentRPOs.add(newRPO);
+                    return currentRPOs;
+                } catch (IOException e) {
+                    Respackopts.LOGGER.error("Couldn't open dir rpo " + rp.getName(), e);
+                }
+                return parentRPOs;
+            });
         }
-        ResourcePath rp;
-        try {
-            rp = new ResourcePath(name + "/" + Respackopts.FILE_EXTENSION);
-        }
-        catch (Exception e) {
-            return null;
-        }
-        InputSupplier<InputStream> is = UserResourceEvents.disable(() -> pack.open(rp.getType(), rp.getId()));
-        if (is == null) return null;
-        try (Reader w = new InputStreamReader(is.get())) {
-            drp = AttachmentHolder.deserialize(state.metadata().version, w, DirRpo.class);
-            drp.hydrate(name);
-            if (drp.fallback != null && !drp.fallback.endsWith("/"))
-                drp.fallback += "/";
-            drpReg.put(name, drp);
-            return drp;
-        } catch (IOException e) {
-            Respackopts.LOGGER.error("Couldn't open dir rpo " + rp.getName(), e);
-        }
-        return null;
     }
 }

src/main/java/io/gitlab/jfronny/respackopts/filters/util/DirRpoResult.java

@@ -0,0 +1,87 @@
+package io.gitlab.jfronny.respackopts.filters.util;
+
+import io.gitlab.jfronny.commons.LazySupplier;
+import io.gitlab.jfronny.muscript.data.Scope;
+import io.gitlab.jfronny.muscript.debug.ObjectGraphPrinter;
+import io.gitlab.jfronny.respackopts.Respackopts;
+import io.gitlab.jfronny.respackopts.model.Condition;
+import io.gitlab.jfronny.respackopts.model.DirRpo;
+import io.gitlab.jfronny.respackopts.model.cache.CacheKey;
+import io.gitlab.jfronny.respackopts.util.MetaCache;
+
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public sealed interface DirRpoResult {
+    FixedState IGNORE = FixedState.IGNORE;
+    FixedState ORIGINAL = FixedState.ORIGINAL;
+    static Replacement replacement(String originalPrefix, String fallbackPrefix, int version) {
+        return new Replacement(originalPrefix, fallbackPrefix, version);
+    }
+
+    static DirRpoResult compute(String file, List<DirRpo> rpos, CacheKey key) {
+        int version = MetaCache.getState(key).metadata().version;
+        if (version < 11) rpos = rpos.isEmpty() ? rpos : List.of(rpos.get(rpos.size() - 1));
+        LazySupplier<Scope> scope = new LazySupplier<>(() -> MetaCache.getScope(key));
+        for (DirRpo rpo : rpos) {
+            if (rpo.condition == null) continue;
+            try {
+                if (rpo.condition.get(scope.get())) continue;
+                // Condition does not apply, find fallback
+                if (rpo.fallback == null) return IGNORE;
+                return replacement(rpo.path + "/", rpo.fallback, version);
+            } catch (Condition.ConditionException e) {
+                String res = "Could not evaluate condition for " + file + " (pack: " + key.packName() + ")";
+                try {
+                    Respackopts.LOGGER.error(res + " with condition:\n" + ObjectGraphPrinter.printGraph(rpo.condition) + ")", e);
+                } catch (Throwable ex) {
+                    Respackopts.LOGGER.error(res, e);
+                }
+            }
+        }
+        return ORIGINAL;
+    }
+
+    enum FixedState implements DirRpoResult {IGNORE, ORIGINAL}
+
+    final class Replacement implements DirRpoResult {
+        private final String originalPrefix;
+        private final Pattern originalPattern;
+        private final String fallbackPrefix;
+        private final Pattern fallbackPattern;
+        private final int version;
+
+        public Replacement(String originalPrefix, String fallbackPrefix, int version) {
+            this.originalPrefix = Matcher.quoteReplacement(originalPrefix);
+            this.originalPattern = Pattern.compile(Pattern.quote(originalPrefix));
+            this.fallbackPrefix = Matcher.quoteReplacement(fallbackPrefix);
+            this.fallbackPattern = Pattern.compile(Pattern.quote(fallbackPrefix));
+            this.version = version;
+        }
+
+        public String toFallback(String original) {
+            Matcher m = originalPattern.matcher(original);
+            if (version < 11) {
+                return m.replaceAll(fallbackPrefix);
+            }
+            if (!m.find()) {
+                Respackopts.LOGGER.error("Attempted conversion to fallback path, but could not find original prefix for: " + original);
+                return original;
+            }
+            return m.replaceFirst(fallbackPrefix);
+        }
+
+        public String toOriginal(String fallback) {
+            Matcher m = fallbackPattern.matcher(fallback);
+            if (version < 11) {
+                return m.replaceAll(originalPrefix);
+            }
+            if (!m.find()) {
+                Respackopts.LOGGER.error("Attempted conversion to original path, but could not find fallback prefix for: " + fallback);
+                return fallback;
+            }
+            return m.replaceFirst(originalPrefix);
+        }
+    }
+}

src/main/java/io/gitlab/jfronny/respackopts/model/cache/CachedPackState.java

@@ -5,8 +5,7 @@ import io.gitlab.jfronny.respackopts.Respackopts;
 import io.gitlab.jfronny.respackopts.model.*;
 import io.gitlab.jfronny.respackopts.model.tree.ConfigBranch;
 
-import java.util.HashMap;
-import java.util.Map;
+import java.util.*;
 
 public record CachedPackState(
         String packId,
@@ -15,7 +14,7 @@ public record CachedPackState(
         ConfigBranch configBranch,
         PackMeta metadata,
         Map<String, FileRpo> cachedFileRPOs,
-        Map<String, DirRpo> cachedDirRPOs,
+        Map<String, List<DirRpo>> cachedDirRPOs, // Directory RPOs, from outermost to innermost
         Scope executionScope
 ) {
     public CachedPackState(CacheKey key, PackMeta meta, ConfigBranch branch) {