diff --git a/src/ModuleCore/Git/GitManager.cs b/src/ModuleCore/Git/GitManager.cs
index 25a5b41..fe1f69e 100644
--- a/src/ModuleCore/Git/GitManager.cs
+++ b/src/ModuleCore/Git/GitManager.cs
@@ -1,4 +1,6 @@
-namespace ModuleCore.Git;
+using System.Collections.Concurrent;
+
+namespace ModuleCore.Git;
// TODO: better name for this
public class GitManager
@@ -11,23 +13,19 @@ public class GitManager
///
internal static GitManager InternalFreshInstance => new();
- ///
- /// Simply .ToString()
- ///
- private static readonly string DirectorySeparator = Path.DirectorySeparatorChar.ToString();
+ private class GitRegistration
+ {
+ public required string Name { get; set; }
+ public required string Location { get; set; }
+ }
- private readonly InternalDirectory _repositories;
- private readonly Lock _readWriteLock = new();
+ private readonly ConcurrentDictionary _registrations;
private GitManager()
{
Console.WriteLine($"{nameof(GitManager)} init");
- // Initialise the root container
- _repositories = new InternalDirectory()
- {
- Name = DirectorySeparator,
- InternalPath = DirectorySeparator
- };
+
+ _registrations = new ConcurrentDictionary();
}
///
@@ -39,131 +37,19 @@ public class GitManager
/// The normalised string the repository was registered against
public string RegisterRepo(string absoluteRepositoryLocation, string registrationName)
{
- // Depending on the caller, it might be possible that they've scripted automatic repo registration. Because I
- // don't really want to account to all the subtle ways that can be parallised, I just naively lock on every
- // registration attempt. This method should be quick regardless, and I could use ConcurrentDictionary except
- // that means every instance of InternalDirectory would need it and yeah nah fuck that I can just lock at the
- // top level
- lock (_readWriteLock)
+ registrationName = string.IsNullOrWhiteSpace(registrationName)
+ ? new DirectoryInfo(absoluteRepositoryLocation).Name
+ : registrationName;
+
+ if (_registrations.TryAdd(registrationName, new GitRegistration()
+ {
+ Name = registrationName,
+ Location = absoluteRepositoryLocation,
+ }))
{
- var normalisedName = NormaliseNamePath(string.IsNullOrWhiteSpace(registrationName)
- ? new DirectoryInfo(absoluteRepositoryLocation).Name
- : registrationName);
-
- // Regardless of if we get a name or not, the fully qualified version for us
- // starts with a /
- var directorySegmentsFromName = NameToSegments(normalisedName);
-
- var added = _repositories.Add(absoluteRepositoryLocation, directorySegmentsFromName);
-
- // Not sure about this, the Add should throw any exceptions on duplicate/failures but for now I'll leave this
- // here
- if (added == null)
- {
- throw new Exception("Failed to register location");
- }
-
- return normalisedName;
+ return registrationName;
}
- }
- ///
- /// Takes a name and returns it as a queue of its parts, starting with a root of
- ///
- ///
- ///
- private Queue NameToSegments(string name)
- {
- var segments = name.Split(DirectorySeparator);
-
- return segments.Length == 1
- ? new Queue([DirectorySeparator, name])
- : new Queue([DirectorySeparator, ..segments]);
- }
-
- ///
- /// Normalises the path separators in the given string to use Path.DirectorySeparatorChar
- ///
- ///
- ///
- private string NormaliseNamePath(string name)
- {
- // Feels a bit hacky, but this will actually normalise a path to a valid form. So if the input is
- // some/directory/paths, Path.GetRelativePath will normalise it to some\directory\paths, relative to ./
- // which is kind of handy but I also just wish there was a Path method that would do this for me. I know that
- // the whole point of Path is that it's based on a file system, but file systems can also be arbitrary and not
- // always be drive rooted.
- // Either way, this works and saves me having to reimplement a worse method when it's more important that users
- // are able to use file paths in whatever form they prefer, which means we leverage the internal implementation
- // in a weird way.
- return Path.GetRelativePath("./", name);
- }
-
- private class InternalDirectory
- {
- ///
- /// Name of the folder this
- ///
- public string Name { get; set; } = null!;
-
- public Dictionary Children { get; set; } = [];
-
- internal string InternalPath { get; set; }
-
- ///
- /// If not null, this is the absolute location of a registered git repository
- ///
- public string? FullRepositoryPath { get; set; }
-
- ///
- ///
- ///
- ///
- ///
- ///
- ///
- internal InternalDirectory? Add(string absoluteRepositoryLocation, Queue directorySegmentsFromName)
- {
- var topStack = directorySegmentsFromName.Dequeue();
-
- if (topStack != Name)
- {
- // logically it shouldn't be possible to have a value on top of the stack that _doesn't_ exist, but
- // just incase we throw as this should only happen if an Add is attempted on the root and the queue was
- // not correctly rooted to /
- throw new Exception($"Directory segment does not seem to exist: {topStack}");
- }
-
- // We're at the end of the directory segments so we can safely say we're at the end of the tree so
- // we add it to the relevant dictionary
- if (directorySegmentsFromName.Count == 0)
- {
- // If this is a new registration, set the FullRepositoryPath and return, otherwise we've got a duplicate
- // entry and we throw
- if (FullRepositoryPath == null)
- {
- FullRepositoryPath = absoluteRepositoryLocation;
- return this;
- }
-
- throw new Exception($"Registration already exists for {InternalPath}");
- }
-
- var nextSegment = directorySegmentsFromName.Peek();
-
- // Attempt to get the next level of the directory. If we don't have a key entry, create one
- if (!Children.TryGetValue(nextSegment, out var nextChild))
- {
- nextChild = new InternalDirectory()
- {
- Name = nextSegment,
- InternalPath = Path.Combine(InternalPath, nextSegment)
- };
- Children.Add(nextSegment, nextChild);
- }
-
- // add the next
- return nextChild.Add(absoluteRepositoryLocation, directorySegmentsFromName);
- }
+ throw new Exception($"Git repo already registered with the name {registrationName}");
}
}
\ No newline at end of file
diff --git a/tests/ModuleTests/Git/AddRegistrationTests.cs b/tests/ModuleTests/Git/AddRegistrationTests.cs
index e04cfc3..511833a 100644
--- a/tests/ModuleTests/Git/AddRegistrationTests.cs
+++ b/tests/ModuleTests/Git/AddRegistrationTests.cs
@@ -90,7 +90,28 @@ public class AddRegistrationTests
var secondRegistration = gitManager.RegisterRepo(testRepoAbsolutePath, Path.Combine(paths[..1]));
Assert.Equal(Path.Combine(paths), firstRegistration);
+ Assert.Equal(Path.Combine(paths[..1]), secondRegistration);
- Assert.Throws(() => gitManager.RegisterRepo(testRepoAbsolutePath, names.AltSeparator));
+ Assert.Throws(() => gitManager.RegisterRepo(testRepoAbsolutePath, Path.Combine(paths[..1])));
+ }
+
+ [Fact]
+ public void DuplicateRepoRegistrationDifferentSlashShouldNotFail()
+ {
+ Settings.UseFileName(nameof(RepoRegistrationWithWhitespaceName));
+
+ var gitManager = GitManager.InternalFreshInstance;
+ var testRepoAbsolutePath = "Test:/some/test/repo";
+ string[] paths = ["test", "nested", "path"];
+ var names = (NormalSeparator: string.Join(Path.DirectorySeparatorChar, paths), AltSeparator: string.Join(Path.AltDirectorySeparatorChar, paths));
+
+ var firstRegistration = gitManager.RegisterRepo(testRepoAbsolutePath, names.NormalSeparator);
+ // TODO: make nested registrations fail in both directions and test
+ var secondRegistration = gitManager.RegisterRepo(testRepoAbsolutePath, Path.Combine(paths[..1]));
+ var differentPathSeparatorRegistration = gitManager.RegisterRepo(testRepoAbsolutePath, names.AltSeparator);
+
+ Assert.Equal(Path.Combine(paths), firstRegistration);
+ Assert.Equal(Path.Combine(paths[..1]), secondRegistration);
+ Assert.Equal(names.AltSeparator, differentPathSeparatorRegistration);
}
}
\ No newline at end of file
diff --git a/tests/ModuleTests/Git/Snapshots/AddRegistrationTests/BasicRepoRegistration_2.verified.txt b/tests/ModuleTests/Git/Snapshots/AddRegistrationTests/BasicRepoRegistration_2.verified.txt
index 3c0b092..7f2c4e6 100644
--- a/tests/ModuleTests/Git/Snapshots/AddRegistrationTests/BasicRepoRegistration_2.verified.txt
+++ b/tests/ModuleTests/Git/Snapshots/AddRegistrationTests/BasicRepoRegistration_2.verified.txt
@@ -1,2 +1,2 @@
Attempted to register: other/path
-Registration result: other\path
+Registration result: other/path