Skip to content

Commit c933b7a

Browse files
me4502me4502
committed
Apply suggestions from code review
Co-authored-by: Octavia Togami <octavia.togami@gmail.com> 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
1 parent ce5385d commit c933b7a

13 files changed

Lines changed: 282 additions & 371 deletions

File tree

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import com.sk89q.worldedit.function.pattern.Pattern;
4747
import com.sk89q.worldedit.internal.SchematicsEventListener;
4848
import com.sk89q.worldedit.internal.expression.invoke.ReturnException;
49+
import com.sk89q.worldedit.internal.schematic.SchematicsManager;
4950
import com.sk89q.worldedit.internal.util.LogManagerCompat;
5051
import com.sk89q.worldedit.math.BlockVector3;
5152
import com.sk89q.worldedit.scripting.CraftScriptContext;
@@ -65,7 +66,6 @@
6566
import com.sk89q.worldedit.util.io.file.FilenameException;
6667
import com.sk89q.worldedit.util.io.file.FilenameResolutionException;
6768
import com.sk89q.worldedit.util.io.file.InvalidFilenameException;
68-
import com.sk89q.worldedit.util.schematic.SchematicsManager;
6969
import com.sk89q.worldedit.util.task.SimpleSupervisor;
7070
import com.sk89q.worldedit.util.task.Supervisor;
7171
import com.sk89q.worldedit.util.translation.TranslationManager;
@@ -406,8 +406,10 @@ private File getSafeFileWithExtension(File dir, String filename, String extensio
406406
return new File(dir, filename);
407407
}
408408

409+
private static final java.util.regex.Pattern SAFE_FILENAME_REGEX = java.util.regex.Pattern.compile("^[A-Za-z0-9_\\- \\./\\\\'\\$@~!%\\^\\*\\(\\)\\[\\]\\+\\{\\},\\?]+\\.[A-Za-z0-9]+$");
410+
409411
private boolean checkFilename(String filename) {
410-
return filename.matches("^[A-Za-z0-9_\\- \\./\\\\'\\$@~!%\\^\\*\\(\\)\\[\\]\\+\\{\\},\\?]+\\.[A-Za-z0-9]+$");
412+
return SAFE_FILENAME_REGEX.matcher(filename).matches();
411413
}
412414

413415
/**

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

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
import com.sk89q.worldedit.extent.clipboard.io.ClipboardWriter;
4040
import com.sk89q.worldedit.extent.clipboard.io.share.ClipboardShareDestination;
4141
import com.sk89q.worldedit.extent.clipboard.io.share.ClipboardShareMetadata;
42+
import com.sk89q.worldedit.internal.annotation.SchematicPath;
43+
import com.sk89q.worldedit.internal.schematic.SchematicsManager;
4244
import com.sk89q.worldedit.internal.util.LogManagerCompat;
4345
import com.sk89q.worldedit.math.transform.Transform;
4446
import com.sk89q.worldedit.session.ClipboardHolder;
@@ -55,8 +57,6 @@
5557
import com.sk89q.worldedit.util.io.Closer;
5658
import com.sk89q.worldedit.util.io.file.FilenameException;
5759
import com.sk89q.worldedit.util.io.file.MorePaths;
58-
import com.sk89q.worldedit.util.schematic.SchematicPath;
59-
import com.sk89q.worldedit.util.schematic.SchematicsManager;
6060
import org.apache.logging.log4j.Logger;
6161
import org.enginehub.piston.annotation.Command;
6262
import org.enginehub.piston.annotation.CommandContainer;
@@ -74,6 +74,7 @@
7474
import java.io.IOException;
7575
import java.io.OutputStream;
7676
import java.nio.file.Path;
77+
import java.util.ArrayList;
7778
import java.util.Comparator;
7879
import java.util.List;
7980
import java.util.concurrent.Callable;
@@ -107,14 +108,15 @@ public SchematicCommands(WorldEdit worldEdit) {
107108
)
108109
@CommandPermissions({"worldedit.clipboard.load", "worldedit.schematic.load"})
109110
public void load(Actor actor, LocalSession session,
111+
@SchematicPath
110112
@Arg(desc = "File name.")
111-
SchematicPath schematic,
113+
Path schematic,
112114
@Arg(desc = "Format name.", def = "sponge")
113115
ClipboardFormat format) throws FilenameException {
114116
LocalConfiguration config = worldEdit.getConfiguration();
115117

116118
// Schematic.path is relative, so treat it as filename
117-
String filename = schematic.path().toString();
119+
String filename = schematic.toString();
118120
File schematicsRoot = worldEdit.getSchematicsManager().getRoot().toFile();
119121
File f = worldEdit.getSafeOpenFile(actor, schematicsRoot, filename,
120122
BuiltInClipboardFormat.SPONGE_V3_SCHEMATIC.getPrimaryFileExtension(),
@@ -247,13 +249,14 @@ public void share(Actor actor, LocalSession session,
247249
)
248250
@CommandPermissions("worldedit.schematic.delete")
249251
public void delete(Actor actor,
252+
@SchematicPath
250253
@Arg(desc = "File name.")
251-
SchematicPath schematic) throws WorldEditException {
254+
Path schematic) throws WorldEditException {
252255
LocalConfiguration config = worldEdit.getConfiguration();
253256
File dir = worldEdit.getWorkingDirectoryPath(config.saveDir).toFile();
254257

255258
// Schematic.path is relative, so treat it as filename
256-
String filename = schematic.path().toString();
259+
String filename = schematic.toString();
257260
File f = worldEdit.getSafeOpenFile(actor,
258261
dir, filename, "schematic", ClipboardFormats.getFileExtensionArray());
259262

@@ -332,10 +335,8 @@ public void list(Actor actor,
332335
final String pageCommand = actor.isPlayer()
333336
? "//schem list -p %page%" + flag : null;
334337

335-
Comparator<SchematicPath> schematicComparator = (s0, s1) -> pathComparator.compare(s0.path(), s1.path());
336-
337338
WorldEditAsyncCommandBuilder.createAndSendMessage(actor,
338-
new SchematicListTask(schematicComparator, page, pageCommand),
339+
new SchematicListTask(pathComparator::compare, page, pageCommand),
339340
SubtleFormat.wrap("(Please wait... gathering schematic list.)"));
340341
}
341342

@@ -442,11 +443,11 @@ public Consumer<Actor> call() throws Exception {
442443
}
443444

444445
private static class SchematicListTask implements Callable<Component> {
445-
private final Comparator<SchematicPath> pathComparator;
446+
private final Comparator<Path> pathComparator;
446447
private final int page;
447448
private final String pageCommand;
448449

449-
SchematicListTask(Comparator<SchematicPath> pathComparator, int page, String pageCommand) {
450+
SchematicListTask(Comparator<Path> pathComparator, int page, String pageCommand) {
450451
this.pathComparator = pathComparator;
451452
this.page = page;
452453
this.pageCommand = pageCommand;
@@ -455,7 +456,8 @@ private static class SchematicListTask implements Callable<Component> {
455456
@Override
456457
public Component call() throws Exception {
457458
SchematicsManager schematicsManager = WorldEdit.getInstance().getSchematicsManager();
458-
List<SchematicPath> fileList = schematicsManager.getList();
459+
// Copy this to a mutable list, we're sorting it below.
460+
List<Path> fileList = new ArrayList<>(schematicsManager.getSchematicPaths());
459461

460462
if (fileList.isEmpty()) {
461463
return ErrorFormat.wrap("No schematics found.");
@@ -470,9 +472,9 @@ public Component call() throws Exception {
470472

471473
private static class SchematicPaginationBox extends PaginationBox {
472474
private final Path rootDir;
473-
private final List<SchematicPath> files;
475+
private final List<Path> files;
474476

475-
SchematicPaginationBox(Path rootDir, List<SchematicPath> files, String pageCommand) {
477+
SchematicPaginationBox(Path rootDir, List<Path> files, String pageCommand) {
476478
super("Available schematics", pageCommand);
477479
this.rootDir = rootDir;
478480
this.files = files;
@@ -481,7 +483,7 @@ private static class SchematicPaginationBox extends PaginationBox {
481483
@Override
482484
public Component getComponent(int number) {
483485
checkArgument(number < files.size() && number >= 0);
484-
Path file = files.get(number).path();
486+
Path file = files.get(number);
485487

486488
String format = ClipboardFormats.getFileExtensionMap()
487489
.get(MoreFiles.getFileExtension(file))

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@
2020
package com.sk89q.worldedit.command.argument;
2121

2222
import com.sk89q.worldedit.WorldEdit;
23+
import com.sk89q.worldedit.internal.annotation.SchematicPath;
24+
import com.sk89q.worldedit.internal.schematic.SchematicsManager;
2325
import com.sk89q.worldedit.util.formatting.text.Component;
2426
import com.sk89q.worldedit.util.formatting.text.TextComponent;
2527
import com.sk89q.worldedit.util.io.file.FilenameException;
26-
import com.sk89q.worldedit.util.schematic.SchematicPath;
27-
import com.sk89q.worldedit.util.schematic.SchematicsManager;
2828
import org.enginehub.piston.CommandManager;
2929
import org.enginehub.piston.converter.ArgumentConverter;
3030
import org.enginehub.piston.converter.ConversionResult;
@@ -39,10 +39,10 @@
3939

4040
import static org.enginehub.piston.converter.SuggestionHelper.limitByPrefix;
4141

42-
public class SchematicConverter implements ArgumentConverter<SchematicPath> {
42+
public class SchematicConverter implements ArgumentConverter<Path> {
4343

4444
public static void register(WorldEdit worldEdit, CommandManager commandManager) {
45-
commandManager.registerConverter(Key.of(SchematicPath.class), new SchematicConverter(worldEdit));
45+
commandManager.registerConverter(Key.of(Path.class, SchematicPath.class), new SchematicConverter(worldEdit));
4646
}
4747

4848
private final WorldEdit worldEdit;
@@ -63,12 +63,12 @@ public List<String> getSuggestions(String input, InjectedValueAccess context) {
6363
SchematicsManager schematicsManager = worldEdit.getSchematicsManager();
6464
Path schematicsRootPath = schematicsManager.getRoot();
6565

66-
return limitByPrefix(schematicsManager.getList().stream()
67-
.map(s -> schematicsRootPath.relativize(s.path()).toString()), input);
66+
return limitByPrefix(schematicsManager.getSchematicPaths().stream()
67+
.map(s -> schematicsRootPath.relativize(s).toString()), input);
6868
}
6969

7070
@Override
71-
public ConversionResult<SchematicPath> convert(String s, InjectedValueAccess injectedValueAccess) {
71+
public ConversionResult<Path> convert(String s, InjectedValueAccess injectedValueAccess) {
7272
Path schematicsRoot = worldEdit.getSchematicsManager().getRoot();
7373
// resolve as subpath of schematicsRoot
7474
Path schematicPath = schematicsRoot.resolve(s).toAbsolutePath();
@@ -80,7 +80,7 @@ public ConversionResult<SchematicPath> convert(String s, InjectedValueAccess inj
8080
if (Files.exists(schematicPath)) {
8181
// continue as relative path to schematicsRoot
8282
schematicPath = schematicsRoot.relativize(schematicPath);
83-
return SuccessfulConversion.fromSingle(new SchematicPath(schematicPath));
83+
return SuccessfulConversion.fromSingle(schematicPath);
8484
} else {
8585
return FailedConversion.from(new FilenameException(s));
8686
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import com.sk89q.worldedit.WorldEdit;
2323
import com.sk89q.worldedit.internal.block.BlockStateIdAccess;
24-
import com.sk89q.worldedit.internal.util.LogManagerCompat;
2524
import com.sk89q.worldedit.world.block.BlockState;
2625
import com.sk89q.worldedit.world.block.BlockType;
2726
import com.sk89q.worldedit.world.registry.BlockRegistry;

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,20 @@
1717
* along with this program. If not, see <https://www.gnu.org/licenses/>.
1818
*/
1919

20-
package com.sk89q.worldedit.util.schematic;
20+
package com.sk89q.worldedit.internal.annotation;
2121

22-
import java.nio.file.Path;
22+
import org.enginehub.piston.inject.InjectAnnotation;
23+
24+
import java.lang.annotation.ElementType;
25+
import java.lang.annotation.Retention;
26+
import java.lang.annotation.RetentionPolicy;
27+
import java.lang.annotation.Target;
2328

2429
/**
25-
* Record representing one Schematic file.
30+
* Annotation to denote an argument as a schematic path.
2631
*/
27-
public record SchematicPath(Path path) {
28-
29-
@Override
30-
public String toString() {
31-
return path.toString();
32-
}
33-
32+
@Retention(RetentionPolicy.RUNTIME)
33+
@Target(ElementType.PARAMETER)
34+
@InjectAnnotation
35+
public @interface SchematicPath {
3436
}

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

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* WorldEdit, a Minecraft world manipulation toolkit
3+
* Copyright (C) sk89q <http://www.sk89q.com>
4+
* Copyright (C) WorldEdit team and contributors
5+
*
6+
* This program is free software: you can redistribute it and/or modify
7+
* it under the terms of the GNU General Public License as published by
8+
* the Free Software Foundation, either version 3 of the License, or
9+
* (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU General Public License
17+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
18+
*/
19+
20+
package com.sk89q.worldedit.internal.schematic;
21+
22+
import com.sk89q.worldedit.WorldEdit;
23+
import com.sk89q.worldedit.internal.schematic.backends.FileWatcherSchematicsBackend;
24+
import com.sk89q.worldedit.internal.schematic.backends.PollingSchematicsBackend;
25+
import com.sk89q.worldedit.internal.schematic.backends.SchematicsBackend;
26+
import com.sk89q.worldedit.internal.util.LogManagerCompat;
27+
import org.apache.logging.log4j.Logger;
28+
29+
import java.io.IOException;
30+
import java.nio.file.Files;
31+
import java.nio.file.Path;
32+
import java.util.Set;
33+
34+
import static com.google.common.base.Preconditions.checkNotNull;
35+
36+
/**
37+
* Class that manages the known schematic files.
38+
*
39+
* <p>This class monitors the schematics folder for changes and maintains an up-to-date list of known
40+
* schematics in order to speed up queries.
41+
*
42+
* <p>If initialization of the file-watching backend fails, a polling backend is used instead.
43+
*/
44+
public class SchematicsManager {
45+
46+
private static final Logger LOGGER = LogManagerCompat.getLogger();
47+
private final WorldEdit worldEdit;
48+
49+
private Path schematicsDir;
50+
private SchematicsBackend backend;
51+
52+
public SchematicsManager(WorldEdit worldEdit) {
53+
this.worldEdit = worldEdit;
54+
}
55+
56+
/**
57+
* Initialize this SchematicsManager.
58+
* This sets everything up, and initially scans the schematics folder.
59+
*/
60+
public void init() {
61+
try {
62+
Path schematicsDirByConfig = worldEdit.getWorkingDirectoryPath(worldEdit.getConfiguration().saveDir);
63+
Files.createDirectories(schematicsDirByConfig);
64+
schematicsDir = schematicsDirByConfig.toRealPath();
65+
66+
try {
67+
backend = FileWatcherSchematicsBackend.create(schematicsDir);
68+
} catch (IOException e) {
69+
LOGGER.warn("Failed to initialize file-monitoring based schematics backend. Falling back to polling.", e);
70+
backend = PollingSchematicsBackend.create(schematicsDir);
71+
}
72+
} catch (IOException e) {
73+
throw new RuntimeException("Failed to create schematics directory", e);
74+
}
75+
backend.init();
76+
}
77+
78+
/**
79+
* Uninitialize this SchematicsManager.
80+
*/
81+
public void uninit() {
82+
if (backend != null) {
83+
backend.uninit();
84+
backend = null;
85+
}
86+
}
87+
88+
/**
89+
* Gets the root folder in which the schematics are stored.
90+
*
91+
* @return the root folder where schematics are stored
92+
*/
93+
public Path getRoot() {
94+
checkNotNull(schematicsDir, "not initialized");
95+
return schematicsDir;
96+
}
97+
98+
/**
99+
* Gets a set of known schematics.
100+
*
101+
* @return a set of all known schematics
102+
*/
103+
public Set<Path> getSchematicPaths() {
104+
checkNotNull(backend);
105+
return backend.getPaths();
106+
}
107+
108+
/**
109+
* Force an update of the list.
110+
*/
111+
public void update() {
112+
backend.update();
113+
}
114+
115+
}

0 commit comments

Comments
 (0)