From ac89e73bf465fbcdbc3460d6b5de9bf20f76f8ff Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Thu, 27 Apr 2017 16:53:51 -0400 Subject: [PATCH 1/6] getting initial tests working --- tests/IntegrationTests/IntegrationTests.csproj | 6 +++++- .../Controllers/CatalogControllerGetImage.cs | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 tests/IntegrationTests/Web/Controllers/CatalogControllerGetImage.cs diff --git a/tests/IntegrationTests/IntegrationTests.csproj b/tests/IntegrationTests/IntegrationTests.csproj index f032596..f27ff35 100644 --- a/tests/IntegrationTests/IntegrationTests.csproj +++ b/tests/IntegrationTests/IntegrationTests.csproj @@ -1,4 +1,4 @@ - + netcoreapp1.1 @@ -10,4 +10,8 @@ + + + + diff --git a/tests/IntegrationTests/Web/Controllers/CatalogControllerGetImage.cs b/tests/IntegrationTests/Web/Controllers/CatalogControllerGetImage.cs new file mode 100644 index 0000000..0217ad1 --- /dev/null +++ b/tests/IntegrationTests/Web/Controllers/CatalogControllerGetImage.cs @@ -0,0 +1,17 @@ +using Microsoft.eShopWeb.Controllers; +using System; +using System.Collections.Generic; +using System.Text; +using Xunit; + +namespace IntegrationTests.Web.Controllers +{ + public class CatalogControllerGetImage + { + [Fact] + public void ReturnsFileContentResultGivenValidId() + { + //var controller = new CatalogController() + } + } +} From 5dbbc4c79101a914fb868baf9bb6fffbbda3e9a4 Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Fri, 28 Apr 2017 19:56:32 -0400 Subject: [PATCH 2/6] Adding unit tests, refactoring file access to a service, adding CatalogImageMissingException. --- .../CatalogImageMissingException.cs | 13 ++++ .../Interfaces/IBasketService.cs | 1 - .../Interfaces/IImageService.cs | 16 +++++ .../FileSystem/LocalFileImageService.cs | 22 +++++++ src/Infrastructure/Infrastructure.csproj | 3 + src/Web/Controllers/CatalogController.cs | 35 +++++++---- src/Web/Startup.cs | 3 + .../IntegrationTests/IntegrationTests.csproj | 4 ++ tests/UnitTests/UnitTests.csproj | 14 ++++- .../Controllers/CatalogControllerGetImage.cs | 60 +++++++++++++++++++ 10 files changed, 157 insertions(+), 14 deletions(-) create mode 100644 src/ApplicationCore/Exceptions/CatalogImageMissingException.cs create mode 100644 src/ApplicationCore/Interfaces/IImageService.cs create mode 100644 src/Infrastructure/FileSystem/LocalFileImageService.cs create mode 100644 tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs diff --git a/src/ApplicationCore/Exceptions/CatalogImageMissingException.cs b/src/ApplicationCore/Exceptions/CatalogImageMissingException.cs new file mode 100644 index 0000000..a7c1062 --- /dev/null +++ b/src/ApplicationCore/Exceptions/CatalogImageMissingException.cs @@ -0,0 +1,13 @@ +using System; + +namespace ApplicationCore.Exceptions +{ + public class CatalogImageMissingException : Exception + { + public CatalogImageMissingException(string message, + Exception innerException = null) + : base(message, innerException: innerException) + { + } + } +} diff --git a/src/ApplicationCore/Interfaces/IBasketService.cs b/src/ApplicationCore/Interfaces/IBasketService.cs index 1b868a1..a5f25b9 100644 --- a/src/ApplicationCore/Interfaces/IBasketService.cs +++ b/src/ApplicationCore/Interfaces/IBasketService.cs @@ -14,5 +14,4 @@ namespace ApplicationCore.Interfaces { T Parse(IPrincipal principal); } - } diff --git a/src/ApplicationCore/Interfaces/IImageService.cs b/src/ApplicationCore/Interfaces/IImageService.cs new file mode 100644 index 0000000..ae41333 --- /dev/null +++ b/src/ApplicationCore/Interfaces/IImageService.cs @@ -0,0 +1,16 @@ +using ApplicationCore.Entities; +using Microsoft.eShopWeb.ApplicationCore.Entities; +using System.Security.Principal; +using System.Threading.Tasks; + +namespace ApplicationCore.Interfaces +{ + + public interface IImageService + { + byte[] GetImageBytesById(int id); + } + + + +} diff --git a/src/Infrastructure/FileSystem/LocalFileImageService.cs b/src/Infrastructure/FileSystem/LocalFileImageService.cs new file mode 100644 index 0000000..ab876af --- /dev/null +++ b/src/Infrastructure/FileSystem/LocalFileImageService.cs @@ -0,0 +1,22 @@ +using ApplicationCore.Interfaces; +using Microsoft.AspNetCore.Hosting; +using System.IO; + +namespace Infrastructure.FileSystem +{ + public class LocalFileImageService : IImageService + { + private readonly IHostingEnvironment _env; + + public LocalFileImageService(IHostingEnvironment env) + { + _env = env; + } + public byte[] GetImageBytesById(int id) + { + var contentRoot = _env.ContentRootPath + "//Pics"; + var path = Path.Combine(contentRoot, id + ".png"); + return File.ReadAllBytes(path); + } + } +} diff --git a/src/Infrastructure/Infrastructure.csproj b/src/Infrastructure/Infrastructure.csproj index b377567..c2c8f95 100644 --- a/src/Infrastructure/Infrastructure.csproj +++ b/src/Infrastructure/Infrastructure.csproj @@ -14,5 +14,8 @@ + + + \ No newline at end of file diff --git a/src/Web/Controllers/CatalogController.cs b/src/Web/Controllers/CatalogController.cs index 25d50ae..fa7bb18 100644 --- a/src/Web/Controllers/CatalogController.cs +++ b/src/Web/Controllers/CatalogController.cs @@ -5,18 +5,24 @@ using Microsoft.AspNetCore.Mvc; using System; using System.IO; using System.Threading.Tasks; +using ApplicationCore.Interfaces; +using ApplicationCore.Exceptions; namespace Microsoft.eShopWeb.Controllers { public class CatalogController : Controller { private readonly IHostingEnvironment _env; - private readonly ICatalogService _catalogSvc; + private readonly ICatalogService _catalogService; + private readonly IImageService _imageService; - public CatalogController(IHostingEnvironment env, ICatalogService catalogSvc) + public CatalogController(IHostingEnvironment env, + ICatalogService catalogService, + IImageService imageService) { _env = env; - _catalogSvc = catalogSvc; + _catalogService = catalogService; + _imageService = imageService; } @@ -24,13 +30,13 @@ namespace Microsoft.eShopWeb.Controllers public async Task Index(int? BrandFilterApplied, int? TypesFilterApplied, int? page) { var itemsPage = 10; - var catalog = await _catalogSvc.GetCatalogItems(page ?? 0, itemsPage, BrandFilterApplied, TypesFilterApplied); + var catalog = await _catalogService.GetCatalogItems(page ?? 0, itemsPage, BrandFilterApplied, TypesFilterApplied); var vm = new CatalogIndex() { CatalogItems = catalog.Data, - Brands = await _catalogSvc.GetBrands(), - Types = await _catalogSvc.GetTypes(), + Brands = await _catalogService.GetBrands(), + Types = await _catalogService.GetTypes(), BrandFilterApplied = BrandFilterApplied ?? 0, TypesFilterApplied = TypesFilterApplied ?? 0, PaginationInfo = new PaginationInfo() @@ -53,13 +59,18 @@ namespace Microsoft.eShopWeb.Controllers // GET: //pic/{id} public IActionResult GetImage(int id) { - var contentRoot = _env.ContentRootPath + "//Pics"; - var path = Path.Combine(contentRoot, id + ".png"); - Byte[] b = System.IO.File.ReadAllBytes(path); - return File(b, "image/png"); - + byte[] imageBytes; + try + { + imageBytes = _imageService.GetImageBytesById(id); + } + catch (CatalogImageMissingException ex) + { + return NotFound(); + } + return File(imageBytes, "image/png"); } - + public IActionResult Error() { return View(); diff --git a/src/Web/Startup.cs b/src/Web/Startup.cs index 0bf788f..6ec6990 100644 --- a/src/Web/Startup.cs +++ b/src/Web/Startup.cs @@ -11,6 +11,8 @@ using Infrastructure.Identity; using Microsoft.AspNetCore.Identity.EntityFrameworkCore; using System.Text; using Microsoft.AspNetCore.Http; +using ApplicationCore.Interfaces; +using Infrastructure.FileSystem; namespace Microsoft.eShopWeb { @@ -65,6 +67,7 @@ namespace Microsoft.eShopWeb services.AddScoped(); services.AddScoped(); services.Configure(Configuration); + services.AddSingleton(); services.AddMvc(); _services = services; diff --git a/tests/IntegrationTests/IntegrationTests.csproj b/tests/IntegrationTests/IntegrationTests.csproj index f27ff35..c3a4881 100644 --- a/tests/IntegrationTests/IntegrationTests.csproj +++ b/tests/IntegrationTests/IntegrationTests.csproj @@ -14,4 +14,8 @@ + + + + diff --git a/tests/UnitTests/UnitTests.csproj b/tests/UnitTests/UnitTests.csproj index f032596..18ba44b 100644 --- a/tests/UnitTests/UnitTests.csproj +++ b/tests/UnitTests/UnitTests.csproj @@ -1,13 +1,25 @@ - + netcoreapp1.1 + + + + + + + + + + + + diff --git a/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs b/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs new file mode 100644 index 0000000..6bcdb4e --- /dev/null +++ b/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs @@ -0,0 +1,60 @@ +using ApplicationCore.Exceptions; +using ApplicationCore.Interfaces; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc; +using Microsoft.eShopWeb.Controllers; +using Moq; +using Xunit; + +namespace UnitTests +{ + public class CatalogControllerGetImage + { + private Mock _mockImageService = new Mock(); + private CatalogController _controller; + private int _testImageId = 123; + private byte[] _testBytes = { 0x01, 0x02, 0x03 }; + + public CatalogControllerGetImage() + { + _controller = new CatalogController(null, null, _mockImageService.Object); + } + + [Fact] + public void CallsImageServiceWithId() + { + _mockImageService + .Setup(i => i.GetImageBytesById(_testImageId)) + .Returns(_testBytes) + .Verifiable(); + + _controller.GetImage(_testImageId); + _mockImageService.Verify(); + } + + [Fact] + public void ReturnsFileResultWithBytesGivenSuccess() + { + _mockImageService + .Setup(i => i.GetImageBytesById(_testImageId)) + .Returns(_testBytes); + + var result = _controller.GetImage(_testImageId); + + var fileResult = Assert.IsType(result); + var bytes = Assert.IsType(fileResult.FileContents); + } + + [Fact] + public void ReturnsNotFoundResultGivenImageMissingException() + { + _mockImageService + .Setup(i => i.GetImageBytesById(_testImageId)) + .Throws(new CatalogImageMissingException("missing image")); + + var result = _controller.GetImage(_testImageId); + + var actionResult = Assert.IsType(result); + } + } +} From 6f908bb8e527e9360944f931dde9b61dd76a596a Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Fri, 28 Apr 2017 20:27:18 -0400 Subject: [PATCH 3/6] Looking to unit test logging but blocked by extension method currently. --- src/Web/Controllers/CatalogController.cs | 11 +++-- .../Controllers/CatalogControllerGetImage.cs | 45 ++++++++++++++----- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/Web/Controllers/CatalogController.cs b/src/Web/Controllers/CatalogController.cs index fa7bb18..eb64454 100644 --- a/src/Web/Controllers/CatalogController.cs +++ b/src/Web/Controllers/CatalogController.cs @@ -3,10 +3,10 @@ using Microsoft.eShopWeb.ViewModels; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc; using System; -using System.IO; using System.Threading.Tasks; using ApplicationCore.Interfaces; using ApplicationCore.Exceptions; +using Microsoft.Extensions.Logging; namespace Microsoft.eShopWeb.Controllers { @@ -15,16 +15,18 @@ namespace Microsoft.eShopWeb.Controllers private readonly IHostingEnvironment _env; private readonly ICatalogService _catalogService; private readonly IImageService _imageService; + private readonly ILogger _logger; public CatalogController(IHostingEnvironment env, ICatalogService catalogService, - IImageService imageService) + IImageService imageService, + ILogger logger) { _env = env; _catalogService = catalogService; _imageService = imageService; - } - + _logger = logger; + } // GET: // public async Task Index(int? BrandFilterApplied, int? TypesFilterApplied, int? page) @@ -66,6 +68,7 @@ namespace Microsoft.eShopWeb.Controllers } catch (CatalogImageMissingException ex) { + _logger.LogWarning($"No image found for id: {id}"); return NotFound(); } return File(imageBytes, "image/png"); diff --git a/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs b/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs index 6bcdb4e..a16b695 100644 --- a/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs +++ b/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs @@ -3,6 +3,7 @@ using ApplicationCore.Interfaces; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc; using Microsoft.eShopWeb.Controllers; +using Microsoft.Extensions.Logging; using Moq; using Xunit; @@ -11,22 +12,21 @@ namespace UnitTests public class CatalogControllerGetImage { private Mock _mockImageService = new Mock(); + private Mock> _mockLogger = new Mock>(); private CatalogController _controller; private int _testImageId = 123; private byte[] _testBytes = { 0x01, 0x02, 0x03 }; public CatalogControllerGetImage() { - _controller = new CatalogController(null, null, _mockImageService.Object); + _controller = new CatalogController(null, null, _mockImageService.Object, + _mockLogger.Object); } [Fact] public void CallsImageServiceWithId() { - _mockImageService - .Setup(i => i.GetImageBytesById(_testImageId)) - .Returns(_testBytes) - .Verifiable(); + SetupImageWithTestBytes(); _controller.GetImage(_testImageId); _mockImageService.Verify(); @@ -35,9 +35,7 @@ namespace UnitTests [Fact] public void ReturnsFileResultWithBytesGivenSuccess() { - _mockImageService - .Setup(i => i.GetImageBytesById(_testImageId)) - .Returns(_testBytes); + SetupImageWithTestBytes(); var result = _controller.GetImage(_testImageId); @@ -48,13 +46,38 @@ namespace UnitTests [Fact] public void ReturnsNotFoundResultGivenImageMissingException() { - _mockImageService - .Setup(i => i.GetImageBytesById(_testImageId)) - .Throws(new CatalogImageMissingException("missing image")); + SetupMissingImage(); var result = _controller.GetImage(_testImageId); var actionResult = Assert.IsType(result); } + + [Fact] + public void LogsWarningGivenImageMissingException() + { + SetupMissingImage(); + _mockLogger.Setup(l => l.LogWarning(It.IsAny())) + .Verifiable(); + + _controller.GetImage(_testImageId); + + _mockLogger.Verify(); + } + + private void SetupMissingImage() + { + _mockImageService + .Setup(i => i.GetImageBytesById(_testImageId)) + .Throws(new CatalogImageMissingException("missing image")); + } + + private void SetupImageWithTestBytes() + { + _mockImageService + .Setup(i => i.GetImageBytesById(_testImageId)) + .Returns(_testBytes) + .Verifiable(); + } } } From dfe0106ce32e3d7f9344b7ab4e2af28c9c7baad1 Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Sun, 30 Apr 2017 08:06:06 -0400 Subject: [PATCH 4/6] Unit tests working; added logger adapter. --- src/ApplicationCore/Interfaces/IAppLogger.cs | 7 +++++++ .../Interfaces/IImageService.cs | 11 +---------- src/Infrastructure/Logging/LoggerAdapter.cs | 18 ++++++++++++++++++ src/Web/Controllers/CatalogController.cs | 4 ++-- src/Web/Startup.cs | 2 ++ .../Controllers/CatalogControllerGetImage.cs | 2 +- 6 files changed, 31 insertions(+), 13 deletions(-) create mode 100644 src/ApplicationCore/Interfaces/IAppLogger.cs create mode 100644 src/Infrastructure/Logging/LoggerAdapter.cs diff --git a/src/ApplicationCore/Interfaces/IAppLogger.cs b/src/ApplicationCore/Interfaces/IAppLogger.cs new file mode 100644 index 0000000..1ae198d --- /dev/null +++ b/src/ApplicationCore/Interfaces/IAppLogger.cs @@ -0,0 +1,7 @@ +namespace ApplicationCore.Interfaces +{ + public interface IAppLogger + { + void LogWarning(string message, params object[] args); + } +} diff --git a/src/ApplicationCore/Interfaces/IImageService.cs b/src/ApplicationCore/Interfaces/IImageService.cs index ae41333..d2d01c7 100644 --- a/src/ApplicationCore/Interfaces/IImageService.cs +++ b/src/ApplicationCore/Interfaces/IImageService.cs @@ -1,16 +1,7 @@ -using ApplicationCore.Entities; -using Microsoft.eShopWeb.ApplicationCore.Entities; -using System.Security.Principal; -using System.Threading.Tasks; - -namespace ApplicationCore.Interfaces +namespace ApplicationCore.Interfaces { - public interface IImageService { byte[] GetImageBytesById(int id); } - - - } diff --git a/src/Infrastructure/Logging/LoggerAdapter.cs b/src/Infrastructure/Logging/LoggerAdapter.cs new file mode 100644 index 0000000..b79e843 --- /dev/null +++ b/src/Infrastructure/Logging/LoggerAdapter.cs @@ -0,0 +1,18 @@ +using ApplicationCore.Interfaces; +using Microsoft.Extensions.Logging; + +namespace Infrastructure.Logging +{ + public class LoggerAdapter : IAppLogger + { + private readonly ILogger _logger; + public LoggerAdapter(ILoggerFactory loggerFactory) + { + _logger = loggerFactory.CreateLogger(); + } + public void LogWarning(string message, params object[] args) + { + _logger.LogWarning(message, args); + } + } +} diff --git a/src/Web/Controllers/CatalogController.cs b/src/Web/Controllers/CatalogController.cs index eb64454..c414e68 100644 --- a/src/Web/Controllers/CatalogController.cs +++ b/src/Web/Controllers/CatalogController.cs @@ -15,12 +15,12 @@ namespace Microsoft.eShopWeb.Controllers private readonly IHostingEnvironment _env; private readonly ICatalogService _catalogService; private readonly IImageService _imageService; - private readonly ILogger _logger; + private readonly IAppLogger _logger; public CatalogController(IHostingEnvironment env, ICatalogService catalogService, IImageService imageService, - ILogger logger) + IAppLogger logger) { _env = env; _catalogService = catalogService; diff --git a/src/Web/Startup.cs b/src/Web/Startup.cs index 6ec6990..7c2f2ed 100644 --- a/src/Web/Startup.cs +++ b/src/Web/Startup.cs @@ -13,6 +13,7 @@ using System.Text; using Microsoft.AspNetCore.Http; using ApplicationCore.Interfaces; using Infrastructure.FileSystem; +using Infrastructure.Logging; namespace Microsoft.eShopWeb { @@ -68,6 +69,7 @@ namespace Microsoft.eShopWeb services.AddScoped(); services.Configure(Configuration); services.AddSingleton(); + services.AddScoped(typeof(IAppLogger<>), typeof(LoggerAdapter<>)); services.AddMvc(); _services = services; diff --git a/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs b/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs index a16b695..2c65931 100644 --- a/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs +++ b/tests/UnitTests/Web/Controllers/CatalogControllerGetImage.cs @@ -12,7 +12,7 @@ namespace UnitTests public class CatalogControllerGetImage { private Mock _mockImageService = new Mock(); - private Mock> _mockLogger = new Mock>(); + private Mock> _mockLogger = new Mock>(); private CatalogController _controller; private int _testImageId = 123; private byte[] _testBytes = { 0x01, 0x02, 0x03 }; From bc088beb831486182387f2c9c438c22d5c9169d9 Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Sun, 30 Apr 2017 08:20:27 -0400 Subject: [PATCH 5/6] Fixed route --- src/Web/Controllers/CatalogController.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Web/Controllers/CatalogController.cs b/src/Web/Controllers/CatalogController.cs index c414e68..0874cd3 100644 --- a/src/Web/Controllers/CatalogController.cs +++ b/src/Web/Controllers/CatalogController.cs @@ -56,9 +56,7 @@ namespace Microsoft.eShopWeb.Controllers return View(vm); } - [HttpGet("{id}")] - [Route("[controller]/pic/{id}")] - // GET: //pic/{id} + [HttpGet("[controller]/pic/{id}")] public IActionResult GetImage(int id) { byte[] imageBytes; @@ -74,6 +72,7 @@ namespace Microsoft.eShopWeb.Controllers return File(imageBytes, "image/png"); } + public IActionResult Error() { return View(); From 3b1e46d4d64e8c814b4624ac6babf305bf585f02 Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Sun, 30 Apr 2017 08:26:08 -0400 Subject: [PATCH 6/6] Implemented CatalogImageMissingException in LocalFileImageService --- .../Exceptions/CatalogImageMissingException.cs | 9 +++++++-- .../FileSystem/LocalFileImageService.cs | 16 ++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/ApplicationCore/Exceptions/CatalogImageMissingException.cs b/src/ApplicationCore/Exceptions/CatalogImageMissingException.cs index a7c1062..ff07258 100644 --- a/src/ApplicationCore/Exceptions/CatalogImageMissingException.cs +++ b/src/ApplicationCore/Exceptions/CatalogImageMissingException.cs @@ -4,10 +4,15 @@ namespace ApplicationCore.Exceptions { public class CatalogImageMissingException : Exception { - public CatalogImageMissingException(string message, - Exception innerException = null) + public CatalogImageMissingException(string message, + Exception innerException = null) : base(message, innerException: innerException) { } + public CatalogImageMissingException(Exception innerException) + : base("No catalog image found for the provided id.", + innerException: innerException) + { + } } } diff --git a/src/Infrastructure/FileSystem/LocalFileImageService.cs b/src/Infrastructure/FileSystem/LocalFileImageService.cs index ab876af..82db8b2 100644 --- a/src/Infrastructure/FileSystem/LocalFileImageService.cs +++ b/src/Infrastructure/FileSystem/LocalFileImageService.cs @@ -1,4 +1,5 @@ -using ApplicationCore.Interfaces; +using ApplicationCore.Exceptions; +using ApplicationCore.Interfaces; using Microsoft.AspNetCore.Hosting; using System.IO; @@ -14,9 +15,16 @@ namespace Infrastructure.FileSystem } public byte[] GetImageBytesById(int id) { - var contentRoot = _env.ContentRootPath + "//Pics"; - var path = Path.Combine(contentRoot, id + ".png"); - return File.ReadAllBytes(path); + try + { + var contentRoot = _env.ContentRootPath + "//Pics"; + var path = Path.Combine(contentRoot, id + ".png"); + return File.ReadAllBytes(path); + } + catch (FileNotFoundException ex) + { + throw new CatalogImageMissingException(ex); + } } } }