diff --git a/gradle/changelog/illegal_state_after_repository_deletion.yaml b/gradle/changelog/illegal_state_after_repository_deletion.yaml new file mode 100644 index 0000000000..b8bd04844b --- /dev/null +++ b/gradle/changelog/illegal_state_after_repository_deletion.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Catch different exceptions after repositories are deleted diff --git a/scm-core/src/main/java/sonia/scm/api/v2/resources/HalAppenderMapper.java b/scm-core/src/main/java/sonia/scm/api/v2/resources/HalAppenderMapper.java index 4022a72e38..746f0b97d0 100644 --- a/scm-core/src/main/java/sonia/scm/api/v2/resources/HalAppenderMapper.java +++ b/scm-core/src/main/java/sonia/scm/api/v2/resources/HalAppenderMapper.java @@ -25,9 +25,11 @@ package sonia.scm.api.v2.resources; import com.google.common.annotations.VisibleForTesting; +import lombok.extern.slf4j.Slf4j; import javax.inject.Inject; +@Slf4j public class HalAppenderMapper { @Inject @@ -56,7 +58,11 @@ public class HalAppenderMapper { protected void applyEnrichers(HalEnricherContext context, HalAppender appender, Class type) { Iterable enrichers = registry.allByType(type); for (HalEnricher enricher : enrichers) { - enricher.enrich(context, appender); + try { + enricher.enrich(context, appender); + } catch (Exception e) { + log.warn("failed to enrich repository; it might be, that the repository has been deleted in the meantime", e); + } } } diff --git a/scm-core/src/main/java/sonia/scm/repository/RepositoryLocationResolver.java b/scm-core/src/main/java/sonia/scm/repository/RepositoryLocationResolver.java index 11ce195045..0cbe8b1e68 100644 --- a/scm-core/src/main/java/sonia/scm/repository/RepositoryLocationResolver.java +++ b/scm-core/src/main/java/sonia/scm/repository/RepositoryLocationResolver.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository; import java.util.function.BiConsumer; @@ -44,7 +44,7 @@ public abstract class RepositoryLocationResolver { /** * Get the existing location for the repository. * @param repositoryId The id of the repository. - * @throws IllegalStateException when there is no known location for the given repository. + * @throws LocationNotFoundException when there is no known location for the given repository. */ T getLocation(String repositoryId); @@ -69,4 +69,10 @@ public abstract class RepositoryLocationResolver { */ void forAllLocations(BiConsumer consumer); } + + public class LocationNotFoundException extends IllegalStateException { + public LocationNotFoundException(String repositoryId) { + super("location for repository " + repositoryId + " does not exist"); + } + } } diff --git a/scm-core/src/main/java/sonia/scm/web/AbstractRepositoryJsonEnricher.java b/scm-core/src/main/java/sonia/scm/web/AbstractRepositoryJsonEnricher.java index 7de8d66e56..c800d926a4 100644 --- a/scm-core/src/main/java/sonia/scm/web/AbstractRepositoryJsonEnricher.java +++ b/scm-core/src/main/java/sonia/scm/web/AbstractRepositoryJsonEnricher.java @@ -21,16 +21,18 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.extern.slf4j.Slf4j; import static java.util.Collections.singletonMap; import static sonia.scm.web.VndMediaType.REPOSITORY; import static sonia.scm.web.VndMediaType.REPOSITORY_COLLECTION; +@Slf4j public abstract class AbstractRepositoryJsonEnricher extends JsonEnricherBase { public AbstractRepositoryJsonEnricher(ObjectMapper objectMapper) { @@ -52,7 +54,11 @@ public abstract class AbstractRepositoryJsonEnricher extends JsonEnricherBase { String namespace = repositoryNode.get("namespace").asText(); String name = repositoryNode.get("name").asText(); - enrichRepositoryNode(repositoryNode, namespace, name); + try { + enrichRepositoryNode(repositoryNode, namespace, name); + } catch (Exception e) { + log.warn("failed to enrich repository; it might be, that the repository has been deleted in the meantime", e); + } } protected abstract void enrichRepositoryNode(JsonNode repositoryNode, String namespace, String name); diff --git a/scm-core/src/test/java/sonia/scm/api/v2/resources/HalAppenderMapperTest.java b/scm-core/src/test/java/sonia/scm/api/v2/resources/HalAppenderMapperTest.java index 928cd7c4f8..4ff4fe46ae 100644 --- a/scm-core/src/test/java/sonia/scm/api/v2/resources/HalAppenderMapperTest.java +++ b/scm-core/src/test/java/sonia/scm/api/v2/resources/HalAppenderMapperTest.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import org.junit.jupiter.api.BeforeEach; @@ -90,9 +90,19 @@ class HalAppenderMapperTest { appender.appendLink(String.valueOf(rel.get()), href.get()); }); - mapper.applyEnrichers(appender, Integer.valueOf(42), "https://hitchhiker.com"); + mapper.applyEnrichers(appender, 42, "https://hitchhiker.com"); verify(appender).appendLink("42", "https://hitchhiker.com"); } + @Test + void shouldHandleExceptionFromEnricher() { + registry.register(String.class, (ctx, appender) -> { + throw new RuntimeException("test"); + }); + + mapper.applyEnrichers(appender, "hello"); + + // no exception has been thrown + } } diff --git a/scm-core/src/test/java/sonia/scm/web/AbstractRepositoryJsonEnricherTest.java b/scm-core/src/test/java/sonia/scm/web/AbstractRepositoryJsonEnricherTest.java index 4fa71141f0..89d4d4b53e 100644 --- a/scm-core/src/test/java/sonia/scm/web/AbstractRepositoryJsonEnricherTest.java +++ b/scm-core/src/test/java/sonia/scm/web/AbstractRepositoryJsonEnricherTest.java @@ -21,13 +21,14 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.io.Resources; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; @@ -48,20 +49,102 @@ class AbstractRepositoryJsonEnricherTest { private JsonNode rootNode; @BeforeEach - void globalSetUp() { + void setUpPathInfoStore() { ScmPathInfoStore pathInfoStore = new ScmPathInfoStore(); pathInfoStore.set(() -> URI.create("/")); + } - linkEnricher = new AbstractRepositoryJsonEnricher(objectMapper) { - @Override - protected void enrichRepositoryNode(JsonNode repositoryNode, String namespace, String name) { - addLink(repositoryNode, "new-link", "/somewhere"); - } - }; + @Nested + class WithWorkingEnricher { + + @BeforeEach + void setUpEnricher() { + linkEnricher = new AbstractRepositoryJsonEnricher(objectMapper) { + @Override + protected void enrichRepositoryNode(JsonNode repositoryNode, String namespace, String name) { + addLink(repositoryNode, "new-link", "/somewhere"); + } + }; + } + + @Test + void shouldEnrichRepositories() throws IOException { + URL resource = Resources.getResource("sonia/scm/repository/repository-001.json"); + rootNode = objectMapper.readTree(resource); + + JsonEnricherContext context = new JsonEnricherContext( + URI.create("/"), + MediaType.valueOf(VndMediaType.REPOSITORY), + rootNode + ); + + linkEnricher.enrich(context); + + String configLink = context.getResponseEntity() + .get("_links") + .get("new-link") + .get("href") + .asText(); + + assertThat(configLink).isEqualTo("/somewhere"); + } + + @Test + void shouldEnrichAllRepositories() throws IOException { + URL resource = Resources.getResource("sonia/scm/repository/repository-collection-001.json"); + rootNode = objectMapper.readTree(resource); + + JsonEnricherContext context = new JsonEnricherContext( + URI.create("/"), + MediaType.valueOf(VndMediaType.REPOSITORY_COLLECTION), + rootNode + ); + + linkEnricher.enrich(context); + + context.getResponseEntity() + .get("_embedded") + .withArray("repositories") + .elements() + .forEachRemaining(node -> { + String configLink = node + .get("_links") + .get("new-link") + .get("href") + .asText(); + + assertThat(configLink).isEqualTo("/somewhere"); + }); + } + + @Test + void shouldNotModifyObjectsWithUnsupportedMediaType() throws IOException { + URL resource = Resources.getResource("sonia/scm/repository/repository-001.json"); + rootNode = objectMapper.readTree(resource); + JsonEnricherContext context = new JsonEnricherContext( + URI.create("/"), + MediaType.valueOf(VndMediaType.USER), + rootNode + ); + + linkEnricher.enrich(context); + + boolean hasNewPullRequestLink = context.getResponseEntity() + .get("_links") + .has("new-link"); + + assertThat(hasNewPullRequestLink).isFalse(); + } } @Test - void shouldEnrichRepositories() throws IOException { + void shouldHandleFailingEnricher() throws IOException { + linkEnricher = new AbstractRepositoryJsonEnricher(objectMapper) { + @Override + protected void enrichRepositoryNode(JsonNode repositoryNode, String namespace, String name) { + throw new NullPointerException(); + } + }; URL resource = Resources.getResource("sonia/scm/repository/repository-001.json"); rootNode = objectMapper.readTree(resource); @@ -73,59 +156,6 @@ class AbstractRepositoryJsonEnricherTest { linkEnricher.enrich(context); - String configLink = context.getResponseEntity() - .get("_links") - .get("new-link") - .get("href") - .asText(); - - assertThat(configLink).isEqualTo("/somewhere"); - } - - @Test - void shouldEnrichAllRepositories() throws IOException { - URL resource = Resources.getResource("sonia/scm/repository/repository-collection-001.json"); - rootNode = objectMapper.readTree(resource); - - JsonEnricherContext context = new JsonEnricherContext( - URI.create("/"), - MediaType.valueOf(VndMediaType.REPOSITORY_COLLECTION), - rootNode - ); - - linkEnricher.enrich(context); - - context.getResponseEntity() - .get("_embedded") - .withArray("repositories") - .elements() - .forEachRemaining(node -> { - String configLink = node - .get("_links") - .get("new-link") - .get("href") - .asText(); - - assertThat(configLink).isEqualTo("/somewhere"); - }); - } - - @Test - void shouldNotModifyObjectsWithUnsupportedMediaType() throws IOException { - URL resource = Resources.getResource("sonia/scm/repository/repository-001.json"); - rootNode = objectMapper.readTree(resource); - JsonEnricherContext context = new JsonEnricherContext( - URI.create("/"), - MediaType.valueOf(VndMediaType.USER), - rootNode - ); - - linkEnricher.enrich(context); - - boolean hasNewPullRequestLink = context.getResponseEntity() - .get("_links") - .has("new-link"); - - assertThat(hasNewPullRequestLink).isFalse(); + // no exception has been thrown } } diff --git a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java index 4f1bcbc7d3..2e0a5f0cda 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java +++ b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java @@ -97,7 +97,7 @@ public class PathBasedRepositoryLocationResolver extends BasicRepositoryLocation if (pathById.containsKey(repositoryId)) { return (T) contextProvider.resolve(pathById.get(repositoryId)); } else { - throw new IllegalStateException("location for repository " + repositoryId + " does not exist"); + throw new LocationNotFoundException(repositoryId); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryToRepositoryDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryToRepositoryDtoMapper.java index 13f14adbea..4eb3329fd6 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryToRepositoryDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryToRepositoryDtoMapper.java @@ -27,6 +27,7 @@ package sonia.scm.api.v2.resources; import de.otto.edison.hal.Embedded; import de.otto.edison.hal.Link; import de.otto.edison.hal.Links; +import lombok.extern.slf4j.Slf4j; import org.mapstruct.AfterMapping; import org.mapstruct.Mapper; import org.mapstruct.Mapping; @@ -34,13 +35,13 @@ import org.mapstruct.MappingTarget; import org.mapstruct.ObjectFactory; import sonia.scm.SCMContextProvider; import sonia.scm.admin.ScmConfigurationStore; -import sonia.scm.config.ScmConfiguration; import sonia.scm.repository.DefaultRepositoryExportingCheck; import sonia.scm.repository.Feature; import sonia.scm.repository.HealthCheckFailure; import sonia.scm.repository.HealthCheckService; import sonia.scm.repository.NamespaceStrategy; import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryLocationResolver; import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.api.Command; import sonia.scm.repository.api.RepositoryService; @@ -52,7 +53,6 @@ import sonia.scm.web.EdisonHalAppender; import sonia.scm.web.api.RepositoryToHalMapper; import javax.inject.Inject; -import javax.inject.Provider; import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -66,6 +66,7 @@ import static java.util.stream.Collectors.toList; // Mapstruct does not support parameterized (i.e. non-default) constructors. Thus, we need to use field injection. @SuppressWarnings("squid:S3306") @Mapper +@Slf4j public abstract class RepositoryToRepositoryDtoMapper extends BaseMapper implements RepositoryToHalMapper { @Inject @@ -169,6 +170,10 @@ public abstract class RepositoryToRepositoryDtoMapper extends BaseMapper