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

Commit 2d5426f

Browse files
dtiveldtivel
authored
Orchestrator: add package hash to blob metadata (#459)
Resolve NuGet/Engineering#1204.
1 parent 0abe198 commit 2d5426f

11 files changed

Lines changed: 296 additions & 56 deletions

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.IO;
66
using System.Threading.Tasks;
77
using NuGetGallery;
8+
using NuGetGallery.Packaging;
89

910
namespace NuGet.Services.Validation.Orchestrator
1011
{
@@ -85,5 +86,16 @@ Task CopyValidationSetPackageToPackageFileAsync(
8586
/// <param name="validationSet">The validation set, containing validation set and package identifiers.</param>
8687
/// <returns>True if file exists, false otherwise</returns>
8788
Task<bool> DoesValidationSetPackageExistAsync(PackageValidationSet validationSet);
89+
90+
/// <summary>
91+
/// Updates package blob metadata.
92+
/// </summary>
93+
/// <param name="package">A package that will have its blob metadata updated.</param>
94+
/// <returns>A task that represents the asynchronous operation.
95+
/// The task result (<see cref="Task{PackageStreamMetadata}.Result" />) returns
96+
/// a <see name="PackageStreamMetadata" />.</returns>
97+
/// <exception cref="Microsoft.WindowsAzure.Storage.StorageException">Thrown if the blob has changed between
98+
/// successive read and write operations.</exception>
99+
Task<PackageStreamMetadata> UpdatePackageBlobMetadataAsync(Package package);
88100
}
89101
}

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

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Diagnostics;
65
using System.Linq;
76
using System.Threading.Tasks;
87
using Microsoft.Extensions.Logging;
@@ -94,10 +93,13 @@ private async Task MakePackageAvailableAsync(Package package, PackageValidationS
9493
// 1) Operate on blob storage.
9594
var copied = await UpdatePublicPackageAsync(validationSet, package);
9695

97-
// 2) Operate on the database.
98-
var fromStatus = await MarkPackageAsAvailableAsync(validationSet, package, copied);
96+
// 2) Update the package's blob metadata in the packages blob storage container.
97+
var metadata = await _packageFileService.UpdatePackageBlobMetadataAsync(package);
9998

100-
// 3) Emit telemetry and clean up.
99+
// 3) Operate on the database.
100+
var fromStatus = await MarkPackageAsAvailableAsync(validationSet, package, metadata, copied);
101+
102+
// 4) Emit telemetry and clean up.
101103
if (fromStatus != PackageStatus.Available)
102104
{
103105
_telemetryService.TrackPackageStatusChange(fromStatus, PackageStatus.Available);
@@ -130,40 +132,24 @@ private async Task MakePackageAvailableAsync(Package package, PackageValidationS
130132
}
131133
}
132134

133-
private async Task<PackageStatus> MarkPackageAsAvailableAsync(PackageValidationSet validationSet, Package package, bool copied)
135+
private async Task<PackageStatus> MarkPackageAsAvailableAsync(
136+
PackageValidationSet validationSet,
137+
Package package,
138+
PackageStreamMetadata streamMetadata,
139+
bool copied)
134140
{
135-
136141
// Use whatever package made it into the packages container. This is what customers will consume so the DB
137142
// record must match.
138-
using (var packageStream = await _packageFileService.DownloadPackageFileToDiskAsync(package))
143+
// We don't immediately commit here. Later, we will commit these changes as well as the new package
144+
// status as part of the same transaction.
145+
if (streamMetadata.Size != package.PackageFileSize
146+
|| streamMetadata.Hash != package.Hash
147+
|| streamMetadata.HashAlgorithm != package.HashAlgorithm)
139148
{
140-
var stopwatch = Stopwatch.StartNew();
141-
var hash = CryptographyService.GenerateHash(packageStream, CoreConstants.Sha512HashAlgorithmId);
142-
_telemetryService.TrackDurationToHashPackage(
143-
stopwatch.Elapsed,
144-
package.PackageRegistration.Id,
145-
package.NormalizedVersion,
146-
CoreConstants.Sha512HashAlgorithmId,
147-
packageStream.GetType().FullName);
148-
149-
var streamMetadata = new PackageStreamMetadata
150-
{
151-
Size = packageStream.Length,
152-
Hash = hash,
153-
HashAlgorithm = CoreConstants.Sha512HashAlgorithmId,
154-
};
155-
156-
// We don't immediately commit here. Later, we will commit these changes as well as the new package
157-
// status as part of the same transaction.
158-
if (streamMetadata.Size != package.PackageFileSize
159-
|| streamMetadata.Hash != package.Hash
160-
|| streamMetadata.HashAlgorithm != package.HashAlgorithm)
161-
{
162-
await _galleryPackageService.UpdatePackageStreamMetadataAsync(
163-
package,
164-
streamMetadata,
165-
commitChanges: false);
166-
}
149+
await _galleryPackageService.UpdatePackageStreamMetadataAsync(
150+
package,
151+
streamMetadata,
152+
commitChanges: false);
167153
}
168154

169155
_logger.LogInformation("Marking package {PackageId} {PackageVersion}, validation set {ValidationSetId} as {PackageStatus} in DB",

‎src/NuGet.Services.Validation.Orchestrator/Telemetry/ITelemetryService.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,6 @@ public interface ITelemetryService
116116
/// <summary>
117117
/// A metric to of how long it took to hash a package.
118118
/// </summary>
119-
void TrackDurationToHashPackage(TimeSpan duration, string packageId, string normalizedVersion, string hashAlgorithm, string streamType);
119+
IDisposable TrackDurationToHashPackage(string packageId, string normalizedVersion, long packageSize, string hashAlgorithm, string streamType);
120120
}
121121
}

‎src/NuGet.Services.Validation.Orchestrator/Telemetry/TelemetryService.cs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System;
55
using System.Collections.Generic;
6-
using Microsoft.ApplicationInsights;
76
using NuGet.Services.Logging;
87
using NuGetGallery;
98

@@ -54,16 +53,20 @@ public TelemetryService(ITelemetryClient telemetryClient)
5453
_telemetryClient = telemetryClient ?? throw new ArgumentNullException(nameof(telemetryClient));
5554
}
5655

57-
public void TrackDurationToHashPackage(TimeSpan duration, string packageId, string normalizedVersion, string hashAlgorithm, string streamType)
56+
public IDisposable TrackDurationToHashPackage(
57+
string packageId,
58+
string normalizedVersion,
59+
long packageSize,
60+
string hashAlgorithm,
61+
string streamType)
5862
{
59-
_telemetryClient.TrackMetric(
63+
return _telemetryClient.TrackDuration(
6064
DurationToHashPackageSeconds,
61-
duration.TotalSeconds,
6265
new Dictionary<string, string>
6366
{
6467
{ PackageId, packageId },
6568
{ NormalizedVersion, normalizedVersion },
66-
{ PackageSize, PackageSize.ToString() },
69+
{ PackageSize, packageSize.ToString() },
6770
{ HashAlgorithm, hashAlgorithm },
6871
{ StreamType, streamType },
6972
});

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

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
using System.Threading.Tasks;
88
using Microsoft.Extensions.Logging;
99
using NuGet.Jobs.Validation;
10+
using NuGet.Services.Validation.Orchestrator.Telemetry;
1011
using NuGetGallery;
12+
using NuGetGallery.Packaging;
1113

1214
namespace NuGet.Services.Validation.Orchestrator
1315
{
@@ -21,15 +23,18 @@ public class ValidationPackageFileService : CorePackageFileService, IValidationP
2123

2224
private readonly ICoreFileStorageService _fileStorageService;
2325
private readonly IPackageDownloader _packageDownloader;
26+
private readonly ITelemetryService _telemetryService;
2427
private readonly ILogger<ValidationPackageFileService> _logger;
2528

2629
public ValidationPackageFileService(
2730
ICoreFileStorageService fileStorageService,
2831
IPackageDownloader packageDownloader,
32+
ITelemetryService telemetryService,
2933
ILogger<ValidationPackageFileService> logger) : base(fileStorageService)
3034
{
3135
_fileStorageService = fileStorageService ?? throw new ArgumentNullException(nameof(fileStorageService));
3236
_packageDownloader = packageDownloader ?? throw new ArgumentNullException(nameof(packageDownloader));
37+
_telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService));
3338
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
3439
}
3540

@@ -170,6 +175,50 @@ public Task CopyPackageUrlForValidationSetAsync(PackageValidationSet validationS
170175
AccessConditionWrapper.GenerateEmptyCondition());
171176
}
172177

178+
public async Task<PackageStreamMetadata> UpdatePackageBlobMetadataAsync(Package package)
179+
{
180+
var fileName = BuildFileName(
181+
package,
182+
CoreConstants.PackageFileSavePathTemplate,
183+
CoreConstants.NuGetPackageFileExtension);
184+
185+
PackageStreamMetadata streamMetadata = null;
186+
187+
// This will throw if the ETag changes between read and write operations,
188+
// so streamMetadata will never be null.
189+
await _fileStorageService.SetMetadataAsync(
190+
CoreConstants.PackagesFolderName,
191+
fileName,
192+
async (lazyStream, metadata) =>
193+
{
194+
var packageStream = await lazyStream.Value;
195+
string hash;
196+
197+
using (_telemetryService.TrackDurationToHashPackage(
198+
package.PackageRegistration.Id,
199+
package.NormalizedVersion,
200+
packageStream.Length,
201+
CoreConstants.Sha512HashAlgorithmId,
202+
packageStream.GetType().FullName))
203+
{
204+
hash = CryptographyService.GenerateHash(packageStream, CoreConstants.Sha512HashAlgorithmId);
205+
}
206+
207+
metadata[CoreConstants.Sha512HashAlgorithmId] = hash;
208+
209+
streamMetadata = new PackageStreamMetadata()
210+
{
211+
Size = packageStream.Length,
212+
Hash = hash,
213+
HashAlgorithm = CoreConstants.Sha512HashAlgorithmId
214+
};
215+
216+
return true;
217+
});
218+
219+
return streamMetadata;
220+
}
221+
173222
private Task<string> CopyFileAsync(
174223
string srcFolderName,
175224
string srcFileName,
@@ -200,4 +249,4 @@ private static string BuildValidationSetPackageFileName(PackageValidationSet val
200249
CoreConstants.NuGetPackageFileExtension;
201250
}
202251
}
203-
}
252+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@
103103
<Version>2.25.0-master-30453</Version>
104104
</PackageReference>
105105
<PackageReference Include="NuGetGallery.Core">
106-
<Version>4.4.4-dev-30533</Version>
106+
<Version>4.4.4-dev-32522</Version>
107107
</PackageReference>
108108
<PackageReference Include="Serilog">
109109
<Version>2.5.0</Version>

‎src/Validation.Common.Job/Validation.Common.Job.nuspec‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
<dependency id="NuGet.Services.Logging" version="2.23.0" />
2121
<dependency id="NuGet.Services.Storage" version="2.23.0" />
2222
<dependency id="NuGet.Services.Validation" version="2.25.0-master-30453" />
23-
<dependency id="NuGetGallery.Core" version="4.4.4-dev-30533" />
23+
<dependency id="NuGetGallery.Core" version="4.4.4-dev-32522" />
2424
<dependency id="Serilog" version="2.5.0" />
2525
<dependency id="System.Net.Http" version="4.3.3" />
2626
</dependencies>

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
<Compile Include="PackageSigning\ProcessSignature\PackageSignatureProcessorFacts.cs" />
5454
<Compile Include="PackageStatusProcessorFacts.cs" />
5555
<Compile Include="Properties\AssemblyInfo.cs" />
56+
<Compile Include="TelemetryServiceFacts.cs" />
5657
<Compile Include="ValidationMessageHandlerFacts.cs" />
5758
<Compile Include="ValidationOutcomeProcessorFacts.cs" />
5859
<Compile Include="ValidationPackageFileServiceFacts.cs" />

‎tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,17 @@ public async Task DoesNotSetPackageStreamMetadataIfNotChanged()
103103
.Setup(x => x.DownloadPackageFileToDiskAsync(Package))
104104
.ReturnsAsync(stream);
105105

106+
var streamMetadata = new PackageStreamMetadata()
107+
{
108+
Size = Package.PackageFileSize,
109+
Hash = Package.Hash,
110+
HashAlgorithm = Package.HashAlgorithm
111+
};
112+
113+
PackageFileServiceMock
114+
.Setup(x => x.UpdatePackageBlobMetadataAsync(It.IsAny<Package>()))
115+
.ReturnsAsync(streamMetadata);
116+
106117
await Target.SetPackageStatusAsync(Package, ValidationSet, PackageStatus.Available);
107118

108119
PackageServiceMock.Verify(
@@ -120,6 +131,15 @@ public async Task SetsPackageStreamMetadataIfChanged()
120131
PackageFileServiceMock
121132
.Setup(x => x.DownloadPackageFileToDiskAsync(Package))
122133
.ReturnsAsync(stream);
134+
var streamMetadata = new PackageStreamMetadata()
135+
{
136+
Size = stream.Length,
137+
Hash = expectedHash,
138+
HashAlgorithm = CoreConstants.Sha512HashAlgorithmId
139+
};
140+
PackageFileServiceMock
141+
.Setup(x => x.UpdatePackageBlobMetadataAsync(It.IsAny<Package>()))
142+
.ReturnsAsync(streamMetadata);
123143
PackageServiceMock
124144
.Setup(x => x.UpdatePackageStreamMetadataAsync(Package, It.IsAny<PackageStreamMetadata>(), false))
125145
.Returns(Task.CompletedTask)
@@ -134,14 +154,6 @@ public async Task SetsPackageStreamMetadataIfChanged()
134154
PackageServiceMock.Verify(
135155
x => x.UpdatePackageStreamMetadataAsync(Package, actual, false),
136156
Times.Once);
137-
TelemetryServiceMock.Verify(
138-
x => x.TrackDurationToHashPackage(
139-
It.Is<TimeSpan>(y => y > TimeSpan.Zero),
140-
Package.PackageRegistration.Id,
141-
Package.NormalizedVersion,
142-
"SHA512",
143-
"System.IO.MemoryStream"),
144-
Times.Once);
145157
}
146158

147159
[Fact]
@@ -509,9 +521,16 @@ public BaseFacts()
509521
TelemetryServiceMock = new Mock<ITelemetryService>();
510522
LoggerMock = new Mock<ILogger<PackageStatusProcessor>>();
511523

524+
var streamMetadata = new PackageStreamMetadata()
525+
{
526+
Size = 1,
527+
Hash = "hash",
528+
HashAlgorithm = CoreConstants.Sha512HashAlgorithmId
529+
};
530+
512531
PackageFileServiceMock
513-
.Setup(x => x.DownloadPackageFileToDiskAsync(It.IsAny<Package>()))
514-
.ReturnsAsync(() => Stream.Null);
532+
.Setup(x => x.UpdatePackageBlobMetadataAsync(It.IsAny<Package>()))
533+
.ReturnsAsync(streamMetadata);
515534

516535
Target = new PackageStatusProcessor(
517536
PackageServiceMock.Object,

0 commit comments

Comments
 (0)