Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merge branch main into dev #9954

Merged
merged 2 commits into from
May 21, 2024
Merged

Merge branch main into dev #9954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c92f5fd
Merge pull request #9941 from NuGet/dev
erdembayar May 9, 2024
c73ce25
Allow unrelated entity changes to be committed in PackageDeprecationS…
joelverhagen May 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/NuGetGallery/Services/PackageDeprecationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,28 @@ public async Task UpdateDeprecation(
{
await _entitiesContext.SaveChangesAsync();

await _packageUpdateService.UpdatePackagesAsync(changedPackages);
// Ideally, the number of changed packages should be zero if and only if the entity context has
// no changes (therefore this line should not be reached). But it is possible that an entity can
// be changed for other reasons, such as https://github.com/NuGet/NuGetGallery/issues/9950.
// Therefore, allow the transaction to be committed but do not update the LastEdited property on
// the package, to avoid unnecessary package edits flowing into V3.
if (changedPackages.Count > 0)
{
await _packageUpdateService.UpdatePackagesAsync(changedPackages);
}

transaction.Commit();

_telemetryService.TrackPackageDeprecate(
changedPackages,
status,
alternatePackageRegistration,
alternatePackage,
!string.IsNullOrWhiteSpace(customMessage),
hasChanges: true);
if (changedPackages.Count > 0)
{
_telemetryService.TrackPackageDeprecate(
changedPackages,
status,
alternatePackageRegistration,
alternatePackage,
!string.IsNullOrWhiteSpace(customMessage),
hasChanges: true);
}

foreach (var package in changedPackages)
{
Expand Down
101 changes: 101 additions & 0 deletions tests/NuGetGallery.Facts/Services/PackageDeprecationServiceFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,107 @@ await service.UpdateDeprecation(
auditingService.WroteNoRecords();
}
}

[Fact]
public async Task SavesChangesWithExtraEntityChangesButNoDeprecationChanges()
{
// Arrange
var lastTimestamp = new DateTime(2019, 3, 4);

var id = "theId";
var registration = new PackageRegistration { Id = id };

var customMessage = "message";
var user = new User { Key = 1 };
var status = (PackageDeprecationStatus)99;

var alternatePackageRegistration = new PackageRegistration { Key = 1 };
var alternatePackage = new Package { Key = 1 };

var packageWithDeprecation = new Package
{
PackageRegistration = registration,
NormalizedVersion = "3.0.0",
LastEdited = lastTimestamp,
Deprecations = new List<PackageDeprecation>
{
new PackageDeprecation
{
DeprecatedByUserKey = user.Key,
CustomMessage = customMessage,
Status = status,
AlternatePackageKey = alternatePackage.Key,
AlternatePackage = alternatePackage,
AlternatePackageRegistrationKey = alternatePackageRegistration.Key,
AlternatePackageRegistration = alternatePackageRegistration,
}
}
};

var packages = new[]
{
packageWithDeprecation,
};

var transactionMock = new Mock<IDbContextTransaction>();
transactionMock
.Setup(x => x.Commit())
.Verifiable();

var databaseMock = new Mock<IDatabase>();
databaseMock
.Setup(x => x.BeginTransaction())
.Returns(transactionMock.Object);

var context = GetFakeContext();
context.HasChanges = true;
context.SetupDatabase(databaseMock.Object);
context.Deprecations.AddRange(
packages
.Select(p => p.Deprecations.SingleOrDefault())
.Where(d => d != null));

var packageUpdateService = GetMock<IPackageUpdateService>();
var auditingService = GetService<IAuditingService>();

var telemetryService = GetMock<ITelemetryService>();
telemetryService
.Setup(x => x.TrackPackageDeprecate(packages, status, alternatePackageRegistration, alternatePackage, true, false))
.Verifiable();

var service = Get<PackageDeprecationService>();

// Act
await service.UpdateDeprecation(
packages,
status,
alternatePackageRegistration,
alternatePackage,
customMessage,
user,
ListedVerb.Unchanged,
PackageUndeprecatedVia.Web);

// Assert
context.VerifyCommitChanges();
databaseMock.Verify();
transactionMock.Verify();
packageUpdateService.Verify();
telemetryService.Verify();

Assert.Equal(packages.Count(), context.Deprecations.Count());
foreach (var package in packages)
{
var deprecation = package.Deprecations.Single();
Assert.Contains(deprecation, context.Deprecations);
Assert.Equal(status, deprecation.Status);
Assert.Equal(alternatePackageRegistration, deprecation.AlternatePackageRegistration);
Assert.Equal(alternatePackage, deprecation.AlternatePackage);
Assert.Equal(customMessage, deprecation.CustomMessage);

auditingService.WroteNoRecords();
}
}
}

public class TheGetDeprecationByPackageMethod : TestContainer
Expand Down