Skip to content

Commit 843a4eb

Browse files
authored
Fix Monitoring2Monitoring blob name generation, handle size mismatch when checking if blobs are synchronized (#10123)
* Handle blob name generate when base URI scheme does not match This was causing monitoring2monitoring to fail with an invalid blob name (double slashes). Scrub blob URL for logging * Do not consider a blob as synchronize if the size is difference * Add UTs
1 parent 32b79e8 commit 843a4eb

3 files changed

Lines changed: 53 additions & 34 deletions

File tree

src/Catalog/Persistence/AzureStorage.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -454,15 +454,21 @@ public async Task<bool> AreSynchronized(ICloudBlockBlob sourceBlockBlob, ICloudB
454454
{
455455
if (await sourceBlockBlob.ExistsAsync(CancellationToken.None))
456456
{
457-
var sourceBlobMetadata = await sourceBlockBlob.GetMetadataAsync(CancellationToken.None);
458-
var destinationBlobMetadata = await destinationBlockBlob.GetMetadataAsync(CancellationToken.None);
459-
if (sourceBlobMetadata == null || destinationBlobMetadata == null)
457+
BlobProperties sourceBlobAttributes = await sourceBlockBlob.FetchAttributesAsync(CancellationToken.None);
458+
BlobProperties destinationBlobAttributes = await destinationBlockBlob.FetchAttributesAsync(CancellationToken.None);
459+
if (sourceBlobAttributes?.Metadata == null || destinationBlobAttributes?.Metadata == null)
460460
{
461461
return false;
462462
}
463463

464-
var sourceBlobHasSha512Hash = sourceBlobMetadata.TryGetValue(Sha512HashAlgorithmId, out var sourceBlobSha512Hash);
465-
var destinationBlobHasSha512Hash = destinationBlobMetadata.TryGetValue(Sha512HashAlgorithmId, out var destinationBlobSha512Hash);
464+
if (sourceBlobAttributes.ContentLength != destinationBlobAttributes.ContentLength)
465+
{
466+
Trace.WriteLine($"The source blob ({RemoveQueryString(sourceBlockBlob.Uri)}) and destination blob ({RemoveQueryString(destinationBlockBlob.Uri)}) have different sizes.");
467+
return false;
468+
}
469+
470+
var sourceBlobHasSha512Hash = sourceBlobAttributes.Metadata.TryGetValue(Sha512HashAlgorithmId, out var sourceBlobSha512Hash);
471+
var destinationBlobHasSha512Hash = destinationBlobAttributes.Metadata.TryGetValue(Sha512HashAlgorithmId, out var destinationBlobSha512Hash);
466472
if (!sourceBlobHasSha512Hash)
467473
{
468474
Trace.TraceWarning($"The source blob ({RemoveQueryString(sourceBlockBlob.Uri)}) doesn't have the SHA512 hash.");

src/Catalog/Persistence/Storage.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,15 +199,19 @@ protected string GetName(Uri uri)
199199
{
200200
address += "/";
201201
}
202-
var uriString = uri.ToString();
202+
var uriString = uri.GetLeftPart(UriPartial.Path);
203203

204204
int baseAddressLength = address.Length;
205205

206-
var name = uriString.Substring(baseAddressLength);
206+
// handle mismatched scheme (http vs https)
207+
var schemeLengthDifference = uri.Scheme.Length - BaseAddress.Scheme.Length;
208+
209+
var name = uriString.Substring(baseAddressLength + schemeLengthDifference);
207210
if (name.Contains("#"))
208211
{
209212
name = name.Substring(0, name.IndexOf("#"));
210213
}
214+
211215
return name;
212216
}
213217

@@ -229,7 +233,7 @@ protected void TraceMethod(string method, Uri resourceUri)
229233
{
230234
//The Uri depends on the storage implementation.
231235
Uri storageUri = GetUri(GetName(resourceUri));
232-
Trace.WriteLine(String.Format("{0} {1}", method, storageUri));
236+
Trace.WriteLine(String.Format("{0} {1}", method, RemoveQueryString(storageUri)));
233237
}
234238
}
235239

@@ -240,7 +244,7 @@ public static string RemoveQueryString(Uri storageUri)
240244

241245
private string TraceException(string method, Uri resourceUri, Exception exception)
242246
{
243-
string message = $"{method} EXCEPTION: {GetUri(GetName(resourceUri))} {exception.ToString()}";
247+
string message = $"{method} EXCEPTION: {RemoveQueryString(GetUri(GetName(resourceUri)))} {exception.ToString()}";
244248
Trace.WriteLine(message);
245249
return message;
246250
}

tests/CatalogMetadataTests/AzureStorageFacts.cs

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -9,6 +9,7 @@
99
using Xunit;
1010
using NuGet.Services.Metadata.Catalog.Persistence;
1111
using Azure.Storage.Blobs;
12+
using Azure.Storage.Blobs.Models;
1213

1314
namespace CatalogMetadataTests
1415
{
@@ -19,25 +20,29 @@ public AzureStorageFacts() : base()
1920
}
2021

2122
[Theory]
22-
[InlineData(true, true, "SHA512Value1", true, true, "SHA512Value1", true)]
23-
[InlineData(true, true, "SHA512Value1", true, true, "SHA512Value2", false)]
24-
[InlineData(false, false, null, true, true, "SHA512Value1", true)]
25-
[InlineData(true, true, "SHA512Value1", false, false, null, false)]
26-
[InlineData(false, false, null, false, false, null, true)]
27-
[InlineData(true, false, null, true, true, "SHA512Value1", false)]
28-
[InlineData(true, true, "SHA512Value1", true, false, null, false)]
29-
[InlineData(true, false, null, true, false, null, false)]
30-
public async Task ValidateAreSynchronizedmethod(bool sourceBlobExists,
23+
[InlineData(true, true, "SHA512Value1", 10, true, true, "SHA512Value1", 10, true)]
24+
[InlineData(true, true, "SHA512Value1", 10, true, true, "SHA512Value2", 10, false)]
25+
[InlineData(false, false, null, 10, true, true, "SHA512Value1", 10, true)]
26+
[InlineData(true, true, "SHA512Value1", 10, false, false, null, 10, false)]
27+
[InlineData(false, false, null, 10, false, false, null, 10, true)]
28+
[InlineData(true, false, null, 10, true, true, "SHA512Value1", 10, false)]
29+
[InlineData(true, true, "SHA512Value1", 10, true, false, null, 10, false)]
30+
[InlineData(true, false, null, 10, true, false, null, 10, false)]
31+
[InlineData(true, true, "SHA512Value1", 10, true, true, "SHA512Value1", 11, false)]
32+
public async Task ValidateAreSynchronizedmethod(
33+
bool sourceBlobExists,
3134
bool hasSourceBlobSHA512Value,
3235
string sourceBlobSHA512Value,
36+
long sourceSize,
3337
bool destinationBlobExists,
3438
bool hasDestinationBlobSHA512Value,
3539
string destinationBlobSHA512Value,
40+
long destinationSize,
3641
bool expected)
3742
{
3843
// Arrange
39-
var sourceBlob = GetMockedBlockBlob(sourceBlobExists, hasSourceBlobSHA512Value, sourceBlobSHA512Value, new Uri("https://blockBlob1"));
40-
var destinationBlob = GetMockedBlockBlob(destinationBlobExists, hasDestinationBlobSHA512Value, destinationBlobSHA512Value, new Uri("https://blockBlob2"));
44+
var sourceBlob = GetMockedBlockBlob(sourceBlobExists, hasSourceBlobSHA512Value, sourceBlobSHA512Value, sourceSize, new Uri("https://blockBlob1"));
45+
var destinationBlob = GetMockedBlockBlob(destinationBlobExists, hasDestinationBlobSHA512Value, destinationBlobSHA512Value, destinationSize, new Uri("https://blockBlob2"));
4146

4247
// Act
4348
var isSynchronized = await _storage.AreSynchronized(sourceBlob.Object, destinationBlob.Object);
@@ -48,8 +53,8 @@ public async Task ValidateAreSynchronizedmethod(bool sourceBlobExists,
4853

4954
if (sourceBlobExists && destinationBlobExists)
5055
{
51-
sourceBlob.Verify(x => x.GetMetadataAsync(CancellationToken.None), Times.Once);
52-
destinationBlob.Verify(x => x.GetMetadataAsync(CancellationToken.None), Times.Once);
56+
sourceBlob.Verify(x => x.FetchAttributesAsync(CancellationToken.None), Times.Once);
57+
destinationBlob.Verify(x => x.FetchAttributesAsync(CancellationToken.None), Times.Once);
5358

5459
if (!hasSourceBlobSHA512Value)
5560
{
@@ -69,7 +74,7 @@ public async Task ValidateAreSynchronizedmethod(bool sourceBlobExists,
6974
Assert.Equal(expected, isSynchronized);
7075
}
7176

72-
private Mock<ICloudBlockBlob> GetMockedBlockBlob(bool isExisted, bool hasSHA512Value, string SHA512Value, Uri blockBlobUri)
77+
private Mock<ICloudBlockBlob> GetMockedBlockBlob(bool isExisted, bool hasSHA512Value, string SHA512Value, long size, Uri blockBlobUri)
7378
{
7479
var mockBlob = new Mock<ICloudBlockBlob>();
7580

@@ -82,16 +87,16 @@ private Mock<ICloudBlockBlob> GetMockedBlockBlob(bool isExisted, bool hasSHA512V
8287

8388
if (hasSHA512Value)
8489
{
85-
mockBlob.Setup(x => x.GetMetadataAsync(CancellationToken.None))
86-
.ReturnsAsync(new Dictionary<string, string>()
90+
mockBlob.Setup(x => x.FetchAttributesAsync(CancellationToken.None))
91+
.ReturnsAsync(BlobsModelFactory.BlobProperties(contentLength: size, metadata: new Dictionary<string, string>()
8792
{
8893
{ "SHA512", SHA512Value }
89-
});
94+
}));
9095
}
9196
else
9297
{
93-
mockBlob.Setup(x => x.GetMetadataAsync(CancellationToken.None))
94-
.ReturnsAsync(new Dictionary<string, string>());
98+
mockBlob.Setup(x => x.FetchAttributesAsync(CancellationToken.None))
99+
.ReturnsAsync(BlobsModelFactory.BlobProperties(contentLength: size, metadata: new Dictionary<string, string>()));
95100
}
96101
}
97102

@@ -116,13 +121,17 @@ private ICloudBlockBlob GetMockedBlockBlobWithNullMetadata(bool isBlobMetadataEx
116121
{
117122
if (isBlobMetadataExisted)
118123
{
119-
return Mock.Of<ICloudBlockBlob>(x => x.ExistsAsync(CancellationToken.None) == Task.FromResult(true) &&
120-
x.GetMetadataAsync(CancellationToken.None) == Task.FromResult<IReadOnlyDictionary<string, string>>(new Dictionary<string, string>()));
124+
var properties = BlobsModelFactory.BlobProperties(metadata: new Dictionary<string, string>());
125+
return Mock.Of<ICloudBlockBlob>(
126+
x => x.ExistsAsync(CancellationToken.None) == Task.FromResult(true) &&
127+
x.FetchAttributesAsync(CancellationToken.None) == Task.FromResult(properties));
121128
}
122129
else
123130
{
124-
return Mock.Of<ICloudBlockBlob>(x => x.ExistsAsync(CancellationToken.None) == Task.FromResult(true) &&
125-
x.GetMetadataAsync(CancellationToken.None) == Task.FromResult<IReadOnlyDictionary<string, string>>(null));
131+
var properties = BlobsModelFactory.BlobProperties(metadata: null);
132+
return Mock.Of<ICloudBlockBlob>(
133+
x => x.ExistsAsync(CancellationToken.None) == Task.FromResult(true) &&
134+
x.FetchAttributesAsync(CancellationToken.None) == Task.FromResult(properties));
126135
}
127136
}
128137
}
@@ -166,4 +175,4 @@ public AzureStorageBaseFacts()
166175
false);
167176
}
168177
}
169-
}
178+
}

0 commit comments

Comments
 (0)