Skip to content

Commit

Permalink
Feat/717 Ensure unique filenames in eFormidling shipment (#774)
Browse files Browse the repository at this point in the history 
* Add missing await in DefaultEFormidlingService.cs.

* Ensure filenames being sent to eFormidling are unique.

* Suggested tests with fix

* Fix sonar cloud issue

* Process data elements ordered by created datetime.

---------

Co-authored-by: Ivar Nesje <ivarne@users.noreply.github.com>
  • Loading branch information
@bjorntore @ivarne
bjorntore and ivarne authored Sep 17, 2024
1 parent 5c250e6 commit eedd776
Show file tree
Hide file tree
Showing 2 changed files with 234 additions and 28 deletions.
86 changes: 76 additions & 10 deletions src/Altinn.App.Core/EFormidling/Implementation/DefaultEFormidlingService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ public async Task SendEFormidlingShipment(Instance instance)
StandardBusinessDocument sbd = await ConstructStandardBusinessDocument(instanceGuid, instance);
await _eFormidlingClient.CreateMessage(sbd, requestHeaders);

(string metadataName, Stream stream) = await _eFormidlingMetadata.GenerateEFormidlingMetadata(instance);
(string metadataFilename, Stream stream) = await _eFormidlingMetadata.GenerateEFormidlingMetadata(instance);

using (stream)
{
await _eFormidlingClient.UploadAttachment(stream, instanceGuid, metadataName, requestHeaders);
await _eFormidlingClient.UploadAttachment(stream, instanceGuid, metadataFilename, requestHeaders);
}

await SendInstanceData(instance, requestHeaders);
await SendInstanceData(instance, requestHeaders, metadataFilename);

try
{
Expand Down Expand Up @@ -192,25 +192,47 @@ Instance instance
return sbd;
}

private async Task SendInstanceData(Instance instance, Dictionary<string, string> requestHeaders)
private async Task SendInstanceData(
Instance instance,
Dictionary<string, string> requestHeaders,
string eformidlingMetadataFilename
)
{
ApplicationMetadata applicationMetadata = await _appMetadata.GetApplicationMetadata();

Guid instanceGuid = Guid.Parse(instance.Id.Split("/")[1]);
int instanceOwnerPartyId = int.Parse(instance.InstanceOwner.PartyId, CultureInfo.InvariantCulture);
foreach (DataElement dataElement in instance.Data)

// Keep track of already used file names to ensure they are unique. eFormidling does not allow duplicate filenames.
HashSet<string> usedFileNames = [eformidlingMetadataFilename];

List<string> dataTypeIds = applicationMetadata.DataTypes.Select(x => x.Id).ToList();

foreach (DataElement dataElement in instance.Data.OrderBy(x => x.Created))
{
if (!applicationMetadata.EFormidling.DataTypes.Contains(dataElement.DataType))
{
continue;
}

bool appLogic = applicationMetadata.DataTypes.Any(d =>
d.Id == dataElement.DataType && d.AppLogic?.ClassRef != null
DataType dataType =
applicationMetadata.DataTypes.Find(d => d.Id == dataElement.DataType)
?? throw new InvalidOperationException(
$"DataType {dataElement.DataType} not found in application metadata"
);

bool hasAppLogic = dataType.AppLogic?.ClassRef is not null;

string uniqueFileName = GetUniqueFileName(
dataElement.Filename,
dataType.Id,
hasAppLogic,
dataTypeIds,
usedFileNames
);
usedFileNames.Add(uniqueFileName);

string fileName = appLogic ? $"{dataElement.DataType}.xml" : dataElement.Filename;
using Stream stream = await _dataClient.GetBinaryData(
await using Stream stream = await _dataClient.GetBinaryData(
applicationMetadata.Org,
applicationMetadata.AppIdentifier.App,
instanceOwnerPartyId,
Expand All @@ -222,7 +244,7 @@ private async Task SendInstanceData(Instance instance, Dictionary<string, string
bool successful = await _eFormidlingClient.UploadAttachment(
stream,
instanceGuid.ToString(),
fileName,
uniqueFileName,
requestHeaders
);

Expand All @@ -236,4 +258,48 @@ private async Task SendInstanceData(Instance instance, Dictionary<string, string
}
}
}

internal static string GetUniqueFileName(
string? fileName,
string dataTypeId,
bool hasAppLogic,
List<string> dataTypeIds,
HashSet<string> usedFileNames
)
{
if (hasAppLogic)
{
// Data types with classRef should get filename based on DataType.
fileName = $"{dataTypeId}.xml";
}
else if (string.IsNullOrWhiteSpace(fileName))
{
// If no filename is set, default to DataType.
fileName = dataTypeId;
}
else if (
!dataTypeIds.TrueForAll(id =>
id == dataTypeId || !fileName.StartsWith(id, StringComparison.OrdinalIgnoreCase)
)
)
{
// If the file starts with another data types id, prepend the current data type id to avoid stealing the counter-less filename from the AppLogic data element.
fileName = $"{dataTypeId}-{fileName}";
}
string name = Path.GetFileNameWithoutExtension(fileName);
string extension = Path.GetExtension(fileName);

// Handle the case where there's no extension.
string uniqueFileName = string.IsNullOrEmpty(extension) ? name : $"{name}{extension}";
var counter = 1;

// Generate unique file name.
while (usedFileNames.Contains(uniqueFileName))
{
uniqueFileName = string.IsNullOrEmpty(extension) ? $"{name}-{counter}" : $"{name}-{counter}{extension}";
counter++;
}

return uniqueFileName;
}
}
176 changes: 158 additions & 18 deletions test/Altinn.App.Core.Tests/Eformidling/Implementation/DefaultEFormidlingServiceTests.cs
View file
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.Constants;
using Altinn.App.Core.EFormidling;
Expand Down Expand Up @@ -39,11 +38,57 @@ public void SendEFormidlingShipment()
var eFormidlingClient = new Mock<IEFormidlingClient>();
var tokenGenerator = new Mock<IAccessTokenGenerator>();
var eFormidlingMetadata = new Mock<IEFormidlingMetadata>();

const string eFormidlingMetadataFilename = "arkivmelding.xml";
const string modelDataType = "model";
const string fileAttachmentsDataType = "file-attachments";
var instanceGuid = Guid.Parse("41C1099C-7EDD-47F5-AD1F-6267B497796F");
var instance = new Instance
{
Id = "1337/41C1099C-7EDD-47F5-AD1F-6267B497796F",
Id = $"1337/{instanceGuid}",
InstanceOwner = new InstanceOwner { PartyId = "1337", },
Data = new List<DataElement>()
Data =
[
new DataElement { Id = Guid.NewGuid().ToString(), DataType = modelDataType, },
new DataElement
{
Id = Guid.NewGuid().ToString(),
DataType = fileAttachmentsDataType,
Filename = "attachment.txt"
},
new DataElement
{
Id = Guid.NewGuid().ToString(),
DataType = fileAttachmentsDataType,
Filename = "attachment.txt"
},
new DataElement
{
Id = Guid.NewGuid().ToString(),
DataType = fileAttachmentsDataType,
Filename = "no-extension"
},
new DataElement
{
Id = Guid.NewGuid().ToString(),
DataType = fileAttachmentsDataType,
Filename = null
},
//Same filename as the eFormidling metadata file.
new DataElement
{
Id = Guid.NewGuid().ToString(),
DataType = fileAttachmentsDataType,
Filename = eFormidlingMetadataFilename
},
//Same filename as model data type.
new DataElement
{
Id = Guid.NewGuid().ToString(),
DataType = fileAttachmentsDataType,
Filename = modelDataType + ".xml"
}
]
};

appMetadata
Expand All @@ -52,14 +97,23 @@ public void SendEFormidlingShipment()
new ApplicationMetadata("ttd/test-app")
{
Org = "ttd",
DataTypes =
[
new DataType
{
Id = modelDataType,
AppLogic = new ApplicationLogic { ClassRef = "SomeClass" }
},
new DataType { Id = fileAttachmentsDataType }
],
EFormidling = new EFormidlingContract
{
Process = "urn:no:difi:profile:arkivmelding:plan:3.0",
Standard = "urn:no:difi:arkivmelding:xsd::arkivmelding",
TypeVersion = "v8",
Type = "arkivmelding",
SecurityLevel = 3,
DataTypes = new List<string>()
DataTypes = [modelDataType, fileAttachmentsDataType]
}
}
);
Expand All @@ -70,8 +124,19 @@ public void SendEFormidlingShipment()
.Setup(em => em.GenerateEFormidlingMetadata(instance))
.ReturnsAsync(() =>
{
return ("fakefilename.txt", Stream.Null);
return (eFormidlingMetadataFilename, Stream.Null);
});
dataClient
.Setup(x =>
x.GetBinaryData(
It.IsAny<string>(),
It.IsAny<string>(),
It.IsAny<int>(),
It.IsAny<Guid>(),
It.IsAny<Guid>()
)
)
.ReturnsAsync(Stream.Null);

var defaultEformidlingService = new DefaultEFormidlingService(
logger,
Expand Down Expand Up @@ -104,15 +169,42 @@ public void SendEFormidlingShipment()
eFormidlingReceivers.Verify(er => er.GetEFormidlingReceivers(instance));
eFormidlingMetadata.Verify(em => em.GenerateEFormidlingMetadata(instance));
eFormidlingClient.Verify(ec => ec.CreateMessage(It.IsAny<StandardBusinessDocument>(), expectedReqHeaders));
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(Stream.Null, instanceGuid.ToString(), eFormidlingMetadataFilename, expectedReqHeaders)
);
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(Stream.Null, instanceGuid.ToString(), $"{modelDataType}.xml", expectedReqHeaders)
);
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(Stream.Null, instanceGuid.ToString(), "attachment.txt", expectedReqHeaders)
);
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(Stream.Null, instanceGuid.ToString(), "attachment-1.txt", expectedReqHeaders)
);
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(Stream.Null, instanceGuid.ToString(), "no-extension", expectedReqHeaders)
);
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(Stream.Null, instanceGuid.ToString(), fileAttachmentsDataType, expectedReqHeaders)
);
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(
Stream.Null,
"41C1099C-7EDD-47F5-AD1F-6267B497796F",
"fakefilename.txt",
instanceGuid.ToString(),
$"{Path.GetFileNameWithoutExtension(eFormidlingMetadataFilename)}-1.xml",
expectedReqHeaders
)
);
eFormidlingClient.Verify(ec => ec.SendMessage("41C1099C-7EDD-47F5-AD1F-6267B497796F", expectedReqHeaders));
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(
Stream.Null,
instanceGuid.ToString(),
$"{fileAttachmentsDataType}-{modelDataType}.xml",
expectedReqHeaders
)
);

eFormidlingClient.Verify(ec => ec.SendMessage(instanceGuid.ToString(), expectedReqHeaders));
eventClient.Verify(e => e.AddEvent(EformidlingConstants.CheckInstanceStatusEventType, instance));

eFormidlingClient.VerifyNoOtherCalls();
Expand All @@ -125,6 +217,57 @@ public void SendEFormidlingShipment()
result.IsCompletedSuccessfully.Should().BeTrue();
}

[Theory]
// Filename does not have a prefix for any data type, but collides with previous test-1.txt file, so it skips
[InlineData("test.txt", "a", false, "test.txt", "test-2.txt")]
// App logic data types, always gets the {dataType}.xml name (and skips existing indexes)
[InlineData("test.txt", "a", true, "a.xml", "a-2.xml")]
// Filename gets "{dataType}-" prefix if the given name is a prefix of another type
[InlineData("abc.txt", "a", false, "a-abc.txt", "a-abc-1.txt")]
// Filename does not get "{dataType}-" prefix if the given name is a prefix of only the same type
[InlineData("abc.txt", "ab", false, "ab-abc.txt", "ab-abc-1.txt")]
// Filename is null without applogic, so just use the dataType, and add suffix for uniqueness
[InlineData(null, "ab", false, "ab", "ab-1")]
// Filename is null, but with app logic, so use {dataType}.xml
[InlineData(null, "ab", true, "ab.xml", "ab-1.xml")]
// Filename prefixes dataType c, so it gets the {dataType}- prefix
[InlineData("car.txt", "a", false, "a-car.txt", "a-car-1.txt")]
// Filename prefixes dataType c, but is the same as the dataType, so it doesn't get {dataType}- prefix
[InlineData("car.txt", "c", false, "car.txt", "car-1.txt")]
public void UniqueFileName(
string? fileName,
string dataTypeId,
bool hasAppLogic,
string expected1,
string expected2
)
{
var dataTypeIds = new List<string> { "a", "ab", "c" };
var usedFileNames = new HashSet<string> { "test-1.txt", "a-1.xml" };

var uniqueFileName = DefaultEFormidlingService.GetUniqueFileName(
fileName,
dataTypeId,
hasAppLogic,
dataTypeIds,
usedFileNames
);
usedFileNames.Add(uniqueFileName);

uniqueFileName.Should().Be(expected1);

uniqueFileName = DefaultEFormidlingService.GetUniqueFileName(
fileName,
dataTypeId,
hasAppLogic,
dataTypeIds,
usedFileNames
);
usedFileNames.Add(uniqueFileName);

uniqueFileName.Should().Be(expected2);
}

[Fact]
public void SendEFormidlingShipment_throws_exception_if_send_fails()
{
Expand All @@ -142,9 +285,10 @@ public void SendEFormidlingShipment_throws_exception_if_send_fails()
var eFormidlingClient = new Mock<IEFormidlingClient>();
var tokenGenerator = new Mock<IAccessTokenGenerator>();
var eFormidlingMetadata = new Mock<IEFormidlingMetadata>();
var instanceGuid = Guid.Parse("41C1099C-7EDD-47F5-AD1F-6267B497796F");
var instance = new Instance
{
Id = "1337/41C1099C-7EDD-47F5-AD1F-6267B497796F",
Id = $"1337/{instanceGuid}",
InstanceOwner = new InstanceOwner { PartyId = "1337", },
Data = new List<DataElement>()
};
Expand All @@ -163,7 +307,8 @@ public void SendEFormidlingShipment_throws_exception_if_send_fails()
Type = "arkivmelding",
SecurityLevel = 3,
DataTypes = new List<string>()
}
},
DataTypes = []
}
);
tokenGenerator.Setup(t => t.GenerateAccessToken("ttd", "test-app")).Returns("access-token");
Expand All @@ -173,7 +318,7 @@ public void SendEFormidlingShipment_throws_exception_if_send_fails()
.Setup(em => em.GenerateEFormidlingMetadata(instance))
.ReturnsAsync(() =>
{
return ("fakefilename.txt", Stream.Null);
return ("arkivmelding.xml", Stream.Null);
});
eFormidlingClient
.Setup(ec => ec.SendMessage(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
Expand Down Expand Up @@ -212,14 +357,9 @@ public void SendEFormidlingShipment_throws_exception_if_send_fails()
eFormidlingMetadata.Verify(em => em.GenerateEFormidlingMetadata(instance));
eFormidlingClient.Verify(ec => ec.CreateMessage(It.IsAny<StandardBusinessDocument>(), expectedReqHeaders));
eFormidlingClient.Verify(ec =>
ec.UploadAttachment(
Stream.Null,
"41C1099C-7EDD-47F5-AD1F-6267B497796F",
"fakefilename.txt",
expectedReqHeaders
)
ec.UploadAttachment(Stream.Null, instanceGuid.ToString(), "arkivmelding.xml", expectedReqHeaders)
);
eFormidlingClient.Verify(ec => ec.SendMessage("41C1099C-7EDD-47F5-AD1F-6267B497796F", expectedReqHeaders));
eFormidlingClient.Verify(ec => ec.SendMessage(instanceGuid.ToString(), expectedReqHeaders));

eFormidlingClient.VerifyNoOtherCalls();
eventClient.VerifyNoOtherCalls();
Expand Down

0 comments on commit eedd776

Please sign in to comment.