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
20 changes: 20 additions & 0 deletions src/Altinn.App.Api/Controllers/ResourceController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,23 @@ public ActionResult GetModelJsonSchema([FromRoute] string id)
return Ok(schema);
}

/// <summary>
/// Get the xsd schema for the model
/// </summary>
[HttpGet]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK, "application/xml")]
[ProducesResponseType(StatusCodes.Status404NotFound)]
[Route("{org}/{app}/api/xsdschema/{id}")]
public ActionResult GetModelXsdSchema([FromRoute] string id)
{
string? schema = _appResourceService.GetXsdSchema(id);
if (schema == null)
{
return NotFound();
}
return Content(schema, "application/xml", System.Text.Encoding.UTF8);
}
Comment on lines +38 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add missing parameter documentation.

The XML documentation is missing the <param name="id"> tag and could benefit from a <returns> tag explaining the 200 vs 404 responses.

📝 Suggested documentation improvement
 /// <summary>
 /// Get the xsd schema for the model
 /// </summary>
+/// <param name="id">Unique identifier of the model to fetch XSD schema for.</param>
+/// <returns>The XSD schema as XML if found; otherwise, 404 NotFound.</returns>
 [HttpGet]
 [ProducesResponseType(typeof(string), StatusCodes.Status200OK, "application/xml")]
 [ProducesResponseType(StatusCodes.Status404NotFound)]

Note: This endpoint properly returns 404 for missing schemas, which is an improvement over GetModelJsonSchema (lines 24-36) that throws FileNotFoundException. Consider aligning the error handling of GetModelJsonSchema with this approach in a future update.

🤖 Prompt for AI Agents
In @src/Altinn.App.Api/Controllers/ResourceController.cs around lines 38 - 53,
Add XML documentation for the GetModelXsdSchema method: include a <param
name="id"> describing the id route parameter (e.g., "The resource id for which
to retrieve the XSD schema") and a <returns> tag that documents the possible
responses (200 OK returning "application/xml" with the schema, and 404 NotFound
when the schema is missing). Update the triple-slash comment immediately above
the GetModelXsdSchema method (in ResourceController.GetModelXsdSchema) to
include these tags and brief descriptions; no code changes required.


/// <summary>
/// Get the form layout
/// </summary>
Expand Down Expand Up @@ -80,6 +97,9 @@ public ActionResult GetLayouts(string org, string app, string id)
[ProducesResponseType(typeof(string), StatusCodes.Status200OK, "application/json")]
[HttpGet]
[Route("{org}/{app}/api/layoutsettings")]
[Obsolete(
"This endpoint is no longer available. Use /{org}/{app}/api/layoutsettings/{id} to get layout settings for a specific layout set."
)]
public ActionResult GetLayoutSettings(string org, string app)
{
string? settings = _appResourceService.GetLayoutSettingsString();
Expand Down
16 changes: 16 additions & 0 deletions src/Altinn.App.Core/Implementation/AppResourcesSI.cs
Original file line number Diff line number Diff line change
Expand Up @@ -523,4 +523,20 @@

return filedata;
}

/// <inheritdoc />
public string? GetXsdSchema(string dataTypeId)
{
string legalPath = Path.Join(_settings.AppBasePath, _settings.ModelsFolder);
string filename = Path.Join(legalPath, $"{dataTypeId}.xsd");
PathHelper.EnsureLegalPath(legalPath, filename);

string? filedata = null;
if (File.Exists(filename))
{
filedata = File.ReadAllText(filename, Encoding.UTF8);
}

return filedata;
}
Comment on lines +527 to +541
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add telemetry to match the established pattern.

The GetXsdSchema method is missing telemetry instrumentation, which is inconsistent with other resource-loading methods in this class. For example, GetValidationConfiguration (line 513) includes:

using var activity = _telemetry?.StartGetValidationConfigurationActivity();

Consider adding similar telemetry to track XSD schema retrieval operations.

📊 Proposed addition
 /// <inheritdoc />
 public string? GetXsdSchema(string dataTypeId)
 {
+    using var activity = _telemetry?.StartGetXsdSchemaActivity();
     string legalPath = Path.Join(_settings.AppBasePath, _settings.ModelsFolder);
     string filename = Path.Join(legalPath, $"{dataTypeId}.xsd");
     PathHelper.EnsureLegalPath(legalPath, filename);

     string? filedata = null;
     if (File.Exists(filename))
     {
         filedata = File.ReadAllText(filename, Encoding.UTF8);
     }

     return filedata;
 }

Note: You'll also need to add the corresponding telemetry activity method to the Telemetry class.

🤖 Prompt for AI Agents
In @src/Altinn.App.Core/Implementation/AppResourcesSI.cs around lines 527 - 541,
GetXsdSchema lacks telemetry; wrap its body with the same pattern used in
GetValidationConfiguration by adding a using var activity =
_telemetry?.StartGetXsdSchemaActivity(); at the start of GetXsdSchema (before
Path.Join calls) and ensure the telemetry StartGetXsdSchemaActivity method is
implemented on the Telemetry class (matching naming/pattern of
StartGetValidationConfigurationActivity) so XSD retrievals are tracked.

}
5 changes: 5 additions & 0 deletions src/Altinn.App.Core/Internal/App/IAppResources.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,9 @@ public interface IAppResources
/// Gets the validation configuration for a given data type
/// </summary>
string? GetValidationConfiguration(string dataTypeId);

/// <summary>
/// Gets the xsd schema for the provided dataTypeId.
/// </summary>
string? GetXsdSchema(string dataTypeId);
}
57 changes: 57 additions & 0 deletions test/Altinn.App.Api.Tests/Controllers/ResourceControllerTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
using Microsoft.AspNetCore.Mvc.Testing;
using Xunit.Abstractions;

namespace Altinn.App.Api.Tests.Controllers;

public class ResourceControllerTests : ApiTestBase, IClassFixture<WebApplicationFactory<Program>>
{
public ResourceControllerTests(WebApplicationFactory<Program> factory, ITestOutputHelper outputHelper)
: base(factory, outputHelper) { }

private const string Org = "tdd";
private const string App = "contributer-restriction";
private const string DefaultDataTypeId = "Skjema";

[Fact]
public async Task GetModelJsonSchema_ReturnsOk()
{
var client = GetRootedClient(Org, App);
using var response = await client.GetAsync($"/{Org}/{App}/api/jsonschema/{DefaultDataTypeId}");
response.EnsureSuccessStatusCode();
var content = await response.Content.ReadAsStringAsync();
Assert.NotNull(content);
Assert.Contains("\"$comment\"", content); // TODO: Update when the schema in the test project has more content.
}

[Fact]
public async Task GetModelJsonSchema_InvalidDataTypeId_ReturnsNotFound()
{
var client = GetRootedClient(Org, App);
await Assert.ThrowsAsync<FileNotFoundException>(async () =>
{
using var response = await client.GetAsync($"/{Org}/{App}/api/jsonschema/InvalidDataTypeId");
// TODO: Should probably return 404 NotFound instead of throwing FileNotFoundException,
// but adding test for current behaviour.
// Assert.Equal(System.Net.HttpStatusCode.NotFound, response.StatusCode);
});
}
Comment on lines +26 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Test name doesn't match the behavior being tested.

The test is named GetModelJsonSchema_InvalidDataTypeId_ReturnsNotFound but it actually expects a FileNotFoundException to be thrown, not a 404 status code. While the TODO comment (lines 33-35) acknowledges this inconsistency, the test name should reflect the actual behavior being verified.

📝 Suggested test name update
-public async Task GetModelJsonSchema_InvalidDataTypeId_ReturnsNotFound()
+public async Task GetModelJsonSchema_InvalidDataTypeId_ThrowsFileNotFoundException()

Note: The TODO comment correctly identifies that the endpoint should return 404 NotFound instead of throwing an exception. This could be addressed in a follow-up improvement to align with the GetModelXsdSchema endpoint behavior.

🤖 Prompt for AI Agents
In @test/Altinn.App.Api.Tests/Controllers/ResourceControllerTests.cs around
lines 26 - 37, The test method
GetModelJsonSchema_InvalidDataTypeId_ReturnsNotFound in
ResourceControllerTests.cs is misnamed because it asserts a
FileNotFoundException rather than a 404; rename the test to reflect the actual
behavior (e.g.,
GetModelJsonSchema_InvalidDataTypeId_ThrowsFileNotFoundException) and update any
references/usages of that method, leaving the test body unchanged (or
alternatively change the assertion to check response.StatusCode == NotFound if
you intend to change behavior instead of the name).


[Fact]
public async Task GetXmlSchema_ReturnsOk()
{
var client = GetRootedClient(Org, App);
using var response = await client.GetAsync($"/{Org}/{App}/api/xsdschema/{DefaultDataTypeId}");
response.EnsureSuccessStatusCode();
var content = await response.Content.ReadAsStringAsync();
Assert.NotNull(content);
Assert.Contains("<xs:schema xmlns:xs=", content); // TODO: Update when the schema in the test project has more content.
}

[Fact]
public async Task GetXmlSchema_InvalidDataTypeId_ReturnsNotFound()
{
var client = GetRootedClient(Org, App);
using var response = await client.GetAsync($"/{Org}/{App}/api/xsdschema/InvalidDataTypeId");
Assert.Equal(System.Net.HttpStatusCode.NotFound, response.StatusCode);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"$comment": "TODO: ensure content if required for test"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="UTF-8"?>
<!-- TODO: make sure this is in sync with actual model-->
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" elementFormDefault="qualified" attributeFormDefault="unqualified">
</xs:schema>
Original file line number Diff line number Diff line change
Expand Up @@ -5085,6 +5085,82 @@
}
}
},
"/{org}/{app}/api/xsdschema/{id}": {
"get": {
"tags": [
"Resource"
],
"summary": "Get the xsd schema for the model",
"parameters": [
{
"name": "id",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
},
{
"name": "org",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
},
{
"name": "app",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"description": "OK",
"content": {
"application/xml": {
"schema": {
"type": "string"
}
}
}
},
"404": {
"description": "Not Found",
"content": {
"text/plain": {
"schema": {
"$ref": "#/components/schemas/ProblemDetails"
}
},
"application/json": {
"schema": {
"$ref": "#/components/schemas/ProblemDetails"
}
},
"text/json": {
"schema": {
"$ref": "#/components/schemas/ProblemDetails"
}
},
"application/xml": {
"schema": {
"$ref": "#/components/schemas/ProblemDetails"
}
},
"text/xml": {
"schema": {
"$ref": "#/components/schemas/ProblemDetails"
}
}
}
}
}
}
},
"/{org}/{app}/api/layouts": {
"get": {
"tags": [
Expand Down Expand Up @@ -5213,7 +5289,8 @@
}
}
}
}
},
"deprecated": true
}
},
"/{org}/{app}/api/layoutsettings/{id}": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,8 @@ namespace Altinn.App.Api.Controllers
[Microsoft.AspNetCore.Mvc.HttpGet]
[Microsoft.AspNetCore.Mvc.ProducesResponseType(typeof(string), 200, "application/json", new string[0])]
[Microsoft.AspNetCore.Mvc.Route("{org}/{app}/api/layoutsettings")]
[System.Obsolete("This endpoint is no longer available. Use /{org}/{app}/api/layoutsettings/{id} to" +
" get layout settings for a specific layout set.")]
public Microsoft.AspNetCore.Mvc.ActionResult GetLayoutSettings(string org, string app) { }
[Microsoft.AspNetCore.Mvc.HttpGet]
[Microsoft.AspNetCore.Mvc.ProducesResponseType(typeof(string), 200, "application/json", new string[0])]
Expand All @@ -549,6 +551,11 @@ namespace Altinn.App.Api.Controllers
[Microsoft.AspNetCore.Mvc.Route("{org}/{app}/api/jsonschema/{id}")]
public Microsoft.AspNetCore.Mvc.ActionResult GetModelJsonSchema([Microsoft.AspNetCore.Mvc.FromRoute] string id) { }
[Microsoft.AspNetCore.Mvc.HttpGet]
[Microsoft.AspNetCore.Mvc.ProducesResponseType(404)]
[Microsoft.AspNetCore.Mvc.ProducesResponseType(typeof(string), 200, "application/xml", new string[0])]
[Microsoft.AspNetCore.Mvc.Route("{org}/{app}/api/xsdschema/{id}")]
public Microsoft.AspNetCore.Mvc.ActionResult GetModelXsdSchema([Microsoft.AspNetCore.Mvc.FromRoute] string id) { }
[Microsoft.AspNetCore.Mvc.HttpGet]
[Microsoft.AspNetCore.Mvc.ProducesResponseType(204)]
[Microsoft.AspNetCore.Mvc.ProducesResponseType(typeof(Microsoft.AspNetCore.Mvc.FileContentResult), 200, "application/json", new string[0])]
[Microsoft.AspNetCore.Mvc.Route("{org}/{app}/api/ruleconfiguration/{id}")]
Expand Down
21 changes: 10 additions & 11 deletions test/Altinn.App.Core.Tests/Implementation/AppResourcesSITests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#nullable disable
using Altinn.App.Core.Configuration;
using Altinn.App.Core.Features.ExternalApi;
using Altinn.App.Core.Implementation;
Expand Down Expand Up @@ -30,7 +29,7 @@ public void GetApplication_desrializes_file_from_disk()
AppResourcesSI appResources = new(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand Down Expand Up @@ -81,7 +80,7 @@ public void GetApplication_handles_onEntry_null()
AppResourcesSI appResources = new(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand Down Expand Up @@ -135,7 +134,7 @@ public void GetApplication_second_read_from_cache()
AppResourcesSI appResources = new(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand Down Expand Up @@ -190,7 +189,7 @@ public void GetApplicationMetadata_throws_ApplicationConfigException_if_file_not
IAppResources appResources = new AppResourcesSI(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand All @@ -206,7 +205,7 @@ public void GetApplicationMetadata_throws_ApplicationConfigException_if_deserial
IAppResources appResources = new AppResourcesSI(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand All @@ -222,7 +221,7 @@ public void GetApplicationXACMLPolicy_return_policyfile_as_string()
IAppResources appResources = new AppResourcesSI(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand All @@ -240,7 +239,7 @@ public void GetApplicationXACMLPolicy_return_null_if_file_not_found()
IAppResources appResources = new AppResourcesSI(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand All @@ -257,7 +256,7 @@ public void GetApplicationBPMNProcess_return_process_as_string()
IAppResources appResources = new AppResourcesSI(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand All @@ -275,7 +274,7 @@ public void GetApplicationBPMNProcess_return_null_if_file_not_found()
IAppResources appResources = new AppResourcesSI(
settings,
appMetadata,
null,
null!,
new NullLogger<AppResourcesSI>(),
_telemetry.Object
);
Expand Down Expand Up @@ -305,7 +304,7 @@ private AppSettings GetAppSettings(

private static IAppMetadata SetupAppMetadata(
IOptions<AppSettings> appsettings,
IFrontendFeatures frontendFeatures = null
IFrontendFeatures? frontendFeatures = null
)
{
var featureManagerMock = new Mock<IFeatureManager>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2353,6 +2353,7 @@ namespace Altinn.App.Core.Implementation
public byte[] GetText(string org, string app, string textResource) { }
public System.Threading.Tasks.Task<Altinn.Platform.Storage.Interface.Models.TextResource?> GetTexts(string org, string app, string language) { }
public string? GetValidationConfiguration(string dataTypeId) { }
public string? GetXsdSchema(string dataTypeId) { }
}
public class DefaultAppEvents : Altinn.App.Core.Internal.App.IAppEvents
{
Expand Down Expand Up @@ -2928,6 +2929,7 @@ namespace Altinn.App.Core.Internal.App
byte[] GetText(string org, string app, string textResource);
System.Threading.Tasks.Task<Altinn.Platform.Storage.Interface.Models.TextResource?> GetTexts(string org, string app, string language);
string? GetValidationConfiguration(string dataTypeId);
string? GetXsdSchema(string dataTypeId);
}
public interface IApplicationClient
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,15 @@
requiredScopesServiceOwners: null
}
},
{
endpoint: GET {org}/{app}/api/xsdschema/{id},
metadata: {
errorMessageTextResourceKeyUser: null,
errorMessageTextResourceKeyServiceOwner: null,
requiredScopesUsers: null,
requiredScopesServiceOwners: null
}
},
{
endpoint: GET {org}/{app}/instances/{instanceOwnerId:int}/{instanceId:guid}/data/{dataGuid:guid}/validate,
metadata: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,15 @@
requiredScopesServiceOwners: null
}
},
{
endpoint: GET {org}/{app}/api/xsdschema/{id},
metadata: {
errorMessageTextResourceKeyUser: null,
errorMessageTextResourceKeyServiceOwner: null,
requiredScopesUsers: null,
requiredScopesServiceOwners: null
}
},
{
endpoint: GET {org}/{app}/instances/{instanceOwnerId:int}/{instanceId:guid}/data/{dataGuid:guid}/validate,
metadata: {
Expand Down
Loading