Skip to content

Commit

Permalink
[GEOS-4616] Downloading zip file using /rest/workspaces/<ws>/datastor…
Browse files Browse the repository at this point in the history 
…es/<ds>/file.shp doesn't work after GeoServer reload
  • Loading branch information
@aaime
aaime committed Feb 25, 2017
1 parent 59cc6c7 commit b692044
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 19 deletions.
48 changes: 33 additions & 15 deletions src/restconfig/src/main/java/org/geoserver/catalog/rest/DataStoreFileResource.java
View file
Expand Up @@ -26,6 +26,8 @@
import org.geoserver.catalog.FeatureTypeInfo;
import org.geoserver.catalog.LayerInfo;
import org.geoserver.catalog.NamespaceInfo;
import org.geoserver.catalog.ResourcePool;
import org.geoserver.platform.GeoServerResourceLoader;
import org.geoserver.platform.resource.Resource;
import org.geoserver.platform.resource.Resources;
import org.geoserver.platform.resource.Resource.Type;
Expand Down Expand Up @@ -150,23 +152,36 @@ public void handleGet() {
if (info == null) {
throw new RestletException("No such datastore " + datastore, Status.CLIENT_ERROR_NOT_FOUND);
}

Map<String,Serializable> params = info.getConnectionParameters();
ResourcePool rp = info.getCatalog().getResourcePool();
GeoServerResourceLoader resourceLoader = info.getCatalog().getResourceLoader();
Map<String,Serializable> rawParamValues = info.getConnectionParameters();
Map<String,Serializable> paramValues = rp.getParams(rawParamValues, resourceLoader);
File directory = null;
for (Map.Entry<String, Serializable> e : params.entrySet()) {
if (e.getValue() instanceof File) {
directory = (File) e.getValue();
}
else if (e.getValue() instanceof URL) {
directory = new File(((URL)e.getValue()).getFile());
}
if (directory != null && !"directory".equals(e.getKey())) {
directory = directory.getParentFile();
}

if (directory != null) {
break;
try {
DataAccessFactory factory = rp.getDataStoreFactory(info);
for (Param param : factory.getParametersInfo()) {
if(File.class.isAssignableFrom(param.getType())) {
Object result = param.lookUp(paramValues);
if(result instanceof File) {
directory = (File) result;
}
} else if(URL.class.isAssignableFrom(param.getType())) {
Object result = param.lookUp(paramValues);
if(result instanceof URL) {
directory = new File(((URL)result).getFile());

This comment has been minimized.

Sign in to view

Copy link
@bencaradocdavies

bencaradocdavies Feb 25, 2017 •
edited

Contributor

This bad URL->File conversion breaks the build in a path with spaces, and in general will break this functionality in any path with non-URL-safe characters. Fixed in 7db694e on master and 94640b1 on 2.10.x.

The failure:

Tests run: 13, Failures: 1, Errors: 0, Skipped: 1, Time elapsed: 2.008 sec <<< FAILURE!
testShapeFileUploadExternal(org.geoserver.catalog.rest.DataStoreFileUploadTest)  Time elapsed: 259 sec  <<< FAILURE!
java.lang.AssertionError: expected:<200> but was:<404>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:743)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at org.junit.Assert.assertEquals(Assert.java:555)
	at org.junit.Assert.assertEquals(Assert.java:542)
	at org.geoserver.catalog.rest.DataStoreFileUploadTest.testShapeFileUploadExternal(DataStoreFileUploa
}
}

if (directory != null && !"directory".equals(param.key)) {
directory = directory.getParentFile();
}

if (directory != null) {
break;
}
}
} catch(Exception e) {
throw new RestletException( "Failed to lookup source directory for store " + datastore, Status.CLIENT_ERROR_NOT_FOUND, e);
}

if ( directory == null || !directory.exists() || !directory.isDirectory() ) {
Expand Down Expand Up @@ -204,6 +219,9 @@ protected void write(Object object, OutputStream out)
zout.close();
}
};
Form headers = RESTUtils.getHeaders(getResponse());
headers.add("Content-type", "application/zip");
headers.add("Content-disposition", "attachment; filename=" + info.getName() + ".zip");
getResponse().setEntity( fmt.toRepresentation( directory ) );
}

Expand Down
33 changes: 29 additions & 4 deletions src/restconfig/src/test/java/org/geoserver/catalog/rest/DataStoreFileUploadTest.java
View file
Expand Up @@ -5,7 +5,12 @@
*/
package org.geoserver.catalog.rest;

import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;

import java.io.BufferedWriter;
import java.io.ByteArrayInputStream;
Expand All @@ -18,7 +23,9 @@
import java.net.URL;
import java.nio.charset.Charset;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import java.util.zip.ZipInputStream;
Expand All @@ -45,12 +52,12 @@
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletResponse;
import org.terracotta.context.query.Matchers;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;

import org.springframework.mock.web.MockHttpServletResponse;

public class DataStoreFileUploadTest extends CatalogRESTTestSupport {

@Override
Expand Down Expand Up @@ -218,6 +225,24 @@ public void testShapeFileUploadExternal() throws Exception {

dom = getAsDOM("wfs?request=getfeature&typename=gs:pds");
assertFeatures(dom);

// try to download it again after a full reload from disk (GEOS-4616)
getGeoServer().reload();

resp = getAsServletResponse("/rest/workspaces/gs/datastores/pds/file.shp");
assertEquals( 200, resp.getStatus() );
assertEquals( "application/zip", resp.getContentType() );

Set<String> entryNames = new HashSet<>();
try(ByteArrayInputStream bin = getBinaryInputStream(resp); ZipInputStream zin = new ZipInputStream( bin )) {
ZipEntry entry;
while((entry= zin.getNextEntry()) != null) {
entryNames.add(entry.getName());
}
}
assertTrue(entryNames.contains("pds.shp"));
assertTrue(entryNames.contains("pds.shx"));
assertTrue(entryNames.contains("pds.dbf"));
} finally {
FileUtils.deleteQuietly(f);
}
Expand Down Expand Up @@ -314,7 +339,7 @@ public void testShapefileUploadMultiple() throws Exception {
}

@Test
public void testGet() throws Exception {
public void testGetProperties() throws Exception {
MockHttpServletResponse resp = getAsServletResponse("/rest/workspaces/gs/datastores/pds/file.properties");
assertEquals( 404, resp.getStatus() );

Expand Down

0 comments on commit b692044

Please sign in to comment.