Enhance repository manager create api to perform actions after creation

scm-core/src/main/java/sonia/scm/repository/RepositoryManager.java

@@ -28,6 +28,7 @@ import sonia.scm.TypeManager;
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.function.Consumer;
 
 /**
  * The central class for managing {@link Repository} objects.
@@ -96,4 +97,11 @@ public interface RepositoryManager
    * @return all namespaces
    */
   Collection<String> getAllNamespaces();
+
+
+  default Repository create(Repository repository, Consumer<Repository> afterCreation) {
+    Repository newRepository = create(repository);
+    afterCreation.accept(newRepository);
+    return newRepository;
+  }
 }

scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryImportResource.java

@@ -24,6 +24,7 @@
 
 package sonia.scm.api.v2.resources;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.inject.Inject;
 import de.otto.edison.hal.Embedded;
@@ -49,7 +50,6 @@ import sonia.scm.repository.RepositoryManager;
 import sonia.scm.repository.RepositoryPermissions;
 import sonia.scm.repository.RepositoryType;
 import sonia.scm.repository.api.Command;
-import sonia.scm.repository.api.ImportFailedException;
 import sonia.scm.repository.api.PullCommandBuilder;
 import sonia.scm.repository.api.RepositoryService;
 import sonia.scm.repository.api.RepositoryServiceFactory;
@@ -63,8 +63,10 @@ import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
+import java.io.IOException;
 import java.net.URI;
 import java.util.Set;
+import java.util.function.Consumer;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -213,29 +215,33 @@ public class RepositoryImportResource {
 
     logger.info("start {} import for external url {}", type, request.getUrl());
 
-    Repository repository = create(request.getNamespace(), request.getName(), type);
-
-    try (RepositoryService service = serviceFactory.create(repository)) {
-      PullCommandBuilder pullCommand = service.getPullCommand();
-      if (!Strings.isNullOrEmpty(request.getUsername()) && !Strings.isNullOrEmpty(request.getPassword())) {
-        pullCommand
-          .withUsername(request.getUsername())
-          .withPassword(request.getPassword());
-      }
-
-      pullCommand.pull(request.getUrl());
-      eventBus.post(new RepositoryImportEvent(HandlerEventType.CREATE, repository, false));
-    } catch (ImportFailedException ex) {
-      handleImportFailure(ex, repository);
-      throw ex;
-    } catch (Exception ex) {
-      handleImportFailure(ex, repository);
-      throw new InternalRepositoryException(repository, "Repository Import failed.", ex);
-    }
+    Repository repository = manager.create(
+      new Repository(null, type, request.getNamespace(), request.getName()),
+      pullChangesFromRemoteUrl(request)
+    );
 
     return Response.created(URI.create(resourceLinks.repository().self(repository.getNamespace(), repository.getName()))).build();
   }
 
+  @VisibleForTesting
+  Consumer<Repository> pullChangesFromRemoteUrl(RepositoryImportDto request) {
+    return repository -> {
+      try (RepositoryService service = serviceFactory.create(repository)) {
+        PullCommandBuilder pullCommand = service.getPullCommand();
+        if (!Strings.isNullOrEmpty(request.getUsername()) && !Strings.isNullOrEmpty(request.getPassword())) {
+          pullCommand
+            .withUsername(request.getUsername())
+            .withPassword(request.getPassword());
+        }
+
+        pullCommand.pull(request.getUrl());
+        eventBus.post(new RepositoryImportEvent(HandlerEventType.CREATE, repository, false));
+      } catch (IOException e) {
+        throw new InternalRepositoryException(repository, "Failed to import from remote url", e);
+      }
+    };
+  }
+
 //  /**
 //   * Imports repositories of the given type from the configured repository
 //   * directory. <strong>Note:</strong> This method requires admin privileges.
@@ -390,27 +396,6 @@ public class RepositoryImportResource {
     }
   }
 
-  /**
-   * Creates a new repository with the given namespace, name and type.
-   *
-   * @param namespace repository namespace
-   * @param name      repository name
-   * @param type      repository type
-   * @return newly created repository
-   */
-  private Repository create(String namespace, String name, String type) {
-    Repository repository = null;
-
-    try {
-      repository = new Repository(null, type, namespace, name);
-      manager.create(repository);
-    } catch (InternalRepositoryException ex) {
-      handleGenericCreationFailure(ex, type, name);
-    }
-
-    return repository;
-  }
-
 //  /**
 //   * Start bundle import.
 //   *
@@ -513,7 +498,6 @@ public class RepositoryImportResource {
 
     try {
       eventBus.post(new RepositoryImportEvent(HandlerEventType.BEFORE_DELETE, repository, true));
-      manager.delete(repository);
     } catch (InternalRepositoryException | NotFoundException e) {
       logger.error("can not delete repository after import failure", e);
     }

scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java

@@ -59,6 +59,7 @@ import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadFactory;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 
 import static java.util.stream.Collectors.toSet;
@@ -123,10 +124,16 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
 
   @Override
   public Repository create(Repository repository) {
-    return create(repository, true);
+    return create(repository, r -> {
+    }, true);
   }
 
-  public Repository create(Repository repository, boolean initRepository) {
+  @Override
+  public Repository create(Repository repository, Consumer<Repository> afterCreation) {
+    return create(repository, afterCreation, true);
+  }
+
+  public Repository create(Repository repository, Consumer<Repository> afterCreation, boolean initRepository) {
     repository.setId(keyGenerator.createKey());
     repository.setNamespace(namespaceStrategyProvider.get().createNamespace(repository));
 
@@ -137,15 +144,19 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
       RepositoryPermissions::create,
       newRepository -> fireEvent(HandlerEventType.BEFORE_CREATE, newRepository),
       newRepository -> {
-        fireEvent(HandlerEventType.CREATE, newRepository);
         if (initRepository) {
           try {
             getHandler(newRepository).create(newRepository);
-          } catch (InternalRepositoryException e) {
+            afterCreation.accept(newRepository);
+            //TODO check if this is okay
+          } catch (Exception e) {
             delete(repository);
             throw e;
           }
+        } else {
+          afterCreation.accept(newRepository);
         }
+        fireEvent(HandlerEventType.CREATE, newRepository);
       },
       newRepository -> {
         if (repositoryDAO.contains(newRepository.getNamespaceAndName())) {
@@ -173,7 +184,8 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
 
   @Override
   public void importRepository(Repository repository) {
-    create(repository, false);
+    create(repository, r -> {
+    }, false);
   }
 
   @Override

scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java

@@ -39,11 +39,11 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import sonia.scm.HandlerEventType;
 import sonia.scm.PageResult;
 import sonia.scm.config.ScmConfiguration;
 import sonia.scm.event.ScmEventBus;
 import sonia.scm.repository.CustomNamespaceStrategy;
-import sonia.scm.repository.ImportHandler;
 import sonia.scm.repository.NamespaceAndName;
 import sonia.scm.repository.NamespaceStrategy;
 import sonia.scm.repository.Repository;
@@ -51,6 +51,7 @@ import sonia.scm.repository.RepositoryHandler;
 import sonia.scm.repository.RepositoryImportEvent;
 import sonia.scm.repository.RepositoryInitializer;
 import sonia.scm.repository.RepositoryManager;
+import sonia.scm.repository.RepositoryTestData;
 import sonia.scm.repository.RepositoryType;
 import sonia.scm.repository.api.Command;
 import sonia.scm.repository.api.ImportFailedException;
@@ -68,6 +69,7 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 
 import static java.util.Collections.singletonList;
@@ -85,7 +87,6 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyMap;
 import static org.mockito.ArgumentMatchers.anyObject;
-import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.RETURNS_SELF;
 import static org.mockito.Mockito.doReturn;
@@ -119,8 +120,6 @@ public class RepositoryRootResourceTest extends RepositoryTestBase {
   @Mock
   private RepositoryHandler repositoryHandler;
   @Mock
-  private ImportHandler importHandler;
-  @Mock
   private ScmPathInfoStore scmPathInfoStore;
   @Mock
   private ScmPathInfo uriInfo;
@@ -466,9 +465,8 @@ public class RepositoryRootResourceTest extends RepositoryTestBase {
   @Test
   public void shouldImportRepositoryFromUrl() throws URISyntaxException, IOException {
     when(manager.getHandler("git")).thenReturn(repositoryHandler);
-    when(repositoryHandler.getImportHandler()).thenReturn(importHandler);
     when(repositoryHandler.getType()).thenReturn(new RepositoryType("git", "git", ImmutableSet.of(Command.PULL)));
-    when(service.getPullCommand()).thenReturn(mock(PullCommandBuilder.class, RETURNS_SELF));
+    when(manager.create(any(Repository.class), any())).thenReturn(RepositoryTestData.create42Puzzle());
 
     URL url = Resources.getResource("sonia/scm/api/v2/import-repo.json");
     byte[] importRequest = Resources.toByteArray(url);
@@ -487,11 +485,8 @@ public class RepositoryRootResourceTest extends RepositoryTestBase {
   @Test
   public void shouldImportRepositoryFromUrlWithCredentials() throws URISyntaxException, IOException {
     when(manager.getHandler("git")).thenReturn(repositoryHandler);
-    when(repositoryHandler.getImportHandler()).thenReturn(importHandler);
     when(repositoryHandler.getType()).thenReturn(new RepositoryType("git", "git", ImmutableSet.of(Command.PULL)));
-    PullCommandBuilder pullCommandBuilder = mock(PullCommandBuilder.class, RETURNS_SELF);
-    when(service.getPullCommand()).thenReturn(pullCommandBuilder);
-
+    when(manager.create(any(Repository.class), any())).thenReturn(RepositoryTestData.create42Puzzle());
 
     URL url = Resources.getResource("sonia/scm/api/v2/import-repo-with-credentials.json");
     byte[] importRequest = Resources.toByteArray(url);
@@ -505,24 +500,13 @@ public class RepositoryRootResourceTest extends RepositoryTestBase {
     dispatcher.invoke(request, response);
 
     assertEquals(SC_CREATED, response.getStatus());
-    verify(pullCommandBuilder).withUsername("trillian");
-    verify(pullCommandBuilder).withPassword("secret");
-
-    ArgumentCaptor<RepositoryImportEvent> eventCaptor = ArgumentCaptor.forClass(RepositoryImportEvent.class);
-    verify(eventBus).post(eventCaptor.capture());
-
-    assertThat(eventCaptor.getValue().isFailed()).isFalse();
   }
 
   @Test
   public void shouldFailOnImportRepositoryFromUrl() throws URISyntaxException, IOException {
     when(manager.getHandler("git")).thenReturn(repositoryHandler);
-    when(repositoryHandler.getImportHandler()).thenReturn(importHandler);
     when(repositoryHandler.getType()).thenReturn(new RepositoryType("git", "git", ImmutableSet.of(Command.PULL)));
-    PullCommandBuilder pullCommandBuilder = mock(PullCommandBuilder.class, RETURNS_SELF);
-    when(service.getPullCommand()).thenReturn(pullCommandBuilder);
-    doThrow(ImportFailedException.class).when(pullCommandBuilder).pull(anyString());
-
+    doThrow(ImportFailedException.class).when(manager).create(any(Repository.class), any());
 
     URL url = Resources.getResource("sonia/scm/api/v2/import-repo.json");
     byte[] importRequest = Resources.toByteArray(url);
@@ -536,11 +520,44 @@ public class RepositoryRootResourceTest extends RepositoryTestBase {
     dispatcher.invoke(request, response);
 
     assertEquals(500, response.getStatus());
+  }
 
-    ArgumentCaptor<RepositoryImportEvent> eventCaptor = ArgumentCaptor.forClass(RepositoryImportEvent.class);
-    verify(eventBus).post(eventCaptor.capture());
+  @Test
+  public void shouldPullChangesFromRemoteUrl() {
+    PullCommandBuilder pullCommandBuilder = mock(PullCommandBuilder.class, RETURNS_SELF);
+    when(service.getPullCommand()).thenReturn(pullCommandBuilder);
 
-    assertThat(eventCaptor.getValue().isFailed()).isTrue();
+    Repository repository = RepositoryTestData.createHeartOfGold();
+    RepositoryImportResource.RepositoryImportDto repositoryImportDto = new RepositoryImportResource.RepositoryImportDto();
+    repositoryImportDto.setUrl("https://scm-manager.org/scm/repo/scmadmin/scm-manager.git");
+    repositoryImportDto.setNamespace("scmadmin");
+    repositoryImportDto.setName("scm-manager");
+
+    Consumer<Repository> repositoryConsumer = repositoryImportResource.pullChangesFromRemoteUrl(repositoryImportDto);
+    repositoryConsumer.accept(repository);
+
+    verify(eventBus).post(new RepositoryImportEvent(HandlerEventType.CREATE, repository, false));
+  }
+
+  @Test
+  public void shouldPullChangesFromRemoteUrlWithCredentials() {
+    PullCommandBuilder pullCommandBuilder = mock(PullCommandBuilder.class, RETURNS_SELF);
+    when(service.getPullCommand()).thenReturn(pullCommandBuilder);
+
+    Repository repository = RepositoryTestData.createHeartOfGold();
+    RepositoryImportResource.RepositoryImportDto repositoryImportDto = new RepositoryImportResource.RepositoryImportDto();
+    repositoryImportDto.setUrl("https://scm-manager.org/scm/repo/scmadmin/scm-manager.git");
+    repositoryImportDto.setNamespace("scmadmin");
+    repositoryImportDto.setName("scm-manager");
+    repositoryImportDto.setUsername("trillian");
+    repositoryImportDto.setPassword("secret");
+
+    Consumer<Repository> repositoryConsumer = repositoryImportResource.pullChangesFromRemoteUrl(repositoryImportDto);
+    repositoryConsumer.accept(repository);
+
+    verify(pullCommandBuilder).withUsername("trillian");
+    verify(pullCommandBuilder).withPassword("secret");
+    verify(eventBus).post(new RepositoryImportEvent(HandlerEventType.CREATE, repository, false));
   }
 
   private PageResult<Repository> createSingletonPageResult(Repository repository) {