#2190 bug can not remove device which is not in aws (#2198)

* Hide certificates when getting the magic ommand * can remove devices that are not in AWS * Fix tests => in next commit * Fix failed tests --------- Co-authored-by: Kevin BEAUGRAND <contact@kbeaugrand.fr>
CGI-FR · Jun 22, 2023 · 3c67f73 · 3c67f73
1 parent d99d001
commit 3c67f73
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 29 deletions.
diff --git a/src/IoTHub.Portal.Infrastructure/Services/AWS/AWSDeviceService.cs b/src/IoTHub.Portal.Infrastructure/Services/AWS/AWSDeviceService.cs
@@ -25,6 +25,8 @@ public class AWSDeviceService : DeviceService
         private readonly IDeviceRepository deviceRepository;
         private readonly IAmazonIoT amazonIoTClient;
         private readonly IAmazonIotData amazonIotDataClient;
+        private readonly ILogger<AWSDeviceService> logger;
+
 
         public AWSDeviceService(PortalDbContext portalDbContext,
             IMapper mapper,
@@ -44,6 +46,7 @@ public AWSDeviceService(PortalDbContext portalDbContext,
             this.deviceRepository = deviceRepository;
             this.amazonIoTClient = amazonIoTClient;
             this.amazonIotDataClient = amazonIotDataClient;
+            this.logger = logger;
         }
 
         public override async Task<DeviceDetails> CreateDevice(DeviceDetails device)
@@ -90,41 +93,52 @@ public override async Task DeleteDevice(string deviceId)
                 throw new ResourceNotFoundException($"The device with id {deviceId} doesn't exist");
             }
 
-            //Retrieve all thing principals and detach it before deleting the thing
-            var principals = await this.amazonIoTClient.ListThingPrincipalsAsync(new ListThingPrincipalsRequest
-            {
-                NextToken = string.Empty,
-                ThingName = device.Name
-            });
-
-            if (principals.HttpStatusCode != System.Net.HttpStatusCode.OK)
+            try
             {
-                throw new InternalServerErrorException($"Unable to retreive Thing {device.Name} principals due to an error in the Amazon IoT API : {principals.HttpStatusCode}");
-            }
-
-            foreach (var principal in principals.Principals)
-            {
-                var detachPrincipal = await this.amazonIoTClient.DetachThingPrincipalAsync(new DetachThingPrincipalRequest
+                //Retrieve all thing principals and detach it before deleting the thing
+                var principals = await this.amazonIoTClient.ListThingPrincipalsAsync(new ListThingPrincipalsRequest
                 {
-                    Principal = principal,
+                    NextToken = string.Empty,
                     ThingName = device.Name
                 });
 
-                if (detachPrincipal.HttpStatusCode != System.Net.HttpStatusCode.OK)
+                if (principals.HttpStatusCode != System.Net.HttpStatusCode.OK)
+                {
+                    throw new InternalServerErrorException($"Unable to retreive Thing {device.Name} principals due to an error in the Amazon IoT API : {principals.HttpStatusCode}");
+                }
+
+                foreach (var principal in principals.Principals)
                 {
-                    throw new InternalServerErrorException($"Unable to detach Thing {device.Name} principal due to an error in the Amazon IoT API : {detachPrincipal.HttpStatusCode}");
+                    var detachPrincipal = await this.amazonIoTClient.DetachThingPrincipalAsync(new DetachThingPrincipalRequest
+                    {
+                        Principal = principal,
+                        ThingName = device.Name
+                    });
+
+                    if (detachPrincipal.HttpStatusCode != System.Net.HttpStatusCode.OK)
+                    {
+                        throw new InternalServerErrorException($"Unable to detach Thing {device.Name} principal due to an error in the Amazon IoT API : {detachPrincipal.HttpStatusCode}");
+                    }
+                }
+
+                //Delete the thing type after detaching the principal
+                var deleteResponse = await this.amazonIoTClient.DeleteThingAsync(this.mapper.Map<DeleteThingRequest>(device));
+                if (deleteResponse.HttpStatusCode != System.Net.HttpStatusCode.OK)
+                {
+                    throw new InternalServerErrorException($"Unable to delete the thing with device name : {device.Name} due to an error in the Amazon IoT API : {deleteResponse.HttpStatusCode}");
                 }
             }
+            catch (AmazonIoTException e)
+            {
+                this.logger.LogWarning("Can not delete the device because it doesn't exist in AWS IoT", e);
 
-            //Delete the thing type after detaching the principal
-            var deleteResponse = await this.amazonIoTClient.DeleteThingAsync(this.mapper.Map<DeleteThingRequest>(device));
-            if (deleteResponse.HttpStatusCode != System.Net.HttpStatusCode.OK)
+            }
+            finally
             {
-                throw new InternalServerErrorException($"Unable to delete the thing with device name : {device.Name} due to an error in the Amazon IoT API : {deleteResponse.HttpStatusCode}");
+                //Delete Thing in DB
+                await DeleteDeviceInDatabase(deviceId);
             }
 
-            //Delete Thing in DB
-            await DeleteDeviceInDatabase(deviceId);
         }
     }
 }
diff --git a/src/IoTHub.Portal.Infrastructure/Services/AWS/AWSEdgeDevicesService.cs b/src/IoTHub.Portal.Infrastructure/Services/AWS/AWSEdgeDevicesService.cs
@@ -16,6 +16,7 @@ namespace IoTHub.Portal.Infrastructure.Services
     using IoTHub.Portal.Domain.Repositories;
     using IoTHub.Portal.Infrastructure.Helpers;
     using IoTHub.Portal.Models.v10;
+    using Microsoft.Extensions.Logging;
 
     public class AWSEdgeDevicesService : EdgeDevicesServiceBase, IEdgeDevicesService
     {
@@ -32,7 +33,7 @@ public class AWSEdgeDevicesService : EdgeDevicesServiceBase, IEdgeDevicesService
         private readonly IMapper mapper;
         private readonly IAmazonIoT amazonIoTClient;
         private readonly IAmazonGreengrassV2 amazonGreengrass;
-
+        private readonly ILogger<AWSEdgeDevicesService> logger;
         public AWSEdgeDevicesService(
             ConfigHandler configHandler,
             IEdgeEnrollementHelper edgeEnrollementHelper,
@@ -47,7 +48,8 @@ public AWSEdgeDevicesService(
             ILabelRepository labelRepository,
             IDeviceModelImageManager deviceModelImageManager,
             IAmazonIoT amazonIoTClient,
-            IAmazonGreengrassV2 amazonGreengrass)
+            IAmazonGreengrassV2 amazonGreengrass,
+            ILogger<AWSEdgeDevicesService> logger)
             : base(deviceTagService, edgeDeviceRepository, mapper, deviceModelImageManager, deviceTagValueRepository, labelRepository)
         {
             this.configHandler = configHandler;
@@ -63,6 +65,7 @@ public AWSEdgeDevicesService(
 
             this.amazonIoTClient = amazonIoTClient;
             this.amazonGreengrass = amazonGreengrass;
+            this.logger = logger;
         }
 
         /// <summary>
@@ -127,12 +130,23 @@ public async Task DeleteEdgeDeviceAsync(string deviceId)
         {
             var device = await base.GetEdgeDevice(deviceId);
 
-            await this.externalDeviceService.RemoveDeviceCredentials(device);
-            await this.externalDeviceService.DeleteDevice(device.DeviceName);
+            try
+            {
+                await this.externalDeviceService.RemoveDeviceCredentials(device);
+                await this.externalDeviceService.DeleteDevice(device.DeviceName);
+            }
+            catch (AmazonIoTException ex)
+            {
+                this.logger.LogWarning("Can not delete the edge device because it doesn't exist in AWS IoT", ex);
 
-            await DeleteEdgeDeviceInDatabase(deviceId);
+            }
+            finally
+            {
+                await DeleteEdgeDeviceInDatabase(deviceId);
+
+                await this.unitOfWork.SaveAsync();
+            }
 
-            await this.unitOfWork.SaveAsync();
         }
 
         /// <summary>

diff --git a/src/IoTHub.Portal.Tests.Unit/Infrastructure/Services/AWSDeviceServiceTests.cs b/src/IoTHub.Portal.Tests.Unit/Infrastructure/Services/AWSDeviceServiceTests.cs
@@ -455,6 +455,23 @@ public async Task DeleteDeviceShouldThrowInternalServerErrorIfHttpStatusCodeIsNo
                 HttpStatusCode = HttpStatusCode.BadRequest
             });
 
+            _ = this.mockDeviceRepository.Setup(repository => repository.GetByIdAsync(deviceDto.DeviceID, d => d.Tags, d => d.Labels))
+               .ReturnsAsync(device);
+
+            _ = this.mockDeviceRepository.Setup(repository => repository.GetByIdAsync(deviceDto.DeviceID))
+                .ReturnsAsync(device);
+
+            this.mockDeviceTagValueRepository.Setup(repository => repository.Delete(It.IsAny<string>()))
+                .Verifiable();
+
+            this.mockLabelRepository.Setup(repository => repository.Delete(It.IsAny<string>()))
+                .Verifiable();
+
+            this.mockDeviceRepository.Setup(repository => repository.Delete(deviceDto.DeviceID))
+                .Verifiable();
+
+            _ = this.mockUnitOfWork.Setup(work => work.SaveAsync())
+                .Returns(Task.CompletedTask);
             // Act
             var result = () => this.awsDeviceService.DeleteDevice(deviceDto.DeviceID);
 
@@ -498,6 +515,24 @@ public async Task DeleteDeviceShouldThrowInternalServerErrorIfHttpStatusCodeIsNo
                 HttpStatusCode = HttpStatusCode.BadRequest
             });
 
+            _ = this.mockDeviceRepository.Setup(repository => repository.GetByIdAsync(deviceDto.DeviceID, d => d.Tags, d => d.Labels))
+               .ReturnsAsync(device);
+
+            _ = this.mockDeviceRepository.Setup(repository => repository.GetByIdAsync(deviceDto.DeviceID))
+                .ReturnsAsync(device);
+
+            this.mockDeviceTagValueRepository.Setup(repository => repository.Delete(It.IsAny<string>()))
+                .Verifiable();
+
+            this.mockLabelRepository.Setup(repository => repository.Delete(It.IsAny<string>()))
+                .Verifiable();
+
+            this.mockDeviceRepository.Setup(repository => repository.Delete(deviceDto.DeviceID))
+                .Verifiable();
+
+            _ = this.mockUnitOfWork.Setup(work => work.SaveAsync())
+                .Returns(Task.CompletedTask);
+
             // Act
             var result = () => this.awsDeviceService.DeleteDevice(deviceDto.DeviceID);
 
@@ -547,6 +582,24 @@ public async Task DeleteDeviceShouldThrowInternalServerErrorIfHttpStatusCodeIsNo
                    HttpStatusCode = HttpStatusCode.BadRequest
                });
 
+            _ = this.mockDeviceRepository.Setup(repository => repository.GetByIdAsync(deviceDto.DeviceID, d => d.Tags, d => d.Labels))
+                .ReturnsAsync(device);
+
+            _ = this.mockDeviceRepository.Setup(repository => repository.GetByIdAsync(deviceDto.DeviceID))
+                .ReturnsAsync(device);
+
+            this.mockDeviceTagValueRepository.Setup(repository => repository.Delete(It.IsAny<string>()))
+                .Verifiable();
+
+            this.mockLabelRepository.Setup(repository => repository.Delete(It.IsAny<string>()))
+                .Verifiable();
+
+            this.mockDeviceRepository.Setup(repository => repository.Delete(deviceDto.DeviceID))
+                .Verifiable();
+
+            _ = this.mockUnitOfWork.Setup(work => work.SaveAsync())
+                .Returns(Task.CompletedTask);
+
             // Act
             var result = () => this.awsDeviceService.DeleteDevice(deviceDto.DeviceID);