Skip to content

Commit 218ceaa

Browse files
noeppi-noeppinoeppi-noeppi
authored
Fix UnionPath (#4)
* Fix UnionPath#relativize * Fix the other of the relativize method as wel land add test for reletivizing absolute union paths. * Went through UnionPath and change methods to behave as they should. It is pasing test now however better testing for the UnionPath methods is required. * Some more union path bugfixes and added tests for many other methods of UnionPath * Little changes to Jar to make it work with the changes and made UnionPath#getFileName return an empty string for the empty absolute path.
1 parent c8a03e2 commit 218ceaa

5 files changed

Lines changed: 398 additions & 45 deletions

File tree

‎src/main/java/cpw/mods/jarhandling/impl/Jar.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,11 @@ public Set<String> getPackages() {
208208
if (this.packages == null) {
209209
try (var walk = Files.walk(this.filesystem.getRoot())) {
210210
this.packages = walk
211+
.filter(path->path.getNameCount()>0)
211212
.filter(path->!path.getName(0).toString().equals("META-INF"))
212213
.filter(path->path.getFileName().toString().endsWith(".class"))
213214
.filter(Files::isRegularFile)
214-
.map(path->path.subpath(0, path.getNameCount()-1))
215+
.map(Path::getParent)
215216
.map(path->path.toString().replace('/','.'))
216217
.filter(pkg->pkg.length()!=0)
217218
.collect(toSet());
@@ -248,7 +249,7 @@ public Path getPath(String first, String... rest) {
248249

249250
@Override
250251
public Path getRootPath() {
251-
return filesystem.getRoot();
252+
return filesystem.getPath("");
252253
}
253254

254255
@Override

‎src/main/java/cpw/mods/niofs/union/UnionFileSystem.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
import java.util.stream.StreamSupport;
1717

1818
public class UnionFileSystem extends FileSystem {
19-
private final UnionPath root = new UnionPath(this, false, UnionPath.ROOT);
20-
private final UnionPath notExistingPath = new UnionPath(this, false, "SNOWMAN");
19+
private final UnionPath root = new UnionPath(this, "/");
20+
private final UnionPath notExistingPath = new UnionPath(this, "/SNOWMAN");
2121
private final UnionFileSystemProvider provider;
2222
private final String key;
2323
private final List<Path> basepaths;
@@ -110,9 +110,9 @@ public Path getPath(final String first, final String... more) {
110110
var args = new String[more.length + 1];
111111
args[0] = first;
112112
System.arraycopy(more, 0, args, 1, more.length);
113-
return new UnionPath(this, false, args);
113+
return new UnionPath(this, args);
114114
}
115-
return new UnionPath(this, false, first);
115+
return new UnionPath(this, first);
116116
}
117117

118118
@Override
@@ -203,8 +203,8 @@ public void checkAccess(final UnionPath p, final AccessMode... modes) throws IOE
203203
}
204204

205205
private Path toRealPath(final Path basePath, final UnionPath path) {
206-
var embeddedpath = path.toString();
207-
var resolvepath = embeddedpath.length() > 1 && path.isAbsolute() ? embeddedpath.substring(1) : embeddedpath;
206+
var embeddedpath = path.isAbsolute() ? this.root.relativize(path) : path;
207+
var resolvepath = embeddedpath.normalize().toString();
208208
var efsm = embeddedFileSystems.get(basePath);
209209
if (efsm != null) {
210210
return efsm.fs().getPath(resolvepath);
@@ -272,7 +272,11 @@ private boolean testFilter(final Path path, final Path basePath) {
272272
sPath = basePath.relativize(path).toString().replace('\\', '/');
273273
if (Files.isDirectory(path))
274274
sPath += '/';
275+
if (sPath.length() > 1 && sPath.startsWith("/"))
276+
sPath = sPath.substring(1);
275277
String sBasePath = basePath.toString().replace('\\', '/');
278+
if (sBasePath.length() > 1 && sBasePath.startsWith("/"))
279+
sBasePath = sBasePath.substring(1);
276280
return pathFilter.test(sPath, sBasePath);
277281
}
278282
}

‎src/main/java/cpw/mods/niofs/union/UnionPath.java‎

Lines changed: 111 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,58 @@
66
import java.nio.file.*;
77
import java.util.*;
88
import java.util.function.IntBinaryOperator;
9+
import java.util.regex.Pattern;
910
import java.util.stream.Collectors;
1011
import java.util.stream.IntStream;
1112

1213
public class UnionPath implements Path {
1314
private final UnionFileSystem fileSystem;
15+
private final boolean absolute;
1416
private final String[] pathParts;
15-
static final String ROOT = "";
16-
17-
UnionPath(final UnionFileSystem fileSystem, boolean knownCorrectSplit, final String... pathParts) {
17+
18+
// Store the normalized path after it has been created first
19+
private UnionPath normalized;
20+
21+
UnionPath(final UnionFileSystem fileSystem, final String... pathParts) {
1822
this.fileSystem = fileSystem;
19-
if (pathParts.length == 0)
23+
if (pathParts.length == 0) {
24+
this.absolute = false;
2025
this.pathParts = new String[0];
21-
else if (knownCorrectSplit)
22-
this.pathParts = pathParts;
23-
else {
24-
final var longstring = String.join(fileSystem.getSeparator(), pathParts);
26+
} else {
27+
final var longstring = Arrays.stream(pathParts).filter(part -> !part.isEmpty()).collect(Collectors.joining(this.getFileSystem().getSeparator()));
28+
this.absolute = longstring.startsWith(this.getFileSystem().getSeparator());
2529
this.pathParts = getPathParts(longstring);
2630
}
31+
this.normalized = null;
32+
}
33+
34+
// Private constructor only for known correct split and extra value for absolute
35+
private UnionPath(final UnionFileSystem fileSystem, boolean absolute, final String... pathParts) {
36+
this(fileSystem, absolute, false, pathParts);
37+
}
38+
39+
private UnionPath(final UnionFileSystem fileSystem, boolean absolute, boolean isNormalized, final String... pathParts) {
40+
this.fileSystem = fileSystem;
41+
this.absolute = absolute;
42+
this.pathParts = pathParts;
43+
if (isNormalized)
44+
this.normalized = this;
45+
else
46+
this.normalized = null;
2747
}
2848

2949
private String[] getPathParts(final String longstring) {
30-
return longstring.replace("\\", this.getFileSystem().getSeparator()).split(this.getFileSystem().getSeparator());
50+
var sep = "(?:" + Pattern.quote(this.getFileSystem().getSeparator()) + ")";
51+
String pathname = longstring
52+
.replace("\\", this.getFileSystem().getSeparator())
53+
// remove separators from start and end of longstring
54+
.replaceAll("^" + sep + "*|" + sep + "*$", "")
55+
// Remove duplicate separators
56+
.replaceAll(sep + "+(?=" + sep + ")", "");
57+
if (pathname.isEmpty())
58+
return new String[0];
59+
else
60+
return pathname.split(this.getFileSystem().getSeparator());
3161
}
3262

3363
@Override
@@ -37,25 +67,34 @@ public UnionFileSystem getFileSystem() {
3767

3868
@Override
3969
public boolean isAbsolute() {
40-
return ROOT.equals(this.pathParts[0]);
70+
return this.absolute;
4171
}
4272

4373
@Override
4474
public Path getRoot() {
45-
return new UnionPath(this.fileSystem, true, ROOT);
75+
// Found nothing in the docs that say a non-absolute path can't have a root
76+
// although this is uncommon. However, other stuff relies on it so leave it
77+
//if (!this.absolute)
78+
// return null;
79+
return this.fileSystem.getRoot();
4680
}
47-
48-
81+
4982
@Override
5083
public Path getFileName() {
51-
return this.pathParts.length > 0 ? new UnionPath(this.getFileSystem(), true, this.pathParts[this.pathParts.length-1]) : null;
84+
if (this.pathParts.length > 0) {
85+
return new UnionPath(this.getFileSystem(), false, this.pathParts[this.pathParts.length - 1]);
86+
} else {
87+
// normally would be null for the empty absolute path and empty string for the empty relative
88+
// path. But again, very much stuff relies on it and there's no current directory for union
89+
// paths, so it does not really matter.
90+
return new UnionPath(this.fileSystem, false);
91+
}
5292
}
5393

54-
5594
@Override
5695
public Path getParent() {
5796
if (this.pathParts.length > 0) {
58-
return new UnionPath(this.fileSystem, true, Arrays.copyOf(this.pathParts,this.pathParts.length - 1));
97+
return new UnionPath(this.fileSystem, this.absolute, Arrays.copyOf(this.pathParts,this.pathParts.length - 1));
5998
} else {
6099
return null;
61100
}
@@ -69,15 +108,20 @@ public int getNameCount() {
69108
@Override
70109
public Path getName(final int index) {
71110
if (index < 0 || index > this.pathParts.length -1) throw new IllegalArgumentException();
72-
return new UnionPath(this.fileSystem, true, this.pathParts[index]);
111+
return new UnionPath(this.fileSystem, false, this.pathParts[index]);
73112
}
74113

75114
@Override
76-
public Path subpath(final int beginIndex, final int endIndex) {
77-
if (beginIndex < 0 || beginIndex > this.pathParts.length - 1 || endIndex < 0 || endIndex > this.pathParts.length || beginIndex > endIndex) {
115+
public UnionPath subpath(final int beginIndex, final int endIndex) {
116+
if (!this.absolute && this.pathParts.length == 0 && beginIndex == 0 && endIndex == 1)
117+
return new UnionPath(this.fileSystem, false);
118+
if (beginIndex < 0 || beginIndex > this.pathParts.length - 1 || endIndex < 0 || endIndex > this.pathParts.length || beginIndex >= endIndex) {
78119
throw new IllegalArgumentException("Out of range "+beginIndex+" to "+endIndex+" for length "+this.pathParts.length);
79120
}
80-
return new UnionPath(this.fileSystem, true, Arrays.copyOfRange(this.pathParts, beginIndex, endIndex));
121+
if (!this.absolute && beginIndex == 0 && endIndex == this.pathParts.length) {
122+
return this;
123+
}
124+
return new UnionPath(this.fileSystem, false, Arrays.copyOfRange(this.pathParts, beginIndex, endIndex));
81125
}
82126

83127
@Override
@@ -86,6 +130,8 @@ public boolean startsWith(final Path other) {
86130
return false;
87131
}
88132
if (other instanceof UnionPath bp) {
133+
if (this.absolute != bp.absolute)
134+
return false;
89135
return checkArraysMatch(this.pathParts, bp.pathParts, false);
90136
}
91137
return false;
@@ -98,6 +144,8 @@ public boolean endsWith(final Path other) {
98144
return false;
99145
}
100146
if (other instanceof UnionPath bp) {
147+
if (!this.absolute && bp.absolute)
148+
return false;
101149
return checkArraysMatch(this.pathParts, bp.pathParts, true);
102150
}
103151
return false;
@@ -115,20 +163,28 @@ private static boolean checkArraysMatch(String[] array1, String[] array2, boolea
115163

116164
@Override
117165
public Path normalize() {
166+
if (normalized != null)
167+
return normalized;
118168
Deque<String> normpath = new ArrayDeque<>();
119169
for (String pathPart : this.pathParts) {
120170
switch (pathPart) {
121171
case ".":
122172
break;
123173
case "..":
124-
normpath.removeLast();
174+
if (normpath.isEmpty() || normpath.getLast().equals("..")) {
175+
// .. on an empty path is allowed, so keep it
176+
normpath.addLast(pathPart);
177+
} else {
178+
normpath.removeLast();
179+
}
125180
break;
126181
default:
127182
normpath.addLast(pathPart);
128183
break;
129184
}
130185
}
131-
return new UnionPath(this.fileSystem, true, normpath.toArray(new String[0]));
186+
normalized = new UnionPath(this.fileSystem, this.absolute, true, normpath.toArray(new String[0]));
187+
return normalized;
132188
}
133189

134190
@Override
@@ -137,7 +193,10 @@ public Path resolve(final Path other) {
137193
if (path.isAbsolute()) {
138194
return path;
139195
}
140-
return new UnionPath(this.fileSystem, false, this+fileSystem.getSeparator()+other);
196+
String[] mergedParts = new String[this.pathParts.length + path.pathParts.length];
197+
System.arraycopy(this.pathParts, 0, mergedParts, 0, this.pathParts.length);
198+
System.arraycopy(path.pathParts, 0, mergedParts, this.pathParts.length, path.pathParts.length);
199+
return new UnionPath(this.fileSystem, this.absolute, mergedParts);
141200
}
142201
return other;
143202
}
@@ -146,27 +205,36 @@ public Path resolve(final Path other) {
146205
public Path relativize(final Path other) {
147206
if (other.getFileSystem()!=this.getFileSystem()) throw new IllegalArgumentException("Wrong filesystem");
148207
if (other instanceof UnionPath p) {
149-
var poff = p.isAbsolute() ? 1 : 0;
150-
var meoff = this.isAbsolute() ? 1 : 0;
151-
var length = Math.min(this.pathParts.length - meoff, p.pathParts.length - poff);
208+
if (this.absolute != p.absolute) {
209+
// Should not be allowed but union fs relies on it
210+
// also there is no such concept of a current directory for union paths
211+
// meaning absolute and relative paths should have the same effect,
212+
// so we just allow this.
213+
//throw new IllegalArgumentException("Different types of path");
214+
}
215+
var length = Math.min(this.pathParts.length, p.pathParts.length);
152216
int i = 0;
153217
while (i < length) {
154-
if (!Objects.equals(this.pathParts[i + meoff], p.pathParts[i + poff]))
218+
if (!Objects.equals(this.pathParts[i], p.pathParts[i]))
155219
break;
156220
i++;
157221
}
158222

159-
var remaining = this.pathParts.length - i - meoff;
223+
var remaining = this.pathParts.length - i;
160224
if (remaining == 0 && i == p.pathParts.length) {
161225
return new UnionPath(this.getFileSystem(), false);
162226
} else if (remaining == 0) {
163227
return p.subpath(i, p.getNameCount());
164228
} else {
165-
var updots = IntStream.range(0, remaining).mapToObj(idx -> "..").collect(Collectors.joining(getFileSystem().getSeparator()));
229+
var updots = IntStream.range(0, remaining).mapToObj(idx -> "..").toArray(String[]::new);
166230
if (i == p.pathParts.length) {
167231
return new UnionPath(this.getFileSystem(), false, updots);
168232
} else {
169-
return new UnionPath(this.getFileSystem(), false, updots + getFileSystem().getSeparator() + p.subpath(i, p.getNameCount()));
233+
var subpath = p.subpath(i, p.getNameCount());
234+
String[] mergedParts = new String[updots.length + subpath.pathParts.length];
235+
System.arraycopy(updots, 0, mergedParts, 0, updots.length);
236+
System.arraycopy(subpath.pathParts, 0, mergedParts, updots.length, subpath.pathParts.length);
237+
return new UnionPath(this.getFileSystem(), false, mergedParts);
170238
}
171239
}
172240
}
@@ -197,7 +265,7 @@ public Path toAbsolutePath() {
197265

198266
@Override
199267
public Path toRealPath(final LinkOption... options) throws IOException {
200-
return null;
268+
return this.toAbsolutePath().normalize();
201269
}
202270

203271
@Override
@@ -207,13 +275,22 @@ public WatchKey register(final WatchService watcher, final WatchEvent.Kind<?>[]
207275

208276
@Override
209277
public int compareTo(final Path other) {
210-
return 0;
278+
if (other instanceof UnionPath path) {
279+
if (this.absolute && !path.absolute)
280+
return 1;
281+
else if (!this.absolute && path.absolute)
282+
return -1;
283+
else
284+
return Arrays.compare(this.pathParts, path.pathParts);
285+
} else {
286+
return 0;
287+
}
211288
}
212289

213290
@Override
214291
public boolean equals(final Object o) {
215292
if (o instanceof UnionPath p) {
216-
return p.getFileSystem() == this.getFileSystem() && Arrays.equals(this.pathParts, p.pathParts);
293+
return p.getFileSystem() == this.getFileSystem() && this.absolute == p.absolute && Arrays.equals(this.pathParts, p.pathParts);
217294
}
218295
return false;
219296
}
@@ -225,6 +302,6 @@ public int hashCode() {
225302

226303
@Override
227304
public String toString() {
228-
return String.join(fileSystem.getSeparator(), this.pathParts);
305+
return (this.absolute ? fileSystem.getSeparator() : "") + String.join(fileSystem.getSeparator(), this.pathParts);
229306
}
230307
}

‎src/test/java/cpw/mods/niofs/union/TestUnionFS.java‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,12 @@ void testUnionFileSystem() throws IOException {
3030
()->assertEquals(Files.readString(masktest), "dir2")
3131
);
3232
assertAll(
33-
Files.walk(masktest.getRoot())
33+
Files.walk(masktest.toAbsolutePath().getRoot())
3434
.map(Files::exists)
3535
.map(f->()->assertTrue(f))
3636
);
3737
var p = ufs.getRoot().resolve("subdir1/masktestd1.txt");
38-
p.subpath(2, 3);
39-
var empty = new UnionPath(ufs, false);
38+
p.subpath(1, 2);
4039
}
4140

4241
@Test
@@ -64,6 +63,32 @@ void testRelativize() {
6463
()->assertEquals(0, p13.relativize(p13plus).getNameCount())
6564
);
6665
}
66+
67+
@Test
68+
void testRelativizeAbsolute() {
69+
final var dir1 = Paths.get("src", "test", "resources", "dir1").toAbsolutePath().normalize();
70+
final var dir2 = Paths.get("src", "test", "resources", "dir2").toAbsolutePath().normalize();
71+
72+
var fsp = (UnionFileSystemProvider)FileSystemProvider.installedProviders().stream().filter(fs-> fs.getScheme().equals("union")).findFirst().orElseThrow();
73+
var ufs = fsp.newFileSystem((path, base) -> true, dir1, dir2);
74+
var p1 = ufs.getPath("/path1");
75+
var p123 = ufs.getPath("/path1/path2/path3");
76+
var p11 = ufs.getPath("/path1/path1");
77+
var p12 = ufs.getPath("/path1/path2");
78+
var p13 = ufs.getPath("/path1/path3");
79+
var p23 = ufs.getPath("/path2/path3");
80+
var p13plus = ufs.getPath("/path1/path3");
81+
assertAll(
82+
()->assertEquals("path2/path3", p1.relativize(p123).toString()),
83+
()->assertEquals("../..", p123.relativize(p1).toString()),
84+
()->assertEquals("path1", p1.relativize(p11).toString()),
85+
()->assertEquals("path2", p1.relativize(p12).toString()),
86+
()->assertEquals("path3", p1.relativize(p13).toString()),
87+
()->assertEquals("../../path1/path1", p23.relativize(p11).toString()),
88+
()->assertEquals("../../path1", p123.relativize(p11).toString()),
89+
()->assertEquals(0, p13.relativize(p13plus).getNameCount())
90+
);
91+
}
6792

6893
@Test
6994
void testPathFiltering() {

0 commit comments

Comments
 (0)