Apply suggestions from code review

Co-authored-by: Octavia Togami <[email protected]>

Resolve many of octy's review notes

Further review fixes

Further review improvements

Fix symlink check

PR notes

Few fixes from review

IntelliJ stop resetting this config option challenge

feedback

remove the unused function

Author: me4502

worldedit-core/src/main/java/com/sk89q/worldedit/WorldEdit.java

@@ -46,6 +46,7 @@
 import com.sk89q.worldedit.function.pattern.Pattern;
 import com.sk89q.worldedit.internal.SchematicsEventListener;
 import com.sk89q.worldedit.internal.expression.invoke.ReturnException;
+import com.sk89q.worldedit.internal.schematic.SchematicsManager;
 import com.sk89q.worldedit.internal.util.LogManagerCompat;
 import com.sk89q.worldedit.math.BlockVector3;
 import com.sk89q.worldedit.scripting.CraftScriptContext;
@@ -65,7 +66,6 @@
 import com.sk89q.worldedit.util.io.file.FilenameException;
 import com.sk89q.worldedit.util.io.file.FilenameResolutionException;
 import com.sk89q.worldedit.util.io.file.InvalidFilenameException;
-import com.sk89q.worldedit.util.schematic.SchematicsManager;
 import com.sk89q.worldedit.util.task.SimpleSupervisor;
 import com.sk89q.worldedit.util.task.Supervisor;
 import com.sk89q.worldedit.util.translation.TranslationManager;
@@ -406,8 +406,10 @@ private File getSafeFileWithExtension(File dir, String filename, String extensio
         return new File(dir, filename);
     }
 
+    private static final java.util.regex.Pattern SAFE_FILENAME_REGEX = java.util.regex.Pattern.compile("^[A-Za-z0-9_\\- \\./\\\\'\\$@~!%\\^\\*\\(\\)\\[\\]\\+\\{\\},\\?]+\\.[A-Za-z0-9]+$");
+
     private boolean checkFilename(String filename) {
-        return filename.matches("^[A-Za-z0-9_\\- \\./\\\\'\\$@~!%\\^\\*\\(\\)\\[\\]\\+\\{\\},\\?]+\\.[A-Za-z0-9]+$");
+        return SAFE_FILENAME_REGEX.matcher(filename).matches();
     }
 
     /**

worldedit-core/src/main/java/com/sk89q/worldedit/command/SchematicCommands.java

@@ -39,6 +39,8 @@
 import com.sk89q.worldedit.extent.clipboard.io.ClipboardWriter;
 import com.sk89q.worldedit.extent.clipboard.io.share.ClipboardShareDestination;
 import com.sk89q.worldedit.extent.clipboard.io.share.ClipboardShareMetadata;
+import com.sk89q.worldedit.internal.annotation.SchematicPath;
+import com.sk89q.worldedit.internal.schematic.SchematicsManager;
 import com.sk89q.worldedit.internal.util.LogManagerCompat;
 import com.sk89q.worldedit.math.transform.Transform;
 import com.sk89q.worldedit.session.ClipboardHolder;
@@ -55,8 +57,6 @@
 import com.sk89q.worldedit.util.io.Closer;
 import com.sk89q.worldedit.util.io.file.FilenameException;
 import com.sk89q.worldedit.util.io.file.MorePaths;
-import com.sk89q.worldedit.util.schematic.SchematicPath;
-import com.sk89q.worldedit.util.schematic.SchematicsManager;
 import org.apache.logging.log4j.Logger;
 import org.enginehub.piston.annotation.Command;
 import org.enginehub.piston.annotation.CommandContainer;
@@ -74,6 +74,7 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
 import java.util.concurrent.Callable;
@@ -107,14 +108,15 @@ public SchematicCommands(WorldEdit worldEdit) {
     )
     @CommandPermissions({"worldedit.clipboard.load", "worldedit.schematic.load"})
     public void load(Actor actor, LocalSession session,
+                     @SchematicPath
                      @Arg(desc = "File name.")
-                     SchematicPath schematic,
+                     Path schematic,
                      @Arg(desc = "Format name.", def = "sponge")
                          ClipboardFormat format) throws FilenameException {
         LocalConfiguration config = worldEdit.getConfiguration();
 
         // Schematic.path is relative, so treat it as filename
-        String filename = schematic.path().toString();
+        String filename = schematic.toString();
         File schematicsRoot = worldEdit.getSchematicsManager().getRoot().toFile();
         File f = worldEdit.getSafeOpenFile(actor, schematicsRoot, filename,
                 BuiltInClipboardFormat.SPONGE_V3_SCHEMATIC.getPrimaryFileExtension(),
@@ -247,13 +249,14 @@ public void share(Actor actor, LocalSession session,
     )
     @CommandPermissions("worldedit.schematic.delete")
     public void delete(Actor actor,
+                       @SchematicPath
                        @Arg(desc = "File name.")
-                       SchematicPath schematic) throws WorldEditException {
+                       Path schematic) throws WorldEditException {
         LocalConfiguration config = worldEdit.getConfiguration();
         File dir = worldEdit.getWorkingDirectoryPath(config.saveDir).toFile();
 
         // Schematic.path is relative, so treat it as filename
-        String filename = schematic.path().toString();
+        String filename = schematic.toString();
         File f = worldEdit.getSafeOpenFile(actor,
                 dir, filename, "schematic", ClipboardFormats.getFileExtensionArray());
 
@@ -332,10 +335,8 @@ public void list(Actor actor,
         final String pageCommand = actor.isPlayer()
                 ? "//schem list -p %page%" + flag : null;
 
-        Comparator<SchematicPath> schematicComparator = (s0, s1) -> pathComparator.compare(s0.path(), s1.path());
-
         WorldEditAsyncCommandBuilder.createAndSendMessage(actor,
-                new SchematicListTask(schematicComparator, page, pageCommand),
+                new SchematicListTask(pathComparator::compare, page, pageCommand),
                 SubtleFormat.wrap("(Please wait... gathering schematic list.)"));
     }
 
@@ -442,11 +443,11 @@ public Consumer<Actor> call() throws Exception {
     }
 
     private static class SchematicListTask implements Callable<Component> {
-        private final Comparator<SchematicPath> pathComparator;
+        private final Comparator<Path> pathComparator;
         private final int page;
         private final String pageCommand;
 
-        SchematicListTask(Comparator<SchematicPath> pathComparator, int page, String pageCommand) {
+        SchematicListTask(Comparator<Path> pathComparator, int page, String pageCommand) {
             this.pathComparator = pathComparator;
             this.page = page;
             this.pageCommand = pageCommand;
@@ -455,7 +456,8 @@ private static class SchematicListTask implements Callable<Component> {
         @Override
         public Component call() throws Exception {
             SchematicsManager schematicsManager = WorldEdit.getInstance().getSchematicsManager();
-            List<SchematicPath> fileList = schematicsManager.getList();
+            // Copy this to a mutable list, we're sorting it below.
+            List<Path> fileList = new ArrayList<>(schematicsManager.getSchematicPaths());
 
             if (fileList.isEmpty()) {
                 return ErrorFormat.wrap("No schematics found.");
@@ -470,9 +472,9 @@ public Component call() throws Exception {
 
     private static class SchematicPaginationBox extends PaginationBox {
         private final Path rootDir;
-        private final List<SchematicPath> files;
+        private final List<Path> files;
 
-        SchematicPaginationBox(Path rootDir, List<SchematicPath> files, String pageCommand) {
+        SchematicPaginationBox(Path rootDir, List<Path> files, String pageCommand) {
             super("Available schematics", pageCommand);
             this.rootDir = rootDir;
             this.files = files;
@@ -481,7 +483,7 @@ private static class SchematicPaginationBox extends PaginationBox {
         @Override
         public Component getComponent(int number) {
             checkArgument(number < files.size() && number >= 0);
-            Path file = files.get(number).path();
+            Path file = files.get(number);
 
             String format = ClipboardFormats.getFileExtensionMap()
                 .get(MoreFiles.getFileExtension(file))

worldedit-core/src/main/java/com/sk89q/worldedit/command/argument/SchematicConverter.java

@@ -20,11 +20,11 @@
 package com.sk89q.worldedit.command.argument;
 
 import com.sk89q.worldedit.WorldEdit;
+import com.sk89q.worldedit.internal.annotation.SchematicPath;
+import com.sk89q.worldedit.internal.schematic.SchematicsManager;
 import com.sk89q.worldedit.util.formatting.text.Component;
 import com.sk89q.worldedit.util.formatting.text.TextComponent;
 import com.sk89q.worldedit.util.io.file.FilenameException;
-import com.sk89q.worldedit.util.schematic.SchematicPath;
-import com.sk89q.worldedit.util.schematic.SchematicsManager;
 import org.enginehub.piston.CommandManager;
 import org.enginehub.piston.converter.ArgumentConverter;
 import org.enginehub.piston.converter.ConversionResult;
@@ -39,10 +39,10 @@
 
 import static org.enginehub.piston.converter.SuggestionHelper.limitByPrefix;
 
-public class SchematicConverter implements ArgumentConverter<SchematicPath> {
+public class SchematicConverter implements ArgumentConverter<Path> {
 
     public static void register(WorldEdit worldEdit, CommandManager commandManager) {
-        commandManager.registerConverter(Key.of(SchematicPath.class), new SchematicConverter(worldEdit));
+        commandManager.registerConverter(Key.of(Path.class, SchematicPath.class), new SchematicConverter(worldEdit));
     }
 
     private final WorldEdit worldEdit;
@@ -63,12 +63,12 @@ public List<String> getSuggestions(String input, InjectedValueAccess context) {
         SchematicsManager schematicsManager = worldEdit.getSchematicsManager();
         Path schematicsRootPath = schematicsManager.getRoot();
 
-        return limitByPrefix(schematicsManager.getList().stream()
-                .map(s -> schematicsRootPath.relativize(s.path()).toString()), input);
+        return limitByPrefix(schematicsManager.getSchematicPaths().stream()
+                .map(s -> schematicsRootPath.relativize(s).toString()), input);
     }
 
     @Override
-    public ConversionResult<SchematicPath> convert(String s, InjectedValueAccess injectedValueAccess) {
+    public ConversionResult<Path> convert(String s, InjectedValueAccess injectedValueAccess) {
         Path schematicsRoot = worldEdit.getSchematicsManager().getRoot();
         // resolve as subpath of schematicsRoot
         Path schematicPath = schematicsRoot.resolve(s).toAbsolutePath();
@@ -80,7 +80,7 @@ public ConversionResult<SchematicPath> convert(String s, InjectedValueAccess inj
         if (Files.exists(schematicPath)) {
             // continue as relative path to schematicsRoot
             schematicPath = schematicsRoot.relativize(schematicPath);
-            return SuccessfulConversion.fromSingle(new SchematicPath(schematicPath));
+            return SuccessfulConversion.fromSingle(schematicPath);
         } else {
             return FailedConversion.from(new FilenameException(s));
         }

worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/Capability.java

@@ -21,7 +21,6 @@
 
 import com.sk89q.worldedit.WorldEdit;
 import com.sk89q.worldedit.internal.block.BlockStateIdAccess;
-import com.sk89q.worldedit.internal.util.LogManagerCompat;
 import com.sk89q.worldedit.world.block.BlockState;
 import com.sk89q.worldedit.world.block.BlockType;
 import com.sk89q.worldedit.world.registry.BlockRegistry;

…ldedit/util/schematic/SchematicPath.java …t/internal/annotation/SchematicPath.javaworldedit-core/src/main/java/com/sk89q/worldedit/util/schematic/SchematicPath.java renamed to worldedit-core/src/main/java/com/sk89q/worldedit/internal/annotation/SchematicPath.java worldedit-core/src/main/java/com/sk89q/worldedit/util/schematic/SchematicPath.java renamed to worldedit-core/src/main/java/com/sk89q/worldedit/internal/annotation/SchematicPath.java

@@ -17,18 +17,20 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-package com.sk89q.worldedit.util.schematic;
+package com.sk89q.worldedit.internal.annotation;
 
-import java.nio.file.Path;
+import org.enginehub.piston.inject.InjectAnnotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
 
 /**
- * Record representing one Schematic file.
+ * Annotation to denote an argument as a schematic path.
  */
-public record SchematicPath(Path path) {
-
-    @Override
-    public String toString() {
-        return path.toString();
-    }
-
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.PARAMETER)
+@InjectAnnotation
+public @interface SchematicPath {
 }

worldedit-core/src/main/java/com/sk89q/worldedit/internal/schematic/SchematicsManager.java

@@ -0,0 +1,115 @@
+/*
+ * WorldEdit, a Minecraft world manipulation toolkit
+ * Copyright (C) sk89q <http://www.sk89q.com>
+ * Copyright (C) WorldEdit team and contributors
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package com.sk89q.worldedit.internal.schematic;
+
+import com.sk89q.worldedit.WorldEdit;
+import com.sk89q.worldedit.internal.schematic.backends.FileWatcherSchematicsBackend;
+import com.sk89q.worldedit.internal.schematic.backends.PollingSchematicsBackend;
+import com.sk89q.worldedit.internal.schematic.backends.SchematicsBackend;
+import com.sk89q.worldedit.internal.util.LogManagerCompat;
+import org.apache.logging.log4j.Logger;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Set;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * Class that manages the known schematic files.
+ *
+ * <p>This class monitors the schematics folder for changes and maintains an up-to-date list of known
+ * schematics in order to speed up queries.
+ *
+ * <p>If initialization of the file-watching backend fails, a polling backend is used instead.
+ */
+public class SchematicsManager {
+
+    private static final Logger LOGGER = LogManagerCompat.getLogger();
+    private final WorldEdit worldEdit;
+
+    private Path schematicsDir;
+    private SchematicsBackend backend;
+
+    public SchematicsManager(WorldEdit worldEdit) {
+        this.worldEdit = worldEdit;
+    }
+
+    /**
+     * Initialize this SchematicsManager.
+     * This sets everything up, and initially scans the schematics folder.
+     */
+    public void init() {
+        try {
+            Path schematicsDirByConfig = worldEdit.getWorkingDirectoryPath(worldEdit.getConfiguration().saveDir);
+            Files.createDirectories(schematicsDirByConfig);
+            schematicsDir = schematicsDirByConfig.toRealPath();
+
+            try {
+                backend = FileWatcherSchematicsBackend.create(schematicsDir);
+            } catch (IOException e) {
+                LOGGER.warn("Failed to initialize file-monitoring based schematics backend. Falling back to polling.", e);
+                backend = PollingSchematicsBackend.create(schematicsDir);
+            }
+        } catch (IOException e) {
+            throw new RuntimeException("Failed to create schematics directory", e);
+        }
+        backend.init();
+    }
+
+    /**
+     * Uninitialize this SchematicsManager.
+     */
+    public void uninit() {
+        if (backend != null) {
+            backend.uninit();
+            backend = null;
+        }
+    }
+
+    /**
+     * Gets the root folder in which the schematics are stored.
+     *
+     * @return the root folder where schematics are stored
+     */
+    public Path getRoot() {
+        checkNotNull(schematicsDir, "not initialized");
+        return schematicsDir;
+    }
+
+    /**
+     * Gets a set of known schematics.
+     *
+     * @return a set of all known schematics
+     */
+    public Set<Path> getSchematicPaths() {
+        checkNotNull(backend);
+        return backend.getPaths();
+    }
+
+    /**
+     * Force an update of the list.
+     */
+    public void update() {
+        backend.update();
+    }
+
+}