Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support null for name resolver and more relaxed logging. #136

Merged
merged 1 commit into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
{
Expand Down
22 changes: 7 additions & 15 deletions src/PetroGlyph.Games.EawFoc/src/Services/GameFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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}");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System.Globalization;
using EawModinfo.Spec;
using PG.StarWarsGame.Infrastructure.Mods;

namespace PG.StarWarsGame.Infrastructure.Services.Name;

Expand All @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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;
}
}

Expand All @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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!");
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -16,7 +14,7 @@
public void NullArgTest_Throws()
{
Assert.Throws<ArgumentNullException>(() => new CompositeModNameResolver(null, null));
Assert.Throws<ArgumentNullException>(() => new CompositeModNameResolver(new Mock<IServiceProvider>().Object, _ => null));

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / Build & Test (ubuntu-latest)

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / test / Build & Test (ubuntu-latest)

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / Build & Test (windows-latest)

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / Build & Test (windows-latest)

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / Analyze

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / Analyze

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / Analyze

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / Analyze

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / test / Build & Test (windows-latest)

Possible null reference return.

Check warning on line 17 in src/PetroGlyph.Games.EawFoc/test/ModServices/CompositeModNameResolverTest.cs

View workflow job for this annotation

GitHub Actions / test / Build & Test (windows-latest)

Possible null reference return.
Assert.Throws<ArgumentException>(() => new CompositeModNameResolver(new Mock<IServiceProvider>().Object, _ => []));
Assert.Throws<ArgumentNullException>(() => new CompositeModNameResolver(null, _ => [null]));
var sp = new Mock<IServiceProvider>();
Expand Down Expand Up @@ -44,19 +42,19 @@
}

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


Expand Down Expand Up @@ -89,12 +87,12 @@

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