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

ABP Storage integration #653

Closed
wants to merge 14 commits into from
Closed

ABP Storage integration #653

wants to merge 14 commits into from

Conversation

hitaspdotnet
Copy link
Contributor

@hitaspdotnet hitaspdotnet commented Dec 14, 2018

Include LocalStorage && AzureBlob

@hikalkan hikalkan added this to the 0.10 milestone Dec 18, 2018
@hikalkan hikalkan self-requested a review December 18, 2018 13:55
@hikalkan
Copy link
Member

Thank you @hitaspdotnet. This is a huge contribution and will take time to review.
If I merge it, do you plan to write documentation of usage?

@hitaspdotnet
Copy link
Contributor Author

hitaspdotnet commented Dec 18, 2018

@hikalkan My English is to bad but sure I try my best. I have created ~113 tests for this module. And now i'm working on commenting all methods. Azure, Amazon S3 and Google storage is completed but i can't test them in real requests. Because I'm leaving in Iran and their companies was blocked Iranian users. AbpStorage's API works well for FileSystem storage, but is possible we have problems for cloud storage. I've read my code many times over and over again for last 2 months to find problems without access to real storage

@hitaspdotnet
Copy link
Contributor Author

hitaspdotnet commented Dec 18, 2018

@hikalkan I want to create new branches for Amazon and Google. #1aada46 is my last push at this branch.

@hikalkan hikalkan self-assigned this Dec 19, 2018
@hikalkan
Copy link
Member

OK, I will review & merge this. Thanks.

@hikalkan hikalkan modified the milestones: 0.10, 0.11 Dec 20, 2018
@hikalkan
Copy link
Member

It is a huge contribution. Thank you again. I tried to review it however it seems I should allocate more time for it. So, I will work on this in the next week. We may need to have a talk on that because I may need to listen some high level explanation, usage and configuration of it.

BTW, it would be very helpful if you can write how to use it.

@hitaspdotnet
Copy link
Contributor Author

hitaspdotnet commented Dec 20, 2018

Hi @hikalkan
Here are the objectives of this libraries :

  • Full fledged FileSystem Based implementation.
  • Configuration based Provider Switching.
  • Allow users to easily Migrate From or To Azure with a simple AzCopy operation.

Volo.Abp.Storage : The storage abstractions. This does nothing by itself and you need at least one storage provider module as well.

Volo.Abp.Storage.Azure : Azure storage based provider which store files in a azure storage blob container.

Volo.Abp.Storage.FileSystem : File system based provider which store files in a file system directory.

Volo.Abp.Storage.FileSystem.ExtendedProperties : File system based Extended Properties provider which provide File System meta data.

Volo.Abp.Storage.FileSystem.Server : Asp.Net Core middleware emulating Azure public container feature. (used for PublicUrlProvider yet because i'm not pro with building middleware and FileSystemStorageServerMiddleware needs refactoring)

Features :

  • Read
  • Write
  • Delete
  • Copy -not yet
  • Move -not yet
  • List files (including globbing support)
  • System Metadata (partial support for FileSystem)
  • Custom Metadata (Azure only)

Usage

I want to explain usages in an Module and an Application.

Module

In modules where we haven't json configuration we can inject IAbpStoreWithOptions<TOptions>.
Then we need to configure TOptions in module initialize.

    public class TestStoreOptions : IAbpStoreOptions
    {
        public string ProviderName { get; set; }

        public string ProviderType { get; set; }

        public AbpStorageAccessLevel AccessLevel { get; set; }

        public string FolderName { get; set; }

        public string Name { get; set; }

        public IEnumerable<IAbpStorageOptionError> Validate(bool throwOnError = true)
        {
            //you can add your validation methods here and push errors
            //
            //var baseErrors = base.Validate(throwOnError);
            //var optionErrors = new List<AbpStorageOptionError>();

            //if (string.IsNullOrEmpty(AbsolutePath))
            //{
            //    PushMissingPropertyError(optionErrors, nameof(AbsolutePath));
            //}

            //var finalErrors = baseErrors.Concat(optionErrors);

            //if (throwOnError && finalErrors.Any())
            //{
            //    throw new BadStoreConfigurationException(Name, finalErrors);
            //}

            //return finalErrors;

            return Enumerable.Empty<IAbpStorageOptionError>();
        }
    }

And:

        [DependsOn(
            typeof(AbpFileSystemStorageModule),
            typeof(AbpFileSystemStorageExtendedPropertiesModule),
            typeof(AbpFileSystemStorageServerModule))]
        public class TestModule : AbpModule
        {
            public override void ConfigureServices(ServiceConfigurationContext context)
            {
                context.Services.Configure<TestStoreOptions>(storageOptions =>
                {
                   //required
                   storageOptions.Name = "StoreName";
                   //required
                   storageOptions.ProviderType = "FileSystem";
                   //optional (if null combine current directory + storageOptions.Name by default
                   storageOptions.FolderName = "StoreName";
                });
            }
        }

        [Fact]
        public async Task List_RootFiles()
        {
            var store = GetRequiredService<IAbpStoreWithOptions<TestStoreOptions>>();

            var results = await store.ListAsync(null);
        }

Application

or modules with appsettings.json configuration

Just add the DependsOn attributes on your AbpModule and configure AppStorage section at your appsettings.json. Then inject IAbpStorageFactory and GetStore by Name.

{
  "ConnectionStrings": {
    "Default": "DefaultEndpointsProtocol=https;AccountName=<YourAccount>;AccountKey=<YourKey>;EndpointSuffix=core.windows.net"
  },

  "AppStorage": {
    "Providers": {
      "FirstAzure": {
        "Type": "Azure",
        "ConnectionString": "DefaultEndpointsProtocol=https;AccountName=<YourAccount>;AccountKey=<YourKey>;EndpointSuffix=core.windows.net"
      },
      "AnotherAzure": {
        "Type": "Azure",
        "ConnectionStringName": "Default"
      },
      "FirstFileSystem": {
        "Type": "FileSystem"
      },
      "AnotherFileSystem": {
        "Type": "FileSystem",
        "RootPath": "../FileVault2"
      }
    },

    "Stores": {
      "FileSystemTestStore1": {
        "ProviderName": "FirstFileSystem",
        "FolderName": "SampleStore"
      },
      "FileSystemTestStore2": {
        "ProviderName": "FirstFileSystem",
        "AccessLevel": "Public",
        "FolderName": "SampleStore"
      },
      "AzureTestStore1": {
        "ProviderName": "FirstAzure",
        "AccessLevel": "Private"
      },
      "AzureTestStore2": {
        "ProviderType": "Azure",
        "ConnectionString": "DefaultEndpointsProtocol=https;AccountName=<YourAccount>;AccountKey=<YourKey>;EndpointSuffix=core.windows.net"
      },
      "AzureTestStore3": {
        "ProviderName": "AnotherAzure"
      },
      "AzureTestStore4": {
        "ProviderType": "Azure",
        "ConnectionStringName": "Default"
      }
    },

    "ScopedStores": {
      "ScopedFileSystemTestStore": {
        "ProviderName": "AnotherFileSystem",
        "FolderNameFormat": "AnotherPath-{0}"
      },
      "ScopedAzureTestStore": {
        "ProviderName": "AnotherAzure",
        "AccessLevel": "Confidential",
        "FolderNameFormat": "AnotherPath-{0}"
      }
    }
  }
}
        [DependsOn(
            typeof(AbpFileSystemStorageModule),
            typeof(AbpFileSystemStorageExtendedPropertiesModule),
            typeof(AbpFileSystemStorageServerModule))]
        public class TestModule : AbpModule
        {

        }

        [Fact]
        public async Task List_RootFiles()
        {
            var storageFactory = GetRequiredService<IAbpStorageFactory>();

            var store = storageFactory.GetStore("FileSystemTestStore1");

            var results = await store.ListAsync(null);
        }

You can filter your files by extensions or name or ETag. See FileSystemStorage_Tests.cs for more.
The store will return a list of IFileReference that expose various method and properties; allowing you to read, edit or delete file and metadata.

@hikalkan hikalkan modified the milestones: 0.11, 0.12 Jan 3, 2019
@hikalkan hikalkan modified the milestones: 0.12, 0.13 Jan 11, 2019
@hikalkan hikalkan modified the milestones: 0.13, 0.14 Jan 25, 2019
@hikalkan hikalkan removed this from the 0.14 milestone Feb 20, 2019
@hikalkan hikalkan modified the milestones: 0.20, 0.21 Aug 26, 2019
@GFoley83
Copy link

GFoley83 commented Sep 2, 2019

Not a great idea to store the con string in plain text

This doesn't really make sense as all connection strings for ABP live in the appsetting.json in plain text?
And technically the ABP EF Core module could also use managed identity but doesn't.
In other words, I think you're being a little overly critical here and while key vault, encrypted app setting values and managed identity would all be great to have, it seems unfair to me to ask for it to be included in a PR when the same requested functionally does not exist anywhere else in the solution yet.

@bbakermmc
Copy link
Contributor

@GFoley83 I dont see a PR for me to comment to use MI for the EF stuff, otherwise yes anything that can use MI it should, but the MI for EF is pretty much a one liner in the dbContext. This PR is enhancing functionality so saying it "should" support MI and shouldnt use appconfig settings arent outside the realm. Plus who wants to take down the site to add a new storage account. Just seems poorly thought out, and honestly it should be configurable per tenant. But I dont remember what all this code does, Ive stopped following the development.

@hikalkan hikalkan modified the milestones: 0.21, 0.20 Sep 10, 2019
@hikalkan hikalkan modified the milestones: 0.20, backlog Sep 20, 2019
@hitaspdotnet
Copy link
Contributor Author

hitaspdotnet commented Oct 4, 2019

@hikalkan I'm back. Ready for any change. Backlog = archived 😄

image

@hitaspdotnet
Copy link
Contributor Author

We can remove Volo.Abp.Storage.FileSystem.Server from Framework and create new module Volo.Abp.StorageManagement to produce web url for all files :
api/storage/{storeName}/{filePath}
api/storage/blogging/postImages/abp-netcore.png

@bbakermmc
Copy link
Contributor

I still confused as to what the point of the PR is suppose to offer. Like shouldnt this all be configurable per tenant? I never want my tenants files in the same SA, nor do I ever want to modify appsettings to add a new one... Currently this module doesnt seem like its ready for Production Multi Tenancy use.

I think the design needs further thought out or expanded.
IE Configurable Per Tenant
How does a module write/pull data, do it make a new container?

If we take blogs for example I would expect all assets uploaded would live in TenantSA -> Blogs container. If the tenant doesnt have a SA then where does it live? So how does this work with already created modules? Does the host/site need a SA? Can the host make a Global SA then all tenants could use? Can you then filter by tenant?

Once you start dealing with Multi Tenancy and Files Security becomes a huge issue. Many clients require data isolation

@hitaspdotnet
Copy link
Contributor Author

hitaspdotnet commented Oct 4, 2019

@bbakermmc I disagree with you. this libraries designed for working in Framework level for read/write binaries. If you or anyone else wants to have multi-tenancy support storage then you need to create that on app module level to isolate tenant files. Simplest way is a File's table which isolate data by tenants and contains store name + file path and file paths are unique. We can configure providers in app modules too.
Are you looking for File system HDrive/Azure Storage Account per tenant? It's just a file reader/writer for sharing binaries with other modules and isn't google storage.

@hitaspdotnet
Copy link
Contributor Author

If we take blogs for example I would expect all assets uploaded would live in TenantSA -> Blogs container.

You can do it simply in file system (needs few changes for azure). Just pass the path of you want,

var filePath = "bbakermmcTenantName/blogging/201909/Images/UniqueFileName.jpg";
await store.SaveAsync(imageBytes, filePath, "image/jpg");

@KidoPlay
Copy link
Contributor

If we take blogs for example I would expect all assets uploaded would live in TenantSA -> Blogs container.

You can do it simply in file system (needs few changes for azure). Just pass the path of you want,

var filePath = "bbakermmcTenantName/blogging/201909/Images/UniqueFileName.jpg";
await store.SaveAsync(imageBytes, filePath, "image/jpg");

Exactly.

Also,

I am currently doing an implementing of tencent cloud storage, but worried it might be conflict with what abp will use.
Glad to see this pr.
When would this pr be accepted?

@hitaspdotnet
Copy link
Contributor Author

hitaspdotnet commented Oct 25, 2019

@KidoPlay I think ABP haven't plan to implementing Binary objects integration at framework layer. You can do this as a module.
I have plan to rewrite Volo.Abp.Storage at framework layer (with full line documenting) and separating others as serviceable module named Volo.Abp.Capicitor.
Then users can write their own module based on Volo.Abp.Storage requirements.
If you interest you can contributions for tencent cloud provider.

@thiag0coelho
Copy link
Contributor

Any plans to accept this PR? It would be a huge improvement for the framework!

@hikalkan
Copy link
Member

hikalkan commented Jan 6, 2020

Any plans to accept this PR

We will implement an ABP Storage module, but not sure to accept this, because if we accept it requires a lot of design change.
ABP Storage module has not a high priority for the framework, but this doesn't restrict you to use any storage system you know.

@hitaspdotnet hitaspdotnet marked this pull request as draft June 4, 2020 12:34
@hikalkan hikalkan closed this Aug 5, 2020
@hikalkan hikalkan removed this from the backlog milestone Aug 7, 2020
@hikalkan hikalkan removed their request for review August 7, 2020 08:26
@hikalkan hikalkan removed their assignment Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants