Skip to content

Commit

Permalink
Multipart form data (#329)
Browse files Browse the repository at this point in the history
* Let ModelDeserializer return a Result type instead of nullable

* Remove NullDataProcessor and let user registrer multiple processors

* Update IDataProcessor interface and support multipart data write

* Update v7tov8 script for new IDataProcessor interface

Also ignore CS1998 in apps (warning when not awaiting anything in async hooks)

* Code review updates

* Update src/Altinn.App.Core/Helpers/Serialization/ModelDeserializer.cs

Co-authored-by: Vemund Gaukstad <[email protected]>

* First draft of tests for controllers

* Finish writing tests

* Set partyId in token to null

* Fix bad case for roles folder in testsetup

* Add partyId correctly in test tokens

Also add a Dockerfile for running tests on linux

* Use proper parsing library to decode multipart/form-data requests

* Continue returning changed only changed values from data put.

* Fix so that it compiles (still failing tests)

* Fix tests by disabeling redirects in applciationfactory client

For some reason backend returns 303 See Other when datamodel updates are availible.
This is a redirect code and crashed the tests as no Location header was set.

* Add more tests

* More tests

* Update src/Altinn.App.Api/Controllers/DataController.cs

Co-authored-by: Vemund Gaukstad <[email protected]>

* Fix code smells

* Fix tests

* Tests OK now?

* More tests for model deserializer

---------

Co-authored-by: Vemund Gaukstad <[email protected]>
  • Loading branch information
ivarne and tjololo authored Dec 13, 2023
1 parent 7cc841e commit db957c0
Show file tree
Hide file tree
Showing 22 changed files with 1,028 additions and 224 deletions.
10 changes: 9 additions & 1 deletion cli-tools/altinn-app-cli/Program.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.CommandLine;
using System.CommandLine;
using System.CommandLine.Invocation;
using System.Reflection;
using altinn_app_cli.v7Tov8.AppSettingsRewriter;
Expand Down Expand Up @@ -206,12 +206,20 @@ static async Task<int> UpgradeCode(string projectFile)
{
await File.WriteAllTextAsync(sourceTree.FilePath, newSource.ToFullString());
}

UsingRewriter usingRewriter = new();
var newUsingSource = usingRewriter.Visit(newSource);
if (newUsingSource != newSource)
{
await File.WriteAllTextAsync(sourceTree.FilePath, newUsingSource.ToFullString());
}

DataProcessorRewriter dataProcessorRewriter = new(sm);
var dataProcessorSource = dataProcessorRewriter.Visit(newUsingSource);
if (dataProcessorSource != newUsingSource)
{
await File.WriteAllTextAsync(sourceTree.FilePath, dataProcessorSource.ToFullString());
}
}

Console.WriteLine("References and using upgraded");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace altinn_app_cli.v7Tov8.CodeRewriters
{
public class DataProcessorRewriter : CSharpSyntaxRewriter
{
private readonly SemanticModel semanticModel;

public DataProcessorRewriter(SemanticModel semanticModel)
{
this.semanticModel = semanticModel;
}

public override SyntaxNode? VisitClassDeclaration(ClassDeclarationSyntax node)
{
// Ignore any classes that don't implement `IDataProcessor` (consider using semantic model to ensure correct reference)
if (node.BaseList?.Types.Any(t => t.Type.ToString() == "IDataProcessor") == true)
{
var processDataWrite = node.Members.OfType<MethodDeclarationSyntax>()
.FirstOrDefault(m => m.Identifier.ValueText == "ProcessDataWrite");
if (processDataWrite is not null)
{
node = node.ReplaceNode(processDataWrite, Update_DataProcessWrite(processDataWrite));
}

var processDataRead = node.Members.OfType<MethodDeclarationSyntax>()
.FirstOrDefault(m => m.Identifier.ValueText == "ProcessDataRead");
if (processDataRead is not null)
{
node = node.ReplaceNode(processDataRead, Update_DataProcessRead(processDataRead));
}
}

return base.VisitClassDeclaration(node);
}

private MethodDeclarationSyntax Update_DataProcessRead(MethodDeclarationSyntax processDataRead)
{
if (processDataRead.ParameterList.Parameters.Count == 3 &&
processDataRead.ReturnType.ToString() == "Task<bool>")
{
processDataRead = ChangeReturnType_FromTaskBool_ToTask(processDataRead);
}

return processDataRead;
}

private MethodDeclarationSyntax Update_DataProcessWrite(MethodDeclarationSyntax processDataWrite)
{
if (processDataWrite.ParameterList.Parameters.Count == 3 &&
processDataWrite.ReturnType.ToString() == "Task<bool>")
{
processDataWrite = AddParameter_ChangedFields(processDataWrite);
processDataWrite = ChangeReturnType_FromTaskBool_ToTask(processDataWrite);
}

return processDataWrite;
}

private MethodDeclarationSyntax AddParameter_ChangedFields(MethodDeclarationSyntax method)
{
return method.ReplaceNode(method.ParameterList,
method.ParameterList.AddParameters(SyntaxFactory.Parameter(SyntaxFactory.Identifier("changedFields"))
.WithLeadingTrivia(SyntaxFactory.Space)
.WithType(SyntaxFactory.ParseTypeName("System.Collections.Generic.Dictionary<string, string?>?"))
.WithLeadingTrivia(SyntaxFactory.Space)));
}

private MethodDeclarationSyntax ChangeReturnType_FromTaskBool_ToTask(MethodDeclarationSyntax method)
{
if (method.ReturnType.ToString() == "Task<bool>")
{
var returnTypeRewriter = new ReturnTypeTaskBooleanRewriter();
method = (MethodDeclarationSyntax)returnTypeRewriter.Visit(method)!;
}

return method;

}
}

public class ReturnTypeTaskBooleanRewriter : CSharpSyntaxRewriter
{
public override SyntaxNode? VisitMethodDeclaration(MethodDeclarationSyntax node)
{
if (node.ReturnType.ToString() == "Task<bool>")
{
// Change return type
node = node.WithReturnType(
SyntaxFactory.ParseTypeName("Task").WithTrailingTrivia(SyntaxFactory.Space));
}
return base.VisitMethodDeclaration(node);
}

public override SyntaxNode? VisitBlock(BlockSyntax node)
{
foreach (var returnStatementSyntax in node.Statements.OfType<ReturnStatementSyntax>())
{
var leadingTrivia = returnStatementSyntax.GetLeadingTrivia();
var trailingTrivia = returnStatementSyntax.GetTrailingTrivia();
// When we add multiple lines of code, we need the indentation and a newline
var leadingTriviaMiddle = leadingTrivia.LastOrDefault(t => t.IsKind(SyntaxKind.WhitespaceTrivia));
var trailingTriviaMiddle = trailingTrivia.FirstOrDefault(t => t.IsKind(SyntaxKind.EndOfLineTrivia));
// If we don't find a newline, just guess that LF is used. Will likely work anyway.
if (trailingTriviaMiddle == default) trailingTriviaMiddle = SyntaxFactory.LineFeed;


switch (returnStatementSyntax.Expression)
{
// return true/false/variableName
case IdentifierNameSyntax:
case LiteralExpressionSyntax:
case null:
node = node.ReplaceNode(returnStatementSyntax,
SyntaxFactory.ReturnStatement()
.WithLeadingTrivia(leadingTrivia).WithTrailingTrivia(trailingTrivia));
break;
// case "Task.FromResult(...)":
case InvocationExpressionSyntax
{
Expression: MemberAccessExpressionSyntax
{
Expression: IdentifierNameSyntax { Identifier: {Text: "Task" } },
Name: { Identifier: {Text: "FromResult"}}
},
ArgumentList: { Arguments: { Count: 1 } }
}:
node = node.ReplaceNode(returnStatementSyntax,
SyntaxFactory.ReturnStatement(SyntaxFactory.ParseExpression(" Task.CompletedTask"))
.WithLeadingTrivia(leadingTrivia).WithTrailingTrivia(trailingTrivia));
break;
// case "await Task.FromResult(...)":
// Assume we need an await to silence CS1998 and rewrite to
// await Task.CompletedTask; return;
// Could be dropped if we ignore CS1998
case AwaitExpressionSyntax
{
Expression: InvocationExpressionSyntax
{
Expression: MemberAccessExpressionSyntax
{
Expression: IdentifierNameSyntax { Identifier: {Text: "Task" } },
Name: { Identifier: {Text: "FromResult"}}
},
ArgumentList: { Arguments: [{Expression: IdentifierNameSyntax or LiteralExpressionSyntax}]}
}
}:
node = node.WithStatements(node.Statements.ReplaceRange(returnStatementSyntax, new StatementSyntax[]
{
// Uncomment if cs1998 isn't disabled
// SyntaxFactory.ParseStatement("await Task.CompletedTask;")
// .WithLeadingTrivia(leadingTrivia).WithTrailingTrivia(trailingTriviaMiddle),

SyntaxFactory.ReturnStatement()
.WithLeadingTrivia(leadingTriviaMiddle).WithTrailingTrivia(trailingTrivia),

}));
break;
// Just add move the return; statement after the existing return value
default:
node = node.WithStatements(node.Statements.ReplaceRange(returnStatementSyntax,
new StatementSyntax[]
{
SyntaxFactory.ExpressionStatement(returnStatementSyntax.Expression)
.WithLeadingTrivia(leadingTrivia).WithTrailingTrivia(trailingTriviaMiddle),

SyntaxFactory.ReturnStatement()
.WithLeadingTrivia(leadingTriviaMiddle).WithTrailingTrivia(trailingTrivia),
}));
break;
}
}

return base.VisitBlock(node);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,41 @@ public ProjectFileRewriter(string projectFilePath, string targetVersion = "8.0.0
public async Task Upgrade()
{
var altinnAppCoreElements = GetAltinnAppCoreElement();
altinnAppCoreElements?.ForEach(c => c.Attribute("Version")?.SetValue(targetVersion));

var altinnAppApiElements = GetAltinnAppApiElement();
if (altinnAppCoreElements != null && altinnAppApiElements != null)
{
altinnAppCoreElements.ForEach(c => c.Attribute("Version")?.SetValue(targetVersion));
altinnAppApiElements.ForEach(a => a.Attribute("Version")?.SetValue(targetVersion));
}

altinnAppApiElements?.ForEach(a => a.Attribute("Version")?.SetValue(targetVersion));

IgnoreWarnings("1591", "1998"); // Require xml doc and await in async methods

GetTargetFrameworkElement()?.ForEach(t => t.SetValue(targetFramework));

await Save();
}

private void IgnoreWarnings(params string[] warnings)
{
var noWarn = doc.Root?.Elements("PropertyGroup").Elements("NoWarn").ToList();
switch (noWarn?.Count)
{
case 0:
doc.Root?.Elements("PropertyGroup").First().Add(new XElement("NoWarn", "$(NoWarn);" + string.Join(';', warnings)));
break;

case 1:
var valueElement = noWarn.First();
foreach (var warning in warnings)
{
if (!valueElement.Value.Contains(warning))
{
valueElement.SetValue($"{valueElement.Value};{warning}");
}
}

break;
}
}

private List<XElement>? GetAltinnAppCoreElement()
{
return doc.Root?.Elements("ItemGroup").Elements("PackageReference").Where(x => x.Attribute("Include")?.Value == "Altinn.App.Core").ToList();
Expand Down
43 changes: 22 additions & 21 deletions src/Altinn.App.Api/Controllers/DataController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class DataController : ControllerBase
{
private readonly ILogger<DataController> _logger;
private readonly IDataClient _dataClient;
private readonly IDataProcessor _dataProcessor;
private readonly IEnumerable<IDataProcessor> _dataProcessors;
private readonly IInstanceClient _instanceClient;
private readonly IInstantiationProcessor _instantiationProcessor;
private readonly IAppModel _appModel;
Expand All @@ -58,7 +58,7 @@ public class DataController : ControllerBase
/// <param name="instanceClient">instance service to store instances</param>
/// <param name="instantiationProcessor">Instantiation processor</param>
/// <param name="dataClient">A service with access to data storage.</param>
/// <param name="dataProcessor">Serive implemnting logic during data read/write</param>
/// <param name="dataProcessors">Services implementing logic during data read/write</param>
/// <param name="appModel">Service for generating app model</param>
/// <param name="appResourcesService">The apps resource service</param>
/// <param name="appMetadata">The app metadata service</param>
Expand All @@ -71,7 +71,7 @@ public DataController(
IInstanceClient instanceClient,
IInstantiationProcessor instantiationProcessor,
IDataClient dataClient,
IDataProcessor dataProcessor,
IEnumerable<IDataProcessor> dataProcessors,
IAppModel appModel,
IAppResources appResourcesService,
IPrefill prefillService,
Expand All @@ -85,7 +85,7 @@ public DataController(
_instanceClient = instanceClient;
_instantiationProcessor = instantiationProcessor;
_dataClient = dataClient;
_dataProcessor = dataProcessor;
_dataProcessors = dataProcessors;
_appModel = appModel;
_appResourcesService = appResourcesService;
_appMetadata = appMetadata;
Expand Down Expand Up @@ -175,7 +175,7 @@ public async Task<ActionResult> Create(
_logger.LogError(errorMessage);
return BadRequest(await GetErrorDetails(new List<ValidationIssue> { error }));
}

bool parseSuccess = Request.Headers.TryGetValue("Content-Disposition", out StringValues headerValues);
string? filename = parseSuccess ? DataRestrictionValidation.GetFileNameFromHeader(headerValues) : null;

Expand Down Expand Up @@ -466,12 +466,14 @@ private async Task<ActionResult> CreateAppModelData(
else
{
ModelDeserializer deserializer = new ModelDeserializer(_logger, _appModel.GetModelType(classRef));
appModel = await deserializer.DeserializeAsync(Request.Body, Request.ContentType);
ModelDeserializerResult deserializerResult = await deserializer.DeserializeAsync(Request.Body, Request.ContentType);

if (!string.IsNullOrEmpty(deserializer.Error) || appModel is null)
if (deserializerResult.HasError)
{
return BadRequest(deserializer.Error);
return BadRequest(deserializerResult.Error);
}

appModel = deserializerResult.Model;
}

// runs prefill from repo configuration if config exists
Expand Down Expand Up @@ -582,7 +584,11 @@ private async Task<ActionResult> GetFormData(
return BadRequest($"Did not find form data for data element {dataGuid}");
}

await _dataProcessor.ProcessDataRead(instance, dataGuid, appModel);
foreach (var dataProcessor in _dataProcessors)
{
_logger.LogInformation("ProcessDataRead for {modelType} using {dataProcesor}", appModel.GetType().Name, dataProcessor.GetType().Name);
await dataProcessor.ProcessDataRead(instance, dataGuid, appModel);
}

string? userOrgClaim = User.GetOrg();
if (userOrgClaim == null || !org.Equals(userOrgClaim, StringComparison.InvariantCultureIgnoreCase))
Expand Down Expand Up @@ -616,26 +622,21 @@ private async Task<ActionResult> PutFormData(string org, string app, Instance in
Guid instanceGuid = Guid.Parse(instance.Id.Split("/")[1]);

ModelDeserializer deserializer = new ModelDeserializer(_logger, _appModel.GetModelType(classRef));
object? serviceModel = await deserializer.DeserializeAsync(Request.Body, Request.ContentType);

if (!string.IsNullOrEmpty(deserializer.Error))
{
return BadRequest(deserializer.Error);
}
ModelDeserializerResult deserializerResult = await deserializer.DeserializeAsync(Request.Body, Request.ContentType);

if (serviceModel == null)
if (deserializerResult.HasError)
{
return BadRequest("No data found in content");
return BadRequest(deserializerResult.Error);
}

Dictionary<string, object?>? changedFields = await JsonHelper.ProcessDataWriteWithDiff(instance, dataGuid, serviceModel, _dataProcessor, _logger);
Dictionary<string, object?>? changedFields = await JsonHelper.ProcessDataWriteWithDiff(instance, dataGuid, deserializerResult.Model, _dataProcessors, deserializerResult.ReportedChanges, _logger);

await UpdatePresentationTextsOnInstance(instance, dataType, serviceModel);
await UpdateDataValuesOnInstance(instance, dataType, serviceModel);
await UpdatePresentationTextsOnInstance(instance, dataType, deserializerResult.Model);
await UpdateDataValuesOnInstance(instance, dataType, deserializerResult.Model);

// Save Formdata to database
DataElement updatedDataElement = await _dataClient.UpdateData(
serviceModel,
deserializerResult.Model,
instanceGuid,
_appModel.GetModelType(classRef),
org,
Expand Down
8 changes: 5 additions & 3 deletions src/Altinn.App.Api/Controllers/InstancesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -971,13 +971,15 @@ private async Task StorePrefillParts(Instance instance, ApplicationMetadata appI
}

ModelDeserializer deserializer = new ModelDeserializer(_logger, type);
object? data = await deserializer.DeserializeAsync(part.Stream, part.ContentType);
ModelDeserializerResult deserializerResult = await deserializer.DeserializeAsync(part.Stream, part.ContentType);

if (!string.IsNullOrEmpty(deserializer.Error) || data is null)
if (deserializerResult.HasError)
{
throw new InvalidOperationException(deserializer.Error);
throw new InvalidOperationException(deserializerResult.Error);
}

object data = deserializerResult.Model;

await _prefillService.PrefillDataModel(instance.InstanceOwner.PartyId, part.Name!, data);

await _instantiationProcessor.DataCreation(instance, data, null);
Expand Down
Loading

0 comments on commit db957c0

Please sign in to comment.