Skip to content

Commit 035e882

Browse files
fleizeanfleizean
authored
Merge pull request #12 from pkgdiff-dev/gsd/quick-261021-pubdev-repo-visibility
fix: pub.dev per-version changelogs and public-only GitHub repositories
2 parents d3ff8c1 + 9b9a6bb commit 035e882

9 files changed

Lines changed: 201 additions & 49 deletions

File tree

‎src/backend/PkgDiff.Application/Interfaces/IGitHubScraperService.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,7 @@ public interface IGitHubScraperService
1111
// The package name tells a monorepo's per-package tags apart
1212
Task<string?> GetReleaseNotesForVersionAsync(string githubRepoUrl, string version, string? packageId) =>
1313
GetReleaseNotesForVersionAsync(githubRepoUrl, version);
14+
15+
// False for a private, missing or malformed repository; null when it could not be checked
16+
Task<bool?> IsPublicRepositoryAsync(string githubRepoUrl) => Task.FromResult<bool?>(null);
1417
}

‎src/backend/PkgDiff.Application/Interfaces/IPackageCacheService.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,7 @@ public interface IPackageCacheService
1515
Task SaveReleaseNoteAsync(CachedReleaseNote releaseNote);
1616
Task<PackageMetadata> GetOrFetchMetadataAsync(string registry, string packageId, bool bypassCache = false);
1717
Task<int> ResolveRegistryIdAsync(string registryValue);
18+
19+
// Null for a repository that is private, missing or not a repository
20+
Task<string?> UsableRepositoryUrlAsync(string? repositoryUrl);
1821
}

‎src/backend/PkgDiff.Application/Services/PackageAnalysisService.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,9 +425,10 @@ private async Task<bool> ScrapeVersionReleaseNotesAsync(
425425
var releaseNotes = StoredMarkdown.Normalize(result.ReleaseNotes);
426426
PkgDiffTelemetry.RecordScrape(activity, registry.Id, result.Source, !string.IsNullOrWhiteSpace(releaseNotes));
427427

428-
if (result.DiscoveredGitHubUrl != null)
428+
if (result.DiscoveredGitHubUrl != null
429+
&& await _cacheService.UsableRepositoryUrlAsync(result.DiscoveredGitHubUrl) is { } discoveredUrl)
429430
{
430-
cachedPackage.GitHubRepositoryUrl = result.DiscoveredGitHubUrl;
431+
cachedPackage.GitHubRepositoryUrl = discoveredUrl;
431432
cachedPackage.ReleaseNotesSource = "GitHub";
432433
await _cacheService.UpdateCachedPackageAsync(cachedPackage);
433434
}

‎src/backend/PkgDiff.Application/Services/PackageCacheService.cs‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class PackageCacheService : IPackageCacheService
1818
private readonly ICachedLookups _lookups;
1919
private readonly IJobScheduler _jobs;
2020
private readonly ILogger<PackageCacheService> _logger;
21+
private readonly IGitHubScraperService? _gitHub;
2122

2223
public PackageCacheService(
2324
IUnitOfWork unitOfWork,
@@ -26,7 +27,8 @@ public PackageCacheService(
2627
ILiveUpdateService liveUpdates,
2728
ICachedLookups lookups,
2829
IJobScheduler jobs,
29-
ILogger<PackageCacheService> logger)
30+
ILogger<PackageCacheService> logger,
31+
IGitHubScraperService? gitHub = null)
3032
{
3133
_unitOfWork = unitOfWork;
3234
_registryFactory = registryFactory;
@@ -35,6 +37,7 @@ public PackageCacheService(
3537
_lookups = lookups;
3638
_jobs = jobs;
3739
_logger = logger;
40+
_gitHub = gitHub;
3841
}
3942

4043
public async Task<CachedPackage?> GetCachedPackageAsync(string packageId, string registry)
@@ -180,6 +183,14 @@ public Task<int> ResolveRegistryIdAsync(string registryValue)
180183
return _lookups.GetRegistryIdAsync(registryValue);
181184
}
182185

186+
public async Task<string?> UsableRepositoryUrlAsync(string? repositoryUrl)
187+
{
188+
if (string.IsNullOrWhiteSpace(repositoryUrl) || _gitHub == null)
189+
return repositoryUrl;
190+
191+
return await _gitHub.IsPublicRepositoryAsync(repositoryUrl) == false ? null : repositoryUrl;
192+
}
193+
183194
private async Task<PackageMetadata> BuildMetadataFromCache(CachedPackage cached, string registryLower, IRegistryClient registry)
184195
{
185196
var cachedNotes = await _unitOfWork.CachedReleaseNotes
@@ -253,6 +264,10 @@ private async Task SaveFreshMetadataToCache(
253264
{
254265
var registryId = await ResolveRegistryIdAsync(registry);
255266

267+
// Checked again on every refresh, so a repository that turns public or private is picked up
268+
var namedRepository = metadata.GitHubRepositoryUrl;
269+
metadata.GitHubRepositoryUrl = await UsableRepositoryUrlAsync(namedRepository);
270+
256271
var repositoryChanged = false;
257272
if (cached == null)
258273
{
@@ -282,7 +297,9 @@ private async Task SaveFreshMetadataToCache(
282297
repositoryChanged = metadata.GitHubRepositoryUrl != null
283298
&& !string.Equals(metadata.GitHubRepositoryUrl, cached.GitHubRepositoryUrl, StringComparison.OrdinalIgnoreCase);
284299
// A repository found while scraping release notes is kept when the registry names none
285-
cached.GitHubRepositoryUrl = metadata.GitHubRepositoryUrl ?? cached.GitHubRepositoryUrl;
300+
cached.GitHubRepositoryUrl = namedRepository != null
301+
? metadata.GitHubRepositoryUrl
302+
: await UsableRepositoryUrlAsync(cached.GitHubRepositoryUrl);
286303
cached.AllVersionsJson = JsonSerializer.Serialize(metadata.Versions.Select(v => v.Version));
287304
cached.CachedAt = DateTime.UtcNow;
288305
cached.ExpiresAt = DateTime.UtcNow.AddDays(14);

‎src/backend/PkgDiff.Infrastructure/Services/GitHubScrapeCache.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,25 @@ public sealed class GitHubScrapeCache : IDisposable
1717
// A repository that had no changelog file may get one, so this knowledge is not kept for good
1818
private static readonly TimeSpan ChangelogLifetime = TimeSpan.FromHours(12);
1919

20+
// A repository can be made public or private at any time
21+
private static readonly TimeSpan VisibilityLifetime = TimeSpan.FromHours(6);
22+
2023
private readonly MemoryCache _cache = new(new MemoryCacheOptions { SizeLimit = MaxEntries });
2124

25+
public bool TryGetVisibility(string repoKey, out bool isPublic)
26+
{
27+
return _cache.TryGetValue("visibility:" + repoKey, out isPublic);
28+
}
29+
30+
public void SetVisibility(string repoKey, bool isPublic)
31+
{
32+
_cache.Set("visibility:" + repoKey, isPublic, new MemoryCacheEntryOptions
33+
{
34+
AbsoluteExpirationRelativeToNow = VisibilityLifetime,
35+
Size = 1
36+
});
37+
}
38+
2239
public bool TryGetReleases(string repositoryUrl, [NotNullWhen(true)] out Dictionary<string, string>? releases)
2340
{
2441
return _cache.TryGetValue(ReleasesKey(repositoryUrl), out releases) && releases != null;

‎src/backend/PkgDiff.Infrastructure/Services/GitHubScraperService.cs‎

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public partial class GitHubScraperService : IGitHubScraperService
2121
private readonly HttpClient _httpClient;
2222
private readonly bool _enableAuthentication;
2323
private readonly bool _useGitHubApi;
24+
private readonly bool _sendsToken;
2425
private readonly GitHubScrapeCache _cache;
2526
private readonly GitHubChangelogReader _changelog;
2627
private readonly GitHubMilestoneReader _milestones;
@@ -53,7 +54,8 @@ public GitHubScraperService(
5354
_httpClient.DefaultRequestHeaders.Add("User-Agent", userAgent);
5455
_httpClient.DefaultRequestHeaders.Add("Accept", "application/vnd.github+json");
5556

56-
if (_enableAuthentication && !string.IsNullOrWhiteSpace(personalAccessToken))
57+
_sendsToken = _enableAuthentication && !string.IsNullOrWhiteSpace(personalAccessToken);
58+
if (_sendsToken)
5759
{
5860
_httpClient.DefaultRequestHeaders.Add("Authorization", $"Bearer {personalAccessToken}");
5961
}
@@ -107,7 +109,10 @@ public async Task<Dictionary<string, string>> GetAllReleasesAsync(string githubR
107109
return await _retryPolicy.ExecuteAsync(async () =>
108110
{
109111
var cleanUrl = GitHubUrls.Clean(githubRepoUrl);
110-
112+
113+
if (!await MayReadAsync(cleanUrl))
114+
return new Dictionary<string, string>();
115+
111116
if (_cache.TryGetReleases(cleanUrl, out var cachedReleases))
112117
{
113118
_logger.LogDebug("[GitHubScraper] Using cached releases for {RepoUrl} ({Count} releases)",
@@ -241,6 +246,9 @@ public async Task<Dictionary<string, string>> GetAllReleasesAsync(string githubR
241246
{
242247
_logger.LogDebug("[GitHubScraper] Looking for release notes: repo={RepoUrl}, version={Version}", githubRepoUrl, version);
243248

249+
if (!await MayReadAsync(githubRepoUrl))
250+
return null;
251+
244252
string? notes = null;
245253

246254
if (!_useGitHubApi || !_enableAuthentication)
@@ -313,6 +321,81 @@ public async Task<Dictionary<string, string>> GetAllReleasesAsync(string githubR
313321
});
314322
}
315323

324+
public async Task<bool?> IsPublicRepositoryAsync(string githubRepoUrl)
325+
{
326+
try
327+
{
328+
return await RepositoryVisibilityAsync(githubRepoUrl);
329+
}
330+
catch (GitHubRateLimitedException)
331+
{
332+
return null;
333+
}
334+
}
335+
336+
// The token may see private repositories; their content must never be published
337+
private async Task<bool> MayReadAsync(string githubRepoUrl)
338+
{
339+
var isPublic = await RepositoryVisibilityAsync(githubRepoUrl);
340+
if (isPublic == true || (isPublic == null && !_sendsToken))
341+
return true;
342+
343+
_logger.LogInformation("[GitHubScraper] Not reading {RepoUrl}: not a public repository ({Visibility})",
344+
githubRepoUrl, isPublic == false ? "private or missing" : "unchecked");
345+
return false;
346+
}
347+
348+
private async Task<bool?> RepositoryVisibilityAsync(string githubRepoUrl)
349+
{
350+
var (owner, repo) = GitHubUrls.Parse(githubRepoUrl, _logger);
351+
if (string.IsNullOrEmpty(owner) || string.IsNullOrEmpty(repo))
352+
return false;
353+
354+
// Without a token nothing private can be read
355+
if (!_sendsToken)
356+
return null;
357+
358+
var repoKey = $"{owner}/{repo}".ToLowerInvariant();
359+
if (_cache.TryGetVisibility(repoKey, out var cached))
360+
return cached;
361+
362+
var apiUrl = $"https://api.github.com/repos/{PackageUrlEncoder.EscapeSegment(owner)}/{PackageUrlEncoder.EscapeSegment(repo)}";
363+
try
364+
{
365+
await _rateLimitHandler.CanMakeRequestAsync();
366+
using var response = await _rateLimiter.ExecuteAsync("github-api", async () => await _httpClient.GetAsync(apiUrl));
367+
await _rateLimitHandler.UpdateFromHeadersAsync(response);
368+
GitHubRateLimitResponse.ThrowIfLimited(response);
369+
370+
bool? isPublic = null;
371+
if (response.StatusCode == System.Net.HttpStatusCode.NotFound)
372+
{
373+
isPublic = false;
374+
}
375+
else if (response.IsSuccessStatusCode)
376+
{
377+
using var document = JsonDocument.Parse(await response.Content.ReadAsStringAsync());
378+
isPublic = document.RootElement.TryGetProperty("private", out var isPrivate)
379+
&& isPrivate.ValueKind == JsonValueKind.False;
380+
}
381+
382+
if (isPublic.HasValue)
383+
_cache.SetVisibility(repoKey, isPublic.Value);
384+
else
385+
_logger.LogWarning("[GitHubScraper] API returned {StatusCode} for {Url}", response.StatusCode, apiUrl);
386+
return isPublic;
387+
}
388+
catch (GitHubRateLimitedException)
389+
{
390+
throw;
391+
}
392+
catch (Exception ex)
393+
{
394+
_logger.LogWarning(ex, "[GitHubScraper] Failed to read {Url}", apiUrl);
395+
return null;
396+
}
397+
}
398+
316399
// Tag spellings of monorepos and package-named tags, most specific first
317400
private static IEnumerable<string> TagCandidates(string? packageId, string version)
318401
{

‎src/backend/PkgDiff.Infrastructure/Services/PubDevService.cs‎

Lines changed: 57 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -25,58 +25,75 @@ public async Task<string> GetReleaseNotesAsync(string packageId, string version)
2525
{
2626
try
2727
{
28-
var url = $"{_baseUrl}/{PackageUrlEncoder.EscapeSegment(packageId)}/changelog";
29-
var html = await _httpClient.GetStringAsync(url);
30-
var doc = new HtmlDocument();
31-
doc.LoadHtml(html);
28+
var packageUrl = $"{_baseUrl}/{PackageUrlEncoder.EscapeSegment(packageId)}";
29+
// The package's changelog page shows only the latest upload; each version's page keeps the file it shipped with
30+
return await ReadChangelogEntryAsync($"{packageUrl}/versions/{PackageUrlEncoder.EscapeSegment(version)}/changelog", version)
31+
?? await ReadChangelogEntryAsync($"{packageUrl}/changelog", version)
32+
?? string.Empty;
33+
}
34+
catch (Exception ex)
35+
{
36+
_logger.LogError(ex, "Error fetching release notes from Pub.dev for package {PackageId} version {Version}", packageId, version);
37+
return string.Empty;
38+
}
39+
}
3240

33-
var changelogNodes = doc.DocumentNode.SelectNodes("//div[@class='changelog-entry']");
34-
if (changelogNodes == null || !changelogNodes.Any())
35-
return string.Empty;
41+
// Null when the page or its entry for the version is missing
42+
private async Task<string?> ReadChangelogEntryAsync(string url, string version)
43+
{
44+
using var response = await _httpClient.GetAsync(url);
45+
if (!response.IsSuccessStatusCode)
46+
return null;
3647

37-
var versionId = version.Replace(".", ""); // pub.dev changelog anchors drop the dots
48+
var doc = new HtmlDocument();
49+
doc.LoadHtml(await response.Content.ReadAsStringAsync());
3850

39-
foreach (var node in changelogNodes)
40-
{
41-
var headerNode = node.SelectSingleNode(".//h2");
42-
if (headerNode != null && headerNode.InnerText.Trim().StartsWith(version))
43-
{
44-
var contentNode = node.SelectSingleNode(".//div[@class='changelog-content']");
45-
if (contentNode == null) return string.Empty;
51+
var changelogNodes = doc.DocumentNode.SelectNodes("//div[@class='changelog-entry']");
52+
if (changelogNodes == null)
53+
return null;
4654

47-
var rawHtml = contentNode.InnerHtml;
55+
foreach (var node in changelogNodes)
56+
{
57+
var headerNode = node.SelectSingleNode(".//h2");
58+
if (headerNode == null || !IsHeaderOf(headerNode.InnerText, version))
59+
continue;
4860

49-
rawHtml = Regex.Replace(rawHtml, "<br[^>]*>", "\n", RegexOptions.IgnoreCase);
50-
rawHtml = Regex.Replace(rawHtml, "</p>", "\n\n", RegexOptions.IgnoreCase);
51-
rawHtml = Regex.Replace(rawHtml, "<li[^>]*>", "\n- ", RegexOptions.IgnoreCase);
52-
rawHtml = Regex.Replace(rawHtml, "<h[1-6][^>]*>", "\n# ", RegexOptions.IgnoreCase);
61+
var contentNode = node.SelectSingleNode(".//div[@class='changelog-content']");
62+
if (contentNode == null) return string.Empty;
5363

54-
rawHtml = Regex.Replace(rawHtml, "<strong[^>]*>(.*?)</strong>", "**$1**", RegexOptions.IgnoreCase | RegexOptions.Singleline);
55-
rawHtml = Regex.Replace(rawHtml, "<b[^>]*>(.*?)</b>", "**$1**", RegexOptions.IgnoreCase | RegexOptions.Singleline);
56-
rawHtml = Regex.Replace(rawHtml, "<em[^>]*>(.*?)</em>", "*$1*", RegexOptions.IgnoreCase | RegexOptions.Singleline);
57-
rawHtml = Regex.Replace(rawHtml, "<i[^>]*>(.*?)</i>", "*$1*", RegexOptions.IgnoreCase | RegexOptions.Singleline);
58-
rawHtml = Regex.Replace(rawHtml, "<code[^>]*>(.*?)</code>", "`$1`", RegexOptions.IgnoreCase | RegexOptions.Singleline);
59-
rawHtml = Regex.Replace(rawHtml, "<pre[^>]*>(.*?)</pre>", "\n```\n$1\n```\n", RegexOptions.IgnoreCase | RegexOptions.Singleline);
64+
var rawHtml = contentNode.InnerHtml;
6065

61-
rawHtml = Regex.Replace(rawHtml, "<a[^>]+href=\"([^\"]+)\"[^>]*>(.*?)</a>", "[$2]($1)", RegexOptions.IgnoreCase | RegexOptions.Singleline);
66+
rawHtml = Regex.Replace(rawHtml, "<br[^>]*>", "\n", RegexOptions.IgnoreCase);
67+
rawHtml = Regex.Replace(rawHtml, "</p>", "\n\n", RegexOptions.IgnoreCase);
68+
rawHtml = Regex.Replace(rawHtml, "<li[^>]*>", "\n- ", RegexOptions.IgnoreCase);
69+
rawHtml = Regex.Replace(rawHtml, "<h[1-6][^>]*>", "\n# ", RegexOptions.IgnoreCase);
6270

63-
var tempDoc = new HtmlDocument();
64-
tempDoc.LoadHtml(rawHtml);
65-
var plainText = tempDoc.DocumentNode.InnerText;
71+
rawHtml = Regex.Replace(rawHtml, "<strong[^>]*>(.*?)</strong>", "**$1**", RegexOptions.IgnoreCase | RegexOptions.Singleline);
72+
rawHtml = Regex.Replace(rawHtml, "<b[^>]*>(.*?)</b>", "**$1**", RegexOptions.IgnoreCase | RegexOptions.Singleline);
73+
rawHtml = Regex.Replace(rawHtml, "<em[^>]*>(.*?)</em>", "*$1*", RegexOptions.IgnoreCase | RegexOptions.Singleline);
74+
rawHtml = Regex.Replace(rawHtml, "<i[^>]*>(.*?)</i>", "*$1*", RegexOptions.IgnoreCase | RegexOptions.Singleline);
75+
rawHtml = Regex.Replace(rawHtml, "<code[^>]*>(.*?)</code>", "`$1`", RegexOptions.IgnoreCase | RegexOptions.Singleline);
76+
rawHtml = Regex.Replace(rawHtml, "<pre[^>]*>(.*?)</pre>", "\n```\n$1\n```\n", RegexOptions.IgnoreCase | RegexOptions.Singleline);
6677

67-
plainText = Regex.Replace(plainText, @"\n{3,}", "\n\n");
78+
rawHtml = Regex.Replace(rawHtml, "<a[^>]+href=\"([^\"]+)\"[^>]*>(.*?)</a>", "[$2]($1)", RegexOptions.IgnoreCase | RegexOptions.Singleline);
6879

69-
return plainText.Trim();
70-
}
71-
}
80+
var tempDoc = new HtmlDocument();
81+
tempDoc.LoadHtml(rawHtml);
82+
var plainText = tempDoc.DocumentNode.InnerText;
7283

73-
return string.Empty;
74-
}
75-
catch (Exception ex)
76-
{
77-
_logger.LogError(ex, "Error fetching release notes from Pub.dev for package {PackageId} version {Version}", packageId, version);
78-
return string.Empty;
84+
plainText = Regex.Replace(plainText, @"\n{3,}", "\n\n");
85+
86+
return plainText.Trim();
7987
}
88+
89+
return null;
90+
}
91+
92+
private static bool IsHeaderOf(string headerText, string version)
93+
{
94+
var firstWord = headerText.Split((char[]?)null, 2, StringSplitOptions.RemoveEmptyEntries).FirstOrDefault() ?? string.Empty;
95+
return firstWord.Trim('[', ']', '(', ')', ':', ',').TrimStart('v', 'V')
96+
.Equals(version, StringComparison.OrdinalIgnoreCase);
8097
}
8198

8299
public async Task<string?> GetGitHubRepositoryUrlAsync(string packageId)

‎src/backend/PkgDiff.Tests/Outbound/RegistryUrlEscapingTests.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,13 @@ public async Task PubDevService_Changelog_EscapesPackageId()
113113

114114
await service.GetReleaseNotesAsync("../x", "1.0.0");
115115

116-
var request = Assert.Single(handler.Requests);
117-
Assert.Equal("https://pub.dev/packages/..%2Fx/changelog", request.AbsoluteUri);
116+
Assert.Equal(
117+
new[]
118+
{
119+
"https://pub.dev/packages/..%2Fx/versions/1.0.0/changelog",
120+
"https://pub.dev/packages/..%2Fx/changelog"
121+
},
122+
handler.Requests.Select(r => r.AbsoluteUri));
118123
}
119124

120125
[Fact]

‎src/backend/PkgDiff.Tests/Smoke/RegistrySmokeTests.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,13 @@ public async Task PubDev_SmokeAnalysis()
197197
Assert.Equal(string.Empty, unknownNotes);
198198

199199
Assert.Equal(
200-
new[] { PubDevVersionsUrl, PubDevChangelogUrl, PubDevChangelogUrl, PubDevChangelogUrl },
200+
new[]
201+
{
202+
PubDevVersionsUrl,
203+
PubDevVersionsUrl + "/1.6.0/changelog", PubDevChangelogUrl,
204+
PubDevVersionsUrl + "/1.4.0/changelog", PubDevChangelogUrl,
205+
PubDevVersionsUrl + "/0.0.1/changelog", PubDevChangelogUrl
206+
},
201207
handler.Requests);
202208
}
203209

0 commit comments

Comments
 (0)