Skip to content

Commit 5254399

Browse files
jibedoublevejibedoubleve
authored
(#1272) Fix favicon HTTP client lifecycle and add thumbnail diagnostics
1 parent b6264af commit 5254399

File tree

11 files changed

+147
-61
lines changed

11 files changed

+147
-61
lines changed

‎src/Infra/Lanceur.Infra.Win32/Thumbnails/Strategies/FavIconAppThumbnailStrategy.cs‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,15 @@ IAliasManagementService aliasManagementService
3636

3737
public async Task UpdateThumbnailAsync(AliasQueryResult alias)
3838
{
39-
if (File.Exists(alias.Thumbnail)) { return; }
40-
41-
var thumbnail = await _favIconService.UpdateFaviconAsync(alias, ResolveCachePath);
42-
if (thumbnail.IsNullOrEmpty()) { return; }
43-
44-
var res = string.Compare(
45-
alias.Thumbnail,
46-
thumbnail,
47-
StringComparison.InvariantCulture
48-
);
49-
50-
if (res == 0)
39+
if (File.Exists(alias.Thumbnail))
5140
{
52-
_logger.LogInformation("Thumbnail for alias {Name} is up to date. Update skipped.", alias.Name);
41+
_logger.LogTrace("Thumbnail for alias {Name} is in cache. Update skipped.", alias.Name);
5342
return;
5443
}
5544

45+
var thumbnail = await _favIconService.UpdateFaviconAsync(alias, ResolveCachePath);
46+
if (thumbnail.IsNullOrEmpty()) { return; }
47+
5648
_logger.LogInformation(
5749
"Updating for alias {Alias} favicon from {Thumbnail} to {FavIconPath}",
5850
alias.Name.DefaultIfNullOrEmpty("<empty>"),

‎src/Infra/Lanceur.Infra.Win32/Thumbnails/Strategies/PackagedAppThumbnailStrategy.cs‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Lanceur.Core.Models;
33
using Lanceur.Core.Services;
44
using Lanceur.Infra.Win32.Extensions;
5+
using Microsoft.Extensions.Logging;
56

67
namespace Lanceur.Infra.Win32.Thumbnails.Strategies;
78

@@ -10,6 +11,7 @@ public class PackagedAppThumbnailStrategy : IThumbnailStrategy
1011
#region Fields
1112

1213
private readonly IAliasManagementService _aliasManagementService;
14+
private readonly ILogger<PackagedAppThumbnailStrategy> _logger;
1315

1416
private readonly IPackagedAppSearchService _packagedAppSearchService;
1517

@@ -19,11 +21,13 @@ public class PackagedAppThumbnailStrategy : IThumbnailStrategy
1921

2022
public PackagedAppThumbnailStrategy(
2123
IPackagedAppSearchService packagedAppSearchService,
22-
IAliasManagementService aliasManagementService
24+
IAliasManagementService aliasManagementService,
25+
ILogger<PackagedAppThumbnailStrategy> logger
2326
)
2427
{
2528
_packagedAppSearchService = packagedAppSearchService;
2629
_aliasManagementService = aliasManagementService;
30+
_logger = logger;
2731
}
2832

2933
#endregion
@@ -32,14 +36,22 @@ IAliasManagementService aliasManagementService
3236

3337
public async Task UpdateThumbnailAsync(AliasQueryResult alias)
3438
{
35-
if (File.Exists(alias.Thumbnail)) { return; }
39+
if (File.Exists(alias.Thumbnail))
40+
{
41+
_logger.LogTrace("Thumbnail for alias {Name} is in cache. Update skipped.", alias.Name);
42+
return;
43+
}
3644

3745
if (!alias.IsPackagedApplication()) { return; }
3846

3947
var app = await _packagedAppSearchService.GetByInstalledDirectoryAsync(alias.FileName);
4048
var response = app.FirstOrDefault();
4149

42-
if (response is null) { return; }
50+
if (response is null)
51+
{
52+
_logger.LogTrace("Failed to download the thumbnail for alias {Name}.", alias.Name);
53+
return;
54+
}
4355

4456
var thumbnailFileName = alias.FileName.GetThumbnailFileName();
4557
response.Logo.LocalPath.CopyToImageRepository(thumbnailFileName);

‎src/Infra/Lanceur.Infra.Win32/Thumbnails/Strategies/Win32AppThumbnailStrategy.cs‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ public class Win32AppThumbnailStrategy : IThumbnailStrategy
1212
#region Fields
1313

1414
private readonly IAliasManagementService _aliasManagementService;
15+
private readonly ILogger<Win32AppThumbnailStrategy> _logger;
1516

1617
private readonly IStaThreadRunner _staThreadRunner;
1718
private readonly Win32ThumbnailService _win32ThumbnailService;
@@ -23,11 +24,13 @@ public class Win32AppThumbnailStrategy : IThumbnailStrategy
2324
public Win32AppThumbnailStrategy(
2425
IStaThreadRunner staThreadRunner,
2526
ILoggerFactory loggerFactory,
26-
IAliasManagementService aliasManagementService
27+
IAliasManagementService aliasManagementService,
28+
ILogger<Win32AppThumbnailStrategy> logger
2729
)
2830
{
2931
_staThreadRunner = staThreadRunner;
3032
_aliasManagementService = aliasManagementService;
33+
_logger = logger;
3134
_win32ThumbnailService = new Win32ThumbnailService(loggerFactory.CreateLogger<Win32ThumbnailService>());
3235
}
3336

@@ -37,10 +40,18 @@ IAliasManagementService aliasManagementService
3740

3841
public async Task UpdateThumbnailAsync(AliasQueryResult alias)
3942
{
40-
if (File.Exists(alias.Thumbnail)) { return; }
43+
if (File.Exists(alias.Thumbnail))
44+
{
45+
_logger.LogTrace("Thumbnail for alias {Name} is in cache. Update skipped.", alias.Name);
46+
return;
47+
}
4148

4249
var imageSource = await _staThreadRunner.RunAsync(() => _win32ThumbnailService.GetThumbnail(alias.FileName));
43-
if (imageSource is null) { return; }
50+
if (imageSource is null)
51+
{
52+
_logger.LogTrace("Failed to download the thumbnail for alias {Name}.", alias.Name);
53+
return;
54+
}
4455

4556
var thumbnailFileName = alias.FileName.GetThumbnailFileName();
4657
imageSource.CopyToImageRepository(thumbnailFileName);

‎src/Infra/Lanceur.Infra.Win32/Thumbnails/Win32ThumbnailService.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ private static BitmapSource GetThumbnail(string path, ThumbnailOptions options)
4545

4646
try
4747
{
48+
path = ResolveFullPath(path);
49+
4850
if (Directory.Exists(path))
4951
{
5052
/* Directories can also have thumbnails instead of shell icons.
@@ -78,6 +80,36 @@ private static BitmapSource GetThumbnail(string path, ThumbnailOptions options)
7880
//The value is returned even if null;
7981
return image;
8082
}
83+
84+
private static string ResolveFullPath(string path)
85+
{
86+
// Already absolute
87+
if (Path.IsPathRooted(path)) { return path; }
88+
89+
// Search in PATH environment variable
90+
var pathEnv = Environment.GetEnvironmentVariable("PATH") ?? string.Empty;
91+
var directories = pathEnv.Split(Path.PathSeparator);
92+
93+
foreach (var dir in directories)
94+
{
95+
// Try as-is
96+
var candidate = Path.Combine(dir, path);
97+
if (File.Exists(candidate)) { return candidate; }
98+
99+
// Try with common Windows executable extensions
100+
foreach (var ext in new[] { ".exe", ".com", ".cmd", ".bat" })
101+
{
102+
if (!path.EndsWith(ext, StringComparison.OrdinalIgnoreCase))
103+
{
104+
var candidateWithExt = candidate + ext;
105+
if (File.Exists(candidateWithExt)) { return candidateWithExt; }
106+
}
107+
}
108+
}
109+
110+
// Return original if resolution fails — let downstream checks handle it
111+
return path;
112+
}
81113

82114
#endregion
83115
}

‎src/Infra/Lanceur.Infra/Services/FavIconDownloader.cs‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,24 @@
11
using System.Net;
2+
using Humanizer;
3+
using Lanceur.Core.Configuration;
4+
using Lanceur.Core.Configuration.Sections;
25
using Lanceur.Core.Services;
36
using Lanceur.SharedKernel.Extensions;
47
using Microsoft.Extensions.Caching.Memory;
58
using Microsoft.Extensions.Logging;
69

710
namespace Lanceur.Infra.Services;
811

12+
/// <summary>
13+
/// Downloads and caches favicons for a given URL by querying multiple favicon providers
14+
/// (e.g. DuckDuckGo, Google) and falling back to manual file discovery.
15+
/// Failed attempts are recorded in a memory cache to avoid redundant retries
16+
/// until the configured retry delay has elapsed.
17+
/// </summary>
918
public class FavIconDownloader : IFavIconDownloader
1019
{
1120
#region Fields
1221

13-
private readonly HttpClient _client;
1422
private readonly IMemoryCache _faviconCache;
1523
private readonly ILogger<FavIconDownloader> _logger;
1624
private readonly TimeSpan _retryDelay;
@@ -23,21 +31,23 @@ public class FavIconDownloader : IFavIconDownloader
2331
["Manual (ico)"] = (IsManual: true, Url: "favicon.ico")
2432
};
2533

34+
private readonly IFavIconHttpClient _httpClient;
35+
2636
#endregion
2737

2838
#region Constructors
2939

3040
public FavIconDownloader(
3141
ILogger<FavIconDownloader> logger,
3242
IMemoryCache faviconCache,
33-
TimeSpan retryDelay,
34-
IHttpClientFactory httpClientFactory
43+
ISection<CachingSection> settings,
44+
IFavIconHttpClient httpClient
3545
)
3646
{
3747
_logger = logger;
3848
_faviconCache = faviconCache;
39-
_retryDelay = retryDelay;
40-
_client = httpClientFactory.CreateClient("favicon");
49+
_httpClient = httpClient;
50+
_retryDelay = settings.Value.ThumbnailCacheDuration.Minutes();
4151
}
4252

4353
#endregion
@@ -92,7 +102,7 @@ public async Task<bool> RetrieveAndSaveFavicon(Uri url, string outputPath)
92102
try
93103
{
94104
var requestUrl = GetFaviconUrl(url, faviconUrl);
95-
var response = await _client.SendAsync(new HttpRequestMessage(HttpMethod.Get, requestUrl));
105+
using var response = await _httpClient.SendAsync(new HttpRequestMessage(HttpMethod.Get, requestUrl));
96106

97107
_logger.LogTrace(
98108
"Checking favicon with {FavIconManager} - Status: {Status} - Host: {Host}",

‎src/Infra/Lanceur.Infra/Services/FavIconHttpClient.cs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
using Humanizer;
2+
using Lanceur.Core.Services;
3+
4+
namespace Lanceur.Infra.Services;
5+
6+
/// <inheritdoc />
7+
public class FavIconHttpClient : IFavIconHttpClient
8+
{
9+
#region Fields
10+
11+
private static readonly HttpClient HttpClient = new(new SocketsHttpHandler
12+
{
13+
PooledConnectionIdleTimeout = 1.Minutes()
14+
});
15+
16+
#endregion
17+
18+
#region Methods
19+
20+
/// <inheritdoc />
21+
public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request) => HttpClient.SendAsync(request);
22+
23+
#endregion
24+
}

‎src/Infra/Lanceur.Infra/Services/FavIconService.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public async Task<string> UpdateFaviconAsync(AliasQueryResult alias, Func<string
4343
// - no url is provided
4444
// - alias is a Macro
4545
// - Uri is malformed
46-
if (url is null || IsMacroRegex.Match(url).Success || !Uri.TryCreate(url, UriKind.Absolute, out var uri))
46+
if (url.IsNullOrEmpty() || IsMacroRegex.Match(url).Success || !Uri.TryCreate(url, UriKind.Absolute, out var uri))
4747
{
4848
return null;
4949
}
@@ -53,8 +53,9 @@ public async Task<string> UpdateFaviconAsync(AliasQueryResult alias, Func<string
5353
if (File.Exists(favIconPath)) { return favIconPath; }
5454

5555
var uriAuthority = uri.GetAuthority();
56+
if(uriAuthority is null) { return null; }
57+
5658
var success = await _favIconDownloader.RetrieveAndSaveFavicon(uriAuthority, favIconPath);
57-
5859
return success ? favIconPath : null;
5960
}
6061

‎src/Lanceur.Core/Services/IFavIconHttpClient.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
namespace Lanceur.Core.Services;
2+
3+
/// <summary>
4+
/// Abstraction over <see cref="System.Net.Http.HttpClient" /> for sending HTTP requests
5+
/// during favicon retrieval, enabling unit testing by decoupling the implementation
6+
/// from the concrete HTTP client.
7+
/// </summary>
8+
public interface IFavIconHttpClient
9+
{
10+
#region Methods
11+
12+
/// <summary>
13+
/// Send an HTTP request as an asynchronous operation.
14+
/// </summary>
15+
/// <param name="request">The HTTP request message to send.</param>
16+
/// <returns>The task object representing the asynchronous operation.</returns>
17+
Task<HttpResponseMessage> SendAsync(HttpRequestMessage request);
18+
19+
#endregion
20+
}

‎src/Lanceur.SharedKernel/Extensions/UriExtensions.cs‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,21 @@ public static class UriExtensions
66

77
private static Uri ToUri(this string path, UriKind kind) => new(path, kind);
88

9-
public static Uri GetAuthority(this Uri baseUri) => new(baseUri.GetLeftPart(UriPartial.Authority));
10-
11-
public static IEnumerable<Uri> GetFavicons(this Uri baseUri)
9+
/// <summary>
10+
/// Returns a <see cref="Uri" /> containing only the scheme, host, and port of the specified URI.
11+
/// Returns <c>null</c> if the authority part cannot be determined.
12+
/// </summary>
13+
/// <param name="baseUri">The URI from which to extract the authority.</param>
14+
/// <returns>
15+
/// A <see cref="Uri" /> representing the authority (e.g. <c>https://example.com</c>),
16+
/// or <c>null</c> if the authority part is empty.
17+
/// </returns>
18+
public static Uri GetAuthority(this Uri baseUri)
1219
{
13-
var uri = baseUri.GetAuthority();
14-
yield return new Uri(uri, "favicon.ico");
15-
yield return new Uri(uri, "favicon.png");
20+
var leftPart = baseUri.GetLeftPart(UriPartial.Authority);
21+
return leftPart.IsNullOrEmpty()
22+
? null
23+
: new Uri(leftPart);
1624
}
1725

1826
public static Uri ToUriRelative(this string path) => path.ToUri(UriKind.Relative);

‎src/Tests/Lanceur.Tests/Functional/FavIconServiceTest.cs‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,6 @@ public class FavIconServiceTest
2424

2525
#region Methods
2626

27-
[Theory]
28-
[InlineData("http://www.google.com", "http://www.google.com/favicon.ico")]
29-
[InlineData("http://www.google.com:4001", "http://www.google.com:4001/favicon.ico")]
30-
[InlineData("http://www.google.com:80", "http://www.google.com:80/favicon.ico")]
31-
[InlineData("https://www.google.com", "https://www.google.com/favicon.ico")]
32-
[InlineData("https://www.google.com:4001", "https://www.google.com:4001/favicon.ico")]
33-
[InlineData("https://www.google.com:80", "https://www.google.com:80/favicon.ico")]
34-
public void When_fetching_favicon_Then_expected_favicons_returned(string url, string expected)
35-
{
36-
var thisUri = new Uri(expected);
37-
new Uri(url).GetFavicons()
38-
.ShouldContain(thisUri);
39-
}
40-
4127
[Theory]
4228
[InlineData("http://www.google.com", "http://www.google.com")]
4329
[InlineData("http://www.google.com:4001", "http://www.google.com:4001")]

0 commit comments

Comments
 (0)