Skip to content

Commit

Permalink
Remove some codesmells
Browse files Browse the repository at this point in the history
  • Loading branch information
tjololo committed Aug 8, 2023
1 parent 4e3c89e commit 86a7377
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 42 deletions.
32 changes: 16 additions & 16 deletions src/Altinn.App.Api/Controllers/ProcessController.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#nullable enable
using System.Net;
using Altinn.App.Api.Infrastructure.Filters;
using Altinn.App.Api.Models;
using Altinn.App.Core.Features.Validation;
using Altinn.App.Core.Helpers;
using Altinn.App.Core.Internal.Auth;
using Altinn.App.Core.Internal.Instances;
using Altinn.App.Core.Internal.Process;
using Altinn.App.Core.Internal.Process.Elements;
Expand Down Expand Up @@ -115,9 +115,9 @@ public async Task<ActionResult<AppProcessState>> StartProcess(
[FromRoute] string app,
[FromRoute] int instanceOwnerPartyId,
[FromRoute] Guid instanceGuid,
[FromQuery] string startEvent = null)
[FromQuery] string? startEvent = null)
{
Instance instance = null;
Instance? instance = null;

try
{
Expand Down Expand Up @@ -170,8 +170,8 @@ public async Task<ActionResult<List<string>>> GetNextElements(
[FromRoute] int instanceOwnerPartyId,
[FromRoute] Guid instanceGuid)
{
Instance instance = null;
string currentTaskId = null;
Instance? instance = null;
string? currentTaskId = null;

try
{
Expand Down Expand Up @@ -244,8 +244,8 @@ public async Task<ActionResult<AppProcessState>> NextElement(
[FromRoute] string app,
[FromRoute] int instanceOwnerPartyId,
[FromRoute] Guid instanceGuid,
[FromQuery] string elementId = null,
[FromQuery] string lang = null)
[FromQuery] string? elementId = null,
[FromQuery] string? lang = null)
{
try
{
Expand All @@ -267,16 +267,16 @@ public async Task<ActionResult<AppProcessState>> NextElement(
return Conflict($"Process is ended.");
}

string altinnTaskType = instance.Process.CurrentTask?.AltinnTaskType;
string? altinnTaskType = instance?.Process?.CurrentTask?.AltinnTaskType;

if (altinnTaskType == null)
{
return Conflict($"Instance does not have current altinn task type information!");
}

bool authorized;
string checkedAction = EnsureActionNotTaskType(processNext?.Action ?? altinnTaskType);
authorized = await AuthorizeAction(checkedAction, org, app, instanceOwnerPartyId, instanceGuid, instance.Process.CurrentTask?.ElementId);
string? checkedAction = EnsureActionNotTaskType(processNext?.Action ?? altinnTaskType);
authorized = await AuthorizeAction(checkedAction, org, app, instanceOwnerPartyId, instanceGuid, instance?.Process?.CurrentTask?.ElementId);

if (!authorized)
{
Expand Down Expand Up @@ -370,9 +370,9 @@ public async Task<ActionResult<AppProcessState>> CompleteProcess(
int counter = 0;
do
{
string altinnTaskType = EnsureActionNotTaskType(instance.Process.CurrentTask?.AltinnTaskType);
string? altinnTaskType = EnsureActionNotTaskType(instance?.Process?.CurrentTask?.AltinnTaskType);

bool authorized = await AuthorizeAction(altinnTaskType, org, app, instanceOwnerPartyId, instanceGuid, instance.Process.CurrentTask?.ElementId);
bool authorized = await AuthorizeAction(altinnTaskType, org, app, instanceOwnerPartyId, instanceGuid, instance?.Process?.CurrentTask?.ElementId);
if (!authorized)
{
return Forbid();
Expand Down Expand Up @@ -413,7 +413,7 @@ public async Task<ActionResult<AppProcessState>> CompleteProcess(

counter++;
}
while (instance.Process.EndEvent == null || counter > MaxIterationsAllowed);
while (instance?.Process?.EndEvent == null || counter > MaxIterationsAllowed);

if (counter > MaxIterationsAllowed)
{
Expand Down Expand Up @@ -491,12 +491,12 @@ private ActionResult ExceptionResponse(Exception exception, string message)
return StatusCode(500, $"{message}");
}

private async Task<bool> AuthorizeAction(string action, string org, string app, int instanceOwnerPartyId, Guid instanceGuid, string taskId = null)
private async Task<bool> AuthorizeAction(string action, string org, string app, int instanceOwnerPartyId, Guid instanceGuid, string? taskId = null)
{
return await _authorization.AuthorizeAction(new AppIdentifier(org, app), new InstanceIdentifier(instanceOwnerPartyId, instanceGuid), HttpContext.User, action, taskId);
}

private static string EnsureActionNotTaskType(string actionOrTaskType)
private static string? EnsureActionNotTaskType(string? actionOrTaskType)
{
switch (actionOrTaskType)
{
Expand Down Expand Up @@ -531,7 +531,7 @@ private ActionResult HandlePlatformHttpException(PlatformHttpException e, string
return ExceptionResponse(e, defaultMessage);
}

private static async Task<T> DeserializeFromStream<T>(Stream stream)
private static async Task<T?> DeserializeFromStream<T>(Stream stream)
{
using StreamReader reader = new StreamReader(stream);
string text = await reader.ReadToEndAsync();
Expand Down
1 change: 1 addition & 0 deletions src/Altinn.App.Api/Models/ProcessNext.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#nullable enable
using System.Text.Json.Serialization;

namespace Altinn.App.Api.Models;
Expand Down
10 changes: 4 additions & 6 deletions src/Altinn.App.Core/Features/Action/SigningUserAction.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using Altinn.App.Core.Internal.App;
using Altinn.App.Core.Internal.Data;
using Altinn.App.Core.Internal.Process;
using Altinn.App.Core.Internal.Process.Elements;
using Altinn.App.Core.Internal.Profile;
Expand All @@ -19,7 +18,6 @@ public class SigningUserAction: IUserAction
{
private readonly IProcessReader _processReader;
private readonly ILogger<SigningUserAction> _logger;
private readonly IAppMetadata _appMetadata;
private readonly IProfileClient _profileClient;
private readonly ISignClient _signClient;

Expand All @@ -28,11 +26,11 @@ public class SigningUserAction: IUserAction
/// </summary>
/// <param name="processReader">The process reader</param>
/// <param name="logger">The logger</param>
/// <param name="appMetadata">The application metadata service</param>
public SigningUserAction(IProcessReader processReader, ILogger<SigningUserAction> logger, IAppMetadata appMetadata, IProfileClient profileClient, ISignClient signClient)
/// <param name="profileClient">The profile client</param>
/// <param name="signClient">The sign client</param>
public SigningUserAction(IProcessReader processReader, ILogger<SigningUserAction> logger, IProfileClient profileClient, ISignClient signClient)
{
_logger = logger;
_appMetadata = appMetadata;
_profileClient = profileClient;
_signClient = signClient;
_processReader = processReader;
Expand Down Expand Up @@ -64,7 +62,7 @@ public async Task<bool> HandleAction(UserActionContext context)
return false;
}

private List<DataElementSignature> GetDataElementSignatures(List<DataElement> dataElements, List<string> dataTypesToSign)
private static List<DataElementSignature> GetDataElementSignatures(List<DataElement> dataElements, List<string> dataTypesToSign)
{
var connectedDataElements = new List<DataElementSignature>();
foreach (var dataType in dataTypesToSign)
Expand Down
11 changes: 11 additions & 0 deletions src/Altinn.App.Core/Features/IUserAction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,20 @@

namespace Altinn.App.Core.Features;

/// <summary>
/// Interface for implementing custom code for user actions
/// </summary>
public interface IUserAction
{
/// <summary>
/// The id of the user action
/// </summary>
string Id { get; }

/// <summary>
/// Method for handling the user action
/// </summary>
/// <param name="context">The user action context</param>
/// <returns>If the handling of the action was a success</returns>
Task<bool> HandleAction(UserActionContext context);
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ public class AltinnPartyClient : IAltinnPartyClient
/// Initializes a new instance of the <see cref="AltinnPartyClient"/> class
/// </summary>
/// <param name="platformSettings">The current platform settings.</param>
/// <param name="dsf">The dsf</param>
/// <param name="organizationClient">The organizationClient</param>
/// <param name="logger">The logger</param>
/// <param name="httpContextAccessor">The http context accessor </param>
/// <param name="settings">The application settings.</param>
Expand All @@ -43,7 +41,6 @@ public class AltinnPartyClient : IAltinnPartyClient
/// <param name="accessTokenGenerator">The platform access token generator</param>
public AltinnPartyClient(
IOptions<PlatformSettings> platformSettings,
IOrganizationClient organizationClient,
ILogger<AltinnPartyClient> logger,
IHttpContextAccessor httpContextAccessor,
IOptionsMonitor<AppSettings> settings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private async Task<LayoutEvaluatorState> GetLayoutEvaluatorState(Instance instan
return state;
}

private bool EvaluateSequenceFlow(LayoutEvaluatorState state, SequenceFlow sequenceFlow)
private static bool EvaluateSequenceFlow(LayoutEvaluatorState state, SequenceFlow sequenceFlow)
{
if (sequenceFlow.ConditionExpression != null)
{
Expand Down Expand Up @@ -123,8 +123,8 @@ private static Expression GetExpressionFromCondition(string condition)
LayoutSet? layoutSet = null;
if (!string.IsNullOrEmpty(layoutSetsString))
{
LayoutSets? layoutSets = JsonSerializer.Deserialize<LayoutSets>(layoutSetsString, options)!;
layoutSet = layoutSets?.Sets?.FirstOrDefault(t => t.Tasks.Contains(taskId));
LayoutSets? layoutSets = JsonSerializer.Deserialize<LayoutSets>(layoutSetsString, options);
layoutSet = layoutSets?.Sets?.Find(t => t.Tasks.Contains(taskId));
}

return layoutSet;
Expand All @@ -136,15 +136,15 @@ private static Expression GetExpressionFromCondition(string condition)
DataType? dataType;
if (dataTypeId != null)
{
dataType = (await _appMetadata.GetApplicationMetadata()).DataTypes.FirstOrDefault(d => d.Id == dataTypeId && d.AppLogic != null);
dataType = (await _appMetadata.GetApplicationMetadata()).DataTypes.Find(d => d.Id == dataTypeId && d.AppLogic != null);
}
else if (layoutSet != null)
{
dataType = (await _appMetadata.GetApplicationMetadata()).DataTypes.FirstOrDefault(d => d.Id == layoutSet.DataType && d.AppLogic != null);
dataType = (await _appMetadata.GetApplicationMetadata()).DataTypes.Find(d => d.Id == layoutSet.DataType && d.AppLogic != null);
}
else
{
dataType = (await _appMetadata.GetApplicationMetadata()).DataTypes.FirstOrDefault(d => d.TaskId == instance.Process.CurrentTask.ElementId && d.AppLogic != null);
dataType = (await _appMetadata.GetApplicationMetadata()).DataTypes.Find(d => d.TaskId == instance.Process.CurrentTask.ElementId && d.AppLogic != null);
}

if (dataType != null)
Expand All @@ -157,7 +157,7 @@ private static Expression GetExpressionFromCondition(string condition)

private static Guid? GetDataId(Instance instance, string dataType)
{
string? dataId = instance.Data.FirstOrDefault(d => d.DataType == dataType)?.Id;
string? dataId = instance.Data.Find(d => d.DataType == dataType)?.Id;
if (dataId != null)
{
return new Guid(dataId);
Expand Down
4 changes: 2 additions & 2 deletions src/Altinn.App.Core/Internal/Process/ProcessEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ public async Task<ProcessChangeResult> StartProcess(ProcessStartRequest processS

// start process
ProcessStateChange? startChange = await ProcessStart(processStartRequest.Instance, validStartElement!, processStartRequest.User);
InstanceEvent? startEvent = startChange?.Events?.First().CopyValues();
InstanceEvent? startEvent = startChange?.Events?[0].CopyValues();
ProcessStateChange? nextChange = await ProcessNext(processStartRequest.Instance, processStartRequest.User);
InstanceEvent? goToNextEvent = nextChange?.Events?.First().CopyValues();
InstanceEvent? goToNextEvent = nextChange?.Events?[0].CopyValues();
List<InstanceEvent> events = new List<InstanceEvent>();
if (startEvent is not null)
{
Expand Down
14 changes: 14 additions & 0 deletions src/Altinn.App.Core/Models/UserAction/UserActionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,29 @@

namespace Altinn.App.Core.Models.UserAction;

/// <summary>
/// Context for user actions
/// </summary>
public class UserActionContext
{
/// <summary>
/// Creates a new instance of the <see cref="UserActionContext"/> class
/// </summary>
/// <param name="instance">The instance the action is performed on</param>
/// <param name="userId">The user performing the action</param>
public UserActionContext(Instance instance, int userId)
{
Instance = instance;
UserId = userId;
}

/// <summary>
/// The instance the action is performed on
/// </summary>
public Instance Instance { get; }

/// <summary>
/// The user performing the action
/// </summary>
public int UserId { get; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,7 @@ public async void HandleAction_throws_ApplicationConfigException_if_SignatureDat
private static (SigningUserAction SigningUserAction, Mock<ISignClient> SignClientMock) CreateSigningUserAction(UserProfile userProfileToReturn = null, PlatformHttpException platformHttpExceptionToThrow = null, string testBpmnfilename = "signing-task-process.bpmn")
{
IProcessReader processReader = ProcessTestUtils.SetupProcessReader(testBpmnfilename, Path.Combine("Features", "Action", "TestData"));
AppSettings appSettings = new AppSettings()
{
AppBasePath = Path.Combine("Features", "Action"),
ConfigurationFolder = "TestData",
ApplicationMetadataFileName = "appmetadata.json"
};

IAppMetadata appMetadata = new AppMetadata(Options.Create<AppSettings>(appSettings), new FrontendFeatures(new Mock<IFeatureManager>().Object));
var profileClientMock = new Mock<IProfileClient>();
var signingClientMock = new Mock<ISignClient>();
profileClientMock.Setup(p => p.GetUserProfile(It.IsAny<int>())).ReturnsAsync(userProfileToReturn);
Expand All @@ -127,7 +120,7 @@ private static (SigningUserAction SigningUserAction, Mock<ISignClient> SignClien
signingClientMock.Setup(p => p.SignDataElements(It.IsAny<SignatureContext>())).ThrowsAsync(platformHttpExceptionToThrow);
}

return (new SigningUserAction(processReader, new NullLogger<SigningUserAction>(), appMetadata, profileClientMock.Object, signingClientMock.Object), signingClientMock);
return (new SigningUserAction(processReader, new NullLogger<SigningUserAction>(), profileClientMock.Object, signingClientMock.Object), signingClientMock);
}

private bool AssertSigningContextAsExpected(SignatureContext s1, SignatureContext s2)
Expand Down

0 comments on commit 86a7377

Please sign in to comment.