Skip to content

Commit

Permalink
Fixes #5521 ResourceCollection NPE (#5527)
Browse files Browse the repository at this point in the history
* Fixes #5521 ResourceCollection NPE

Fix constructor and addPath so that all resources in a RC must exist when created.

* Fixes #5521 ResourceCollection NPE

Cleanup the vestiges of non existent directories detected by resource ending in /

* Fixes #5521 ResourceCollection NPE

Revert adding paths ending in / as jar:file resource needs them

* feedback from review

improved javadoc.
  • Loading branch information
gregw committed Nov 4, 2020
1 parent 448d700 commit 1904d32
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 46 deletions.
Expand Up @@ -144,7 +144,7 @@ public boolean exists()
String fileUrl = _urlString.substring(4, _urlString.length() - 2);
try
{
return newResource(fileUrl).exists();
return _directory = newResource(fileUrl).exists();
}
catch (Exception e)
{
Expand Down Expand Up @@ -236,15 +236,10 @@ else if (entry.isDirectory())
return _exists;
}

/**
* Returns true if the represented resource is a container/directory.
* If the resource is not a file, resources ending with "/" are
* considered directories.
*/
@Override
public boolean isDirectory()
{
return _urlString.endsWith("/") || exists() && _directory;
return exists() && _directory;
}

/**
Expand Down
Expand Up @@ -320,8 +320,6 @@ public static boolean isContainedIn(Resource r, Resource containingResource) thr

/**
* @return true if the represented resource is a container/directory.
* if the resource is not a file, resources ending with "/" are
* considered directories.
*/
public abstract boolean isDirectory();

Expand Down Expand Up @@ -412,7 +410,7 @@ public abstract boolean renameTo(Resource dest)

/**
* Returns the resource contained inside the current resource with the
* given name.
* given name, which may or may not exist.
*
* @param path The path segment to add, which is not encoded
* @return the Resource for the resolved path within this Resource, never null
Expand Down
Expand Up @@ -62,8 +62,19 @@ public ResourceCollection()
* @param resources the resources to be added to collection
*/
public ResourceCollection(Resource... resources)
{
this(Arrays.asList(resources));
}

/**
* Instantiates a new resource collection.
*
* @param resources the resources to be added to collection
*/
public ResourceCollection(Collection<Resource> resources)
{
_resources = new ArrayList<>();

for (Resource r : resources)
{
if (r == null)
Expand All @@ -82,17 +93,6 @@ public ResourceCollection(Resource... resources)
}
}

/**
* Instantiates a new resource collection.
*
* @param resources the resources to be added to collection
*/
public ResourceCollection(Collection<Resource> resources)
{
_resources = new ArrayList<>();
_resources.addAll(resources);
}

/**
* Instantiates a new resource collection.
*
Expand Down Expand Up @@ -226,8 +226,16 @@ public void setResources(String resources) throws IOException
}

/**
* Add a path to the resource collection.
* @param path The path segment to add
* @return The contained resource (found first) in the collection of resources
* @return The resulting resource(s) :
* <ul>
* <li>is a file that exists in at least one of the collection, then the first one found is returned</li>
* <li>is a directory that exists in at exactly one of the collection, then that directory resource is returned </li>
* <li>is a directory that exists in several of the collection, then a ResourceCollection of those directories is returned</li>
* <li>do not exist in any of the collection, then a new non existent resource relative to the first in the collection is returned.</li>
* </ul>
* @throws MalformedURLException if the resolution of the path fails because the input path parameter is malformed against any of the collection
*/
@Override
public Resource addPath(String path) throws IOException
Expand All @@ -247,27 +255,28 @@ public Resource addPath(String path) throws IOException
ArrayList<Resource> resources = null;

// Attempt a simple (single) Resource lookup that exists
Resource addedResource = null;
for (Resource res : _resources)
{
Resource r = res.addPath(path);
if (!r.isDirectory() && r.exists())
{
// Return simple (non-directory) Resource
return r;
}

addedResource = res.addPath(path);
if (!addedResource.exists())
continue;
if (!addedResource.isDirectory())
return addedResource; // Return simple (non-directory) Resource
if (resources == null)
{
resources = new ArrayList<>();
}
resources.add(addedResource);
}

resources.add(r);
if (resources == null)
{
if (addedResource != null)
return addedResource; // This will not exist
return EmptyResource.INSTANCE;
}

if (resources.size() == 1)
{
return resources.get(0);
}

return new ResourceCollection(resources);
}
Expand Down Expand Up @@ -384,7 +393,6 @@ public URI getURI()
public boolean isDirectory()
{
assertResourcesSet();

return true;
}

Expand Down
Expand Up @@ -128,11 +128,6 @@ public boolean exists()
return _in != null;
}

/**
* Returns true if the represented resource is a container/directory.
* If the resource is not a file, resources ending with "/" are
* considered directories.
*/
@Override
public boolean isDirectory()
{
Expand Down
Expand Up @@ -35,7 +35,6 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.emptyArray;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -189,7 +188,7 @@ public void testList() throws Exception

assertThat(Arrays.asList(rc1.list()), contains("1.txt", "2.txt", "3.txt", "dir/"));
assertThat(Arrays.asList(rc1.addPath("dir").list()), contains("1.txt", "2.txt", "3.txt"));
assertThat(rc1.addPath("unknown").list(), emptyArray());
assertThat(rc1.addPath("unknown").list(), nullValue());
}

@Test
Expand Down
Expand Up @@ -215,15 +215,15 @@ public static Stream<Arguments> scenarios() throws Exception
cases.addCase(new Scenario(tdata1, "alphabet.txt", EXISTS, !DIR, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
cases.addCase(new Scenario(tdata2, "alphabet.txt", EXISTS, !DIR, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"));

cases.addCase(new Scenario("jar:file:/somejar.jar!/content/", !EXISTS, DIR));
cases.addCase(new Scenario("jar:file:/somejar.jar!/", !EXISTS, DIR));
cases.addCase(new Scenario("jar:file:/somejar.jar!/content/", !EXISTS, !DIR));
cases.addCase(new Scenario("jar:file:/somejar.jar!/", !EXISTS, !DIR));

String urlRef = cases.uriRef.toASCIIString();
Scenario zdata = new Scenario("jar:" + urlRef + "TestData/test.zip!/", EXISTS, DIR);
cases.addCase(zdata);

cases.addCase(new Scenario(zdata, "Unknown", !EXISTS, !DIR));
cases.addCase(new Scenario(zdata, "/Unknown/", !EXISTS, DIR));
cases.addCase(new Scenario(zdata, "/Unknown/", !EXISTS, !DIR));

cases.addCase(new Scenario(zdata, "subdir", EXISTS, DIR));
cases.addCase(new Scenario(zdata, "/subdir/", EXISTS, DIR));
Expand Down
Expand Up @@ -31,7 +31,6 @@
import java.util.EventListener;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down

0 comments on commit 1904d32

Please sign in to comment.