From b06720db1034087fb15977f3db257ce3c13fc713 Mon Sep 17 00:00:00 2001 From: Farshad DASHTI Date: Thu, 8 Aug 2024 16:32:09 +0100 Subject: [PATCH 1/2] General refactoring --- .../Repositories/Repository.cs | 43 ++++++------- PersonsApi/Extensions/PredicateBuilder.cs | 31 ---------- PersonsApi/Extensions/StringExtensions.cs | 10 ---- .../Middleware/ExceptionHandlerMiddleware.cs | 7 +-- PersonsApi/Middleware/UrlDecoderMiddleware.cs | 6 +- PersonsApi/Program.cs | 14 +---- PersonsApi/ResponseModels/ApiResponseV2.cs | 21 ------- .../ResponseModels/ApiSingleResponseV2.cs | 10 ---- PersonsApi/ResponseModels/PagingResponse.cs | 9 --- .../ResponseModels/PagingResponseFactory.cs | 60 ------------------- PersonsApi/Services/ApiClient.cs | 54 ----------------- PersonsApi/Services/DateTimeService.cs | 27 --------- PersonsApi/Startup.cs | 23 +++---- .../AuthenticationHeaderOperationFilter.cs | 1 - PersonsApi/Swagger/SwaggerOptions.cs | 1 - PersonsApi/UseCases/ApiKeyService.cs | 3 - 16 files changed, 34 insertions(+), 286 deletions(-) delete mode 100644 PersonsApi/Extensions/PredicateBuilder.cs delete mode 100644 PersonsApi/Extensions/StringExtensions.cs delete mode 100644 PersonsApi/ResponseModels/ApiResponseV2.cs delete mode 100644 PersonsApi/ResponseModels/ApiSingleResponseV2.cs delete mode 100644 PersonsApi/ResponseModels/PagingResponse.cs delete mode 100644 PersonsApi/ResponseModels/PagingResponseFactory.cs delete mode 100644 PersonsApi/Services/ApiClient.cs delete mode 100644 PersonsApi/Services/DateTimeService.cs diff --git a/Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs b/Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs index 58ff568d4..b967c6972 100644 --- a/Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs +++ b/Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs @@ -1,13 +1,13 @@ using Dfe.Academies.Domain.Repositories; using Microsoft.EntityFrameworkCore; -using Microsoft.EntityFrameworkCore.ChangeTracking; using System.Linq.Expressions; namespace Dfe.Academies.Infrastructure.Repositories { +#pragma warning disable CS8603 // Possible null reference return, behaviour expected public abstract class Repository : IRepository - where TEntity : class, new() - where TDbContext : DbContext + where TEntity : class, new() + where TDbContext : DbContext { /// /// The @@ -46,15 +46,15 @@ public virtual async Task> FetchAsync( public virtual TEntity Find(params object[] keyValues) => this.DbSet().Find(keyValues); /// - public virtual async Task FindAsync(params object[] keyValues) + public virtual TEntity Find(Expression> predicate) { - return await this.DbSet().FindAsync(keyValues); + return ((IQueryable)this.DbSet()).FirstOrDefault(predicate); } /// - public virtual TEntity Find(Expression> predicate) + public virtual async Task FindAsync(params object[] keyValues) { - return ((IQueryable)this.DbSet()).FirstOrDefault(predicate); + return await this.DbSet().FindAsync(keyValues); } /// @@ -66,27 +66,27 @@ public virtual async Task FindAsync( } /// - public virtual TEntity Get(params object[] keyValues) + public virtual TEntity Get(Expression> predicate) { - return this.Find(keyValues) ?? throw new InvalidOperationException(string.Format("Entity type {0} is null for primary key {1}", (object)typeof(TEntity), (object)keyValues)); + return ((IQueryable)this.DbSet()).Single(predicate); } /// - public virtual async Task GetAsync(params object[] keyValues) + public virtual TEntity Get(params object[] keyValues) { - return await this.FindAsync(keyValues) ?? throw new InvalidOperationException(string.Format("Entity type {0} is null for primary key {1}", (object)typeof(TEntity), (object)keyValues)); + return this.Find(keyValues) ?? throw new InvalidOperationException(string.Format("Entity type {0} is null for primary key {1}", (object)typeof(TEntity), (object)keyValues)); } /// - public virtual TEntity Get(Expression> predicate) + public virtual async Task GetAsync(Expression> predicate) { - return ((IQueryable)this.DbSet()).Single(predicate); + return await EntityFrameworkQueryableExtensions.SingleAsync((IQueryable)this.DbSet(), predicate, new CancellationToken()); } /// - public virtual async Task GetAsync(Expression> predicate) + public virtual async Task GetAsync(params object[] keyValues) { - return await EntityFrameworkQueryableExtensions.SingleAsync((IQueryable)this.DbSet(), predicate, new CancellationToken()); + return await this.FindAsync(keyValues) ?? throw new InvalidOperationException(string.Format("Entity type {0} is null for primary key {1}", (object)typeof(TEntity), (object)keyValues)); } /// @@ -100,8 +100,8 @@ public virtual TEntity Add(TEntity entity) /// public virtual async Task AddAsync(TEntity entity, CancellationToken cancellationToken = default(CancellationToken)) { - EntityEntry entityEntry = await this.DbContext.AddAsync(entity, cancellationToken); - int num = await this.DbContext.SaveChangesAsync(cancellationToken); + await this.DbContext.AddAsync(entity, cancellationToken); + await this.DbContext.SaveChangesAsync(cancellationToken); return entity; } @@ -119,7 +119,7 @@ public virtual async Task> AddRangeAsync( CancellationToken cancellationToken = default(CancellationToken)) { await this.DbContext.AddRangeAsync((IEnumerable)entities, cancellationToken); - int num = await this.DbContext.SaveChangesAsync(cancellationToken); + await this.DbContext.SaveChangesAsync(cancellationToken); return (IEnumerable)entities; } @@ -137,7 +137,7 @@ public virtual async Task RemoveAsync( CancellationToken cancellationToken = default(CancellationToken)) { this.DbContext.Remove(entity); - int num = await this.DbContext.SaveChangesAsync(cancellationToken); + await this.DbContext.SaveChangesAsync(cancellationToken); return entity; } @@ -161,7 +161,7 @@ public virtual async Task> RemoveRangeAsync( CancellationToken cancellationToken = default(CancellationToken)) { this.DbSet().RemoveRange((IEnumerable)entities); - int num = await this.DbContext.SaveChangesAsync(cancellationToken); + await this.DbContext.SaveChangesAsync(cancellationToken); return (IEnumerable)entities; } @@ -179,8 +179,9 @@ public virtual async Task UpdateAsync( CancellationToken cancellationToken = default(CancellationToken)) { this.DbContext.Update(entity); - int num = await this.DbContext.SaveChangesAsync(cancellationToken); + await this.DbContext.SaveChangesAsync(cancellationToken); return entity; } } + #pragma warning restore CS8603 // Possible null reference return } diff --git a/PersonsApi/Extensions/PredicateBuilder.cs b/PersonsApi/Extensions/PredicateBuilder.cs deleted file mode 100644 index ce9440643..000000000 --- a/PersonsApi/Extensions/PredicateBuilder.cs +++ /dev/null @@ -1,31 +0,0 @@ -using System; -using System.Linq; -using System.Linq.Expressions; - -namespace PersonsApi.Extensions -{ - /// - /// http://www.albahari.com/nutshell/predicatebuilder.aspx - /// - public static class PredicateBuilder - { - public static Expression> True() { return f => true; } - public static Expression> False() { return f => false; } - - public static Expression> Or(this Expression> expr1, - Expression> expr2) - { - var invokedExpr = Expression.Invoke(expr2, expr1.Parameters.Cast()); - return Expression.Lambda> - (Expression.OrElse(expr1.Body, invokedExpr), expr1.Parameters); - } - - public static Expression> And(this Expression> expr1, - Expression> expr2) - { - var invokedExpr = Expression.Invoke(expr2, expr1.Parameters.Cast()); - return Expression.Lambda> - (Expression.AndAlso(expr1.Body, invokedExpr), expr1.Parameters); - } - } -} diff --git a/PersonsApi/Extensions/StringExtensions.cs b/PersonsApi/Extensions/StringExtensions.cs deleted file mode 100644 index cf4b73c5d..000000000 --- a/PersonsApi/Extensions/StringExtensions.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace PersonsApi.Extensions -{ - public static class StringExtensions - { - public static int ToInt(this string value) - { - return int.TryParse(value, out var result) ? result : 0; - } - } -} diff --git a/PersonsApi/Middleware/ExceptionHandlerMiddleware.cs b/PersonsApi/Middleware/ExceptionHandlerMiddleware.cs index b6f10cd93..8cb07293b 100644 --- a/PersonsApi/Middleware/ExceptionHandlerMiddleware.cs +++ b/PersonsApi/Middleware/ExceptionHandlerMiddleware.cs @@ -1,10 +1,5 @@ -using System; -using System.Net; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using PersonsApi.ResponseModels; +using System.Net; public class ExceptionHandlerMiddleware { diff --git a/PersonsApi/Middleware/UrlDecoderMiddleware.cs b/PersonsApi/Middleware/UrlDecoderMiddleware.cs index 387f4f5b7..d0bbe502d 100644 --- a/PersonsApi/Middleware/UrlDecoderMiddleware.cs +++ b/PersonsApi/Middleware/UrlDecoderMiddleware.cs @@ -1,10 +1,6 @@ -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; -using System.Web; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.WebUtilities; +using System.Web; namespace PersonsApi.Middleware { diff --git a/PersonsApi/Program.cs b/PersonsApi/Program.cs index 1a58e1905..aa4b8bfee 100644 --- a/PersonsApi/Program.cs +++ b/PersonsApi/Program.cs @@ -1,11 +1,7 @@ -using Microsoft.ApplicationInsights.Extensibility; -using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Mvc.ApiExplorer; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Serilog; using PersonsApi; using PersonsApi.SerilogCustomEnrichers; +using Serilog; var builder = WebApplication.CreateBuilder(args); @@ -16,14 +12,6 @@ builder.Host.UseSerilog((context, services, loggerConfiguration) => { var enricher = services.GetRequiredService(); - - // TODO EA - //loggerConfiguration - // .ReadFrom.Configuration(context.Configuration) - // .WriteTo.ApplicationInsights(services.GetRequiredService(), TelemetryConverter.Traces) - // .Enrich.FromLogContext() - // .Enrich.With(enricher) - // .WriteTo.Console(); }); builder.Services.AddApplicationDependencyGroup(builder.Configuration); diff --git a/PersonsApi/ResponseModels/ApiResponseV2.cs b/PersonsApi/ResponseModels/ApiResponseV2.cs deleted file mode 100644 index bc095b0bf..000000000 --- a/PersonsApi/ResponseModels/ApiResponseV2.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System.Collections.Generic; - -namespace PersonsApi.ResponseModels -{ - public class ApiResponseV2 where TResponse : class - { - public IEnumerable Data { get; set; } - public PagingResponse Paging { get; set; } - - public ApiResponseV2() => Data = new List(); - - public ApiResponseV2(IEnumerable data, PagingResponse pagingResponse) - { - Data = data; - Paging = pagingResponse; - } - - public ApiResponseV2(TResponse data) => Data = new List{ data }; - - } -} \ No newline at end of file diff --git a/PersonsApi/ResponseModels/ApiSingleResponseV2.cs b/PersonsApi/ResponseModels/ApiSingleResponseV2.cs deleted file mode 100644 index 34addada2..000000000 --- a/PersonsApi/ResponseModels/ApiSingleResponseV2.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace PersonsApi.ResponseModels -{ - public class ApiSingleResponseV2 where TResponse : class - { - public TResponse Data { get; set; } - - public ApiSingleResponseV2() => Data = null; - public ApiSingleResponseV2(TResponse data) => Data = data ; - } -} \ No newline at end of file diff --git a/PersonsApi/ResponseModels/PagingResponse.cs b/PersonsApi/ResponseModels/PagingResponse.cs deleted file mode 100644 index 8a1d2ab63..000000000 --- a/PersonsApi/ResponseModels/PagingResponse.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace PersonsApi.ResponseModels -{ - public class PagingResponse - { - public int Page { get; set; } - public int RecordCount { get; set; } - public string NextPageUrl { get; set; } - } -} \ No newline at end of file diff --git a/PersonsApi/ResponseModels/PagingResponseFactory.cs b/PersonsApi/ResponseModels/PagingResponseFactory.cs deleted file mode 100644 index b65572bd2..000000000 --- a/PersonsApi/ResponseModels/PagingResponseFactory.cs +++ /dev/null @@ -1,60 +0,0 @@ -using System.Collections.Generic; -using System.Linq; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Extensions; - -namespace PersonsApi.ResponseModels -{ - public static class PagingResponseFactory - { - public static PagingResponse Create(int page, int count, int recordCount, HttpRequest request) - { - var pagingResponse = new PagingResponse - { - RecordCount = recordCount, - Page = page - }; - - if ((count * page) >= recordCount) return pagingResponse; - - var queryAttributes = request.Query - .Where(q => q.Key != nameof(page) && q.Key != nameof(count)) - .Select(q => new KeyValuePair(q.Key, q.Value)); - - var queryBuilder = new QueryBuilder(queryAttributes) - { - {nameof(page), $"{page + 1}"}, - {nameof(count), $"{count}"} - }; - - pagingResponse.NextPageUrl = $"{request.Path}{queryBuilder}"; - - return pagingResponse; - } - - public static Dfe.Academies.Contracts.V4.PagingResponse CreateV4PagingResponse(int page, int count, int recordCount, HttpRequest request) - { - var pagingResponse = new Dfe.Academies.Contracts.V4.PagingResponse - { - RecordCount = recordCount, - Page = page - }; - - if ((count * page) >= recordCount) return pagingResponse; - - var queryAttributes = request.Query - .Where(q => q.Key != nameof(page) && q.Key != nameof(count)) - .Select(q => new KeyValuePair(q.Key, q.Value)); - - var queryBuilder = new QueryBuilder(queryAttributes) - { - {nameof(page), $"{page + 1}"}, - {nameof(count), $"{count}"} - }; - - pagingResponse.NextPageUrl = $"{request.Path}{queryBuilder}"; - - return pagingResponse; - } - } -} \ No newline at end of file diff --git a/PersonsApi/Services/ApiClient.cs b/PersonsApi/Services/ApiClient.cs deleted file mode 100644 index acdf791a6..000000000 --- a/PersonsApi/Services/ApiClient.cs +++ /dev/null @@ -1,54 +0,0 @@ -using Microsoft.Extensions.Logging; -using Newtonsoft.Json; -using System; -using System.Net.Http; -using System.Threading.Tasks; - -namespace PersonsApi.Services -{ - public abstract class ApiClient - { - private readonly IHttpClientFactory _clientFactory; - private readonly ILogger _logger; - private string _httpClientName; - - protected ApiClient(IHttpClientFactory clientFactory, ILogger logger, string httpClientName) - { - _clientFactory = clientFactory; - _logger = logger; - _httpClientName = httpClientName; - } - - public async Task Get(string endpoint) where T : class - { - try - { - var request = new HttpRequestMessage(HttpMethod.Get, endpoint); - - var client = CreateHttpClient(); - - var response = await client.SendAsync(request); - - response.EnsureSuccessStatusCode(); - - var content = await response.Content.ReadAsStringAsync(); - - var result = JsonConvert.DeserializeObject(content); - - return result; - } - catch (Exception ex) - { - _logger.LogError(ex, ex.Message); - throw; - } - } - - private HttpClient CreateHttpClient() - { - var client = _clientFactory.CreateClient(_httpClientName); - - return client; - } - } -} diff --git a/PersonsApi/Services/DateTimeService.cs b/PersonsApi/Services/DateTimeService.cs deleted file mode 100644 index 8854c4fed..000000000 --- a/PersonsApi/Services/DateTimeService.cs +++ /dev/null @@ -1,27 +0,0 @@ -using System; - -namespace PersonsApi.Services -{ - /// - /// Mechanism for retrieving DateTime data that provides a way to intercept and set the returned values to facilitate testing - /// - /// - /// - /// DateTime expected = DateTime.UtcNow; - /// DateTimeSource.UtcNow = () => expected; - /// - /// - public static class DateTimeSource - { - /// - /// Returns the value of unless overridden for testing. - /// - public static Func UtcNow { get; set; } = () => DateTime.UtcNow; - - /// - /// Returns the value of unless overridden for testing. - /// - public static Func UkTime { get; set; } = () => - TimeZoneInfo.ConvertTimeBySystemTimeZoneId(DateTime.Now, "GMT Standard Time"); - } -} \ No newline at end of file diff --git a/PersonsApi/Startup.cs b/PersonsApi/Startup.cs index 44f4a84b7..6a7d05f24 100644 --- a/PersonsApi/Startup.cs +++ b/PersonsApi/Startup.cs @@ -1,34 +1,29 @@ -using System.Text.Json.Serialization; using Dfe.Academisation.CorrelationIdMiddleware; using Microsoft.AspNetCore.HttpOverrides; +using System.Text.Json.Serialization; namespace PersonsApi { - //using DatabaseModels; - //using Gateways; + using Dfe.Academies.Application.MappingProfiles; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; + using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ApiExplorer; - using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; + using Microsoft.FeatureManagement; using Middleware; + using NetEscapades.AspNetCore.SecurityHeaders; + using PersonsApi.ResponseModels; + using PersonsApi.SerilogCustomEnrichers; + using PersonsApi.Swagger; using Swashbuckle.AspNetCore.SwaggerUI; using System; using System.IO; using System.Reflection; - //using PersonsApi.Configuration; - using PersonsApi.ResponseModels; - using PersonsApi.SerilogCustomEnrichers; - using UseCases; - using Microsoft.FeatureManagement; - using PersonsApi.Services; - using Microsoft.AspNetCore.Http; using System.Text; - using NetEscapades.AspNetCore.SecurityHeaders; - using PersonsApi.Swagger; - using Dfe.Academies.Application.MappingProfiles; + using UseCases; public class Startup { diff --git a/PersonsApi/Swagger/AuthenticationHeaderOperationFilter.cs b/PersonsApi/Swagger/AuthenticationHeaderOperationFilter.cs index 399dd3c8d..756e57b63 100644 --- a/PersonsApi/Swagger/AuthenticationHeaderOperationFilter.cs +++ b/PersonsApi/Swagger/AuthenticationHeaderOperationFilter.cs @@ -1,4 +1,3 @@ -using System.Collections.Generic; using Microsoft.OpenApi.Models; using Swashbuckle.AspNetCore.SwaggerGen; diff --git a/PersonsApi/Swagger/SwaggerOptions.cs b/PersonsApi/Swagger/SwaggerOptions.cs index f41520105..154e80572 100644 --- a/PersonsApi/Swagger/SwaggerOptions.cs +++ b/PersonsApi/Swagger/SwaggerOptions.cs @@ -1,6 +1,5 @@ using Microsoft.AspNetCore.Mvc.ApiExplorer; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Microsoft.OpenApi.Models; using Swashbuckle.AspNetCore.SwaggerGen; diff --git a/PersonsApi/UseCases/ApiKeyService.cs b/PersonsApi/UseCases/ApiKeyService.cs index 8fcd43f71..e6d89d369 100644 --- a/PersonsApi/UseCases/ApiKeyService.cs +++ b/PersonsApi/UseCases/ApiKeyService.cs @@ -1,6 +1,3 @@ -using System.Linq; -using Microsoft.Extensions.Configuration; -using Newtonsoft.Json; using PersonsApi.ResponseModels; namespace PersonsApi.UseCases From 71ed5e2fdd0c2378f8bc11de99119cb2ea9b654e Mon Sep 17 00:00:00 2001 From: Farshad DASHTI Date: Thu, 8 Aug 2024 16:44:41 +0100 Subject: [PATCH 2/2] Excluded generic repository from the code coverage --- Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs b/Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs index b967c6972..c742de607 100644 --- a/Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs +++ b/Dfe.Academies.Api.Infrastructure/Repositories/Repository.cs @@ -1,10 +1,12 @@ using Dfe.Academies.Domain.Repositories; using Microsoft.EntityFrameworkCore; +using System.Diagnostics.CodeAnalysis; using System.Linq.Expressions; namespace Dfe.Academies.Infrastructure.Repositories { -#pragma warning disable CS8603 // Possible null reference return, behaviour expected + #pragma warning disable CS8603 // Possible null reference return, behaviour expected + [ExcludeFromCodeCoverage] public abstract class Repository : IRepository where TEntity : class, new() where TDbContext : DbContext