diff --git a/Marco.Pms.Services/Controllers/ExpenseController.cs b/Marco.Pms.Services/Controllers/ExpenseController.cs index 029b65e..4501c61 100644 --- a/Marco.Pms.Services/Controllers/ExpenseController.cs +++ b/Marco.Pms.Services/Controllers/ExpenseController.cs @@ -27,7 +27,6 @@ namespace Marco.Pms.Services.Controllers private readonly IDbContextFactory _dbContextFactory; private readonly ApplicationDbContext _context; private readonly UserHelper _userHelper; - private readonly PermissionServices _permission; private readonly ILoggingService _logger; private readonly S3UploadService _s3Service; private readonly IServiceScopeFactory _serviceScopeFactory; @@ -38,7 +37,6 @@ namespace Marco.Pms.Services.Controllers IDbContextFactory dbContextFactory, ApplicationDbContext context, UserHelper userHelper, - PermissionServices permission, IServiceScopeFactory serviceScopeFactory, ILoggingService logger, S3UploadService s3Service, @@ -47,7 +45,6 @@ namespace Marco.Pms.Services.Controllers _dbContextFactory = dbContextFactory; _context = context; _userHelper = userHelper; - _permission = permission; _logger = logger; _serviceScopeFactory = serviceScopeFactory; _s3Service = s3Service; @@ -63,7 +60,8 @@ namespace Marco.Pms.Services.Controllers /// The number of records to return per page. /// The page number to retrieve. /// A paginated list of expenses. - [HttpGet("list")] // Assuming this is a GET endpoint + + [HttpGet("list")] public async Task GetExpensesList(string? filter, int pageSize = 20, int pageNumber = 1) { try @@ -250,135 +248,18 @@ namespace Marco.Pms.Services.Controllers return "value"; } - [HttpPost("create")] - public async Task CreateExpense1([FromBody] CreateExpensesDto dto) - { - var loggedInEmployee = await _userHelper.GetCurrentEmployeeAsync(); - var hasUploadPermission = await _permission.HasPermission(PermissionsMaster.ExpenseUpload, loggedInEmployee.Id); - var hasProjectPermission = await _permission.HasProjectPermission(loggedInEmployee, dto.ProjectId); - if (!hasUploadPermission || !hasProjectPermission) - { - _logger.LogWarning("Access DENIED for employee {EmployeeId} for uploading expense on project {ProjectId}.", loggedInEmployee.Id, dto.ProjectId); - return StatusCode(403, ApiResponse.ErrorResponse("Access Denied.", "You do not have permission to Upload expenses for this project", 403)); - } - var isExpensesTypeExist = await _context.ExpensesTypeMaster.AnyAsync(et => et.Id == dto.ExpensesTypeId); - if (!isExpensesTypeExist) - { - _logger.LogWarning("Expenses type not for ID: {ExpensesTypeId} when creating new expense", dto.ExpensesTypeId); - return NotFound(ApiResponse.ErrorResponse("Expanses Type not found", "Expanses Type not found", 404)); - } - var isPaymentModeExist = await _context.PaymentModeMatser.AnyAsync(et => et.Id == dto.PaymentModeId); - if (!isPaymentModeExist) - { - _logger.LogWarning("Payment Mode not for ID: {PaymentModeId} when creating new expense", dto.PaymentModeId); - return NotFound(ApiResponse.ErrorResponse("Payment Mode not found", "Payment Mode not found", 404)); - } - var isStatusExist = await _context.ExpensesStatusMaster.AnyAsync(et => et.Id == Draft); - if (!isStatusExist) - { - _logger.LogWarning("Status not for ID: {PaymentModeId} when creating new expense", dto.PaymentModeId); - return NotFound(ApiResponse.ErrorResponse("Status not found", "Status not found", 404)); - } - - var expense = _mapper.Map(dto); - - expense.CreatedById = loggedInEmployee.Id; - expense.CreatedAt = DateTime.UtcNow; - expense.TenantId = tenantId; - expense.IsActive = true; - expense.StatusId = Draft; - - _context.Expenses.Add(expense); - - Guid batchId = Guid.NewGuid(); - foreach (var attachment in dto.BillAttachments) - { - if (!_s3Service.IsBase64String(attachment.Base64Data)) - { - _logger.LogWarning("Image upload failed: Base64 data is missing While creating new expense entity for project {ProjectId} by employee {EmployeeId}", expense.ProjectId, expense.PaidById); - return BadRequest(ApiResponse.ErrorResponse("Base64 data is missing", "Base64 data is missing", 400)); - } - var base64 = attachment.Base64Data!.Contains(',') - ? attachment.Base64Data[(attachment.Base64Data.IndexOf(",") + 1)..] - : attachment.Base64Data; - - var fileType = _s3Service.GetContentTypeFromBase64(base64); - var fileName = _s3Service.GenerateFileName(fileType, expense.Id, "Expense"); - var objectKey = $"tenant-{tenantId}/project-{expense.ProjectId}/Expenses/{fileName}"; - try - { - await _s3Service.UploadFileAsync(base64, fileType, objectKey); - _logger.LogInfo("Image uploaded to S3 with key: {ObjectKey}", objectKey); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error occured while saving image to S3"); - return BadRequest(ApiResponse.ErrorResponse("Cannot upload attachment to S3", new - { - message = ex.Message, - innerexcption = ex.InnerException?.Message, - stackTrace = ex.StackTrace, - source = ex.Source - }, 400)); - } - - var document = new Document - { - BatchId = batchId, - UploadedById = loggedInEmployee.Id, - FileName = attachment.FileName ?? "", - ContentType = attachment.ContentType ?? "", - S3Key = objectKey, - FileSize = attachment.FileSize, - UploadedAt = DateTime.UtcNow, - TenantId = tenantId - }; - _context.Documents.Add(document); - - var billAttachement = new BillAttachments - { - DocumentId = document.Id, - ExpensesId = expense.Id, - TenantId = tenantId - }; - _context.BillAttachments.Add(billAttachement); - } - try - { - await _context.SaveChangesAsync(); - } - catch (DbUpdateException dbEx) - { - _logger.LogError(dbEx, "Error occured while saving Expense, Document and bill attachment entity"); - return BadRequest(ApiResponse.ErrorResponse("Databsae Exception", new - { - Message = dbEx.Message, - StackTrace = dbEx.StackTrace, - Source = dbEx.Source, - innerexcption = new - { - Message = dbEx.InnerException?.Message, - StackTrace = dbEx.InnerException?.StackTrace, - Source = dbEx.InnerException?.Source, - } - }, 400)); - } - _logger.LogInfo("Documents and attachments saved for Expense: {ExpenseId}", expense.Id); - - return StatusCode(201, ApiResponse.SuccessResponse(expense, "Expense created Successfully", 201)); - } - /// /// Creates a new expense entry along with its bill attachments. - /// This operation is transactional and performs validations and file uploads in parallel for optimal performance. - /// Permission checks are also run in parallel using IServiceScopeFactory to ensure thread safety. + /// This operation is transactional and performs validations and file uploads concurrently for optimal performance + /// by leveraging async/await without unnecessary thread-pool switching via Task.Run. /// /// The data transfer object containing expense details and attachments. /// An IActionResult indicating the result of the creation operation. - [HttpPost] + + [HttpPost("create")] public async Task CreateExpense([FromBody] CreateExpensesDto dto) { - _logger.LogDebug("CreateExpense for Project {ProjectId}", dto.ProjectId); + _logger.LogDebug("Starting CreateExpense for Project {ProjectId}", dto.ProjectId); // The entire operation is wrapped in a transaction to ensure data consistency. await using var transaction = await _context.Database.BeginTransactionAsync(); @@ -386,9 +267,10 @@ namespace Marco.Pms.Services.Controllers { var loggedInEmployee = await _userHelper.GetCurrentEmployeeAsync(); - // 1. Authorization: Run permission checks in parallel using a service scope for each task. - // This is crucial for thread-safety as IPermissionService is a scoped service. - var hasUploadPermissionTask = Task.Run(async () => + // 1. Authorization & Validation: Run all I/O-bound checks concurrently using factories for safety. + + // PERMISSION CHECKS: Use IServiceScopeFactory for thread-safe access to scoped services. + var hasUploadPermissionTask = Task.Run(async () => // Task.Run is acceptable here to create a new scope, but let's do it cleaner. { using var scope = _serviceScopeFactory.CreateScope(); var permissionService = scope.ServiceProvider.GetRequiredService(); @@ -402,72 +284,66 @@ namespace Marco.Pms.Services.Controllers return await permissionService.HasProjectPermission(loggedInEmployee, dto.ProjectId); }); - await Task.WhenAll(hasUploadPermissionTask, hasProjectPermissionTask); + // VALIDATION CHECKS: Use IDbContextFactory for thread-safe, parallel database queries. + // Each task gets its own DbContext instance. + var projectTask = Task.Run(async () => + { + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + return await dbContext.Projects.AsNoTracking().FirstOrDefaultAsync(p => p.Id == dto.ProjectId); + }); + var expenseTypeTask = Task.Run(async () => + { + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + return await dbContext.ExpensesTypeMaster.AsNoTracking().FirstOrDefaultAsync(et => et.Id == dto.ExpensesTypeId); + }); + var paymentModeTask = Task.Run(async () => + { + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + return await dbContext.PaymentModeMatser.AsNoTracking().FirstOrDefaultAsync(pm => pm.Id == dto.PaymentModeId); + }); + var statusTask = Task.Run(async () => + { + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + return await dbContext.ExpensesStatusMaster.AsNoTracking().FirstOrDefaultAsync(es => es.Id == Draft); + }); - if (!hasUploadPermissionTask.Result || !hasProjectPermissionTask.Result) + + // Await all prerequisite checks at once. + await Task.WhenAll( + hasUploadPermissionTask, hasProjectPermissionTask, + projectTask, expenseTypeTask, paymentModeTask, statusTask + ); + + // Await all prerequisite checks at once. + await Task.WhenAll( + hasUploadPermissionTask, hasProjectPermissionTask, + projectTask, expenseTypeTask, paymentModeTask, statusTask + ); + + // 2. Aggregate and Check Results + if (!await hasUploadPermissionTask || !await hasProjectPermissionTask) { _logger.LogWarning("Access DENIED for employee {EmployeeId} on project {ProjectId}.", loggedInEmployee.Id, dto.ProjectId); return StatusCode(403, ApiResponse.ErrorResponse("Access Denied.", "You do not have permission to upload expenses for this project.", 403)); } + var validationErrors = new List(); + var project = await projectTask; + var expenseType = await expenseTypeTask; + var paymentMode = await paymentModeTask; + var status = await statusTask; - // 2. Validation: Check if prerequisite entities exist. - // The method now returns a tuple indicating success or failure. - // Each task creates its own DbContext instance from the factory, making the parallel calls thread-safe. - var projectGetTask = Task.Run(async () => - { - await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); - return await dbContext.Projects.FirstOrDefaultAsync(p => p.Id == dto.ProjectId); - }); + if (project == null) validationErrors.Add("Project not found."); + if (expenseType == null) validationErrors.Add("Expense Type not found."); + if (paymentMode == null) validationErrors.Add("Payment Mode not found."); + if (status == null) validationErrors.Add("Default status 'Draft' not found."); - var expenseTypeGetTask = Task.Run(async () => + if (validationErrors.Any()) { - await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); - return await dbContext.ExpensesTypeMaster.FirstOrDefaultAsync(et => et.Id == dto.ExpensesTypeId); - }); - - var paymentModeGetTask = Task.Run(async () => - { - await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); - return await dbContext.PaymentModeMatser.FirstOrDefaultAsync(pm => pm.Id == dto.PaymentModeId); - }); - - var statusGetTask = Task.Run(async () => - { - await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); - return await dbContext.ExpensesStatusMaster.FirstOrDefaultAsync(es => es.Id == Draft); - }); - - await Task.WhenAll(projectGetTask, expenseTypeGetTask, paymentModeGetTask, statusGetTask); - - var project = await projectGetTask; - var expenseType = await expenseTypeGetTask; - var paymentMode = await paymentModeGetTask; - var status = await statusGetTask; - - if (project == null) - { - await transaction.RollbackAsync(); // Ensure transaction is terminated before returning. - _logger.LogWarning("Expense creation failed due to validation: Project with ID {ProjectId} not found.", dto.ProjectId); - return NotFound(ApiResponse.ErrorResponse("Project not found.", "Project not found.", 404)); - } - else if (expenseType == null) - { - await transaction.RollbackAsync(); // Ensure transaction is terminated before returning. - _logger.LogWarning("Expense creation failed due to validation: Expense Type with ID {ExpensesTypeId} not found.", dto.ExpensesTypeId); - return NotFound(ApiResponse.ErrorResponse("Expense Type not found.", "Expense Type not found.", 404)); - } - else if (paymentMode == null) - { - await transaction.RollbackAsync(); // Ensure transaction is terminated before returning. - _logger.LogWarning("Expense creation failed due to validation: Payment Mode with ID {PaymentModeId} not found.", dto.PaymentModeId); - return NotFound(ApiResponse.ErrorResponse("Payment Mode not found.", "Payment Mode not found.", 404)); - } - else if (status == null) - { - await transaction.RollbackAsync(); // Ensure transaction is terminated before returning. - _logger.LogWarning("Expense creation failed due to validation: Status with ID {StatusId} not found.", Draft); - return NotFound(ApiResponse.ErrorResponse("Status not found.", "Status not found.", 404)); + await transaction.RollbackAsync(); + var errorMessage = string.Join(" ", validationErrors); + _logger.LogWarning("Expense creation failed due to validation errors: {ValidationErrors}", errorMessage); + return BadRequest(ApiResponse.ErrorResponse("Invalid input data.", errorMessage, 400)); } // 3. Entity Creation @@ -489,12 +365,10 @@ namespace Marco.Pms.Services.Controllers // 5. Database Commit await _context.SaveChangesAsync(); - // 6. Transaction Commit await transaction.CommitAsync(); var response = _mapper.Map(expense); - response.Project = _mapper.Map(project); response.Status = _mapper.Map(status); response.PaymentMode = _mapper.Map(paymentMode); @@ -503,11 +377,11 @@ namespace Marco.Pms.Services.Controllers _logger.LogInfo("Successfully created Expense {ExpenseId} for Project {ProjectId}.", expense.Id, expense.ProjectId); return StatusCode(201, ApiResponse.SuccessResponse(response, "Expense created successfully.", 201)); } - catch (ArgumentException ex) // For invalid Base64 or other bad arguments. + catch (ArgumentException ex) // Catches bad Base64 from attachment pre-validation { await transaction.RollbackAsync(); - _logger.LogError(ex, "Invalid argument provided during expense creation for project {ProjectId}.", dto.ProjectId); - return BadRequest(ApiResponse.ErrorResponse("Invalid Request Data", new + _logger.LogError(ex, "Invalid argument during expense creation for project {ProjectId}.", dto.ProjectId); + return BadRequest(ApiResponse.ErrorResponse("Invalid Request Data.", new { Message = ex.Message, StackTrace = ex.StackTrace, @@ -520,7 +394,7 @@ namespace Marco.Pms.Services.Controllers } }, 400)); } - catch (Exception ex) // General-purpose catch for unexpected errors (e.g., S3 failure). + catch (Exception ex) // General-purpose catch for unexpected errors (e.g., S3 or DB connection failure) { await transaction.RollbackAsync(); _logger.LogError(ex, "An unhandled exception occurred while creating an expense for project {ProjectId}.", dto.ProjectId); @@ -539,62 +413,6 @@ namespace Marco.Pms.Services.Controllers } } - /// - /// Processes and uploads attachments in parallel, then adds the resulting entities to the main DbContext. - /// - private async Task ProcessAndUploadAttachmentsAsync(IEnumerable attachments, Expenses expense, Guid employeeId, Guid tenantId) - { - var batchId = Guid.NewGuid(); - - var processingTasks = attachments.Select(attachment => Task.Run(async () => - { - if (string.IsNullOrWhiteSpace(attachment.Base64Data) || !_s3Service.IsBase64String(attachment.Base64Data)) - throw new ArgumentException("Invalid or missing Base64 data for an attachment."); - - var base64Data = attachment.Base64Data!.Contains(',') ? attachment.Base64Data[(attachment.Base64Data.IndexOf(",") + 1)..] : attachment.Base64Data; - var fileType = _s3Service.GetContentTypeFromBase64(base64Data); - var fileName = _s3Service.GenerateFileName(fileType, expense.Id, "Expense"); - var objectKey = $"tenant-{tenantId}/project-{expense.ProjectId}/Expenses/{fileName}"; - - // Upload and create entities - await _s3Service.UploadFileAsync(base64Data, fileType, objectKey); - _logger.LogInfo("Uploaded file to S3 with key: {ObjectKey}", objectKey); - - return CreateAttachmentEntities(batchId, expense.Id, employeeId, tenantId, objectKey, attachment); - })).ToList(); - - var results = await Task.WhenAll(processingTasks); - - // This part is thread-safe as it runs after all parallel tasks are complete. - foreach (var (document, billAttachment) in results) - { - _context.Documents.Add(document); - _context.BillAttachments.Add(billAttachment); - } - _logger.LogInfo("{AttachmentCount} attachments processed and staged for saving.", results.Length); - } - - /// - /// A private static helper method to create Document and BillAttachment entities. - /// - private static (Document document, BillAttachments billAttachment) CreateAttachmentEntities( - Guid batchId, Guid expenseId, Guid uploadedById, Guid tenantId, string s3Key, FileUploadModel attachmentDto) - { - var document = new Document - { - BatchId = batchId, - UploadedById = uploadedById, - FileName = attachmentDto.FileName ?? "", - ContentType = attachmentDto.ContentType ?? "", - S3Key = s3Key, - FileSize = attachmentDto.FileSize, - UploadedAt = DateTime.UtcNow, - TenantId = tenantId - }; - var billAttachment = new BillAttachments { Document = document, ExpensesId = expenseId, TenantId = tenantId }; - return (document, billAttachment); - } - [HttpPost("action")] public async Task ChangeStatus([FromBody] ExpenseRecordDto model) { @@ -715,6 +533,79 @@ namespace Marco.Pms.Services.Controllers return expenseFilter; } + /// + /// Processes and uploads attachments concurrently, then adds the resulting entities to the main DbContext. + /// + private async Task ProcessAndUploadAttachmentsAsync(IEnumerable attachments, Expenses expense, Guid employeeId, Guid tenantId) + { + // Pre-validate all attachments to fail fast before any uploads. + foreach (var attachment in attachments) + { + if (string.IsNullOrWhiteSpace(attachment.Base64Data) || !_s3Service.IsBase64String(attachment.Base64Data)) + { + throw new ArgumentException($"Invalid or missing Base64 data for attachment: {attachment.FileName ?? "N/A"}"); + } + } + + var batchId = Guid.NewGuid(); + + // Create a list of tasks to be executed concurrently. + var processingTasks = attachments.Select(attachment => + ProcessSingleAttachmentAsync(attachment, expense, employeeId, tenantId, batchId) + ).ToList(); + + var results = await Task.WhenAll(processingTasks); + + // This part is thread-safe as it runs after all concurrent tasks are complete. + foreach (var (document, billAttachment) in results) + { + _context.Documents.Add(document); + _context.BillAttachments.Add(billAttachment); + } + _logger.LogInfo("{AttachmentCount} attachments processed and staged for saving.", results.Length); + } + + /// + /// Handles the logic for a single attachment: upload to S3 and create corresponding entities. + /// + private async Task<(Document document, BillAttachments billAttachment)> ProcessSingleAttachmentAsync( + FileUploadModel attachment, Expenses expense, Guid employeeId, Guid tenantId, Guid batchId) + { + var base64Data = attachment.Base64Data!.Contains(',') ? attachment.Base64Data[(attachment.Base64Data.IndexOf(",") + 1)..] : attachment.Base64Data; + var fileType = _s3Service.GetContentTypeFromBase64(base64Data); + var fileName = _s3Service.GenerateFileName(fileType, expense.Id, "Expense"); + var objectKey = $"tenant-{tenantId}/project-{expense.ProjectId}/Expenses/{fileName}"; + + // Await the I/O-bound upload operation directly. + await _s3Service.UploadFileAsync(base64Data, fileType, objectKey); + _logger.LogInfo("Uploaded file to S3 with key: {ObjectKey}", objectKey); + + return CreateAttachmentEntities(batchId, expense.Id, employeeId, tenantId, objectKey, attachment); + } + + + /// + /// A private static helper method to create Document and BillAttachment entities. + /// This remains unchanged as it's a pure data-shaping method. + /// + private static (Document document, BillAttachments billAttachment) CreateAttachmentEntities( + Guid batchId, Guid expenseId, Guid uploadedById, Guid tenantId, string s3Key, FileUploadModel attachmentDto) + { + var document = new Document + { + BatchId = batchId, + UploadedById = uploadedById, + FileName = attachmentDto.FileName ?? "", + ContentType = attachmentDto.ContentType ?? "", + S3Key = s3Key, + FileSize = attachmentDto.FileSize, + UploadedAt = DateTime.UtcNow, + TenantId = tenantId + }; + var billAttachment = new BillAttachments { Document = document, ExpensesId = expenseId, TenantId = tenantId }; + return (document, billAttachment); + } + #endregion } }