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.

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

Fix Monitoring2Monitoring blob name generation, handle size mismatch when checking if blobs are synchronized #10123

Merged
merged 5 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 11 additions & 5 deletions src/Catalog/Persistence/AzureStorage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -454,15 +454,21 @@ public async Task<bool> AreSynchronized(ICloudBlockBlob sourceBlockBlob, ICloudB
{
if (await sourceBlockBlob.ExistsAsync(CancellationToken.None))
{
var sourceBlobMetadata = await sourceBlockBlob.GetMetadataAsync(CancellationToken.None);
var destinationBlobMetadata = await destinationBlockBlob.GetMetadataAsync(CancellationToken.None);
if (sourceBlobMetadata == null || destinationBlobMetadata == null)
BlobProperties sourceBlobAttributes = await sourceBlockBlob.FetchAttributesAsync(CancellationToken.None);
BlobProperties destinationBlobAttributes = await destinationBlockBlob.FetchAttributesAsync(CancellationToken.None);
if (sourceBlobAttributes?.Metadata == null || destinationBlobAttributes?.Metadata == null)
{
return false;
}

var sourceBlobHasSha512Hash = sourceBlobMetadata.TryGetValue(Sha512HashAlgorithmId, out var sourceBlobSha512Hash);
var destinationBlobHasSha512Hash = destinationBlobMetadata.TryGetValue(Sha512HashAlgorithmId, out var destinationBlobSha512Hash);
if (sourceBlobAttributes.ContentLength != destinationBlobAttributes.ContentLength)
{
Trace.WriteLine($"The source blob ({RemoveQueryString(sourceBlockBlob.Uri)}) and destination blob ({RemoveQueryString(destinationBlockBlob.Uri)}) have different sizes.");
return false;
}

var sourceBlobHasSha512Hash = sourceBlobAttributes.Metadata.TryGetValue(Sha512HashAlgorithmId, out var sourceBlobSha512Hash);
var destinationBlobHasSha512Hash = destinationBlobAttributes.Metadata.TryGetValue(Sha512HashAlgorithmId, out var destinationBlobSha512Hash);
if (!sourceBlobHasSha512Hash)
{
Trace.TraceWarning($"The source blob ({RemoveQueryString(sourceBlockBlob.Uri)}) doesn't have the SHA512 hash.");
Expand Down
12 changes: 8 additions & 4 deletions src/Catalog/Persistence/Storage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,19 @@ protected string GetName(Uri uri)
{
address += "/";
}
var uriString = uri.ToString();
var uriString = uri.GetLeftPart(UriPartial.Path);

int baseAddressLength = address.Length;

var name = uriString.Substring(baseAddressLength);
// handle mismatched scheme (http vs https)
var schemeLengthDifference = uri.Scheme.Length - BaseAddress.Scheme.Length;

var name = uriString.Substring(baseAddressLength + schemeLengthDifference);
if (name.Contains("#"))
{
name = name.Substring(0, name.IndexOf("#"));
}

return name;
}

Expand All @@ -229,7 +233,7 @@ protected void TraceMethod(string method, Uri resourceUri)
{
//The Uri depends on the storage implementation.
Uri storageUri = GetUri(GetName(resourceUri));
Trace.WriteLine(String.Format("{0} {1}", method, storageUri));
Trace.WriteLine(String.Format("{0} {1}", method, RemoveQueryString(storageUri)));
joelverhagen marked this conversation as resolved.
Show resolved Hide resolved
}
}

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

private string TraceException(string method, Uri resourceUri, Exception exception)
{
string message = $"{method} EXCEPTION: {GetUri(GetName(resourceUri))} {exception.ToString()}";
string message = $"{method} EXCEPTION: {RemoveQueryString(GetUri(GetName(resourceUri)))} {exception.ToString()}";
Trace.WriteLine(message);
return message;
}
Expand Down
59 changes: 34 additions & 25 deletions tests/CatalogMetadataTests/AzureStorageFacts.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand All @@ -9,6 +9,7 @@
using Xunit;
using NuGet.Services.Metadata.Catalog.Persistence;
using Azure.Storage.Blobs;
using Azure.Storage.Blobs.Models;

namespace CatalogMetadataTests
{
Expand All @@ -19,25 +20,29 @@ public AzureStorageFacts() : base()
}

[Theory]
[InlineData(true, true, "SHA512Value1", true, true, "SHA512Value1", true)]
[InlineData(true, true, "SHA512Value1", true, true, "SHA512Value2", false)]
[InlineData(false, false, null, true, true, "SHA512Value1", true)]
[InlineData(true, true, "SHA512Value1", false, false, null, false)]
[InlineData(false, false, null, false, false, null, true)]
[InlineData(true, false, null, true, true, "SHA512Value1", false)]
[InlineData(true, true, "SHA512Value1", true, false, null, false)]
[InlineData(true, false, null, true, false, null, false)]
public async Task ValidateAreSynchronizedmethod(bool sourceBlobExists,
[InlineData(true, true, "SHA512Value1", 10, true, true, "SHA512Value1", 10, true)]
[InlineData(true, true, "SHA512Value1", 10, true, true, "SHA512Value2", 10, false)]
[InlineData(false, false, null, 10, true, true, "SHA512Value1", 10, true)]
[InlineData(true, true, "SHA512Value1", 10, false, false, null, 10, false)]
[InlineData(false, false, null, 10, false, false, null, 10, true)]
[InlineData(true, false, null, 10, true, true, "SHA512Value1", 10, false)]
[InlineData(true, true, "SHA512Value1", 10, true, false, null, 10, false)]
[InlineData(true, false, null, 10, true, false, null, 10, false)]
[InlineData(true, true, "SHA512Value1", 10, true, true, "SHA512Value1", 11, false)]
public async Task ValidateAreSynchronizedmethod(
bool sourceBlobExists,
bool hasSourceBlobSHA512Value,
string sourceBlobSHA512Value,
long sourceSize,
bool destinationBlobExists,
bool hasDestinationBlobSHA512Value,
string destinationBlobSHA512Value,
long destinationSize,
bool expected)
{
// Arrange
var sourceBlob = GetMockedBlockBlob(sourceBlobExists, hasSourceBlobSHA512Value, sourceBlobSHA512Value, new Uri("https://blockBlob1"));
var destinationBlob = GetMockedBlockBlob(destinationBlobExists, hasDestinationBlobSHA512Value, destinationBlobSHA512Value, new Uri("https://blockBlob2"));
var sourceBlob = GetMockedBlockBlob(sourceBlobExists, hasSourceBlobSHA512Value, sourceBlobSHA512Value, sourceSize, new Uri("https://blockBlob1"));
var destinationBlob = GetMockedBlockBlob(destinationBlobExists, hasDestinationBlobSHA512Value, destinationBlobSHA512Value, destinationSize, new Uri("https://blockBlob2"));

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

if (sourceBlobExists && destinationBlobExists)
{
sourceBlob.Verify(x => x.GetMetadataAsync(CancellationToken.None), Times.Once);
destinationBlob.Verify(x => x.GetMetadataAsync(CancellationToken.None), Times.Once);
sourceBlob.Verify(x => x.FetchAttributesAsync(CancellationToken.None), Times.Once);
destinationBlob.Verify(x => x.FetchAttributesAsync(CancellationToken.None), Times.Once);

if (!hasSourceBlobSHA512Value)
{
Expand All @@ -69,7 +74,7 @@ public async Task ValidateAreSynchronizedmethod(bool sourceBlobExists,
Assert.Equal(expected, isSynchronized);
}

private Mock<ICloudBlockBlob> GetMockedBlockBlob(bool isExisted, bool hasSHA512Value, string SHA512Value, Uri blockBlobUri)
private Mock<ICloudBlockBlob> GetMockedBlockBlob(bool isExisted, bool hasSHA512Value, string SHA512Value, long size, Uri blockBlobUri)
{
var mockBlob = new Mock<ICloudBlockBlob>();

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

if (hasSHA512Value)
{
mockBlob.Setup(x => x.GetMetadataAsync(CancellationToken.None))
.ReturnsAsync(new Dictionary<string, string>()
mockBlob.Setup(x => x.FetchAttributesAsync(CancellationToken.None))
.ReturnsAsync(BlobsModelFactory.BlobProperties(contentLength: size, metadata: new Dictionary<string, string>()
{
{ "SHA512", SHA512Value }
});
}));
}
else
{
mockBlob.Setup(x => x.GetMetadataAsync(CancellationToken.None))
.ReturnsAsync(new Dictionary<string, string>());
mockBlob.Setup(x => x.FetchAttributesAsync(CancellationToken.None))
.ReturnsAsync(BlobsModelFactory.BlobProperties(contentLength: size, metadata: new Dictionary<string, string>()));
}
}

Expand All @@ -116,13 +121,17 @@ private ICloudBlockBlob GetMockedBlockBlobWithNullMetadata(bool isBlobMetadataEx
{
if (isBlobMetadataExisted)
{
return Mock.Of<ICloudBlockBlob>(x => x.ExistsAsync(CancellationToken.None) == Task.FromResult(true) &&
x.GetMetadataAsync(CancellationToken.None) == Task.FromResult<IReadOnlyDictionary<string, string>>(new Dictionary<string, string>()));
var properties = BlobsModelFactory.BlobProperties(metadata: new Dictionary<string, string>());
return Mock.Of<ICloudBlockBlob>(
x => x.ExistsAsync(CancellationToken.None) == Task.FromResult(true) &&
x.FetchAttributesAsync(CancellationToken.None) == Task.FromResult(properties));
}
else
{
return Mock.Of<ICloudBlockBlob>(x => x.ExistsAsync(CancellationToken.None) == Task.FromResult(true) &&
x.GetMetadataAsync(CancellationToken.None) == Task.FromResult<IReadOnlyDictionary<string, string>>(null));
var properties = BlobsModelFactory.BlobProperties(metadata: null);
return Mock.Of<ICloudBlockBlob>(
x => x.ExistsAsync(CancellationToken.None) == Task.FromResult(true) &&
x.FetchAttributesAsync(CancellationToken.None) == Task.FromResult(properties));
}
}
}
Expand Down Expand Up @@ -166,4 +175,4 @@ public AzureStorageBaseFacts()
false);
}
}
}
}