Skip to content

Commit

Permalink
Fix CodeQL analysis alerts (#205)
Browse files Browse the repository at this point in the history 
* Fix Dereferenced variable may be null

* Fix warnings in GatewaysController

* Fix analysis warnings in ConfigsController

* Fix some notes

* Remove generic catch clauses

* Remove ineficient use of ContainsKey

* Fix Generic catch clause issue

* Combine Nested 'if' statements

* Fix Missed ternary opportunity

* Fix Nested 'if' statements

* Use select

* Fix Missed opportunity to use Where

* Fix Missed ternary opportunity

* Fix Missed opportunity to use Where

* Fix Missed opportunity to use Select

* Fix some notes
  • Loading branch information
@kbeaugrand
kbeaugrand authored Feb 6, 2022
1 parent 233066b commit ddec3d4
Show file tree
Hide file tree
Showing 10 changed files with 189 additions and 315 deletions.
25 changes: 9 additions & 16 deletions src/AzureIoTHub.Portal/Server/Controllers/CommandsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,16 @@ public CommandsController(
[HttpPost("{modelId}")]
public async Task<IActionResult> Post(string modelId, DeviceModelCommand command)
{
try
TableEntity entity = new TableEntity()
{
TableEntity entity = new TableEntity()
{
PartitionKey = modelId,
RowKey = command.Name
};
this.deviceModelCommandMapper.UpdateTableEntity(entity, command);
await this.tableClientFactory
.GetDeviceCommands()
.AddEntityAsync(entity);
return this.Ok("Command successfully added");
}
catch (Exception e)
{
return this.BadRequest(e.Message);
}
PartitionKey = modelId,
RowKey = command.Name
};
this.deviceModelCommandMapper.UpdateTableEntity(entity, command);
await this.tableClientFactory
.GetDeviceCommands()
.AddEntityAsync(entity);
return this.Ok("Command successfully added");
}

/// <summary>
Expand Down
85 changes: 36 additions & 49 deletions src/AzureIoTHub.Portal/Server/Controllers/ConfigsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

namespace AzureIoTHub.Portal.Server.Controllers
{
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using AzureIoTHub.Portal.Server.Helpers;
using AzureIoTHub.Portal.Server.Services;
Expand All @@ -17,18 +19,10 @@ namespace AzureIoTHub.Portal.Server.Controllers
[Route("api/[controller]")]
public class ConfigsController : ControllerBase
{
private readonly ILogger<ConfigsController> logger;

private readonly RegistryManager registryManager;
private readonly ConfigsServices configService;

public ConfigsController(
ILogger<ConfigsController> logger,
ConfigsServices configService,
RegistryManager registryManager)
public ConfigsController(ConfigsServices configService)
{
this.logger = logger;
this.registryManager = registryManager;
this.configService = configService;
}

Expand All @@ -42,26 +36,21 @@ public async Task<IEnumerable<ConfigListItem>> Get()
// Retrieve every Configurations, regardless of the parameter given... Why?
// TODO : Check & fix this
// List<Configuration> configList = await this.registryManager.GetConfigurationsAsync(0) as List<Configuration>;
List<Configuration> configList = await this.configService.GetAllConfigs() as List<Configuration>;
var configList = await this.configService.GetAllConfigs();
var results = new List<ConfigListItem>();

// Azure Configurations may have different types: "Configuration", "Deployment" or "LayeredDeployment"
foreach (Configuration config in configList)
{
List<GatewayModule> moduleList = new ();
var moduleList = new List<GatewayModule>();

// Only deployments have modules. If it doesn't, it's a configuration and we don't want to keep it.
if (config.Content.ModulesContent != null)
{
// Retrieve the name of each module of this deployment
foreach (var module in config.Content.ModulesContent)
moduleList.AddRange(config.Content.ModulesContent.Select(x => new GatewayModule
{
var newModule = new GatewayModule
{
ModuleName = module.Key
};
moduleList.Add(newModule);
}
ModuleName = x.Key
}));

ConfigListItem result = ConfigHelper.CreateConfigListItem(config, moduleList);
results.Add(result);
Expand All @@ -81,40 +70,38 @@ public async Task<IEnumerable<ConfigListItem>> Get()
public async Task<ConfigListItem> Get(string configurationID)
{
var config = await this.configService.GetConfigItem(configurationID);
List<GatewayModule> moduleList = new ();
if (config.Content.ModulesContent != null)
var moduleList = new List<GatewayModule>();

// Details of every modules are stored within the EdgeAgent module data
if (config.Content.ModulesContent != null
&& config.Content.ModulesContent.TryGetValue("$edgeAgent", out IDictionary<string, object> edgeAgentModule)
&& edgeAgentModule.TryGetValue("properties.desired", out object edgeAgentDesiredProperties))
{
// Details of every modules are stored within the EdgeAgent module data
if (config.Content.ModulesContent.ContainsKey("$edgeAgent"))
// Converts the object to a JObject to access its properties more easily
JObject modObject = edgeAgentDesiredProperties as JObject;

if (modObject == null)
{
if (config.Content.ModulesContent["$edgeAgent"].ContainsKey("properties.desired"))
{
// Converts the object to a JObject to access its properties more easily
JObject modObject = config.Content.ModulesContent["$edgeAgent"]["properties.desired"] as JObject;
throw new InvalidOperationException("Could not parse properties.desired.");
}

// Adds regular modules to the list of modules
if (modObject.ContainsKey("modules"))
{
// Converts it to a JObject to be able to iterate through it
JObject modules = modObject["modules"] as JObject;
foreach (var m in modules)
{
GatewayModule newModule = ConfigHelper.CreateGatewayModule(config, m);
moduleList.Add(newModule);
}
}
// Adds regular modules to the list of modules
if (modObject.TryGetValue("modules", out JToken modules))
{
foreach (var m in modules.Values<JProperty>())
{
GatewayModule newModule = ConfigHelper.CreateGatewayModule(config, m);
moduleList.Add(newModule);
}
}

// Adds system modules to the list of modules
if (modObject.ContainsKey("systemModules"))
{
// Converts it to a JObject to be able to iterate through it
JObject systemModules = modObject["systemModules"] as JObject;
foreach (var sm in systemModules)
{
GatewayModule newModule = ConfigHelper.CreateGatewayModule(config, sm);
moduleList.Add(newModule);
}
}
// Adds system modules to the list of modules
if (modObject.TryGetValue("systemModules", out JToken systemModulesToken))
{
foreach (var sm in systemModulesToken.Values<JProperty>())
{
GatewayModule newModule = ConfigHelper.CreateGatewayModule(config, sm);
moduleList.Add(newModule);
}
}
}
Expand Down
24 changes: 7 additions & 17 deletions src/AzureIoTHub.Portal/Server/Controllers/DeviceModelsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,24 +148,17 @@ public async Task<IActionResult> Delete(string deviceModelID)
.GetDeviceCommands()
.Query<TableEntity>(t => t.PartitionKey == deviceModel.ModelId);

foreach (var twin in deviceList)
if (deviceList.Any(x => DeviceHelper.RetrieveTagValue(x, "modelId") == deviceModel.ModelId))
{
if (DeviceHelper.RetrieveTagValue(twin, "modelId") == deviceModel.ModelId)
{
return this.Unauthorized("This model is already in use by a device and cannot be deleted.");
}
return this.Unauthorized("This model is already in use by a device and cannot be deleted.");
}

var commands = queryCommand.Select(item => this.deviceModelCommandMapper.GetDeviceModelCommand(item)).ToList();

// if we have command
if (commands.Count > 0)
foreach (var item in commands)
{
foreach (var item in commands)
{
_ = await this.tableClientFactory
.GetDeviceCommands().DeleteEntityAsync(deviceModelID, item.Name);
}
_ = await this.tableClientFactory
.GetDeviceCommands().DeleteEntityAsync(deviceModelID, item.Name);
}

// Image deletion
Expand All @@ -192,12 +185,9 @@ await this.tableClientFactory

await foreach (var page in commandsPage)
{
foreach (var item in page.Values)
foreach (var item in page.Values.Where(x => !deviceModelObject.Commands.Any(c => c.Name == x.RowKey)))
{
if (!deviceModelObject.Commands.Any(c => c.Name == item.RowKey))
{
await commandsTable.DeleteEntityAsync(item.PartitionKey, item.RowKey);
}
await commandsTable.DeleteEntityAsync(item.PartitionKey, item.RowKey);
}
}

Expand Down
44 changes: 12 additions & 32 deletions src/AzureIoTHub.Portal/Server/Controllers/DevicesController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,29 +121,21 @@ public async Task<IActionResult> CreateDeviceAsync(DeviceDetails device)
[HttpPut]
public async Task<IActionResult> UpdateDeviceAsync(DeviceDetails device)
{
try
{
// Device status (enabled/disabled) has to be dealt with afterwards
Device currentDevice = await this.devicesService.GetDevice(device.DeviceID);
currentDevice.Status = device.IsEnabled ? DeviceStatus.Enabled : DeviceStatus.Disabled;
// Device status (enabled/disabled) has to be dealt with afterwards
Device currentDevice = await this.devicesService.GetDevice(device.DeviceID);
currentDevice.Status = device.IsEnabled ? DeviceStatus.Enabled : DeviceStatus.Disabled;

_ = await this.devicesService.UpdateDevice(currentDevice);
_ = await this.devicesService.UpdateDevice(currentDevice);

// Get the current twin from the hub, based on the device ID
Twin currentTwin = await this.devicesService.GetDeviceTwin(device.DeviceID);
// Get the current twin from the hub, based on the device ID
Twin currentTwin = await this.devicesService.GetDeviceTwin(device.DeviceID);

// Update the twin properties
this.deviceTwinMapper.UpdateTwin(currentTwin, device);
// Update the twin properties
this.deviceTwinMapper.UpdateTwin(currentTwin, device);

_ = await this.devicesService.UpdateDeviceTwin(device.DeviceID, currentTwin);
_ = await this.devicesService.UpdateDeviceTwin(device.DeviceID, currentTwin);

return this.Ok();
}
catch (Exception e)
{
this.logger.LogError($"{device.DeviceID} - Update device failed", e);
return this.BadRequest();
}
return this.Ok();
}

/// <summary>
Expand All @@ -154,16 +146,8 @@ public async Task<IActionResult> UpdateDeviceAsync(DeviceDetails device)
[HttpDelete("{deviceID}")]
public async Task<IActionResult> Delete(string deviceID)
{
try
{
await this.devicesService.DeleteDevice(deviceID);
return this.Ok();
}
catch (Exception e)
{
this.logger.LogError($"{deviceID} - Device deletion failed", e);
return this.BadRequest();
}
await this.devicesService.DeleteDevice(deviceID);
return this.Ok();
}

/// <summary>
Expand Down Expand Up @@ -202,10 +186,6 @@ public async Task<IActionResult> ExecuteCommand(string deviceId, string commandI

return this.BadRequest("Something went wrong when executing the command.");
}
catch (Exception e)
{
return this.BadRequest(e.Message);
}
}
}
}
Loading

0 comments on commit ddec3d4

Please sign in to comment.