Skip to content

feat/add metadata #1598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
walldenfilippa wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat/add metadata #1598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
46a1556
addMetadata
Sep 25, 2025
5ebd0d9
Merge branch 'main' of https://github.com/walldenfilippa/app-lib-dotn…
walldenfilippa Oct 21, 2025
6382df0
minor update to merge
walldenfilippa Oct 21, 2025
e46fc82
minor updates coderabbitai
walldenfilippa Dec 11, 2025
d2f9f10
Merge branch 'main' of https://github.com/Altinn/app-lib-dotnet into …
walldenfilippa Dec 11, 2025
c738709
minor formatting edit
walldenfilippa Dec 11, 2025
e46b828
minor edit
walldenfilippa Dec 11, 2025
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
8 changes: 8 additions & 0 deletions src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,14 @@ DataElementChange change
new MemoryAsStream(bytes),
authenticationMethod: GetAuthenticationMethod(change.DataType)
);

if (change.Metadata is not null)
{
dataElement.Metadata = [.. change.Metadata];
await _dataClient.Update(Instance, dataElement);
change.Lock = true;
}
Comment on lines +482 to +487
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Consider error handling for metadata Update failure.

If _dataClient.Update fails after InsertBinaryData succeeds, the data element will exist in storage without the intended metadata. While the exception will propagate and fail the operation, there's no cleanup or rollback of the already-created data element.

Additionally, this approach makes two sequential storage calls for each new data element with metadata. Consider whether the storage API could support inserting with metadata in a single operation to improve reliability and performance.

Suggested approach for error handling

Consider wrapping the metadata persistence in a try-catch to handle failures gracefully:

 if (change.Metadata is not null)
 {
     dataElement.Metadata = [.. change.Metadata];
-    await _dataClient.Update(Instance, dataElement);
-    change.Lock = true;
+    try
+    {
+        await _dataClient.Update(Instance, dataElement);
+        change.Lock = true;
+    }
+    catch (Exception ex)
+    {
+        // Log the failure and consider whether to:
+        // 1. Delete the created data element (rollback)
+        // 2. Continue without metadata and log a warning
+        // 3. Rethrow and fail the entire operation (current behavior)
+        throw;
+    }
 }

For the performance concern, investigate whether InsertBinaryData can accept metadata as a parameter to avoid the second call.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In @src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs around lines
482-487, The code currently calls _dataClient.Update(Instance, dataElement)
after InsertBinaryData and doesn't handle failures, risking orphaned binary data
and extra round-trips; modify the logic in InstanceDataUnitOfWork where
InsertBinaryData and the metadata handling occur so that: if change.Metadata is
not null you either (a) call a single insert API that accepts metadata
(investigate and use InsertBinaryData overload that accepts metadata) to avoid
two calls, or (b) wrap the _dataClient.Update(Instance, dataElement) in a
try-catch and on failure perform compensating cleanup by deleting the previously
inserted binary (use the same identifier on dataElement/InsertBinaryData) and
rethrow or translate the exception; ensure change.Lock is only set to true after
a successful metadata persist and reference the symbols InsertBinaryData,
dataElement, change.Metadata, _dataClient.Update, change.Lock and Instance when
making the changes.


// Update caches
_binaryCache.Set(dataElement, bytes);
change.DataElement = dataElement; // Set the data element so that it can be referenced later in the save process
Expand Down
36 changes: 36 additions & 0 deletions src/Altinn.App.Core/Models/DataElementChanges.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ private protected DataElementChange(
ContentType = contentType;
}

/// <summary>
/// The metadata of the data element
/// </summary>
private List<KeyValueEntry>? _metadata;

/// <summary>
/// The type of update: Create, Update or Delete
/// </summary>
Expand All @@ -72,6 +77,37 @@ private protected DataElementChange(
/// The contentType of an element in storage
/// </summary>
public string ContentType { get; }

/// <summary>
/// The metadata of the data element
/// </summary>
internal IReadOnlyCollection<KeyValueEntry>? Metadata => _metadata;

/// <summary>
/// If true, no more metadata can be added
/// </summary>
internal bool Lock { get; set; }

/// <summary>
/// Add metadata to a created data element
/// </summary>
public void AddMetadata(string key, string value)
{
ArgumentException.ThrowIfNullOrWhiteSpace(key);
ArgumentException.ThrowIfNullOrWhiteSpace(value);

if (Type != ChangeType.Created)
{
throw new InvalidOperationException("Metadata can only be added to created data elements");
}

if (Lock)
{
throw new InvalidOperationException("Metadata already locked");
}
_metadata ??= [];
_metadata.Add(new KeyValueEntry { Key = key, Value = value });
}
}

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions ...tinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4157,6 +4157,7 @@ namespace Altinn.App.Core.Models
public Altinn.App.Core.Models.DataElementIdentifier DataElementIdentifier { get; }
public Altinn.Platform.Storage.Interface.Models.DataType DataType { get; }
public Altinn.App.Core.Models.ChangeType Type { get; }
public void AddMetadata(string key, string value) { }
}
public sealed class DataElementChanges
{
Expand Down
Loading