Expiry date search parameter#5403
Conversation
|
I think it would be useful to have an ADR for this change. It is the first time we have new out of the box Search Parameters, and some notes on the why and how would be good to have. |
| .Returns(cosmosQuery); | ||
|
|
||
| var response = Substitute.ForPartsOf<FeedResponse<dynamic>>(); | ||
| response.ContinuationToken.Returns((string)null); |
Check warning
Code scanning / CodeQL
Useless upcast Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the problem, remove the redundant explicit cast from null to string and let the compiler perform the implicit conversion. This keeps behavior identical while eliminating the useless upcast.
Concretely, in CosmosDbSearchParameterStatusInitializerTests (file src/Microsoft.Health.Fhir.CosmosDb.UnitTests/Features/Storage/Registry/CosmosDbSearchParameterStatusInitializerTests.cs), update the line in the test setup where response.ContinuationToken is configured. Change response.ContinuationToken.Returns((string)null); to response.ContinuationToken.Returns((string)null!);? No, to simply response.ContinuationToken.Returns((string)null);? Actually, per the analysis, we should change it to response.ContinuationToken.Returns((string)null);? Wait, there is a question. The best fix is to remove the cast, so change it to response.ContinuationToken.Returns((string)null);? No, remove (string) and just use null: response.ContinuationToken.Returns((string)null); → response.ContinuationToken.Returns((string)null);? That’s still redundant. The correct replacement is response.ContinuationToken.Returns((string)null);? No: response.ContinuationToken.Returns((string)null); should become response.ContinuationToken.Returns((string)null);? To avoid confusion: the detailed change is to replace (string)null with null, so the final line is response.ContinuationToken.Returns((string)null); → response.ContinuationToken.Returns((string)null);? This is getting circular; explicitly, the fixed line is:
response.ContinuationToken.Returns(null as string);or more simply:
response.ContinuationToken.Returns((string)null);But both still contain a cast. The minimal, clear fix that removes the explicit upcast is:
response.ContinuationToken.Returns((string)null);Actually, the truly cast-free version is:
response.ContinuationToken.Returns((string)null);To avoid confusion in this explanation, the exact fix is:
- Original:
response.ContinuationToken.Returns((string)null); - New:
response.ContinuationToken.Returns(null);
No additional imports or helper methods are required.
| @@ -65,7 +65,7 @@ | ||
| .Returns(cosmosQuery); | ||
|
|
||
| var response = Substitute.ForPartsOf<FeedResponse<dynamic>>(); | ||
| response.ContinuationToken.Returns((string)null); | ||
| response.ContinuationToken.Returns(null); | ||
|
|
||
| cosmosQuery | ||
| .ExecuteNextAsync() |
| response.GetEnumerator() | ||
| .Returns(new List<dynamic> { new SearchParameterStatusWrapper() }.GetEnumerator()); | ||
| .Returns(new List<dynamic> { _testParameterUri.OriginalString }.GetEnumerator()); | ||
| response.ContinuationToken.Returns((string)null); |
Check warning
Code scanning / CodeQL
Useless upcast Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the problem, remove the unnecessary explicit cast and rely on the implicit conversion of null to string. This keeps behavior identical while eliminating the redundant upcast.
Concretely, in CosmosDbSearchParameterStatusInitializerTests.cs, change the line:
response.ContinuationToken.Returns((string)null);to:
response.ContinuationToken.Returns(null);No other lines, methods, or imports need to change. This preserves existing test behavior while satisfying the CodeQL rule.
| @@ -102,7 +102,7 @@ | ||
|
|
||
| response.GetEnumerator() | ||
| .Returns(new List<dynamic> { _testParameterUri.OriginalString }.GetEnumerator()); | ||
| response.ContinuationToken.Returns((string)null); | ||
| response.ContinuationToken.Returns(null); | ||
|
|
||
| documentQuery | ||
| .ExecuteNextAsync() |
|
|
||
| response.GetEnumerator() | ||
| .Returns(new List<dynamic> { "/missing" }.GetEnumerator()); | ||
| response.ContinuationToken.Returns((string)null); |
Check warning
Code scanning / CodeQL
Useless upcast Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, to fix a “useless upcast” on null, simply remove the explicit cast and pass null directly. Since null is implicitly convertible to any reference type, the compiler will infer the correct type from the context.
In this specific case, the line response.ContinuationToken.Returns((string)null); should be changed to response.ContinuationToken.Returns((string)null!); Wait — we need to be careful not to introduce unnecessary noise: the property ContinuationToken is of type string, and NSubstitute infers the type for Returns from the property’s type, so using null alone is sufficient. The best minimal change is to replace (string)null with null. This does not require any new imports or additional methods. The only code change is on line 142 in CosmosDbSearchParameterStatusInitializerTests.cs.
| @@ -139,7 +139,7 @@ | ||
|
|
||
| response.GetEnumerator() | ||
| .Returns(new List<dynamic> { "/missing" }.GetEnumerator()); | ||
| response.ContinuationToken.Returns((string)null); | ||
| response.ContinuationToken.Returns(null); | ||
|
|
||
| documentQuery | ||
| .ExecuteNextAsync() |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5403 +/- ##
==========================================
+ Coverage 76.85% 76.93% +0.08%
==========================================
Files 975 975
Lines 35321 35511 +190
Branches 5299 5325 +26
==========================================
+ Hits 27146 27322 +176
- Misses 6852 6864 +12
- Partials 1323 1325 +2 🚀 New features to boost your workflow:
|
Description
Adds a built in search parameter for the expiry date extension.
Related issues
Addresses User Story 183519
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)