diff --git a/.editorconfig b/.editorconfig index dc50b66..5fec909 100644 --- a/.editorconfig +++ b/.editorconfig @@ -5,7 +5,7 @@ root = true [*] trim_trailing_whitespace = true -indent_size = 4 +indent_size = 2 indent_style = space end_of_line = lf diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..83e0d81 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,17 @@ +When working with files in this repository, please follow these guidelines: + +- If you don't know how do something, say you don't know. Do not make assumptions. +- If an instruction is unclear, ask for clarification. +- Write clear and concise code comments where necessary. +- Follow the existing coding style and conventions used in the repository. +- Ensure proper error handling and input validation. + +- Use the modern Elastic.Clients.Elasticsearch. +- Use System.Text.Json instead of Newtonsoft.Json. +- Remove any unneeded Using statements. +- Using statements should be split into groups in this order: + 1. "Out of the box" packages. + 2. Third-party packages (e.g. the elasticsearch packages) + 3. Packages which are part of this solution. +- Within a group, Using statements should be in alphabetical order. +- When committing changes, include a brief summary of what was changed and why. \ No newline at end of file diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index ba40eea..8eb52ca 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -39,10 +39,10 @@ jobs: steps: - name: Check out code - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Setup dotnet - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: global-json-file: global.json @@ -58,10 +58,10 @@ jobs: dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=lcov /p:CoverletOutput=../../${{matrix.package}}.lcov test/${{matrix.package}}.Tests fi - - name: Save Code Coverage Output + - name: Save code coverage output # Coverage output isn't created on Windows, so don't upload it. if: matrix.operating-system != 'windows-latest' - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v6 with: name: coverage-${{matrix.package}}-${{ matrix.operating-system }} path: ${{matrix.package}}.lcov @@ -86,7 +86,7 @@ jobs: output-name: "${{ github.workspace }}/out/wwwroot/build-info.json" - name: Upload Published Artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v6 with: name: ${{matrix.package}}-${{ matrix.operating-system }} path: out @@ -109,13 +109,29 @@ jobs: steps: - name: Check out code - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Setup dotnet - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: global-json-file: global.json + - name: Set up reporting tools + shell: bash + run: | + dotnet tool install --global coverlet.console --version 6.* + dotnet tool install --global dotnet-reportgenerator-globaltool --version 5.* + mkdir -p "${{github.workspace}}/coverage-report" + + - name: Retrieve unit test coverage reports + uses: actions/download-artifact@v7 + with: + name: coverage-NCI.OCPL.Api.Common-ubuntu-latest + + - name: Rename unit test coverage file + run: | + mv "${{github.workspace}}/NCI.OCPL.Api.Common.lcov" "${{github.workspace}}/integration-coverage.lcov" + - name: Setup test output directory shell: bash run: | @@ -141,12 +157,16 @@ jobs: APP_ASSEMBLY: integration-test-harness.dll shell: bash run: | - ## TODO: This should become a GitHub Action. - - ## Start the app and log output + ## Start the app and log output while recording code coverage. ## NOTE: We must change directory because while you can call `dotnet "${APP_PATH}/${APP_ASSEMBLY}"` ## it will not find the appsettings.json, so we must cd into the APP_PATH first - cd $APP_PATH && dotnet $APP_ASSEMBLY > ../integration-tests/target/api_log.txt 2>&1 & + cd $APP_PATH && \ + coverlet . --target "dotnet" \ + --targetargs "$APP_ASSEMBLY" \ + --format lcov \ + --include-directory "." \ + --include "[NCI.OCPL.Api.Common]*" \ + --merge-with "${{github.workspace}}/integration-coverage.lcov" > ../integration-tests/target/api_log.txt 2>&1 & time_waited=1 echo "Checking status of ${API_URL}." @@ -164,6 +184,13 @@ jobs: echo "API is up" + - name: Save the API startup Log + if: always() + uses: actions/upload-artifact@v6 + with: + name: integration-tests-api-startup-log + path: integration-tests/target/api_log.txt + - name: Run tests requiring no data be present. ## Normally bash runs with -e which exits the shell upon hitting ## an error which breaks our ability to capture those errors. @@ -190,9 +217,6 @@ jobs: ./integration-tests/bin/load-integration-data.sh - name: Run Integration Test - ## Normally bash runs with -e which exits the shell upon hitting - ## an error which breaks our ability to capture those errors. - shell: bash --noprofile --norc -o pipefail {0} run: | ## Run Karate cd integration-tests && ./bin/karate ./features @@ -203,22 +227,40 @@ jobs: echo TEST_EXIT_CODE_2=$? >> $GITHUB_ENV exit 0 + - name: Stop API + shell: bash + run: | + ## Cause a **controlled** shutdown the API so coverlet can get the coverage statistics. + curl -X POST http://localhost:5000/shutdown + - name: Upload Integration test results - uses: actions/upload-artifact@v4 + if: always() + uses: actions/upload-artifact@v6 with: name: integration-test-results path: integration-tests/target - - name: Fail build on bad tests - shell: bash + - name: Upload code coverage data + if: always() + uses: actions/upload-artifact@v6 + with: + name: coverage-integration + path: ${{github.workspace}}/integration-coverage.lcov + + - name: Generate coverage report run: | - ## Check if we had errors on the test step, and if so, fail the job - if [ $TEST_EXIT_CODE_1 -ne 0 ] || [ $TEST_EXIT_CODE_2 -ne 0 ]; then - echo "Tests Failed -- See Run Integration Test step or integration-test-results artifact for more information" - exit $TEST_EXIT_CODE - else - echo "Tests passed" - fi + reportgenerator \ + -reports:${{github.workspace}}/integration-coverage.lcov \ + -targetdir:${{github.workspace}}/coverage-report \ + -reporttypes:Html \ + -sourcedirs:${{github.workspace}} \ + -verbosity:Verbose + + - name: Upload Coverage Report + uses: actions/upload-artifact@v6 + with: + name: Code Coverage Report + path: ${{github.workspace}}/coverage-report determine_whether_to_publish: @@ -235,8 +277,6 @@ jobs: runs-on: windows-latest outputs: should_publish: ${{ steps.set_outputs.outputs.should_publish }} - needs: - - integration_tests steps: - name: Set outputs @@ -286,12 +326,12 @@ jobs: path: ${{ github.workspace }} - name: Download global.json - uses: actions/checkout@v4 + uses: actions/checkout@v6 with: path: source - name: Setup dotnet - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: global-json-file: source/global.json source-url: "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" diff --git a/integration-tests/bin/karate.jar b/integration-tests/bin/karate.jar index 6bb7d9e..67fcc9b 100644 Binary files a/integration-tests/bin/karate.jar and b/integration-tests/bin/karate.jar differ diff --git a/integration-tests/docker-nciocpl-api-common/docker-compose.yml b/integration-tests/docker-nciocpl-api-common/docker-compose.yml index 70ea8fe..4ef80ff 100644 --- a/integration-tests/docker-nciocpl-api-common/docker-compose.yml +++ b/integration-tests/docker-nciocpl-api-common/docker-compose.yml @@ -1,11 +1,20 @@ services: elasticsearch: - image: elasticsearch:7.17.28 + build: + # Point to the root of the project. + context: ../../ + # This path is relative to the context. + dockerfile: integration-tests/docker-nciocpl-api-common/elasticsearch/Dockerfile ## All of the ES settings can be set via the environment ## vars. environment: - discovery.type=single-node - ES_JAVA_OPTS=-Xms750m -Xmx750m + # Turn security settings off for testing. (Allows http instead of https.) + - xpack.security.enabled=false + - xpack.security.transport.ssl.enabled=false + - xpack.security.http.ssl.enabled=false + - http.host=0.0.0.0 ## These exposed ports are for debugging only. .NET + ## Docker + MacOS == bad scene. (.NET always wants to ## use the hosts name, and on a mac that is actually diff --git a/integration-tests/docker-nciocpl-api-common/elasticsearch/Dockerfile b/integration-tests/docker-nciocpl-api-common/elasticsearch/Dockerfile new file mode 100644 index 0000000..a1aef64 --- /dev/null +++ b/integration-tests/docker-nciocpl-api-common/elasticsearch/Dockerfile @@ -0,0 +1,12 @@ +FROM elasticsearch:8.19.8 + +USER root + +# Get the NIH Root certificates and install them so we work on VPN +# Download from https://ocio.nih.gov/Smartcard/Pages/PKI_chain.aspx +RUN mkdir -p /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 + +USER elasticsearch \ No newline at end of file diff --git a/integration-tests/features/default-route/custom-serialization.feature b/integration-tests/features/default-route/custom-serialization.feature index 5fe61aa..7fc87c9 100644 --- a/integration-tests/features/default-route/custom-serialization.feature +++ b/integration-tests/features/default-route/custom-serialization.feature @@ -8,7 +8,7 @@ Feature: The API library allows for custom deserialization of data retrieved fro Given path 'test/custom-serialization/', identifier When method get Then status 200 - And match each $.[*].default == 'Default serialization' + And match $.default == 'Default serialization' Examples: | identifier | @@ -23,6 +23,6 @@ Feature: The API library allows for custom deserialization of data retrieved fro And match $.custom == expected Examples: - | identifier | expected | - | simple-string | Took the string path | - | has-array | Took the not string path | + | identifier | expected | + | simple-string | simple-string-value | + | has-array | first-string-value | diff --git a/src/NCI.OCPL.Api.Common.Testing/ElastcsearchTestingTools.cs b/src/NCI.OCPL.Api.Common.Testing/ElastcsearchTestingTools.cs index dbf4b48..b33a25e 100644 --- a/src/NCI.OCPL.Api.Common.Testing/ElastcsearchTestingTools.cs +++ b/src/NCI.OCPL.Api.Common.Testing/ElastcsearchTestingTools.cs @@ -1,6 +1,5 @@ using System.IO; using System.Text; -using Newtonsoft.Json.Linq; namespace NCI.OCPL.Api.Common.Testing { diff --git a/src/NCI.OCPL.Api.Common.Testing/ElasticTools.cs b/src/NCI.OCPL.Api.Common.Testing/ElasticTools.cs index 162a4c6..1674db1 100644 --- a/src/NCI.OCPL.Api.Common.Testing/ElasticTools.cs +++ b/src/NCI.OCPL.Api.Common.Testing/ElasticTools.cs @@ -1,8 +1,8 @@ using System; +using System.Collections.Generic; -using Elasticsearch.Net; -using Nest; -using Nest.JsonNetSerializer; +using Elastic.Clients.Elasticsearch; +using Elastic.Transport; namespace NCI.OCPL.Api.Common.Testing { @@ -13,50 +13,62 @@ namespace NCI.OCPL.Api.Common.Testing public static class ElasticTools { /// - /// Gets an ElasticClient backed by an InMemoryConnection. This is used to mock the - /// JSON returned by the elastic search so that we test the Nest mappings to our models. + /// Gets an ElasticsearchClient backed by an InMemoryConnection. This is used to mock the + /// JSON returned by the elastic search so that we test the Elasticsearch mappings to our models. /// /// /// - public static IElasticClient GetInMemoryElasticClient(string testFile) { + public static ElasticsearchClient GetInMemoryElasticClient(string testFile) { //Get Response JSON byte[] responseBody = TestingTools.GetTestFileAsBytes(testFile); //While this has a URI, it does not matter, an InMemoryConnection never requests //from the server. - var pool = new SingleNodeConnectionPool(new Uri("http://localhost:9200")); + var pool = new SingleNodePool(new Uri("http://localhost:9200")); + + // Headers required to identify the server as a genuine Elasticsearch product. + var headers = new Dictionary> + { + { "x-elastic-product", new[] { "Elasticsearch" } } + }; // Setup ElasticSearch stuff using the contents of the JSON file as the client response. - InMemoryConnection conn = new InMemoryConnection(responseBody); + InMemoryRequestInvoker conn = new InMemoryRequestInvoker(responseBody, headers: headers); - var connectionSettings = new ConnectionSettings(pool, conn, sourceSerializer: JsonNetSerializer.Default); + var connectionSettings = new ElasticsearchClientSettings(pool, conn); - return new ElasticClient(connectionSettings); + return new ElasticsearchClient(connectionSettings); } /// - /// Gets an ElasticClient which simulates a failed request. Success is defined by + /// Gets an ElasticsearchClient which simulates a failed request. Success is defined by /// statuses with a 200-series response, so anything from the 400 or 503 series /// should be treated as an error. /// /// /// - public static IElasticClient GetErrorElasticClient(int statusCode) + public static ElasticsearchClient GetErrorElasticClient(int statusCode) { //While this has a URI, it does not matter, an InMemoryConnection never requests //from the server. - var pool = new SingleNodeConnectionPool(new Uri("http://localhost:9200")); + var pool = new SingleNodePool(new Uri("http://localhost:9200")); //Get Response JSON - byte[] responseBody = new byte[0]; + byte[] responseBody = Array.Empty(); + + // Headers required to identify the server as a genuine Elasticsearch product. + var headers = new Dictionary> + { + { "x-elastic-product", new[] { "Elasticsearch" } } + }; // Setup ElasticSearch stuff using the contents of the JSON file as the client response. - InMemoryConnection conn = new InMemoryConnection(responseBody, statusCode); + InMemoryRequestInvoker conn = new InMemoryRequestInvoker(responseBody, statusCode: statusCode, headers: headers); - var connectionSettings = new ConnectionSettings(pool, conn, sourceSerializer: JsonNetSerializer.Default); + var connectionSettings = new ElasticsearchClientSettings(pool, conn); - return new ElasticClient(connectionSettings); + return new ElasticsearchClient(connectionSettings); } } diff --git a/src/NCI.OCPL.Api.Common.Testing/ElasticsearchInterceptingConnection.cs b/src/NCI.OCPL.Api.Common.Testing/ElasticsearchInterceptingConnection.cs deleted file mode 100644 index 6c98ed5..0000000 --- a/src/NCI.OCPL.Api.Common.Testing/ElasticsearchInterceptingConnection.cs +++ /dev/null @@ -1,257 +0,0 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Reflection; -using System.Text; -using System.Threading.Tasks; - -using Elasticsearch.Net; -using Newtonsoft.Json.Linq; - -namespace NCI.OCPL.Api.Common.Testing -{ - /// - /// A mock Elasticsearch connection, allowing inspection of values that would be passed - /// to the Elasticsearch server as well as creation of simulated responses. - /// - /// - /// Use to set simulated Elasticsearch responses. - /// - public class ElasticsearchInterceptingConnection : IConnection - { - /// - /// Container for simulated Elasticsearch responses. - /// - public class ResponseData : IDisposable - { - private bool disposed; - - /// - /// Stream representing the response body. - /// - public Stream Stream { get; set; } - - /// - /// The simulated Elasticsearch HTTP status code. Required if Stream is set. - /// - public int? StatusCode { get; set; } - - /// - /// The simulated response MIME type. - /// - public string ResponseMimeType { get; set; } - - /// - /// For the IDispose pattern. - /// - protected virtual void Dispose(bool disposing) - { - if (!disposed) - { - if (disposing) - { - Stream.Dispose(); - } - - // TODO: free unmanaged resources (unmanaged objects) and override finalizer - // TODO: set large fields to null - disposed = true; - } - } - - /// - /// For the IDispose pattern. - /// - public void Dispose() - { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method - Dispose(disposing: true); - GC.SuppressFinalize(this); - } - } - - private Dictionary _callbackHandlers = new Dictionary(); - private Action _defCallbackHandler = null; - - /// - /// For the IDispose pattern. - /// - public void Dispose() - { - - } - - /// - /// Register a Request Handler for a given return type. - /// NOTE: DO NOT REGISTER BOTH A CLASS AND ITS BASE CLASS!!! - /// - /// - /// - public void RegisterRequestHandlerForType(Action callback) - where TReturn : class - { - Type returnType = typeof(TReturn); - Type handlerType = null; - - //Loop through the register handlers and see if our type is registered, OR - //if a base class is registered. - foreach (Type type in _callbackHandlers.Keys) - { - if (returnType == type || returnType.GetTypeInfo().IsSubclassOf(type)) - { - handlerType = type; - break; - } - } - - //If there is no handler, then add one. - //If there is one, then error out - if (handlerType == null) - { - _callbackHandlers.Add(typeof(TReturn), (object)callback); - } else - { - throw new ArgumentException( - String.Format( - "There is already a handler defined that would be called for type. Trying to add for: {0}, Already Existing: {1}", - returnType.ToString(), - handlerType.ToString() - )); - } - } - - /// - /// Allows for registration of a general request handler if a more specific one - /// isn't registered. - /// - /// - public void RegisterDefaultHandler(Action callback) - { - if (_defCallbackHandler != null) - throw new ArgumentException("Cannot add more than one default handler"); - - this._defCallbackHandler = callback; - } - - /// !!!! DO NOT OVERRIDE THIS METHOD! !!!! - /// This is the shared guts of the Request/RequestAsync methods. It really ought to be private, - /// but the ResponseBuilder class required of those methods is static (it not only can't be mocked, - /// it can't even be passed in), rendering both methods untestable. Making this one protected at least - /// allows the real logic to be tested. - protected void ProcessRequest(RequestData requestData, ResponseData responseData) - where TReturn : class - { - Type returnType = typeof(TReturn); - bool foundHandler = false; - - // Loop through the registered handlers and see if our type is registered, OR - // if a base class is registered. - foreach (Type type in _callbackHandlers.Keys) - { - if (returnType == type || returnType.GetTypeInfo().IsSubclassOf(type)) - { - foundHandler = true; - - Action callback = - (Action)_callbackHandlers[typeof(TReturn)]; - - callback( - requestData, - responseData - ); - - break; - } - } - - //If we did not find one, then fallback to the default. - if (!foundHandler && _defCallbackHandler != null) - { - foundHandler = true; - _defCallbackHandler( - requestData, - responseData - ); - } - - //If we did not find any, throw an exception - if (!_callbackHandlers.ContainsKey(typeof(TReturn)) && _defCallbackHandler == null) - throw new ArgumentOutOfRangeException("There is no callback handler for defined for type, " + typeof(TReturn).ToString()); - - //It looks like, based on the code and use of the code, not because of actual commeents, that MadeItToResponse gets set - //once the Connection was able to get a response from a server. I am going to set it here, but we may need to update later - //if we want to test connection failures. - requestData.MadeItToResponse = true; - - // If the dev writing the test DID provide response data, but DID NOT set a MIME type, we'll just have to - // assume they meant to set "applicaton/json" since that's what Elasticsearch normally sends back. - if (String.IsNullOrWhiteSpace(responseData.ResponseMimeType) - && responseData.StatusCode.HasValue - && responseData.Stream != null) - { - responseData.ResponseMimeType = "application/json"; - } - - // This is much friendlier than the "Attempt to read a closed stream" message that will otherwise occur. - if ( responseData.Stream != null && !responseData.StatusCode.HasValue) - { - throw new ArgumentException("If a response stream is set, a status code must also be set."); - } - - //Basically all requests, even HEAD requests (e.g. AliasExists) need to have a stream to work correctly. - //Note, a stream of nothing is still a stream. So if you did not set a stream, we will do it for you. - //I am sure this will cause issues when trying to test failures of other kinds... Good use of 4hrs tracking - //this stupid issue down. - if (responseData.Stream == null) - { - using (MemoryStream stream = new MemoryStream(new byte[0])) { - responseData.Stream = stream; - } - } - } - - TReturn IConnection.Request(RequestData requestData) - { - Exception processingException = null; - using(ResponseData responseData = new ResponseData()) - { - this.ProcessRequest(requestData, responseData); - - return ResponseBuilder.ToResponse(requestData, processingException, responseData.StatusCode, null, responseData.Stream, responseData.ResponseMimeType); - } - } - - async Task IConnection.RequestAsync(RequestData requestData, System.Threading.CancellationToken cancellationToken) - { - Exception processingException = null; - using( ResponseData responseData = new ResponseData()) - { - this.ProcessRequest(requestData, responseData); - - return await ResponseBuilder.ToResponseAsync(requestData, processingException, responseData.StatusCode, null, responseData.Stream, responseData.ResponseMimeType, cancellationToken); - } - } - - /// - /// Helper function to extract the body of a request that would be sent to the Elasticsearch server. - /// - /// The request object - /// JObject containing the request - public JToken GetRequestPost(RequestData requestData) - { - //Some requests can have this as null. That is ok... - if (requestData.PostData == null) - return null; - - String postBody = string.Empty; - - using (MemoryStream stream = new MemoryStream()) - { - requestData.PostData.Write(stream, requestData.ConnectionSettings); - postBody = Encoding.UTF8.GetString(stream.ToArray()); - } - - return JToken.Parse(postBody); - } - } -} diff --git a/src/NCI.OCPL.Api.Common.Testing/NCI.OCPL.Api.Common.Testing.csproj b/src/NCI.OCPL.Api.Common.Testing/NCI.OCPL.Api.Common.Testing.csproj index e63b918..d797181 100644 --- a/src/NCI.OCPL.Api.Common.Testing/NCI.OCPL.Api.Common.Testing.csproj +++ b/src/NCI.OCPL.Api.Common.Testing/NCI.OCPL.Api.Common.Testing.csproj @@ -1,21 +1,19 @@ - netcoreapp8.0 + net8.0 true - 3.0.0 + 4.0.0-alpha1 - Shared helper code for unit testing. + Testing utilities for NCI.OCPL.Api.Common. https://github.com/NCIOCPL/NCI.OCPL.Api.Shared National Cancer Institute - Office of Communications and Public Liaison - true - - - - + + + diff --git a/src/NCI.OCPL.Api.Common.Testing/TestingTools.cs b/src/NCI.OCPL.Api.Common.Testing/TestingTools.cs index 14160b6..5e0bb7a 100644 --- a/src/NCI.OCPL.Api.Common.Testing/TestingTools.cs +++ b/src/NCI.OCPL.Api.Common.Testing/TestingTools.cs @@ -1,11 +1,10 @@ using System.IO; using System.Reflection; using System.Text; +using System.Text.Json; using System.Xml; using System.Xml.Serialization; -using Newtonsoft.Json.Linq; - namespace NCI.OCPL.Api.Common.Testing { @@ -84,14 +83,14 @@ public static T DeserializeXML(string testFile) where T : class } /// - /// Gets a JSON file and parses it into a JObject structure. + /// Gets a JSON file and parses it into a JsonDocument structure. /// /// Name of the file to load. - /// A JObject structure containing the parsed data. - public static JObject GetDataFileAsJObject(string testFile) + /// A JsonDocument structure containing the parsed data. + public static JsonDocument GetDataFileAsJsonDocument(string testFile) { string path = GetPathToTestFile(testFile); - return JObject.Parse(File.ReadAllText(path)); + return JsonDocument.Parse(File.ReadAllText(path)); } /// diff --git a/src/NCI.OCPL.Api.Common/Models/ErrorMessage.cs b/src/NCI.OCPL.Api.Common/Models/ErrorMessage.cs index f34dcf8..f122a22 100644 --- a/src/NCI.OCPL.Api.Common/Models/ErrorMessage.cs +++ b/src/NCI.OCPL.Api.Common/Models/ErrorMessage.cs @@ -1,4 +1,4 @@ -using Newtonsoft.Json; +using System.Text.Json; namespace NCI.OCPL.Api.Common { @@ -19,7 +19,7 @@ public class ErrorMessage /// A that represents the current . public override string ToString() { - return JsonConvert.SerializeObject(this); + return JsonSerializer.Serialize(this); } } } \ No newline at end of file diff --git a/src/NCI.OCPL.Api.Common/NCI.OCPL.Api.Common.csproj b/src/NCI.OCPL.Api.Common/NCI.OCPL.Api.Common.csproj index baf2b77..c76ac61 100644 --- a/src/NCI.OCPL.Api.Common/NCI.OCPL.Api.Common.csproj +++ b/src/NCI.OCPL.Api.Common/NCI.OCPL.Api.Common.csproj @@ -1,10 +1,10 @@ - netcoreapp8.0 + net8.0 true - 3.0.0 + 4.0.0-alpha1 Common code for use between APIs. https://github.com/NCIOCPL/NCI.OCPL.Api.Shared @@ -15,10 +15,8 @@ - - - + diff --git a/src/NCI.OCPL.Api.Common/NciStartupBase.cs b/src/NCI.OCPL.Api.Common/NciStartupBase.cs index f06a3fb..ec31b46 100644 --- a/src/NCI.OCPL.Api.Common/NciStartupBase.cs +++ b/src/NCI.OCPL.Api.Common/NciStartupBase.cs @@ -13,9 +13,8 @@ using NCI.OCPL.Api.Common.Models.Options; -using Nest; -using Nest.JsonNetSerializer; -using Elasticsearch.Net; +using Elastic.Clients.Elasticsearch; +using Elastic.Transport; namespace NCI.OCPL.Api.Common @@ -68,13 +67,13 @@ have been in the base class to begin with? services.AddSingleton(); - // This will inject an IElasticClient using our configuration into any - // controllers that take an IElasticClient parameter into its constructor. + // This will inject an ElasticsearchClient using our configuration into any + // controllers that take an ElasticsearchClient parameter into its constructor. // // AddTransient means that it will instantiate a new instance of our client // for each instance of the controller. So the function below will be called // on each request. - services.AddTransient(p => + services.AddSingleton(p => { // Get the ElasticSearch credentials. @@ -88,18 +87,18 @@ have been in the base class to begin with? // keep tabs on the health of the servers in the cluster and // probe them to ensure they are healthy. This is how we handle // redundancy and load balancing. - var connectionPool = new SniffingConnectionPool(uris); + var connectionPool = new SniffingNodePool(uris); - //Return a new instance of an ElasticClient with our settings - ConnectionSettings settings = new ConnectionSettings(connectionPool, sourceSerializer: JsonNetSerializer.Default); + //Return a new instance of an ElasticsearchClient with our settings + var settings = new ElasticsearchClientSettings(connectionPool); //Let's only try and use credentials if the username is set. if (!string.IsNullOrWhiteSpace(username)) { - settings.BasicAuthentication(username, password); + settings.Authentication(new BasicAuthentication(username, password)); } - return new ElasticClient(settings); + return new ElasticsearchClient(settings); }); //Add in Application specific services @@ -121,8 +120,7 @@ have been in the base class to begin with? services.AddCors(); // Make the application's routes available. - services.AddControllers() - .AddNewtonsoftJson(); + services.AddControllers(); // Enable Swagger // This creates the Swagger Json diff --git a/src/NCI.OCPL.Api.Common/Services/ESAliasNameProvider.cs b/src/NCI.OCPL.Api.Common/Services/ESAliasNameProvider.cs index f0b250c..d99b0d9 100644 --- a/src/NCI.OCPL.Api.Common/Services/ESAliasNameProvider.cs +++ b/src/NCI.OCPL.Api.Common/Services/ESAliasNameProvider.cs @@ -16,11 +16,17 @@ public class ESAliasNameProvider : IESAliasNameProvider /// public string Name { - get { return _name; } + get + { + if (String.IsNullOrWhiteSpace(_name)) + throw new InvalidOperationException("ES Alias Name has not been set."); + return _name; + } + set { if (String.IsNullOrWhiteSpace(value)) - throw new ArgumentNullException(nameof(Name)); + throw new ArgumentException(nameof(Name)); _name = value; } diff --git a/src/NCI.OCPL.Api.Common/Services/ESHealthCheckService.cs b/src/NCI.OCPL.Api.Common/Services/ESHealthCheckService.cs index 1af89de..7cb4224 100644 --- a/src/NCI.OCPL.Api.Common/Services/ESHealthCheckService.cs +++ b/src/NCI.OCPL.Api.Common/Services/ESHealthCheckService.cs @@ -3,8 +3,8 @@ using Microsoft.Extensions.Logging; -using Elasticsearch.Net; -using Nest; +using Elastic.Clients.Elasticsearch; +using Elastic.Clients.Elasticsearch.Cluster; @@ -16,7 +16,7 @@ namespace NCI.OCPL.Api.Common /// public class ESHealthCheckService : IHealthCheckService { - private IElasticClient _elasticClient; + private ElasticsearchClient _elasticClient; private string _aliasName; private readonly ILogger _logger; @@ -26,7 +26,7 @@ public class ESHealthCheckService : IHealthCheckService /// The client to be used for connections /// The name of the alias to check /// Logger instance. - public ESHealthCheckService(IElasticClient client, + public ESHealthCheckService(ElasticsearchClient client, IESAliasNameProvider aliasNamer, ILogger logger) { @@ -49,19 +49,16 @@ public async Task IndexIsHealthy() try { - Indices idx = Indices.Index(_aliasName); + HealthResponse response = await _elasticClient.Cluster.HealthAsync(new HealthRequest(_aliasName)); - //ClusterHealthResponse response = await _elasticClient.Cluster.HealthAsync(idx, hd => hd.Index(_aliasName)); - ClusterHealthResponse response = await _elasticClient.Cluster.HealthAsync(idx); - - if (!response.IsValid) + if (!response.IsValidResponse) { _logger.LogError($"Error checking ElasticSearch health for {_aliasName}."); _logger.LogError($"Returned debug info: {response.DebugInformation}."); } else { - if (response.Status == Health.Green || response.Status == Health.Yellow) + if (response.Status == HealthStatus.Green || response.Status == HealthStatus.Yellow) { //This is the only condition that will return true return true; diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/NCI.OCPL.Api.Common.Testing.Tests.csproj b/test/NCI.OCPL.Api.Common.Testing.Tests/NCI.OCPL.Api.Common.Testing.Tests.csproj index 5c8fe09..a5454a5 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/NCI.OCPL.Api.Common.Testing.Tests.csproj +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/NCI.OCPL.Api.Common.Testing.Tests.csproj @@ -1,7 +1,7 @@ - netcoreapp8.0 + net8.0 false @@ -10,12 +10,17 @@ - + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + - - - + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/TestData/README.md b/test/NCI.OCPL.Api.Common.Testing.Tests/TestData/README.md new file mode 100644 index 0000000..ce52864 --- /dev/null +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/TestData/README.md @@ -0,0 +1,16 @@ + +## test-response.md + + This file contains results for autosuggest + based on the keyword "breast" + + ```json + POST /autosg/terms/_search/template + { + "file" : "autosg_suggest_cgov_en", + "params" : { + "searchstring" : "breast", + "my_size" : 20 + } + } + ``` \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/TestData/test-response.json b/test/NCI.OCPL.Api.Common.Testing.Tests/TestData/test-response.json index b6f6b17..651ad6e 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/TestData/test-response.json +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/TestData/test-response.json @@ -1,16 +1,3 @@ -/* - Suggestions based on the keyword "breast" - - POST /autosg/terms/_search/template - { - "file" : "autosg_suggest_cgov_en", - "params" : { - "searchstring" : "breast", - "my_size" : 20 - } - } -*/ - { "took": 28, "timed_out": false, diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.Common.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.Common.cs deleted file mode 100644 index 8394243..0000000 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.Common.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace NCI.OCPL.Api.Common -{ - public partial class ElasticsearchInterceptingConnectionTest - { - public class MockType { } - public class MockType2 { } - } -} \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.GetRequestPost.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.GetRequestPost.cs deleted file mode 100644 index aad45b5..0000000 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.GetRequestPost.cs +++ /dev/null @@ -1,75 +0,0 @@ -using System; -using System.IO; - -using Elasticsearch.Net; -using Xunit; - -using NCI.OCPL.Api.Common.Testing; -using Moq; -using System.Text; -using Newtonsoft.Json.Linq; -using Newtonsoft.Json; - -namespace NCI.OCPL.Api.Common -{ - /// - /// GetRequestPost tests - /// - public partial class ElasticsearchInterceptingConnectionTest - { - /// - /// Fail to set up multiple default handlers. - /// - [Fact] - public void GetRequestPost_NullPostData() - { - Mock mockConfig = new Mock(); - mockConfig.Setup(cfg => cfg.RequestTimeout).Returns(new TimeSpan(0, 0, 5)); - mockConfig.Setup(cfg => cfg.PingTimeout).Returns(new TimeSpan(0, 0, 5)); - IConnectionConfigurationValues config = mockConfig.Object; - - // None of these values really matter except the post data which is the third parameter. - RequestData requestData = new RequestData(HttpMethod.GET, "foo", null, config, null, null); - - ElasticsearchInterceptingConnection conn = new ElasticsearchInterceptingConnection(); - - Assert.Null(conn.GetRequestPost(requestData)); - } - - [Theory] - [InlineData(@"{ ""string_property"": ""string value"", ""bool_property"": true, ""integer_property"": 19, ""null"": null }")] - [InlineData(@"[""array value 1"", ""array value 2"", ""array value 3""]")] - public void GetRequestPost_WithData(string data) - { - JToken expected = JToken.Parse(data); - - Mock mockConfig = new Mock(); - mockConfig.Setup(cfg => cfg.RequestTimeout).Returns(new TimeSpan(0, 0, 5)); - mockConfig.Setup(cfg => cfg.PingTimeout).Returns(new TimeSpan(0, 0, 5)); - IConnectionConfigurationValues config = mockConfig.Object; - - Mock mockData = new Mock(); - mockData.Setup( - d => d.Write(It.IsAny(), It.IsAny()) - ) - .Callback(( - Stream str, IConnectionConfigurationValues iccv) => - { - byte[] buf = Encoding.UTF8.GetBytes(data); - str.Write(buf, 0, buf.Length); - } - ); - - - // None of these values really matter except the post data which is the third parameter. - RequestData requestData = new RequestData(HttpMethod.GET, "foo", mockData.Object, config, null, null); - - ElasticsearchInterceptingConnection conn = new ElasticsearchInterceptingConnection(); - - JToken actual = conn.GetRequestPost(requestData); - - Assert.Equal(expected, actual, new JTokenEqualityComparer()); - } - - } -} \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.ProcessRequest.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.ProcessRequest.cs deleted file mode 100644 index d84e943..0000000 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.ProcessRequest.cs +++ /dev/null @@ -1,259 +0,0 @@ -using System; -using System.IO; -using System.Text; - -using Elasticsearch.Net; -using Moq; -using Moq.Protected; -using Nest; -using Xunit; - -using NCI.OCPL.Api.Common.Testing; -using static NCI.OCPL.Api.Common.Testing.ElasticsearchInterceptingConnection; -using Newtonsoft.Json.Linq; - -namespace NCI.OCPL.Api.Common -{ - - /// - /// This is dirty. Normally we'd call one of the public methods which calls ProcessRequest(), - /// but the NEST ResponseBuilder class is static, meaning there's no way to pass in a mock and - /// use that. On top of that, a ridiculous amount of mocking is required to support instantiating - /// a RequestData object (another class which can't be mocked) that won't make ResponseBuilder - /// blow up. So, if we want to test ProcessRequest(), we have to make it protected and then create - /// a test class for purposes of actually calling the thing. - /// - class DirtyTestClass : ElasticsearchInterceptingConnection - { - public void DoProcessing(RequestData requestData, ResponseData responseData) where TReturn : class - { - ProcessRequest(requestData, responseData); - } - } - - public partial class ElasticsearchInterceptingConnectionTest - { - /// - /// What happens when there are no handlers registered? - /// - [Fact] - public void ProcessRequest_NoHandlerRegistered() - { - - DirtyTestClass conn = new DirtyTestClass(); - - // The specific value here shouldn't matter. - RequestData req = RequestPlaceholder; - ResponseData res = new ResponseData(); - - Assert.Throws( - () => conn.DoProcessing>(req, res) - ); - } - - /// - /// What happens when the only handler is for the wrong type and there's no default? - /// - [Fact] - public void ProcessRequest_NoDefault_NoMatch() - { - DirtyTestClass conn = new DirtyTestClass(); - conn.RegisterRequestHandlerForType>((reqd, resd) => { }); - - // The specific value here shouldn't matter. - RequestData req = RequestPlaceholder; - ResponseData res = new ResponseData(); - - Assert.Throws( - () => conn.DoProcessing>(req, res) - ); - } - - /// - /// Simulate an error during a response. - /// - [Fact] - public void ProcessRequest_ResponseError() - { - DirtyTestClass conn = new DirtyTestClass(); - conn.RegisterRequestHandlerForType>((reqd, resd) => { - throw new NotImplementedException(); - }); - - // The specific value here shouldn't matter. - RequestData req = RequestPlaceholder; - ResponseData res = new ResponseData(); - - // What matters here is that - // a. The exception was thrown. - // b. It's the same one. - // Possibly b. doesn't matter as much, but if that asssumption changes, - // it should be a deliberate change. - Assert.Throws( - () => conn.DoProcessing>(req, res) - ); - } - - /// - /// Match exists for the document type, no default registered. - /// - /// - [Fact] - public void ProcessRequest_NoDefault_MatchExists() - { - // How many times was the handler called? - int timesHandlerWasCalled = 0; - - DirtyTestClass conn = new DirtyTestClass(); - conn.RegisterRequestHandlerForType>((reqd, resd) => { - resd.Stream = ElastcsearchTestingTools.MockEmptyResponse; - resd.StatusCode = 200; - timesHandlerWasCalled++; - }); - - // The specific value here shouldn't matter. - RequestData req = RequestPlaceholder; - ResponseData res = new ResponseData(); - - conn.DoProcessing>(req, res); - - Assert.Equal(1, timesHandlerWasCalled); - Assert.Equal(200, res.StatusCode); - - JToken actual; - using (StreamReader sr = new StreamReader(res.Stream)) - { - actual = JToken.Parse(sr.ReadToEnd()); - } - - JToken expected = JToken.Parse(ElastcsearchTestingTools.MockEmptyResponseString); - - Assert.Equal(expected, actual, new JTokenEqualityComparer()); - } - - - /// - /// Multiple handlers registered, no default. - /// - [Fact] - public void ProcessRequest_NoDefault_MultipleHandlers() - { - // How many times was the handler called? - int timesHandlerWasCalled = 0; - - DirtyTestClass conn = new DirtyTestClass(); - conn.RegisterRequestHandlerForType>((reqd, resd) => - { - resd.Stream = ElastcsearchTestingTools.MockEmptyResponse; - resd.StatusCode = 200; - timesHandlerWasCalled++; - }); - conn.RegisterRequestHandlerForType>((reqd, resd) => - { - throw new NotImplementedException(); - }); - - // The specific value here shouldn't matter. - RequestData req = RequestPlaceholder; - ResponseData res = new ResponseData(); - - conn.DoProcessing>(req, res); - - Assert.Equal(1, timesHandlerWasCalled); - Assert.Equal(200, res.StatusCode); - - JToken actual; - using (StreamReader sr = new StreamReader(res.Stream)) - { - actual = JToken.Parse(sr.ReadToEnd()); - } - - JToken expected = JToken.Parse(ElastcsearchTestingTools.MockEmptyResponseString); - - Assert.Equal(expected, actual, new JTokenEqualityComparer()); - } - - /// - /// Take the default handler when there's no match. - /// - [Fact] - public void ProcessRequest_UseDefault() - { - // How many times was the handler called? - int timesHandlerWasCalled = 0; - - DirtyTestClass conn = new DirtyTestClass(); - conn.RegisterRequestHandlerForType>((reqd, resd) => - { - throw new NotImplementedException(); - }); - conn.RegisterDefaultHandler((RequestData reqd, Object resd) => - { - ((ResponseData)resd).Stream = ElastcsearchTestingTools.MockEmptyResponse; - ((ResponseData)resd).StatusCode = 200; - timesHandlerWasCalled++; - }); - - // The specific value here shouldn't matter. - RequestData req = RequestPlaceholder; - ResponseData res = new ResponseData(); - - // MockType is registered, but we're looking for MockType2. - conn.DoProcessing>(req, res); - - Assert.Equal(1, timesHandlerWasCalled); - Assert.Equal(200, res.StatusCode); - - JToken actual; - using (StreamReader sr = new StreamReader(res.Stream)) - { - actual = JToken.Parse(sr.ReadToEnd()); - } - - JToken expected = JToken.Parse(ElastcsearchTestingTools.MockEmptyResponseString); - - Assert.Equal(expected, actual, new JTokenEqualityComparer()); - } - - - /// - /// Instantiate the mock RequestData needed to satisfy the NEST internals called by the - /// Request methods. - /// - /// A instance of RequestData. - private RequestData RequestPlaceholder - { - get - { - string data = @"{ ""string_property"": ""string value"", ""bool_property"": true, ""integer_property"": 19, ""null"": null }"; - - Mock mockGlobalConfig = new Mock(); - mockGlobalConfig.Setup(cfg => cfg.RequestTimeout).Returns(new TimeSpan(0, 0, 5)); - mockGlobalConfig.Setup(cfg => cfg.PingTimeout).Returns(new TimeSpan(0, 0, 5)); - mockGlobalConfig.Setup(cfg => cfg.DisableDirectStreaming).Returns(true); - - Mock mockRequestConfig = new Mock(); - mockRequestConfig.Setup(cfg => cfg.AllowedStatusCodes).Returns(new int[]{200}); - - Mock mockLocalConfig = new Mock(); - mockLocalConfig.Setup(cfg => cfg.RequestConfiguration).Returns(mockRequestConfig.Object); - - - Mock mockData = new Mock(); - mockData.Setup( - d => d.Write(It.IsAny(), It.IsAny()) - ) - .Callback(( - Stream str, IConnectionConfigurationValues iccv) => - { - byte[] buf = Encoding.UTF8.GetBytes(data); - str.Write(buf, 0, buf.Length); - } - ); - - return new RequestData(HttpMethod.GET, "foo", mockData.Object, mockGlobalConfig.Object, mockLocalConfig.Object, null); - } - } - - } -} \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.RegisterDefaultHandler.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.RegisterDefaultHandler.cs deleted file mode 100644 index e6815b3..0000000 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.RegisterDefaultHandler.cs +++ /dev/null @@ -1,36 +0,0 @@ -using System; - -using Elasticsearch.Net; - -using NCI.OCPL.Api.Common.Testing; -using Xunit; - -namespace NCI.OCPL.Api.Common -{ - /// - /// RegisterDefaultHandler tests - /// - public partial class ElasticsearchInterceptingConnectionTest - { - /// - /// Fail to set up multiple default handlers. - /// - [Fact] - public void RegisterRequestHandlerForType_MultipleDefaults() - { - void callback(RequestData req, object res) { } - void callback2(RequestData req, object res) { } - - ElasticsearchInterceptingConnection conn = new ElasticsearchInterceptingConnection(); - - // First registration should always succeed. - conn.RegisterDefaultHandler(callback); - - Exception ex = Assert.Throws( - () => conn.RegisterDefaultHandler(callback2) - ); - - Assert.Equal("Cannot add more than one default handler", ex.Message); - } - } -} \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.RegisterRequestHandlerForType.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.RegisterRequestHandlerForType.cs deleted file mode 100644 index 0454bf1..0000000 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Elasticsearch/ElasticsearchInterceptingConnection.RegisterRequestHandlerForType.cs +++ /dev/null @@ -1,66 +0,0 @@ -using System; - -using Elasticsearch.Net; - -using NCI.OCPL.Api.Common.Testing; -using Xunit; - -namespace NCI.OCPL.Api.Common -{ - /// - /// RegisterRequestHandlerForType tests. - /// - public partial class ElasticsearchInterceptingConnectionTest - { - /// - /// Multiple handlers can't be registered for the same type. - /// - [Fact] - public void RegisterRequestHandlerForType_DuplicateTypes() - { - void callback(RequestData req, ElasticsearchInterceptingConnection.ResponseData res) { } - void callback2(RequestData req, ElasticsearchInterceptingConnection.ResponseData res) { } - - ElasticsearchInterceptingConnection conn = new ElasticsearchInterceptingConnection(); - - // First registration should always succeed. - conn.RegisterRequestHandlerForType>(callback); - - // Second registration for same type should always fail. - Exception ex = Assert.Throws( - () => conn.RegisterRequestHandlerForType>(callback) - ); - - Assert.Equal( - "There is already a handler defined that would be called for type. Trying to add for: Nest.SearchResponse`1[NCI.OCPL.Api.Common.ElasticsearchInterceptingConnectionTest+MockType], Already Existing: Nest.SearchResponse`1[NCI.OCPL.Api.Common.ElasticsearchInterceptingConnectionTest+MockType]", - ex.Message - ); - - // Second registration for the same type should still fail with a different handler. - ex = Assert.Throws( - () => conn.RegisterRequestHandlerForType>(callback2) - ); - } - - - /// - /// Registering handlers for completely different types does work. - /// - [Fact] - public void RegisterRequestHandlerForType_DifferentTypes() - { - void callback(RequestData req, ElasticsearchInterceptingConnection.ResponseData res) { } - ElasticsearchInterceptingConnection conn = new ElasticsearchInterceptingConnection(); - - // This registration should always succeed. - conn.RegisterRequestHandlerForType>(callback); - - // This registration should also succeed. - Exception ex = Record.Exception( - () => conn.RegisterRequestHandlerForType>(callback) - ); - - Assert.Null(ex); - } - } -} \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElastcsearchTestingTools.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElastcsearchTestingTools.cs index e6274dd..46549e4 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElastcsearchTestingTools.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElastcsearchTestingTools.cs @@ -1,24 +1,26 @@ using System.IO; +using System.Text.Json.Nodes; -using Newtonsoft.Json.Linq; using Xunit; namespace NCI.OCPL.Api.Common.Testing { public partial class ElastcsearchTestingToolsTest { + /** + * Test to ensure that the mock empty response matches the original string. + */ [Fact] public void EmptyResponse() { - JToken expected = JToken.Parse(ElastcsearchTestingTools.MockEmptyResponseString); - JToken actual; + // Parse the string version of an empty response. + JsonNode expected = JsonNode.Parse(ElastcsearchTestingTools.MockEmptyResponseString); - using(StreamReader sr = new StreamReader(ElastcsearchTestingTools.MockEmptyResponse)) - { - actual = JToken.Parse(sr.ReadToEnd()); - } + // Parse what comes back as a stream. + JsonNode actual = JsonNode.Parse(new StreamReader(ElastcsearchTestingTools.MockEmptyResponse).ReadToEnd()); - Assert.Equal(expected, actual, new JTokenEqualityComparer()); + // Compare JSON structures - JsonNode.DeepEquals ignores property order. + Assert.True(JsonNode.DeepEquals(expected, actual), "JSON structures do not match."); } } } \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools.GetErrorElasticClient.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools.GetErrorElasticClient.cs index c44c4c4..c0cc06b 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools.GetErrorElasticClient.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools.GetErrorElasticClient.cs @@ -1,4 +1,7 @@ -using Nest; +using System.Threading.Tasks; + +using Elastic.Clients.Elasticsearch; +using Elastic.Clients.Elasticsearch.QueryDsl; using Xunit; namespace NCI.OCPL.Api.Common.Testing @@ -18,18 +21,17 @@ public partial class ElasticToolsTest [InlineData(403, false)] [InlineData(404, false)] [InlineData(408, false)] - async public void GetErrorElasticClient_InvalidResponse(int returnCode, bool expectedValid) + async public Task GetErrorElasticClient_InvalidResponse(int returnCode, bool expectedValid) { - IElasticClient client = ElasticTools.GetErrorElasticClient(returnCode); - Indices index = Indices.Index(new string[] { "someIndex" }); - SearchRequest request = new SearchRequest(index) + ElasticsearchClient client = ElasticTools.GetErrorElasticClient(returnCode); + SearchRequest request = new SearchRequest("someIndex") { - Query = new TermQuery{Field = "someField", Value = "someValue"} + Query = new TermQuery { Field = "someField", Value = "someValue" } }; - ISearchResponse response = await client.SearchAsync(request); + SearchResponse response = await client.SearchAsync(request); - Assert.Equal(expectedValid, response.IsValid); + Assert.Equal(expectedValid, response.IsValidResponse); } } } \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools.GetInMemoryElasticClient.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools.GetInMemoryElasticClient.cs index 6674de6..7a5f345 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools.GetInMemoryElasticClient.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools.GetInMemoryElasticClient.cs @@ -1,31 +1,28 @@ -using Nest; +using Elastic.Clients.Elasticsearch; using Xunit; namespace NCI.OCPL.Api.Common.Testing { -#pragma warning disable CS0618 - [ElasticsearchType(Name = "terms")] -#pragma warning restore CS0618 - public partial class ElasticToolsTest { [Fact] public void GetInMemoryElasticClient() { - IElasticClient client = ElasticTools.GetInMemoryElasticClient("test-response.json"); + ElasticsearchClient client = ElasticTools.GetInMemoryElasticClient("test-response.json"); var response = client.SearchTemplate(sd => sd - .Index("AliasName") + .Indices("AliasName") .Params(pd => pd .Add("searchstring", "search_term") .Add("my_size", 10) ) ); - Assert.True(response.IsValid); - Assert.Equal(222, response.Total); - Assert.Equal(20, response.Documents.Count); - Assert.All(response.Documents, doc => Assert.NotNull(doc)); - Assert.All(response.Documents, doc => Assert.IsType(doc)); + Assert.True(response.IsValidResponse); + var totalHits = response.Hits.Total.Match(t => t.Value, l => l); + Assert.Equal(222, totalHits); + Assert.Equal(20, response.Hits.Hits.Count); + Assert.All(response.Hits.Hits, hit => Assert.NotNull(hit.Source)); + Assert.All(response.Hits.Hits, hit => Assert.IsType(hit.Source)); } } } \ No newline at end of file diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools._common.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools._common.cs index 5c8fa1d..7a3dc7f 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools._common.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/ElasticTools._common.cs @@ -1,5 +1,3 @@ -using Nest; - namespace NCI.OCPL.Api.Common.Testing { public partial class ElasticToolsTest @@ -13,7 +11,6 @@ public class TestType /// The Backend ID for this item /// /// - [Text(Name = "term")] public string Term { get; set; } } } diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetDataFileAsJObject.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetDataFileAsJObject.cs index 7c835cb..785b2a2 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetDataFileAsJObject.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetDataFileAsJObject.cs @@ -1,7 +1,7 @@ using System; using System.IO; - -using Newtonsoft.Json.Linq; +using System.Text.Json; +using System.Text.Json.Nodes; using Xunit; namespace NCI.OCPL.Api.Common.Testing @@ -12,7 +12,7 @@ public partial class TestingToolsTests public void GetDataFileAsJObject_Null() { Assert.Throws( - () => TestingTools.GetDataFileAsJObject(null) + () => TestingTools.GetDataFileAsJsonDocument(null) ); } @@ -20,7 +20,7 @@ public void GetDataFileAsJObject_Null() public void GetDataFileAsJObject_NonexistingFile() { Assert.Throws( - () => TestingTools.GetDataFileAsJObject("NonExistingFile.json") + () => TestingTools.GetDataFileAsJsonDocument("NonExistingFile.json") ); } @@ -36,12 +36,15 @@ public void GetDataFileAsJObject_NonexistingFile() }")] public void GetDataFileAsJObject_SimpleString(string filename, string expectedValue) { - JObject expected = JObject.Parse(expectedValue); + JsonNode expected = JsonNode.Parse(expectedValue); string path = Path.Join("Tools/TestingTools/GetDataFileAsJObject", filename); - JObject actual = TestingTools.GetDataFileAsJObject(path); + JsonDocument actual = TestingTools.GetDataFileAsJsonDocument(path); + + // We need to convert JsonDocument to JsonNode to do a deep comparison. + JsonNode actualNode = JsonNode.Parse(actual.RootElement.GetRawText()); - Assert.Equal(expected, actual, new JTokenEqualityComparer()); + Assert.True(JsonNode.DeepEquals(expected, actualNode), "JSON structures do not match."); } } diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetFileAsStream.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetFileAsStream.cs index 2878e33..084214a 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetFileAsStream.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetFileAsStream.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Text; + using Xunit; namespace NCI.OCPL.Api.Common.Testing diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetPathToTestFile.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetPathToTestFile.cs index 742a3d0..72240fd 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetPathToTestFile.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetPathToTestFile.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Text; + using Xunit; namespace NCI.OCPL.Api.Common.Testing diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetStringAsStream.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetStringAsStream.cs index c085342..d2f715a 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetStringAsStream.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetStringAsStream.cs @@ -1,5 +1,6 @@ using System; using System.IO; + using Xunit; namespace NCI.OCPL.Api.Common.Testing diff --git a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetTestFileAsBytes.cs b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetTestFileAsBytes.cs index e6ea676..3841850 100644 --- a/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetTestFileAsBytes.cs +++ b/test/NCI.OCPL.Api.Common.Testing.Tests/Tests/Tools/TestingTools.Test.GetTestFileAsBytes.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Text; + using Xunit; namespace NCI.OCPL.Api.Common.Testing diff --git a/test/NCI.OCPL.Api.Common.Tests/NCI.OCPL.Api.Common.Tests.csproj b/test/NCI.OCPL.Api.Common.Tests/NCI.OCPL.Api.Common.Tests.csproj index 8941d1a..daa8abb 100644 --- a/test/NCI.OCPL.Api.Common.Tests/NCI.OCPL.Api.Common.Tests.csproj +++ b/test/NCI.OCPL.Api.Common.Tests/NCI.OCPL.Api.Common.Tests.csproj @@ -1,7 +1,7 @@ - netcoreapp8.0 + net8.0 false @@ -10,12 +10,17 @@ - + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + - - - + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + diff --git a/test/NCI.OCPL.Api.Common.Tests/Tests/Exceptions/ConfigurationExceptionTest.cs b/test/NCI.OCPL.Api.Common.Tests/Tests/Exceptions/ConfigurationExceptionTest.cs index e2f86e1..60d9395 100644 --- a/test/NCI.OCPL.Api.Common.Tests/Tests/Exceptions/ConfigurationExceptionTest.cs +++ b/test/NCI.OCPL.Api.Common.Tests/Tests/Exceptions/ConfigurationExceptionTest.cs @@ -1,11 +1,25 @@ -using Xunit; +using System; -using NCI.OCPL.Api.Common; +using Xunit; namespace NCI.OCPL.Api.Common.Tests { public partial class ConfigurationExceptionTest { + /// + /// Verify the default constructor creates an exception. + /// + [Fact] + public void DefaultConstructor() + { + // Act + ConfigurationException ex = new ConfigurationException(); + + // Assert + Assert.NotNull(ex); + Assert.IsType(ex); + } + /// /// Verify the exception message is unchanged. /// @@ -17,9 +31,34 @@ public void MessageIntact() { throw new ConfigurationException(theMessage); } - catch (System.Exception ex) + catch (Exception ex) + { + Assert.Equal(theMessage, ex.Message); + } + } + + /// + /// Verify the message and inner exception are preserved. + /// + [Fact] + public void MessageAndInnerExceptionIntact() + { + // Arrange + string theMessage = "the outer message"; + Exception innerException = new InvalidOperationException("inner exception message"); + + // Act + try + { + throw new ConfigurationException(theMessage, innerException); + } + catch (ConfigurationException ex) { + // Assert Assert.Equal(theMessage, ex.Message); + Assert.NotNull(ex.InnerException); + Assert.Same(innerException, ex.InnerException); + Assert.Equal("inner exception message", ex.InnerException.Message); } } diff --git a/test/NCI.OCPL.Api.Common.Tests/Tests/Models/Options/ElasticsearchOptionsTest.cs b/test/NCI.OCPL.Api.Common.Tests/Tests/Models/Options/ElasticsearchOptionsTest.cs index 3b86c95..86159f8 100644 --- a/test/NCI.OCPL.Api.Common.Tests/Tests/Models/Options/ElasticsearchOptionsTest.cs +++ b/test/NCI.OCPL.Api.Common.Tests/Tests/Models/Options/ElasticsearchOptionsTest.cs @@ -1,5 +1,5 @@ -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; +using System.Text.Json; + using Xunit; namespace NCI.OCPL.Api.Common.Models.Options @@ -12,8 +12,7 @@ public partial class ElasticsearchOptionsTest [Fact] public void Serialize() { - // Use JToken for comparisons because we don't care about the order. - JToken expected = JToken.Parse(@" + ElasticsearchOptions expected = JsonSerializer.Deserialize(@" { ""Servers"": ""Server list"", ""Userid"": ""the user id"", @@ -29,10 +28,10 @@ public void Serialize() MaximumRetries = 3 }; - string actualText = JsonConvert.SerializeObject(options); - JToken actualObject = JToken.Parse(actualText); + string actualText = JsonSerializer.Serialize(options); + ElasticsearchOptions actual = JsonSerializer.Deserialize(actualText); - Assert.Equal(expected, actualObject, new JTokenEqualityComparer()); + Assert.Equivalent(expected, actual, strict: true); } [Fact] @@ -51,7 +50,7 @@ public void Deserialize() ""MaximumRetries"": 5 }"; - ElasticsearchOptions actual = JsonConvert.DeserializeObject(input); + ElasticsearchOptions actual = JsonSerializer.Deserialize(input); Assert.Equal(expectedServers, actual.Servers); Assert.Equal(expectedUserid, actual.Userid); diff --git a/test/NCI.OCPL.Api.Common.Tests/Tests/Models/Options/NSwagOptionsTest.cs b/test/NCI.OCPL.Api.Common.Tests/Tests/Models/Options/NSwagOptionsTest.cs index 39ca471..dd2d7c5 100644 --- a/test/NCI.OCPL.Api.Common.Tests/Tests/Models/Options/NSwagOptionsTest.cs +++ b/test/NCI.OCPL.Api.Common.Tests/Tests/Models/Options/NSwagOptionsTest.cs @@ -1,5 +1,5 @@ -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; +using System.Text.Json; + using Xunit; namespace NCI.OCPL.Api.Common.Models.Options @@ -12,8 +12,7 @@ public partial class NSwagOptionsTest [Fact] public void Serialize() { - // Use JToken for comparisons because we don't care about the order. - JToken expected = JToken.Parse(@" + NSwagOptions expected = JsonSerializer.Deserialize(@" { ""Title"": ""Swagger Doc Title"", ""Description"": ""API Description"" @@ -25,10 +24,10 @@ public void Serialize() Description = "API Description" }; - string actualText = JsonConvert.SerializeObject(options); - JToken actualObject = JToken.Parse(actualText); + string actualText = JsonSerializer.Serialize(options); + NSwagOptions actual = JsonSerializer.Deserialize(actualText); - Assert.Equal(expected, actualObject, new JTokenEqualityComparer()); + Assert.Equivalent(expected, actual, strict: true); } [Fact] @@ -43,7 +42,7 @@ public void Deserialize() ""Description"": ""API Description"" }"; - NSwagOptions actual = JsonConvert.DeserializeObject(input); + NSwagOptions actual = JsonSerializer.Deserialize(input); Assert.Equal(expectedTitle, actual.Title); Assert.Equal(expectedDescription, actual.Description); diff --git a/test/NCI.OCPL.Api.Common.Tests/Tests/NciStartupBase/NciStartupBase.ConfigureServices.cs b/test/NCI.OCPL.Api.Common.Tests/Tests/NciStartupBase/NciStartupBase.ConfigureServices.cs index 69a5239..33ce488 100644 --- a/test/NCI.OCPL.Api.Common.Tests/Tests/NciStartupBase/NciStartupBase.ConfigureServices.cs +++ b/test/NCI.OCPL.Api.Common.Tests/Tests/NciStartupBase/NciStartupBase.ConfigureServices.cs @@ -5,11 +5,13 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; + using Moq; using Moq.Protected; -using Nest; using Xunit; +using Elastic.Clients.Elasticsearch; + namespace NCI.OCPL.Api.Common { /// @@ -92,7 +94,7 @@ public void ConfigureServices_ElasticsearchBadConfiguration() Assert.NotNull(serviceProvider); Exception ex = Assert.Throws( - () => serviceProvider.GetService(typeof(IElasticClient)) + () => serviceProvider.GetService(typeof(ElasticsearchClient)) ); Assert.Equal("No servers configured", ex.Message); @@ -130,9 +132,9 @@ public void ConfigureServices_ElasticsearchGoodConfiguration() Assert.NotNull(serviceProvider); - Object svc = serviceProvider.GetService(typeof(IElasticClient)); + Object svc = serviceProvider.GetService(typeof(ElasticsearchClient)); Assert.NotNull(svc); - Assert.IsAssignableFrom(svc); + Assert.IsAssignableFrom(svc); } /// @@ -173,9 +175,9 @@ public void ConfigureServices_GetLoggers() Object svc; - svc = serviceProvider.GetService(typeof(ILogger)); + svc = serviceProvider.GetService(typeof(ILogger)); Assert.NotNull(svc); - Assert.IsAssignableFrom>(svc); + Assert.IsAssignableFrom>(svc); } /// diff --git a/test/NCI.OCPL.Api.Common.Tests/Tests/Services/ESAliasNameProvider.cs b/test/NCI.OCPL.Api.Common.Tests/Tests/Services/ESAliasNameProvider.cs new file mode 100644 index 0000000..a7eed41 --- /dev/null +++ b/test/NCI.OCPL.Api.Common.Tests/Tests/Services/ESAliasNameProvider.cs @@ -0,0 +1,126 @@ +using System; + +using Xunit; + +namespace NCI.OCPL.Api.Common.Tests +{ + /// + /// Unit tests for the class. + /// + public class ESAliasNameProviderTest + { + /// + /// Verify that the Name property returns the value that was set. + /// + [Fact] + public void Name_SetValidValue_ReturnsValue() + { + // Arrange + string expectedName = "test-alias"; + ESAliasNameProvider provider = new ESAliasNameProvider(); + + // Act + provider.Name = expectedName; + + // Assert + Assert.Equal(expectedName, provider.Name); + } + + /// + /// Verify that setting Name to null throws ArgumentException. + /// + [Fact] + public void Name_SetNull_ThrowsArgumentException() + { + // Arrange + ESAliasNameProvider provider = new ESAliasNameProvider(); + + // Act & Assert + Assert.Throws(() => provider.Name = null); + } + + /// + /// Verify that setting Name to an empty string throws ArgumentException. + /// + [Fact] + public void Name_SetEmptyString_ThrowsArgumentException() + { + // Arrange + ESAliasNameProvider provider = new ESAliasNameProvider(); + + // Act & Assert + Assert.Throws(() => provider.Name = string.Empty); + } + + /// + /// Verify that setting Name to a whitespace string throws ArgumentException. + /// + [Theory] + [InlineData(" ")] + [InlineData(" ")] + [InlineData("\t")] + [InlineData("\n")] + [InlineData(" \t\n")] + public void Name_SetWhitespace_ThrowsArgumentException(string whitespace) + { + // Arrange + ESAliasNameProvider provider = new ESAliasNameProvider(); + + // Act & Assert + Assert.Throws(() => provider.Name = whitespace); + } + + /// + /// Verify that the Name property can be set multiple times with different valid values. + /// + [Fact] + public void Name_SetMultipleTimes_ReturnsLatestValue() + { + // Arrange + ESAliasNameProvider provider = new ESAliasNameProvider(); + + // Act + provider.Name = "first-alias"; + provider.Name = "second-alias"; + provider.Name = "final-alias"; + + // Assert + Assert.Equal("final-alias", provider.Name); + } + + /// + /// Verify that the Name property correctly handles values with special characters. + /// + [Theory] + [InlineData("alias-with-dashes")] + [InlineData("alias_with_underscores")] + [InlineData("alias.with.dots")] + [InlineData("alias123")] + [InlineData("UPPERCASE-ALIAS")] + [InlineData("MixedCase-Alias")] + public void Name_SetValueWithSpecialCharacters_ReturnsValue(string aliasName) + { + // Arrange + ESAliasNameProvider provider = new ESAliasNameProvider(); + + // Act + provider.Name = aliasName; + + // Assert + Assert.Equal(aliasName, provider.Name); + } + + /// + /// Verify that accessing an uninitialized Name property throws InvalidOperationException. + /// + [Fact] + public void Name_NotInitialized_ThrowsInvalidOperationException() + { + // Arrange + ESAliasNameProvider provider = new ESAliasNameProvider(); + + // Act and Assert + Assert.Throws(() => { var name = provider.Name; }); + } + } +} diff --git a/test/NCI.OCPL.Api.Common.Tests/Tests/Services/ESHealthCheckService.IndexIsHealthy.cs b/test/NCI.OCPL.Api.Common.Tests/Tests/Services/ESHealthCheckService.IndexIsHealthy.cs new file mode 100644 index 0000000..8a2e324 --- /dev/null +++ b/test/NCI.OCPL.Api.Common.Tests/Tests/Services/ESHealthCheckService.IndexIsHealthy.cs @@ -0,0 +1,303 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; + +using Elastic.Clients.Elasticsearch; +using Elastic.Transport; + +using Microsoft.Extensions.Logging; + +using Moq; + +using Xunit; + +namespace NCI.OCPL.Api.Common.Tests +{ + /// + /// Unit tests for the method. + /// + public partial class ESHealthCheckServiceTest + { + /// + /// Verify that returns true + /// when the cluster status is green. + /// + [Fact] + public async Task IndexIsHealthy_GreenStatus_ReturnsTrue() + { + // Arrange + string aliasName = "test-alias"; + ElasticsearchClient client = GetMockElasticClient(GetGreenStatusResponse()); + Mock mockAliasProvider = new Mock(); + mockAliasProvider.Setup(p => p.Name).Returns(aliasName); + Mock> mockLogger = new Mock>(); + + ESHealthCheckService service = new ESHealthCheckService( + client, + mockAliasProvider.Object, + mockLogger.Object + ); + + // Act + bool result = await service.IndexIsHealthy(); + + // Assert + Assert.True(result); + } + + /// + /// Verify that returns true + /// when the cluster status is yellow. + /// + [Fact] + public async Task IndexIsHealthy_YellowStatus_ReturnsTrue() + { + // Arrange + string aliasName = "test-alias"; + ElasticsearchClient client = GetMockElasticClient(GetYellowStatusResponse()); + Mock mockAliasProvider = new Mock(); + mockAliasProvider.Setup(p => p.Name).Returns(aliasName); + Mock> mockLogger = new Mock>(); + + ESHealthCheckService service = new ESHealthCheckService( + client, + mockAliasProvider.Object, + mockLogger.Object + ); + + // Act + bool result = await service.IndexIsHealthy(); + + // Assert + Assert.True(result); + } + + /// + /// Verify that returns false + /// when the cluster status is red. + /// + [Fact] + public async Task IndexIsHealthy_RedStatus_ReturnsFalse() + { + // Arrange + string aliasName = "test-alias"; + ElasticsearchClient client = GetMockElasticClient(GetRedStatusResponse()); + Mock mockAliasProvider = new Mock(); + mockAliasProvider.Setup(p => p.Name).Returns(aliasName); + Mock> mockLogger = new Mock>(); + + ESHealthCheckService service = new ESHealthCheckService( + client, + mockAliasProvider.Object, + mockLogger.Object + ); + + // Act + bool result = await service.IndexIsHealthy(); + + // Assert + Assert.False(result); + } + + /// + /// Verify that returns false + /// when the Elasticsearch request returns an error status code. + /// + [Theory] + [InlineData(400)] + [InlineData(404)] + [InlineData(500)] + [InlineData(503)] + public async Task IndexIsHealthy_ErrorStatusCode_ReturnsFalse(int statusCode) + { + // Arrange + string aliasName = "test-alias"; + ElasticsearchClient client = GetErrorElasticClient(statusCode); + Mock mockAliasProvider = new Mock(); + mockAliasProvider.Setup(p => p.Name).Returns(aliasName); + Mock> mockLogger = new Mock>(); + + ESHealthCheckService service = new ESHealthCheckService( + client, + mockAliasProvider.Object, + mockLogger.Object + ); + + // Act + bool result = await service.IndexIsHealthy(); + + // Assert + Assert.False(result); + } + + /// + /// Verify that returns false + /// when an exception is thrown during the health check. + /// + [Fact] + public async Task IndexIsHealthy_ExceptionThrown_ReturnsFalse() + { + // Arrange + string aliasName = "test-alias"; + ElasticsearchClient client = GetExceptionThrowingClient(); + Mock mockAliasProvider = new Mock(); + mockAliasProvider.Setup(p => p.Name).Returns(aliasName); + Mock> mockLogger = new Mock>(); + + ESHealthCheckService service = new ESHealthCheckService( + client, + mockAliasProvider.Object, + mockLogger.Object + ); + + // Act + bool result = await service.IndexIsHealthy(); + + // Assert + Assert.False(result); + } + + #region Helper Methods + + /// + /// Gets a mock ElasticsearchClient that returns the specified response body. + /// + private static ElasticsearchClient GetMockElasticClient(byte[] responseBody) + { + var pool = new SingleNodePool(new Uri("http://localhost:9200")); + + var headers = new Dictionary> + { + { "x-elastic-product", new[] { "Elasticsearch" } } + }; + + InMemoryRequestInvoker conn = new InMemoryRequestInvoker(responseBody, headers: headers); + var connectionSettings = new ElasticsearchClientSettings(pool, conn); + + return new ElasticsearchClient(connectionSettings); + } + + /// + /// Gets a mock ElasticsearchClient that returns an error status code. + /// + private static ElasticsearchClient GetErrorElasticClient(int statusCode) + { + var pool = new SingleNodePool(new Uri("http://localhost:9200")); + byte[] responseBody = Array.Empty(); + + var headers = new Dictionary> + { + { "x-elastic-product", new[] { "Elasticsearch" } } + }; + + InMemoryRequestInvoker conn = new InMemoryRequestInvoker(responseBody, statusCode: statusCode, headers: headers); + var connectionSettings = new ElasticsearchClientSettings(pool, conn); + + return new ElasticsearchClient(connectionSettings); + } + + /// + /// Gets a mock ElasticsearchClient that throws an exception. + /// + private static ElasticsearchClient GetExceptionThrowingClient() + { + var pool = new SingleNodePool(new Uri("http://localhost:9200")); + + var headers = new Dictionary> + { + { "x-elastic-product", new[] { "Elasticsearch" } } + }; + + // Use an empty response with a failure status to simulate connection issues + InMemoryRequestInvoker conn = new InMemoryRequestInvoker( + Array.Empty(), + statusCode: 0, + exception: new Exception("Connection failed"), + headers: headers + ); + var connectionSettings = new ElasticsearchClientSettings(pool, conn); + + return new ElasticsearchClient(connectionSettings); + } + + /// + /// Returns a JSON response representing a green cluster health status. + /// + private static byte[] GetGreenStatusResponse() + { + string json = @"{ + ""cluster_name"" : ""docker - cluster"", + ""status"" : ""green"", + ""timed_out"" : false, + ""number_of_nodes"" : 1, + ""number_of_data_nodes"" : 1, + ""active_primary_shards"" : 1, + ""active_shards"" : 1, + ""relocating_shards"" : 0, + ""initializing_shards"" : 0, + ""unassigned_shards"" : 1, + ""unassigned_primary_shards"" : 0, + ""delayed_unassigned_shards"" : 0, + ""number_of_pending_tasks"" : 0, + ""number_of_in_flight_fetch"" : 0, + ""task_max_waiting_in_queue_millis"" : 0, + ""active_shards_percent_as_number"" : 50.0 + }"; + return System.Text.Encoding.UTF8.GetBytes(json); + } + + /// + /// Returns a JSON response representing a yellow cluster health status. + /// + private static byte[] GetYellowStatusResponse() + { + string json = @"{ + ""cluster_name"" : ""docker - cluster"", + ""status"" : ""yellow"", + ""timed_out"" : false, + ""number_of_nodes"" : 1, + ""number_of_data_nodes"" : 1, + ""active_primary_shards"" : 1, + ""active_shards"" : 1, + ""relocating_shards"" : 0, + ""initializing_shards"" : 0, + ""unassigned_shards"" : 1, + ""unassigned_primary_shards"" : 0, + ""delayed_unassigned_shards"" : 0, + ""number_of_pending_tasks"" : 0, + ""number_of_in_flight_fetch"" : 0, + ""task_max_waiting_in_queue_millis"" : 0, + ""active_shards_percent_as_number"" : 50.0 + }"; + return System.Text.Encoding.UTF8.GetBytes(json); + } + + /// + /// Returns a JSON response representing a red cluster health status. + /// + private static byte[] GetRedStatusResponse() + { + string json = @"{ + ""cluster_name"" : ""docker - cluster"", + ""status"" : ""red"", + ""timed_out"" : false, + ""number_of_nodes"" : 1, + ""number_of_data_nodes"" : 1, + ""active_primary_shards"" : 1, + ""active_shards"" : 1, + ""relocating_shards"" : 0, + ""initializing_shards"" : 0, + ""unassigned_shards"" : 1, + ""unassigned_primary_shards"" : 0, + ""delayed_unassigned_shards"" : 0, + ""number_of_pending_tasks"" : 0, + ""number_of_in_flight_fetch"" : 0, + ""task_max_waiting_in_queue_millis"" : 0, + ""active_shards_percent_as_number"" : 50.0 + }"; + return System.Text.Encoding.UTF8.GetBytes(json); + } + + #endregion + } +} diff --git a/test/integration-test-harness/Controllers/HealthCheckController.cs b/test/integration-test-harness/Controllers/HealthCheckController.cs index 9e5d366..21eb9d3 100644 --- a/test/integration-test-harness/Controllers/HealthCheckController.cs +++ b/test/integration-test-harness/Controllers/HealthCheckController.cs @@ -1,11 +1,9 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; -using Microsoft.Extensions.Options; using NCI.OCPL.Api.Common; - namespace integration_test_harness.Controllers { [Route("HealthCheck")] diff --git a/test/integration-test-harness/Controllers/ShutdownController.cs b/test/integration-test-harness/Controllers/ShutdownController.cs new file mode 100644 index 0000000..5a72ebf --- /dev/null +++ b/test/integration-test-harness/Controllers/ShutdownController.cs @@ -0,0 +1,36 @@ +using System.Threading.Tasks; +using Microsoft.Extensions.Hosting; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Options; + +namespace integration_test_harness.Controllers +{ + + /// + /// Controller for shutting down the application. + /// + [Route("shutdown")] + public class ShutdownController : ControllerBase + { + private readonly IHostApplicationLifetime _appLifetime; + + /// + /// Constructor. + /// + /// Application lifetime instance. + public ShutdownController(IHostApplicationLifetime appLifetime) + => _appLifetime = appLifetime; + + /// + /// Shutdown the application. + /// + [HttpPost] + public async Task Shutdown() + { + // Trigger application shutdown. + await Task.Run(() => _appLifetime.StopApplication()); + + return "Shutting down the application."; + } + } +} \ No newline at end of file diff --git a/test/integration-test-harness/Controllers/TestController.cs b/test/integration-test-harness/Controllers/TestController.cs index 8705237..1340e56 100644 --- a/test/integration-test-harness/Controllers/TestController.cs +++ b/test/integration-test-harness/Controllers/TestController.cs @@ -3,8 +3,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; -using Nest; - +using Elastic.Clients.Elasticsearch; namespace integration_test_harness.Controllers { @@ -14,7 +13,7 @@ namespace integration_test_harness.Controllers [Route("test")] public class TestController : ControllerBase { - private readonly IElasticClient _elasticClient; + private readonly ElasticsearchClient _elasticClient; private readonly ESIndexOptions _indexConfig; @@ -29,7 +28,7 @@ public class TestController : ControllerBase /// Elasticsearch client instance. /// Configuration/settings for the query. /// - public TestController(IElasticClient elasticClient, IOptions config) + public TestController(ElasticsearchClient elasticClient, IOptions config) => (_elasticClient, _indexConfig) = (elasticClient, config.Value); /// @@ -40,13 +39,13 @@ public async Task CustomSerialization(string identifie { // Again, don't put Elasticsearch queries into the controller. This is only being // done in order to keep the test harness super-simple. - IndexName index = Indices.Index(this._indexConfig.AliasName); + IndexName index = this._indexConfig.AliasName; - IGetResponse resp = null; + GetResponse resp = null; try { - IGetRequest req = new GetRequest(index, identifier); + GetRequest req = new GetRequest(index, identifier); resp = await _elasticClient.GetAsync(req); } catch (System.Exception) diff --git a/test/integration-test-harness/Controllers/ValidRouteController.cs b/test/integration-test-harness/Controllers/ValidRouteController.cs index dc06130..142461f 100644 --- a/test/integration-test-harness/Controllers/ValidRouteController.cs +++ b/test/integration-test-harness/Controllers/ValidRouteController.cs @@ -1,7 +1,4 @@ -using System.Threading.Tasks; - using Microsoft.AspNetCore.Mvc; -using NCI.OCPL.Api.Common; namespace integration_test_harness.Controllers { diff --git a/test/integration-test-harness/Models/CustomJsonConverter.cs b/test/integration-test-harness/Models/CustomJsonConverter.cs index 1d95234..bdcea94 100644 --- a/test/integration-test-harness/Models/CustomJsonConverter.cs +++ b/test/integration-test-harness/Models/CustomJsonConverter.cs @@ -1,6 +1,7 @@ using System; - -using Newtonsoft.Json; +using System.Text.Json; +using System.Text.Json.Serialization; +using Microsoft.AspNetCore.Http; namespace integration_test_harness { @@ -9,48 +10,69 @@ namespace integration_test_harness /// Converts a JSON element containing either a single string, or an array of strings, into /// a single string. /// - public class CustomJsonConverter : JsonConverter + public class CustomJsonConverter : JsonConverter { /// - /// Responsible for reading a JSON element containing either a single string or an array of strings - /// and converting it into a single string by discarding all but the first one. + /// Responsible for deserialinng CustomSerializationModel, handling the tricky caso of + /// a custom JSON element containing either a single string or an array of strings + /// and (either way) converting it to a single string. /// - /// The JsonReader to read from. - /// Type of the destination object (always System.String). - /// The existing value of the destination object - /// Boolean. Does the destination object already have a value? - /// The calling serializer + /// The Utf8JsonReader to read from. + /// Type of the destination object (always System.String). + /// The serializer options. /// - public override string ReadJson(JsonReader reader, Type objectType, string existingValue, bool hasExistingValue, JsonSerializer serializer) + public override CustomSerializationModel Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - // If it's just a string, return the value. - if (reader.ValueType == typeof(string)) - { - return "Took the string path"; - } - else + var model = new CustomSerializationModel(); + + while(reader.Read() && reader.TokenType != JsonTokenType.EndObject) { - // Otherwise, save the first value and skip past the rest. - string value = reader.ReadAsString(); - while (reader.ReadAsString() != null) ; - return "Took the not string path"; + if (reader.TokenType == JsonTokenType.PropertyName) + { + if(reader.ValueTextEquals("default")) + { + model.Default = reader.Read() ? reader.GetString() : null; + continue; + } + else if (reader.ValueTextEquals("custom")) + { + reader.Read(); // Move to property value + if(reader.TokenType == JsonTokenType.String) + { + // This is the simple string path, so we'll report that fact, + // and deliberately ignore the actual value for demonstration purposes. + model.Custom = reader.GetString(); + } + else + { + // For the array path, read the first value, then skip to end. + // For simplicity, this assumes the array contains at least one value, + // a real implementation would need to be more robust. + model.Custom = reader.Read() ? reader.GetString() : null; + while(reader.Read() && reader.TokenType != JsonTokenType.EndArray) + { + // consume array elements + } + } + } + } } - } - /// - /// Mark the converter as not being used for writing JSON. - /// - public override bool CanWrite => false; + return model; + } /// - /// Writes the JSON representation of the object. + /// Writes the JSON representation of the object using default serialization. /// - /// The JsonWriter to write to. + /// The Utf8JsonWriter to write to. /// The value. - /// The calling serializer. - public override void WriteJson(JsonWriter writer, string value, JsonSerializer serializer) + /// The serializer options. + public override void Write(Utf8JsonWriter writer, CustomSerializationModel value, JsonSerializerOptions options) { - throw new NotImplementedException("This converter not intended for writing."); + writer.WriteStartObject(); + writer.WriteString("default", value.Default); + writer.WriteString("custom", value.Custom); + writer.WriteEndObject(); } } } \ No newline at end of file diff --git a/test/integration-test-harness/Models/CustomSerializationModel.cs b/test/integration-test-harness/Models/CustomSerializationModel.cs index f987ba9..ccdd6f6 100644 --- a/test/integration-test-harness/Models/CustomSerializationModel.cs +++ b/test/integration-test-harness/Models/CustomSerializationModel.cs @@ -1,25 +1,22 @@ -using Nest; -using Newtonsoft.Json; +using System.Text.Json.Serialization; namespace integration_test_harness { /// /// Demonstration of Elasticsearch serialization. /// + [JsonConverter(typeof(CustomJsonConverter))] public class CustomSerializationModel { /// /// Property which uses default serialization. /// - [Text(Name = "default")] public string Default { get; set; } /// /// Property which uses custom serialization to convert an array /// to a single value. /// - [Text(Name = "custom")] - [JsonConverter(typeof(CustomJsonConverter))] public string Custom { get; set; } } } \ No newline at end of file diff --git a/test/integration-test-harness/Program.cs b/test/integration-test-harness/Program.cs index 407c676..675de4d 100644 --- a/test/integration-test-harness/Program.cs +++ b/test/integration-test-harness/Program.cs @@ -1,12 +1,4 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Hosting; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Hosting; using NCI.OCPL.Api.Common; diff --git a/test/integration-test-harness/Startup.cs b/test/integration-test-harness/Startup.cs index a9e042b..47fcde1 100644 --- a/test/integration-test-harness/Startup.cs +++ b/test/integration-test-harness/Startup.cs @@ -1,15 +1,7 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; - using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.HttpsPolicy; -using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Hosting; using NCI.OCPL.Api.Common; diff --git a/test/integration-test-harness/integration-test-harness.csproj b/test/integration-test-harness/integration-test-harness.csproj index 6601e87..8cf813b 100644 --- a/test/integration-test-harness/integration-test-harness.csproj +++ b/test/integration-test-harness/integration-test-harness.csproj @@ -1,7 +1,7 @@ - netcoreapp8.0 + net8.0 @@ -9,7 +9,11 @@ - + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + +