(#106) Update to Elasticsearch 8#110
Open
blairlearn wants to merge 1 commit into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| string includeNameTypesString = "[" + String.Join(',', includeNameTypes) + "]"; | ||
| string excludeNameTypesString = "[" + String.Join(',', excludeNameTypes) + "]"; | ||
| _logger.LogError(ex, $"Internal error occured expanding character: '{character}', size: '{size}', from: '{size}', includeResourceTypes: {includeResourceTypesString}, includeNameTypes: {includeNameTypesString}, excludeNameTypes: {excludeNameTypesString}."); | ||
| _logger.LogError(ex, $"Internal error occurred expanding character: '{character}', size: '{size}', from: '{from}', includeResourceTypes: {includeResourceTypesString}, includeNameTypes: {includeNameTypesString}, excludeNameTypes: {excludeNameTypesString}."); |
blairlearn
force-pushed
the
ticket/106-es8
branch
4 times, most recently
from
51f3f18 to
d3f044d
Compare
- Replace NEST client with Elastic.Clients.Elasticsearch.
- Migrate from Newtonsoft.Json to System.Text.Json.
- Refactored serialization:
- Deserializing from Elasticsearch to POCO uses JsonNamingPolicy.SnakeCaseLower via
NCI.OCPL.Api.Common NuGet component.
- Serializing from POCO to the wire uses default .Net naming policy with overrides
as needed via custom JsonConverter implmentations.
- Model classes migrated from NEST mapping attributes ([Keyword], [Number], [Nested])
to custom JsonConverters.
- In ResultsMetadata, replace fields wtih same-name properties to allow serialization
via System.Text.Json.
- Tests:
- Test mocking migrated to TestingElasticsearchClientSettingsFactory in order
to use the same ElasticsearchClientSettings initialization as executable code.
- Add xunit.analyzers package to allow xunit to properly discover test objects
with JsonNode properties.
- Modify expected query structure JSON files to match ES 8 serialization changes
(e.g. arrays with only one element become objects).
- Switch tests from async void to async Task.
- Remove unsupported (garbage) fields from test data files.
- Add test descriptions.
- Project file updates
- Target framework corrected from netcoreapp8.0 to net8.0.
- Bump NCI.OCPL.Api.Common packages to 4.0.0.
- Remove explicit NSwag dependency
- Miscellaneous
- Cleaned up using statements across many files.
- Update location for NIH TLS certificates.
- Correct typos and copy-paste comments from glossary-api.
- Remove UnitTest1.cs sample test.
Closes #106
blairlearn
force-pushed
the
ticket/106-es8
branch
from
d3f044d to
230f0ae
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Drug Dictionary API to be compatible with Elasticsearch 8 by migrating from the legacy NEST client/Newtonsoft.Json stack to Elastic.Clients.Elasticsearch and System.Text.Json, updating query/request serialization and adjusting unit/integration tests and tooling to match the new ES8 wire formats.
Changes:
- Migrated Elasticsearch client usage from NEST (
IElasticClient) toElasticsearchClient, updating request construction and response handling. - Switched JSON handling from Newtonsoft (
JObject/JToken) toSystem.Text.Json(JsonNode,JsonConverter), including new model converters and metadata serialization fixes. - Refactored test infrastructure and expected query JSON shapes (ES8 serialization differences) and modernized async tests (
async Task).
Reviewed changes
Copilot reviewed 76 out of 76 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/NCI.OCPL.Api.DrugDictionary.Tests/UnitTest1.cs | Removes placeholder sample test. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/SearchSvc_ZeroOffset_Trametinib.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/SearchSvc_Contains_Paclitaxel.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/SearchSvc_Contains_LongName.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/SearchSvc_Contains_Cetuximab.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/SearchSvc_Begins_Olaparib.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/SearchSvc_Begins_LongName.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/Search/SearchResults_Data_Base.cs | Fixes XML doc comment typo in test data base. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetByName_WithDashes.cs | Updates expected ES request JSON to System.Text.Json + ES8 query shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetByName_olaparib.cs | Updates expected ES request JSON to System.Text.Json + ES8 query shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetByName_LongPrettyURL.cs | Updates expected ES request JSON to System.Text.Json + ES8 query shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetAllSvc_Contains_SingleResourceType_SingleInclude_SingleExclude.cs | Updates expected ES request JSON to System.Text.Json + ES8 bool/must_not shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetAllSvc_Contains_MultipleResourceTypes_No_Include_MultipleExcludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 bool must/must_not shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetAllSvc_Contains_MultipleResourceTypes_MultipleIncludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetAllSvc_Begins_SingleResourceType_SingleInclude_SingleExclude.cs | Updates expected ES request JSON to System.Text.Json + ES8 bool/must_not shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetAllSvc_Begins_MultipleResourceTypes_No_Include_MultipleExcludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 bool must/must_not shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/GetAllSvc_Begins_MultipleResourceTypes_MultipleIncludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/ExpandSvc_Contains_SingleResourceType_SingleInclude_SingleExclude.cs | Updates expected ES request JSON to System.Text.Json + ES8 bool/must_not shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/ExpandSvc_Contains_MultipleResourceTypes_No_Include_MultipleExcludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 bool/must_not shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/ExpandSvc_Contains_MultipleResourceTypes_MultipleIncludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/ExpandSvc_Begins_SingleResourceType_SingleInclude_SingleExclude.cs | Updates expected ES request JSON to System.Text.Json + ES8 bool/must_not shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/ExpandSvc_Begins_MultipleResourceTypes_No_Include_MultipleExcludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 bool/must_not shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/ExpandSvc_Begins_MultipleResourceTypes_MultipleIncludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/BaseSearchSvcRequestScenario.cs | Switches ExpectedData to JsonNode and adds IXunitSerializable to avoid discovery issues. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/BaseGetByNameSvcRequestScenario.cs | Switches ExpectedData to JsonNode and adds IXunitSerializable to avoid discovery issues. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/BaseGetAllSvcRequestScenario.cs | Switches ExpectedData to JsonNode and adds IXunitSerializable to avoid discovery issues. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/DrugsService/BaseExpandSvcRequestScenario.cs | Switches ExpectedData to JsonNode and adds IXunitSerializable to avoid discovery issues. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/AutosuggestService/BaseAutosuggestServiceScenario.cs | Switches ExpectedData to JsonNode and adds IXunitSerializable to avoid discovery issues. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/AutosuggestService/AutosuggestSvc_Contains_SingleResourceType_SingleInclude_SingleExclude.cs | Updates expected ES request JSON to System.Text.Json + ES8 filter/sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/AutosuggestService/AutosuggestSvc_Contains_MultipleResourceTypes_No_Include_MultipleExcludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 filter/sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/AutosuggestService/AutosuggestSvc_Contains_MultipleResourceTypes_MultipleIncludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 must_not/filter/sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/AutosuggestService/AutosuggestSvc_Begins_SingleResourceType_SingleInclude_SingleExclude.cs | Updates expected ES request JSON to System.Text.Json + ES8 must_not/filter/sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/AutosuggestService/AutosuggestSvc_Begins_MultipleResourceTypes_No_Include_MultipleExcludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 must_not/filter/sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/TestDataObjects/AutosuggestService/AutosuggestSvc_Begins_MultipleResourceTypes_MultipleIncludes.cs | Updates expected ES request JSON to System.Text.Json + ES8 filter/sort shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Services/ESDrugsQueryServiceTest.Search.cs | Migrates tests to Elastic.Clients + JsonNode request inspection; converts async void → Task. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Services/ESDrugsQueryServiceTest.GetByName.cs | Migrates tests to Elastic.Clients + JsonNode request inspection; converts async void → Task. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Services/ESDrugsQueryServiceTest.GetById.cs | Migrates tests to Elastic.Clients interception pattern; converts async void → Task; updates ES8 response JSON. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Services/ESDrugsQueryServiceTest.GetAll.cs | Migrates tests to Elastic.Clients interception pattern; converts async void → Task. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Services/ESDrugsQueryServiceTest.Expand.cs | Migrates tests to Elastic.Clients interception pattern; converts async void → Task; improves doc comments. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Services/ESDrugsQueryServiceTest._Common.cs | Refactors mock ES responses to string bodies and ES8 hits.total shape. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Services/ESAutosuggestQueryServiceTest.Request.cs | Migrates autosuggest request tests to Elastic.Clients interception + JsonNode. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Models/IDrugResourceComparer.cs | Fixes spelling in exception messages; removes unused usings. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Controllers/HealthCheckControllerTests.cs | Converts async void tests to async Task. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Controllers/DrugsControllerTests.Search.cs | Converts async void tests to async Task; removes unused using. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Controllers/DrugsControllerTests.GetByName.cs | Converts async void tests to async Task. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Controllers/DrugsControllerTests.GetById.cs | Converts async void tests to async Task; fixes comment typo. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Controllers/DrugsControllerTests.GetAll.cs | Converts async void tests to async Task; fixes comment typos. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Controllers/DrugsControllerTests.Expand.cs | Converts async void tests to async Task; removes unused usings. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Controllers/DrugsControllerTests._Constants.cs | Removes unused usings from constants partial. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/Tests/Controllers/AutosuggestControllerTest.cs | Converts async void tests to async Task; fixes comment typo. |
| test/NCI.OCPL.Api.DrugDictionary.Tests/NCI.OCPL.Api.DrugDictionary.Tests.csproj | Updates target framework, test packages, and shared component versions; adds xunit analyzers. |
| src/NCI.OCPL.Api.DrugDictionary/Services/ESDrugsQueryService.cs | Migrates query service to Elastic.Clients, updates request construction, and adjusts response validity handling. |
| src/NCI.OCPL.Api.DrugDictionary/Services/ESAutosuggestQueryService.cs | Migrates autosuggest service to Elastic.Clients, updates query construction, and fixes error message spelling. |
| src/NCI.OCPL.Api.DrugDictionary/NCI.OCPL.Api.DrugDictionary.csproj | Updates target framework to net8.0 and switches to Elastic.Clients.Elasticsearch + shared components v4. |
| src/NCI.OCPL.Api.DrugDictionary/Models/TermNameType.cs | Migrates enum JSON conversion to System.Text.Json. |
| src/NCI.OCPL.Api.DrugDictionary/Models/TermAlias.cs | Removes NEST mapping attributes (migration to STJ-based handling). |
| src/NCI.OCPL.Api.DrugDictionary/Models/SuggestionConverter.cs | Adds STJ converter for autosuggest suggestion naming and shape. |
| src/NCI.OCPL.Api.DrugDictionary/Models/Suggestion.cs | Applies STJ converter and removes NEST attributes. |
| src/NCI.OCPL.Api.DrugDictionary/Models/ResultsMetadata.cs | Converts fields to properties so System.Text.Json can serialize metadata. |
| src/NCI.OCPL.Api.DrugDictionary/Models/MatchType.cs | Migrates enum JSON conversion to System.Text.Json. |
| src/NCI.OCPL.Api.DrugDictionary/Models/IDrugResource.cs | Migrates to STJ converter attribute for polymorphic deserialization. |
| src/NCI.OCPL.Api.DrugDictionary/Models/DrugTerm.cs | Removes NEST mapping attributes (migration to STJ-based handling). |
| src/NCI.OCPL.Api.DrugDictionary/Models/DrugResourceType.cs | Migrates enum JSON conversion to System.Text.Json. |
| src/NCI.OCPL.Api.DrugDictionary/Models/DrugResourceConverter.cs | Replaces Newtonsoft-based polymorphic converter with System.Text.Json converter (read/write). |
| src/NCI.OCPL.Api.DrugDictionary/Models/DrugResource.cs | Removes NEST mapping attributes; fixes doc typos. |
| src/NCI.OCPL.Api.DrugDictionary/Models/DrugInfoSummaryLinkConverter.cs | Adds STJ converter for ES field name vs API field name translation. |
| src/NCI.OCPL.Api.DrugDictionary/Models/DrugInfoSummaryLink.cs | Applies STJ converter and removes NEST attributes. |
| src/NCI.OCPL.Api.DrugDictionary/Models/DrugAlias.cs | Removes NEST mapping attributes. |
| src/NCI.OCPL.Api.DrugDictionary/Models/Definition.cs | Removes NEST mapping attributes. |
| src/NCI.OCPL.Api.DrugDictionary/Interfaces/IAutosuggestQueryService.cs | Updates interface documentation to reflect drug term domain. |
| src/NCI.OCPL.Api.DrugDictionary/Controllers/DrugsController.cs | Fixes error message spelling and corrects logged from value in Expand error path. |
| integration-tests/README.md | Fixes spelling/typos in integration test documentation. |
| integration-tests/docker-drug-api/elasticsearch/Dockerfile | Updates Elasticsearch image to 8.19.12 and adjusts NIH cert installation steps. |
| integration-tests/docker-drug-api/docker-compose.yml | Updates Elasticsearch security-related settings for ES8 and adjusts volumes. |
| integration-tests/docker-drug-api/api/Dockerfile | Updates NIH cert installation URL locations in the integration test API container. |
| .github/workflows/ci.yml | Updates shared CI workflow reference to v4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+25
to
+27
| long termId = root.TryGetProperty("term_id", out JsonElement termIdElement) | ||
| ? long.Parse(termIdElement.GetString()) | ||
| : 0; |
Comment on lines
+10
to
+14
| /// - The Uri property is serialized as "url". | ||
| /// - The Text is serialized as "text". | ||
| /// No special handling is needed for deserialization, the application is expected to | ||
| /// set the correct NamingPolicy for deserializing from Elasticsearch. | ||
| /// </summary> |
Comment on lines
13
to
+17
| # Turn off warnings about not using HTTPS. | ||
| - xpack.security.enabled=false | ||
| # Turn security settings off for testing. (Allows http instead of https.) | ||
| - xpack.security.enabled=false | ||
| - xpack.security.transport.ssl.enabled=false |
Comment on lines
49
to
+52
| # Use the user's existing GitHub credentials | ||
| - ~/.nuget/NuGet/NuGet.Config:/root/.nuget/NuGet/NuGet.Config | ||
|
|
||
| - ~/MyNuGet:/Users/learnb/MyNuGet |
Comment on lines
7
to
+10
| RUN mkdir /usr/local/share/ca-certificates/nih \ | ||
| && curl -o /usr/local/share/ca-certificates/nih/NIH-DPKI-ROOT-1A-base64.crt https://ocio.nih.gov/Smartcard/Documents/Certificates/NIH-DPKI-ROOT-1A-base64.cer \ | ||
| && curl -o /usr/local/share/ca-certificates/nih/NIH-DPKI-CA-1A-base64.crt https://ocio.nih.gov/Smartcard/Documents/Certificates/NIH-DPKI-CA-1A-base64.cer \ | ||
| && update-ca-certificates | ||
| && curl -o /usr/local/share/ca-certificates/nih/NIH-DPKI-ROOT-1A-base64.crt http://nihdpkicrl.nih.gov/certchains/NIH-DPKI-CA-1A%281%29.crt \ | ||
| && curl -o /usr/local/share/ca-certificates/nih/NIH-DPKI-CA-1A-base64.crt http://nihdpkicrl.nih.gov/certchains/NIH-DPKI-ROOT-1A.cer \ | ||
| && update-ca-certificates |
Comment on lines
6
to
+9
| RUN mkdir /usr/local/share/ca-certificates/nih \ | ||
| && curl -o /usr/local/share/ca-certificates/nih/NIH-DPKI-ROOT-1A-base64.crt https://ocio.nih.gov/Smartcard/Documents/Certificates/NIH-DPKI-ROOT-1A-base64.cer \ | ||
| && curl -o /usr/local/share/ca-certificates/nih/NIH-DPKI-CA-1A-base64.crt https://ocio.nih.gov/Smartcard/Documents/Certificates/NIH-DPKI-CA-1A-base64.cer \ | ||
| && update-ca-certificates | ||
| && curl -o /usr/local/share/ca-certificates/nih/NIH-DPKI-ROOT-1A-base64.crt http://nihdpkicrl.nih.gov/certchains/NIH-DPKI-CA-1A%281%29.crt \ | ||
| && curl -o /usr/local/share/ca-certificates/nih/NIH-DPKI-CA-1A-base64.crt http://nihdpkicrl.nih.gov/certchains/NIH-DPKI-ROOT-1A.cer \ | ||
| && update-ca-certificates |
Comment on lines
65
to
+67
| Assert.Equal(HttpMethod.POST, esMethod); | ||
| Assert.Equal(data.ExpectedData, requestBody, new JTokenEqualityComparer()); | ||
| //Assert.Equal(data.ExpectedData, requestBody, new JTokenEqualityComparer()); | ||
| Assert.True(JsonNode.DeepEquals(data.ExpectedData, requestBody)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace NEST client with Elastic.Clients.Elasticsearch.
Migrate from Newtonsoft.Json to System.Text.Json.
Refactored serialization:
NCI.OCPL.Api.Common NuGet component.
as needed via custom JsonConverter implmentations.
to custom JsonConverters.
via System.Text.Json.
Tests:
to use the same ElasticsearchClientSettings initialization as executable code.
with JsonNode properties.
(e.g. arrays with only one element become objects).
Project file updates
Miscellaneous
Closes #106