-
Notifications
You must be signed in to change notification settings - Fork 574
Updates healthcare shared components #5235
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.
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
base: main
Are you sure you want to change the base?
Conversation
ae54710 to
e60562d
Compare
| } | ||
|
|
||
| resources.AddRange(patients.Reverse()); | ||
| resources.AddRange(((IEnumerable<Patient>)patients).Reverse()); |
Check warning
Code scanning / CodeQL
Useless upcast Warning test
Patient\[\]
IEnumerable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
In general, the fix for a "useless upcast" is to remove the explicit cast and let the compiler apply the implicit conversion. This keeps the code simpler without affecting functionality.
Here, on line 492, we have resources.AddRange(((IEnumerable<Patient>)patients).Reverse());. The explicit cast to IEnumerable<Patient> is unnecessary because patients is a Patient[], which already implements IEnumerable<Patient>, and Reverse() is an extension method on IEnumerable<T>. The best minimal fix is to remove the cast and call Reverse() directly on patients, relying on type inference and implicit conversion. No additional imports or helper methods are needed, and existing behavior (adding the reversed patient sequence to resources) remains identical.
Concretely:
- In
test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs, locate theresources.AddRange(((IEnumerable<Patient>)patients).Reverse());line. - Replace it with
resources.AddRange(patients.Reverse());. - No other code or imports need to be changed.
-
Copy modified line R492
| @@ -489,7 +489,7 @@ | ||
| observations.Add(obs.First()); | ||
| } | ||
|
|
||
| resources.AddRange(((IEnumerable<Patient>)patients).Reverse()); | ||
| resources.AddRange(patients.Reverse()); | ||
| resources.AddRange(observations); | ||
|
|
||
| // Ask to get all patient with specific tag order by birthdate (timestamp) |
| } | ||
|
|
||
| resources.AddRange(patients.Reverse()); | ||
| resources.AddRange(((IEnumerable<Patient>)patients).Reverse()); |
Check warning
Code scanning / CodeQL
Useless upcast Warning test
Patient\[\]
IEnumerable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To fix a useless upcast, remove the explicit cast and rely on the existing implicit conversion. Here, patients is a Patient[], which already implements IEnumerable<Patient>. List<Resource>.AddRange takes IEnumerable<Resource>, and because Patient derives from Resource, the array can be used directly without any cast.
The specific change is in test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs at line 564. Replace:
resources.AddRange(((IEnumerable<Patient>)patients).Reverse());with:
resources.AddRange(patients.Reverse());No additional methods, imports, or definitions are needed. The behavior remains the same: patients is enumerated in reverse order and each Patient (a Resource) is added to the resources list.
-
Copy modified line R564
| @@ -561,7 +561,7 @@ | ||
| observations.Add(obs.First()); | ||
| } | ||
|
|
||
| resources.AddRange(((IEnumerable<Patient>)patients).Reverse()); | ||
| resources.AddRange(patients.Reverse()); | ||
| observations.Reverse(); | ||
| resources.AddRange(observations); | ||
|
|
|
|
||
| var request = new CreateExportRequest(RequestUrl, ExportJobType.All, null, formatName: formatName); | ||
| CreateExportResponse response = await _createExportRequestHandler.Handle(request, _cancellationToken); | ||
| CreateExportResponse response = await _createExportRequestHandler.HandleAsync(request, _cancellationToken); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning test
response
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this issue, we should remove the useless assignment to the local variable response. We want to preserve the await of the asynchronous method to ensure program logic and timing remains unchanged. Thus, instead of assigning to a variable, we should simply await the operation on its own. All remaining code is unchanged, since none relies on the value assigned to response. Edits are needed for both of the affected methods in the file: remove the assignment and just await the method call.
-
Copy modified line R389 -
Copy modified line R410
| @@ -386,7 +386,7 @@ | ||
| Arg.Any<CancellationToken>()); | ||
|
|
||
| var request = new CreateExportRequest(RequestUrl, ExportJobType.All, null, formatName: formatName); | ||
| CreateExportResponse response = await _createExportRequestHandler.HandleAsync(request, _cancellationToken); | ||
| await _createExportRequestHandler.HandleAsync(request, _cancellationToken); | ||
|
|
||
| Assert.Equal(expectedFormat, actualRecord.ExportFormat); | ||
| } | ||
| @@ -407,7 +407,7 @@ | ||
| Arg.Any<CancellationToken>()); | ||
|
|
||
| var request = new CreateExportRequest(RequestUrl, ExportJobType.All, containerName: containerSpecified ? "test" : null); | ||
| CreateExportResponse response = await _createExportRequestHandler.HandleAsync(request, _cancellationToken); | ||
| await _createExportRequestHandler.HandleAsync(request, _cancellationToken); | ||
|
|
||
| Assert.Equal(expectedFormat, actualRecord.ExportFormat); | ||
| } |
|
|
||
| var request = new CreateExportRequest(RequestUrl, ExportJobType.All, containerName: containerSpecified ? "test" : null); | ||
| CreateExportResponse response = await _createExportRequestHandler.Handle(request, _cancellationToken); | ||
| CreateExportResponse response = await _createExportRequestHandler.HandleAsync(request, _cancellationToken); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning test
response
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
The best fix is very simple: remove the assignment of the result of _createExportRequestHandler.HandleAsync to the unused local variable response. Instead, simply await the method directly (without capturing its result), preserving proper sequencing and exception propagation, but eliminating unnecessary unused assignment. Only the assignment line itself (CreateExportResponse response = await ...) needs to be replaced with an awaited call without the variable and type declaration. No additional imports, methods, or definitions are needed.
-
Copy modified line R410
| @@ -407,7 +407,7 @@ | ||
| Arg.Any<CancellationToken>()); | ||
|
|
||
| var request = new CreateExportRequest(RequestUrl, ExportJobType.All, containerName: containerSpecified ? "test" : null); | ||
| CreateExportResponse response = await _createExportRequestHandler.HandleAsync(request, _cancellationToken); | ||
| await _createExportRequestHandler.HandleAsync(request, _cancellationToken); | ||
|
|
||
| Assert.Equal(expectedFormat, actualRecord.ExportFormat); | ||
| } |
|
|
||
| var request = new CreateExportRequest(RequestUrl, ExportJobType.All, filters: filters); | ||
| CreateExportResponse response = await _createExportRequestHandler.Handle(request, _cancellationToken); | ||
| CreateExportResponse response = await _createExportRequestHandler.HandleAsync(request, _cancellationToken); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning test
response
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the problem, you should remove the assignment to the variable response while retaining the awaited call to _createExportRequestHandler.HandleAsync(...), in order to preserve any expected side effects or exceptions that may result from calling it. Specifically, in file test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Export/CreateExportRequestHandlerTests.cs, in method GivenARequestWithFilters_WhenConverted_ThenTheFiltersArePopulated, replace:
CreateExportResponse response = await _createExportRequestHandler.HandleAsync(request, _cancellationToken);with:
await _createExportRequestHandler.HandleAsync(request, _cancellationToken);No additional imports, methods, or definitions are required.
-
Copy modified line R437
| @@ -434,7 +434,7 @@ | ||
| Arg.Any<CancellationToken>()); | ||
|
|
||
| var request = new CreateExportRequest(RequestUrl, ExportJobType.All, filters: filters); | ||
| CreateExportResponse response = await _createExportRequestHandler.HandleAsync(request, _cancellationToken); | ||
| await _createExportRequestHandler.HandleAsync(request, _cancellationToken); | ||
|
|
||
| Assert.Collection( | ||
| actualRecord.Filters, |
| } | ||
|
|
||
| CreateReindexResponse response = await _createReindexRequestHandler.Handle(request, CancellationToken.None); | ||
| CreateReindexResponse response = await _createReindexRequestHandler.HandleAsync(request, CancellationToken.None); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning test
response
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To fix the problem, remove the unnecessary assignment to the local variable while still invoking _createReindexRequestHandler.HandleAsync(request, CancellationToken.None) so the test continues to exercise the code path and potentially throw exceptions. Since the test only cares that an exception is thrown (and the associated errorMessage), there is no need to capture the return value at all.
Concretely, in test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs, within the GivenOutOfRangeReindexParameter_WhenCreatingAReindexJob_ThenExceptionShouldBeThrown method, replace the line:
CreateReindexResponse response = await _createReindexRequestHandler.HandleAsync(request, CancellationToken.None);with a statement that simply awaits the task:
await _createReindexRequestHandler.HandleAsync(request, CancellationToken.None);No additional imports, methods, or definitions are required, and existing functionality is preserved because the method is still executed and any thrown exceptions are still caught and asserted.
-
Copy modified line R419
| @@ -416,7 +416,7 @@ | ||
| break; | ||
| } | ||
|
|
||
| CreateReindexResponse response = await _createReindexRequestHandler.HandleAsync(request, CancellationToken.None); | ||
| await _createReindexRequestHandler.HandleAsync(request, CancellationToken.None); | ||
| } | ||
| catch (FhirException fhirExp) | ||
| { |
7c41da5 to
dc2ac20
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
f1158bb to
37981ca
Compare
37981ca to
cddf0ba
Compare
34f2691 to
79a897e
Compare
- Fix DeleteSearchParameterBehavior.cs to use Medino instead of MediatR - Update test method calls from Handle to HandleAsync for Medino compatibility - Add ConfigureAwait(false) to async enumeration in ResourceManagerCollectionSetup These changes resolve build failures that were preventing CosmosDB deployments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Update test calls from Handle to HandleAsync - Handle method is now protected, tests must use public HandleAsync 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
79a897e to
0e5a915
Compare
Description
This pull request migrates the project from using the
MediatRlibrary to theMedinolibrary for in-process messaging and notifications. All usages, references, and documentation have been updated accordingly. Additionally, the minimum version of the shared healthcare package has been updated. The most significant changes are summarized below.Dependency and Package Management:
Replaced the
MediatRNuGet package withMedinoin all project and props files, and updated the shared healthcare package version from10.0.68to11.0.3. (Directory.Packages.props[1] [2];src/Microsoft.Health.Fhir.Api/Microsoft.Health.Fhir.Api.csproj[3]Updated third-party notices to reflect the switch from MediatR 9.0.0 to Medino 3.0.2. (
THIRDPARTYNOTICES.mdTHIRDPARTYNOTICES.mdL542-R543)Codebase Migration to Medino:
Replaced all
using MediatR;statements withusing Medino;across the codebase and tests. (src/Microsoft.Health.Fhir.Api/Features/ApiNotifications/ApiNotificationMiddleware.cs[1]src/Microsoft.Health.Fhir.Api/Features/BackgroundJobService/HostingBackgroundService.cs[2]src/Microsoft.Health.Fhir.Api/Features/ExceptionNotifications/ExceptionNotificationMiddleware.cs[3]src/Microsoft.Health.Fhir.Api/Features/Health/ImproperBehaviorHealthCheck.cs[4]src/Microsoft.Health.Fhir.Api/Features/Health/StorageInitializedHealthCheck.cs[5]src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs[6]src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkUpdate/BulkUpdateProcessingJobTests.cs[7]src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/Export/CancelExportRequestHandlerTests.cs[8]Updated all code and tests to use
PublishAsyncinstead ofPublishfor publishing notifications with theMedinomediator. (src/Microsoft.Health.Fhir.Api/Features/ApiNotifications/ApiNotificationMiddleware.cs[1]src/Microsoft.Health.Fhir.Api/Features/ExceptionNotifications/ExceptionNotificationMiddleware.cs[2]src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkUpdate/BulkUpdateProcessingJobTests.cs[3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]Documentation and Comments:
Medinoinstead ofMediatR. (src/Microsoft.Health.Fhir.Api/Features/ApiNotifications/ApiResponseNotification.cs[1]src/Microsoft.Health.Fhir.Api/Features/ExceptionNotifications/ExceptionNotification.cs[2]Related issues
Addresses [issue #].
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)