Merge pull request #136 from AlamoEngine-Tools/develop

support null for name resolver and more relaxed logging.

src/PetroGlyph.Games.EawFoc.Clients/src/Arguments/ArgumentValidityStatus.cs

@@ -1,7 +1,7 @@
 namespace PG.StarWarsGame.Infrastructure.Clients.Arguments;
 
 /// <summary>
-/// Indicates the validity of a <see cref="IGameArgument"/>
+/// Indicates the validity of a <see cref="IGameArgument"/>.
 /// </summary>
 public enum ArgumentValidityStatus
 {

src/PetroGlyph.Games.EawFoc/src/Services/GameFactory.cs

@@ -8,22 +8,11 @@
 
 namespace PG.StarWarsGame.Infrastructure.Services;
 
-/// <inheritdoc/>
-internal sealed class GameFactory : IGameFactory
+internal sealed class GameFactory(IServiceProvider serviceProvider) : IGameFactory
 {
-    private readonly IGameNameResolver _nameResolver;
-    private readonly IServiceProvider _serviceProvider;
+    private readonly IGameNameResolver _nameResolver = serviceProvider.GetRequiredService<IGameNameResolver>();
+    private readonly IServiceProvider _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider));
 
-    /// <summary>
-    /// Creates a new factory instances. Uses <see cref="EnglishGameNameResolver"/> for the game's name resolution.
-    /// </summary>
-    /// <param name="serviceProvider">The service provider which gets passed to the game instances.</param>
-    public GameFactory(IServiceProvider serviceProvider)
-    {
-        _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider));
-        _nameResolver = serviceProvider.GetRequiredService<IGameNameResolver>();
-    }
-
     /// <inheritdoc/>
     public IGame CreateGame(GameDetectionResult gameDetection, CultureInfo culture)
     {
@@ -45,7 +34,10 @@ public IGame CreateGame(IGameIdentity identity, IDirectoryInfo location, bool ch
             throw new ArgumentException("Cannot create a game with undefined platform");
 
         var name = _nameResolver.ResolveName(identity, culture);
-        var game = new PetroglyphStarWarsGame(identity, location, name, _serviceProvider);
+        if (string.IsNullOrEmpty(name))
+            throw new GameException("Cannot create game with null or empty name.");
+
+        var game = new PetroglyphStarWarsGame(identity, location, name!, _serviceProvider);
         if (checkGameExists && !game.Exists())
             throw new GameException($"Game does not exists at location: {location}");
 

src/PetroGlyph.Games.EawFoc/src/Services/Name/Games/EnglishGameNameResolver.cs

@@ -9,12 +9,20 @@ namespace PG.StarWarsGame.Infrastructure.Services.Name;
 /// </summary>
 public sealed class EnglishGameNameResolver : IGameNameResolver
 {
+    /// <summary>
+    /// Returns the english name of the game, regardless of the given <paramref name="culture"/>
+    /// </summary>
+    public string ResolveName(IGameIdentity game, CultureInfo culture)
+    {
+        return ResolveName(game);
+    }
+
     /// <summary>
     /// Returns the english name of the game.
     /// </summary>
-    public string ResolveName(IGameIdentity game)
+    private string ResolveName(IGameIdentity game)
     {
-        if (game == null) 
+        if (game == null)
             throw new ArgumentNullException(nameof(game));
         var gameName = game.Type == GameType.Eaw
             ? PetroglyphStarWarsGameConstants.EmpireAtWarEnglishNameShort
@@ -23,11 +31,4 @@ public string ResolveName(IGameIdentity game)
         return $"{gameName} ({platform})";
     }
 
-    /// <summary>
-    /// Returns the english name of the game, regardless of the given <paramref name="culture"/>
-    /// </summary>
-    public string ResolveName(IGameIdentity game, CultureInfo culture)
-    {
-        return ResolveName(game);
-    }
 }

src/PetroGlyph.Games.EawFoc/src/Services/Name/Games/IGameNameResolver.cs

@@ -14,5 +14,5 @@ public interface IGameNameResolver
     /// <param name="game">The game which name shall get resolved.</param>
     /// <param name="culture">The culture context.</param>
     /// <returns>The resolved name.</returns>
-    string ResolveName(IGameIdentity game, CultureInfo culture);
+    string? ResolveName(IGameIdentity game, CultureInfo culture);
 }

src/PetroGlyph.Games.EawFoc/src/Services/Name/Mods/CompositeModNameResolver.cs

@@ -4,36 +4,36 @@
 using EawModinfo.Spec;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
-using PG.StarWarsGame.Infrastructure.Mods;
 
 namespace PG.StarWarsGame.Infrastructure.Services.Name;
 
 /// <summary>
-/// <see cref="IModNameResolver"/> which takes a sorted list of <see cref="IModNameResolver"/> and returns the first valid result.
+/// <see cref="IModNameResolver"/> which takes a sorted list of <see cref="IModNameResolver"/> and returns the first non-null or non-empty result.
 /// </summary>
 public sealed class CompositeModNameResolver : IModNameResolver
 {
     private readonly IList<IModNameResolver> _resolvers;
     private readonly ILogger? _logger;
 
     /// <summary>
-    /// Create a new instance.
+    /// Initializes a new instance of the <see cref="CompositeModNameResolver"/> class with a factory method for subsequent resolvers.
     /// </summary>
     /// <param name="serviceProvider">The service provider.</param>
     /// <param name="resolverFactory">Factory method to create a list of <see cref="IModNameResolver"/>.</param>
     public CompositeModNameResolver(IServiceProvider serviceProvider, Func<IServiceProvider, IList<IModNameResolver>> resolverFactory)
     {
         if (serviceProvider == null)
             throw new ArgumentNullException(nameof(serviceProvider));
-        if (resolverFactory == null) throw new ArgumentNullException(nameof(resolverFactory));
+        if (resolverFactory == null)
+            throw new ArgumentNullException(nameof(resolverFactory));
         var resolvers = resolverFactory(serviceProvider);
         ThrowHelper.ThrowIfCollectionNullOrEmpty(resolvers);
         _resolvers = resolvers;
         _logger = serviceProvider.GetService<ILoggerFactory>()?.CreateLogger(GetType());
     }
 
     /// <inheritdoc/>
-    public string ResolveName(IModReference modReference, CultureInfo culture)
+    public string? ResolveName(IModReference modReference, CultureInfo culture)
     {
         if (modReference == null)
             throw new ArgumentNullException(nameof(modReference));
@@ -44,24 +44,19 @@ public string ResolveName(IModReference modReference, CultureInfo culture)
         {
             try
             {
-                try
-                {
-                    _logger?.LogDebug($"Resolving mod name with {nameResolver}");
-                    var name = nameResolver.ResolveName(modReference, culture);
-                    if (!string.IsNullOrEmpty(name))
-                        return name;
-                }
-                catch (Exception e)
-                {
-                    throw new ModException(modReference, "Error while resolving a mod's name", e);
-                }
+                _logger?.LogDebug($"Resolving mod name with {nameResolver}");
+                var name = nameResolver.ResolveName(modReference, culture);
+                if (!string.IsNullOrEmpty(name))
+                    return name;
             }
-            catch (ModException e)
+            catch (Exception e)
             {
-                _logger?.LogDebug(e, e.Message);
+                _logger?.LogDebug(e, $"Error while resolving mod name for '{modReference}' with resolver {nameResolver.GetType()}: {e.Message}");
+                throw;
             }
         }
 
-        throw new ModException(modReference, "Unable to resolve the mod's name.");
+        _logger?.LogTrace($"Unable to resolve name for '{modReference}'");
+        return null;
     }
 }

src/PetroGlyph.Games.EawFoc/src/Services/Name/Mods/IModNameResolver.cs

@@ -1,6 +1,5 @@
 using System.Globalization;
 using EawModinfo.Spec;
-using PG.StarWarsGame.Infrastructure.Mods;
 
 namespace PG.StarWarsGame.Infrastructure.Services.Name;
 
@@ -15,6 +14,5 @@ public interface IModNameResolver
     /// <param name="modReference">The game which name shall get resolved.</param>
     /// <param name="culture">The culture context.</param>
     /// <returns>The resolved name. May return <see langword="null"/> if no name for the <paramref name="culture"/> could be found.</returns>
-    /// <exception cref="ModException">The mod's name could not be resolved.</exception>
-    string ResolveName(IModReference modReference, CultureInfo culture);
+    string? ResolveName(IModReference modReference, CultureInfo culture);
 }

src/PetroGlyph.Games.EawFoc/src/Services/Name/Mods/ModNameResolverBase.cs

@@ -32,7 +32,7 @@ protected ModNameResolverBase(IServiceProvider serviceProvider)
     }
 
     /// <inheritdoc/>
-    public string ResolveName(IModReference modReference, CultureInfo culture)
+    public string? ResolveName(IModReference modReference, CultureInfo culture)
     {
         if (modReference == null)
             throw new ArgumentNullException(nameof(modReference));
@@ -43,19 +43,13 @@ public string ResolveName(IModReference modReference, CultureInfo culture)
         {
             var name = ResolveCore(modReference, culture);
             if (string.IsNullOrEmpty(name))
-                throw new PetroglyphException($"Unable to resolve the mod's name {modReference}");
+                Logger?.LogTrace($"Resolver '{this}' resolved null or empty name for '{modReference}'.");
             return name;
         }
-        catch (PetroglyphException ex)
-        {
-            Logger?.LogError(ex, ex.Message);
-            throw;
-        }
         catch (Exception ex)
         {
-            var e = new PetroglyphException($"Unable to resolve the mod's name {modReference}: {this}", ex);
-            Logger?.LogError(e, e.Message);
-            throw e;
+            Logger?.LogError(ex, $"Resolver '{this}' had an error while resolving the name for {modReference}: {ex.Message}");
+            throw;
         }
     }
 
@@ -71,5 +65,5 @@ public override string ToString()
     /// <param name="modReference">The target <see cref="IModReference"/>.</param>
     /// <param name="culture">The target <see cref="CultureInfo"/>.</param>
     /// <returns>The resolved name or <see langword="null"/>.</returns>
-    protected internal abstract string ResolveCore(IModReference modReference, CultureInfo culture);
+    protected internal abstract string? ResolveCore(IModReference modReference, CultureInfo culture);
 }

src/PetroGlyph.Games.EawFoc/src/Services/Name/Mods/OfflineWorkshopNameResolver.cs

@@ -23,13 +23,13 @@ public OfflineWorkshopNameResolver(IServiceProvider serviceProvider) : base(serv
     }
 
     /// <inheritdoc/>
-    protected internal override string ResolveCore(IModReference modReference, CultureInfo culture)
+    protected internal override string? ResolveCore(IModReference modReference, CultureInfo culture)
     {
         if (modReference.Type != ModType.Workshops)
             throw new NotSupportedException("Can only resolve for Steam Workshop mods!");
         if (!_steamHelper.ToSteamWorkshopsId(modReference.Identifier, out var modId))
             throw new ModException(modReference, $"Cannot get SteamID from workshops object {modReference.Identifier}");
 
-        return _workshopCache.ContainsMod(modId) ? _workshopCache.GetName(modId) : null!;
+        return _workshopCache.ContainsMod(modId) ? _workshopCache.GetName(modId) : null;
     }
 }

src/PetroGlyph.Games.EawFoc/src/Services/Name/Mods/OnlineWorkshopNameResolver.cs

@@ -4,6 +4,7 @@
 using EawModinfo.Spec;
 using HtmlAgilityPack;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using PG.StarWarsGame.Infrastructure.Mods;
 using PG.StarWarsGame.Infrastructure.Services.Steam;
 
@@ -26,7 +27,7 @@ public OnlineWorkshopNameResolver(IServiceProvider serviceProvider) : base(servi
     }
 
     /// <inheritdoc/>
-    protected internal override string ResolveCore(IModReference modReference, CultureInfo culture)
+    protected internal override string? ResolveCore(IModReference modReference, CultureInfo culture)
     {
         if (modReference.Type != ModType.Workshops)
             throw new NotSupportedException("Can only resolve for Steam Workshop mods!");
@@ -42,21 +43,25 @@ protected internal override string ResolveCore(IModReference modReference, Cultu
             var downloader = ServiceProvider.GetService<ISteamWorkshopWebpageDownloader>() ??
                              new SteamWorkshopWebpageDownloader();
             var modsWorkshopWebpage = downloader.GetSteamWorkshopsPageHtmlAsync(modId, culture).GetAwaiter().GetResult();
-            if (modsWorkshopWebpage is null)
-                throw new InvalidOperationException("Unable to get the mod's workshop web page.");
 
-            return GetName(modsWorkshopWebpage);
+            if (modsWorkshopWebpage == null)
+            {
+                Logger?.LogTrace($"Unable to download website for Steam ID '{modId}'.");
+                return null;
+            }
+
+            return GetName(modsWorkshopWebpage, modId);
         })!;
     }
 
-    private static string GetName(HtmlDocument htmlDocument)
+    private string? GetName(HtmlDocument htmlDocument, ulong modId)
     {
-        if (htmlDocument == null)
-            throw new ArgumentNullException(nameof(htmlDocument));
-
         var node = htmlDocument.DocumentNode.SelectSingleNode("//div[contains(@class, 'workshopItemTitle')]");
         if (node is null)
-            throw new InvalidOperationException("Unable to get name form Workshop's web page. Missing 'workshopItemTitle' node.");
+        {
+            Logger?.LogTrace($"Unable to find the item title on website for Steam ID '{modId}',");
+            return null;
+        }
         return node.InnerHtml;
     }
 }

src/PetroGlyph.Games.EawFoc/test/GameServices/EnglishGameNameResolverTest.cs

@@ -13,7 +13,7 @@ public void IgnoreCulture()
         var resolver = new EnglishGameNameResolver();
         var id = new GameIdentity(GameType.Eaw, GamePlatform.SteamGold);
         resolver.ResolveName(id, CultureInfo.GetCultureInfo("de"));
-        var name = resolver.ResolveName(id);
+        var name = resolver.ResolveName(id, CultureInfo.CurrentCulture);
         Assert.Contains("Steam", name);
         Assert.Contains("Empire at War", name);
     }
@@ -23,7 +23,7 @@ public void EawSteamName()
     {
         var resolver = new EnglishGameNameResolver();
         var id = new GameIdentity(GameType.Eaw, GamePlatform.SteamGold);
-        var name = resolver.ResolveName(id);
+        var name = resolver.ResolveName(id, CultureInfo.InvariantCulture);
         Assert.Contains("Steam", name);
         Assert.Contains("Empire at War", name);
     }
@@ -33,7 +33,7 @@ public void FocSteamName()
     {
         var resolver = new EnglishGameNameResolver();
         var id = new GameIdentity(GameType.Foc, GamePlatform.SteamGold);
-        var name = resolver.ResolveName(id);
+        var name = resolver.ResolveName(id, CultureInfo.CurrentCulture);
         Assert.Contains("Steam", name);
         Assert.Contains("Corruption", name);
     }
@@ -43,7 +43,7 @@ public void DiskSteamName()
     {
         var resolver = new EnglishGameNameResolver();
         var id = new GameIdentity(GameType.Foc, GamePlatform.Disk);
-        var name = resolver.ResolveName(id);
+        var name = resolver.ResolveName(id, CultureInfo.InvariantCulture);
         Assert.DoesNotContain("Steam", name);
         Assert.Contains("Corruption", name);
     }

src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

@@ -1,10 +1,8 @@
 using System;
-using System.Collections.Generic;
 using System.Globalization;
 using EawModinfo.Model;
 using EawModinfo.Spec;
 using Moq;
-using PG.StarWarsGame.Infrastructure.Mods;
 using PG.StarWarsGame.Infrastructure.Services.Name;
 using Xunit;
 
@@ -44,19 +42,19 @@ public void SingleInternalResolver()
     }
 
     [Fact]
-    public void RequiresInvariant_Throws()
+    public void ReturnsNull()
     {
         var sp = new Mock<IServiceProvider>();
 
         var internalResolver = new Mock<IModNameResolver>();
         internalResolver.Setup(r => r.ResolveName(It.IsAny<IModReference>(), It.IsAny<CultureInfo>()))
-            .Returns((string)null!);
+            .Returns((string?)null);
 
         var modRef = new ModReference("Id", ModType.Default);
 
         var resolver = new CompositeModNameResolver(sp.Object, _ => [internalResolver.Object]);
 
-        Assert.Throws<ModException>(() => resolver.ResolveName(modRef, CultureInfo.InvariantCulture));
+        Assert.Null(resolver.ResolveName(modRef, CultureInfo.InvariantCulture));
     }
 
 
@@ -89,12 +87,12 @@ public void InternalThrows_Throws()
 
         var internalResolver = new Mock<IModNameResolver>();
         internalResolver.Setup(r => r.ResolveName(It.IsAny<IModReference>(), It.IsAny<CultureInfo>()))
-            .Throws<Exception>();
+            .Throws<InvalidOperationException>();
 
         var modRef = new ModReference("Id", ModType.Default);
 
         var resolver = new CompositeModNameResolver(sp.Object, _ => [internalResolver.Object]);
 
-        Assert.Throws<ModException>(() => resolver.ResolveName(modRef, CultureInfo.InvariantCulture));
+        Assert.Throws<InvalidOperationException>(() => resolver.ResolveName(modRef, CultureInfo.InvariantCulture));
     }
 }