From 1c3497b238ebd83fd0790675c2a79beea8f1d4e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20G=C3=B3mez-S=C3=A1nchez?= <7352559+magicDGS@users.noreply.github.com> Date: Fri, 31 Aug 2018 14:28:40 +0200 Subject: [PATCH] Implement relative path --- .../magicdgs/http/jsr203/HttpFileSystem.java | 2 +- .../org/magicdgs/http/jsr203/HttpPath.java | 121 +++++++++-- .../http/jsr203/HttpPathUnitTest.java | 194 ++++++++++++++++-- 3 files changed, 282 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/magicdgs/http/jsr203/HttpFileSystem.java b/src/main/java/org/magicdgs/http/jsr203/HttpFileSystem.java index 2f7f475..d7d03fd 100644 --- a/src/main/java/org/magicdgs/http/jsr203/HttpFileSystem.java +++ b/src/main/java/org/magicdgs/http/jsr203/HttpFileSystem.java @@ -120,7 +120,7 @@ public HttpPath getPath(final String first, final String... more) { + String.join(getSeparator(), Utils.nonNull(more, () -> "null more")); if (!path.isEmpty() && !path.startsWith(getSeparator())) { - throw new InvalidPathException(path, "Relative paths are not supported", 0); + throw new InvalidPathException(path, "Cannot construct a relative http/s path", 0); } try { diff --git a/src/main/java/org/magicdgs/http/jsr203/HttpPath.java b/src/main/java/org/magicdgs/http/jsr203/HttpPath.java index b2b921e..20c9607 100644 --- a/src/main/java/org/magicdgs/http/jsr203/HttpPath.java +++ b/src/main/java/org/magicdgs/http/jsr203/HttpPath.java @@ -12,9 +12,11 @@ import java.nio.file.WatchEvent; import java.nio.file.WatchKey; import java.nio.file.WatchService; +import java.util.Arrays; import java.util.Comparator; import java.util.Iterator; import java.util.Objects; +import java.util.stream.IntStream; /** * {@link Path} for HTTP/S. @@ -59,6 +61,9 @@ final class HttpPath implements Path { // reference for the URL (may be null) / fragment for the URI representation private final String reference; + // true if the paht is absolute; false otherwise + private final boolean absolute; + /** * Internal constructor. * @@ -71,6 +76,7 @@ final class HttpPath implements Path { */ private HttpPath(final HttpFileSystem fs, final String query, final String reference, + final boolean absolute, final byte... normalizedPath) { this.fs = fs; @@ -78,6 +84,9 @@ private HttpPath(final HttpFileSystem fs, this.query = query; this.reference = reference; + // set the absolute status + this.absolute = absolute; + // normalized path bytes (shouldn't be null) this.normalizedPath = normalizedPath; } @@ -85,13 +94,15 @@ private HttpPath(final HttpFileSystem fs, /** * Creates a new Path in the provided {@link HttpFileSystem}, with optional query and reference. * - * @param fs file system representing the base URL (scheme and authority). - * @param path path component for the URL (required). - * @param query query component for the URL (optional). + * @param fs file system representing the base URL (scheme and authority). + * @param path path (absolute) component for the URL (required). + * @param query query component for the URL (optional). * @param reference reference component for the URL (optional). */ - HttpPath(final HttpFileSystem fs, final String path, final String query, final String reference) { - this(Utils.nonNull(fs, () -> "null fs"), query, reference, + HttpPath(final HttpFileSystem fs, final String path, final String query, + final String reference) { + // always absolute and checking it when converting to byte[] + this(Utils.nonNull(fs, () -> "null fs"), query, reference, true, getNormalizedPathBytes(Utils.nonNull(path, () -> "null path"), true)); } @@ -102,24 +113,45 @@ public HttpFileSystem getFileSystem() { @Override public boolean isAbsolute() { - // TODO - change when we support relative Paths (https://github.com/magicDGS/jsr203-http/issues/12) - return true; + return absolute; } @Override public Path getRoot() { - // root is a Path with only the byte array - return new HttpPath(fs, null, null); + // root is a Path with only the byte array (always absolute) + return new HttpPath(fs, null, null, true); } + /** + * {@inheritDoc} + * + * @implNote returns always a relative path. + */ @Override public Path getFileName() { - throw new UnsupportedOperationException("Not implemented"); + initOffsets(); + // following the contract, for the getNameCounts() == 0 (root) we return null + if (offsets.length == 0) { + return null; + } + // file names are always relative paths + return subpath(offsets.length - 1, offsets.length, false); } + /** + * {@inheritDoc} + * + * @implNote returned path keeps the {@link #isAbsolute()} status of the current path. + */ @Override public Path getParent() { - throw new UnsupportedOperationException("Not implemented"); + initOffsets(); + // returns the root if there is no + if (offsets.length == 0) { + return getRoot(); + } + // parent names are absolute/relative depending on the current status + return subpath(0, offsets.length - 1, absolute); } @Override @@ -128,14 +160,59 @@ public int getNameCount() { return offsets.length; } + /** + * {@inheritDoc} + * + * @implNote returns always a relative path. + */ @Override public Path getName(final int index) { - throw new UnsupportedOperationException("Not implemented"); + initOffsets(); + // returns always a relative path + return subpath(index, index + 1, false); } @Override public Path subpath(final int beginIndex, final int endIndex) { - throw new UnsupportedOperationException("Not implemented"); + initOffsets(); + // following the contract for invalid indexes + if (beginIndex < 0 || beginIndex >= offsets.length || + endIndex <= beginIndex || endIndex > offsets.length) { + throw new IllegalArgumentException(String + .format("Invalid indexes for path with %s name(s): [%s, %s]", + getNameCount(), beginIndex, endIndex)); + } + // return the new path (always relative path following the contract) + return subpath(beginIndex, endIndex, false); + } + + /** + * Helper method to implement different subpath routines with different absolute/relative + * status. + * + *

The contract of this method is the same as {@link Path#subpath(int, int)}). + * + * @param beginIndex the index of the first element, inclusive + * @param endIndex the index of the last element, exclusive + * @param absolute {@code true} if the returned path is absolute; {@code false} otherwise. + * + * @return a new path object that is a subsequence of the nams elements in this {@code + * HttpPath}. + * + * @implNote assumes that the caller already initialized the offsets and that the indexes are + * correct. + */ + private HttpPath subpath(final int beginIndex, final int endIndex, final boolean absolute) { + // get the coordinates to copy the path array + final int begin = offsets[beginIndex]; + final int end = (endIndex == offsets.length) ? normalizedPath.length : offsets[endIndex]; + + // construct the result + final byte[] newPath = Arrays.copyOfRange(normalizedPath, begin, end); + + // return the new path (always relative path) + // TODO: should the query/reference be propagated? + return new HttpPath(this.fs, null, null, absolute, newPath); } @Override @@ -163,6 +240,7 @@ public boolean startsWith(final String other) { * for the path component. * * @param other the other path component. + * * @return {@code true} if {@link #normalizedPath} ends with {@code other}; {@code false} * otherwise. */ @@ -304,8 +382,8 @@ public Path toAbsolutePath() { if (isAbsolute()) { return this; } - // TODO - change when we support relative Paths (https://github.com/magicDGS/jsr203-http/issues/12) - throw new IllegalStateException("Should not appear a relative HTTP/S paths (unsupported)"); + // just create a new path with a different absolute status + return new HttpPath(fs, query, reference, true, normalizedPath); } @Override @@ -333,7 +411,7 @@ public WatchKey register(final WatchService watcher, final WatchEvent.Kind... @Override public Iterator iterator() { - throw new UnsupportedOperationException("Not implemented"); + return IntStream.range(0, getNameCount()).mapToObj(this::getName).iterator(); } /** @@ -392,12 +470,12 @@ public int compareTo(final Path other) { /** * {@inheritDoc} * - * @implNote it uses the {@link #compareTo(Path)} method. + * @implNote it uses the {@link #compareTo(Path)} method and the absolute status. */ @Override public boolean equals(final Object other) { try { - return compareTo((Path) other) == 0; + return ((HttpPath) other).absolute == this.absolute && compareTo((Path) other) == 0; } catch (ClassCastException e) { return false; } @@ -406,13 +484,13 @@ public boolean equals(final Object other) { /** * {@inheritDoc} * - * @implNote Includes all the components of the path in a case-sensitive way, except the scheme - * and the authority. + * @implNote Includes the absolute status and all the components of the path in a + * case-sensitive way, except the scheme and the authority. */ @Override public int hashCode() { // TODO - maybe we should cache (https://github.com/magicDGS/jsr203-http/issues/18) - int h = fs.hashCode(); + int h = 31 * Boolean.hashCode(absolute) + fs.hashCode(); for (int i = 0; i < normalizedPath.length; i++) { h = 31 * h + (normalizedPath[i] & 0xff); } @@ -487,7 +565,6 @@ private void initOffsets() { * @return array of bytes, without multiple slashes together. */ private static byte[] getNormalizedPathBytes(final String path, final boolean checkRelative) { - // TODO - change when we support relative Paths (https://github.com/magicDGS/jsr203-http/issues/12) if (checkRelative && !path.isEmpty() && !path.startsWith(HttpUtils.HTTP_PATH_SEPARATOR_STRING)) { throw new InvalidPathException(path, "Relative HTTP/S path are not supported"); } diff --git a/src/test/java/org/magicdgs/http/jsr203/HttpPathUnitTest.java b/src/test/java/org/magicdgs/http/jsr203/HttpPathUnitTest.java index 89d5d33..9052de7 100644 --- a/src/test/java/org/magicdgs/http/jsr203/HttpPathUnitTest.java +++ b/src/test/java/org/magicdgs/http/jsr203/HttpPathUnitTest.java @@ -9,6 +9,7 @@ import java.net.URI; import java.nio.file.InvalidPathException; import java.nio.file.Path; +import java.util.stream.StreamSupport; /** * @author Daniel Gomez-Sanchez (magicDGS) @@ -109,6 +110,13 @@ public void testStartsWithNullPath() { TEST_FS.getPath("/file.txt").startsWith((Path) null); } + @Test + public void testStartsWithRelativePath() { + final Path path = TEST_FS.getPath("/dir/dir/dir"); + // check endsWith a relative subpath + Assert.assertTrue(path.startsWith(path.subpath(2,3))); + } + @Test public void testStartsWithDifferentProvider() { final Path httpPath = TEST_FS.getPath("/file.txt"); @@ -179,6 +187,14 @@ public void testEndsWithNullPath() { TEST_FS.getPath("/file.txt").endsWith((Path) null); } + @Test + public void testEndsWithRelativePath() { + final Path path = TEST_FS.getPath("/first/second/third"); + // check endsWith the subpath + Assert.assertTrue(path.endsWith(path.subpath(1, 3))); + Assert.assertTrue(path.endsWith(path.getFileName())); + } + @Test public void testEndsWithDifferentProvider() { final Path httpPath = TEST_FS.getPath("/file.txt"); @@ -186,16 +202,94 @@ public void testEndsWithDifferentProvider() { Assert.assertFalse(httpPath.endsWith(localPath)); } + @DataProvider + public Object[][] fileNames() { + return new Object[][] { + {"/index.html", "/index.html"}, + {"/dir/index.html", "/index.html"}, + {"/dir1/dir2/index.html", "/index.html"}, + // should also work with redundant paths (as we are already normalizing) + {"/dir//index.html", "/index.html"}, + {"/dir1//dir2//index.txt", "/index.txt"} + }; + } + + @Test(dataProvider = "fileNames") + public void testGetFileName(final String pathWithoutAuthority, final String expectedName) { + final HttpPath path = TEST_FS.getPath(pathWithoutAuthority); + // the best way to check if it is correct is to check the string representation + // because the equal method should include the absolute status + Assert.assertEquals(path.getFileName().toString(), + TEST_FS.getPath(expectedName).toString()); + // file names are never absolute + Assert.assertFalse(path.getFileName().isAbsolute()); + } + + @Test + public void testGetFileNameForRoot() { + Assert.assertNull(TEST_FS.getPath("/").getFileName()); + } + + @DataProvider + public static Object[][] parentData() { + return new Object[][] { + {"/dir/index.html", "/dir"}, + {"/dir1/dir2/index.html", "/dir1/dir2"}, + // should also work with redundant paths (as we are already normalizing) + {"/dir//index.html", "/dir"}, + {"/dir1//dir2//index.txt", "/dir1/dir2"} + }; + } + + @Test(dataProvider = "parentData") + public void testGetParentAbsolute(final String pathWithoutAuthority, + final String expectedParent) { + final HttpPath path = TEST_FS.getPath(pathWithoutAuthority); + final Path absoluteParent = path.getParent(); + // check that the paths are the same (even absolute in this case) + assertEqualsPath(absoluteParent, TEST_FS.getPath(expectedParent)); + Assert.assertTrue(absoluteParent.isAbsolute()); + } + + @Test(dataProvider = "parentData") + public void testGetParentRelative(final String pathWithoutAuthority, + final String expectedParent) { + final HttpPath path = TEST_FS.getPath(pathWithoutAuthority); + // get first a relative path by using subpath, and then the parent of it + final Path relativeParent = path.subpath(0, path.getNameCount() - 1).getParent(); + // the best way to check if it is correct is to check the string representation + // because the equal method should include the absolute status + Assert.assertEquals(path.getParent().toString(), + TEST_FS.getPath(expectedParent).toString()); + Assert.assertFalse(relativeParent.isAbsolute()); + } + + @Test + public void testGetParentIsRoot() { + // gets a file sited on the root path + final HttpPath path = TEST_FS.getPath("/index.html"); + // check that the parent and the root are the same + assertEqualsPath(path.getParent(), path.getRoot()); + // and that as root, it is always absolute + Assert.assertTrue(path.getParent().isAbsolute()); + } + + @Test + public void testGetParentForRoot() { + final HttpPath root = TEST_FS.getPath("/"); + assertEqualsPath(root.getParent(), root); + } + @DataProvider public Object[][] nameCounts() { - return new Object[][]{ + return new Object[][] { // contract says that root returns 0 counts {"http://" + TEST_AUTHORITY, 0}, {"http://" + TEST_AUTHORITY + "/", 0}, // files (never trailing slash) {"http://" + TEST_AUTHORITY + "/index.html", 1}, {"http://" + TEST_AUTHORITY + "/dir1/index.html", 2}, - {"http://" + TEST_AUTHORITY + "/dir1/dir2/index.html", 3}, + {"http://" + TEST_AUTHORITY + "/dir1/dir2/index.html", 3}, // directories (with and without trailing slash) {"https://" + TEST_AUTHORITY + "/dir", 1}, {"https://" + TEST_AUTHORITY + "/dir/", 1}, @@ -205,17 +299,38 @@ public Object[][] nameCounts() { } @Test(dataProvider = "nameCounts") - public void testGetNameCount(final String uriString, final int count) throws MalformedURLException { + public void testGetNameCount(final String uriString, final int count) + throws MalformedURLException { final HttpPath path = createPathFromUriStringOnTestProvider(uriString); Assert.assertEquals(path.getNameCount(), count); -// // check that the iterator returns the same number of elements -// // TODO: failing until the iterator is implemented () -// Assert.assertEquals(StreamSupport.stream(path.spliterator(), false).count(), count); -// // TODO: failing until the iterator is implemented () -// // check that getName(i) does not fail -// for (int i = 0; i < path.getNameCount(); i++) { -// Assert.assertNotNull(path.getName(i)); -// } + Assert.assertEquals(StreamSupport.stream(path.spliterator(), false).count(), count); + // check that getName(i) does not fail + for (int i = 0; i < path.getNameCount(); i++) { + Assert.assertNotNull(path.getName(i)); + } + } + + @DataProvider + public Object[][] invalidIndexSubpath() { + final HttpPath testPath = TEST_FS.getPath("/dir1/dir2/dir3/index.html"); + return new Object[][]{ + // for the root one, it should not work + {testPath.getRoot(), 0, 1}, + // negative start and end + {testPath, -1, 1}, + {testPath, 1, -1}, + // lower end than start + {testPath, 2, 1}, + // larger end than counts + {testPath, 1, testPath.getNameCount() + 1}, + // larger start than counts + {testPath, testPath.getNameCount() + 1, 2} + }; + } + + @Test(dataProvider = "invalidIndexSubpath", expectedExceptions = IllegalArgumentException.class) + public void testInvalidIndexSubpath(final HttpPath path, final int beginIndex, final int endIndex) { + path.subpath(beginIndex, endIndex); } @DataProvider @@ -242,6 +357,43 @@ public void testToUri(final String uriString) throws MalformedURLException { Assert.assertEquals(path.toUri().toURL(), uri.toURL()); } + @Test + public void testRelativeToUri() { + final String fileString = "/file.html"; + final Path path = TEST_FS.getPath("/dir1/dir2", fileString).getFileName(); + final URI expected = URI.create("http://" + TEST_AUTHORITY + fileString); + Assert.assertEquals(path.toUri(), expected); + } + + @Test + public void testToAbsolutePath() { + final HttpPath path = TEST_FS.getPath("/dir/index.html"); + Assert.assertSame(path.toAbsolutePath(), path); + } + + @Test + public void testToAbsolutePathFromRelative() { + final HttpPath path = TEST_FS.getPath("/dir/index.html"); + final HttpPath expectedAbsolute = TEST_FS.getPath("/index.html"); + assertEqualsPath(path.getFileName().toAbsolutePath(), expectedAbsolute); + } + + @Test + public void testIterator() { + final String[] parts = new String[] {"/dir1", "/dir2", "/index.html"}; + final HttpPath path = TEST_FS.getPath(String.join("")); + int index = 0; + for (final Path next : path) { + // check as a String (because it is relative) + Assert.assertEquals(next.toString(), TEST_FS.getPath(parts[index]).toString(), + "index=" + index); + // check that it is relative + Assert.assertFalse(next.isAbsolute(), "index=" + index); + // check the Path::getName + Assert.assertEquals(next, path.getName(index), "index=" + index); + } + } + @DataProvider public Object[][] compareToUriStrings() { // default values for testing @@ -347,7 +499,7 @@ public Object[][] compareToUriStrings() { "http://" + auth1 + "/" + file1 + "?" + query1 + "#" + ref1, "http://" + auth1 + "/" + file1 + "?" + query1 + "#" + ref2, ref1.compareTo(ref2) - }, + } }; } @@ -401,6 +553,15 @@ public void testEqualsDifferentProvider() { assertNotEqualsPath(httpPath, httpsPath); } + @Test + public void testEqualsAbsoluteRelative() { + final HttpPath absolute = TEST_FS.getPath("/index.html"); + final Path relative = absolute.subpath(0, absolute.getNameCount()); + // sanity check in case something change in the implementation + Assert.assertFalse(relative.isAbsolute(), "error in test data"); + assertNotEqualsPath(absolute, relative); + } + @Test(dataProvider = "validUriStrings") public void testHashCodeSameObject(final String uriString) { final HttpPath path = createPathFromUriStringOnTestProvider(uriString); @@ -413,6 +574,15 @@ public void testHashCodeEqualObjects(final String uriString) { createPathFromUriStringOnTestProvider(uriString).hashCode()); } + @Test + public void testHashCodeAbsoluteRelativeDiffers() { + final HttpPath absolute = TEST_FS.getPath("/index.html"); + final Path relative = absolute.subpath(0, absolute.getNameCount()); + // sanity check in case something change in the implementation + Assert.assertFalse(relative.isAbsolute(), "error in test data"); + Assert.assertNotEquals(absolute.hashCode(), relative.hashCode()); + } + @Test(dataProvider = "validUriStrings") public void testToString(final String uriString) { final HttpPath path = createPathFromUriStringOnTestProvider(uriString);