From c04bb1915ef334e25887d058bbe534d1941dd837 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 29 Aug 2024 18:07:45 -0400 Subject: [PATCH 01/10] - added new method to delete old packages - updated SDK in global.json - validated regression and unit tests --- global.json | 2 +- .../Configuration/BaGetterOptions.cs | 6 +++++ .../Indexing/IPackageDeletionService.cs | 9 +++++++ .../Indexing/PackageDeletionService.cs | 18 +++++++++++++ .../Indexing/PackageIndexingService.cs | 27 +++++++++++++++++++ .../Services/PackageIndexingServiceTests.cs | 3 +++ 6 files changed, 64 insertions(+), 1 deletion(-) diff --git a/global.json b/global.json index 789bff3bd..f202156de 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "8.0.100", + "version": "8.0.401", "rollForward": "latestFeature", "allowPrerelease": false } diff --git a/src/BaGetter.Core/Configuration/BaGetterOptions.cs b/src/BaGetter.Core/Configuration/BaGetterOptions.cs index c69628091..a3e19ddd0 100644 --- a/src/BaGetter.Core/Configuration/BaGetterOptions.cs +++ b/src/BaGetter.Core/Configuration/BaGetterOptions.cs @@ -47,6 +47,12 @@ public class BaGetterOptions /// public uint MaxPackageSizeGiB { get; set; } = 8; + /// + /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// the older versions will be deleted. + /// + public uint? MaxVersionsPerPackage { get; set; } = null; + public DatabaseOptions Database { get; set; } public StorageOptions Storage { get; set; } diff --git a/src/BaGetter.Core/Indexing/IPackageDeletionService.cs b/src/BaGetter.Core/Indexing/IPackageDeletionService.cs index df4aea761..1f48c9b34 100644 --- a/src/BaGetter.Core/Indexing/IPackageDeletionService.cs +++ b/src/BaGetter.Core/Indexing/IPackageDeletionService.cs @@ -6,6 +6,15 @@ namespace BaGetter.Core; public interface IPackageDeletionService { + /// + /// Delete old versions of packages + /// + /// Current package object to clean + /// Maximum number of packages to keep + /// + /// Number of packages deleted + Task DeleteOldVersionsAsync(Package package, uint maxPackagesToKeep, CancellationToken cancellationToken); + /// /// Attempt to delete a package. /// diff --git a/src/BaGetter.Core/Indexing/PackageDeletionService.cs b/src/BaGetter.Core/Indexing/PackageDeletionService.cs index e1b468f80..c85416e52 100644 --- a/src/BaGetter.Core/Indexing/PackageDeletionService.cs +++ b/src/BaGetter.Core/Indexing/PackageDeletionService.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -88,4 +89,21 @@ private async Task TryHardDeletePackageAsync(string id, NuGetVersion versi return found; } + + public async Task DeleteOldVersionsAsync(Package package, uint maxPackages, CancellationToken cancellationToken) + { + // list all versions of the package + var versions = await _packages.FindAsync(package.Id, includeUnlisted: true, cancellationToken); + // sort by version and take everything except the last maxPackages + var versionsToDelete = versions + .OrderByDescending(p => p.Version) + .Skip((int)maxPackages) + .ToList(); + var deleted = 0; + foreach (var version in versions) + { + if (await TryDeletePackageAsync(package.Id, version.Version, cancellationToken)) deleted++; + } + return deleted; + } } diff --git a/src/BaGetter.Core/Indexing/PackageIndexingService.cs b/src/BaGetter.Core/Indexing/PackageIndexingService.cs index 90a47ff7c..c6d8e6cf7 100644 --- a/src/BaGetter.Core/Indexing/PackageIndexingService.cs +++ b/src/BaGetter.Core/Indexing/PackageIndexingService.cs @@ -16,10 +16,12 @@ public class PackageIndexingService : IPackageIndexingService private readonly SystemTime _time; private readonly IOptionsSnapshot _options; private readonly ILogger _logger; + private readonly IPackageDeletionService _packageDeletionService; public PackageIndexingService( IPackageDatabase packages, IPackageStorageService storage, + IPackageDeletionService packageDeletionService, ISearchIndexer search, SystemTime time, IOptionsSnapshot options, @@ -31,6 +33,7 @@ public PackageIndexingService( _time = time ?? throw new ArgumentNullException(nameof(time)); _options = options ?? throw new ArgumentNullException(nameof(options)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _packageDeletionService = packageDeletionService ?? throw new ArgumentNullException(nameof(packageDeletionService)); } public async Task IndexAsync(Stream packageStream, CancellationToken cancellationToken) @@ -153,6 +156,30 @@ await _storage.SavePackageContentAsync( await _search.IndexAsync(package, cancellationToken); + if (_options.Value.MaxVersionsPerPackage.HasValue) + { + try { + _logger.LogInformation( + "Deleting older packages for package {PackageId} {PackageVersion}", + package.Id, + package.NormalizedVersionString); + var deleted = await _packageDeletionService.DeleteOldVersionsAsync(package, _options.Value.MaxVersionsPerPackage.Value, cancellationToken); + _logger.LogInformation( + "Deleted {packages} older packages for package {PackageId} {PackageVersion}", + deleted, + package.Id, + package.NormalizedVersionString); + } + catch (Exception e) + { + _logger.LogError( + e, + "Failed to cleanup older versions of package {PackageId} {PackageVersion}", + package.Id, + package.NormalizedVersionString); + } + } + _logger.LogInformation( "Successfully indexed package {Id} {Version} in search", package.Id, diff --git a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs index a7375a0fb..b60c3fcae 100644 --- a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs +++ b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs @@ -15,6 +15,7 @@ public class PackageIndexingServiceTests private readonly Mock _packages; private readonly Mock _storage; private readonly Mock _search; + private readonly Mock _deleter; private readonly Mock _time; private readonly PackageIndexingService _target; private readonly BaGetterOptions _mockOptions; @@ -24,6 +25,7 @@ public PackageIndexingServiceTests() _packages = new Mock(MockBehavior.Strict); _storage = new Mock(MockBehavior.Strict); _search = new Mock(MockBehavior.Strict); + _deleter = new Mock(MockBehavior.Strict); _time = new Mock(MockBehavior.Loose); _mockOptions = new(); var options = new Mock>(MockBehavior.Strict); @@ -32,6 +34,7 @@ public PackageIndexingServiceTests() _target = new PackageIndexingService( _packages.Object, _storage.Object, + _deleter.Object, _search.Object, _time.Object, options.Object, From f65be0ce45deabd1bf3ad9042d5eef4b10e41f0d Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 29 Aug 2024 23:14:48 -0400 Subject: [PATCH 02/10] - added unit test for version deletion - added new in memory package database --- .../Indexing/PackageDeletionService.cs | 5 +- .../Indexing/PackageIndexingService.cs | 13 +- .../Services/PackageIndexingServiceTests.cs | 130 ++++++++++++++---- .../Support/InMemoryPackageDatabase.cs | 61 ++++++++ 4 files changed, 176 insertions(+), 33 deletions(-) create mode 100644 tests/BaGetter.Core.Tests/Support/InMemoryPackageDatabase.cs diff --git a/src/BaGetter.Core/Indexing/PackageDeletionService.cs b/src/BaGetter.Core/Indexing/PackageDeletionService.cs index c85416e52..8b3fe35b4 100644 --- a/src/BaGetter.Core/Indexing/PackageDeletionService.cs +++ b/src/BaGetter.Core/Indexing/PackageDeletionService.cs @@ -94,15 +94,16 @@ public async Task DeleteOldVersionsAsync(Package package, uint maxPackages, { // list all versions of the package var versions = await _packages.FindAsync(package.Id, includeUnlisted: true, cancellationToken); + if (versions is null || versions.Count <= maxPackages) return 0; // sort by version and take everything except the last maxPackages var versionsToDelete = versions .OrderByDescending(p => p.Version) .Skip((int)maxPackages) .ToList(); var deleted = 0; - foreach (var version in versions) + foreach (var version in versionsToDelete) { - if (await TryDeletePackageAsync(package.Id, version.Version, cancellationToken)) deleted++; + if (await TryHardDeletePackageAsync(package.Id, version.Version, cancellationToken)) deleted++; } return deleted; } diff --git a/src/BaGetter.Core/Indexing/PackageIndexingService.cs b/src/BaGetter.Core/Indexing/PackageIndexingService.cs index c6d8e6cf7..0fd3573dd 100644 --- a/src/BaGetter.Core/Indexing/PackageIndexingService.cs +++ b/src/BaGetter.Core/Indexing/PackageIndexingService.cs @@ -164,11 +164,14 @@ await _storage.SavePackageContentAsync( package.Id, package.NormalizedVersionString); var deleted = await _packageDeletionService.DeleteOldVersionsAsync(package, _options.Value.MaxVersionsPerPackage.Value, cancellationToken); - _logger.LogInformation( - "Deleted {packages} older packages for package {PackageId} {PackageVersion}", - deleted, - package.Id, - package.NormalizedVersionString); + if (deleted > 0) + { + _logger.LogInformation( + "Deleted {packages} older packages for package {PackageId} {PackageVersion}", + deleted, + package.Id, + package.NormalizedVersionString); + } } catch (Exception e) { diff --git a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs index b60c3fcae..6ee62319d 100644 --- a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs +++ b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.IO; using System.Threading.Tasks; +using BaGetter.Core.Tests.Support; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; @@ -12,29 +13,37 @@ namespace BaGetter.Core.Tests.Services; public class PackageIndexingServiceTests { - private readonly Mock _packages; + private readonly IPackageDatabase _packages; private readonly Mock _storage; private readonly Mock _search; - private readonly Mock _deleter; + private readonly IPackageDeletionService _deleter; private readonly Mock _time; private readonly PackageIndexingService _target; - private readonly BaGetterOptions _mockOptions; + private readonly BaGetterOptions _options; public PackageIndexingServiceTests() { - _packages = new Mock(MockBehavior.Strict); + _packages = new InMemoryPackageDatabase(); _storage = new Mock(MockBehavior.Strict); _search = new Mock(MockBehavior.Strict); - _deleter = new Mock(MockBehavior.Strict); + _options = new(); + + var optionsSnapshot = new Mock>(); + optionsSnapshot.Setup(o => o.Value).Returns(_options); + + _deleter = new PackageDeletionService( + _packages, + _storage.Object, + optionsSnapshot.Object, + Mock.Of>()); _time = new Mock(MockBehavior.Loose); - _mockOptions = new(); var options = new Mock>(MockBehavior.Strict); - options.Setup(o => o.Value).Returns(_mockOptions); + options.Setup(o => o.Value).Returns(_options); _target = new PackageIndexingService( - _packages.Object, + _packages, _storage.Object, - _deleter.Object, + _deleter, _search.Object, _time.Object, options.Object, @@ -47,7 +56,7 @@ public PackageIndexingServiceTests() public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteForbidden_ReturnsPackageAlreadyExists() { // Arrange - _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.False; + _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; var builder = new PackageBuilder { @@ -64,20 +73,30 @@ public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteForbidden_Retu }); var stream = new MemoryStream(); builder.Save(stream); - _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act var result = await _target.IndexAsync(stream, default); + var stream2 = new MemoryStream(); + builder.Save(stream); + + Assert.Equal(PackageIndexingResult.Success, result); + + // Act + var result2 = await _target.IndexAsync(stream, default); + // Assert - Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result); + Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result2); + } [Fact] public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_IndexesPackage() { // Arrange - _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.True; + _options.AllowPackageOverwrites = PackageOverwriteAllowed.True; var builder = new PackageBuilder { @@ -94,9 +113,9 @@ public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_Indexe }); var stream = new MemoryStream(); builder.Save(stream); - _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); - _packages.Setup(p => p.HardDeletePackageAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); - _packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); + //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); + //_packages.Setup(p => p.HardDeletePackageAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); + //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); @@ -114,7 +133,7 @@ public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_Indexe public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePrereleaseAllowed_IndexesPackage() { // Arrange - _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.PrereleaseOnly; + _options.AllowPackageOverwrites = PackageOverwriteAllowed.PrereleaseOnly; var builder = new PackageBuilder { @@ -131,9 +150,6 @@ public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePrer }); var stream = new MemoryStream(); builder.Save(stream); - _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); - _packages.Setup(p => p.HardDeletePackageAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); - _packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); @@ -151,7 +167,7 @@ public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePrer public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwriteForbidden_ReturnsPackageAlreadyExists() { // Arrange - _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.False; + _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; var builder = new PackageBuilder { @@ -168,20 +184,32 @@ public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwriteForb }); var stream = new MemoryStream(); builder.Save(stream); - _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); + + _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act var result = await _target.IndexAsync(stream, default); + var stream2 = new MemoryStream(); + builder.Save(stream); + + Assert.Equal(PackageIndexingResult.Success, result); + + // Act + var result2 = await _target.IndexAsync(stream, default); + // Assert - Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result); + Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result2); } [Fact] public async Task IndexAsync_WithValidPackage_ReturnsSuccess() { // Arrange - _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.False; + _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; var builder = new PackageBuilder { Id = "bagetter-test", @@ -197,8 +225,8 @@ public async Task IndexAsync_WithValidPackage_ReturnsSuccess() }); var stream = new MemoryStream(); builder.Save(stream); - _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); - _packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); + //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); + //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); @@ -211,6 +239,56 @@ public async Task IndexAsync_WithValidPackage_ReturnsSuccess() Assert.Equal(PackageIndexingResult.Success, result); } + [Fact] + public async Task IndexAsync_WithValidPackage_CleansOldVersions() + { + // Arrange + _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; + _options.MaxVersionsPerPackage = 5; + // Add 10 packages + for (var i = 0; i < 10; i++) + { + var builder = new PackageBuilder + { + Id = "bagetter-test", + Version = NuGetVersion.Parse($"1.0.{i}"), + Description = "Test Description", + }; + builder.Authors.Add("Test Author"); + var assemblyFile = GetType().Assembly.Location; + builder.Files.Add(new PhysicalPackageFile + { + SourcePath = assemblyFile, + TargetPath = "lib/Test.dll" + }); + var stream = new MemoryStream(); + builder.Save(stream); + //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); + //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); + + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + + // Act + var result = await _target.IndexAsync(stream, default); + + // Assert + Assert.Equal(PackageIndexingResult.Success, result); + + var packageCount = await _packages.FindAsync(builder.Id, true, default); + if (i < 5) + { + Assert.Equal(i + 1, packageCount.Count); + } + else + { + Assert.Equal(5, packageCount.Count); + } + } + } + + [Fact] public async Task WhenDatabaseAddFailsBecausePackageAlreadyExists_ReturnsPackageAlreadyExists() { diff --git a/tests/BaGetter.Core.Tests/Support/InMemoryPackageDatabase.cs b/tests/BaGetter.Core.Tests/Support/InMemoryPackageDatabase.cs new file mode 100644 index 000000000..0595a34c8 --- /dev/null +++ b/tests/BaGetter.Core.Tests/Support/InMemoryPackageDatabase.cs @@ -0,0 +1,61 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using NuGet.Versioning; + +namespace BaGetter.Core.Tests.Support; +public class InMemoryPackageDatabase : IPackageDatabase +{ + private readonly List _packages = new List(); + + public Task AddAsync(Package package, CancellationToken cancellationToken) + { + _packages.Add(package); + return Task.FromResult(PackageAddResult.Success); + } + + public Task AddDownloadAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + { + throw new NotImplementedException(); + } + + public async Task ExistsAsync(string id, CancellationToken cancellationToken) + { + throw new NotImplementedException(); + } + + public Task ExistsAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + { + var exists = _packages.Any(p => p.Id == id && p.Version == version); + return Task.FromResult(exists); + } + + public Task> FindAsync(string id, bool includeUnlisted, CancellationToken cancellationToken) + { + return Task.FromResult((IReadOnlyList)_packages.Where(p => p.Id == id).ToList().AsReadOnly()); + } + + public async Task FindOrNullAsync(string id, NuGetVersion version, bool includeUnlisted, CancellationToken cancellationToken) + { + throw new NotImplementedException(); + } + + public Task HardDeletePackageAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + { + var removed = _packages.RemoveAll(p => p.Id == id && p.Version == version); + return Task.FromResult(removed > 0); + } + + public async Task RelistPackageAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + { + throw new NotImplementedException(); + } + + public Task UnlistPackageAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + { + return Task.FromResult(true); + } +} From 1c61d16a7824ac6a72c98e179e6e6f577ffad9ec Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 4 Sep 2024 15:40:30 -0400 Subject: [PATCH 03/10] added new test class for in memory tests and updated documentation --- docs/docs/configuration.md | 14 + .../PackageIndexingServiceInMemoryTests.cs | 289 ++++++++++++++++++ .../Services/PackageIndexingServiceTests.cs | 130 ++------ 3 files changed, 329 insertions(+), 104 deletions(-) create mode 100644 tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs diff --git a/docs/docs/configuration.md b/docs/docs/configuration.md index b617df560..a443862fa 100644 --- a/docs/docs/configuration.md +++ b/docs/docs/configuration.md @@ -154,6 +154,20 @@ downloaded if you know the package's id and version. You can override this behav } ``` +## Enable package auto-deletion + +If your build server generates many nuget packages, your BaGet server can quickly run out of space. To avoid this issue, `MaxVersionsPerPackage` can be configured to auto-delete packages older packages when a new one is uploaded. This will use the `HardDelete` option detailed above and will unlist and delete the files for the older packages. By default this value is not configured and no packages will be deleted automatically. + +```json +{ + ... + + "MaxVersionsPerPackage ": 5, + + ... +} +``` + ## Enable package overwrites Normally, BaGetter will reject a package upload if the id and version are already taken. This is to maintain the [immutability of semantically versioned packages](https://learn.microsoft.com/azure/devops/artifacts/artifacts-key-concepts?view=azure-devops#immutability). diff --git a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs new file mode 100644 index 000000000..c27609d00 --- /dev/null +++ b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs @@ -0,0 +1,289 @@ +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using BaGetter.Core.Tests.Support; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; +using NuGet.Packaging; +using NuGet.Versioning; +using Xunit; + +namespace BaGetter.Core.Tests.Services; + +/// +/// These tests are similar to the ones in , but they use an in-memory package database. +/// +public class PackageIndexingServiceInMemoryTests +{ + private readonly IPackageDatabase _packages; + private readonly Mock _storage; + private readonly Mock _search; + private readonly IPackageDeletionService _deleter; + private readonly Mock _time; + private readonly PackageIndexingService _target; + private readonly BaGetterOptions _options; + + public PackageIndexingServiceInMemoryTests() + { + _packages = new InMemoryPackageDatabase(); + _storage = new Mock(MockBehavior.Strict); + _search = new Mock(MockBehavior.Strict); + _options = new(); + + var optionsSnapshot = new Mock>(); + optionsSnapshot.Setup(o => o.Value).Returns(_options); + + _deleter = new PackageDeletionService( + _packages, + _storage.Object, + optionsSnapshot.Object, + Mock.Of>()); + _time = new Mock(MockBehavior.Loose); + var options = new Mock>(MockBehavior.Strict); + options.Setup(o => o.Value).Returns(_options); + + _target = new PackageIndexingService( + _packages, + _storage.Object, + _deleter, + _search.Object, + _time.Object, + options.Object, + Mock.Of>()); + } + + // TODO: Add malformed package tests + + [Fact] + public async Task IndexIMAsync_WhenPackageAlreadyExists_AndOverwriteForbidden_ReturnsPackageAlreadyExists() + { + // Arrange + _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; + + var builder = new PackageBuilder + { + Id = "bagetter-test", + Version = NuGetVersion.Parse("1.0.0"), + Description = "Test Description", + }; + builder.Authors.Add("Test Author"); + var assemblyFile = GetType().Assembly.Location; + builder.Files.Add(new PhysicalPackageFile + { + SourcePath = assemblyFile, + TargetPath = "lib/Test.dll" + }); + var stream = new MemoryStream(); + builder.Save(stream); + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + + // Act + var result = await _target.IndexAsync(stream, default); + + var stream2 = new MemoryStream(); + builder.Save(stream); + + Assert.Equal(PackageIndexingResult.Success, result); + + // Act + var result2 = await _target.IndexAsync(stream, default); + + // Assert + Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result2); + + } + + [Fact] + public async Task IndexIMAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_IndexesPackage() + { + // Arrange + _options.AllowPackageOverwrites = PackageOverwriteAllowed.True; + + var builder = new PackageBuilder + { + Id = "bagetter-test", + Version = NuGetVersion.Parse("1.0.0"), + Description = "Test Description", + }; + builder.Authors.Add("Test Author"); + var assemblyFile = GetType().Assembly.Location; + builder.Files.Add(new PhysicalPackageFile + { + SourcePath = assemblyFile, + TargetPath = "lib/Test.dll" + }); + var stream = new MemoryStream(); + builder.Save(stream); + + _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + + // Act + var result = await _target.IndexAsync(stream, default); + + // Assert + Assert.Equal(PackageIndexingResult.Success, result); + } + + [Fact] + public async Task IndexIMAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePrereleaseAllowed_IndexesPackage() + { + // Arrange + _options.AllowPackageOverwrites = PackageOverwriteAllowed.PrereleaseOnly; + + var builder = new PackageBuilder + { + Id = "bagetter-test", + Version = NuGetVersion.Parse("1.0.0-beta"), + Description = "Test Description", + }; + builder.Authors.Add("Test Author"); + var assemblyFile = GetType().Assembly.Location; + builder.Files.Add(new PhysicalPackageFile + { + SourcePath = assemblyFile, + TargetPath = "lib/Test.dll" + }); + var stream = new MemoryStream(); + builder.Save(stream); + + _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + + // Act + var result = await _target.IndexAsync(stream, default); + + // Assert + Assert.Equal(PackageIndexingResult.Success, result); + } + + [Fact] + public async Task IndexIMAsync_WhenPrereleasePackageAlreadyExists_AndOverwriteForbidden_ReturnsPackageAlreadyExists() + { + // Arrange + _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; + + var builder = new PackageBuilder + { + Id = "bagetter-test", + Version = NuGetVersion.Parse("1.0.0-beta"), + Description = "Test Description", + }; + builder.Authors.Add("Test Author"); + var assemblyFile = GetType().Assembly.Location; + builder.Files.Add(new PhysicalPackageFile + { + SourcePath = assemblyFile, + TargetPath = "lib/Test.dll" + }); + var stream = new MemoryStream(); + builder.Save(stream); + + _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + + // Act + var result = await _target.IndexAsync(stream, default); + + var stream2 = new MemoryStream(); + builder.Save(stream); + + Assert.Equal(PackageIndexingResult.Success, result); + + // Act + var result2 = await _target.IndexAsync(stream, default); + + // Assert + Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result2); + } + + [Fact] + public async Task IndexIMAsync_WithValidPackage_ReturnsSuccess() + { + // Arrange + _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; + var builder = new PackageBuilder + { + Id = "bagetter-test", + Version = NuGetVersion.Parse("1.0.0"), + Description = "Test Description", + }; + builder.Authors.Add("Test Author"); + var assemblyFile = GetType().Assembly.Location; + builder.Files.Add(new PhysicalPackageFile + { + SourcePath = assemblyFile, + TargetPath = "lib/Test.dll" + }); + var stream = new MemoryStream(); + builder.Save(stream); + + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + + // Act + var result = await _target.IndexAsync(stream, default); + + // Assert + Assert.Equal(PackageIndexingResult.Success, result); + } + + [Fact] + public async Task IndexIMAsync_WithValidPackage_CleansOldVersions() + { + // Arrange + _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; + _options.MaxVersionsPerPackage = 5; + // Add 10 packages + for (var i = 0; i < 10; i++) + { + var builder = new PackageBuilder + { + Id = "bagetter-test", + Version = NuGetVersion.Parse($"1.0.{i}"), + Description = "Test Description", + }; + builder.Authors.Add("Test Author"); + var assemblyFile = GetType().Assembly.Location; + builder.Files.Add(new PhysicalPackageFile + { + SourcePath = assemblyFile, + TargetPath = "lib/Test.dll" + }); + var stream = new MemoryStream(); + builder.Save(stream); + //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); + //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); + + _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + + // Act + var result = await _target.IndexAsync(stream, default); + + // Assert + Assert.Equal(PackageIndexingResult.Success, result); + + var packageCount = await _packages.FindAsync(builder.Id, true, default); + if (i < 5) + { + Assert.Equal(i + 1, packageCount.Count); + } + else + { + Assert.Equal(5, packageCount.Count); + } + } + } + +} diff --git a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs index 6ee62319d..d353b8194 100644 --- a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs +++ b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs @@ -1,7 +1,6 @@ using System.Collections.Generic; using System.IO; using System.Threading.Tasks; -using BaGetter.Core.Tests.Support; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; @@ -13,37 +12,29 @@ namespace BaGetter.Core.Tests.Services; public class PackageIndexingServiceTests { - private readonly IPackageDatabase _packages; + private readonly Mock _packages; private readonly Mock _storage; private readonly Mock _search; - private readonly IPackageDeletionService _deleter; private readonly Mock _time; + private readonly Mock _deleter; private readonly PackageIndexingService _target; - private readonly BaGetterOptions _options; + private readonly BaGetterOptions _mockOptions; public PackageIndexingServiceTests() { - _packages = new InMemoryPackageDatabase(); + _packages = new Mock(MockBehavior.Strict); _storage = new Mock(MockBehavior.Strict); _search = new Mock(MockBehavior.Strict); - _options = new(); - - var optionsSnapshot = new Mock>(); - optionsSnapshot.Setup(o => o.Value).Returns(_options); - - _deleter = new PackageDeletionService( - _packages, - _storage.Object, - optionsSnapshot.Object, - Mock.Of>()); _time = new Mock(MockBehavior.Loose); + _deleter = new Mock(MockBehavior.Strict); + _mockOptions = new(); var options = new Mock>(MockBehavior.Strict); - options.Setup(o => o.Value).Returns(_options); + options.Setup(o => o.Value).Returns(_mockOptions); _target = new PackageIndexingService( - _packages, + _packages.Object, _storage.Object, - _deleter, + _deleter.Object, _search.Object, _time.Object, options.Object, @@ -56,7 +47,7 @@ public PackageIndexingServiceTests() public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteForbidden_ReturnsPackageAlreadyExists() { // Arrange - _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; + _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.False; var builder = new PackageBuilder { @@ -73,30 +64,20 @@ public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteForbidden_Retu }); var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); // Act var result = await _target.IndexAsync(stream, default); - var stream2 = new MemoryStream(); - builder.Save(stream); - - Assert.Equal(PackageIndexingResult.Success, result); - - // Act - var result2 = await _target.IndexAsync(stream, default); - // Assert - Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result2); - + Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result); } [Fact] public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_IndexesPackage() { // Arrange - _options.AllowPackageOverwrites = PackageOverwriteAllowed.True; + _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.True; var builder = new PackageBuilder { @@ -113,9 +94,9 @@ public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_Indexe }); var stream = new MemoryStream(); builder.Save(stream); - //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); - //_packages.Setup(p => p.HardDeletePackageAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); - //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); + _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); + _packages.Setup(p => p.HardDeletePackageAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); + _packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); @@ -133,7 +114,7 @@ public async Task IndexAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_Indexe public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePrereleaseAllowed_IndexesPackage() { // Arrange - _options.AllowPackageOverwrites = PackageOverwriteAllowed.PrereleaseOnly; + _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.PrereleaseOnly; var builder = new PackageBuilder { @@ -150,6 +131,9 @@ public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePrer }); var stream = new MemoryStream(); builder.Save(stream); + _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); + _packages.Setup(p => p.HardDeletePackageAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); + _packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); @@ -167,7 +151,7 @@ public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePrer public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwriteForbidden_ReturnsPackageAlreadyExists() { // Arrange - _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; + _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.False; var builder = new PackageBuilder { @@ -184,32 +168,20 @@ public async Task IndexAsync_WhenPrereleasePackageAlreadyExists_AndOverwriteForb }); var stream = new MemoryStream(); builder.Save(stream); - - _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(true); // Act var result = await _target.IndexAsync(stream, default); - var stream2 = new MemoryStream(); - builder.Save(stream); - - Assert.Equal(PackageIndexingResult.Success, result); - - // Act - var result2 = await _target.IndexAsync(stream, default); - // Assert - Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result2); + Assert.Equal(PackageIndexingResult.PackageAlreadyExists, result); } [Fact] public async Task IndexAsync_WithValidPackage_ReturnsSuccess() { // Arrange - _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; + _mockOptions.AllowPackageOverwrites = PackageOverwriteAllowed.False; var builder = new PackageBuilder { Id = "bagetter-test", @@ -225,8 +197,8 @@ public async Task IndexAsync_WithValidPackage_ReturnsSuccess() }); var stream = new MemoryStream(); builder.Save(stream); - //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); - //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); + _packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); + _packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); @@ -239,56 +211,6 @@ public async Task IndexAsync_WithValidPackage_ReturnsSuccess() Assert.Equal(PackageIndexingResult.Success, result); } - [Fact] - public async Task IndexAsync_WithValidPackage_CleansOldVersions() - { - // Arrange - _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; - _options.MaxVersionsPerPackage = 5; - // Add 10 packages - for (var i = 0; i < 10; i++) - { - var builder = new PackageBuilder - { - Id = "bagetter-test", - Version = NuGetVersion.Parse($"1.0.{i}"), - Description = "Test Description", - }; - builder.Authors.Add("Test Author"); - var assemblyFile = GetType().Assembly.Location; - builder.Files.Add(new PhysicalPackageFile - { - SourcePath = assemblyFile, - TargetPath = "lib/Test.dll" - }); - var stream = new MemoryStream(); - builder.Save(stream); - //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); - //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); - - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); - - // Act - var result = await _target.IndexAsync(stream, default); - - // Assert - Assert.Equal(PackageIndexingResult.Success, result); - - var packageCount = await _packages.FindAsync(builder.Id, true, default); - if (i < 5) - { - Assert.Equal(i + 1, packageCount.Count); - } - else - { - Assert.Equal(5, packageCount.Count); - } - } - } - - [Fact] public async Task WhenDatabaseAddFailsBecausePackageAlreadyExists_ReturnsPackageAlreadyExists() { From f7edf52c91c19dfa3feac546ee4899464562511d Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 11 Sep 2024 14:04:19 -0400 Subject: [PATCH 04/10] rewrote cleanup logic to limit packages per major/minor/patch/pre --- .../Configuration/BaGetterOptions.cs | 25 +++- src/BaGetter.Core/Entities/Package.cs | 2 + .../Indexing/IPackageDeletionService.cs | 16 ++- .../Indexing/PackageDeletionService.cs | 100 ++++++++++++++- .../Indexing/PackageIndexingService.cs | 10 +- .../PackageIndexingServiceInMemoryTests.cs | 121 +++++++++++------- .../Support/NullStorageService.cs | 43 +++++++ 7 files changed, 255 insertions(+), 62 deletions(-) create mode 100644 tests/BaGetter.Core.Tests/Support/NullStorageService.cs diff --git a/src/BaGetter.Core/Configuration/BaGetterOptions.cs b/src/BaGetter.Core/Configuration/BaGetterOptions.cs index a3e19ddd0..cdf84489c 100644 --- a/src/BaGetter.Core/Configuration/BaGetterOptions.cs +++ b/src/BaGetter.Core/Configuration/BaGetterOptions.cs @@ -49,9 +49,32 @@ public class BaGetterOptions /// /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// The limit is applied to each major version of the package, and if the limit is exceeded, /// the older versions will be deleted. /// - public uint? MaxVersionsPerPackage { get; set; } = null; + public uint? MaxHistoryPerMajorVersion { get; set; } = null; + + /// + /// This corresponds to the maximum number of minor versions for each major version. + /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// The limit is applied to each minor version of the package, and if the limit is exceeded, + /// the older versions will be deleted. + /// + public uint? MaxHistoryPerMinorVersion { get; set; } + + /// + /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// The limit is applied to each patch number of the package, and if the limit is exceeded, + /// the older versions will be deleted. + /// + public uint? MaxHistoryPerPatch { get; set; } + + /// + /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// The limit is applied to each pre-release of the package, and if the limit is exceeded, + /// the older versions will be deleted. + /// + public uint? MaxHistoryPerPrerelease { get; set; } public DatabaseOptions Database { get; set; } diff --git a/src/BaGetter.Core/Entities/Package.cs b/src/BaGetter.Core/Entities/Package.cs index d681e9f9e..d3c44ebdf 100644 --- a/src/BaGetter.Core/Entities/Package.cs +++ b/src/BaGetter.Core/Entities/Package.cs @@ -1,10 +1,12 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using NuGet.Versioning; namespace BaGetter.Core; // See NuGetGallery's: https://github.com/NuGet/NuGetGallery/blob/master/src/NuGetGallery.Core/Entities/Package.cs +[DebuggerDisplay("{Id} {Version}")] public class Package { public int Key { get; set; } diff --git a/src/BaGetter.Core/Indexing/IPackageDeletionService.cs b/src/BaGetter.Core/Indexing/IPackageDeletionService.cs index 1f48c9b34..3934873f6 100644 --- a/src/BaGetter.Core/Indexing/IPackageDeletionService.cs +++ b/src/BaGetter.Core/Indexing/IPackageDeletionService.cs @@ -7,13 +7,19 @@ namespace BaGetter.Core; public interface IPackageDeletionService { /// - /// Delete old versions of packages + /// This method deletes old versions of a package. + /// This leverages semver 2.0 - and assume a package is major.minor.patch-prerelease.build + /// It can leverage the to list all versions of a package and then delete all but the last versions. + /// It also takes into account the , and parameters to further filter the versions to delete. /// - /// Current package object to clean - /// Maximum number of packages to keep - /// + /// Package name + /// Maximum of major versions to keep (optional) + /// Maximum of minor versions to keep (optional) + /// Maximum of patch versions to keep (optional) + /// Maximum of pre-release versions (optional) + /// Cancel the operation /// Number of packages deleted - Task DeleteOldVersionsAsync(Package package, uint maxPackagesToKeep, CancellationToken cancellationToken); + Task DeleteOldVersionsAsync(Package package, uint? maxMajor, uint? maxMinor, uint? maxPatch, uint? maxPrerelease, CancellationToken cancellationToken); /// /// Attempt to delete a package. diff --git a/src/BaGetter.Core/Indexing/PackageDeletionService.cs b/src/BaGetter.Core/Indexing/PackageDeletionService.cs index 8b3fe35b4..4ec24bc4a 100644 --- a/src/BaGetter.Core/Indexing/PackageDeletionService.cs +++ b/src/BaGetter.Core/Indexing/PackageDeletionService.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -90,16 +91,77 @@ private async Task TryHardDeletePackageAsync(string id, NuGetVersion versi return found; } - public async Task DeleteOldVersionsAsync(Package package, uint maxPackages, CancellationToken cancellationToken) + private List GetValidVersions(IEnumerable versions, Func getParent, Func getSelector, int versionsToKeep) + where S : IComparable, IEquatable + where T : IComparable, IEquatable + { + var validVersions = versions + // for each parent group + .GroupBy(v => getParent(v)) + // get all versions by selector + .SelectMany(g => g.Select(k => (parent: g.Key, selector: getSelector(k))) + .Distinct() + .OrderByDescending(k => k.selector) + .Take(versionsToKeep)) + .ToList(); + return versions.Where(k => validVersions.Any(v => getParent(k).Equals(v.parent) && getSelector(k).Equals(v.selector))).ToList(); + } + + public async Task DeleteOldVersionsAsync(Package package, uint? maxMajor, uint? maxMinor, uint? maxPatch, uint? maxPrerelease, CancellationToken cancellationToken) { // list all versions of the package - var versions = await _packages.FindAsync(package.Id, includeUnlisted: true, cancellationToken); - if (versions is null || versions.Count <= maxPackages) return 0; + var packages = await _packages.FindAsync(package.Id, includeUnlisted: true, cancellationToken); + if (packages is null || packages.Count <= maxMajor) return 0; + + var goodVersions = new HashSet(); + + if (maxMajor.HasValue) + { + goodVersions = GetValidVersions(packages.Select(t => t.Version), v => 0, v => v.Major, (int)maxMajor).ToHashSet(); + } + else + { + goodVersions = packages.Select(p => p.Version).ToHashSet(); + } + + if (maxMinor.HasValue) + { + goodVersions.IntersectWith(GetValidVersions(goodVersions, v => (v.Major), v => v.Minor, (int)maxMinor)); + } + + if (maxPatch.HasValue) + { + goodVersions.IntersectWith(GetValidVersions(goodVersions, v => (v.Major, v.Minor), v => v.Patch, (int)maxPatch)); + } + + if (maxPrerelease.HasValue) + { + // this assume we have something like 1.1.1-alpha.1 - alpha is the release type + var preReleases = packages.Select(p => p.Version).Where(p => p.IsPrerelease).ToList(); + // this will give us 'alpha' or 'beta' etc + var prereleaseTypes = preReleases + .Select(v => v.ReleaseLabels?.FirstOrDefault()) + .Where(lb => lb is not null) + .Distinct(); + + var allPreReleaseValidVersions = new HashSet(); + foreach (var preReleaseType in prereleaseTypes) + { + var preReleaseVersions = preReleases.Where(p => p.ReleaseLabels!.FirstOrDefault() == preReleaseType + && GetPreReleaseBuild(p) is not null).ToList(); + + + allPreReleaseValidVersions.UnionWith + (GetValidVersions(preReleaseVersions, + v => (v.Major, v.Minor, v.Patch), v => GetPreReleaseBuild(v).Value, (int)maxPrerelease)); + + } + goodVersions.IntersectWith(allPreReleaseValidVersions); + } + // sort by version and take everything except the last maxPackages - var versionsToDelete = versions - .OrderByDescending(p => p.Version) - .Skip((int)maxPackages) - .ToList(); + var versionsToDelete = packages.Where(p => !goodVersions.Contains(p.Version)).ToList(); + var deleted = 0; foreach (var version in versionsToDelete) { @@ -107,4 +169,28 @@ public async Task DeleteOldVersionsAsync(Package package, uint maxPackages, } return deleted; } + + /// + /// If we have 1.1.1-alpha.1 , this will return 1 + /// or null if not valid + /// + /// + private int? GetPreReleaseBuild(NuGetVersion nuGetVersion) + { + if (nuGetVersion.IsPrerelease && nuGetVersion.ReleaseLabels != null) + { + // Assuming the last part of the release label is the build number + var lastLabel = nuGetVersion.ReleaseLabels.LastOrDefault(); + if (int.TryParse(lastLabel, out var buildNumber)) + { + return buildNumber; + } + else + { + _logger.LogWarning("Could not parse build number from prerelease label {PrereleaseLabel} - prerelease number is expected to be like 2.3.4-alpha.1 where 1 is prerelease", nuGetVersion); + } + } + return null; + } + } diff --git a/src/BaGetter.Core/Indexing/PackageIndexingService.cs b/src/BaGetter.Core/Indexing/PackageIndexingService.cs index 0fd3573dd..374cf2618 100644 --- a/src/BaGetter.Core/Indexing/PackageIndexingService.cs +++ b/src/BaGetter.Core/Indexing/PackageIndexingService.cs @@ -156,14 +156,20 @@ await _storage.SavePackageContentAsync( await _search.IndexAsync(package, cancellationToken); - if (_options.Value.MaxVersionsPerPackage.HasValue) + if (_options.Value.MaxHistoryPerMajorVersion.HasValue) { try { _logger.LogInformation( "Deleting older packages for package {PackageId} {PackageVersion}", package.Id, package.NormalizedVersionString); - var deleted = await _packageDeletionService.DeleteOldVersionsAsync(package, _options.Value.MaxVersionsPerPackage.Value, cancellationToken); + var deleted = await _packageDeletionService.DeleteOldVersionsAsync( + package, + _options.Value.MaxHistoryPerMajorVersion, + _options.Value.MaxHistoryPerMinorVersion, + _options.Value.MaxHistoryPerPatch, + _options.Value.MaxHistoryPerPrerelease, + cancellationToken); if (deleted > 0) { _logger.LogInformation( diff --git a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs index c27609d00..08472242d 100644 --- a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs +++ b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs @@ -1,5 +1,7 @@ +using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Threading.Tasks; using BaGetter.Core.Tests.Support; using Microsoft.Extensions.Logging; @@ -17,7 +19,7 @@ namespace BaGetter.Core.Tests.Services; public class PackageIndexingServiceInMemoryTests { private readonly IPackageDatabase _packages; - private readonly Mock _storage; + private readonly IPackageStorageService _storage; private readonly Mock _search; private readonly IPackageDeletionService _deleter; private readonly Mock _time; @@ -27,7 +29,9 @@ public class PackageIndexingServiceInMemoryTests public PackageIndexingServiceInMemoryTests() { _packages = new InMemoryPackageDatabase(); - _storage = new Mock(MockBehavior.Strict); + var storageService = new NullStorageService(); + _storage = new PackageStorageService(storageService, Mock.Of>()); + _search = new Mock(MockBehavior.Strict); _options = new(); @@ -36,7 +40,7 @@ public PackageIndexingServiceInMemoryTests() _deleter = new PackageDeletionService( _packages, - _storage.Object, + _storage, optionsSnapshot.Object, Mock.Of>()); _time = new Mock(MockBehavior.Loose); @@ -45,7 +49,7 @@ public PackageIndexingServiceInMemoryTests() _target = new PackageIndexingService( _packages, - _storage.Object, + _storage, _deleter, _search.Object, _time.Object, @@ -76,7 +80,6 @@ public async Task IndexIMAsync_WhenPackageAlreadyExists_AndOverwriteForbidden_Re }); var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -117,8 +120,6 @@ public async Task IndexIMAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_Inde var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); @@ -151,9 +152,6 @@ public async Task IndexIMAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePr var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -185,9 +183,6 @@ public async Task IndexIMAsync_WhenPrereleasePackageAlreadyExists_AndOverwriteFo var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -226,8 +221,6 @@ public async Task IndexIMAsync_WithValidPackage_ReturnsSuccess() var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -242,48 +235,82 @@ public async Task IndexIMAsync_WithValidPackage_CleansOldVersions() { // Arrange _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; - _options.MaxVersionsPerPackage = 5; + _options.MaxHistoryPerMajorVersion = 2; + _options.MaxHistoryPerMinorVersion = 2; + _options.MaxHistoryPerPatch = 5; + _options.MaxHistoryPerPrerelease = 5; // Add 10 packages - for (var i = 0; i < 10; i++) + for (var major = 1; major < 4; major++) { - var builder = new PackageBuilder - { - Id = "bagetter-test", - Version = NuGetVersion.Parse($"1.0.{i}"), - Description = "Test Description", - }; - builder.Authors.Add("Test Author"); - var assemblyFile = GetType().Assembly.Location; - builder.Files.Add(new PhysicalPackageFile + for (var minor = 1; minor < 4; minor++) { - SourcePath = assemblyFile, - TargetPath = "lib/Test.dll" - }); - var stream = new MemoryStream(); - builder.Save(stream); - //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); - //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); + for (var patch = 1; patch < 7; patch++) + { + await StoreVersion(NuGetVersion.Parse($"{major}.{minor}.{patch}")); + for (var prerelease = 1; prerelease < 7; prerelease++) + { + await StoreVersion(NuGetVersion.Parse($"{major}.{minor}.{patch}-staging.{prerelease}")); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); + var version = NuGetVersion.Parse($"{major}.{minor}.{patch}-beta.{prerelease}"); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + var builder = await StoreVersion(version); - // Act - var result = await _target.IndexAsync(stream, default); + var packageVersions = await _packages.FindAsync(builder.Id, true, default); + var majorCount = packageVersions.Select(p => p.Version.Major).Distinct().Count(); + Assert.Equal(majorCount, Math.Min(major, (int)_options.MaxHistoryPerMajorVersion)); + Assert.True(majorCount <= _options.MaxHistoryPerMajorVersion, $"Major version {major} has {majorCount} packages"); - // Assert - Assert.Equal(PackageIndexingResult.Success, result); + // validate maximum number of minor versions for each major version. + var minorVersions = packageVersions.GroupBy(m => m.Version.Major) + .Select(gp => (version: gp.Key, versionCount: gp.Select(p => p.Version.Major + "." + p.Version.Minor).Distinct().Count())).ToList(); + Assert.All(minorVersions, g => Assert.True(g.versionCount <= _options.MaxHistoryPerMinorVersion, $"Minor version {g.version} has {g.versionCount} packages")); - var packageCount = await _packages.FindAsync(builder.Id, true, default); - if (i < 5) - { - Assert.Equal(i + 1, packageCount.Count); - } - else - { - Assert.Equal(5, packageCount.Count); + // validate maximum number of minor versions for each major version. + var patches = packageVersions.GroupBy(m => (m.Version.Major, m.Version.Minor)) + .Select(gp => (version: gp.Key, versionCount: gp.Select(p => p.Version.Major + "." + p.Version.Minor + "." + p.Version.Patch).Distinct().Count())).ToList(); + Assert.All(patches, g => Assert.True(g.versionCount <= _options.MaxHistoryPerPatch, $"Patch version {g.version} has {g.versionCount} packages")); + + // validate maximum number of beta versions for each major,minor,patch version. + var betaVersions = packageVersions.Where(p => p.IsPrerelease && p.Version.ReleaseLabels.First() == "beta") + .GroupBy(m => (m.Version.Major, m.Version.Minor, m.Version.Patch)) + .Select(gp => (version: gp.Key, versionCount: gp.Select(p => p.Version.Major + "." + p.Version.Minor + "." + p.Version.Patch).Distinct().Count())).ToList(); + Assert.All(betaVersions, g => Assert.True(g.versionCount <= _options.MaxHistoryPerPatch, $"Pre-Release version {g.version} has {g.versionCount} packages")); + + + } + } } + } } + private async Task StoreVersion(NuGetVersion version) + { + var builder = new PackageBuilder + { + Id = "bagetter-test", + Version = version, + Description = "Test Description", + }; + builder.Authors.Add("Test Author"); + var assemblyFile = GetType().Assembly.Location; + builder.Files.Add(new PhysicalPackageFile + { + SourcePath = assemblyFile, + TargetPath = "lib/Test.dll" + }); + var stream = new MemoryStream(); + builder.Save(stream); + //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); + //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); + + _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); + + // Act + var result = await _target.IndexAsync(stream, default); + + // Assert + Assert.Equal(PackageIndexingResult.Success, result); + return builder; + } } diff --git a/tests/BaGetter.Core.Tests/Support/NullStorageService.cs b/tests/BaGetter.Core.Tests/Support/NullStorageService.cs new file mode 100644 index 000000000..05f4b94e3 --- /dev/null +++ b/tests/BaGetter.Core.Tests/Support/NullStorageService.cs @@ -0,0 +1,43 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace BaGetter.Core.Tests.Support; +public class NullStorageService : IStorageService +{ + private HashSet _files = new(); + + public NullStorageService() + { + } + + public Task DeleteAsync(string path, CancellationToken cancellationToken = default) + { + if (!_files.Contains(path)) + { + throw new FileNotFoundException(); + } + _files.Remove(path); + return Task.CompletedTask; + } + + public Task GetAsync(string path, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public Task GetDownloadUriAsync(string path, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public Task PutAsync(string path, Stream content, string contentType, CancellationToken cancellationToken = default) + { + _files.Add(path); + return Task.FromResult(StoragePutResult.Success); + } +} From 1e8f2cce18e240531725db80831b5fd2c90c483d Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 11 Sep 2024 14:16:02 -0400 Subject: [PATCH 05/10] updated documentation --- docs/docs/configuration.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/docs/configuration.md b/docs/docs/configuration.md index a443862fa..7a2c7d179 100644 --- a/docs/docs/configuration.md +++ b/docs/docs/configuration.md @@ -154,16 +154,26 @@ downloaded if you know the package's id and version. You can override this behav } ``` -## Enable package auto-deletion +## Package auto-deletion -If your build server generates many nuget packages, your BaGet server can quickly run out of space. To avoid this issue, `MaxVersionsPerPackage` can be configured to auto-delete packages older packages when a new one is uploaded. This will use the `HardDelete` option detailed above and will unlist and delete the files for the older packages. By default this value is not configured and no packages will be deleted automatically. +If your build server generates many nuget packages, your BaGet server can quickly run out of space. Bagetter leverages [SemVer 2](https://semver.org/) and has logic to keep a history of packages based on the version numbering such as `..-.`. + +The following parameters can be enabled to limit history for each level of the version. If none of these are set, there are no cleaning rules enforced. Each parameter is optional, e.g. if you specify only a `MaxHistoryPerPatch`, the package limit will only enforced for each major and minor version combination. +Packages deleted are always the oldest based on version numbers. + +- MaxHistoryPerMajorVersion: Maximum number of major versions +- MaxHistoryPerMinorVersion: Maximum number of minor versions for each major version +- MaxHistoryPerPatch: Maximum number of patch versions for each major + minor version +- MaxHistoryPerPrerelease: Maximum number of prerelease versions for each major + minor + patch version and prerelease type. if you have `beta` and `alpha` this will keep `MaxHistoryPerPrerelease` versions for both `beta` and `alpha`. ```json { ... - "MaxVersionsPerPackage ": 5, - + "MaxHistoryPerMajorVersion": 5, + "MaxHistoryPerMinorVersion": 5, + "MaxHistoryPerPatch": 5, + "MaxHistoryPerPrerelease": 5, ... } ``` From bcfb1b46de0e9c03daa30972a2486c2b35455b35 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 11 Sep 2024 14:21:16 -0400 Subject: [PATCH 06/10] fixed build --- .../PackageIndexingServiceInMemoryTests.cs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs index 03489aefd..2c47fd3c8 100644 --- a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs +++ b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs @@ -80,7 +80,6 @@ public async Task IndexIMAsync_WhenPackageAlreadyExists_AndOverwriteForbidden_Re }); var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -121,9 +120,6 @@ public async Task IndexIMAsync_WhenPackageAlreadyExists_AndOverwriteAllowed_Inde var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -155,9 +151,6 @@ public async Task IndexIMAsync_WhenPrereleasePackageAlreadyExists_AndOverwritePr var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -189,9 +182,6 @@ public async Task IndexIMAsync_WhenPrereleasePackageAlreadyExists_AndOverwriteFo var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.DeleteAsync(builder.Id, builder.Version, default)).Returns(Task.CompletedTask); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -230,8 +220,6 @@ public async Task IndexIMAsync_WithValidPackage_ReturnsSuccess() var stream = new MemoryStream(); builder.Save(stream); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act @@ -315,8 +303,6 @@ private async Task StoreVersion(NuGetVersion version) //_packages.Setup(p => p.ExistsAsync(builder.Id, builder.Version, default)).ReturnsAsync(false); //_packages.Setup(p => p.AddAsync(It.Is(p1 => p1.Id == builder.Id && p1.Version.ToString() == builder.Version.ToString()), default)).ReturnsAsync(PackageAddResult.Success); - _storage.Setup(s => s.SavePackageContentAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), stream, It.IsAny(), default, default, default)).Returns(Task.CompletedTask); - _search.Setup(s => s.IndexAsync(It.Is(p => p.Id == builder.Id && p.Version.ToString() == builder.Version.ToString()), default)).Returns(Task.CompletedTask); // Act From 3adfcf8df0b7a2a6f7a833bbc473f8fb671dbdec Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 23 Sep 2024 14:47:44 -0400 Subject: [PATCH 07/10] moved all new options to retention section --- docs/docs/configuration.md | 13 ++++---- .../Configuration/BaGetterOptions.cs | 29 +--------------- .../Configuration/RetentionOptions.cs | 33 +++++++++++++++++++ .../DependencyInjectionExtensions.cs | 1 + .../Indexing/PackageIndexingService.cs | 13 +++++--- .../PackageIndexingServiceInMemoryTests.cs | 24 +++++++++----- .../Services/PackageIndexingServiceTests.cs | 5 +++ 7 files changed, 70 insertions(+), 48 deletions(-) create mode 100644 src/BaGetter.Core/Configuration/RetentionOptions.cs diff --git a/docs/docs/configuration.md b/docs/docs/configuration.md index 7a2c7d179..3bec7c270 100644 --- a/docs/docs/configuration.md +++ b/docs/docs/configuration.md @@ -158,7 +158,7 @@ downloaded if you know the package's id and version. You can override this behav If your build server generates many nuget packages, your BaGet server can quickly run out of space. Bagetter leverages [SemVer 2](https://semver.org/) and has logic to keep a history of packages based on the version numbering such as `..-.`. -The following parameters can be enabled to limit history for each level of the version. If none of these are set, there are no cleaning rules enforced. Each parameter is optional, e.g. if you specify only a `MaxHistoryPerPatch`, the package limit will only enforced for each major and minor version combination. +There is an optional section for `Retention` and the following parameters can be enabled to limit history for each level of the version. If none of these are set, there are no cleaning rules enforced. Each parameter is optional, e.g. if you specify only a `MaxHistoryPerPatch`, the package limit will only enforced for each major and minor version combination. Packages deleted are always the oldest based on version numbers. - MaxHistoryPerMajorVersion: Maximum number of major versions @@ -169,11 +169,12 @@ Packages deleted are always the oldest based on version numbers. ```json { ... - - "MaxHistoryPerMajorVersion": 5, - "MaxHistoryPerMinorVersion": 5, - "MaxHistoryPerPatch": 5, - "MaxHistoryPerPrerelease": 5, + "Retention": { + "MaxHistoryPerMajorVersion": 5, + "MaxHistoryPerMinorVersion": 5, + "MaxHistoryPerPatch": 5, + "MaxHistoryPerPrerelease": 5, + } ... } ``` diff --git a/src/BaGetter.Core/Configuration/BaGetterOptions.cs b/src/BaGetter.Core/Configuration/BaGetterOptions.cs index cdf84489c..f966779ad 100644 --- a/src/BaGetter.Core/Configuration/BaGetterOptions.cs +++ b/src/BaGetter.Core/Configuration/BaGetterOptions.cs @@ -47,34 +47,7 @@ public class BaGetterOptions /// public uint MaxPackageSizeGiB { get; set; } = 8; - /// - /// If this is set to a value, it will limit the number of versions that can be pushed for a package. - /// The limit is applied to each major version of the package, and if the limit is exceeded, - /// the older versions will be deleted. - /// - public uint? MaxHistoryPerMajorVersion { get; set; } = null; - - /// - /// This corresponds to the maximum number of minor versions for each major version. - /// If this is set to a value, it will limit the number of versions that can be pushed for a package. - /// The limit is applied to each minor version of the package, and if the limit is exceeded, - /// the older versions will be deleted. - /// - public uint? MaxHistoryPerMinorVersion { get; set; } - - /// - /// If this is set to a value, it will limit the number of versions that can be pushed for a package. - /// The limit is applied to each patch number of the package, and if the limit is exceeded, - /// the older versions will be deleted. - /// - public uint? MaxHistoryPerPatch { get; set; } - - /// - /// If this is set to a value, it will limit the number of versions that can be pushed for a package. - /// The limit is applied to each pre-release of the package, and if the limit is exceeded, - /// the older versions will be deleted. - /// - public uint? MaxHistoryPerPrerelease { get; set; } + public RetentionOptions Retention { get; set; } public DatabaseOptions Database { get; set; } diff --git a/src/BaGetter.Core/Configuration/RetentionOptions.cs b/src/BaGetter.Core/Configuration/RetentionOptions.cs new file mode 100644 index 000000000..08c4e7a6a --- /dev/null +++ b/src/BaGetter.Core/Configuration/RetentionOptions.cs @@ -0,0 +1,33 @@ +namespace BaGetter.Core; + +public class RetentionOptions +{ + /// + /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// The limit is applied to each major version of the package, and if the limit is exceeded, + /// the older versions will be deleted. + /// + public uint? MaxHistoryPerMajorVersion { get; set; } = null; + + /// + /// This corresponds to the maximum number of minor versions for each major version. + /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// The limit is applied to each minor version of the package, and if the limit is exceeded, + /// the older versions will be deleted. + /// + public uint? MaxHistoryPerMinorVersion { get; set; } + + /// + /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// The limit is applied to each patch number of the package, and if the limit is exceeded, + /// the older versions will be deleted. + /// + public uint? MaxHistoryPerPatch { get; set; } + + /// + /// If this is set to a value, it will limit the number of versions that can be pushed for a package. + /// The limit is applied to each pre-release of the package, and if the limit is exceeded, + /// the older versions will be deleted. + /// + public uint? MaxHistoryPerPrerelease { get; set; } +} diff --git a/src/BaGetter.Core/Extensions/DependencyInjectionExtensions.cs b/src/BaGetter.Core/Extensions/DependencyInjectionExtensions.cs index a0cfc0e21..9a424298a 100644 --- a/src/BaGetter.Core/Extensions/DependencyInjectionExtensions.cs +++ b/src/BaGetter.Core/Extensions/DependencyInjectionExtensions.cs @@ -68,6 +68,7 @@ private static void AddConfiguration(this IServiceCollection services) services.AddBaGetterOptions(nameof(BaGetterOptions.Database)); services.AddBaGetterOptions(nameof(BaGetterOptions.Storage)); services.AddBaGetterOptions(nameof(BaGetterOptions.Mirror)); + services.AddBaGetterOptions(nameof(BaGetterOptions.Retention)); services.AddBaGetterOptions(nameof(BaGetterOptions.Search)); services.AddBaGetterOptions(nameof(BaGetterOptions.Storage)); services.AddBaGetterOptions(nameof(BaGetterOptions.Statistics)); diff --git a/src/BaGetter.Core/Indexing/PackageIndexingService.cs b/src/BaGetter.Core/Indexing/PackageIndexingService.cs index 374cf2618..cf86ea25e 100644 --- a/src/BaGetter.Core/Indexing/PackageIndexingService.cs +++ b/src/BaGetter.Core/Indexing/PackageIndexingService.cs @@ -15,6 +15,7 @@ public class PackageIndexingService : IPackageIndexingService private readonly ISearchIndexer _search; private readonly SystemTime _time; private readonly IOptionsSnapshot _options; + private readonly IOptionsSnapshot _retentionOptions; private readonly ILogger _logger; private readonly IPackageDeletionService _packageDeletionService; @@ -25,6 +26,7 @@ public PackageIndexingService( ISearchIndexer search, SystemTime time, IOptionsSnapshot options, + IOptionsSnapshot retentionOptions, ILogger logger) { _packages = packages ?? throw new ArgumentNullException(nameof(packages)); @@ -32,6 +34,7 @@ public PackageIndexingService( _search = search ?? throw new ArgumentNullException(nameof(search)); _time = time ?? throw new ArgumentNullException(nameof(time)); _options = options ?? throw new ArgumentNullException(nameof(options)); + _retentionOptions = retentionOptions ?? throw new ArgumentNullException(nameof(retentionOptions)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _packageDeletionService = packageDeletionService ?? throw new ArgumentNullException(nameof(packageDeletionService)); } @@ -156,7 +159,7 @@ await _storage.SavePackageContentAsync( await _search.IndexAsync(package, cancellationToken); - if (_options.Value.MaxHistoryPerMajorVersion.HasValue) + if (_retentionOptions.Value.MaxHistoryPerMajorVersion.HasValue) { try { _logger.LogInformation( @@ -165,10 +168,10 @@ await _storage.SavePackageContentAsync( package.NormalizedVersionString); var deleted = await _packageDeletionService.DeleteOldVersionsAsync( package, - _options.Value.MaxHistoryPerMajorVersion, - _options.Value.MaxHistoryPerMinorVersion, - _options.Value.MaxHistoryPerPatch, - _options.Value.MaxHistoryPerPrerelease, + _retentionOptions.Value.MaxHistoryPerMajorVersion, + _retentionOptions.Value.MaxHistoryPerMinorVersion, + _retentionOptions.Value.MaxHistoryPerPatch, + _retentionOptions.Value.MaxHistoryPerPrerelease, cancellationToken); if (deleted > 0) { diff --git a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs index 2c47fd3c8..a218d28cc 100644 --- a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs +++ b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceInMemoryTests.cs @@ -25,6 +25,7 @@ public class PackageIndexingServiceInMemoryTests private readonly Mock _time; private readonly PackageIndexingService _target; private readonly BaGetterOptions _options; + private readonly RetentionOptions _retentionOptions; public PackageIndexingServiceInMemoryTests() { @@ -34,6 +35,7 @@ public PackageIndexingServiceInMemoryTests() _search = new Mock(MockBehavior.Strict); _options = new(); + _retentionOptions = new(); var optionsSnapshot = new Mock>(); optionsSnapshot.Setup(o => o.Value).Returns(_options); @@ -46,6 +48,8 @@ public PackageIndexingServiceInMemoryTests() _time = new Mock(MockBehavior.Loose); var options = new Mock>(MockBehavior.Strict); options.Setup(o => o.Value).Returns(_options); + var retentionOptions = new Mock>(MockBehavior.Strict); + retentionOptions.Setup(o => o.Value).Returns(_retentionOptions); _target = new PackageIndexingService( _packages, @@ -54,6 +58,7 @@ public PackageIndexingServiceInMemoryTests() _search.Object, _time.Object, options.Object, + retentionOptions.Object, Mock.Of>()); } @@ -234,10 +239,11 @@ public async Task IndexIMAsync_WithValidPackage_CleansOldVersions() { // Arrange _options.AllowPackageOverwrites = PackageOverwriteAllowed.False; - _options.MaxHistoryPerMajorVersion = 2; - _options.MaxHistoryPerMinorVersion = 2; - _options.MaxHistoryPerPatch = 5; - _options.MaxHistoryPerPrerelease = 5; + + _retentionOptions.MaxHistoryPerMajorVersion = 2; + _retentionOptions.MaxHistoryPerMinorVersion = 2; + _retentionOptions.MaxHistoryPerPatch = 5; + _retentionOptions.MaxHistoryPerPrerelease = 5; // Add 10 packages for (var major = 1; major < 4; major++) { @@ -256,24 +262,24 @@ public async Task IndexIMAsync_WithValidPackage_CleansOldVersions() var packageVersions = await _packages.FindAsync(builder.Id, true, default); var majorCount = packageVersions.Select(p => p.Version.Major).Distinct().Count(); - Assert.Equal(majorCount, Math.Min(major, (int)_options.MaxHistoryPerMajorVersion)); - Assert.True(majorCount <= _options.MaxHistoryPerMajorVersion, $"Major version {major} has {majorCount} packages"); + Assert.Equal(majorCount, Math.Min(major, (int)_retentionOptions.MaxHistoryPerMajorVersion)); + Assert.True(majorCount <= _retentionOptions.MaxHistoryPerMajorVersion, $"Major version {major} has {majorCount} packages"); // validate maximum number of minor versions for each major version. var minorVersions = packageVersions.GroupBy(m => m.Version.Major) .Select(gp => (version: gp.Key, versionCount: gp.Select(p => p.Version.Major + "." + p.Version.Minor).Distinct().Count())).ToList(); - Assert.All(minorVersions, g => Assert.True(g.versionCount <= _options.MaxHistoryPerMinorVersion, $"Minor version {g.version} has {g.versionCount} packages")); + Assert.All(minorVersions, g => Assert.True(g.versionCount <= _retentionOptions.MaxHistoryPerMinorVersion, $"Minor version {g.version} has {g.versionCount} packages")); // validate maximum number of minor versions for each major version. var patches = packageVersions.GroupBy(m => (m.Version.Major, m.Version.Minor)) .Select(gp => (version: gp.Key, versionCount: gp.Select(p => p.Version.Major + "." + p.Version.Minor + "." + p.Version.Patch).Distinct().Count())).ToList(); - Assert.All(patches, g => Assert.True(g.versionCount <= _options.MaxHistoryPerPatch, $"Patch version {g.version} has {g.versionCount} packages")); + Assert.All(patches, g => Assert.True(g.versionCount <= _retentionOptions.MaxHistoryPerPatch, $"Patch version {g.version} has {g.versionCount} packages")); // validate maximum number of beta versions for each major,minor,patch version. var betaVersions = packageVersions.Where(p => p.IsPrerelease && p.Version.ReleaseLabels.First() == "beta") .GroupBy(m => (m.Version.Major, m.Version.Minor, m.Version.Patch)) .Select(gp => (version: gp.Key, versionCount: gp.Select(p => p.Version.Major + "." + p.Version.Minor + "." + p.Version.Patch).Distinct().Count())).ToList(); - Assert.All(betaVersions, g => Assert.True(g.versionCount <= _options.MaxHistoryPerPatch, $"Pre-Release version {g.version} has {g.versionCount} packages")); + Assert.All(betaVersions, g => Assert.True(g.versionCount <= _retentionOptions.MaxHistoryPerPatch, $"Pre-Release version {g.version} has {g.versionCount} packages")); } diff --git a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs index d353b8194..5cd273c8e 100644 --- a/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs +++ b/tests/BaGetter.Core.Tests/Services/PackageIndexingServiceTests.cs @@ -19,6 +19,7 @@ public class PackageIndexingServiceTests private readonly Mock _deleter; private readonly PackageIndexingService _target; private readonly BaGetterOptions _mockOptions; + private RetentionOptions _retentionOptions; public PackageIndexingServiceTests() { @@ -28,8 +29,11 @@ public PackageIndexingServiceTests() _time = new Mock(MockBehavior.Loose); _deleter = new Mock(MockBehavior.Strict); _mockOptions = new(); + _retentionOptions = new(); var options = new Mock>(MockBehavior.Strict); options.Setup(o => o.Value).Returns(_mockOptions); + var retentionOptions = new Mock>(MockBehavior.Strict); + retentionOptions.Setup(o => o.Value).Returns(_retentionOptions); _target = new PackageIndexingService( _packages.Object, @@ -38,6 +42,7 @@ public PackageIndexingServiceTests() _search.Object, _time.Object, options.Object, + retentionOptions.Object, Mock.Of>()); } From 56b3d77c14f42fb79997f53d5239bcad3aef8ea9 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 14 Nov 2024 17:21:32 -0500 Subject: [PATCH 08/10] Update src/BaGetter.Core/Indexing/PackageDeletionService.cs Co-authored-by: seriouz Signed-off-by: Erik --- src/BaGetter.Core/Indexing/PackageDeletionService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BaGetter.Core/Indexing/PackageDeletionService.cs b/src/BaGetter.Core/Indexing/PackageDeletionService.cs index 4ec24bc4a..7c3c47a3d 100644 --- a/src/BaGetter.Core/Indexing/PackageDeletionService.cs +++ b/src/BaGetter.Core/Indexing/PackageDeletionService.cs @@ -91,7 +91,7 @@ private async Task TryHardDeletePackageAsync(string id, NuGetVersion versi return found; } - private List GetValidVersions(IEnumerable versions, Func getParent, Func getSelector, int versionsToKeep) + private static IList GetValidVersions(IEnumerable versions, Func getParent, Func getSelector, int versionsToKeep) where S : IComparable, IEquatable where T : IComparable, IEquatable { From 8eedfa76d7a9aaacbb6ffbd15c3eccd0e28fbc73 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 14 Nov 2024 17:21:42 -0500 Subject: [PATCH 09/10] Update src/BaGetter.Core/Indexing/PackageDeletionService.cs Co-authored-by: seriouz Signed-off-by: Erik --- src/BaGetter.Core/Indexing/PackageDeletionService.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/BaGetter.Core/Indexing/PackageDeletionService.cs b/src/BaGetter.Core/Indexing/PackageDeletionService.cs index 7c3c47a3d..73dec4171 100644 --- a/src/BaGetter.Core/Indexing/PackageDeletionService.cs +++ b/src/BaGetter.Core/Indexing/PackageDeletionService.cs @@ -150,7 +150,6 @@ public async Task DeleteOldVersionsAsync(Package package, uint? maxMajor, u var preReleaseVersions = preReleases.Where(p => p.ReleaseLabels!.FirstOrDefault() == preReleaseType && GetPreReleaseBuild(p) is not null).ToList(); - allPreReleaseValidVersions.UnionWith (GetValidVersions(preReleaseVersions, v => (v.Major, v.Minor, v.Patch), v => GetPreReleaseBuild(v).Value, (int)maxPrerelease)); From 3c9c1740968f532133e18a448b64d8f2c4b8e30c Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 14 Nov 2024 17:22:15 -0500 Subject: [PATCH 10/10] Update src/BaGetter.Core/Indexing/PackageDeletionService.cs Co-authored-by: seriouz Signed-off-by: Erik --- src/BaGetter.Core/Indexing/PackageDeletionService.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/BaGetter.Core/Indexing/PackageDeletionService.cs b/src/BaGetter.Core/Indexing/PackageDeletionService.cs index 73dec4171..51dbfbb3e 100644 --- a/src/BaGetter.Core/Indexing/PackageDeletionService.cs +++ b/src/BaGetter.Core/Indexing/PackageDeletionService.cs @@ -170,10 +170,10 @@ public async Task DeleteOldVersionsAsync(Package package, uint? maxMajor, u } /// - /// If we have 1.1.1-alpha.1 , this will return 1 - /// or null if not valid + /// Tries to get the version number of a pre-release build.
+ /// If we have 1.1.1-alpha.1 , this will return 1 or null if not valid. /// - /// + /// The version as int or null if not found. private int? GetPreReleaseBuild(NuGetVersion nuGetVersion) { if (nuGetVersion.IsPrerelease && nuGetVersion.ReleaseLabels != null)