Skip to content

Conversation

@LTA-Thinking
Copy link
Contributor

@LTA-Thinking LTA-Thinking commented Nov 18, 2025

Description

When using _sort and _include/_revinclude together there are issues when there are enough included resources to create an include continuation token.

  • If sorting by last updated descending the included results won't be in the bundle and no include continuation token will be returned.
  • If sorting by any other field it will cause a 500 if there are enough results with the sort field to fill a page of data.
  • If during a search the number of matched results with the sort value isn't enough to fill a page, but is enough to generate an includes continuation token, the includes continuation token will be lost when the second search is run to get results that don't contain the sort value.
  • If an includes continuation token is used with a sort field it will get data for both matches that contain the sort field and those that don't contain the sort field regardless of which type of matched result generated the includes continuation token.

This PR fixes all of these issues.

Related issues

Addresses Bug 177120

Testing

Manual testing and new E2E tests.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch

@LTA-Thinking LTA-Thinking added Bug Bug bug bug. Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs labels Nov 19, 2025
@LTA-Thinking LTA-Thinking added this to the FY26\Q2\2Wk\2Wk11 milestone Nov 19, 2025
Robert Johnson added 2 commits November 20, 2025 09:39
@LTA-Thinking LTA-Thinking marked this pull request as ready for review November 20, 2025 21:36
@LTA-Thinking LTA-Thinking requested a review from a team as a code owner November 20, 2025 21:36
Copy link
Contributor

@mikaelweave mikaelweave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple questions

if (_tokens.Length == 5
&& short.TryParse(_tokens[3]?.ToString(), out tid)
&& long.TryParse(_tokens[4]?.ToString(), out sid0))
if (_tokens.Length >= 5)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know a lot this code was existing but I'm seeing a lot of magic numbers in this block. Any chance you can add some comments to improve readability for those new to this area?

if (includesContinuationToken == null)
{
_logger.LogWarning("Bad Request (InvalidIncludesContinuationToken)");
throw new BadRequestException(Resources.InvalidIncludesContinuationToken);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a BadRequestException correct here? If the token is malformed is it an issue from the client or in our code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the safer expectation is that the client passed in the wrong value for the CT. It is possible for us to have a bug in the CT generation code, but I don't think that is the right assumption to take with a user provided value.

@LTA-Thinking
Copy link
Contributor Author

There are still more edge cases to fix here. The behavior when using a second phase includes continuation token after the second page of first phase includes results isn't working correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Bug Bug bug bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants