Prevent duplicate GeoGig repository names

Signed-off-by: Erik Merkle <emerkle@boundlessgeo.com>

src/community/geogig/src/main/java/org/geogig/geoserver/config/GeoServerGeoGigRepositoryResolver.java

@@ -41,15 +41,8 @@ public boolean repoExists(URI repoURI) throws IllegalArgumentException {
         String name = getName(repoURI);
         RepositoryManager repoMgr = RepositoryManager.get();
         // get the repo by name
-        RepositoryInfo repoInfo;
-        try {
-            repoInfo = repoMgr.getByRepoName(name);
-            // if it doesn't throw an exception, it exists
-            return repoInfo != null;
-        } catch (Exception ex) {
-            // doesn't exist
-            return false;
-        }
+        RepositoryInfo repoInfo = repoMgr.getByRepoName(name);
+        return repoInfo != null;
     }
 
     @Override
@@ -79,8 +72,8 @@ public Repository open(URI repositoryLocation) throws RepositoryConnectionExcept
         // get a handle to the RepositoryManager
         RepositoryManager repoMgr = RepositoryManager.get();
         // get the repo by name
-        try {
-            RepositoryInfo info = repoMgr.getByRepoName(name);
+        RepositoryInfo info = repoMgr.getByRepoName(name);
+        if (info != null) {
             // get the native RepositoryResolver for the location and open it directly
             // Using the RepositryManager to get the repo would cause the repo to be managed by the RepositoryManager,
             // when this repo should be managed by the DataStore. The DataStore will close this repo instance when
@@ -89,11 +82,10 @@ public Repository open(URI repositoryLocation) throws RepositoryConnectionExcept
             checkState(repo.isOpen(), "RepositoryManager returned a closed repository for %s",
                     name);
             return repo;
-        } catch (Exception ex) {
+        } else {
             // didn't find a repo
             RepositoryConnectionException rce = new RepositoryConnectionException(
                     "No GeoGig repository found with NAME or ID: " + name);
-            rce.initCause(ex);
             throw rce;
         }
     }

src/community/geogig/src/main/java/org/geogig/geoserver/config/RepositoryManager.java

@@ -189,13 +189,16 @@ public RepositoryInfo get(final String repoId) throws IOException {
         }
     }
 
+    /**
+     * Retrieves a RepositoryInfo with a specified name.
+     *
+     * @param name The name of the repository desired.
+     *
+     * @return a RepositoryInfo object, if found. If not found, returns null.
+     */
     public RepositoryInfo getByRepoName(final String name) {
         RepositoryInfo info = configStore.getByName(name);
-        if (info != null) {
-            return info;
-        }
-        // didn't find it
-        throw new NoSuchElementException("No repository with name " + name + " exists");
+        return info;
     }
 
     public List<DataStoreInfo> findGeogigStores() {

src/community/geogig/src/main/java/org/geogig/geoserver/rest/ImportRepoCommandResource.java

@@ -1,9 +1,16 @@
 package org.geogig.geoserver.rest;
 
+import org.geogig.geoserver.config.RepositoryManager;
 import org.locationtech.geogig.rest.repository.CommandResource;
+import org.locationtech.geogig.web.api.CommandSpecException;
 import org.locationtech.geogig.web.api.ParameterSet;
+import org.locationtech.geogig.web.api.RESTUtils;
 import org.locationtech.geogig.web.api.WebAPICommand;
+import org.restlet.data.MediaType;
 import org.restlet.data.Request;
+import org.restlet.data.Status;
+import org.restlet.resource.Representation;
+import org.restlet.resource.Variant;
 
 public class ImportRepoCommandResource extends CommandResource {
 
@@ -22,4 +29,21 @@ protected ParameterSet handleRequestEntity(Request request) {
     protected WebAPICommand buildCommand(String commandName) {
         return new ImportExistingRepo();
     }
+
+    @Override
+    protected Representation runCommand(Variant variant, Request request, MediaType outputFormat) {
+        // before importing the repository, extract the repository name from the request and see if
+        // a repository with that name already exists
+        final String repoName = RESTUtils.getStringAttribute(request, "repository");
+        if (repoName != null) {
+            if (RepositoryManager.get().getByRepoName(repoName) != null) {
+                // repo already exists
+                throw new CommandSpecException(
+                    "The specified repository name is already in use, please try a different name",
+                    Status.CLIENT_ERROR_CONFLICT);
+            }
+        }
+        // all good
+        return super.runCommand(variant, request, outputFormat);
+    }
 }

src/community/geogig/src/main/java/org/geogig/geoserver/rest/InitCommandResource.java

@@ -9,28 +9,29 @@
 
 import org.geogig.geoserver.config.RepositoryInfo;
 import org.geogig.geoserver.config.RepositoryManager;
-import org.locationtech.geogig.web.api.ParameterSet;
+import org.locationtech.geogig.web.api.CommandSpecException;
+import org.locationtech.geogig.web.api.RESTUtils;
 import org.restlet.data.MediaType;
 import org.restlet.data.Request;
 import org.restlet.data.Status;
 import org.restlet.resource.Representation;
 import org.restlet.resource.Variant;
 
 public class InitCommandResource extends org.locationtech.geogig.rest.repository.InitCommandResource {
-
-    @Override
-    protected String getCommandName() {
-        return "init";
-    }
-
-    @Override
-    protected ParameterSet handleRequestEntity(Request request) {
-    	// the request handler will take care of the entity.
-    	return null;
-    }
 
     @Override
     protected Representation runCommand(Variant variant, Request request, MediaType outputFormat) {
+        // before running the Init command, extract the repository name from the request and see if
+        // a repository with that name already exists
+        final String repoName = RESTUtils.getStringAttribute(request, "repository");
+        if (repoName != null) {
+            if (RepositoryManager.get().getByRepoName(repoName) != null) {
+                // repo already exists
+                throw new CommandSpecException(
+                        "The specified repository name is already in use, please try a different name",
+                        Status.CLIENT_ERROR_CONFLICT);
+            }
+        }
         Representation representation = super.runCommand(variant, request, outputFormat);
 
         if (getResponse().getStatus() == Status.SUCCESS_CREATED) {

src/community/geogig/src/main/java/org/geogig/geoserver/web/repository/RepositoryEditPanel.java

@@ -100,11 +100,6 @@ public void convertInput() {
     }
 
     private boolean repoNameExists(String repoName) {
-        for (RepositoryInfo repo : RepositoryManager.get().getAll()) {
-            if (repo.getRepoName().equals(repoName)) {
-                return true;
-            }
-        }
-        return false;
+        return RepositoryManager.get().getByRepoName(repoName) != null;
     }
 }

src/community/geogig/src/main/java/org/geogig/geoserver/web/repository/RepositoryImportPanel.java

@@ -127,6 +127,12 @@ protected void onUpdate(AjaxRequestTarget target) {
             public void validate(IValidatable<RepositoryInfo> validatable) {
                 ValidationError error = new ValidationError();
                 RepositoryInfo repo = validatable.getValue();
+                // block duplicate names first
+                if (null != RepositoryManager.get().getByRepoName(repo.getRepoName())) {
+                    error.addKey("errRepositoryNameExists");
+                    validatable.error(error);
+                    return;
+                }
                 final URI location = repo.getLocation();
                 // look for already configured repos
                 List<RepositoryInfo> existingRepos = RepositoryManager.get().getAll();

src/community/geogig/src/test/java/org/geogig/geoserver/GeoGigTestData.java

@@ -124,6 +124,10 @@ public void setUp(String repoName) throws Exception {
         this.geogig = createRepository(repoName);
     }
 
+    public void setUp(String repoName, File root) throws Exception {
+        this.geogig = createRepository(repoName, root);
+    }
+
     @Override
     protected void after() {
         tearDown();
@@ -161,6 +165,19 @@ public GeoGIG createRepository(String name) {
         return Geogig;
     }
 
+    public GeoGIG createRepository(String name, File root) {
+        repoDir = new File(root, name);
+        Assert.assertTrue(repoDir.mkdir());
+
+        TestPlatform testPlatform = new TestPlatform(repoDir);
+        testPlatform.setUserHome(root);
+        GlobalContextBuilder.builder(new CLITestContextBuilder(testPlatform));
+        Context context = GlobalContextBuilder.builder().build();
+        GeoGIG Geogig = new GeoGIG(context);
+
+        return Geogig;
+    }
+
     public TemporaryFolder tmpFolder() {
         return this.tmpFolder;
     }

src/community/geogig/src/test/java/org/geogig/geoserver/config/RepositoryManagerTest.java

@@ -7,6 +7,7 @@
 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.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -144,13 +145,8 @@ public void testCreateAndGetRepos() throws IOException {
         assertEquals(info1, info1Get);
         assertEquals(info2, info2Get);
 
-        try {
-            repoManager.getByRepoName("nonexistent");
-            fail();
-        } catch (NoSuchElementException e) {
-            // expected;
-            assertEquals("No repository with name nonexistent exists", e.getMessage());
-        }
+        RepositoryInfo rpoByName = repoManager.getByRepoName("nonexistent");
+        assertNull("Expected repository to be non-existent", rpoByName);
     }
 
     @Test
@@ -319,13 +315,8 @@ public void testRenameRepository() throws IOException {
         infoGet = repoManager.getByRepoName("repo1_renamed");
         assertEquals(renamed, infoGet);
 
-        try {
-            repoManager.getByRepoName("repo1");
-            fail();
-        } catch (NoSuchElementException e) {
-            // expected;
-            assertEquals("No repository with name repo1 exists", e.getMessage());
-        }
+        RepositoryInfo repoByName = repoManager.getByRepoName("repo1");
+        assertNull("Expected \"repo1\" to be non-existent", repoByName);
     }
 
     @Test
@@ -439,13 +430,8 @@ public void testDeleteRepository() throws IOException {
         repositoryInfos = repoManager.getAll();
         assertEquals(0, repositoryInfos.size());
 
-        try {
-            repoManager.getByRepoName("repo1");
-            fail();
-        } catch (NoSuchElementException e) {
-            // expected;
-            assertEquals("No repository with name repo1 exists", e.getMessage());
-        }
+        RepositoryInfo repoByName = repoManager.getByRepoName("repo1");
+        assertNull("Expected \"repo1\" to be non-existent", repoByName);
 
         try {
             repoManager.getRepository(info.getId());
@@ -518,6 +504,20 @@ public void testPingRemote() throws Exception {
         }
     }
 
+    @Test
+    public void testCreate() throws Exception {
+        File repoFolder = new File(testData.getDataDirectoryRoot(), "someRepoName");
+        URI uri = repoFolder.toURI();
+        RepositoryInfo repoInfo = new RepositoryInfo();
+        repoInfo.setLocation(uri);
+        // now call save() with the RepositoryInfo
+        // since the repo doesn't exist, save() should try to create it
+        RepositoryInfo savedInfo = RepositoryManager.get().save(repoInfo);
+        assertNotNull(savedInfo);
+        // make sure it's retrievable as well
+        assertNotNull(RepositoryManager.get().getByRepoName("someRepoName"));
+    }
+
     private RepositoryInfo saveRepository(Repository repo) {
         RepositoryInfo repoInfo = new RepositoryInfo();
         URI location = repo.getLocation().normalize();

src/community/geogig/src/test/java/org/geogig/geoserver/functional/GeoServerFunctionalTestContext.java

@@ -255,6 +255,31 @@ public Repository getRepo(String name) {
         return repoProvider.getGeogig(name).orNull();
     }
 
+    /**
+     * Initialize a repository with the given name. Does not register it with the Catalog or
+     * associate it to a DataStore.
+     *
+     * @param name the repository name
+     *
+     * @throws Exception
+     */
+    void initRepo(String name) throws Exception {
+        testData.setUp(name);
+        testData.init().config("user.name", "John").config("user.email", "John.Doe@example.com");
+    }
+
+    @Override
+    protected TestData createUnmanagedRepo(String name) throws Exception {
+        initRepo(name);
+        return new TestData(testData.getGeogig());
+    }
+
+    protected TestData createUnManagedRepoWithAltRoot(String name) throws Exception {
+        File unmanagedRoot = testData.tmpFolder().newFolder("unmanagedRoot");
+        testData.setUp(name, unmanagedRoot);
+        testData.init().config("user.name", "John").config("user.email", "John.Doe@example.com");
+        return new TestData(testData.getGeogig());
+    }
     /**
      * Create a repository with the given name for testing.
      *
@@ -266,8 +291,7 @@ public Repository getRepo(String name) {
      */
     @Override
     protected TestData createRepo(String name) throws Exception {
-        testData.setUp(name);
-        testData.init().config("user.name", "John").config("user.email", "John.Doe@example.com");
+        initRepo(name);
         GeoGIG geogig = testData.getGeogig();
 
         Catalog catalog = helper.getCatalog();

src/community/geogig/src/test/java/org/geogig/geoserver/functional/PluginWebAPICucumberHooks.java

@@ -1,21 +1,18 @@
+/* (c) 2017 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
 package org.geogig.geoserver.functional;
 
+import java.io.IOException;
+
+import org.geogig.web.functional.WebAPICucumberHooks;
+import org.locationtech.geogig.repository.Repository;
+
 import com.google.inject.Inject;
 import cucumber.api.java.en.Given;
-import cucumber.api.java.en.When;
 import cucumber.runtime.java.StepDefAnnotation;
 import cucumber.runtime.java.guice.ScenarioScoped;
-import org.geogig.web.functional.WebAPICucumberHooks;
-import org.locationtech.geogig.web.api.TestData;
-import org.restlet.data.MediaType;
-import org.restlet.data.Method;
-
-import javax.json.Json;
-import javax.json.JsonException;
-import javax.json.JsonObject;
-import java.io.IOException;
-
-import static com.google.common.base.Preconditions.checkArgument;
 
 /**
  * Extensions to the GeoGig Web API Functional tests. These these are specific to the GeoServer
@@ -27,6 +24,8 @@ public class PluginWebAPICucumberHooks {
 
     public GeoServerFunctionalTestContext context;
 
+    private String repoName;
+
     /**
      * Create an instance of this set of Steps with the GeoGig Web API Hooks as a parent. Since you
      * cannot extend a Step Definition class, just inject the one that gets created during the test
@@ -46,8 +45,27 @@ String systemTempPath() throws IOException {
         return context.getTempFolder().getCanonicalPath().replace("\\", "/");
     }
 
-    @Given("^A repository named \"([^\"]*)\" is initialized$")
-    public void initEmptyRepo(String repoName) throws Exception {
-        context.createRepo(repoName);
+    @Given("I have \"([^\"]*)\" that is not managed by GeoServer$")
+    public void setupExtraUnMangedRepo(String repoName) throws Exception {
+        context.createUnManagedRepoWithAltRoot(repoName)
+                .init("geogigUser", "repo1_Owner@geogig.org")
+                .loadDefaultData()
+                .getRepo().close();
+        this.repoName = repoName;
+    }
+
+    @cucumber.api.java.After
+    public void after() {
+        if (repoName != null) {
+            try {
+                Repository repo = this.context.getRepo(repoName);
+                if (repo != null) {
+                    repo.close();
+                }
+            } catch (Exception ex) {
+                // repo doesn't exist
+            }
+        }
+        context.after();
     }
 }