From 8969c9a18cd86e0eff3dd5651fe6ddb165b2ad58 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 14 Feb 2019 08:02:24 -0500 Subject: [PATCH] Issue #3278 - more cleanup based on review of older codebase with simone --- .../util/resource/ResourceCollection.java | 69 ++----------------- .../util/resource/ResourceCollectionTest.java | 45 ++++++------ 2 files changed, 28 insertions(+), 86 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index bb5f4795915..563de253eb0 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -26,6 +26,7 @@ import java.net.URL; import java.nio.channels.ReadableByteChannel; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.StringTokenizer; @@ -60,7 +61,7 @@ public class ResourceCollection extends Resource */ public ResourceCollection(Resource... resources) { - List list = new ArrayList(); + List list = new ArrayList<>(); for (Resource r : resources) { if (r == null) @@ -69,17 +70,14 @@ public class ResourceCollection extends Resource } if (r instanceof ResourceCollection) { - for (Resource r2 : ((ResourceCollection) r).getResources()) - { - list.add(r2); - } + Collections.addAll(list, ((ResourceCollection) r).getResources()); } else { list.add(r); } } - _resources = list.toArray(new Resource[list.size()]); + _resources = list.toArray(new Resource[0]); for (Resource r : _resources) { assertResourceValid(r); @@ -107,7 +105,7 @@ public class ResourceCollection extends Resource { if (strResource == null || strResource.length() == 0) { - throw new IllegalStateException("empty resource path not supported"); + throw new IllegalArgumentException("empty/null resource path not supported"); } Resource resource = Resource.newResource(strResource); assertResourceValid(resource); @@ -306,58 +304,6 @@ public class ResourceCollection extends Resource return null; } - /** - * @param path the path to look for - * @return the resource(file) if found, returns a list of resource dirs if its a dir, else null. - * @throws IOException if unable to look for path - * @throws MalformedURLException if failed to look for path due to url issue - */ - protected Object findResource(String path) throws IOException, MalformedURLException - { - assertResourcesSet(); - - Resource resource = null; - ArrayList resources = null; - int i = 0; - for (; i < _resources.length; i++) - { - resource = _resources[i].addPath(path); - if (resource.exists()) - { - if (resource.isDirectory()) - { - break; - } - - return resource; - } - } - - for (i++; i < _resources.length; i++) - { - Resource r = _resources[i].addPath(path); - if (r.exists() && r.isDirectory()) - { - if (resource != null) - { - resources = new ArrayList<>(); - resources.add(resource); - } - resources.add(r); - } - } - - if (resource != null) - { - return resource; - } - if (resources != null) - { - return resources; - } - return null; - } - @Override public boolean delete() throws SecurityException { @@ -493,10 +439,7 @@ public class ResourceCollection extends Resource HashSet set = new HashSet<>(); for (Resource r : _resources) { - for (String s : r.list()) - { - set.add(s); - } + Collections.addAll(set, r.list()); } String[] result = set.toArray(new String[0]); Arrays.sort(result); diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java index 9cd2da52d04..f6fa4e80afa 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java @@ -23,6 +23,7 @@ import java.io.File; import java.io.InputStreamReader; import java.nio.file.Path; +import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; @@ -30,6 +31,8 @@ import org.eclipse.jetty.util.IO; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.CoreMatchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -72,11 +75,10 @@ public class ResourceCollectionTest } @Test - public void testStringArrayWithNull_ThrowsISE() + public void testStringArrayWithNull_ThrowsIAE() { - ResourceCollection coll = new ResourceCollection(new String[]{null}); - - assertThrowIllegalStateException(coll); + assertThrows(IllegalArgumentException.class, + ()-> new ResourceCollection(new String[]{null})); } @Test @@ -143,25 +145,25 @@ public class ResourceCollectionTest ResourceCollection coll = new ResourceCollection(resource); // Reset collection to invalid state - coll.setResources(new Resource[]{null,null,null}); + assertThrows(IllegalStateException.class, ()-> coll.setResources(new Resource[]{null, null, null})); - assertThrowIllegalStateException(coll); + // Ensure not modified. + assertThat(coll.getResources().length, is(1)); } private void assertThrowIllegalStateException(ResourceCollection coll) { assertThrows(IllegalStateException.class, ()->coll.addPath("foo")); - assertThrows(IllegalStateException.class, ()->coll.findResource("bar")); - assertThrows(IllegalStateException.class, ()->coll.exists()); - assertThrows(IllegalStateException.class, ()->coll.getFile()); - assertThrows(IllegalStateException.class, ()->coll.getInputStream()); - assertThrows(IllegalStateException.class, ()->coll.getReadableByteChannel()); - assertThrows(IllegalStateException.class, ()->coll.getURL()); - assertThrows(IllegalStateException.class, ()->coll.getName()); - assertThrows(IllegalStateException.class, ()->coll.isDirectory()); - assertThrows(IllegalStateException.class, ()->coll.lastModified()); - assertThrows(IllegalStateException.class, ()->coll.list()); - assertThrows(IllegalStateException.class, ()->coll.close()); + assertThrows(IllegalStateException.class, coll::exists); + assertThrows(IllegalStateException.class, coll::getFile); + assertThrows(IllegalStateException.class, coll::getInputStream); + assertThrows(IllegalStateException.class, coll::getReadableByteChannel); + assertThrows(IllegalStateException.class, coll::getURL); + assertThrows(IllegalStateException.class, coll::getName); + assertThrows(IllegalStateException.class, coll::isDirectory); + assertThrows(IllegalStateException.class, coll::lastModified); + assertThrows(IllegalStateException.class, coll::list); + assertThrows(IllegalStateException.class, coll::close); assertThrows(IllegalStateException.class, ()-> { Path destPath = workdir.getPathFile("bar"); @@ -219,11 +221,8 @@ public class ResourceCollectionTest "src/test/resources/org/eclipse/jetty/util/resource/three/" }); - File dest = File.createTempFile("copyto",null); - if (dest.exists()) - dest.delete(); - dest.mkdir(); - dest.deleteOnExit(); + File dest = MavenTestingUtils.getTargetTestingDir("copyto"); + FS.ensureDirExists(dest); rc.copyTo(dest); Resource r = Resource.newResource(dest.toURI()); @@ -241,7 +240,7 @@ public class ResourceCollectionTest static String getContent(Resource r, String path) throws Exception { StringBuilder buffer = new StringBuilder(); - String line = null; + String line; try (BufferedReader br = new BufferedReader(new InputStreamReader(r.addPath(path).getURL().openStream()))) { while((line=br.readLine())!=null)