Skip to content

Commit ad7efcd

Browse files
StuartFergusonStuartFerguson
authored
Merge pull request #1020 from TransactionProcessing/codacy/high_fixes
more codacy fixes
2 parents dc35931 + 98caec8 commit ad7efcd

9 files changed

Lines changed: 51 additions & 29 deletions

File tree

‎Shared.EventStore.Tests/AggregateServiceTests.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ public async Task Get_ShouldReturnAggregateFromRepository_GetLatestThrowsExcepti
124124
public async Task Save_ShouldSaveAggregateToRepository_NoCaching() {
125125
// Arrange
126126
var aggregate = new TestAggregate { AggregateId = Guid.NewGuid() };
127+
aggregate.SetAggregateName("1", Guid.NewGuid());
127128
var repositoryMock = new Mock<IAggregateRepository<TestAggregate, DomainEvent>>();
128129

129130
repositoryMock.Setup(repo => repo.SaveChanges(aggregate, It.IsAny<CancellationToken>())).ReturnsAsync(Result.Success());
@@ -142,6 +143,7 @@ public async Task Save_ShouldSaveAggregateToRepository_NoCaching() {
142143
public async Task Save_ShouldSaveAggregateToRepository_AndCacheIt() {
143144
// Arrange
144145
var aggregate = new TestAggregate { AggregateId = Guid.NewGuid() };
146+
aggregate.SetAggregateName("1", Guid.NewGuid());
145147
var repositoryMock = new Mock<IAggregateRepository<TestAggregate, DomainEvent>>();
146148
this._aggregateService.AddCachedAggregate(typeof(TestAggregate));
147149

@@ -157,6 +159,26 @@ public async Task Save_ShouldSaveAggregateToRepository_AndCacheIt() {
157159
repositoryMock.Verify(repo => repo.SaveChanges(aggregate, It.IsAny<CancellationToken>()), Times.Once);
158160
}
159161

162+
[Fact]
163+
public async Task Save_ShouldSaveAggregateToRepository_NoChanges_SaveNotCalled()
164+
{
165+
// Arrange
166+
var aggregate = new TestAggregate { AggregateId = Guid.NewGuid() };
167+
var repositoryMock = new Mock<IAggregateRepository<TestAggregate, DomainEvent>>();
168+
this._aggregateService.AddCachedAggregate(typeof(TestAggregate));
169+
170+
repositoryMock.Setup(repo => repo.SaveChanges(aggregate, It.IsAny<CancellationToken>())).ReturnsAsync(Result.Success());
171+
172+
_repositoryResolverMock.Setup(resolver => resolver.Resolve<TestAggregate, DomainEvent>()).Returns(repositoryMock.Object);
173+
174+
// Act
175+
var result = await _aggregateService.Save(aggregate, CancellationToken.None);
176+
177+
// Assert
178+
result.IsSuccess.ShouldBeTrue();
179+
repositoryMock.Verify(repo => repo.SaveChanges(aggregate, It.IsAny<CancellationToken>()), Times.Never);
180+
}
181+
160182
[Fact]
161183
public async Task Save_ShouldSaveAggregateToRepository_AndCacheIt_SecondSave_UpdatesCache() {
162184
// Arrange
@@ -190,6 +212,7 @@ public async Task Save_ShouldSaveAggregateToRepository_AndCacheIt_SecondSave_Upd
190212
public async Task Save_ShouldSaveAggregateToRepository_SaveFails() {
191213
// Arrange
192214
var aggregate = new TestAggregate { AggregateId = Guid.NewGuid() };
215+
aggregate.SetAggregateName("1", Guid.NewGuid());
193216
var repositoryMock = new Mock<IAggregateRepository<TestAggregate, DomainEvent>>();
194217

195218
repositoryMock.Setup(repo => repo.SaveChanges(aggregate, It.IsAny<CancellationToken>())).ReturnsAsync(Result.Failure);

‎Shared.EventStore/Aggregate/AggregateMemoryCache.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,9 @@ public Boolean TryGetValueWithMetrics<TAggregate>(String key,
5959
out TAggregate aggregate) where TAggregate : Aggregate, new() {
6060
String g = typeof(TAggregate).Name;
6161

62-
var found = this.MemoryCache.TryGetValue(key, out aggregate);
62+
Boolean found = this.MemoryCache.TryGetValue(key, out aggregate);
6363

6464
if (!found) {
65-
//TODO: Failed cache hit?
6665
Counter counterCalls = AggregateService.GetCounterMetric($"AggregateService_{g}_failed_cache_hit");
6766
counterCalls.Inc();
6867
}

‎Shared.EventStore/Aggregate/AggregateService.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ public async Task<Result<TAggregate>> Get<TAggregate>(Guid aggregateId,
188188
TAggregate aggregate = null;
189189
try
190190
{
191-
var aggregateResult = await getLatestVersionFunc(repository, aggregateId, cancellationToken);
191+
Result<TAggregate> aggregateResult = await getLatestVersionFunc(repository, aggregateId, cancellationToken);
192192
if (aggregateResult.IsFailed)
193193
return aggregateResult;
194194
aggregate = aggregateResult.Data;
@@ -217,9 +217,11 @@ public async Task<Result> Save<TAggregate>(TAggregate aggregate,
217217
Histogram histogramMetric = AggregateService.GetHistogramMetric($"{m}_{g}_saved");
218218

219219
counterCalls.Inc();
220-
221-
// TODO: Check the pending events so dont save blindly, this would need a change to the base aggregate ?
222-
Result result = await repository.SaveChanges(aggregate, cancellationToken);
220+
Result result = Result.Success();
221+
// Check the pending events so dont save blindly
222+
if (aggregate.PendingEvents.Any()) {
223+
result = await repository.SaveChanges(aggregate, cancellationToken);
224+
}
223225

224226
stopwatch.Stop();
225227

‎Shared.EventStore/Extensions/IApplicationBuilderExtenstions.cs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,7 @@ public static async Task ConfigureSubscriptionService(this IApplicationBuilder a
3737

3838
ISubscriptionRepository subscriptionRepository = subscriptionRepositoryResolver(eventStoreConnectionString,
3939
workerConfig.InternalSubscriptionServiceCacheDuration);
40-
41-
// TODO: Some logging....
42-
//((SubscriptionRepository)subscriptionRepository).Trace += (sender,
43-
// s) => traceHandler(TraceEventType.Information, "REPOSITORY", s);
44-
40+
4541
// init our SubscriptionRepository
4642
await subscriptionRepository.PreWarm(cts.Token);
4743

‎Shared.EventStore/SubscriptionWorker/PersistentSubscription.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ public async Task ConnectToSubscription(CancellationToken cancellationToken)
9191
}
9292
catch (Exception e)
9393
{
94-
//TODO: Should we kill the process?
9594
Logger.Logger.LogError(e);
9695
}
9796
}

‎Shared.EventStore/SubscriptionWorker/SubscriptionWorkerHelper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public static HttpClient CreateHttpClient(EventStoreClientSettings settings)
4242
HttpClient client = new(httpClientHandler);
4343

4444
client.BaseAddress = new Uri(settings.ConnectivitySettings.Address.AbsoluteUri);
45-
client.Timeout = TimeSpan.FromSeconds(5); //TODO: configurable? - not sure it should be tbh
45+
client.Timeout = TimeSpan.FromSeconds(5);
4646

4747
if (settings.DefaultCredentials != null)
4848
{

‎Shared.Logger/Logger.cs‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public static void Initialise(ILogger loggerObject) {
3131
Logger.IsInitialised = true;
3232
}
3333

34+
private const String CorrelationIdPropertyName = "correlationId";
35+
private const String TenantIdPropertyName = "tenantId";
36+
3437
public static void LogCritical(Exception exception) {
3538
ValidateLoggerObject();
3639

@@ -40,13 +43,13 @@ public static void LogCritical(Exception exception) {
4043
LoggerObject.LogCritical(exception);
4144
return;
4245
}
43-
using (ScopeContext.PushProperty("correlationId", $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
46+
using (ScopeContext.PushProperty(CorrelationIdPropertyName, $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
4447
// Write to the normal log
4548
LoggerObject.LogCritical(exception);
4649

4750
if (tenantContext.PerTenantLogsEnabled && tenantContext.EstateId != Guid.Empty) {
4851
// Write to the tenant log
49-
using (ScopeContext.PushProperty("tenantId", $"_{tenantContext.EstateId.ToString()}")) {
52+
using (ScopeContext.PushProperty(TenantIdPropertyName, $"_{tenantContext.EstateId.ToString()}")) {
5053
LoggerObject.LogCritical(exception);
5154
}
5255
}
@@ -63,13 +66,13 @@ public static void LogDebug(String message) {
6366
return;
6467
}
6568

66-
using (ScopeContext.PushProperty("correlationId", $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
69+
using (ScopeContext.PushProperty(CorrelationIdPropertyName, $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
6770
// Write to the normal log
6871
LoggerObject.LogDebug(message);
6972

7073
if (tenantContext.PerTenantLogsEnabled && tenantContext.EstateId != Guid.Empty) {
7174
// Write to the tenant log
72-
using (ScopeContext.PushProperty("tenantId", $"_{tenantContext.EstateId.ToString()}")) {
75+
using (ScopeContext.PushProperty(TenantIdPropertyName, $"_{tenantContext.EstateId.ToString()}")) {
7376
LoggerObject.LogDebug(message);
7477
}
7578
}
@@ -86,13 +89,13 @@ public static void LogError(Exception exception) {
8689
return;
8790
}
8891

89-
using (ScopeContext.PushProperty("correlationId", $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
92+
using (ScopeContext.PushProperty(CorrelationIdPropertyName, $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
9093
// Write to the normal log
9194
LoggerObject.LogError(exception);
9295

9396
if (tenantContext.PerTenantLogsEnabled && tenantContext.EstateId != Guid.Empty) {
9497
// Write to the tenant log
95-
using (ScopeContext.PushProperty("tenantId", $"_{tenantContext.EstateId.ToString()}")) {
98+
using (ScopeContext.PushProperty(TenantIdPropertyName, $"_{tenantContext.EstateId.ToString()}")) {
9699
LoggerObject.LogError(exception);
97100
}
98101
}
@@ -108,13 +111,13 @@ public static void LogError(String message,
108111
LoggerObject.LogError(message, exception);
109112
return;
110113
}
111-
using (ScopeContext.PushProperty("correlationId", $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
114+
using (ScopeContext.PushProperty(CorrelationIdPropertyName, $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
112115
// Write to the normal log
113116
LoggerObject.LogError(message, exception);
114117

115118
if (tenantContext.PerTenantLogsEnabled && tenantContext.EstateId != Guid.Empty) {
116119
// Write to the tenant log
117-
using (ScopeContext.PushProperty("tenantId", $"_{tenantContext.EstateId.ToString()}")) {
120+
using (ScopeContext.PushProperty(TenantIdPropertyName, $"_{tenantContext.EstateId.ToString()}")) {
118121
LoggerObject.LogError(message, exception);
119122
}
120123
}
@@ -129,13 +132,13 @@ public static void LogInformation(String message) {
129132
Logger.LoggerObject.LogInformation(message);
130133
return;
131134
}
132-
using (ScopeContext.PushProperty("correlationId", $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
135+
using (ScopeContext.PushProperty(CorrelationIdPropertyName, $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
133136
// Write to the normal log
134137
Logger.LoggerObject.LogInformation(message);
135138

136139
if (tenantContext.PerTenantLogsEnabled && tenantContext.EstateId != Guid.Empty) {
137140
// Write to the tenant log
138-
using (ScopeContext.PushProperty("tenantId", $"_{tenantContext.EstateId.ToString()}")) {
141+
using (ScopeContext.PushProperty(TenantIdPropertyName, $"_{tenantContext.EstateId.ToString()}")) {
139142
Logger.LoggerObject.LogInformation(message);
140143
}
141144
}
@@ -150,13 +153,13 @@ public static void LogTrace(String message) {
150153
Logger.LoggerObject.LogTrace(message);
151154
return;
152155
}
153-
using (ScopeContext.PushProperty("correlationId", $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
156+
using (ScopeContext.PushProperty(CorrelationIdPropertyName, $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
154157
// Write to the normal log
155158
Logger.LoggerObject.LogTrace(message);
156159

157160
if (tenantContext.PerTenantLogsEnabled && tenantContext.EstateId != Guid.Empty) {
158161
// Write to the tenant log
159-
using (ScopeContext.PushProperty("tenantId", $"_{tenantContext.EstateId.ToString()}")) {
162+
using (ScopeContext.PushProperty(TenantIdPropertyName, $"_{tenantContext.EstateId.ToString()}")) {
160163
Logger.LoggerObject.LogTrace(message);
161164
}
162165
}
@@ -171,13 +174,13 @@ public static void LogWarning(String message) {
171174
Logger.LoggerObject.LogWarning(message);
172175
return;
173176
}
174-
using (ScopeContext.PushProperty("correlationId", $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
177+
using (ScopeContext.PushProperty(CorrelationIdPropertyName, $"Correlation ID: {tenantContext.CorrelationId.ToString()}")) {
175178
// Write to the normal log
176179
Logger.LoggerObject.LogWarning(message);
177180

178181
if (tenantContext.PerTenantLogsEnabled && tenantContext.EstateId != Guid.Empty) {
179182
// Write to the tenant log
180-
using (ScopeContext.PushProperty("tenantId", $"_{tenantContext.EstateId.ToString()}")) {
183+
using (ScopeContext.PushProperty(TenantIdPropertyName, $"_{tenantContext.EstateId.ToString()}")) {
181184
Logger.LoggerObject.LogWarning(message);
182185
}
183186
}

‎Shared/General/ClaimsHelper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Shared.General;
99
using Exceptions;
1010

1111
[ExcludeFromCodeCoverage]
12-
public class ClaimsHelper
12+
public static class ClaimsHelper
1313
{
1414
#region Methods
1515

‎Shared/Middleware/CorrelationIdMiddleware.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public TenantMiddleware(RequestDelegate next)
3131
this.Next = next;
3232
}
3333

34-
public const String KeyNameCorrelationId = "correlationId";
34+
public static readonly String KeyNameCorrelationId = "correlationId";
3535

3636
#endregion
3737

0 commit comments

Comments
 (0)