Merge pull request #1306 from TransactionProcessing/codacy/medium_fixes

StuartFerguson · web-flow · commit ce1075089832 · 2025-10-08T14:30:26.000+01:00
fix some of the TODO's
diff --git a/TransactionProcessor.Aggregates/MerchantDepositListAggregate.cs b/TransactionProcessor.Aggregates/MerchantDepositListAggregate.cs
@@ -75,7 +75,6 @@ public static Result MakeDeposit(this MerchantDepositListAggregate aggregate,
             if (result.IsFailed)
                 return result;
 
-            // TODO: Change amount to a value object (PositiveAmount VO)
             result = aggregate.EnsureDepositSourceHasBeenSet(source);
             if (result.IsFailed)
                 return result;
diff --git a/TransactionProcessor.Aggregates/MerchantStatementAggregate.cs b/TransactionProcessor.Aggregates/MerchantStatementAggregate.cs
@@ -257,8 +257,6 @@ public static Result AddDailySummaryRecord(this MerchantStatementAggregate aggre
                 return Result.Success();
             }
 
-            // TODO: should this check the date has been added to the statement, before allowing the summary?
-
             MerchantStatementDomainEvents.StatementSummaryForDateEvent statementSummaryForDateEvent = new(aggregate.AggregateId, aggregate.EstateId, aggregate.MerchantId, activityDate,aggregate.MerchantStatementSummaries.Count +1
                 ,numberOfTransactions, valueOfTransactions, numberOfSettledFees, valueOfSettledFees, numberOfDepoits, valueOfDepoits, numberOfWithdrawals, valueOfWithdrawals);
             aggregate.ApplyAndAppend(statementSummaryForDateEvent);
diff --git a/TransactionProcessor.BusinessLogic/EventHandling/VoucherDomainEventHandler.cs b/TransactionProcessor.BusinessLogic/EventHandling/VoucherDomainEventHandler.cs
@@ -120,8 +120,6 @@ public async Task<Result> Handle(IDomainEvent domainEvent,
     private async Task<String> GetVoucherOperator(Models.Voucher voucherModel,
                                                   CancellationToken cancellationToken)
     {
-        // TODO: Can this be done in a better way than direct db access ?
-
         using ResolvedDbContext<EstateManagementContext>? resolvedContext = this.Resolver.Resolve(EstateManagementDatabaseName, voucherModel.EstateId.ToString());
         await using EstateManagementContext context = resolvedContext.Context;
 
diff --git a/TransactionProcessor.BusinessLogic/Manager/TransactionProcessorManager.cs b/TransactionProcessor.BusinessLogic/Manager/TransactionProcessorManager.cs
@@ -183,8 +183,6 @@ public async Task<Result<List<Merchant>>> GetMerchants(Guid estateId,
                                                                              Guid productId,
                                                                              CancellationToken cancellationToken)
         {
-            // TODO: this will need updated to handle merchant specific fees when that is available
-
             Result<ContractAggregate> getContractResult = await this.AggregateService.GetLatest<ContractAggregate>(contractId, cancellationToken);
             if (getContractResult.IsFailed)
                 return ResultHelpers.CreateFailure(getContractResult);
diff --git a/TransactionProcessor.BusinessLogic/Services/SettlementDomainService.cs b/TransactionProcessor.BusinessLogic/Services/SettlementDomainService.cs
@@ -35,8 +35,6 @@ public class SettlementDomainService : ISettlementDomainService
     {
         private readonly IAggregateService AggregateService;
         
-        // TODO: Add in a Get Settlement 
-
         public async Task<Result<Guid>> ProcessSettlement(SettlementCommands.ProcessSettlementCommand command,
                                                           CancellationToken cancellationToken) {
 
diff --git a/TransactionProcessor.BusinessLogic/Services/TransactionDomainService.cs b/TransactionProcessor.BusinessLogic/Services/TransactionDomainService.cs
@@ -274,19 +274,7 @@ public async Task<Result<ProcessSaleTransactionResponse>> ProcessSaleTransaction
                     operatorStartDateTime =DateTime.Now;
                     Result<OperatorResponse> operatorResult = await this.ProcessMessageWithOperator(merchantResult.Data, command.TransactionId, command.TransactionDateTime, command.OperatorId, command.AdditionalTransactionMetadata, transactionReference, cancellationToken);
                     operatorEndDateTime = DateTime.Now;
-                    // Act on the operator response
-                    // TODO: see if we still need this case...
-                    //if (operatorResult.IsFailed) {
-                    //    // Failed to perform sed/receive with the operator
-                    //    TransactionResponseCode transactionResponseCode =
-                    //        TransactionResponseCode.OperatorCommsError;
-                    //    String responseMessage = "OPERATOR COMMS ERROR";
-
-                    //    transactionAggregate.DeclineTransactionLocally(
-                    //        ((Int32)transactionResponseCode).ToString().PadLeft(4, '0'), responseMessage);
-                    //}
-                    //else {
-
+                    
                     if (operatorResult.IsSuccess) {
                         TransactionResponseCode transactionResponseCode = TransactionResponseCode.Success;
                         String responseMessage = "SUCCESS";
@@ -405,12 +393,12 @@ public async Task<Result> CalculateFeesForTransaction(TransactionCommands.Calcul
                 if (RequireFeeCalculation(transactionAggregate) == false)
                     return Result.Success();
 
-                List<TransactionFeeToCalculate> feesForCalculation = await this.GetTransactionFeesForCalculation(transactionAggregate, cancellationToken);
+                Result<List<TransactionFeeToCalculate>> feesForCalculationResult = await this.GetTransactionFeesForCalculation(transactionAggregate, cancellationToken);
 
-                if (feesForCalculation == null)
-                    return Result.Failure("Error getting transaction fees");
+                if (feesForCalculationResult.IsFailed)
+                    return ResultHelpers.CreateFailure(feesForCalculationResult);
 
-                List<CalculatedFee> resultFees = this.FeeCalculationManager.CalculateFees(feesForCalculation, transactionAggregate.TransactionAmount.Value, command.CompletedDateTime);
+                List<CalculatedFee> resultFees = this.FeeCalculationManager.CalculateFees(feesForCalculationResult.Data, transactionAggregate.TransactionAmount.Value, command.CompletedDateTime);
 
                 IEnumerable<CalculatedFee> nonMerchantFees = resultFees.Where(f => f.FeeType == TransactionProcessor.Models.Contract.FeeType.ServiceProvider);
                 foreach (CalculatedFee calculatedFee in nonMerchantFees) {
@@ -558,15 +546,14 @@ internal static DateTime CalculateSettlementDate(Models.Merchant.SettlementSched
             return completeDateTime.Date;
         }
 
-        private async Task<List<TransactionFeeToCalculate>> GetTransactionFeesForCalculation(TransactionAggregate transactionAggregate,
+        private async Task<Result<List<TransactionFeeToCalculate>>> GetTransactionFeesForCalculation(TransactionAggregate transactionAggregate,
                                                                                              CancellationToken cancellationToken) {
-            // TODO: convert to result??
             // Get the fees to be calculated
             Result<List<ContractProductTransactionFee>> feesForProduct = await this.GetTransactionFeesForProduct(transactionAggregate.ContractId, transactionAggregate.ProductId, cancellationToken);
         
             if (feesForProduct.IsFailed) {
                 Logger.LogWarning($"Failed to get fees {feesForProduct.Message}");
-                return null;
+                return ResultHelpers.CreateFailure(feesForProduct);
             }
 
             Logger.LogInformation($"After getting Fees {feesForProduct.Data.Count} returned");
@@ -579,7 +566,7 @@ private async Task<List<TransactionFeeToCalculate>> GetTransactionFeesForCalcula
                 feesForCalculation.Add(transactionFeeToCalculate);
             }
 
-            return feesForCalculation;
+            return Result.Success(feesForCalculation);
         }
 
         private async Task<Result> AddDeviceToMerchant(Guid merchantId,
diff --git a/TransactionProcessor.Models/Merchant/MerchantStatementLine.cs b/TransactionProcessor.Models/Merchant/MerchantStatementLine.cs
@@ -32,7 +32,6 @@ public class MerchantStatementLine
     /// </value>
     public String Description { get; set; }
 
-    // TODO: maybe make this an enum type
     /// <summary>
     /// Gets or sets the type of the line.
     /// </summary>
diff --git a/TransactionProcessor.ProjectionEngine/EventHandling/EventHandler.cs b/TransactionProcessor.ProjectionEngine/EventHandling/EventHandler.cs
@@ -12,18 +12,10 @@ namespace TransactionProcessor.ProjectionEngine.EventHandling
     public class EventHandler : IDomainEventHandler
     {
         private readonly Func<String, IDomainEventHandler> Resolver;
-
-        public static Dictionary<String, Type> StateTypes;
-
+        
         public EventHandler(Func<String, IDomainEventHandler> resolver)
         {
             this.Resolver = resolver;
-            List<Type> subclassTypes = Assembly.GetAssembly(typeof(Shared.EventStore.ProjectionEngine.State))?.GetTypes().Where(t => t.IsSubclassOf(typeof(Shared.EventStore.ProjectionEngine.State))).ToList();
-
-            if (subclassTypes != null)
-            {
-                EventHandler.StateTypes = subclassTypes.ToDictionary(x => x.Name, x => x);
-            }
         }
         
         public async Task<Result> Handle(IDomainEvent domainEvent,
diff --git a/TransactionProcessor.Repository/ITransactionProcessorReadModelRepository.cs b/TransactionProcessor.Repository/ITransactionProcessorReadModelRepository.cs
@@ -1161,7 +1161,8 @@ public async Task<Result> DisableContractProductTransactionFee(ContractDomainEve
             EstateManagementContext context = await this.GetContext(domainEvent.EstateId);
 
             Result<ContractProductTransactionFee> loadContractProductTransactionFeeResult = await context.LoadContractProductTransactionFee(domainEvent, cancellationToken);
-            // TODO: Check the result value
+            if (loadContractProductTransactionFeeResult.IsFailed)
+                return ResultHelpers.CreateFailure(loadContractProductTransactionFeeResult);
 
             ContractProductTransactionFee transactionFee = loadContractProductTransactionFeeResult.Data;
 
@@ -1174,7 +1175,6 @@ public async Task<Result> MarkMerchantFeeAsSettled(SettlementDomainEvents.Mercha
                                                            CancellationToken cancellationToken) {
             EstateManagementContext context = await this.GetContext(domainEvent.EstateId);
 
-            // TODO: LoadMerchantSettlementFee
             MerchantSettlementFee merchantFee = await context.MerchantSettlementFees.Where(m =>
                     m.MerchantId == domainEvent.MerchantId &&
                     m.TransactionId == domainEvent.TransactionId &&
diff --git a/TransactionProcessor/Bootstrapper/RepositoryRegistry.cs b/TransactionProcessor/Bootstrapper/RepositoryRegistry.cs
@@ -36,27 +36,24 @@ namespace TransactionProcessor.Bootstrapper
     [ExcludeFromCodeCoverage]
     public class RepositoryRegistry : ServiceRegistry
     {
-        #region Constructors
+        private static bool CachedAggregatesAdded;
+        private static readonly object CachedAggregatesLock = new();
 
-        private static Boolean CachedAggregatesAdded;
-        /// <summary>
-        /// Initializes a new instance of the <see cref="RepositoryRegistry"/> class.
-        /// </summary>
         public RepositoryRegistry()
         {
-            String eventStoreConnectionString = Startup.Configuration.GetValue<String>("EventStoreSettings:ConnectionString");
+            string eventStoreConnectionString = Startup.Configuration.GetValue<string>("EventStoreSettings:ConnectionString");
 
             this.AddEventStoreProjectionManagementClient(eventStoreConnectionString);
             this.AddEventStorePersistentSubscriptionsClient(eventStoreConnectionString);
-
             this.AddEventStoreClient(eventStoreConnectionString);
 
-            this.AddSingleton<Func<String, Int32, ISubscriptionRepository>>(cont => (esConnString, cacheDuration) => {
-                return SubscriptionRepository.Create(esConnString, cacheDuration);
-            });
+            this.AddSingleton<Func<string, int, ISubscriptionRepository>>(cont => (esConnString, cacheDuration) =>
+                SubscriptionRepository.Create(esConnString, cacheDuration));
 
             this.AddSingleton(typeof(IDbContextResolver<>), typeof(DbContextResolver<>));
-            if (Startup.WebHostEnvironment.IsEnvironment("IntegrationTest") || Startup.Configuration.GetValue<Boolean>("ServiceOptions:UseInMemoryDatabase") == true)
+
+            if (Startup.WebHostEnvironment.IsEnvironment("IntegrationTest") ||
+                Startup.Configuration.GetValue<bool>("ServiceOptions:UseInMemoryDatabase"))
             {
                 this.AddDbContext<EstateManagementContext>(builder => builder.UseInMemoryDatabase("TransactionProcessorReadModel"));
             }
@@ -67,28 +64,37 @@ public RepositoryRegistry()
             }
 
             this.AddTransient<IEventStoreContext, EventStoreContext>();
-            this.AddSingleton<Func<IAggregateService>>(c => () => {
-                
-                IAggregateService aggregateService = Startup.ServiceProvider.GetService<IAggregateService>();
-                if (RepositoryRegistry.CachedAggregatesAdded == false)
+
+            // ✅ Defer to the container — safe and compliant
+            this.AddSingleton<Func<IAggregateService>>(c => () =>
+            {
+                var aggregateService = c.GetService<IAggregateService>();
+
+                // Thread-safe single initialization
+                if (!CachedAggregatesAdded)
                 {
-                    aggregateService.AddCachedAggregate(typeof(EstateAggregate), null);
-                    aggregateService.AddCachedAggregate(typeof(ContractAggregate), null);
-                    aggregateService.AddCachedAggregate(typeof(OperatorAggregate), null);
-                    aggregateService.AddCachedAggregate(typeof(MerchantAggregate), null);
-                    RepositoryRegistry.CachedAggregatesAdded=true;
+                    lock (CachedAggregatesLock)
+                    {
+                        if (!CachedAggregatesAdded)
+                        {
+                            aggregateService.AddCachedAggregate(typeof(EstateAggregate), null);
+                            aggregateService.AddCachedAggregate(typeof(ContractAggregate), null);
+                            aggregateService.AddCachedAggregate(typeof(OperatorAggregate), null);
+                            aggregateService.AddCachedAggregate(typeof(MerchantAggregate), null);
+
+                            CachedAggregatesAdded = true;
+                        }
+                    }
                 }
-                
+
                 return aggregateService;
             });
+
             this.AddSingleton<IAggregateService, AggregateService>();
             this.AddSingleton<IAggregateRepositoryResolver, AggregateRepositoryResolver>();
-            this.AddSingleton<IAggregateRepository<TransactionAggregate, DomainEvent>,
-                AggregateRepository<TransactionAggregate, DomainEvent>>();
-            this.AddSingleton<IAggregateRepository<ReconciliationAggregate, DomainEvent>,
-                AggregateRepository<ReconciliationAggregate, DomainEvent>>();
-            this.AddSingleton<IAggregateRepository<SettlementAggregate, DomainEvent>,
-                AggregateRepository<SettlementAggregate, DomainEvent>>();
+            this.AddSingleton<IAggregateRepository<TransactionAggregate, DomainEvent>, AggregateRepository<TransactionAggregate, DomainEvent>>();
+            this.AddSingleton<IAggregateRepository<ReconciliationAggregate, DomainEvent>, AggregateRepository<ReconciliationAggregate, DomainEvent>>();
+            this.AddSingleton<IAggregateRepository<SettlementAggregate, DomainEvent>, AggregateRepository<SettlementAggregate, DomainEvent>>();
             this.AddSingleton<IAggregateRepository<VoucherAggregate, DomainEvent>, AggregateRepository<VoucherAggregate, DomainEvent>>();
             this.AddSingleton<IAggregateRepository<FloatAggregate, DomainEvent>, AggregateRepository<FloatAggregate, DomainEvent>>();
             this.AddSingleton<IAggregateRepository<FloatActivityAggregate, DomainEvent>, AggregateRepository<FloatActivityAggregate, DomainEvent>>();
@@ -99,17 +105,15 @@ public RepositoryRegistry()
             this.AddSingleton<IAggregateRepository<MerchantDepositListAggregate, DomainEvent>, AggregateRepository<MerchantDepositListAggregate, DomainEvent>>();
             this.AddSingleton<IAggregateRepository<MerchantStatementAggregate, DomainEvent>, AggregateRepository<MerchantStatementAggregate, DomainEvent>>();
             this.AddSingleton<IAggregateRepository<MerchantStatementForDateAggregate, DomainEvent>, AggregateRepository<MerchantStatementForDateAggregate, DomainEvent>>();
-
             this.AddSingleton<IProjectionStateRepository<MerchantBalanceState>, MerchantBalanceStateRepository>();
             this.AddSingleton<IProjectionStateRepository<VoucherState>, VoucherStateRepository>();
             this.AddSingleton<ITransactionProcessorReadRepository, TransactionProcessorReadRepository>();
             this.AddSingleton<ITransactionProcessorReadModelRepository, TransactionProcessorReadModelRepository>();
             this.AddSingleton<IProjection<MerchantBalanceState>, MerchantBalanceProjection>();
         }
-
-        #endregion
     }
 
+
     [ExcludeFromCodeCoverage]
     public class ConfigurationReaderConnectionStringRepository : IConnectionStringConfigurationRepository
     {
diff --git a/TransactionProcessor/Controllers/MerchantController.cs b/TransactionProcessor/Controllers/MerchantController.cs
diff --git a/TransactionProcessor/Extensions.cs b/TransactionProcessor/Extensions.cs

Original file line number	Diff line number	Diff line change
`@@ -257,8 +257,6 @@ public static Result AddDailySummaryRecord(this MerchantStatementAggregate aggre`
`257`	`257`	`return Result.Success();`
`258`	`258`	`}`
`259`	`259`
`260`		`- // TODO: should this check the date has been added to the statement, before allowing the summary?`
`261`		`-`
`262`	`260`	`MerchantStatementDomainEvents.StatementSummaryForDateEvent statementSummaryForDateEvent = new(aggregate.AggregateId, aggregate.EstateId, aggregate.MerchantId, activityDate,aggregate.MerchantStatementSummaries.Count +1`
`263`	`261`	`,numberOfTransactions, valueOfTransactions, numberOfSettledFees, valueOfSettledFees, numberOfDepoits, valueOfDepoits, numberOfWithdrawals, valueOfWithdrawals);`
`264`	`262`	`aggregate.ApplyAndAppend(statementSummaryForDateEvent);`
Original file line number	Diff line number	Diff line change
`@@ -120,8 +120,6 @@ public async Task<Result> Handle(IDomainEvent domainEvent,`
`120`	`120`	`private async Task<String> GetVoucherOperator(Models.Voucher voucherModel,`
`121`	`121`	`CancellationToken cancellationToken)`
`122`	`122`	`{`
`123`		`- // TODO: Can this be done in a better way than direct db access ?`
`124`		`-`
`125`	`123`	`using ResolvedDbContext<EstateManagementContext>? resolvedContext = this.Resolver.Resolve(EstateManagementDatabaseName, voucherModel.EstateId.ToString());`
`126`	`124`	`await using EstateManagementContext context = resolvedContext.Context;`
`127`	`125`
Original file line number	Diff line number	Diff line change
`@@ -183,8 +183,6 @@ public async Task<Result<List<Merchant>>> GetMerchants(Guid estateId,`
`183`	`183`	`Guid productId,`
`184`	`184`	`CancellationToken cancellationToken)`
`185`	`185`	`{`
`186`		`- // TODO: this will need updated to handle merchant specific fees when that is available`
`187`		`-`
`188`	`186`	`Result<ContractAggregate> getContractResult = await this.AggregateService.GetLatest<ContractAggregate>(contractId, cancellationToken);`
`189`	`187`	`if (getContractResult.IsFailed)`
`190`	`188`	`return ResultHelpers.CreateFailure(getContractResult);`
Original file line number	Diff line number	Diff line change
`@@ -35,8 +35,6 @@ public class SettlementDomainService : ISettlementDomainService`
`35`	`35`	`{`
`36`	`36`	`private readonly IAggregateService AggregateService;`
`37`	`37`
`38`		`- // TODO: Add in a Get Settlement`
`39`		`-`
`40`	`38`	`public async Task<Result<Guid>> ProcessSettlement(SettlementCommands.ProcessSettlementCommand command,`
`41`	`39`	`CancellationToken cancellationToken) {`
`42`	`40`
Original file line number	Diff line number	Diff line change
`@@ -12,18 +12,10 @@ namespace TransactionProcessor.ProjectionEngine.EventHandling`
`12`	`12`	`public class EventHandler : IDomainEventHandler`
`13`	`13`	`{`
`14`	`14`	`private readonly Func<String, IDomainEventHandler> Resolver;`
`15`		`-`
`16`		`- public static Dictionary<String, Type> StateTypes;`
`17`		`-`
	`15`	`+`
`18`	`16`	`public EventHandler(Func<String, IDomainEventHandler> resolver)`
`19`	`17`	`{`
`20`	`18`	`this.Resolver = resolver;`
`21`		`- List<Type> subclassTypes = Assembly.GetAssembly(typeof(Shared.EventStore.ProjectionEngine.State))?.GetTypes().Where(t => t.IsSubclassOf(typeof(Shared.EventStore.ProjectionEngine.State))).ToList();`
`22`		`-`
`23`		`- if (subclassTypes != null)`
`24`		`- {`
`25`		`- EventHandler.StateTypes = subclassTypes.ToDictionary(x => x.Name, x => x);`
`26`		`- }`
`27`	`19`	`}`
`28`	`20`
`29`	`21`	`public async Task<Result> Handle(IDomainEvent domainEvent,`