Skip to content
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

Set max item count in search options in SearchParameterDefinitionManager #2141

Merged
merged 2 commits into from
Aug 9, 2021

Conversation

feordin
Copy link
Contributor

@feordin feordin commented Aug 4, 2021

Description

Sets a value for MaxItemCount in searchOptions when searching for SearchParameter resources in SearchParameterDefinitionManager.

Downstream users of the searchOptions object expect a non-zero value to be populated in this field. When it was left at zero, a bug would be encountered which generated an invalid continuationToken causing searches to fail.

This manifested in the Stu3-CI-SQL environment, where the SearchParameterDefinitionManager would fail to initialize, then the searchParameterHash value would not be initialized, and then subsequent requests would fail with an error that the searchParameterHash was not specified for a stored procedure.

Related issues

Addresses [issue AB#84143].

Testing

Pointed my local environment at CI sql server, was able to repro the issue, made the fix and then the issue did not repro.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 50 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, Dependencies, Enhancement, or New-Feature
  • Tag the PR with Azure API for FHIR if this will release to the managed service
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)
Skip

@feordin feordin added the Bug Bug bug bug. label Aug 4, 2021
@feordin feordin added this to the S68 milestone Aug 4, 2021
@feordin feordin requested a review from a team as a code owner August 4, 2021 23:10
@@ -275,7 +276,7 @@ private async Task LoadSearchParamsFromDataStore(CancellationToken cancellationT
}
}
}
while (continuationToken != null);
while (continuationToken != null && !continuationToken.Contains("null", StringComparison.OrdinalIgnoreCase));
Copy link
Contributor

Choose a reason for hiding this comment

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

!continuationToken.Contains("null", StringComparison.OrdinalIgnoreCase))

I don't get this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! That was not necessary.

Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka left a comment

Choose a reason for hiding this comment

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

:shipit:

@feordin feordin enabled auto-merge (squash) August 5, 2021 00:03
@feordin
Copy link
Contributor Author

feordin commented Aug 5, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@feordin
Copy link
Contributor Author

feordin commented Aug 5, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@CaitlinV39 CaitlinV39 modified the milestones: S68, S69 Aug 5, 2021
@feordin feordin merged commit c8fd591 into main Aug 9, 2021
@feordin feordin deleted the personal/jaerwin/fix-search-options branch August 9, 2021 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug bug bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants