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

Jump to bottom

Update BlobStorage to use Azure.Storage.Blobs #1564

Merged
merged 21 commits into from
Oct 8, 2021
Merged

Update BlobStorage to use Azure.Storage.Blobs #1564

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
107416d
Use Azure.Storage.Blobs Nuget packge to replace deprecated Microsoft.…
Mar 24, 2021
5d0eff1
Add missing PiranhaServiceBuilder extension method
Mar 25, 2021
17a8945
Remove unused Nuget package
Mar 25, 2021
465bc65
Add extension method check for success HTTP Status code
Mar 25, 2021
b2591fe
Optimize code
Mar 25, 2021
c923859
Add interface IInitializable to make Init() available in select places
Mar 25, 2021
0e46ed1
Use IInitializable and IInitializable.Init() to transpile Init calls
Mar 25, 2021
51f9122
Merge IStorageSession and IStorage
Mar 25, 2021
48d849f
Move Media.Init() call to App class
Mar 25, 2021
4ae3be6
Modify docs
Mar 25, 2021
0d64de5
Fix
Mar 25, 2021
25c9b10
Correct order of code
Mar 25, 2021
682a890
Reverse some of the breaking changes
Mar 25, 2021
69e6973
Remove duplicate empty line (make CodeFactor test pass)
Mar 25, 2021
dbba1b7
Remove unused `using` refs
Mar 25, 2021
1ad80e5
Undo changes to Piranha.Local.FileStorage
Mar 26, 2021
a7efc1c
Undo changes to IStorageSession
Mar 26, 2021
d9dcf2a
Undo changes
tedvanderveen Mar 26, 2021
daa90a3
Restore IStorageSession.cs
Mar 26, 2021
00a155f
Merge branch 'master' of https://github.com/tedvanderveen/piranha.core
Mar 26, 2021
a881b58
Revert changes to IMediaService
Mar 26, 2021
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
142 changes: 84 additions & 58 deletions core/Piranha.Azure.BlobStorage/BlobStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,22 @@
*
*/

using Microsoft.Azure.Storage;
using Microsoft.Azure.Storage.Auth;
using Microsoft.Azure.Storage.Blob;
using System;
using System.IO;
using System.Threading.Tasks;
using Azure.Core;
using Azure.Storage.Blobs;
using Azure.Storage.Blobs.Models;
using Piranha.Models;

namespace Piranha.Azure
{
public class BlobStorage : IStorage
public class BlobStorage : IStorage, IStorageSession
{
/// <summary>
/// The private storage account.
/// </summary>
private readonly CloudStorageAccount _storage;

/// <summary>
/// The name of the container to use.
/// </summary>
private readonly string _containerName;

/// <summary>
/// The container url.
/// </summary>
private string _containerUrl;
private readonly BlobContainerClient _storage;

/// <summary>
/// How uploaded files should be named to
Expand All @@ -42,17 +34,20 @@ public class BlobStorage : IStorage
/// <summary>
/// Creates a new Blog Storage service from the given credentials.
/// </summary>
/// <param name="credentials">The connection credentials</param>
/// <param name="containerName">The container name</param>
/// <param name="blobContainerUri">
/// A <see cref="Uri"/> referencing the blob service.
/// This is likely to be similar to "https://{account_name}.blob.core.windows.net".
/// </param>
/// <param name="tokenCredential">The connection credentials</param>
/// <param name="naming">How uploaded media files should be named</param>
public BlobStorage(
StorageCredentials credentials,
string containerName = "uploads",
Uri blobContainerUri,
TokenCredential tokenCredential,
BlobStorageNaming naming = BlobStorageNaming.UniqueFileNames)
{
_storage = new CloudStorageAccount(credentials, true);
_containerName = containerName;
_storage = new BlobContainerClient(blobContainerUri, tokenCredential);
_naming = naming;
_storage.CreateIfNotExists(PublicAccessType.Blob);
}

/// <summary>
Expand All @@ -66,31 +61,18 @@ public BlobStorage(
string containerName = "uploads",
BlobStorageNaming naming = BlobStorageNaming.UniqueFileNames)
{
_storage = CloudStorageAccount.Parse(connectionString);
_containerName = containerName;
_storage = new BlobContainerClient(connectionString, containerName);
_naming = naming;
_storage.CreateIfNotExists(PublicAccessType.Blob);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this create a request towards Azure? If so I would recommend keeping the async variant as was before.

Copy link
Contributor Author

@tedvanderveen tedvanderveen Mar 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikaellindemann this should not be inside the constructor, agree. On the other hand, the method IStorage.OpenAsync() would not be a proper place either. Maybe an idea to add a method IStorage.InitializeAsync() where initial plumbing and checks can be performed?

When using Azure.Storage.Blobs assembly classes, there is no real concept of "sessions".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikaellindemann this now moved to:
https://github.com/PiranhaCMS/piranha.core/pull/1564/files#diff-1d62fb7d41884231bb1421e147495f8cd812a633cde9981b64684e29639056c4R164

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point was that if it does asynchronous operations (IO being one of them) then it should be an asynchronous function. I didn't check what is behind CreateIfNotExists, but since ExistsAsync and CreateAsync was in use before, I guess that CreateIfNotExists is waiting on the asynchronous operation behind the scenes.

Not sure if I encouraged this change you have made, sorry if I did, but now you have introduced a breaking change in the storage extension point of Piranha. Also, you have limited any storage implementation not to be session based.

Wouldn't it make sense to keep the storage and session implementation as was, such that a BlobContainerClient was the underlying session for a BlobStorage? (Tbh. I don't know the details of IStorage, IStorageSession and Azure BlobStorage, so maybe this is a bad suggestion).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikaellindemann I reversed most of those breaking changes. Guess I was a bit too enthousiastic. Please take a second look at the result.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And with regards to that CreateIfNotExists, it's about ensuring the parent Storage Container is available. So this should optimally be part of the App Init chaining.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And with Azure.Storage.Blobs assembly there is no real concept of sessions.
The BlobContainerClient is maintained as a singleton via DI / BlobStorage class. And BlobClient class instances are lightweight and are instantiated from the BlobContainerClient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikaellindemann I have now reverted any breaking changes to existing interfaces. And reverted all changes to the Piranha.Local.FileStorage source.
What remains now inside this PR:

  1. the move to the Azure SDK v.12 using the recommended Azure.Storage.Blobs Nuget packages
  2. Add the interface IInitializable to optionally indicate that a class implements Init() method, which is then called via App.cs

}

/// <summary>
/// Opens a new storage session.
/// </summary>
/// <returns>A new open session</returns>
public async Task<IStorageSession> OpenAsync()
public Task<IStorageSession> OpenAsync()
{
var session = _storage.CreateCloudBlobClient();
var container = session.GetContainerReference(_containerName);

if (!await container.ExistsAsync())
{
await container.CreateAsync();
await container.SetPermissionsAsync(new BlobContainerPermissions
{
PublicAccess = BlobContainerPublicAccessType.Blob
});
}
_containerUrl = container.Uri.AbsoluteUri;

return new BlobStorageSession(this, container, _naming);
return Task.FromResult<IStorageSession>(this);
}

/// <summary>
Expand All @@ -101,18 +83,7 @@ await container.SetPermissionsAsync(new BlobContainerPermissions
/// <returns>The public url</returns>
public string GetPublicUrl(Media media, string id)
{
if (media != null && !string.IsNullOrWhiteSpace(id))
{
if (string.IsNullOrEmpty(_containerUrl))
{
var session = _storage.CreateCloudBlobClient();
var container = session.GetContainerReference(_containerName);

_containerUrl = container.Uri.AbsoluteUri;
}
return $"{ _containerUrl }/{ GetResourceName(media, id, true) }";
}
return null;
return string.IsNullOrWhiteSpace(id) ? default : $"{_storage.Uri.AbsoluteUri}/{GetResourceName(media, id, true)}";
}

/// <summary>
Expand All @@ -135,14 +106,69 @@ public string GetResourceName(Media media, string filename)
/// <returns>The public url</returns>
public string GetResourceName(Media media, string filename, bool encode)
{
if (_naming == BlobStorageNaming.UniqueFileNames)
{
return $"{ media.Id }-{ (encode ? System.Web.HttpUtility.UrlPathEncode(filename) : filename) }";
}
else
{
return $"{ media.Id }/{ (encode ? System.Web.HttpUtility.UrlPathEncode(filename) : filename) }";
}
return _naming == BlobStorageNaming.UniqueFileNames ? $"{media.Id}-{(encode ? System.Web.HttpUtility.UrlPathEncode(filename) : filename)}" : $"{media.Id}/{(encode ? System.Web.HttpUtility.UrlPathEncode(filename) : filename)}";
}

public void Dispose()
{
}

/// <summary>
/// Writes the content for the specified media content to the given stream.
/// </summary>
/// <param name="media">The media file</param>
/// <param name="filename">The file name</param>
/// <param name="stream">The output stream</param>
/// <returns>If the media was found</returns>
public async Task<bool> GetAsync(Media media, string filename, Stream stream)
{
var blob = _storage.GetBlobClient(GetResourceName(media, filename));

return await blob.ExistsAsync() && (await blob.DownloadToAsync(stream)).Status.IsSuccessStatusCode();
}

/// <summary>
/// Stores the given media content.
/// </summary>
/// <param name="media">The media file</param>
/// <param name="filename">The file name</param>
/// <param name="contentType">The content type</param>
/// <param name="stream">The input stream</param>
/// <returns>The public URL</returns>
public async Task<string> PutAsync(Media media, string filename, string contentType, Stream stream)
{
var blob = _storage.GetBlobClient(GetResourceName(media, filename));

var blobHttpHeader = new BlobHttpHeaders {ContentType = contentType};

await blob.UploadAsync(stream, blobHttpHeader);

return blob.Uri.AbsoluteUri;
}

/// <summary>
/// Stores the given media content.
/// </summary>
/// <param name="media">The media file</param>
/// <param name="filename">The file name</param>
/// <param name="contentType">The content type</param>
/// <param name="bytes">The binary data</param>
/// <returns>The public URL</returns>
public async Task<string> PutAsync(Media media, string filename, string contentType, byte[] bytes)
{
return await PutAsync(media, filename, contentType, new MemoryStream(bytes));
}

/// <summary>
/// Deletes the content for the specified media.
/// </summary>
/// <param name="media">The media file</param>
/// <param name="filename">The file name</param>
public async Task<bool> DeleteAsync(Media media, string filename)
{
var blob = _storage.GetBlobClient(GetResourceName(media, filename));

return await blob.DeleteIfExistsAsync();
}
}
}
35 changes: 23 additions & 12 deletions core/Piranha.Azure.BlobStorage/BlobStorageExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
*
*/

using System;
using Azure.Core;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Azure.Storage.Auth;
using Piranha;
using Piranha.Azure;

Expand All @@ -19,23 +20,28 @@ public static class BlobStorageExtensions
/// Adds the Azure BlobStorage module.
/// </summary>
/// <param name="serviceBuilder">The service builder</param>
/// <param name="credentials">The auth credentials</param>
/// <param name="containerName">The optional container name</param>
/// <param name="credential">
/// The token credential used to sign requests.
/// </param>
/// <param name="blobContainerUri">
/// A <see cref="Uri"/> referencing the blob service.
/// This is likely to be similar to "https://{account_name}.blob.core.windows.net".
/// </param>
/// <param name="naming">How uploaded media files should be named</param>
/// <param name="scope">The optional service scope. Default is singleton</param>
/// <returns>The service collection</returns>
public static PiranhaServiceBuilder UseBlobStorage(
this PiranhaServiceBuilder serviceBuilder,
StorageCredentials credentials,
string containerName = "uploads",
Uri blobContainerUri,
TokenCredential credential,
BlobStorageNaming naming = BlobStorageNaming.UniqueFileNames,
ServiceLifetime scope = ServiceLifetime.Singleton)
{
serviceBuilder.Services.AddPiranhaBlobStorage(credentials, containerName, naming, scope);
serviceBuilder.Services.AddPiranhaBlobStorage(blobContainerUri, credential, naming, scope);

return serviceBuilder;
}

/// <summary>
/// Adds the Azure BlobStorage module.
/// </summary>
Expand All @@ -61,22 +67,27 @@ public static PiranhaServiceBuilder UseBlobStorage(
/// Adds the services for the Azure BlobStorage service.
/// </summary>
/// <param name="services">The current service collection</param>
/// <param name="credentials">The auth credentials</param>
/// <param name="containerName">The optional container name</param>
/// <param name="credential">
/// The token credential used to sign requests.
/// </param>
/// <param name="blobContainerUri">
/// A <see cref="Uri"/> referencing the blob service.
/// This is likely to be similar to "https://{account_name}.blob.core.windows.net".
/// </param>
/// <param name="naming">How uploaded media files should be named</param>
/// <param name="scope">The optional service scope. Default is singleton</param>
/// <returns>The service collection</returns>
public static IServiceCollection AddPiranhaBlobStorage(
this IServiceCollection services,
StorageCredentials credentials,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constitutes the main breaking change when moving to the v.12 Azure SDK, as the StorageCredentials class is no longer available.

string containerName = "uploads",
Uri blobContainerUri,
TokenCredential credential,
BlobStorageNaming naming = BlobStorageNaming.UniqueFileNames,
ServiceLifetime scope = ServiceLifetime.Singleton)
{
App.Modules.Register<BlobStorageModule>();

services.Add(new ServiceDescriptor(typeof(IStorage), sp =>
new BlobStorage(credentials, containerName, naming), scope));
new BlobStorage(blobContainerUri, credential, naming), scope));

return services;
}
Expand Down
Loading