Store repository id in git config for each repository

This is needed after migration from v1 to v2 and is done in
GitV1UpdateStep.java. Therefore we hat to make the 'forAllPaths' method
in PathBasedRepositoryLocationResolver available in the interface of
RepositoryLocationResolver.

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

@@ -1,5 +1,7 @@
 package sonia.scm.repository;
 
+import java.util.function.BiConsumer;
+
 public abstract class RepositoryLocationResolver {
 
   public abstract boolean supportsLocationType(Class<?> type);
@@ -35,5 +37,12 @@ public abstract class RepositoryLocationResolver {
      * @throws IllegalStateException when there already is a location for the given repository registered.
      */
     void setLocation(String repositoryId, T location);
+
+    /**
+     * Iterates all repository locations known to this resolver instance and calls the consumer giving the repository id
+     * and its location for each repository.
+     * @param consumer This callback will be called for each repository with the repository id and its location.
+     */
+    void forAllLocations(BiConsumer<String, T> consumer);
   }
 }

scm-core/src/main/java/sonia/scm/update/UpdateStepRepositoryMetadataAccess.java

@@ -0,0 +1,11 @@
+package sonia.scm.update;
+
+import sonia.scm.repository.Repository;
+
+/**
+ * Use this in {@link sonia.scm.migration.UpdateStep}s only to read repository objects directly from locations given by
+ * {@link sonia.scm.repository.RepositoryLocationResolver}.
+ */
+public interface UpdateStepRepositoryMetadataAccess<T> {
+  Repository read(T location);
+}

scm-dao-xml/src/main/java/sonia/scm/repository/xml/MetadataStore.java

@@ -5,19 +5,21 @@ import org.slf4j.LoggerFactory;
 import sonia.scm.ContextEntry;
 import sonia.scm.repository.InternalRepositoryException;
 import sonia.scm.repository.Repository;
+import sonia.scm.store.StoreConstants;
+import sonia.scm.update.UpdateStepRepositoryMetadataAccess;
 
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
 import javax.xml.bind.Marshaller;
 import java.nio.file.Path;
 
-class MetadataStore {
+public class MetadataStore implements UpdateStepRepositoryMetadataAccess<Path> {
 
   private static final Logger LOG = LoggerFactory.getLogger(MetadataStore.class);
 
   private final JAXBContext jaxbContext;
 
-  MetadataStore() {
+  public MetadataStore() {
     try {
        jaxbContext = JAXBContext.newInstance(Repository.class);
     } catch (JAXBException ex) {
@@ -25,10 +27,10 @@ class MetadataStore {
     }
   }
 
-  Repository read(Path path) {
+  public Repository read(Path path) {
     LOG.trace("read repository metadata from {}", path);
     try {
-      return (Repository) jaxbContext.createUnmarshaller().unmarshal(path.toFile());
+      return (Repository) jaxbContext.createUnmarshaller().unmarshal(resolveDataPath(path).toFile());
     } catch (JAXBException ex) {
       throw new InternalRepositoryException(
         ContextEntry.ContextBuilder.entity(Path.class, path.toString()).build(), "failed read repository metadata", ex
@@ -41,10 +43,13 @@ class MetadataStore {
     try {
       Marshaller marshaller = jaxbContext.createMarshaller();
       marshaller.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, Boolean.TRUE);
-      marshaller.marshal(repository, path.toFile());
+      marshaller.marshal(repository, resolveDataPath(path).toFile());
     } catch (JAXBException ex) {
       throw new InternalRepositoryException(repository, "failed write repository metadata", ex);
     }
   }
 
+  private Path resolveDataPath(Path repositoryPath) {
+    return repositoryPath.resolve(StoreConstants.REPOSITORY_METADATA.concat(StoreConstants.FILE_EXTENSION));
+  }
 }

scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java

@@ -94,6 +94,11 @@ public class PathBasedRepositoryLocationResolver extends BasicRepositoryLocation
           PathBasedRepositoryLocationResolver.this.setLocation(repositoryId, ((Path) location).toAbsolutePath());
         }
       }
+
+      @Override
+      public void forAllLocations(BiConsumer<String, T> consumer) {
+        pathById.forEach((id, path) -> consumer.accept(id, (T) contextProvider.resolve(path)));
+      }
     };
   }
 
@@ -115,10 +120,6 @@ public class PathBasedRepositoryLocationResolver extends BasicRepositoryLocation
     return contextProvider.resolve(removedPath);
   }
 
-  void forAllPaths(BiConsumer<String, Path> consumer) {
-    pathById.forEach((id, path) -> consumer.accept(id, contextProvider.resolve(path)));
-  }
-
   void updateModificationDate() {
     this.writePathDatabase();
   }

scm-dao-xml/src/main/java/sonia/scm/repository/xml/SingleRepositoryUpdateProcessor.java

@@ -1,5 +1,7 @@
 package sonia.scm.repository.xml;
 
+import sonia.scm.repository.RepositoryLocationResolver;
+
 import javax.inject.Inject;
 import java.nio.file.Path;
 import java.util.function.BiConsumer;
@@ -7,9 +9,9 @@ import java.util.function.BiConsumer;
 public class SingleRepositoryUpdateProcessor {
 
   @Inject
-  private PathBasedRepositoryLocationResolver locationResolver;
+  private RepositoryLocationResolver locationResolver;
 
   public void doUpdate(BiConsumer<String, Path> forEachRepository) {
-    locationResolver.forAllPaths(forEachRepository);
+    locationResolver.forClass(Path.class).forAllLocations(forEachRepository);
   }
 }

scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java

@@ -40,7 +40,7 @@ import sonia.scm.repository.InternalRepositoryException;
 import sonia.scm.repository.NamespaceAndName;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.RepositoryDAO;
-import sonia.scm.store.StoreConstants;
+import sonia.scm.repository.RepositoryLocationResolver;
 
 import javax.inject.Inject;
 import java.io.IOException;
@@ -76,18 +76,14 @@ public class XmlRepositoryDAO implements RepositoryDAO {
   }
 
   private void init() {
-    repositoryLocationResolver.forAllPaths((repositoryId, repositoryPath) -> {
+    RepositoryLocationResolver.RepositoryLocationResolverInstance<Path> pathRepositoryLocationResolverInstance = repositoryLocationResolver.create(Path.class);
-      Path metadataPath = resolveDataPath(repositoryPath);
+    pathRepositoryLocationResolverInstance.forAllLocations((repositoryId, repositoryPath) -> {
-      Repository repository = metadataStore.read(metadataPath);
+      Repository repository = metadataStore.read(repositoryPath);
       byNamespaceAndName.put(repository.getNamespaceAndName(), repository);
       byId.put(repositoryId, repository);
     });
   }
 
-  private Path resolveDataPath(Path repositoryPath) {
-    return repositoryPath.resolve(StoreConstants.REPOSITORY_METADATA.concat(StoreConstants.FILE_EXTENSION));
-  }
-
   @Override
   public String getType() {
     return "xml";
@@ -108,8 +104,7 @@ public class XmlRepositoryDAO implements RepositoryDAO {
       Path repositoryPath = (Path) location;
 
       try {
-        Path metadataPath = resolveDataPath(repositoryPath);
+        metadataStore.write(repositoryPath, repository);
-        metadataStore.write(metadataPath, repository);
       } catch (Exception e) {
         repositoryLocationResolver.remove(repository.getId());
         throw new InternalRepositoryException(repository, "failed to create filesystem", e);
@@ -166,9 +161,8 @@ public class XmlRepositoryDAO implements RepositoryDAO {
     Path repositoryPath = repositoryLocationResolver
       .create(Path.class)
       .getLocation(repository.getId());
-    Path metadataPath = resolveDataPath(repositoryPath);
     repositoryLocationResolver.updateModificationDate();
-    metadataStore.write(metadataPath, clone);
+    metadataStore.write(repositoryPath, clone);
   }
 
   @Override

scm-dao-xml/src/test/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolverTest.java

@@ -120,7 +120,7 @@ class PathBasedRepositoryLocationResolverTest {
     @Test
     void shouldInitWithExistingData() {
       Map<String, Path> foundRepositories = new HashMap<>();
-      resolverWithExistingData.forAllPaths(
+      resolverWithExistingData.forClass(Path.class).forAllLocations(
         foundRepositories::put
       );
       assertThat(foundRepositories)

scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOTest.java

@@ -26,15 +26,13 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.function.BiConsumer;
+import java.util.function.Consumer;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.fail;
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -47,6 +45,7 @@ class XmlRepositoryDAOTest {
 
   @Mock
   private PathBasedRepositoryLocationResolver locationResolver;
+  private Consumer<BiConsumer<String, Path>> triggeredOnForAllLocations = none -> {};
 
   private FileSystem fileSystem = new DefaultFileSystem();
 
@@ -69,6 +68,11 @@ class XmlRepositoryDAOTest {
         @Override
         public void setLocation(String repositoryId, Path location) {
         }
+
+        @Override
+        public void forAllLocations(BiConsumer<String, Path> consumer) {
+          triggeredOnForAllLocations.accept(consumer);
+        }
       }
     );
     when(locationResolver.create(anyString())).thenAnswer(invocation -> createMockedRepoPath(basePath, invocation));
@@ -332,11 +336,10 @@ class XmlRepositoryDAOTest {
     @Test
     void shouldRefreshWithExistingRepositoriesFromPathDatabase() {
       // given
-      doNothing().when(locationResolver).forAllPaths(any());
-      XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem);
-
       mockExistingPath();
 
+      XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem);
+
       // when
       dao.refresh();
 
@@ -346,12 +349,7 @@ class XmlRepositoryDAOTest {
     }
 
     private void mockExistingPath() {
-      doAnswer(
+      triggeredOnForAllLocations = consumer -> consumer.accept("existing", repositoryPath);
-        invocation -> {
-          ((BiConsumer<String, Path>) invocation.getArgument(0)).accept("existing", repositoryPath);
-          return null;
-        }
-      ).when(locationResolver).forAllPaths(any());
     }
   }
 

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitConfigHelper.java

@@ -0,0 +1,21 @@
+package sonia.scm.repository;
+
+import org.eclipse.jgit.lib.StoredConfig;
+
+import java.io.IOException;
+
+public class GitConfigHelper {
+
+  private static final String CONFIG_SECTION_SCMM = "scmm";
+  private static final String CONFIG_KEY_REPOSITORY_ID = "repositoryid";
+
+  public void createScmmConfig(Repository repository, org.eclipse.jgit.lib.Repository gitRepository) throws IOException {
+    StoredConfig config = gitRepository.getConfig();
+    config.setString(CONFIG_SECTION_SCMM, null, CONFIG_KEY_REPOSITORY_ID, repository.getId());
+    config.save();
+  }
+
+  public String getRepositoryId(StoredConfig gitConfig) {
+    return gitConfig.getString(CONFIG_SECTION_SCMM, null, CONFIG_KEY_REPOSITORY_ID);
+  }
+}

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java

@@ -89,8 +89,6 @@ public class GitRepositoryHandler
                                     GitRepositoryServiceProvider.COMMANDS);
 
   private static final Object LOCK = new Object();
-  private static final String CONFIG_SECTION_SCMM = "scmm";
-  private static final String CONFIG_KEY_REPOSITORY_ID = "repositoryid";
 
   private final Scheduler scheduler;
 
@@ -185,7 +183,7 @@ public class GitRepositoryHandler
   }
 
   public String getRepositoryId(StoredConfig gitConfig) {
-    return gitConfig.getString(GitRepositoryHandler.CONFIG_SECTION_SCMM, null, GitRepositoryHandler.CONFIG_KEY_REPOSITORY_ID);
+    return new GitConfigHelper().getRepositoryId(gitConfig);
   }
 
   //~--- methods --------------------------------------------------------------
@@ -194,9 +192,7 @@ public class GitRepositoryHandler
   protected void create(Repository repository, File directory) throws IOException {
     try (org.eclipse.jgit.lib.Repository gitRepository = build(directory)) {
       gitRepository.create(true);
-      StoredConfig config = gitRepository.getConfig();
+      new GitConfigHelper().createScmmConfig(repository, gitRepository);
-      config.setString(CONFIG_SECTION_SCMM, null, CONFIG_KEY_REPOSITORY_ID, repository.getId());
-      config.save();
     }
   }
 

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/update/GitV1UpdateStep.java

@@ -0,0 +1,70 @@
+package sonia.scm.repository.update;
+
+import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
+import sonia.scm.migration.UpdateException;
+import sonia.scm.migration.UpdateStep;
+import sonia.scm.plugin.Extension;
+import sonia.scm.repository.GitConfigHelper;
+import sonia.scm.repository.GitRepositoryHandler;
+import sonia.scm.repository.Repository;
+import sonia.scm.repository.RepositoryLocationResolver;
+import sonia.scm.update.UpdateStepRepositoryMetadataAccess;
+import sonia.scm.version.Version;
+
+import javax.inject.Inject;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
+
+import static sonia.scm.version.Version.parse;
+
+@Extension
+public class GitV1UpdateStep implements UpdateStep {
+
+  private final RepositoryLocationResolver locationResolver;
+  private final UpdateStepRepositoryMetadataAccess<Path> repositoryMetadataAccess;
+
+  @Inject
+  public GitV1UpdateStep(RepositoryLocationResolver locationResolver, UpdateStepRepositoryMetadataAccess<Path> repositoryMetadataAccess) {
+    this.locationResolver = locationResolver;
+    this.repositoryMetadataAccess = repositoryMetadataAccess;
+  }
+
+  @Override
+  public void doUpdate() {
+    locationResolver.forClass(Path.class).forAllLocations(
+      (repositoryId, path) -> {
+        Repository repository = repositoryMetadataAccess.read(path);
+        if (isGitDirectory(repository)) {
+          try (org.eclipse.jgit.lib.Repository gitRepository = build(path.resolve("data").toFile())) {
+            new GitConfigHelper().createScmmConfig(repository, gitRepository);
+          } catch (IOException e) {
+            throw new UpdateException("could not update repository with id " + repositoryId + " in path " + path, e);
+          }
+        }
+      }
+    );
+  }
+
+  private org.eclipse.jgit.lib.Repository build(File directory) throws IOException {
+    return new FileRepositoryBuilder()
+      .setGitDir(directory)
+      .readEnvironment()
+      .findGitDir()
+      .build();
+  }
+
+  private boolean isGitDirectory(Repository repository) {
+    return GitRepositoryHandler.TYPE_NAME.equals(repository.getType());
+  }
+
+  @Override
+  public Version getTargetVersion() {
+    return parse("2.0.0");
+  }
+
+  @Override
+  public String getAffectedDataType() {
+    return "sonia.scm.plugin.git";
+  }
+}

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/HgRepositoryHandler.java

@@ -41,13 +41,11 @@ import com.google.inject.Singleton;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.ConfigurationException;
-import sonia.scm.ContextEntry;
 import sonia.scm.SCMContextProvider;
 import sonia.scm.installer.HgInstaller;
 import sonia.scm.installer.HgInstallerFactory;
 import sonia.scm.io.ExtendedCommand;
 import sonia.scm.io.INIConfiguration;
-import sonia.scm.io.INIConfigurationReader;
 import sonia.scm.io.INIConfigurationWriter;
 import sonia.scm.io.INISection;
 import sonia.scm.plugin.Extension;
@@ -347,14 +345,6 @@ public class HgRepositoryHandler
     writer.write(hgrc, hgrcFile);
   }
 
-  public String getRepositoryId(File directory) {
-    try {
-      return new INIConfigurationReader().read(new File(directory, PATH_HGRC)).getSection(CONFIG_SECTION_SCMM).getParameter(CONFIG_KEY_REPOSITORY_ID);
-    } catch (IOException e) {
-      throw new InternalRepositoryException(ContextEntry.ContextBuilder.entity("Directory", directory.toString()), "could not read scm configuration file", e);
-    }
-  }
-
   //~--- get methods ----------------------------------------------------------
 
   /**

scm-test/src/main/java/sonia/scm/TempDirRepositoryLocationResolver.java

@@ -4,6 +4,7 @@ import sonia.scm.repository.BasicRepositoryLocationResolver;
 
 import java.io.File;
 import java.nio.file.Path;
+import java.util.function.BiConsumer;
 
 public class TempDirRepositoryLocationResolver extends BasicRepositoryLocationResolver {
   private final File tempDirectory;
@@ -30,6 +31,11 @@ public class TempDirRepositoryLocationResolver extends BasicRepositoryLocationRe
       public void setLocation(String repositoryId, T location) {
         throw new UnsupportedOperationException("not implemented for tests");
       }
+
+      @Override
+      public void forAllLocations(BiConsumer<String, T> consumer) {
+        consumer.accept("id", (T) tempDirectory.toPath());
+      }
     };
   }
 }

scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java

@@ -10,6 +10,7 @@ import sonia.scm.io.DefaultFileSystem;
 import sonia.scm.io.FileSystem;
 import sonia.scm.plugin.PluginLoader;
 import sonia.scm.repository.RepositoryLocationResolver;
+import sonia.scm.repository.xml.MetadataStore;
 import sonia.scm.repository.xml.PathBasedRepositoryLocationResolver;
 import sonia.scm.security.CipherHandler;
 import sonia.scm.security.CipherUtil;
@@ -25,6 +26,7 @@ import sonia.scm.store.JAXBConfigurationStoreFactory;
 import sonia.scm.store.JAXBDataStoreFactory;
 import sonia.scm.store.JAXBPropertyFileAccess;
 import sonia.scm.update.PropertyFileAccess;
+import sonia.scm.update.UpdateStepRepositoryMetadataAccess;
 import sonia.scm.update.V1PropertyDAO;
 import sonia.scm.update.xml.XmlV1PropertyDAO;
 
@@ -65,6 +67,7 @@ public class BootstrapModule extends AbstractModule {
     bind(PluginLoader.class).toInstance(pluginLoader);
     bind(V1PropertyDAO.class, XmlV1PropertyDAO.class);
     bind(PropertyFileAccess.class, JAXBPropertyFileAccess.class);
+    bind(UpdateStepRepositoryMetadataAccess.class).to(MetadataStore.class);
   }
 
   private <T> void bind(Class<T> clazz, Class<? extends T> defaultImplementation) {