From 23f68ce8d1bdfded623f5a98958c578da128f196 Mon Sep 17 00:00:00 2001 From: Hocine Hacherouf Date: Fri, 16 Sep 2022 12:56:35 +0200 Subject: [PATCH] Fix loading device model properties on a device #1249 --- .../DeviceModelPropertiesServiceTests.cs | 96 +++++++------------ .../Services/DeviceModelPropertiesService.cs | 50 +++------- 2 files changed, 43 insertions(+), 103 deletions(-) diff --git a/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/DeviceModelPropertiesServiceTests.cs b/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/DeviceModelPropertiesServiceTests.cs index c5a12926b..8c1498980 100644 --- a/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/DeviceModelPropertiesServiceTests.cs +++ b/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/DeviceModelPropertiesServiceTests.cs @@ -6,18 +6,13 @@ namespace AzureIoTHub.Portal.Tests.Unit.Server.Services using System; using System.Collections.Generic; using System.Linq; - using System.Threading; using System.Threading.Tasks; - using Azure; - using Azure.Data.Tables; using AzureIoTHub.Portal.Domain; using AzureIoTHub.Portal.Domain.Entities; using AzureIoTHub.Portal.Domain.Exceptions; using AzureIoTHub.Portal.Domain.Repositories; using AzureIoTHub.Portal.Server.Services; - using Microsoft.AspNetCore.Http; using Microsoft.EntityFrameworkCore; - using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; @@ -25,21 +20,17 @@ namespace AzureIoTHub.Portal.Tests.Unit.Server.Services public class DeviceModelPropertiesServiceTests { private MockRepository mockRepository; - private Mock mockTableClientFactory; - private Mock mockDeviceTemplatesTableClient; private Mock mockDeviceModelPropertiesRepository; + private Mock mockDeviceModelRepository; private Mock mockUnitOfWork; - private Mock> mockLogger; [SetUp] public void SetUp() { this.mockRepository = new MockRepository(MockBehavior.Strict); - this.mockTableClientFactory = this.mockRepository.Create(); - this.mockDeviceTemplatesTableClient = this.mockRepository.Create(); this.mockDeviceModelPropertiesRepository = this.mockRepository.Create(); + this.mockDeviceModelRepository = this.mockRepository.Create(); this.mockUnitOfWork = this.mockRepository.Create(); - this.mockLogger = this.mockRepository.Create>(); } [Test] @@ -49,18 +40,18 @@ public async Task GetPropertiesStateUnderTestExpectedBehavior() var instance = CreateDeviceModelPropertiesService(); var entity = SetupMockEntity(); - _ = this.mockDeviceModelPropertiesRepository.Setup(c => c.GetModelProperties(entity.RowKey)) + _ = this.mockDeviceModelPropertiesRepository.Setup(c => c.GetModelProperties(entity.Id)) .ReturnsAsync(new[] { new DeviceModelProperty { Id = Guid.NewGuid().ToString(), - ModelId = entity.RowKey + ModelId = entity.Id } }); // Act - var result = await instance.GetModelProperties(entity.RowKey); + var result = await instance.GetModelProperties(entity.Id); // Assert Assert.NotNull(result); @@ -80,6 +71,17 @@ public void WhenDeviceModelNotExistsGetPropertiesShouldReturnHttp404() _ = Assert.ThrowsAsync(async () => await instance.GetModelProperties(Guid.NewGuid().ToString())); } + [Test] + public void SavePropertiesForModelShouldThrowResourceNotFoundExceptionWhenDeviceModelNotExists() + { + // Arrange + var instance = CreateDeviceModelPropertiesService(); + SetupNotFoundEntity(); + + // Act + _ = Assert.ThrowsAsync(async () => await instance.SavePropertiesForModel(Guid.NewGuid().ToString(), new List())); + } + [Test] public async Task SetPropertiesStateUnderTestExpectedBehavior() { @@ -96,14 +98,14 @@ public async Task SetPropertiesStateUnderTestExpectedBehavior() } }; - _ = this.mockDeviceModelPropertiesRepository.Setup(c => c.SavePropertiesForModel(entity.RowKey, It.IsAny>())) + _ = this.mockDeviceModelPropertiesRepository.Setup(c => c.SavePropertiesForModel(entity.Id, It.IsAny>())) .Returns(Task.CompletedTask); _ = this.mockUnitOfWork.Setup(c => c.SaveAsync()) .Returns(Task.CompletedTask); // Act - await instance.SavePropertiesForModel(entity.RowKey, properties); + await instance.SavePropertiesForModel(entity.Id, properties); // Assert this.mockRepository.VerifyAll(); @@ -117,76 +119,42 @@ public void WhenExceptionOccuresSavePropertiesForModelShouldThrowInternalServerE var entity = SetupMockEntity(); - _ = this.mockDeviceModelPropertiesRepository.Setup(c => c.SavePropertiesForModel(entity.RowKey, It.IsAny>())) + _ = this.mockDeviceModelPropertiesRepository.Setup(c => c.SavePropertiesForModel(entity.Id, It.IsAny>())) .Returns(Task.CompletedTask); _ = this.mockUnitOfWork.Setup(c => c.SaveAsync()) .Throws(); // Act - _ = Assert.ThrowsAsync(async () => await instance.SavePropertiesForModel(entity.RowKey, Array.Empty())); + _ = Assert.ThrowsAsync(async () => await instance.SavePropertiesForModel(entity.Id, Array.Empty())); } - [Test] - public void WhenExceptionOccuresDuringCheckingModelExistanceShouldThrowInternalServerErrorException() + private DeviceModel SetupMockEntity() { - // Arrange - var instance = CreateDeviceModelPropertiesService(); - - _ = this.mockTableClientFactory.Setup(c => c.GetDeviceTemplates()) - .Returns(this.mockDeviceTemplatesTableClient.Object); - - _ = this.mockDeviceTemplatesTableClient.Setup(c => c.GetEntityAsync(It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny())) - .Throws(new RequestFailedException("failed")); - - _ = this.mockLogger.Setup(c => c.Log(LogLevel.Error, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - - // Act - _ = Assert.ThrowsAsync(async () => await instance.GetModelProperties(Guid.NewGuid().ToString())); - _ = Assert.ThrowsAsync(async () => await instance.SavePropertiesForModel(Guid.NewGuid().ToString(), Array.Empty())); - - this.mockRepository.VerifyAll(); - } - - private TableEntity SetupMockEntity() - { - var mockResponse = this.mockRepository.Create>(); var modelId = Guid.NewGuid().ToString(); - var entity = new TableEntity("0", modelId); - - _ = this.mockDeviceTemplatesTableClient.Setup(c => c.GetEntityAsync( - It.Is(p => p == "0"), - It.Is(k => k == modelId), - It.IsAny>(), - It.IsAny())) - .ReturnsAsync(mockResponse.Object); + var deviceModelEntity = new DeviceModel + { + Id = modelId + }; - _ = this.mockTableClientFactory.Setup(c => c.GetDeviceTemplates()) - .Returns(this.mockDeviceTemplatesTableClient.Object); + _ = this.mockDeviceModelRepository.Setup(repository => repository.GetByIdAsync(modelId)) + .ReturnsAsync(deviceModelEntity); - return entity; + return deviceModelEntity; } private void SetupNotFoundEntity() { - _ = this.mockDeviceTemplatesTableClient.Setup(c => c.GetEntityAsync( - It.Is(p => p == "0"), - It.IsAny(), - It.IsAny>(), - It.IsAny())) - .Throws(new RequestFailedException(StatusCodes.Status404NotFound, "Not Found")); - - _ = this.mockTableClientFactory.Setup(c => c.GetDeviceTemplates()) - .Returns(this.mockDeviceTemplatesTableClient.Object); + _ = this.mockDeviceModelRepository.Setup(repository => repository.GetByIdAsync(It.IsAny())) + .ReturnsAsync((DeviceModel)null); } private DeviceModelPropertiesService CreateDeviceModelPropertiesService() { return new DeviceModelPropertiesService( - this.mockLogger.Object, this.mockUnitOfWork.Object, - this.mockTableClientFactory.Object, - this.mockDeviceModelPropertiesRepository.Object); + this.mockDeviceModelPropertiesRepository.Object, + this.mockDeviceModelRepository.Object); } } diff --git a/src/AzureIoTHub.Portal/Server/Services/DeviceModelPropertiesService.cs b/src/AzureIoTHub.Portal/Server/Services/DeviceModelPropertiesService.cs index 1ae52e62c..49f01cbda 100644 --- a/src/AzureIoTHub.Portal/Server/Services/DeviceModelPropertiesService.cs +++ b/src/AzureIoTHub.Portal/Server/Services/DeviceModelPropertiesService.cs @@ -5,15 +5,11 @@ namespace AzureIoTHub.Portal.Server.Services { using System.Collections.Generic; using System.Threading.Tasks; - using Azure; - using Azure.Data.Tables; - using AzureIoTHub.Portal.Domain; + using Domain; using AzureIoTHub.Portal.Domain.Entities; - using AzureIoTHub.Portal.Domain.Exceptions; - using AzureIoTHub.Portal.Domain.Repositories; - using Microsoft.AspNetCore.Http; + using Domain.Exceptions; + using Domain.Repositories; using Microsoft.EntityFrameworkCore; - using Microsoft.Extensions.Logging; public class DeviceModelPropertiesService : IDeviceModelPropertiesService { @@ -22,28 +18,18 @@ public class DeviceModelPropertiesService : IDeviceModelPropertiesService /// private readonly IUnitOfWork unitOfWork; - /// - /// The table client factory. - /// - private readonly ITableClientFactory tableClientFactory; - /// /// The device model properties repository. /// private readonly IDeviceModelPropertiesRepository deviceModelPropertiesRepository; - /// - /// The logger. - /// - private readonly ILogger log; + private readonly IDeviceModelRepository deviceModelRepository; - public DeviceModelPropertiesService( - ILogger log, IUnitOfWork unitOfWork, ITableClientFactory tableClientFactory, IDeviceModelPropertiesRepository deviceModelPropertiesRepository) + public DeviceModelPropertiesService(IUnitOfWork unitOfWork, IDeviceModelPropertiesRepository deviceModelPropertiesRepository, IDeviceModelRepository deviceModelRepository) { - this.log = log; this.unitOfWork = unitOfWork; - this.tableClientFactory = tableClientFactory; this.deviceModelPropertiesRepository = deviceModelPropertiesRepository; + this.deviceModelRepository = deviceModelRepository; } public async Task> GetModelProperties(string modelId) @@ -69,27 +55,13 @@ public async Task SavePropertiesForModel(string modelId, IEnumerable AssertModelExists(string id) + private async Task AssertModelExists(string deviceModelId) { - try - { - _ = await this.tableClientFactory - .GetDeviceTemplates() - .GetEntityAsync("0", id); - - return true; - } - catch (RequestFailedException e) - { - if (e.Status == StatusCodes.Status404NotFound) - { - throw new ResourceNotFoundException($"The model {id} doesn't exist."); - } - - this.log.LogError(e.Message, e); + var deviceModelEntity = await this.deviceModelRepository.GetByIdAsync(deviceModelId); - throw new InternalServerErrorException($"Unable to check if device model with id {id} exist", e); - } + return deviceModelEntity == null + ? throw new ResourceNotFoundException($"The device model {deviceModelId} doesn't exist") + : true; } } }