LNK-4366: Submission Service: Pass Query Parameter to Endpoint, Default to Internal ABS - #1243
Conversation
By default, download from internal (since submissions will be removed from external after downstream ingestion). But for smoke testing, download from external to test full end-to-end flow.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces a query parameter to the DownloadReport endpoint to select between internal and external blob storage, defaulting to internal. The blob storage service is refactored with a shared private helper method supporting both download paths. Changes
Sequence DiagramsequenceDiagram
participant Client
participant SubmissionController
participant BlobStorageService
participant BlobContainerClient
Client->>SubmissionController: DownloadReport(facilityId, reportId, external=true/false)
alt external = true
SubmissionController->>BlobStorageService: DownloadFromExternalAsync(uri)
else external = false (default)
SubmissionController->>BlobStorageService: DownloadFromInternalAsync(uri)
end
BlobStorageService->>BlobStorageService: DownloadAsync(containerClient, prefix)
BlobStorageService->>BlobContainerClient: List blobs with prefix
BlobContainerClient-->>BlobStorageService: Blob list
BlobStorageService->>BlobContainerClient: Download each blob
BlobContainerClient-->>BlobStorageService: Blob contents
BlobStorageService-->>SubmissionController: IDictionary<string, byte[]>
SubmissionController-->>Client: Response with files
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
DotNet/Submission/Controllers/SubmissionController.cs (1)
93-95: Consider adding unit tests for the branching logic.The conditional logic introduced here should have corresponding unit tests to verify:
external=trueinvokesDownloadFromExternalAsyncexternal=false(default) invokesDownloadFromInternalAsync- Proper exception handling when blob storage clients are not configured
As per coding guidelines, if/else blocks should have unit tests covering each branch.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
DotNet/Submission/Application/Services/BlobStorageService.cs(2 hunks)DotNet/Submission/Controllers/SubmissionController.cs(2 hunks)Tests/BackendE2ETests/AdhocReportingSmokeTest.cs(1 hunks)Tests/BackendE2ETests/ApiRequests/AdHocReportApiRequests.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs
⚙️ CodeRabbit configuration file
**/*.cs: TheHtmlInputSanitizerclass'sSanitize()andSanitizeAndRemove()methods should be used when dealing withstringquery parameters from REST requests.
Files:
Tests/BackendE2ETests/AdhocReportingSmokeTest.csTests/BackendE2ETests/ApiRequests/AdHocReportApiRequests.csDotNet/Submission/Controllers/SubmissionController.csDotNet/Submission/Application/Services/BlobStorageService.cs
**
⚙️ CodeRabbit configuration file
**: Pull requests that have "TECH_DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates,
and logging improvements. These TECH_DEBT PRs must not affect core functionality. All PRs that are not considered technical debt must include information on what
testing was performed in the description of the PR. If it does not, ask the author to provide details on what testing was performed.
When reviewing code, suggest unit tests using XUnit in the following scenarios:
- If/Else or Switch/Case blocks are introduced or modified — ensure each branch has a corresponding unit test.
- Logic that depends on service or interface configuration — suggest tests to validate different implementations are correctly resolved.
- No network activity (HTTP calls, sockets, etc.) should appear in unit tests. Recommend using mocks (via Moq) for any external communication.
Large unit tests should be avoided; keeping unit tests small and focused on targeted business logic (i.e. string sanitization)
**: Pull requests that have DOCS in the title should only contain changes related to documentation within the /docs folder or in .md files through-out the code-base. The description
of the PR should specify what documentation was updated. Documentation updates should use EventCatalog.dev structure, where service-specific functionality should be described
in the service's index.mdx (i.e. /services/XXX/index.mdx or /domains/XXX/services/YYY/index.mdx). Configurations that are shared by multiple services should be
reflected in the /docs/docs/config files.
Files:
Tests/BackendE2ETests/AdhocReportingSmokeTest.csTests/BackendE2ETests/ApiRequests/AdHocReportApiRequests.csDotNet/Submission/Controllers/SubmissionController.csDotNet/Submission/Application/Services/BlobStorageService.cs
🧠 Learnings (6)
📓 Common learnings
Learnt from: smailliwcs
Repo: lantanagroup/link-cloud PR: 968
File: DotNet/Submission/Controllers/SubmissionController.cs:77-83
Timestamp: 2025-08-05T20:23:10.394Z
Learning: In the Submission service's SubmissionController.DownloadReport method, smailliwcs confirmed that error handling for blob storage operations is not needed since the endpoint is intended only for QA workflows, following the pattern of keeping QA endpoints simpler than production workflows.
Learnt from: smailliwcs
Repo: lantanagroup/link-cloud PR: 968
File: DotNet/Submission/Application/Services/BlobStorageService.cs:74-76
Timestamp: 2025-08-05T20:11:11.132Z
Learning: In the Link Cloud Submission service's BlobStorageService, smailliwcs deferred implementing container validation for PayloadUri until it's confirmed that strict validation aligns with the intended system architecture and security requirements.
📚 Learning: 2025-03-20T22:11:00.226Z
Learnt from: amphillipsLGC
Repo: lantanagroup/link-cloud PR: 737
File: DotNet/Admin.BFF/Presentation/Endpoints/Aggregation/Handlers/Report/GetReportSummaries.cs:23-23
Timestamp: 2025-03-20T22:11:00.226Z
Learning: The facilityId validation in GetReportSummaries.Handle method in DotNet/Admin.BFF/Presentation/Endpoints/Aggregation/Handlers/Report/GetReportSummaries.cs will be implemented in a future phase of work by amphillipsLGC.
Applied to files:
Tests/BackendE2ETests/AdhocReportingSmokeTest.csTests/BackendE2ETests/ApiRequests/AdHocReportApiRequests.csDotNet/Submission/Controllers/SubmissionController.cs
📚 Learning: 2025-08-05T20:23:10.394Z
Learnt from: smailliwcs
Repo: lantanagroup/link-cloud PR: 968
File: DotNet/Submission/Controllers/SubmissionController.cs:77-83
Timestamp: 2025-08-05T20:23:10.394Z
Learning: In the Submission service's SubmissionController.DownloadReport method, smailliwcs confirmed that error handling for blob storage operations is not needed since the endpoint is intended only for QA workflows, following the pattern of keeping QA endpoints simpler than production workflows.
Applied to files:
Tests/BackendE2ETests/AdhocReportingSmokeTest.csDotNet/Submission/Controllers/SubmissionController.csDotNet/Submission/Application/Services/BlobStorageService.cs
📚 Learning: 2025-05-27T16:32:09.282Z
Learnt from: smailliwcs
Repo: lantanagroup/link-cloud PR: 851
File: Tests/BackendE2ETests/AdhocReportingSmokeTest.cs:16-16
Timestamp: 2025-05-27T16:32:09.282Z
Learning: The smoke test (AdhocReportingSmokeTest) in Tests/BackendE2ETests/AdhocReportingSmokeTest.cs typically takes around 90 seconds to run locally, but uses a 5-minute timeout (MaxRetryCount = 60 with 5-second polling intervals) to account for potentially longer cloud-based runtimes.
Applied to files:
Tests/BackendE2ETests/AdhocReportingSmokeTest.cs
📚 Learning: 2025-08-05T16:54:22.837Z
Learnt from: smailliwcs
Repo: lantanagroup/link-cloud PR: 1042
File: DotNet/Submission/Controllers/SubmissionController.cs:79-83
Timestamp: 2025-08-05T16:54:22.837Z
Learning: The SubmissionController's DownloadReport endpoint in the Submission service is intended only for QA workflows, so memory optimization for large file compression is not necessary.
Applied to files:
DotNet/Submission/Controllers/SubmissionController.cs
📚 Learning: 2025-08-05T20:11:11.132Z
Learnt from: smailliwcs
Repo: lantanagroup/link-cloud PR: 968
File: DotNet/Submission/Application/Services/BlobStorageService.cs:74-76
Timestamp: 2025-08-05T20:11:11.132Z
Learning: In the Link Cloud Submission service's BlobStorageService, smailliwcs deferred implementing container validation for PayloadUri until it's confirmed that strict validation aligns with the intended system architecture and security requirements.
Applied to files:
DotNet/Submission/Application/Services/BlobStorageService.cs
🧬 Code graph analysis (3)
Tests/BackendE2ETests/ApiRequests/AdHocReportApiRequests.cs (1)
Tests/BackendE2ETests/TestConfig.cs (1)
TestConfig(5-122)
DotNet/Submission/Controllers/SubmissionController.cs (2)
DotNet/Submission/Application/Services/BlobStorageService.cs (5)
Task(65-86)Task(93-131)Task(133-148)Task(150-159)Task(161-170)Tests/BackendE2ETests/AdhocReportingSmokeTest.cs (11)
Task(21-39)Task(41-58)Task(149-196)Task(197-242)Task(243-254)Task(255-266)Task(267-293)Task(294-323)Task(325-340)Task(342-831)Task(834-848)
DotNet/Submission/Application/Services/BlobStorageService.cs (1)
DotNet/Report/Services/BlobStorageService.cs (1)
Uri(51-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Unit Tests for Java
- GitHub Check: Unit Tests for DotNet
- GitHub Check: Smoke Test with Docker Compose
- GitHub Check: Integration Tests
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (6)
Tests/BackendE2ETests/AdhocReportingSmokeTest.cs (1)
199-199: LGTM! Smoke test now exercises external download path.The addition of
?external=truealigns with the PR objectives to enable full end-to-end smoke testing against external ABS.Tests/BackendE2ETests/ApiRequests/AdHocReportApiRequests.cs (1)
1025-1025: LGTM! Consistent with smoke test changes.The addition of
?external=trueenables API request helper to exercise the external download path.DotNet/Submission/Controllers/SubmissionController.cs (1)
37-40: LGTM! Default to internal storage aligns with PR objectives.The
externalquery parameter with default valuefalsecorrectly implements the requirement to default to internal ABS while allowing external selection when needed.DotNet/Submission/Application/Services/BlobStorageService.cs (3)
133-148: Shared download helper reduces duplication.The private
DownloadAsynchelper effectively eliminates code duplication between internal and external download paths. The use ofblob.Name.Split('/').Last()(line 144) to extract filenames assumes that all blobs under the given prefix have unique last path segments. This is valid given the upload logic creates unique filenames per report (manifest.ndjson, patient-{id}.ndjson, etc.), but would fail if the prefix spans multiple reports with identically-named files.
150-159: LGTM! New overload supports internal report downloads.The new
DownloadFromInternalAsyncoverload correctly validates internal client availability and delegates to the shared helper with the appropriate prefix.
161-170: LGTM! External download correctly adjusts blob prefix.The method properly transforms the internal blob prefix to external using
ChangeBlobRootbefore delegating to the shared helper.
2 participants
🛠️ Description of Changes
Support download from both (internal and external) blob containers through the Submission API. By default, download from internal (since submissions will be removed from external after downstream ingestion). But for smoke testing, download from external in order to test full end-to-end flow.
🧪 Testing Performed
Tested locally (and as part of automated smoke testing).
🧑🔬 Unit Testing
N/A
📓 Documentation Updated
N/A
Summary by CodeRabbit