Skip to content

Commit

Permalink
Fix: Bug-When device model is delete some Azure resources are still t…
Browse files Browse the repository at this point in the history
…here #1832 (#1837)

* fix delete device model + unit tests

* update unit tests + services

* update unit tests azure device registry provider
  • Loading branch information
GuillaumeM-2ISA authored Mar 3, 2023
1 parent 014a64b commit 4323ea0
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,7 @@ public interface IDeviceRegistryProvider
Task<EnrollmentCredentials> GetEnrollmentCredentialsAsync(string deviceId, string modelId);

Task DeleteEnrollmentGroupAsync(EnrollmentGroup enrollmentGroup, CancellationToken cancellationToken);

Task DeleteEnrollmentGroupByDeviceModelIdAsync(string deviceModelId, CancellationToken cancellationToken);
}
}
2 changes: 2 additions & 0 deletions src/AzureIoTHub.Portal.Application/Services/IConfigService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public interface IConfigService

Task RollOutDeviceModelConfiguration(string modelId, Dictionary<string, object> desiredProperties);

Task DeleteDeviceModelConfigurationByConfigurationNamePrefix(string configurationNamePrefix);

Task RollOutEdgeModelConfiguration(IoTEdgeModel edgeModel);

Task RollOutDeviceConfiguration(string modelId, Dictionary<string, object> desiredProperties, string configurationId, Dictionary<string, string> targetTags, int priority = 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,25 @@ public async Task DeleteEnrollmentGroupAsync(EnrollmentGroup enrollmentGroup, Ca
await this.dps.DeleteEnrollmentGroupAsync(enrollmentGroup, cancellationToken);
}

public async Task DeleteEnrollmentGroupByDeviceModelIdAsync(string deviceModelId, CancellationToken cancellationToken = default)
{
var enrollmentGroupName = ComputeEnrollmentGroupName(deviceModelId);
EnrollmentGroup? enrollmentGroup;

try
{
enrollmentGroup = await this.dps.GetEnrollmentGroupAsync(enrollmentGroupName);
}
catch (HttpRequestException e)
when (e.StatusCode == System.Net.HttpStatusCode.NotFound)
{
// Nothing to do, the enrollement group does not exist.
return;
}

await this.dps.DeleteEnrollmentGroupAsync(enrollmentGroup, cancellationToken);
}

/// <summary>
/// this function get the attestation mechanism of the DPS.
/// </summary>
Expand Down
25 changes: 15 additions & 10 deletions src/AzureIoTHub.Portal.Server/Services/ConfigService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,27 @@ public async Task DeleteConfiguration(string configId)

public async Task RollOutDeviceModelConfiguration(string modelId, Dictionary<string, object> desiredProperties)
{
var configurations = await this.registryManager.GetConfigurationsAsync(0);

#pragma warning disable CA1308 // Normalize strings to uppercase
var configurationNamePrefix = modelId?.Trim()
.ToLowerInvariant()
.Replace(" ", "-", StringComparison.OrdinalIgnoreCase);
#pragma warning restore CA1308 // Normalize strings to uppercase

await DeleteDeviceModelConfigurationByConfigurationNamePrefix(configurationNamePrefix);

var newConfiguration = new Configuration($"{configurationNamePrefix}-{DateTime.UtcNow.Ticks}");

newConfiguration.Labels.Add("created-by", "Azure IoT hub Portal");
newConfiguration.TargetCondition = $"tags.modelId = '{modelId}'";
newConfiguration.Content.DeviceContent = desiredProperties;

_ = await this.registryManager.AddConfigurationAsync(newConfiguration);
}

public async Task DeleteDeviceModelConfigurationByConfigurationNamePrefix(string configurationNamePrefix)
{
var configurations = await this.registryManager.GetConfigurationsAsync(0);

foreach (var item in configurations)
{
if (!item.Id.StartsWith(configurationNamePrefix, StringComparison.OrdinalIgnoreCase))
Expand All @@ -219,14 +232,6 @@ public async Task RollOutDeviceModelConfiguration(string modelId, Dictionary<str

await this.registryManager.RemoveConfigurationAsync(item.Id);
}

var newConfiguration = new Configuration($"{configurationNamePrefix}-{DateTime.UtcNow.Ticks}");

newConfiguration.Labels.Add("created-by", "Azure IoT hub Portal");
newConfiguration.TargetCondition = $"tags.modelId = '{modelId}'";
newConfiguration.Content.DeviceContent = desiredProperties;

_ = await this.registryManager.AddConfigurationAsync(newConfiguration);
}

public async Task RollOutEdgeModelConfiguration(IoTEdgeModel edgeModel)
Expand Down
5 changes: 5 additions & 0 deletions src/AzureIoTHub.Portal.Server/Services/DeviceModelService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ public async Task DeleteDeviceModel(string deviceModelId)
$"The device model {deviceModelId} is already in use by a device and cannot be deleted");
}

// TODO : Delete DPS and Configurations
await this.deviceRegistryProvider.DeleteEnrollmentGroupByDeviceModelIdAsync(deviceModelId, default);

await this.configService.DeleteDeviceModelConfigurationByConfigurationNamePrefix(deviceModelId);

var deviceModelCommands = this.deviceModelCommandRepository.GetAll().Where(command =>
command.DeviceModelId.Equals(deviceModelId, StringComparison.Ordinal)).ToList();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ namespace AzureIoTHub.Portal.Tests.Unit.Client.Pages.Devices
using UnitTests.Mocks;
using AutoFixture;
using AzureIoTHub.Portal.Shared.Models.v10.Filters;
using AzureIoTHub.Portal.Shared.Models.v10;
using System.Linq;
using AzureIoTHub.Portal.Shared.Models;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

namespace AzureIoTHub.Portal.Tests.Unit.Infrastructure.Providers
{
using System;
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using AzureIoTHub.Portal.Application.Wrappers;
using AzureIoTHub.Portal.Infrastructure;
Expand Down Expand Up @@ -330,5 +332,51 @@ public async Task WhenEnrollmentGoupsNotExistGetEnrollmentCredentialsAsyncShould

this.mockRepository.VerifyAll();
}

[Test]
public async Task DeleteEnrollmentGroupByDeviceModelIdAsyncShouldDeleteEnrollmentGroup()
{
// Arrange
var manager = CreateManager();
var deviceModelId = Guid.NewGuid().ToString();
var attestation = new SymmetricKeyAttestation("aaa", "bbb");

_ = this.mockProvisioningServiceClient
.Setup(c => c.GetEnrollmentGroupAsync(It.Is<string>(x => x == deviceModelId)))
.ReturnsAsync(new EnrollmentGroup(deviceModelId, attestation)
{
Capabilities = new DeviceCapabilities
{
IotEdge = true
}
});

_ = this.mockProvisioningServiceClient.Setup(c => c.DeleteEnrollmentGroupAsync(It.IsAny<EnrollmentGroup>(), It.IsAny<CancellationToken>()))
.Returns(Task.CompletedTask);

// Act
await manager.DeleteEnrollmentGroupByDeviceModelIdAsync(deviceModelId);

// Assert
this.mockRepository.VerifyAll();
}

[Test]
public async Task DeleteEnrollmentGroupByDeviceModelIdAsyncShouldBeNotFound()
{
// Arrange
var manager = CreateManager();
var deviceModelId = Guid.NewGuid().ToString();

_ = this.mockProvisioningServiceClient
.Setup(c => c.GetEnrollmentGroupAsync(It.Is<string>(x => x == deviceModelId)))
.Throws(new HttpRequestException(null, null, HttpStatusCode.NotFound));

// Act
await manager.DeleteEnrollmentGroupByDeviceModelIdAsync(deviceModelId);

// Assert
this.mockRepository.VerifyAll();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -963,5 +963,31 @@ public void WhenAddGonfigurationFailedRollOutEdgeModelConfigurationShouldThrowIn
this.mockRepository.VerifyAll();
this.mockRegistryManager.Verify(c => c.GetConfigurationsAsync(It.IsAny<int>()), Times.Once());
}

[Test]
public async Task WhenConfigurationExistsDeleteDeviceModelConfigurationByConfigurationNamePrefixShouldRemoveIt()
{
// Arrange
var configsServices = CreateConfigsServices();
var configurationPrefix = Guid.NewGuid().ToString();
var suffix = Guid.NewGuid().ToString();

_ = this.mockRegistryManager.Setup(c => c.GetConfigurationsAsync(It.IsAny<int>()))
.ReturnsAsync(new Configuration[]
{
new Configuration($"{configurationPrefix}-{suffix}"),
new Configuration($"null-{suffix}")
});

_ = this.mockRegistryManager.Setup(c => c.RemoveConfigurationAsync(It.Is<string>(x => x == $"{configurationPrefix}-{suffix}")))
.Returns(Task.CompletedTask);

// Act
await configsServices.DeleteDeviceModelConfigurationByConfigurationNamePrefix(configurationPrefix);

// Assert
this.mockRegistryManager.Verify(c => c.GetConfigurationsAsync(It.IsAny<int>()), Times.Once());
this.mockRegistryManager.Verify(c => c.RemoveConfigurationAsync(It.IsAny<string>()), Times.Once());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,12 @@ public async Task DeleteDeviceModelShouldDeleteDeviceModel()
Items = Array.Empty<Twin>()
});

_ = this.mockRegistryProvider.Setup(repository => repository.DeleteEnrollmentGroupByDeviceModelIdAsync(It.IsAny<string>(), It.IsAny<CancellationToken>()))
.Returns(Task.CompletedTask);

_ = this.mockConfigService.Setup(repository => repository.DeleteDeviceModelConfigurationByConfigurationNamePrefix(It.IsAny<string>()))
.Returns(Task.CompletedTask);

_ = this.mockDeviceModelCommandRepository.Setup(repository => repository.GetAll())
.Returns(commands);

Expand Down Expand Up @@ -399,6 +405,12 @@ public async Task DeleteDeviceModelShouldThrowAnErrorExceptionWhenDDbUpdateExcep
Items = Array.Empty<Twin>()
});

_ = this.mockRegistryProvider.Setup(repository => repository.DeleteEnrollmentGroupByDeviceModelIdAsync(It.IsAny<string>(), It.IsAny<CancellationToken>()))
.Returns(Task.CompletedTask);

_ = this.mockConfigService.Setup(repository => repository.DeleteDeviceModelConfigurationByConfigurationNamePrefix(It.IsAny<string>()))
.Returns(Task.CompletedTask);

_ = this.mockDeviceModelCommandRepository.Setup(repository => repository.GetAll())
.Returns(commands);

Expand Down

0 comments on commit 4323ea0

Please sign in to comment.