[GEOS-8332] DataStoreFileController wrong management of FeatureTypes …

…when dealing with DB-based DataStores
geoserver · Nov 16, 2017 · d1e244f · d1e244f
1 parent e19908f
commit d1e244f
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 45 deletions.
diff --git a/src/rest/src/main/java/org/geoserver/rest/util/IOUtils.java b/src/rest/src/main/java/org/geoserver/rest/util/IOUtils.java
@@ -10,7 +10,6 @@
 import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.FilenameFilter;
 import java.io.IOException;
 import java.io.InputStream;
@@ -749,12 +748,26 @@ public static String getStringFromStreamSource(StreamSource src) throws IOExcept
      * 
      * @param archive the {@link ZipFile} to inflate.
      * @param outputDirectory the directory where to inflate the archive.
+     * @param fileName name of the file if present.
      * @throws IOException in case something bad happens.
-     * @throws FileNotFoundException in case something bad happens.
      */
     public static void inflate(ZipFile archive, Resource outputDirectory, String fileName)
             throws IOException {
-        inflate(archive, outputDirectory, fileName, null, null, null, false);
+        inflate(archive, outputDirectory, fileName, null, null, null, false, false);
+    }
+
+    /**
+     * Inflate the provided {@link ZipFile} in the provided output directory.
+     * 
+     * @param archive the {@link ZipFile} to inflate.
+     * @param outputDirectory the directory where to inflate the archive.
+     * @param fileName name of the file if present.
+     * @param external
+     * @throws IOException in case something bad happens.
+     */
+    public static void inflate(ZipFile archive, Resource outputDirectory, String fileName,
+            boolean external) throws IOException {
+        inflate(archive, outputDirectory, fileName, null, null, null, external, false);
     }
 
     /**
@@ -767,9 +780,26 @@ public static void inflate(ZipFile archive, Resource outputDirectory, String fil
      * @param files empty list of the extracted files (or null if there is no desire to collect the list)
      * @throws IOException in case something bad happens.
      */
+    public static void inflate(ZipFile archive, Resource outputDirectory, String fileName,
+            String workspace, String store, List<Resource> files, boolean external)
+            throws IOException {
+        inflate(archive, outputDirectory, fileName, null, null, files, external, true);
+    }
+
+    /**
+     * Inflate the provided {@link ZipFile} in the provided output directory.
+     * 
+     * @param archive the {@link ZipFile} to inflate.
+     * @param outputDirectory the directory where to inflate the archive.
+     * @param fileName name of the file if present.
+     * @param external
+     * @param saveFile boolean to specify to save or not the list of the extracted files
+     * @param files empty list of the extracted files (or null if there is no desire to collect the list)
+     * @throws IOException in case something bad happens.
+     */
     public static void inflate(ZipFile archive, Resource outputDirectory, String fileName,
             String workspace, String store, List<Resource> files,
-            boolean external) throws IOException {
+            boolean external, boolean saveFile) throws IOException {
 
         final Enumeration<? extends ZipEntry> entries = archive.entries();
         try {
@@ -799,7 +829,7 @@ public static void inflate(ZipFile archive, Resource outputDirectory, String fil
 
                     IOUtils.copyStream(in, out, true, true);
                     // If the file must be listed, then the file is added to the list
-                    if (files != null) {
+                    if (saveFile && files != null) {
                         files.add(outFile);
                     }
                 }

diff --git a/src/rest/src/main/java/org/geoserver/rest/util/RESTUtils.java b/src/rest/src/main/java/org/geoserver/rest/util/RESTUtils.java
@@ -268,7 +268,7 @@ public static void unzipFile(org.geoserver.platform.resource.Resource zipFile,
         }
         ZipFile archive = new ZipFile(zipFile.file());
 
-        IOUtils.inflate(archive, outputDirectory, null, workspace, store, files, external);
+        IOUtils.inflate(archive, outputDirectory, null, workspace, store, files, external, true);
         zipFile.delete();
     }
 

diff --git a/src/restconfig/src/main/java/org/geoserver/rest/catalog/AbstractStoreUploadController.java b/src/restconfig/src/main/java/org/geoserver/rest/catalog/AbstractStoreUploadController.java
@@ -127,9 +127,11 @@ protected List<Resource> handleFileUpload(String store, String workspace, String
             }
         }
         // If the File List is empty then the uploaded file must be added
-        if (uploadedFile != null) {
+        if (files.isEmpty() && uploadedFile != null) {
             files.clear();
             files.add(uploadedFile);
+        } else {
+            files.add(0, uploadedFile);
         }
 
         return files;

diff --git a/src/restconfig/src/main/java/org/geoserver/rest/catalog/DataStoreFileController.java b/src/restconfig/src/main/java/org/geoserver/rest/catalog/DataStoreFileController.java
@@ -4,7 +4,35 @@
  */
 package org.geoserver.rest.catalog;
 
-import org.geoserver.catalog.*;
+import java.io.BufferedOutputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.net.URI;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.logging.Level;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.geoserver.catalog.Catalog;
+import org.geoserver.catalog.CatalogBuilder;
+import org.geoserver.catalog.CoverageStoreInfo;
+import org.geoserver.catalog.DataStoreInfo;
+import org.geoserver.catalog.FeatureTypeInfo;
+import org.geoserver.catalog.LayerInfo;
+import org.geoserver.catalog.NamespaceInfo;
+import org.geoserver.catalog.ResourcePool;
 import org.geoserver.platform.GeoServerExtensions;
 import org.geoserver.platform.GeoServerResourceLoader;
 import org.geoserver.platform.resource.Paths;
@@ -14,7 +42,15 @@
 import org.geoserver.rest.RestException;
 import org.geoserver.rest.util.IOUtils;
 import org.geoserver.rest.util.RESTUploadPathMapper;
-import org.geotools.data.*;
+import org.geotools.data.DataAccess;
+import org.geotools.data.DataAccessFactory;
+import org.geotools.data.DataStore;
+import org.geotools.data.DataUtilities;
+import org.geotools.data.DefaultTransaction;
+import org.geotools.data.FeatureSource;
+import org.geotools.data.FeatureStore;
+import org.geotools.data.FileDataStoreFactorySpi;
+import org.geotools.data.Transaction;
 import org.geotools.feature.FeatureCollection;
 import org.geotools.geometry.jts.ReferencedEnvelope;
 import org.geotools.jdbc.JDBCDataStoreFactory;
@@ -27,24 +63,15 @@
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
-import org.springframework.web.bind.annotation.*;
+import org.springframework.web.bind.annotation.ControllerAdvice;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
 import org.vfny.geoserver.util.DataStoreUtils;
 
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import java.io.*;
-import java.net.URI;
-import java.net.URL;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.logging.Level;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipOutputStream;
-
 @RestController
 @ControllerAdvice
 @RequestMapping(path = RestBaseController.ROOT_PATH
@@ -492,6 +519,14 @@ public void dataStorePut(
                     if (!uploadedFile.parent().delete()) {
                         LOGGER.info("Unable to delete " + uploadedFile.path());
                     }
+                } else if (uploadedFile.getType() == Resource.Type.DIRECTORY) {
+                    for (Resource file : files) {
+                        if (file.getType() == Resource.Type.RESOURCE) {
+                            if (!file.delete()) {
+                                LOGGER.info("Unable to delete " + file.path());
+                            }
+                        }
+                    }
                 }
             }
         }

diff --git a/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreFileUploadTest.java b/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreFileUploadTest.java
@@ -4,19 +4,37 @@
  */
 package org.geoserver.rest.catalog;
 
-import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.IOUtils;
+import static org.geoserver.rest.RestBaseController.ROOT_PATH;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.BufferedWriter;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStreamWriter;
+import java.net.URL;
+import java.nio.charset.Charset;
+import java.util.Collections;
+import java.util.List;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
+
+import javax.servlet.Filter;
+
 import org.geoserver.catalog.Catalog;
 import org.geoserver.catalog.DataStoreInfo;
 import org.geoserver.catalog.NamespaceInfo;
 import org.geoserver.catalog.WorkspaceInfo;
 import org.geoserver.catalog.impl.NamespaceInfoImpl;
 import org.geoserver.catalog.impl.WorkspaceInfoImpl;
-import org.geoserver.data.test.MockData;
 import org.geoserver.data.test.SystemTestData;
 import org.geoserver.filters.LoggingFilter;
 import org.geoserver.platform.GeoServerResourceLoader;
-import org.geoserver.platform.resource.Files;
 import org.geotools.data.DataUtilities;
 import org.h2.tools.DeleteDbFiles;
 import org.junit.After;
@@ -28,22 +46,6 @@
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
-import javax.servlet.Filter;
-import java.io.*;
-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;
-import java.util.zip.ZipOutputStream;
-
-import static org.geoserver.rest.RestBaseController.ROOT_PATH;
-import static org.junit.Assert.*;
-
 public class DataStoreFileUploadTest extends CatalogRESTTestSupport {
     @Override
     protected void onSetUp(SystemTestData testData) throws Exception {
@@ -79,6 +81,7 @@ public void cleanUpDbFiles() throws Exception {
         DeleteDbFiles.execute("target", "foo", true);
         DeleteDbFiles.execute("target", "pds", true);
         DeleteDbFiles.execute("target", "chinese_poly", true);
+        DeleteDbFiles.execute("target", "san_andres_y_providencia", true);
     }
 
     byte[] propertyFile() throws IOException {
@@ -108,6 +111,10 @@ byte[] shpChineseZipAsBytes() throws IOException {
         return toBytes(getClass().getResourceAsStream( "test-data/chinese_poly.zip" ));
     }
 
+    byte[] shpSanAndresShapefilesZipAsBytes() throws IOException {
+        return toBytes(getClass().getResourceAsStream("test-data/san_andres_y_providencia.zip"));
+    }
+
     byte[] shpMultiZipAsBytes() throws IOException {
         return toBytes(getClass().getResourceAsStream( "test-data/pdst.zip" ));
     }
@@ -166,6 +173,21 @@ public void testShapefileUploadMultiple() throws Exception {
         assertEquals(2, cat.getFeatureTypesByDataStore(ds).size());
     }
 
+    @Test
+    public void testShapefileUploadZip() throws Exception {
+        Catalog cat = getCatalog();
+        assertNull(cat.getDataStoreByName("gs", "san_andres_y_providencia"));
+
+        put(ROOT_PATH + "/workspaces/gs/datastores/san_andres_y_providencia/file.shp",
+                shpSanAndresShapefilesZipAsBytes(),
+                "application/zip");
+
+        DataStoreInfo ds = cat.getDataStoreByName("gs", "san_andres_y_providencia");
+        assertNotNull(ds);
+
+        assertEquals(1, cat.getFeatureTypesByDataStore(ds).size());
+    }
+
     @Test
     public void testGetProperties() throws Exception {
         MockHttpServletResponse resp = getAsServletResponse(ROOT_PATH+"/workspaces/gs/datastores/pds/file.properties");

diff --git a/...estconfig/src/test/java/org/geoserver/rest/catalog/test-data/san_andres_y_providencia.zip b/...estconfig/src/test/java/org/geoserver/rest/catalog/test-data/san_andres_y_providencia.zip