Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/Altinn.App.Api/packaging/Altinn.App.Api.Experimental.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="utf-8"?>
<Project>
<!--
This props file is included in the Altinn.App.Api nuget package and will
apply to the App projects when they reference said nuget package.
-->
<PropertyGroup>
<IsAltinnApp>true</IsAltinnApp>
</PropertyGroup>
<ItemGroup>
<!--
This props file is included in the build/ path of the Altinn.App.Api nuget package
but the Rosly analyzer is included transitively, for example in unit test projects or similar.
We only need to do app-based analysis on actual app projects, so this let's us distinguish
in the Roslyn Analyzer code.
-->
<CompilerVisibleProperty Include="IsAltinnApp" />
</ItemGroup>
<ItemGroup>
<!--
We include config files as addition files so that they are more easily referenced
in analyzers and source generators.
-->
<AdditionalFiles Include="config/**" />
<AdditionalFiles Include="ui/**/*.json" />
</ItemGroup>
</Project>
5 changes: 5 additions & 0 deletions src/Altinn.App.Core/Configuration/AppSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@ public class AppSettings
/// </summary>
public bool ExpressionValidation { get; set; }

/// <summary>
/// Enable the functionality to validate form data against corresponding XSD if present
/// </summary>
public bool XsdValidation { get; set; }

/// <summary>
/// Enables OpenTelemetry as a substitute for Application Insights SDK
/// Improves instrumentation throughout the Altinn app libraries.
Expand Down
5 changes: 5 additions & 0 deletions src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ private static void AddValidationServices(IServiceCollection services, IConfigur
{
services.AddTransient<IValidator, ExpressionValidator>();
}

if (appSettings?.XsdValidation is true)
{
services.AddTransient<IValidator, XsdValidator>();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bør det være standard å ha både DataAnnotationValidator og XsdValidator, eller bør aktivering av XsdValidation være et alternativ til dataAnnotations?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jeg kan ikke si noe veldig konsekvent, men kan dele vår erfaring med disse typene validering.
Vi har opplevd at en god del innsendinger har blitt godkjent av DataAnnotationValidator, deretter kommet til backend som kjører xsd validering og klager på innsendingen. Så det virker som xsd validering er strengere enn DataAnnotationValidator. Vi kjører alltid xsd datamodeller gjennom altinn studio sitt generer datamodeller-verktøy.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jeg har også grublet litt over om xsd validering burde være standard. Jeg føler det finnes et sterkt argument for, og et sterkt argument mot.

For: siden datamodeller kan genereres ut fra Xsd, kan man ofte anta at hvis man har en Xsd med samme navn som en modell, i models mappa, så vil man gjerne validere dataen utfra den xsden. og hvis man utvikler og finner at det skaper problemer, kan man deretter slå av. Ref det jeg nevnte over, med at xsd validering virker som det er strengere. Dette hadde vært "least trust" prinsipp, som noen setter pris på.

Imot: Jeg vet om hvertfall ett prosjekt, som startet i utgangspunktet med datamodell generert ut fra Xsd, og deretter gikk over til å bare redigere .cs filene direkte når de gjorde endringer i modellene. Jeg vet ikke hvor vanlig dette er, men slike utviklere vil helst ikke ha denne funksjonaliteten som standard.

}
}

/// <summary>
Expand Down
120 changes: 120 additions & 0 deletions src/Altinn.App.Core/Features/Validation/Default/XsdValidator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
using System.Xml;
using System.Xml.Schema;
using Altinn.App.Core.Helpers;
using Altinn.App.Core.Helpers.Serialization;
using Altinn.App.Core.Internal.App;
using Altinn.App.Core.Models;
using Altinn.App.Core.Models.Validation;
using Microsoft.Extensions.Logging;

namespace Altinn.App.Core.Features.Validation.Default;

/// <summary>
/// Validates form data against the XSD schema for the data model, if it exists
/// </summary>
public class XsdValidator : IValidator
{
private readonly ILogger<XsdValidator> _logger;
private readonly IAppResources _appResourceService;
private readonly IAppMetadata _appMetadata;
private readonly ModelSerializationService _modelSerializationService;

/// <summary>
/// Constructor for the expression validator
/// </summary>
public XsdValidator(
ILogger<XsdValidator> logger,
IAppResources appResourceService,
IAppMetadata appMetadata,
ModelSerializationService modelSerializationService
)
{
_logger = logger;
_appResourceService = appResourceService;
_appMetadata = appMetadata;
_modelSerializationService = modelSerializationService;
}

/// <summary>
/// We implement <see cref="ShouldRunForTask"/> instead
/// </summary>
public string TaskId => "*";

/// <summary>
/// Only run for tasks that specifies a layout set
/// </summary>
public bool ShouldRunForTask(string taskId) =>
_appMetadata
.GetApplicationMetadata()
.Result.DataTypes.Exists(dt => dt.TaskId == taskId && dt.AppLogic?.ClassRef is not null);

/// <inheritdoc />
public string ValidationSource => "Xsd";

/// <inheritdoc />
public bool NoIncrementalValidation => true;

/// <summary>
/// This is not used for incremental validation
/// </summary>
public Task<bool> HasRelevantChanges(
IInstanceDataAccessor dataAccessor,
string taskId,
DataElementChanges changes
) => Task.FromResult(true);

/// <inheritdoc />
public async Task<List<ValidationIssue>> Validate(
IInstanceDataAccessor dataAccessor,
string taskId,
string? language
)
{
var validationIssues = new List<ValidationIssue>();
foreach (var (dataType, dataElement) in dataAccessor.GetDataElementsForTask(taskId))
{
var schema = _appResourceService.GetXsdSchema(dataType.Id);
if (schema is null)
{
_logger.LogInformation(
"No XSD schema found for data type {DataTypeId}, skipping XSD validation",
dataType.Id
);
continue;
}
var formData = await dataAccessor.GetFormData(dataElement);
ObjectUtils.RemoveAltinnRowId(formData);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bør vi fjerne altinnRowId her, eller bare kreve at den legges til som optional i xsd? Det er nok flere forskjeller, men dette er jo det eneste eksemplet (jeg har sett det har blitt klaget på) der xsd validering og data annotations gir ulikt resultat, men kanskje dere har andre grunner for å gjøre dette?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I testing, så arresterer validatoren deg om du har rowId og det ikke er i Xsd.
Det er ingen enkel måte å si blankt "Alle elementer kan ha AltinnRowId"; man må legge det til på hvert enkelt element, og det blir mye rot i Xsdene. For vårt prosjekt ville det vært ~1000 endringer.
Jeg har snakket med produkteieren vår angående altinnRowId og fått tydelig beskjed at vi ikke skal forholde oss til det i Xsd. Så her har egen erfaring blitt med inn som en beslutning i design av løsningen.


var serializedFormData = _modelSerializationService.SerializeToXml(formData);
var parsedSchema = new XmlSchemaSet();
using (var xsdReader = XmlReader.Create(new StringReader(schema)))
{
parsedSchema.Add(null, xsdReader);
}
var settings = new XmlReaderSettings { ValidationType = ValidationType.Schema, Schemas = parsedSchema };
settings.ValidationEventHandler += (sender, e) =>
{
validationIssues.Add(
new ValidationIssue()
{
Code = "Xsd",
CustomTextKey = "backend.xsd_validation",
DataElementId = dataElement.Id,
Severity = ValidationIssueSeverity.Error,
CustomTextParameters = new Dictionary<string, string>()
{
{ "schema", dataType.Id },
{ "message", e.Message },
},
}
);
};

using var xmlStream = new MemoryStream(serializedFormData.ToArray(), writable: false);
using var reader = XmlReader.Create(xmlStream, settings);
while (reader.Read()) { }
}

return validationIssues;
}
}
19 changes: 19 additions & 0 deletions src/Altinn.App.Core/Internal/Texts/TranslationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,25 @@ await state.GetModelData(binding, context?.DataElementIdentifier, context?.RowIn
_ => "Dokumentet er en forhåndsvisning",
},
};
case "backend.xsd_validation":
return new TextResourceElement()
{
Id = "backend.xsd_validation",
Value = language switch
{
LanguageConst.Nb => "Et felt bryter reglene satt av XSD. Melding: {0]",
LanguageConst.Nn => "Eit felt bryt reglane sette av XSD. Melding: {0}",
_ => "A field is in violation of the rules set by the XSD schema. Message: {0}",
},
Variables = [
new TextResourceVariable()
{
DataSource = "customTextParameters",
Key = "message",
DefaultValue = ""
}
]
};
}

return null;
Expand Down
Loading