Skip to content

Commit

Permalink
Rename Schematic to SchematicPath and cleanup
Browse files Browse the repository at this point in the history
- Resolve first batch of review comments
  - Renamed Schematic to SchematicPath
  - Removed custom hashCode() and equals() implementations
  - Added unit-test to make sure it behaves as expected
  • Loading branch information
seijikun committed Nov 8, 2022
1 parent a0bdd4b commit 40c045e
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
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.Schematic;
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;
Expand All @@ -73,10 +73,7 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.concurrent.Callable;
Expand Down Expand Up @@ -111,13 +108,13 @@ public SchematicCommands(WorldEdit worldEdit) {
@CommandPermissions({"worldedit.clipboard.load", "worldedit.schematic.load"})
public void load(Actor actor, LocalSession session,
@Arg(desc = "File name.")
Schematic schematic,
SchematicPath 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.getPath().toString();
String filename = schematic.path().toString();
File schematicsRoot = worldEdit.getSchematicsManager().getRoot().toFile();
File f = worldEdit.getSafeOpenFile(actor, schematicsRoot, filename,
BuiltInClipboardFormat.SPONGE_V3_SCHEMATIC.getPrimaryFileExtension(),
Expand Down Expand Up @@ -251,12 +248,12 @@ public void share(Actor actor, LocalSession session,
@CommandPermissions("worldedit.schematic.delete")
public void delete(Actor actor,
@Arg(desc = "File name.")
Schematic schematic) throws WorldEditException {
SchematicPath 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.getPath().toString();
String filename = schematic.path().toString();
File f = worldEdit.getSafeOpenFile(actor,
dir, filename, "schematic", ClipboardFormats.getFileExtensionArray());

Expand Down Expand Up @@ -335,7 +332,7 @@ public void list(Actor actor,
final String pageCommand = actor.isPlayer()
? "//schem list -p %page%" + flag : null;

Comparator<Schematic> schematicComparator = (s0, s1) -> pathComparator.compare(s0.getPath(), s1.getPath());
Comparator<SchematicPath> schematicComparator = (s0, s1) -> pathComparator.compare(s0.path(), s1.path());

WorldEditAsyncCommandBuilder.createAndSendMessage(actor,
new SchematicListTask(schematicComparator, page, pageCommand),
Expand Down Expand Up @@ -445,11 +442,11 @@ public Consumer<Actor> call() throws Exception {
}

private static class SchematicListTask implements Callable<Component> {
private final Comparator<Schematic> pathComparator;
private final Comparator<SchematicPath> pathComparator;
private final int page;
private final String pageCommand;

SchematicListTask(Comparator<Schematic> pathComparator, int page, String pageCommand) {
SchematicListTask(Comparator<SchematicPath> pathComparator, int page, String pageCommand) {
this.pathComparator = pathComparator;
this.page = page;
this.pageCommand = pageCommand;
Expand All @@ -458,7 +455,7 @@ private static class SchematicListTask implements Callable<Component> {
@Override
public Component call() throws Exception {
SchematicsManager schematicsManager = WorldEdit.getInstance().getSchematicsManager();
List<Schematic> fileList = schematicsManager.getList();
List<SchematicPath> fileList = schematicsManager.getList();

if (fileList.isEmpty()) {
return ErrorFormat.wrap("No schematics found.");
Expand All @@ -473,9 +470,9 @@ public Component call() throws Exception {

private static class SchematicPaginationBox extends PaginationBox {
private final Path rootDir;
private final List<Schematic> files;
private final List<SchematicPath> files;

SchematicPaginationBox(Path rootDir, List<Schematic> files, String pageCommand) {
SchematicPaginationBox(Path rootDir, List<SchematicPath> files, String pageCommand) {
super("Available schematics", pageCommand);
this.rootDir = rootDir;
this.files = files;
Expand All @@ -484,7 +481,7 @@ private static class SchematicPaginationBox extends PaginationBox {
@Override
public Component getComponent(int number) {
checkArgument(number < files.size() && number >= 0);
Path file = files.get(number).getPath();
Path file = files.get(number).path();

String format = ClipboardFormats.getFileExtensionMap()
.get(MoreFiles.getFileExtension(file))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@
package com.sk89q.worldedit.command.argument;

import com.sk89q.worldedit.WorldEdit;
import com.sk89q.worldedit.internal.util.LogManagerCompat;
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.Schematic;
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;
Expand All @@ -40,10 +39,10 @@

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

public class SchematicConverter implements ArgumentConverter<Schematic> {
public class SchematicConverter implements ArgumentConverter<SchematicPath> {

public static void register(WorldEdit worldEdit, CommandManager commandManager) {
commandManager.registerConverter(Key.of(Schematic.class), new SchematicConverter(worldEdit));
commandManager.registerConverter(Key.of(SchematicPath.class), new SchematicConverter(worldEdit));
}

private final WorldEdit worldEdit;
Expand All @@ -65,11 +64,11 @@ public List<String> getSuggestions(String input, InjectedValueAccess context) {
Path schematicsRootPath = schematicsManager.getRoot();

return limitByPrefix(schematicsManager.getList().stream()
.map(s -> schematicsRootPath.relativize(s.getPath()).toString()), input);
.map(s -> schematicsRootPath.relativize(s.path()).toString()), input);
}

@Override
public ConversionResult<Schematic> convert(String s, InjectedValueAccess injectedValueAccess) {
public ConversionResult<SchematicPath> convert(String s, InjectedValueAccess injectedValueAccess) {
Path schematicsRoot = worldEdit.getSchematicsManager().getRoot();
// resolve as subpath of schematicsRoot
Path schematicPath = schematicsRoot.resolve(s).toAbsolutePath();
Expand All @@ -81,7 +80,7 @@ public ConversionResult<Schematic> convert(String s, InjectedValueAccess injecte
if (Files.exists(schematicPath)) {
// continue as relative path to schematicsRoot
schematicPath = schematicsRoot.relativize(schematicPath);
return SuccessfulConversion.fromSingle(new Schematic(schematicPath));
return SuccessfulConversion.fromSingle(new SchematicPath(schematicPath));
} else {
return FailedConversion.from(new FilenameException(s));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,7 @@
/**
* Record representing one Schematic file.
*/
public record Schematic(Path path) {

/**
* Get this Schematic's path.
* @return This Schematic's path.
*/
public Path getPath() {
return path;
}
public record SchematicPath(Path path) {

@Override
public String toString() {
Expand All @@ -46,8 +38,8 @@ public int hashCode() {

@Override
public boolean equals(Object obj) {
Schematic other = (Schematic) obj;
SchematicPath other = (SchematicPath) obj;
if (other == null) { return false; }
return path.equals(other.getPath());
return path.equals(other.path());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public Path getRoot() {
* Get a list of all known schematics.
* @return List of all known schematics.
*/
public List<Schematic> getList() {
public List<SchematicPath> getList() {
return backend.getList();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@

package com.sk89q.worldedit.util.schematic.backends;

import com.sk89q.worldedit.util.schematic.Schematic;
import com.sk89q.worldedit.util.schematic.SchematicPath;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -39,7 +38,7 @@ public void uninit() {
}

@Override
public List<Schematic> getList() {
public List<SchematicPath> getList() {
return new ArrayList<>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import com.sk89q.worldedit.internal.util.LogManagerCompat;
import com.sk89q.worldedit.util.io.file.RecursiveDirectoryWatcher;
import com.sk89q.worldedit.util.schematic.Schematic;
import com.sk89q.worldedit.util.schematic.SchematicPath;
import org.apache.logging.log4j.Logger;

import java.io.IOException;
Expand All @@ -41,7 +41,7 @@ public class FileWatcherSchematicsBackend implements SchematicsBackend {
private static final Logger LOGGER = LogManagerCompat.getLogger();

private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
private final Set<Schematic> schematics = new HashSet<>();
private final Set<SchematicPath> schematics = new HashSet<>();
private final RecursiveDirectoryWatcher directoryWatcher;

private FileWatcherSchematicsBackend(RecursiveDirectoryWatcher directoryWatcher) {
Expand All @@ -64,9 +64,9 @@ public void init() {
directoryWatcher.start(event -> {
lock.writeLock().lock();
if (event instanceof RecursiveDirectoryWatcher.FileCreatedEvent) {
schematics.add(new Schematic(event.getPath()));
schematics.add(new SchematicPath(event.getPath()));
} else if (event instanceof RecursiveDirectoryWatcher.FileDeletedEvent) {
schematics.remove(new Schematic(event.getPath()));
schematics.remove(new SchematicPath(event.getPath()));
}
lock.writeLock().unlock();
if (event instanceof RecursiveDirectoryWatcher.FileCreatedEvent) {
Expand All @@ -84,9 +84,9 @@ public void uninit() {
}

@Override
public List<Schematic> getList() {
public List<SchematicPath> getList() {
lock.readLock().lock();
List<Schematic> result = new ArrayList<>(schematics);
List<SchematicPath> result = new ArrayList<>(schematics);
lock.readLock().unlock();
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
package com.sk89q.worldedit.util.schematic.backends;

import com.sk89q.worldedit.internal.util.LogManagerCompat;
import com.sk89q.worldedit.util.schematic.Schematic;
import com.sk89q.worldedit.util.schematic.SchematicPath;
import org.apache.logging.log4j.Logger;

import java.io.IOException;
Expand All @@ -46,7 +46,7 @@ public class PollingSchematicsBackend implements SchematicsBackend {
private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
private final Path schematicsDir;
private Instant lastUpdateTs = Instant.EPOCH;
private List<Schematic> schematics = new ArrayList<>();
private List<SchematicPath> schematics = new ArrayList<>();

private PollingSchematicsBackend(Path schematicsDir) {
this.schematicsDir = schematicsDir;
Expand All @@ -61,14 +61,14 @@ public static PollingSchematicsBackend create(Path schematicsFolder) {
return new PollingSchematicsBackend(schematicsFolder);
}

private List<Schematic> scanFolder(Path root) {
List<Schematic> pathList = new ArrayList<>();
private List<SchematicPath> scanFolder(Path root) {
List<SchematicPath> pathList = new ArrayList<>();
try (DirectoryStream<Path> stream = Files.newDirectoryStream(root)) {
for (Path path : stream) {
if (Files.isDirectory(path)) {
pathList.addAll(scanFolder(path));
} else {
pathList.add(new Schematic(path));
pathList.add(new SchematicPath(path));
}
}
} catch (IOException e) {
Expand All @@ -94,7 +94,7 @@ public void uninit() {
}

@Override
public synchronized List<Schematic> getList() {
public synchronized List<SchematicPath> getList() {
// udpate internal cache if requried (determined by age)
Duration age = Duration.between(lastUpdateTs, Instant.now());
if (age.compareTo(MAX_RESULT_AGE) >= 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

package com.sk89q.worldedit.util.schematic.backends;

import com.sk89q.worldedit.util.schematic.Schematic;
import com.sk89q.worldedit.util.schematic.SchematicPath;

import java.util.List;

Expand All @@ -43,7 +43,7 @@ public interface SchematicsBackend {
*
* @return List of known schematics.
*/
List<Schematic> getList();
List<SchematicPath> getList();

/**
* Tells the backend that there are changes it should take into account.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* 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.util.schematic;

import org.junit.jupiter.api.Test;

import java.nio.file.Path;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* Tests {@link SchematicPath}.
*
* @see <a href="https://github.com/EngineHub/WorldEdit/pull/2212#discussion_r1016580671">
* Trusting me4502 is good, controlling is better! :)
* </a>
*/
public class SchematicPathTest {

@Test
public void testHashAndEquality() {
Path p0 = Path.of("/tmp/testpath0");
Path p1 = Path.of("/tmp/testpath1");
Path p0equiv = Path.of("/tmp/testpath0");

SchematicPath s0 = new SchematicPath(p0);
SchematicPath s1 = new SchematicPath(p1);
SchematicPath s0equiv = new SchematicPath(p0equiv);

assertEquals(s0.hashCode(), s0equiv.hashCode());
assertNotEquals(s0.hashCode(), s1.hashCode());

assertTrue(s0.equals(s0equiv));
assertFalse(s0.equals(s1));
}

}

0 comments on commit 40c045e

Please sign in to comment.