1272 file attechment submodel - #192
Closed
mm-hsh wants to merge 48 commits into
Closed
Conversation
…nhance asset ID queries
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Refactor the return statement in `SubmodelRepositoryService.cs` to use the C# 8.0 range operator for improved collection handling. Add comprehensive unit tests in `GetAllSubmodelsControllerTests.cs` to cover various scenarios for the `GetAllSubmodelsAsync` method, including handling of query parameters, paging, and exception cases. Utilize `NSubstitute` for mocking and `Xunit` for test structuring.
…th semaphore and add configuration for concurrent operations limit
Introduce `_templateManagementOptions` in `SubmodelRepositoryServiceTests` to configure `TemplateManagementConfig` with a `ServiceInstance` and `ConcurrentOperationsLimit`. Update the constructor of `SubmodelRepositoryService` to include the new parameter. Add necessary using directives for configuration and options handling.
…tAllSubmodelsAsync functionality and improve request handling
…mplateAsync and adjust request paths
…SubmodelTemplateAsync for improved query handling
…cluding adjustments to semaphore release and data structure initialization.
…and response models, and add unit and integration tests
…Tests for improved readability
…apping in SubmodelsMapperProfile
…irect URLs and remove unused code
… methods; delete unused configuration classes.
…urn FileAttachmentResult; implement streaming timeout configuration; improve unit tests for file attachment retrieval.
…nd Service to support max file size; add unit tests for GetFileAttachment; adjust configuration settings.
…r interface and FileAttachmentStreamProvider class; refactor SubmodelRepositoryService and tests to utilize new provider.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Test & Coverage ReportTest Results Summary
Code CoverageUnit Tests Coverage
Minimum allowed line rate is Module Tests Coverage
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “file attachment download” capability for File SubmodelElements in the Submodel Repository API, including service/handler/controller plumbing, configuration additions, and a suite of unit/module tests to validate streaming behavior and error scenarios.
Changes:
- Adds an attachment download endpoint and handler/service method to fetch and stream File SubmodelElement content from an upstream HTTP URL.
- Introduces a stream provider abstraction (
IFileAttachmentStreamProvider) plus infrastructure implementation and DI wiring. - Expands unit/module tests and test data to cover the new attachment streaming pathway.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| source/AAS.TwinEngine.DataEngine/ServiceConfiguration/InfrastructureDependencyInjectionExtensions.cs | Registers new streaming provider and HttpClient wiring for attachment fetching. |
| source/AAS.TwinEngine.DataEngine/ServiceConfiguration/Config/HttpClientNames.cs | Adds a named HttpClient constant for file attachment fetching. |
| source/AAS.TwinEngine.DataEngine/ServiceConfiguration/Config/GeneralConfig.cs | Adds file attachment sizing config; needs to align with new timeout config key. |
| source/AAS.TwinEngine.DataEngine/Infrastructure/Streaming/FileAttachmentStreamProvider.cs | Implements HTTP-based upstream header retrieval and streaming. |
| source/AAS.TwinEngine.DataEngine/DomainModel/SubmodelRepository/FileAttachmentResult.cs | Adds a domain result type for attachment stream + metadata + disposables. |
| source/AAS.TwinEngine.DataEngine/appsettings.json | Adds a new file attachment streaming timeout setting under General. |
| source/AAS.TwinEngine.DataEngine/appsettings.development.json | Adds max attachment size override and JSON formatting adjustments. |
| source/AAS.TwinEngine.DataEngine/ApplicationLogic/Services/SubmodelRepository/SubmodelTemplateService.cs | Adjusts exception types for template/element missing scenarios. |
| source/AAS.TwinEngine.DataEngine/ApplicationLogic/Services/SubmodelRepository/SubmodelRepositoryService.cs | Implements GetFileAttachmentAsync with size checks and streaming wrapper. |
| source/AAS.TwinEngine.DataEngine/ApplicationLogic/Services/SubmodelRepository/Providers/IFileAttachmentStreamProvider.cs | Adds abstraction for upstream response/stream retrieval. |
| source/AAS.TwinEngine.DataEngine/ApplicationLogic/Services/SubmodelRepository/ISubmodelRepositoryService.cs | Extends service contract to include attachment retrieval. |
| source/AAS.TwinEngine.DataEngine/ApplicationLogic/Observability/DataEngineTracing.cs | Adds span constants for attachment load/streaming. |
| source/AAS.TwinEngine.DataEngine/ApplicationLogic/Exceptions/Application/InvalidSubmodelElementTypeException.cs | Adds a 400-mapped exception when element is not File. |
| source/AAS.TwinEngine.DataEngine/Api/SubmodelRepository/SubmodelRepositoryController.cs | Adds HTTP endpoint to stream file attachments as FileStreamResult. |
| source/AAS.TwinEngine.DataEngine/Api/SubmodelRepository/Responses/SubmodelElementsDto.cs | Normalizes file header/encoding (BOM) for the file. |
| source/AAS.TwinEngine.DataEngine/Api/SubmodelRepository/Requests/GetAllSubmodelElementsRequest.cs | Normalizes file header/encoding (BOM) for the file. |
| source/AAS.TwinEngine.DataEngine/Api/SubmodelRepository/Handler/SubmodelRepositoryHandler.cs | Adds handler method to register upstream disposables for response disposal. |
| source/AAS.TwinEngine.DataEngine/Api/SubmodelRepository/Handler/ISubmodelRepositoryHandler.cs | Extends handler contract to include attachment retrieval. |
| source/AAS.TwinEngine.DataEngine.UnitTests/ApplicationLogic/Services/SubmodelRepository/TestData.cs | Adds helper for minimal submodel creation for new tests. |
| source/AAS.TwinEngine.DataEngine.UnitTests/ApplicationLogic/Services/SubmodelRepository/SubmodelTemplateServiceTests.cs | Updates tests to match new exception types. |
| source/AAS.TwinEngine.DataEngine.UnitTests/ApplicationLogic/Services/SubmodelRepository/SubmodelRepositoryServiceTests.cs | Adds unit tests for GetFileAttachmentAsync behavior and failure modes. |
| source/AAS.TwinEngine.DataEngine.UnitTests/Api/SubmodelRepository/SubmodelRepositoryControllerTests.cs | Adds controller tests validating FileStreamResult output and route passing. |
| source/AAS.TwinEngine.DataEngine.UnitTests/Api/SubmodelRepository/Handler/SubmodelRepositoryHandlerTests.cs | Adds handler tests covering disposables registration and input validation. |
| source/AAS.TwinEngine.DataEngine.ModuleTests/Api/Services/SubmodelRepository/TestData.cs | Adds plugin response test data for a file URL (thumbnail). |
| source/AAS.TwinEngine.DataEngine.ModuleTests/Api/Services/SubmodelRepository/SubmodelRepositoryControllerTests.cs | Adds module tests verifying attachment streaming end-to-end with mocks. |
Suppressed comments (1)
source/AAS.TwinEngine.DataEngine/Api/SubmodelRepository/Handler/SubmodelRepositoryHandler.cs:122
- If HttpContext is unavailable, ResponseDisposables are silently not disposed, which can leak HttpResponseMessage/streams. Consider disposing immediately when there’s no HTTP response to register against.
foreach (var disposable in attachment.ResponseDisposables)
{
httpContextAccessor.HttpContext?.Response.RegisterForDispose(disposable);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+109
to
+110
| _ = services.AddHttpClientWithoutResilience(HttpClientNames.FileAttachmentProvider,null); | ||
|
|
Comment on lines
18
to
+21
| public CacheConfig Cache { get; set; } = new(); | ||
|
|
||
| public long MaxFileAttachmentSizeBytes { get; set; } = 100 * 1024 * 1024; // 100 MB | ||
|
|
Comment on lines
+246
to
+270
| if (!fileUrl.StartsWith("http://", StringComparison.OrdinalIgnoreCase) && | ||
| !fileUrl.StartsWith("https://", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| throw new NotImplementedException("File URL must start with http:// or https:// to be accessible."); | ||
| } | ||
|
|
||
| var upstreamResponse = await fileAttachmentStreamProvider.GetResponseHeadersAsync(fileUrl, cancellationToken).ConfigureAwait(false); | ||
| _ = upstreamResponse.EnsureSuccessStatusCode(); | ||
|
|
||
| // Fast rejection if server declared a size over the limit — avoids opening the body at all. | ||
| var declaredLength = upstreamResponse.Content.Headers.ContentLength; | ||
| if (declaredLength.HasValue && declaredLength.Value > _maxFileAttachmentSizeBytes) | ||
| { | ||
| upstreamResponse.Dispose(); | ||
| throw new NotImplementedException($"File exceeds maximum allowed size of {_maxFileAttachmentSizeBytes} bytes."); | ||
| } | ||
|
|
||
| var contentType = upstreamResponse.Content.Headers.ContentType?.ToString() ?? "application/octet-stream"; | ||
|
|
||
| var fileName = Path.GetFileName(new Uri(fileUrl).LocalPath); | ||
| if (string.IsNullOrWhiteSpace(fileName)) | ||
| { | ||
| fileName = fileElement.IdShort; | ||
| } | ||
|
|
Comment on lines
+240
to
+244
| var fileUrl = fileElement.Value; | ||
| if (string.IsNullOrWhiteSpace(fileUrl)) | ||
| { | ||
| throw new SubmodelElementNotFoundException(idShortPath); | ||
| } |
Comment on lines
+332
to
+334
| throw new NotImplementedException( | ||
| $"File attachment at '{_idShortPath}' exceeds the maximum allowed size of {_maxBytes} bytes."); | ||
| } |
Comment on lines
+109
to
+117
| var decodedSubmodelId = request?.SubmodelId?.DecodeBase64Url(logger); | ||
| var decodedIdShortPath = Uri.UnescapeDataString(request?.IdShortPath ?? string.Empty); | ||
| decodedIdShortPath.ValidateIdShortPath(nameof(request.IdShortPath), logger); | ||
|
|
||
| logger.LogInformation("Get File Attachment. SubmodelId: {SubmodelId}, IdShortPath: {IdShortPath}", decodedSubmodelId, decodedIdShortPath); | ||
|
|
||
| var attachment = await submodelRepositoryService | ||
| .GetFileAttachmentAsync(decodedSubmodelId!, decodedIdShortPath, cancellationToken) | ||
| .ConfigureAwait(false); |
Comment on lines
+155
to
+158
| /// <summary> | ||
| /// Downloads the binary file content of a File SubmodelElement. | ||
| /// The Content-Type header is derived from the element's contentType attribute. | ||
| /// </summary> |
Comment on lines
+673
to
+683
| public async Task GetFileAttachmentAsync_WhenFileUrlIsNotHttpOrHttps_ThrowsNotImplementedException() | ||
| { | ||
| const string IdShortPath = "Documents.ProductImage"; | ||
| const string FileUrl = "ftp://fake-plugin.local/files/product.png"; | ||
|
|
||
| var fileElement = new AasCore.Aas3_1.File(contentType: "image/png") { Value = FileUrl, IdShort = "ProductImage" }; | ||
| ArrangeAttachmentElement(IdShortPath, fileElement); | ||
|
|
||
| await Assert.ThrowsAsync<NotImplementedException>(() => | ||
| _sut.GetFileAttachmentAsync(SubmodelId, IdShortPath, CancellationToken.None)); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces comprehensive tests for the new file attachment streaming functionality in the Submodel Repository API, along with related dependency and test setup updates. The main focus is on ensuring that file attachments (such as images or documents) can be retrieved, streamed, and handled securely and correctly, including error handling for edge cases. Several test classes are updated to support and validate these features.
File Attachment Streaming and Testing Enhancements:
SubmodelRepositoryControllerTests.cs:IFileAttachmentStreamProviderdependency to support file streaming in tests.SubmodelRepositoryHandlerTests.cs:SubmodelRepositoryControllerTests.cs(Unit Tests):FileStreamResultand passes route values to the handler when serving file attachments.Dependency Injection and Test Setup:
IFileAttachmentStreamProviderwhere required.Test Data and Configuration Updates:
Minor Fixes and Code Quality:
These changes significantly improve test coverage for file attachment streaming and ensure the new functionality is robust, secure, and well-integrated with the existing API and service infrastructure.