Posting images from request to Azure blob

Question

I have base Controller for Attachments, here is code of it

In this controller, I pass data from the request and return URL of the posted object.

public class ApiAttachmentControllerBase<T> : PM101MobileApiController where T : Entity<int>
{
 private readonly IObjectStorageManager _storageManager;
 protected readonly IRepository<T> Repository;
 public ApiAttachmentControllerBase(IObjectStorageManager storageManager, IRepository<T> repository)
 {
 _storageManager = storageManager;
 Repository = repository;
 }
 private void CheckFileSize(IFormFile file)
 {
 if (file.Length > PM101Consts.MaxAttachmentSize)
 {
 throw new UserFriendlyException(L("Attachment_Warn_SizeLimit",
 PM101Consts.MaxAttachmentSizeMb.ToString()));
 }
 }
 private void CheckFileType(IFormFile file, params string[] supportedTypes)
 {
 if (supportedTypes.Any())
 {
 var extention = Path.GetExtension(file.FileName);
 if (!supportedTypes.ToList().Contains(extention))
 {
 throw new UserFriendlyException(L("Attachment_Warn_Type", extention));
 }
 }
 }
 private async Task<T> GetEntityAsync(int entityId)
 {
 var entity = await Repository.FirstOrDefaultAsync(entityId);
 if (entity == null)
 {
 throw new UserFriendlyException(L("EntityNotFound"));
 }
 return entity;
 }
 protected async Task<List<string>> UploadMultipleAttachmentAsync(
 int entityId,
 ICollection<IFormFile> uploadFiles,
 Func<T, string> getAttachments,
 Action<T, string> setAttachments,
 params string[] supportedTypes
 )
 {
 var entity = await GetEntityAsync(entityId);
 if (uploadFiles != null)
 {
 var files = uploadFiles.ToList();
 files.ForEach(f =>
 {
 CheckFileType(f, supportedTypes);
 CheckFileSize(f);
 });
 var attachmentUrls = new List<string>();
 foreach (var file in files)
 {
 if (file.Length > 0)
 {
 using (var ms = new MemoryStream())
 {
 file.CopyTo(ms);
 var fileName =
 AttachmentHelper.GenerateFilePath(file.FileName, entityId, entity.GetType().Name);
 var url = await _storageManager.UploadFileAsync(fileName, ms.ToArray(), file.ContentType);
 attachmentUrls.Add(url);
 }
 }
 }
 return attachmentUrls;
 }
 return null;
 }
}

I inherited it in another controller like this

public class InspectionsController : ApiAttachmentControllerBase<Inspection>
{
 public InspectionsController(IObjectStorageManager storageManager, IRepository<Inspection> repository) : base(
 storageManager, repository)
 {
 }

And then I use it in a method like this

[HttpPost]
 public async Task<IActionResult> AddPreInspection([FromForm] CreatePreInspectionDto input)
 {
 var preInspectionCount = await Repository.GetAll().Where(x => x.InspectionTypeId == 1).ToListAsync();
 if (preInspectionCount.Count > 0)
 {
 return Conflict("Pre inspection already exists");
 }
 var preInspection = new Inspection();
 ObjectMapper.Map(input, preInspection);
 var id = await Repository.InsertAndGetIdAsync(preInspection);
 var inspectionGet = await Repository.GetAll().FirstOrDefaultAsync(x => x.Id == id);
 if (input.EvidenceAttachments != null)
 {
 var evidenceAttachments = string.Join(";", await UploadMultipleAttachmentAsync(
 id,
 input.EvidenceAttachments,
 a => a.EvidenceAttachments,
 (a, value) => a.EvidenceAttachments = value,
 ".jpeg", ".png", ".jpg"
 ));
 inspectionGet.FrontPropertyAttachments = evidenceAttachments;
 }
 if (input.FrontPropertyAttachments != null)
 {
 var frontPropertyAttachments = string.Join(";", await UploadMultipleAttachmentAsync(
 id,
 input.EvidenceAttachments,
 a => a.EvidenceAttachments,
 (a, value) => a.EvidenceAttachments = value,
 ".jpeg", ".png", ".jpg"));
 inspectionGet.FrontPropertyAttachments = frontPropertyAttachments;
 }
 await Repository.UpdateAsync(inspectionGet);
 return Ok();
 }

Maybe I can have a more elegant way to post an image, get URL and update entity than do all these checks in controller action?

Answer 1

I have a few ideas...

• CheckFileSize & CheckFileType should be implemented either via action-filters or Model validation attributes
• the exceptions they throw are too generic and don't tell me which files where invalid
• Repository.GetAll().Where(x => x.InspectionTypeId == 1) getting everything and then filtering it is very inefficient. You should remove GetAll
• Conflict("Pre inspection already exists") is not helpful. I'd like to know which pre-inspection caused the error.
• var files = uploadFiles.ToList(); is unnecessary becuase files.ForEach(f => is an ugly way to work with a collection. Use a normal foreach instead. As a matter of fact you can do this with the other loop you have there. Is it by design that you don't want to uplodad valid attachements but stop processing them if anyone is invalid?
• .Where(x => x.InspectionTypeId == 1) doesn't make any sense with the hardcoded 1. Did you mean to write .Where(x => x.InspectionTypeId == input.InspectionTypeId)?
• var preInspectionCount = await Repository.GetAll().Where(x => x.InspectionTypeId == 1).ToListAsync(); you could use CountAsync here like var preInspectionCount = await Repository.CountAsync(x => x.InspectionTypeId == 1)
• what do you need getAttachments and setAttachments for? They are not used anywhere
• what if if (input.EvidenceAttachments != null) and if (input.FrontPropertyAttachments != null) are true? Both ifs assign the value to inspectionGet.FrontPropertyAttachments = so the second one will overwirte the result of the first one. Is this a bug or by design? They are also both using input.EvidenceAttachments, with the UploadMultipleAttachmentAsync so there is another bug.

It looks like you have (at least) three bugs there. Are you sure this actually works?

• Eugene Sukh: Thank's for advice. Yeah, it works
• Eugene Sukh: I find some little bugs. Thank's