Catch exception when repositories are deleted

Committed-by: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>
Co-authored-by: René Pfeuffer <rene.pfeuffer@cloudogu.com>

gradle/changelog/illegal_state_after_repository_deletion.yaml

@@ -0,0 +1,2 @@
+- type: fixed
+  description: Catch different exceptions after repositories are deleted

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<HalEnricher> 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);
+      }
     }
   }
 

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<String, T> consumer);
   }
+
+  public class LocationNotFoundException extends IllegalStateException {
+    public LocationNotFoundException(String repositoryId) {
+      super("location for repository " + repositoryId + " does not exist");
+    }
+  }
 }

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);

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
+  }
 }

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
   }
 }

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);
         }
       }
 

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<Repository, RepositoryDto> implements RepositoryToHalMapper {
 
   @Inject
@@ -169,6 +170,10 @@ public abstract class RepositoryToRepositoryDtoMapper extends BaseMapper<Reposit
           linksBuilder.single(link("incomingDiffParsed", resourceLinks.incoming().diffParsed(repository.getNamespace(), repository.getName())));
         }
       }
+    } catch (RepositoryLocationResolver.LocationNotFoundException e) {
+      // This might happen, when the repository has been deleted in the meantime
+      // There is nothing we can do here, so we just log this
+      log.warn("could not create repository service for repository; the repository might have been deleted", e);
     }
     linksBuilder.single(link("changesets", resourceLinks.changeset().all(repository.getNamespace(), repository.getName())));
     linksBuilder.single(link("sources", resourceLinks.source().selfWithoutRevision(repository.getNamespace(), repository.getName())));