fix: Better error handling in git code

Code for cloning the repository refactored to handle failures better.
Namely this means deleting the repository and cloning it again if there
is a failure.

CHANGELOG.md

@@ -16,14 +16,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Schema added for `settings.yml`.
 - Add setting to bypass HTTPS certificate validation (useful for self-signed certificates used with
   Sonarr and Radarr instances) ([#20]).
-
-[setref]: https://github.com/rcdailey/trash-updater/wiki/Settings-Reference
-[#20]: https://github.com/rcdailey/trash-updater/issues/20
+- A progress bar that is visible when pulling down Custom Formats (Radarr Only).
 
 ### Fixed
 
 - Remove `System.Reactive.xml` from the published ZIP files.
 - Fix exception that may occur at startup.
+- Sometimes the "Requesting and parsing guide markdown" step would appear stuck and fail after
+  several minutes. Many changes have been made to try to alleviate this.
+
+[setref]: https://github.com/rcdailey/trash-updater/wiki/Settings-Reference
+[#20]: https://github.com/rcdailey/trash-updater/issues/20
 
 ## [1.6.6] - 2021-10-30
 

src/Common/ProgressBar.cs

@@ -0,0 +1,113 @@
+using System;
+using System.Diagnostics.CodeAnalysis;
+using System.Text;
+using System.Threading;
+
+namespace Common;
+
+/// <summary>
+///     An ASCII progress bar
+/// </summary>
+/// <remarks>
+///     Full credit to: https://gist.github.com/DanielSWolf/0ab6a96899cc5377bf54
+/// </remarks>
+[SuppressMessage("Reliability", "CA2002:Do not lock on objects with weak identity",
+    Justification = "Code is from third party")]
+public sealed class ProgressBar : IDisposable, IProgress<double>
+{
+    private const int BlockCount = 10;
+    private readonly TimeSpan _animationInterval = TimeSpan.FromSeconds(1.0 / 8);
+    private const string Animation = @"|/-\";
+
+    private readonly Timer _timer;
+
+    private double _currentProgress;
+    private string _currentText = string.Empty;
+    private bool _disposed;
+    private int _animationIndex;
+
+    public ProgressBar()
+    {
+        _timer = new Timer(TimerHandler);
+
+        // A progress bar is only for temporary display in a console window.
+        // If the console output is redirected to a file, draw nothing.
+        // Otherwise, we'll end up with a lot of garbage in the target file.
+        if (!Console.IsOutputRedirected)
+        {
+            ResetTimer();
+        }
+    }
+
+    public void Report(double value)
+    {
+        // Make sure value is in [0..1] range
+        value = Math.Max(0, Math.Min(1, value));
+        Interlocked.Exchange(ref _currentProgress, value);
+    }
+
+    private void TimerHandler(object? state)
+    {
+        lock (_timer)
+        {
+            if (_disposed)
+            {
+                return;
+            }
+
+            var progressBlockCount = (int) (_currentProgress * BlockCount);
+            var percent = (int) (_currentProgress * 100);
+            var progressBlocks = new string('#', progressBlockCount);
+            var progressBlocksUnfilled = new string('-', BlockCount - progressBlockCount);
+            var currentAnimationFrame = Animation[_animationIndex++ % Animation.Length];
+            var text = $"[{progressBlocks}{progressBlocksUnfilled}] {percent,3}% {currentAnimationFrame}";
+            UpdateText(text);
+
+            ResetTimer();
+        }
+    }
+
+    private void UpdateText(string text)
+    {
+        // Get length of common portion
+        var commonPrefixLength = 0;
+        var commonLength = Math.Min(_currentText.Length, text.Length);
+        while (commonPrefixLength < commonLength && text[commonPrefixLength] == _currentText[commonPrefixLength])
+        {
+            commonPrefixLength++;
+        }
+
+        // Backtrack to the first differing character
+        var outputBuilder = new StringBuilder();
+        outputBuilder.Append('\b', _currentText.Length - commonPrefixLength);
+
+        // Output new suffix
+        outputBuilder.Append(text.AsSpan(commonPrefixLength));
+
+        // If the new text is shorter than the old one: delete overlapping characters
+        var overlapCount = _currentText.Length - text.Length;
+        if (overlapCount > 0)
+        {
+            outputBuilder.Append(' ', overlapCount);
+            outputBuilder.Append('\b', overlapCount);
+        }
+
+        Console.Write(outputBuilder);
+        _currentText = text;
+    }
+
+    private void ResetTimer()
+    {
+        _timer.Change(_animationInterval, TimeSpan.FromMilliseconds(-1));
+    }
+
+    public void Dispose()
+    {
+        lock (_timer)
+        {
+            _disposed = true;
+            UpdateText(string.Empty);
+            _timer.Dispose();
+        }
+    }
+}

src/Trash/CompositionRoot.cs

@@ -16,6 +16,7 @@ using TrashLib.Radarr;
 using TrashLib.Radarr.Config;
 using TrashLib.Sonarr;
 using TrashLib.Startup;
+using VersionControl;
 using YamlDotNet.Serialization;
 
 namespace Trash;
@@ -92,6 +93,7 @@ public static class CompositionRoot
 
         builder.RegisterModule<SonarrAutofacModule>();
         builder.RegisterModule<RadarrAutofacModule>();
+        builder.RegisterModule<VersionControlAutofacModule>();
 
         builder.Register(_ => AutoMapperConfig.Setup()).SingleInstance();
 

src/TrashLib.Tests/Radarr/CustomFormat/Processors/GuideProcessorTest.cs

@@ -6,7 +6,6 @@ using FluentAssertions;
 using Newtonsoft.Json.Linq;
 using NSubstitute;
 using NUnit.Framework;
-using Serilog;
 using TestLibrary.FluentAssertions;
 using Trash.TestLibrary;
 using TrashLib.Radarr.Config;
@@ -32,16 +31,9 @@ public class GuideProcessorTest
     {
         public Context()
         {
-            Logger = new LoggerConfiguration()
-                .WriteTo.TestCorrelator()
-                .WriteTo.NUnitOutput()
-                .MinimumLevel.Debug()
-                .CreateLogger();
-
             Data = new ResourceDataReader(typeof(GuideProcessorTest), "Data");
         }
 
-        public ILogger Logger { get; }
         public ResourceDataReader Data { get; }
 
         public string ReadText(string textFile) => Data.ReadData(textFile);
@@ -54,10 +46,10 @@ public class GuideProcessorTest
     {
         var ctx = new Context();
         var guideService = Substitute.For<IRadarrGuideService>();
-        var guideProcessor = new GuideProcessor(ctx.Logger, guideService, () => new TestGuideProcessorSteps());
+        var guideProcessor = new GuideProcessor(guideService, () => new TestGuideProcessorSteps());
 
         // simulate guide data
-        guideService.GetCustomFormatJsonAsync().Returns(new[]
+        guideService.GetCustomFormatJson().Returns(new[]
         {
             ctx.ReadText("ImportableCustomFormat1.json"),
             ctx.ReadText("ImportableCustomFormat2.json"),

src/TrashLib/Radarr/CustomFormat/Guide/IRadarrGuideService.cs

@@ -1,9 +1,8 @@
 using System.Collections.Generic;
-using System.Threading.Tasks;
 
 namespace TrashLib.Radarr.CustomFormat.Guide;
 
 public interface IRadarrGuideService
 {
-    Task<IEnumerable<string>> GetCustomFormatJsonAsync();
+    IEnumerable<string> GetCustomFormatJson();
 }

src/TrashLib/Radarr/CustomFormat/Guide/LocalRepoCustomFormatJsonParser.cs

@@ -1,82 +1,86 @@
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.IO.Abstractions;
 using System.Linq;
 using System.Threading.Tasks;
+using Common;
 using LibGit2Sharp;
 using Serilog;
 using TrashLib.Config.Settings;
 using TrashLib.Radarr.Config;
+using VersionControl;
 
-namespace TrashLib.Radarr.CustomFormat.Guide
-{
-    internal class LocalRepoCustomFormatJsonParser : IRadarrGuideService
-    {
-        private readonly IFileSystem _fileSystem;
-        private readonly ISettingsProvider _settings;
-        private readonly ILogger _log;
-        private readonly string _repoPath;
-
-        public LocalRepoCustomFormatJsonParser(
-            IFileSystem fileSystem,
-            IResourcePaths paths,
-            ISettingsProvider settings,
-            ILogger log)
-        {
-            _fileSystem = fileSystem;
-            _settings = settings;
-            _log = log;
-            _repoPath = paths.RepoPath;
-        }
-
-        public async Task<IEnumerable<string>> GetCustomFormatJsonAsync()
-        {
-            CloneOrUpdateGitRepo();
+namespace TrashLib.Radarr.CustomFormat.Guide;
 
-            var jsonDir = Path.Combine(_repoPath, "docs/json/radarr");
-            var tasks = _fileSystem.Directory.GetFiles(jsonDir, "*.json")
-                .Select(async f => await _fileSystem.File.ReadAllTextAsync(f));
+internal class LocalRepoCustomFormatJsonParser : IRadarrGuideService
+{
+    private readonly ILogger _log;
+    private readonly IFileSystem _fileSystem;
+    private readonly IGitRepositoryFactory _repositoryFactory;
+    private readonly IFileUtilities _fileUtils;
+    private readonly ISettingsProvider _settingsProvider;
+    private readonly string _repoPath;
 
-            return await Task.WhenAll(tasks);
-        }
+    public LocalRepoCustomFormatJsonParser(
+        ILogger log,
+        IFileSystem fileSystem,
+        IResourcePaths paths,
+        IGitRepositoryFactory repositoryFactory,
+        IFileUtilities fileUtils,
+        ISettingsProvider settingsProvider)
+    {
+        _log = log;
+        _fileSystem = fileSystem;
+        _repositoryFactory = repositoryFactory;
+        _fileUtils = fileUtils;
+        _settingsProvider = settingsProvider;
+        _repoPath = paths.RepoPath;
+    }
 
-        private void CloneOrUpdateGitRepo()
+    public IEnumerable<string> GetCustomFormatJson()
+    {
+        // Retry only once if there's a failure. This gives us an opportunity to delete the git repository and start
+        // fresh.
+        var exception = CheckoutAndUpdateRepo();
+        if (exception is not null)
         {
-            var cloneUrl = _settings.Settings.Repository.CloneUrl;
+            _log.Information("Deleting local git repo and retrying git operation...");
+            _fileUtils.DeleteReadOnlyDirectory(_repoPath);
 
-            if (!Repository.IsValid(_repoPath))
+            exception = CheckoutAndUpdateRepo();
+            if (exception is not null)
             {
-                if (_fileSystem.Directory.Exists(_repoPath))
-                {
-                    _fileSystem.Directory.Delete(_repoPath, true);
-                }
-
-                Repository.Clone(cloneUrl, _repoPath, new CloneOptions
-                {
-                    RecurseSubmodules = false
-                });
+                throw exception;
             }
+        }
 
-            using var repo = new Repository(_repoPath);
-            Commands.Checkout(repo, "master", new CheckoutOptions
-            {
-                CheckoutModifiers = CheckoutModifiers.Force
-            });
-
-            var origin = repo.Network.Remotes["origin"];
-            if (origin.Url != cloneUrl)
-            {
-                _log.Debug(
-                    "Origin's URL ({OriginUrl}) does not match the clone URL from settings ({CloneUrl}) and will be updated",
-                    origin.Url, cloneUrl);
+        var jsonDir = Path.Combine(_repoPath, "docs/json/radarr");
+        var tasks = _fileSystem.Directory.GetFiles(jsonDir, "*.json")
+            .Select(async f => await _fileSystem.File.ReadAllTextAsync(f));
 
-                repo.Network.Remotes.Update("origin", updater => updater.Url = cloneUrl);
-                origin = repo.Network.Remotes["origin"];
-            }
+        return Task.WhenAll(tasks).Result;
+    }
 
-            Commands.Fetch(repo, origin.Name, origin.FetchRefSpecs.Select(s => s.Specification), null, "");
+    private Exception? CheckoutAndUpdateRepo()
+    {
+        var repoSettings = _settingsProvider.Settings.Repository;
+        var cloneUrl = repoSettings.CloneUrl;
+        const string branch = "master";
 
-            repo.Reset(ResetMode.Hard, repo.Branches["origin/master"].Tip);
+        try
+        {
+            using var repo = _repositoryFactory.CreateAndCloneIfNeeded(cloneUrl, _repoPath, branch);
+            repo.ForceCheckout(branch);
+            repo.Fetch();
+            repo.ResetHard($"origin/{branch}");
         }
+        catch (LibGit2SharpException e)
+        {
+            _log.Error(e, "An exception occurred during git operations on path: {RepoPath}", _repoPath);
+            return e;
+        }
+
+        return null;
     }
 }

src/TrashLib/Radarr/CustomFormat/Processors/GuideProcessor.cs

@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
-using Serilog;
 using TrashLib.Radarr.Config;
 using TrashLib.Radarr.CustomFormat.Guide;
 using TrashLib.Radarr.CustomFormat.Models;
@@ -25,16 +24,13 @@ internal class GuideProcessor : IGuideProcessor
     private IList<string>? _guideCustomFormatJson;
     private IGuideProcessorSteps _steps;
 
-    public GuideProcessor(ILogger log, IRadarrGuideService guideService, Func<IGuideProcessorSteps> stepsFactory)
+    public GuideProcessor(IRadarrGuideService guideService, Func<IGuideProcessorSteps> stepsFactory)
     {
         _guideService = guideService;
         _stepsFactory = stepsFactory;
-        Log = log;
         _steps = stepsFactory();
     }
 
-    private ILogger Log { get; }
-
     public IReadOnlyCollection<ProcessedCustomFormatData> ProcessedCustomFormats
         => _steps.CustomFormat.ProcessedCustomFormats;
 
@@ -59,12 +55,11 @@ internal class GuideProcessor : IGuideProcessor
     public IDictionary<string, List<ProcessedCustomFormatData>> DuplicatedCustomFormats
         => _steps.CustomFormat.DuplicatedCustomFormats;
 
-    public async Task BuildGuideDataAsync(IReadOnlyCollection<CustomFormatConfig> config, CustomFormatCache? cache)
+    public Task BuildGuideDataAsync(IReadOnlyCollection<CustomFormatConfig> config, CustomFormatCache? cache)
     {
         if (_guideCustomFormatJson == null)
         {
-            Log.Information("Requesting and parsing guide markdown");
-            _guideCustomFormatJson = (await _guideService.GetCustomFormatJsonAsync()).ToList();
+            _guideCustomFormatJson = _guideService.GetCustomFormatJson().ToList();
         }
 
         // Step 1: Process and filter the custom formats from the guide.
@@ -84,6 +79,8 @@ internal class GuideProcessor : IGuideProcessor
         // Score precedence logic is utilized here to decide the CF score per profile (same CF can actually have
         // different scores depending on which profile it goes into).
         _steps.QualityProfile.Process(_steps.Config.ConfigData);
+
+        return Task.CompletedTask;
     }
 
     public void Reset()

src/VersionControl.Tests/GitRepositoryFactoryTest.cs

@@ -0,0 +1,50 @@
+using AutoFixture.NUnit3;
+using Common;
+using FluentAssertions;
+using LibGit2Sharp;
+using NSubstitute;
+using NUnit.Framework;
+using TestLibrary.AutoFixture;
+using TestLibrary.NSubstitute;
+using VersionControl.Wrappers;
+
+namespace VersionControl.Tests;
+
+[TestFixture]
+[Parallelizable(ParallelScope.All)]
+public class GitRepositoryFactoryTest
+{
+    [Test, AutoMockData]
+    public void Delete_and_clone_when_repo_is_not_valid(
+        [Frozen] IRepositoryStaticWrapper wrapper,
+        [Frozen] IFileUtilities fileUtils,
+        GitRepositoryFactory sut)
+    {
+        wrapper.IsValid(Arg.Any<string>()).Returns(false);
+
+        sut.CreateAndCloneIfNeeded("repo_url", "repo_path", "branch");
+
+        Received.InOrder(() =>
+        {
+            wrapper.IsValid("repo_path");
+            fileUtils.DeleteReadOnlyDirectory("repo_path");
+            wrapper.Clone("repo_url", "repo_path",
+                Verify.That<CloneOptions>(x => x.BranchName.Should().Be("branch")));
+        });
+    }
+
+    [Test, AutoMockData]
+    public void No_delete_and_clone_when_repo_is_valid(
+        [Frozen] IRepositoryStaticWrapper wrapper,
+        [Frozen] IFileUtilities fileUtils,
+        GitRepositoryFactory sut)
+    {
+        wrapper.IsValid(Arg.Any<string>()).Returns(true);
+
+        sut.CreateAndCloneIfNeeded("repo_url", "repo_path", "branch");
+
+        wrapper.Received().IsValid("repo_path");
+        fileUtils.DidNotReceiveWithAnyArgs().DeleteReadOnlyDirectory(default!);
+        wrapper.DidNotReceiveWithAnyArgs().Clone(default!, default!, default!);
+    }
+}

src/VersionControl/GitRepository.cs

@@ -0,0 +1,39 @@
+using System.Linq;
+using LibGit2Sharp;
+
+namespace VersionControl;
+
+public sealed class GitRepository : IGitRepository
+{
+    private readonly Repository _repo;
+
+    public GitRepository(string repoPath)
+    {
+        _repo = new Repository(repoPath);
+    }
+
+    public void Dispose()
+    {
+        _repo.Dispose();
+    }
+
+    public void ForceCheckout(string branch)
+    {
+        Commands.Checkout(_repo, branch, new CheckoutOptions
+        {
+            CheckoutModifiers = CheckoutModifiers.Force
+        });
+    }
+
+    public void Fetch(string remote = "origin")
+    {
+        var origin = _repo.Network.Remotes[remote];
+        Commands.Fetch(_repo, origin.Name, origin.FetchRefSpecs.Select(s => s.Specification), null, "");
+    }
+
+    public void ResetHard(string toBranch)
+    {
+        var commit = _repo.Branches[toBranch].Tip;
+        _repo.Reset(ResetMode.Hard, commit);
+    }
+}

src/VersionControl/GitRepositoryFactory.cs

@@ -0,0 +1,53 @@
+using System;
+using Common;
+using LibGit2Sharp;
+using VersionControl.Wrappers;
+
+namespace VersionControl;
+
+public class GitRepositoryFactory : IGitRepositoryFactory
+{
+    private readonly IFileUtilities _fileUtils;
+    private readonly IRepositoryStaticWrapper _staticWrapper;
+    private readonly Func<string, IGitRepository> _repoFactory;
+
+    public GitRepositoryFactory(
+        IFileUtilities fileUtils,
+        IRepositoryStaticWrapper staticWrapper,
+        Func<string, IGitRepository> repoFactory)
+    {
+        _fileUtils = fileUtils;
+        _staticWrapper = staticWrapper;
+        _repoFactory = repoFactory;
+    }
+
+    public IGitRepository CreateAndCloneIfNeeded(string repoUrl, string repoPath, string branch)
+    {
+        if (!_staticWrapper.IsValid(repoPath))
+        {
+            DeleteAndCloneRepo(repoUrl, repoPath, branch);
+        }
+
+        return _repoFactory(repoPath);
+    }
+
+    private void DeleteAndCloneRepo(string repoUrl, string repoPath, string branch)
+    {
+        _fileUtils.DeleteReadOnlyDirectory(repoPath);
+
+        Console.Write("Requesting and parsing guide markdown ");
+
+        using var progress = new ProgressBar();
+        _staticWrapper.Clone(repoUrl, repoPath, new CloneOptions
+        {
+            RecurseSubmodules = false,
+            BranchName = branch,
+            OnTransferProgress = gitProgress =>
+            {
+                // ReSharper disable once AccessToDisposedClosure
+                progress.Report((float) gitProgress.ReceivedObjects / gitProgress.TotalObjects);
+                return true;
+            }
+        });
+    }
+}

src/VersionControl/IGitRepository.cs

@@ -0,0 +1,10 @@
+using System;
+
+namespace VersionControl;
+
+public interface IGitRepository : IDisposable
+{
+    void ForceCheckout(string branch);
+    void Fetch(string remote = "origin");
+    void ResetHard(string toBranch);
+}

src/VersionControl/IGitRepositoryFactory.cs

@@ -0,0 +1,6 @@
+namespace VersionControl;
+
+public interface IGitRepositoryFactory
+{
+    IGitRepository CreateAndCloneIfNeeded(string repoUrl, string repoPath, string branch);
+}

src/VersionControl/VersionControlAutofacModule.cs

@@ -0,0 +1,15 @@
+using Autofac;
+using VersionControl.Wrappers;
+
+namespace VersionControl;
+
+public class VersionControlAutofacModule : Module
+{
+    protected override void Load(ContainerBuilder builder)
+    {
+        builder.RegisterType<GitRepository>().As<IGitRepository>();
+        builder.RegisterType<LibGit2SharpRepositoryStaticWrapper>().As<IRepositoryStaticWrapper>();
+        builder.RegisterType<GitRepositoryFactory>().As<IGitRepositoryFactory>();
+        base.Load(builder);
+    }
+}

src/VersionControl/Wrappers/IRepositoryStaticWrapper.cs

@@ -0,0 +1,9 @@
+using LibGit2Sharp;
+
+namespace VersionControl.Wrappers;
+
+public interface IRepositoryStaticWrapper
+{
+    string Clone(string sourceUrl, string workdirPath, CloneOptions options);
+    bool IsValid(string path);
+}

src/VersionControl/Wrappers/LibGit2SharpRepositoryStaticWrapper.cs

@@ -0,0 +1,12 @@
+using LibGit2Sharp;
+
+namespace VersionControl.Wrappers;
+
+public class LibGit2SharpRepositoryStaticWrapper : IRepositoryStaticWrapper
+{
+    public string Clone(string sourceUrl, string workdirPath, CloneOptions options)
+        => Repository.Clone(sourceUrl, workdirPath, options);
+
+    public bool IsValid(string path)
+        => Repository.IsValid(path);
+}