From d42560aba3e5327956e3120e63beeeb3b8f84310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 24 Feb 2021 10:43:13 +0100 Subject: [PATCH] Add log id to imported event --- .../scm/repository/RepositoryImportEvent.java | 4 +- .../scm/importexport/FromBundleImporter.java | 5 +- .../scm/importexport/FromUrlImporter.java | 31 ++++--- .../FullScmRepositoryImporter.java | 10 ++- .../sonia/scm/importexport/ImportState.java | 1 + .../importexport/RepositoryImportLogger.java | 3 + .../RepositoryTypeSupportChecker.java | 18 ++++- .../main/resources/locales/de/plugins.json | 4 + .../main/resources/locales/en/plugins.json | 4 + .../scm/importexport/FromUrlImporterTest.java | 80 ++++++++++++++++--- .../FullScmRepositoryImporterTest.java | 37 +++++++-- 11 files changed, 156 insertions(+), 41 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/RepositoryImportEvent.java b/scm-core/src/main/java/sonia/scm/repository/RepositoryImportEvent.java index a8fefefef0..737d95ff1f 100644 --- a/scm-core/src/main/java/sonia/scm/repository/RepositoryImportEvent.java +++ b/scm-core/src/main/java/sonia/scm/repository/RepositoryImportEvent.java @@ -39,10 +39,12 @@ import sonia.scm.event.Event; @EqualsAndHashCode(callSuper = true) public class RepositoryImportEvent extends RepositoryEvent { + private final String logId; private final boolean failed; - public RepositoryImportEvent(HandlerEventType eventType, Repository repository, boolean failed) { + public RepositoryImportEvent(HandlerEventType eventType, Repository repository, String logId, boolean failed) { super(eventType, repository); + this.logId = logId; this.failed = failed; } } diff --git a/scm-webapp/src/main/java/sonia/scm/importexport/FromBundleImporter.java b/scm-webapp/src/main/java/sonia/scm/importexport/FromBundleImporter.java index c2fd6afdd5..be9533763d 100644 --- a/scm-webapp/src/main/java/sonia/scm/importexport/FromBundleImporter.java +++ b/scm-webapp/src/main/java/sonia/scm/importexport/FromBundleImporter.java @@ -24,7 +24,6 @@ package sonia.scm.importexport; -import com.google.common.annotations.VisibleForTesting; import com.google.common.io.Files; import org.apache.shiro.SecurityUtils; import org.slf4j.Logger; @@ -90,13 +89,13 @@ public class FromBundleImporter { logger.start(DUMP, repository); repository = manager.create(repository, unbundleImport(inputStream, compressed, logger)); logger.finished(); - eventBus.post(new RepositoryImportEvent(HandlerEventType.MODIFY, repository, false)); } catch (Exception e) { logger.failed(e); - eventBus.post(new RepositoryImportEvent(HandlerEventType.MODIFY, repository, true)); + eventBus.post(new RepositoryImportEvent(HandlerEventType.CREATE, repository, logger.getLogId(), true)); throw e; } + eventBus.post(new RepositoryImportEvent(HandlerEventType.CREATE, repository, logger.getLogId(), false)); return repository; } diff --git a/scm-webapp/src/main/java/sonia/scm/importexport/FromUrlImporter.java b/scm-webapp/src/main/java/sonia/scm/importexport/FromUrlImporter.java index d3c4bdbb4f..09d7c7c7e4 100644 --- a/scm-webapp/src/main/java/sonia/scm/importexport/FromUrlImporter.java +++ b/scm-webapp/src/main/java/sonia/scm/importexport/FromUrlImporter.java @@ -24,7 +24,6 @@ package sonia.scm.importexport; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import lombok.Getter; import lombok.Setter; @@ -41,6 +40,7 @@ import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.RepositoryPermission; import sonia.scm.repository.RepositoryPermissions; 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; @@ -50,6 +50,7 @@ import java.io.IOException; import java.util.function.Consumer; import static java.util.Collections.singletonList; +import static sonia.scm.ContextEntry.ContextBuilder.noContext; import static sonia.scm.importexport.RepositoryTypeSupportChecker.checkSupport; import static sonia.scm.importexport.RepositoryTypeSupportChecker.type; @@ -79,24 +80,26 @@ public class FromUrlImporter { repository.setPermissions(singletonList(new RepositoryPermission(SecurityUtils.getSubject().getPrincipal().toString(), "OWNER", false))); + RepositoryImportLogger logger = loggerFactory.createLogger(); + logger.start(RepositoryImportLog.ImportType.URL, repository); + Repository createdRepository; try { - repository = manager.create( + logger.step("creating repository"); + createdRepository = manager.create( repository, - pullChangesFromRemoteUrl(parameters) + pullChangesFromRemoteUrl(parameters, logger) ); - eventBus.post(new RepositoryImportEvent(HandlerEventType.MODIFY, repository, false)); } catch (Exception e) { - eventBus.post(new RepositoryImportEvent(HandlerEventType.MODIFY, repository, true)); - throw e; + logger.failed(e); + eventBus.post(new RepositoryImportEvent(HandlerEventType.CREATE, repository, logger.getLogId(), true)); + throw new ImportFailedException(noContext(), "Could not import repository from url " + parameters.getImportUrl(), e); } - return repository; + eventBus.post(new RepositoryImportEvent(HandlerEventType.CREATE, createdRepository, logger.getLogId(), false)); + return createdRepository; } - @VisibleForTesting - Consumer pullChangesFromRemoteUrl(RepositoryImportParameters parameters) { + private Consumer pullChangesFromRemoteUrl(RepositoryImportParameters parameters, RepositoryImportLogger logger) { return repository -> { - RepositoryImportLogger logger = loggerFactory.createLogger(); - logger.start(RepositoryImportLog.ImportType.URL, repository); try (RepositoryService service = serviceFactory.create(repository)) { PullCommandBuilder pullCommand = service.getPullCommand(); if (!Strings.isNullOrEmpty(parameters.getUsername()) && !Strings.isNullOrEmpty(parameters.getPassword())) { @@ -110,11 +113,7 @@ public class FromUrlImporter { pullCommand.pull(parameters.getImportUrl()); logger.finished(); } catch (IOException e) { - logger.failed(e); - throw new InternalRepositoryException(repository, "Failed to import from remote url", e); - } catch (RuntimeException e) { - logger.failed(e); - throw e; + throw new InternalRepositoryException(repository, "Failed to import from remote url: " + e.getMessage(), e); } }; } diff --git a/scm-webapp/src/main/java/sonia/scm/importexport/FullScmRepositoryImporter.java b/scm-webapp/src/main/java/sonia/scm/importexport/FullScmRepositoryImporter.java index 9286cb7d3e..a225ee964f 100644 --- a/scm-webapp/src/main/java/sonia/scm/importexport/FullScmRepositoryImporter.java +++ b/scm-webapp/src/main/java/sonia/scm/importexport/FullScmRepositoryImporter.java @@ -31,8 +31,10 @@ import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.ContextEntry; +import sonia.scm.HandlerEventType; import sonia.scm.event.ScmEventBus; import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryImportEvent; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.api.ImportFailedException; @@ -43,9 +45,9 @@ import java.io.IOException; import java.io.InputStream; import static java.util.Arrays.stream; +import static sonia.scm.ContextEntry.ContextBuilder.noContext; import static sonia.scm.importexport.RepositoryImportLog.ImportType.FULL; import static sonia.scm.util.Archives.createTarInputStream; -import static sonia.scm.ContextEntry.ContextBuilder.noContext; public class FullScmRepositoryImporter { @@ -65,13 +67,12 @@ public class FullScmRepositoryImporter { RepositoryManager repositoryManager, RepositoryImportExportEncryption repositoryImportExportEncryption, RepositoryImportLoggerFactory loggerFactory, - ScmEventBus eventBus - ) { + ScmEventBus eventBus) { this.repositoryManager = repositoryManager; this.loggerFactory = loggerFactory; this.repositoryImportExportEncryption = repositoryImportExportEncryption; - importSteps = new ImportStep[]{environmentCheckStep, metadataImportStep, storeImportStep, repositoryImportStep}; this.eventBus = eventBus; + importSteps = new ImportStep[]{environmentCheckStep, metadataImportStep, storeImportStep, repositoryImportStep}; } public Repository importFromStream(Repository repository, InputStream inputStream, String password) { @@ -139,6 +140,7 @@ public class FullScmRepositoryImporter { } else { // Delete the repository if any error occurs during the import repositoryManager.delete(state.getRepository()); + eventBus.post(new RepositoryImportEvent(HandlerEventType.CREATE, repository, logger.getLogId(), true)); } } diff --git a/scm-webapp/src/main/java/sonia/scm/importexport/ImportState.java b/scm-webapp/src/main/java/sonia/scm/importexport/ImportState.java index bc268b132d..b0a80dafbe 100644 --- a/scm-webapp/src/main/java/sonia/scm/importexport/ImportState.java +++ b/scm-webapp/src/main/java/sonia/scm/importexport/ImportState.java @@ -102,6 +102,7 @@ class ImportState { public void addPendingEvent(Object event) { this.pendingEvents.add(event); } + RepositoryImportLogger getLogger() { return logger; } diff --git a/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryImportLogger.java b/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryImportLogger.java index f586ef7da1..7e72d70611 100644 --- a/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryImportLogger.java +++ b/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryImportLogger.java @@ -54,6 +54,9 @@ class RepositoryImportLogger { addLogEntry(new RepositoryImportLog.Entry("import started")); } + public String getLogId() { + return logId; + } public void finished() { log.setSuccess(true); diff --git a/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryTypeSupportChecker.java b/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryTypeSupportChecker.java index fc9345427c..87f55a46b6 100644 --- a/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryTypeSupportChecker.java +++ b/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryTypeSupportChecker.java @@ -26,6 +26,7 @@ package sonia.scm.importexport; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.BadRequestException; import sonia.scm.Type; import sonia.scm.repository.RepositoryHandler; import sonia.scm.repository.RepositoryManager; @@ -36,6 +37,8 @@ import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; import java.util.Set; +import static sonia.scm.ContextEntry.ContextBuilder.noContext; + public class RepositoryTypeSupportChecker { private RepositoryTypeSupportChecker() { @@ -60,7 +63,7 @@ public class RepositoryTypeSupportChecker { logger.warn("type {} does not support this command {}", type.getName(), cmd.name()); - throw new WebApplicationException(Response.Status.BAD_REQUEST); + throw new IllegalTypeForImportException("type does not support command"); } } @@ -69,8 +72,19 @@ public class RepositoryTypeSupportChecker { RepositoryHandler handler = manager.getHandler(type); if (handler == null) { logger.warn("no handler for type {} found", type); - throw new WebApplicationException(Response.Status.NOT_FOUND); + throw new IllegalTypeForImportException("unsupported repository type: " + type); } return handler.getType(); } + + private static class IllegalTypeForImportException extends BadRequestException { + public IllegalTypeForImportException(String message) { + super(noContext(), message); + } + + @Override + public String getCode() { + return "CISPvega31"; + } + } } diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 57b446d24f..b9c8adfe8b 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -338,6 +338,10 @@ "5GSO9ZkzX1": { "displayName": "Inkompatible Umgebung", "description": "Die Version dieses SCM-Managers oder eines der installierten Plugins ist zu alt für den Import des Dumps. Bitte installieren Sie die neuesten Versionen. Nähere Informationen finden sich im Log." + }, + "CISPvega31": { + "displayName": "Ungültiger Repository-Typ für Import", + "description": "Der Import ist für den gegebenen Repository-Typen nicht möglich." } }, "namespaceStrategies": { diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index a3a2d7ea51..6a2487d09c 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -338,6 +338,10 @@ "8YR7aawFW1": { "displayName": "Wrong current password", "description": "The current password is wrong. Please try again." + }, + "CISPvega31": { + "displayName": "Illegal repository type for import", + "description": "The import is not possible for the given repository type." } }, "namespaceStrategies": { diff --git a/scm-webapp/src/test/java/sonia/scm/importexport/FromUrlImporterTest.java b/scm-webapp/src/test/java/sonia/scm/importexport/FromUrlImporterTest.java index e221548ce8..3e83d7a351 100644 --- a/scm-webapp/src/test/java/sonia/scm/importexport/FromUrlImporterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/importexport/FromUrlImporterTest.java @@ -24,6 +24,9 @@ package sonia.scm.importexport; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -32,8 +35,11 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.event.ScmEventBus; import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryHandler; +import sonia.scm.repository.RepositoryImportEvent; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.RepositoryTestData; +import sonia.scm.repository.RepositoryType; import sonia.scm.repository.api.ImportFailedException; import sonia.scm.repository.api.PullCommandBuilder; import sonia.scm.repository.api.RepositoryService; @@ -42,14 +48,18 @@ import sonia.scm.repository.api.RepositoryServiceFactory; import java.io.IOException; import java.util.function.Consumer; +import static java.util.Collections.singleton; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.RETURNS_SELF; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static sonia.scm.repository.api.Command.PULL; @ExtendWith(MockitoExtension.class) class FromUrlImporterTest { @@ -66,14 +76,47 @@ class FromUrlImporterTest { private RepositoryImportLoggerFactory loggerFactory; @Mock private RepositoryImportLogger logger; + @Mock + private Subject subject; @InjectMocks private FromUrlImporter importer; + private final Repository repository = RepositoryTestData.createHeartOfGold("git"); + @BeforeEach void setUpMocks() { when(serviceFactory.create(any(Repository.class))).thenReturn(service); when(loggerFactory.createLogger()).thenReturn(logger); + when(manager.create(any(), any())).thenAnswer( + invocation -> { + Repository repository = invocation.getArgument(0, Repository.class); + Repository createdRepository = repository.clone(); + createdRepository.setNamespace("created"); + invocation.getArgument(1, Consumer.class).accept(createdRepository); + return createdRepository; + } + ); + } + + @BeforeEach + void setUpRepositoryType() { + RepositoryHandler repositoryHandler = mock(RepositoryHandler.class); + when(manager.getHandler(repository.getType())).thenReturn(repositoryHandler); + RepositoryType repositoryType = mock(RepositoryType.class); + when(repositoryHandler.getType()).thenReturn(repositoryType); + when(repositoryType.getSupportedCommands()).thenReturn(singleton(PULL)); + } + + @BeforeEach + void mockSubject() { + ThreadContext.bind(subject); + when(subject.getPrincipal()).thenReturn("trillian"); + } + + @AfterEach + void cleanupSubject() { + ThreadContext.unbindSubject(); } @Test @@ -81,14 +124,23 @@ class FromUrlImporterTest { PullCommandBuilder pullCommandBuilder = mock(PullCommandBuilder.class, RETURNS_SELF); when(service.getPullCommand()).thenReturn(pullCommandBuilder); - Repository repository = RepositoryTestData.createHeartOfGold(); FromUrlImporter.RepositoryImportParameters parameters = new FromUrlImporter.RepositoryImportParameters(); parameters.setImportUrl("https://scm-manager.org/scm/repo/scmadmin/scm-manager.git"); - Consumer repositoryConsumer = importer.pullChangesFromRemoteUrl(parameters); - repositoryConsumer.accept(repository); + Repository createdRepository = importer.importFromUrl(parameters, repository); + assertThat(createdRepository.getNamespace()).isEqualTo("created"); verify(pullCommandBuilder).pull("https://scm-manager.org/scm/repo/scmadmin/scm-manager.git"); + verify(logger).finished(); + verify(eventBus).post(argThat( + event -> { + assertThat(event).isInstanceOf(RepositoryImportEvent.class); + RepositoryImportEvent repositoryImportEvent = (RepositoryImportEvent) event; + assertThat(repositoryImportEvent.getItem().getNamespace()).isEqualTo("created"); + assertThat(repositoryImportEvent.isFailed()).isFalse(); + return true; + } + )); } @Test @@ -96,14 +148,12 @@ class FromUrlImporterTest { PullCommandBuilder pullCommandBuilder = mock(PullCommandBuilder.class, RETURNS_SELF); when(service.getPullCommand()).thenReturn(pullCommandBuilder); - Repository repository = RepositoryTestData.createHeartOfGold(); FromUrlImporter.RepositoryImportParameters parameters = new FromUrlImporter.RepositoryImportParameters(); parameters.setImportUrl("https://scm-manager.org/scm/repo/scmadmin/scm-manager.git"); parameters.setUsername("trillian"); parameters.setPassword("secret"); - Consumer repositoryConsumer = importer.pullChangesFromRemoteUrl(parameters); - repositoryConsumer.accept(repository); + importer.importFromUrl(parameters, repository); verify(pullCommandBuilder).withUsername("trillian"); verify(pullCommandBuilder).withPassword("secret"); @@ -113,13 +163,23 @@ class FromUrlImporterTest { void shouldThrowImportFailedEvent() throws IOException { PullCommandBuilder pullCommandBuilder = mock(PullCommandBuilder.class, RETURNS_SELF); when(service.getPullCommand()).thenReturn(pullCommandBuilder); - doThrow(ImportFailedException.class).when(pullCommandBuilder).pull(anyString()); + doThrow(TestException.class).when(pullCommandBuilder).pull(anyString()); - Repository repository = RepositoryTestData.createHeartOfGold(); FromUrlImporter.RepositoryImportParameters parameters = new FromUrlImporter.RepositoryImportParameters(); parameters.setImportUrl("https://scm-manager.org/scm/repo/scmadmin/scm-manager.git"); - Consumer repositoryConsumer = importer.pullChangesFromRemoteUrl(parameters); - assertThrows(ImportFailedException.class, () -> repositoryConsumer.accept(repository)); + assertThrows(ImportFailedException.class, () -> importer.importFromUrl(parameters, repository)); + verify(logger).failed(argThat(e -> e instanceof TestException)); + verify(eventBus).post(argThat( + event -> { + assertThat(event).isInstanceOf(RepositoryImportEvent.class); + RepositoryImportEvent repositoryImportEvent = (RepositoryImportEvent) event; + assertThat(repositoryImportEvent.getItem()).isEqualTo(repository); + assertThat(repositoryImportEvent.isFailed()).isTrue(); + return true; + } + )); } + + private static class TestException extends RuntimeException {} } diff --git a/scm-webapp/src/test/java/sonia/scm/importexport/FullScmRepositoryImporterTest.java b/scm-webapp/src/test/java/sonia/scm/importexport/FullScmRepositoryImporterTest.java index fc9f5e758f..b83a114626 100644 --- a/scm-webapp/src/test/java/sonia/scm/importexport/FullScmRepositoryImporterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/importexport/FullScmRepositoryImporterTest.java @@ -28,6 +28,7 @@ import com.google.common.io.Resources; import org.apache.shiro.authz.AuthorizationException; import org.apache.shiro.subject.Subject; import org.apache.shiro.util.ThreadContext; +import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -43,6 +44,7 @@ import sonia.scm.event.ScmEventBus; import sonia.scm.repository.ImportRepositoryHookEvent; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryHookEvent; +import sonia.scm.repository.RepositoryImportEvent; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.RepositoryPermission; import sonia.scm.repository.RepositoryTestData; @@ -104,6 +106,8 @@ class FullScmRepositoryImporterTest { private RepositoryImportLoggerFactory loggerFactory; @Mock private Subject subject; + @Mock + private ScmEventBus eventBus; @InjectMocks private EnvironmentCheckStep environmentCheckStep; @@ -114,9 +118,6 @@ class FullScmRepositoryImporterTest { @InjectMocks private RepositoryImportStep repositoryImportStep; - @Mock - private ScmEventBus eventBus; - @Mock private RepositoryHookEvent event; @@ -135,8 +136,7 @@ class FullScmRepositoryImporterTest { repositoryManager, repositoryImportExportEncryption, loggerFactory, - eventBus - ); + eventBus); } @BeforeEach @@ -176,6 +176,16 @@ class FullScmRepositoryImporterTest { IncompatibleEnvironmentForImportException.class, () -> fullImporter.importFromStream(REPOSITORY, importStream, "") ); + + verify(eventBus).post(argThat( + event -> { + assertThat(event).isInstanceOf(RepositoryImportEvent.class); + RepositoryImportEvent repositoryImportEvent = (RepositoryImportEvent) event; + assertThat(repositoryImportEvent.getItem()).isEqualTo(REPOSITORY); + assertThat(repositoryImportEvent.isFailed()).isTrue(); + return true; + } + )); } @Test @@ -226,6 +236,23 @@ class FullScmRepositoryImporterTest { assertThat(workDirExists).isFalse(); } + @Test + void shouldSendImportedEventForImportedRepository() throws IOException { + InputStream stream = Resources.getResource("sonia/scm/repository/import/scm-import.tar.gz").openStream(); + + fullImporter.importFromStream(REPOSITORY, stream, null); + + verify(eventBus).post(argThat( + event -> { + assertThat(event).isInstanceOf(RepositoryImportEvent.class); + RepositoryImportEvent repositoryImportEvent = (RepositoryImportEvent) event; + assertThat(repositoryImportEvent.getItem()).isEqualTo(REPOSITORY); + assertThat(repositoryImportEvent.isFailed()).isFalse(); + return true; + } + )); + } + @Test void shouldTriggerUpdateForImportedRepository() throws IOException { InputStream stream = Resources.getResource("sonia/scm/repository/import/scm-import.tar.gz").openStream();