Skip to content
This repository was archived by the owner on Jul 30, 2024. It is now read-only.

Commit 2850e5f

Browse files
joelverhagenjoelverhagen
committed
Check package hash even when the package is already available (#367)
Progress on NuGet/Engineering#1190
1 parent 10c1074 commit 2850e5f

12 files changed

Lines changed: 426 additions & 181 deletions

‎src/NuGet.Services.Validation.Orchestrator/IValidationPackageFileService.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,19 @@ public interface IValidationPackageFileService : ICorePackageFileService
2929
/// validation set to have its own copy of the package to mutate (via <see cref="IProcessor"/>) and validate.
3030
/// </summary>
3131
/// <param name="validationSet">The validation set, containing validation set and package identifiers.</param>
32-
Task CopyPackageFileForValidationSetAsync(PackageValidationSet validationSet);
32+
/// <returns>The etag of the source package.</returns>
33+
Task<string> CopyPackageFileForValidationSetAsync(PackageValidationSet validationSet);
3334

3435
/// <summary>
3536
/// Copy a package from a location specific for the validation set to the packages container.
3637
/// </summary>
3738
/// <param name="validationSet">The validation set, containing validation set and package identifiers.</param>
38-
Task CopyValidationSetPackageToPackageFileAsync(PackageValidationSet validationSet);
39+
/// <param name="destAccessCondition">
40+
/// The access condition used for asserting the state of the destination file.
41+
/// </param>
42+
Task CopyValidationSetPackageToPackageFileAsync(
43+
PackageValidationSet validationSet,
44+
IAccessCondition destAccessCondition);
3945

4046
/// <summary>
4147
/// Copy a package from the validation container to the packages container.

‎src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@
108108
</ItemGroup>
109109
<ItemGroup>
110110
<PackageReference Include="NuGet.Services.Validation.Issues">
111-
<Version>2.17.0</Version>
111+
<Version>2.18.0</Version>
112112
</PackageReference>
113113
</ItemGroup>
114114
<ItemGroup>

‎src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs‎

Lines changed: 126 additions & 87 deletions
Large diffs are not rendered by default.

‎src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,18 @@ ValidationConfigurationItem GetValidationConfigurationItem(string validationName
115115
package.NormalizedVersion,
116116
validationSet.ValidationTrackingId);
117117

118-
if (package.PackageStatusKey != PackageStatus.Available)
119-
{
120-
await _packageStateProcessor.SetPackageStatusAsync(package, validationSet, PackageStatus.Available);
118+
var fromStatus = package.PackageStatusKey;
119+
120+
// Always set the package status to available so that processors can have a change to fix packages
121+
// that are already available. Processors should no-op when their work is already done, so the
122+
// modification of an already available package should be rare. The most common case for this is if
123+
// the processor has never been run on a package that was published before the processor was
124+
// implemented. In this case, the processor has to play catch-up.
125+
await _packageStateProcessor.SetPackageStatusAsync(package, validationSet, PackageStatus.Available);
121126

127+
// Only send the email when first transitioning into the Available state.
128+
if (fromStatus != PackageStatus.Available)
129+
{
122130
_messageService.SendPackagePublishedMessage(package);
123131
}
124132

‎src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@ public Task CopyValidationPackageForValidationSetAsync(PackageValidationSet vali
4646
CoreConstants.ValidationFolderName,
4747
srcFileName,
4848
CoreConstants.ValidationFolderName,
49-
BuildValidationSetPackageFileName(validationSet));
49+
BuildValidationSetPackageFileName(validationSet),
50+
AccessConditionWrapper.GenerateEmptyCondition());
5051
}
5152

52-
public Task CopyPackageFileForValidationSetAsync(PackageValidationSet validationSet)
53+
public Task<string> CopyPackageFileForValidationSetAsync(PackageValidationSet validationSet)
5354
{
5455
var srcFileName = BuildFileName(
5556
validationSet.PackageId,
@@ -61,7 +62,8 @@ public Task CopyPackageFileForValidationSetAsync(PackageValidationSet validation
6162
CoreConstants.PackagesFolderName,
6263
srcFileName,
6364
CoreConstants.ValidationFolderName,
64-
BuildValidationSetPackageFileName(validationSet));
65+
BuildValidationSetPackageFileName(validationSet),
66+
AccessConditionWrapper.GenerateEmptyCondition());
6567
}
6668

6769
public Task CopyValidationPackageToPackageFileAsync(string id, string normalizedVersion)
@@ -76,10 +78,13 @@ public Task CopyValidationPackageToPackageFileAsync(string id, string normalized
7678
CoreConstants.ValidationFolderName,
7779
fileName,
7880
CoreConstants.PackagesFolderName,
79-
fileName);
81+
fileName,
82+
AccessConditionWrapper.GenerateIfNotExistsCondition());
8083
}
8184

82-
public Task CopyValidationSetPackageToPackageFileAsync(PackageValidationSet validationSet)
85+
public Task CopyValidationSetPackageToPackageFileAsync(
86+
PackageValidationSet validationSet,
87+
IAccessCondition destAccessCondition)
8388
{
8489
var srcFileName = BuildValidationSetPackageFileName(validationSet);
8590

@@ -93,7 +98,8 @@ public Task CopyValidationSetPackageToPackageFileAsync(PackageValidationSet vali
9398
CoreConstants.ValidationFolderName,
9499
srcFileName,
95100
CoreConstants.PackagesFolderName,
96-
destFileName);
101+
destFileName,
102+
destAccessCondition);
97103
}
98104

99105
public Task<bool> DoesValidationSetPackageExistAsync(PackageValidationSet validationSet)
@@ -123,7 +129,12 @@ public Task<Uri> GetPackageForValidationSetReadUriAsync(PackageValidationSet val
123129
return _fileStorageService.GetFileReadUriAsync(CoreConstants.ValidationFolderName, fileName, endOfAccess);
124130
}
125131

126-
private Task CopyFileAsync(string srcFolderName, string srcFileName, string destFolderName, string destFileName)
132+
private Task<string> CopyFileAsync(
133+
string srcFolderName,
134+
string srcFileName,
135+
string destFolderName,
136+
string destFileName,
137+
IAccessCondition destAccessCondition)
127138
{
128139
_logger.LogInformation(
129140
"Copying file {SrcFolderName}/{SrcFileName} to {DestFolderName}/{DestFileName}.",
@@ -136,7 +147,8 @@ private Task CopyFileAsync(string srcFolderName, string srcFileName, string dest
136147
srcFolderName,
137148
srcFileName,
138149
destFolderName,
139-
destFileName);
150+
destFileName,
151+
destAccessCondition);
140152
}
141153

142154
private static string BuildValidationSetPackageFileName(PackageValidationSet validationSet)

‎src/NuGet.Services.Validation.Orchestrator/ValidationSetProvider.cs‎

Lines changed: 61 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -52,48 +52,94 @@ public async Task<PackageValidationSet> TryGetOrCreateValidationSetAsync(Guid va
5252
return null;
5353
}
5454

55-
validationSet = await CreateValidationSet(validationTrackingId, package);
55+
validationSet = InitializeValidationSet(validationTrackingId, package);
5656

5757
if (package.PackageStatusKey == PackageStatus.Available)
5858
{
59-
await _packageFileService.CopyPackageFileForValidationSetAsync(validationSet);
59+
var packageETag = await _packageFileService.CopyPackageFileForValidationSetAsync(validationSet);
60+
61+
// This indicates that the package in the package container is expected to not change.
62+
validationSet.PackageETag = packageETag;
6063
}
6164
else
6265
{
6366
await _packageFileService.CopyValidationPackageForValidationSetAsync(validationSet);
67+
68+
// This indicates that the package in the packages container is expected to not exist (i.e. it has
69+
// has no etag at all).
70+
validationSet.PackageETag = null;
6471
}
72+
73+
validationSet = await PersistValidationSetAsync(validationSet, package);
6574
}
6675
else
6776
{
68-
var sameId = package.PackageRegistration.Id.Equals(validationSet.PackageId, StringComparison.InvariantCultureIgnoreCase);
69-
var sameVersion = package.NormalizedVersion.Equals(validationSet.PackageNormalizedVersion, StringComparison.InvariantCultureIgnoreCase);
77+
var sameId = package.PackageRegistration.Id.Equals(
78+
validationSet.PackageId,
79+
StringComparison.InvariantCultureIgnoreCase);
80+
81+
var sameVersion = package.NormalizedVersion.Equals(
82+
validationSet.PackageNormalizedVersion,
83+
StringComparison.InvariantCultureIgnoreCase);
84+
7085
if (!sameId || !sameVersion)
7186
{
72-
throw new Exception($"Validation set package identity ({validationSet.PackageId} {validationSet.PackageNormalizedVersion})" +
73-
$"does not match expected package identity ({package.PackageRegistration.Id} {package.NormalizedVersion})");
87+
throw new InvalidOperationException(
88+
$"Validation set package identity ({validationSet.PackageId} {validationSet.PackageNormalizedVersion})" +
89+
$"does not match expected package identity ({package.PackageRegistration.Id} {package.NormalizedVersion}).");
90+
}
91+
92+
var sameKey = package.Key == validationSet.PackageKey;
93+
94+
if (!sameKey)
95+
{
96+
throw new InvalidOperationException($"Validation set package key ({validationSet.PackageKey}) " +
97+
$"does not match expected package key ({package.Key}).");
7498
}
7599
}
76100

77101
return validationSet;
78102
}
79103

80-
private async Task<PackageValidationSet> CreateValidationSet(Guid validationTrackingId, Package package)
104+
private async Task<PackageValidationSet> PersistValidationSetAsync(PackageValidationSet validationSet, Package package)
81105
{
82-
_logger.LogInformation("Creating validation set {ValidationSetId} for package {PackageId} {PackageVersion}",
106+
_logger.LogInformation("Persisting validation set {ValidationSetId} for package {PackageId} {PackageVersion} (package key {PackageKey})",
107+
validationSet.ValidationTrackingId,
108+
package.PackageRegistration.Id,
109+
package.NormalizedVersion,
110+
package.Key);
111+
112+
var persistedValidationSet = await _validationStorageService.CreateValidationSetAsync(validationSet);
113+
114+
// Only track the validation set creation time when this is the first validation set to be created for that
115+
// package. There will be more than one validation set when an admin has requested a manual revalidation.
116+
// This can happen much later than when the package was created so the duration is less interesting in that
117+
// case.
118+
if (await _validationStorageService.GetValidationSetCountAsync(package.Key) == 1)
119+
{
120+
_telemetryService.TrackDurationToValidationSetCreation(validationSet.Created - package.Created);
121+
}
122+
123+
return persistedValidationSet;
124+
}
125+
126+
private PackageValidationSet InitializeValidationSet(Guid validationTrackingId, Package package)
127+
{
128+
_logger.LogInformation("Initializing validation set {ValidationSetId} for package {PackageId} {PackageVersion} (package key {PackageKey})",
83129
validationTrackingId,
84130
package.PackageRegistration.Id,
85-
package.NormalizedVersion);
131+
package.NormalizedVersion,
132+
package.Key);
86133

87-
PackageValidationSet validationSet;
88-
var packageValidations = new List<PackageValidation>();
89134
var now = DateTime.UtcNow;
90-
validationSet = new PackageValidationSet
135+
136+
var validationSet = new PackageValidationSet
91137
{
92138
Created = now,
93139
PackageId = package.PackageRegistration.Id,
94140
PackageNormalizedVersion = package.NormalizedVersion,
95141
PackageKey = package.Key,
96-
PackageValidations = packageValidations,
142+
PackageValidations = new List<PackageValidation>(),
97143
Updated = now,
98144
ValidationTrackingId = validationTrackingId,
99145
};
@@ -108,21 +154,10 @@ private async Task<PackageValidationSet> CreateValidationSet(Guid validationTrac
108154
ValidationStatusTimestamp = now,
109155
};
110156

111-
packageValidations.Add(packageValidation);
112-
}
113-
114-
var persistedValidationSet = await _validationStorageService.CreateValidationSetAsync(validationSet);
115-
116-
// Only track the validation set creation time when this is the first validation set to be created for that
117-
// package. There will be more than one validation set when an admin has requested a manual revalidation.
118-
// This can happen much later than when the package was created so the duration is less interesting in that
119-
// case.
120-
if (await _validationStorageService.GetValidationSetCountAsync(package.Key) == 1)
121-
{
122-
_telemetryService.TrackDurationToValidationSetCreation(now - package.Created);
157+
validationSet.PackageValidations.Add(packageValidation);
123158
}
124159

125-
return persistedValidationSet;
160+
return validationSet;
126161
}
127162
}
128163
}

‎src/Validation.Common.Job/Validation.Common.Job.csproj‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,19 @@
7272
<Version>4.7.0-preview1-4886</Version>
7373
</PackageReference>
7474
<PackageReference Include="NuGet.Services.Configuration">
75-
<Version>2.17.0</Version>
75+
<Version>2.18.0</Version>
7676
</PackageReference>
7777
<PackageReference Include="NuGet.Services.Logging">
78-
<Version>2.17.0</Version>
78+
<Version>2.18.0</Version>
7979
</PackageReference>
8080
<PackageReference Include="NuGet.Services.Storage">
81-
<Version>2.17.0</Version>
81+
<Version>2.18.0</Version>
8282
</PackageReference>
8383
<PackageReference Include="NuGet.Services.Validation">
84-
<Version>2.17.0</Version>
84+
<Version>2.18.0</Version>
8585
</PackageReference>
8686
<PackageReference Include="NuGetGallery.Core">
87-
<Version>4.4.4-dev-25129</Version>
87+
<Version>4.4.4-dev-25609</Version>
8888
</PackageReference>
8989
<PackageReference Include="Serilog">
9090
<Version>2.5.0</Version>

‎tests/NuGet.Services.Validation.Orchestrator.Tests/NuGet.Services.Validation.Orchestrator.Tests.csproj‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,6 @@
7171
<PackageReference Include="moq">
7272
<Version>4.7.145</Version>
7373
</PackageReference>
74-
<PackageReference Include="NuGet.Services.Validation.Issues">
75-
<Version>2.17.0</Version>
76-
</PackageReference>
7774
<PackageReference Include="System.Net.Http">
7875
<Version>4.3.3</Version>
7976
</PackageReference>

0 commit comments

Comments
 (0)