-
Notifications
You must be signed in to change notification settings - Fork 58
chore: add file manager blob storage example #105
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
base: master
Are you sure you want to change the base?
Conversation
gdenchevprog
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The overall logic looks fine to me, I just have some nitpicks about the code itself.
|
|
||
| // Handles creating new folders or copying/pasting files in Azure Blob Storage | ||
| [HttpPost] | ||
| public async Task<IActionResult> FileManager_Create([FromForm] string target, [FromForm] string name, [FromForm] int entry) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a class instead of individual parameters for each prop? E.g. [FromForm] FormRequest formRequest
It's a minor nitpick, but I think it can be a bit cleaner.
|
|
||
| // Handles file uploads to Azure Blob Storage | ||
| [HttpPost] | ||
| public async Task<IActionResult> FileManager_Upload([FromForm] string target, IFormFile file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the above comment
|
|
||
| // Handles deleting files or folders from Azure Blob Storage | ||
| [HttpPost] | ||
| public async Task<IActionResult> FileManager_Destroy([FromForm] string models) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the above comment
| try | ||
| { | ||
| // Parse the request to extract the path of the item to delete | ||
| var targetPath = FileManagerRequestParser.ResolveTargetPath(Request.Form, models); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need an entirely separate helper to just get the target path?
| { | ||
| try | ||
| { | ||
| var targetPath = Request.Form["path"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be slightly better(not sure) if we use the model-binder pattern with [FromForm] FormRequest formRequest instead of relying on Request.Form. We'll have direct access to properties that way, instead of using magic strings.
|
|
||
| namespace KendoUI.FileManager.BlobStorage.Models | ||
| { | ||
| public sealed class FileManagerCreateContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we double-check if this class is really needed or if we can simplify?
|
|
||
| namespace KendoUI.FileManager.BlobStorage.Helpers | ||
| { | ||
| public static class FileManagerRequestParser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to the previous comments, but we may not need the parser at all.
| public sealed class BlobFileManagerService : IBlobFileManagerService | ||
| { | ||
| // The Azure Blob Storage container name where all files will be stored | ||
| private const string ContainerName = "filemanager-demo"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move the ContainerName to the appsettings.json file alongside the other configs?
| Path = blobPath, | ||
| Extension = fileExtension, | ||
| Size = file.Length, | ||
| Created = now, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we assign these as default values in the model itself? That way we can shorten this to
return new FileManagerEntry
{
Name = fileNameWithoutExtension,
IsDirectory = false,
HasDirectories = false,
Path = blobPath,
Extension = fileExtension,
Size = file.Length,
}
| @@ -0,0 +1,718 @@ | |||
| using System.Text; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we double-check this service and see if we can refactor/simplify it a bit? I know this is where all the magic happens, but I feel like we should be able to cut it down a bit.
Part of: https://github.com/telerik/kendo/issues/23839