search

umbraco / Umbraco.StorageProviders

MIT License
30 stars 22 forks source link

Use directory delimiter when listing blobs #64

Closed ronaldbarendse closed 6 months ago

ronaldbarendse commented 6 months ago

While reviewing PR https://github.com/umbraco/Umbraco.StorageProviders/pull/63 (targeting v13), I noticed the IFileProvider (an ASP.NET Core read-only file system abstraction) implementation already correctly retrieved the directories, because it specifies the delimiter that enables traversing a virtual hierarchy of blobs as though it were a file system:

https://github.com/umbraco/Umbraco.StorageProviders/blob/aa5623b7b5f5c7d014847fa675b0679d8bf28118/src/Umbraco.StorageProviders.AzureBlob/AzureBlobFileProvider.cs#L53

This wasn't specified in the IFileSystem (the Umbraco read-write file system abstraction) implementation when listing blobs:

https://github.com/umbraco/Umbraco.StorageProviders/blob/93dc0596fb758fc9b685ca85551d9726535394a8/src/Umbraco.StorageProviders.AzureBlob/IO/AzureBlobFileSystem.cs#L359

Without this delimiter, only blobs are returned that start with the specified prefix. This means the GetDirectories() didn't return anything (because none of the returned items were prefixes), DirectoryExists() always returned false (because a prefix is not a blob) and most notably, GetFiles() would return all blobs including the ones in 'subdirectories' (similar to using SearchOption.AllDirectories).

This PR fixes these issues and aligns the behavior between IFileProvider and IFileSystem. Testing can be done using the same instructions as the linked PR. Once reviewed and merged, this can be released as 10.0.1 and merged up to support/12.0.x (released as 12.0.1) and develop (released as 13.0.1).

AndyButland commented 6 months ago

I've tried testing this for Umbraco 13 (as that's what I had set up locally) using the same setup is in the linked PR. I did this by copying your changes into what's currently in develop. But I don't see it working I'm afraid. DirectoryExists works only if I adjust the provided path to be media/aaa rather than just aaa. And I don't see GetDirectories return anything.

Might be the manual steps I did to test, but maybe could you try it on Umbraco 13 too and see if you see the same? If so, maybe there's something between versions that differs and needs handling in a different way.

ronaldbarendse commented 6 months ago

I've fixed the issue that caused the root path to be included in returned values by reverting some changes. The path gets prefixed with a forward slash again, before passing it onto GetRelativePath(), so it matches the root path and correctly removes it. This method was updated in PR #63 to automatically do this, hence the discrepancy...

ronaldbarendse commented 6 months ago

I've also optimized the DirectoryExists() call by trying to only get a single item/page. By default, the List Blobs endpoint will return up to 5000 items (see maxresults URL parameter) per page, but a single prefix or blob is enough to determine whether the directory exists 👌🏻