From 1650a0463191319566e680f1a623edaf96c8ca83 Mon Sep 17 00:00:00 2001 From: Assaf Tirangel Date: Mon, 25 Dec 2023 15:57:21 +0200 Subject: [PATCH 1/2] Add support for PATCH in HttpConnection.ConvertHttpMethod Signed-off-by: Assaf Tirangel --- CHANGELOG.md | 5 +- .../Connection/HttpConnection.cs | 25 +++++---- .../Connection/Http/HttpConnectionTests.cs | 52 +++++++++++++++++++ 3 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 tests/Tests/Connection/Http/HttpConnectionTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8491e7dc14..f876bb3972 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Removed the `Features` API which is not supported by OpenSearch from the low-level client ([#331](https://github.com/opensearch-project/opensearch-net/pull/331)) - Removed the deprecated low-level `IndexTemplateV2` APIs in favour of the `ComposableIndexTemplate` APIs ([#437](https://github.com/opensearch-project/opensearch-net/pull/437)) +### Fixed +- Fix `HttpConnection.ConvertHttpMethod` to support `Patch` method ([#489](https://github.com/opensearch-project/opensearch-net/pull/489)) + ### Dependencies - Bumps `Microsoft.CodeAnalysis.CSharp` from 4.2.0 to 4.6.0 - Bumps `NSwag.Core.Yaml` from 13.19.0 to 13.20.0 @@ -130,4 +133,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) [1.6.0]: https://github.com/opensearch-project/opensearch-net/compare/v1.5.0...v1.6.0 [1.5.0]: https://github.com/opensearch-project/opensearch-net/compare/v1.4.0...v1.5.0 [1.4.0]: https://github.com/opensearch-project/opensearch-net/compare/v1.3.0...v1.4.0 -[1.3.0]: https://github.com/opensearch-project/opensearch-net/compare/v1.2.0...v1.3.0 \ No newline at end of file +[1.3.0]: https://github.com/opensearch-project/opensearch-net/compare/v1.2.0...v1.3.0 diff --git a/src/OpenSearch.Net/Connection/HttpConnection.cs b/src/OpenSearch.Net/Connection/HttpConnection.cs index 57c63a34af..dfb8506fd9 100644 --- a/src/OpenSearch.Net/Connection/HttpConnection.cs +++ b/src/OpenSearch.Net/Connection/HttpConnection.cs @@ -27,7 +27,6 @@ */ using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; @@ -70,6 +69,8 @@ public class HttpConnection : IConnection + $" please set {nameof(ConnectionConfiguration.ConnectionLimit)} to -1 on your connection configuration/settings." + $" this will cause the {nameof(HttpClientHandler.MaxConnectionsPerServer)} not to be set on {nameof(HttpClientHandler)}"; + private static readonly System.Net.Http.HttpMethod Patch = new System.Net.Http.HttpMethod("PATCH"); + private RequestDataHttpClientFactory HttpClientFactory { get; } public int InUseHandlers => HttpClientFactory.InUseHandlers; @@ -430,19 +431,17 @@ private static async Task SetContentAsync(HttpRequestMessage message, RequestDat } } - private static System.Net.Http.HttpMethod ConvertHttpMethod(HttpMethod httpMethod) - { - switch (httpMethod) + private static System.Net.Http.HttpMethod ConvertHttpMethod(HttpMethod httpMethod) => + httpMethod switch { - case HttpMethod.GET: return System.Net.Http.HttpMethod.Get; - case HttpMethod.POST: return System.Net.Http.HttpMethod.Post; - case HttpMethod.PUT: return System.Net.Http.HttpMethod.Put; - case HttpMethod.DELETE: return System.Net.Http.HttpMethod.Delete; - case HttpMethod.HEAD: return System.Net.Http.HttpMethod.Head; - default: - throw new ArgumentException("Invalid value for HttpMethod", nameof(httpMethod)); - } - } + HttpMethod.GET => System.Net.Http.HttpMethod.Get, + HttpMethod.POST => System.Net.Http.HttpMethod.Post, + HttpMethod.PUT => System.Net.Http.HttpMethod.Put, + HttpMethod.DELETE => System.Net.Http.HttpMethod.Delete, + HttpMethod.HEAD => System.Net.Http.HttpMethod.Head, + HttpMethod.PATCH => Patch, + _ => throw new ArgumentException("Invalid value for HttpMethod", nameof(httpMethod)) + }; internal static int GetClientKey(RequestData requestData) { diff --git a/tests/Tests/Connection/Http/HttpConnectionTests.cs b/tests/Tests/Connection/Http/HttpConnectionTests.cs new file mode 100644 index 0000000000..2b21eb997b --- /dev/null +++ b/tests/Tests/Connection/Http/HttpConnectionTests.cs @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +using System; +using System.Net.Http; +using FluentAssertions; +using OpenSearch.Net; +using OpenSearch.OpenSearch.Xunit.XunitPlumbing; +using Xunit; +using HttpMethod = OpenSearch.Net.HttpMethod; + +namespace Tests.Connection.Http; + +public class HttpConnectionTests +{ + public static TheoryData HttpMethods() + { + var data = new TheoryData(); + foreach (var httpMethod in Enum.GetValues()) data.Add(httpMethod); + return data; + } + + [TU] + [MemberData(nameof(HttpMethods))] + public void UsesCorrectHttpMethod(HttpMethod method) + { + var mockHttpConnection = new MockHttpConnection(); + mockHttpConnection.Request(new RequestData(method, "", null, null, null, null)); + mockHttpConnection.LastRequest.Method.Should().Be(new System.Net.Http.HttpMethod(method.ToString())); + } + + private class MockHttpConnection : HttpConnection + { + public HttpRequestMessage LastRequest { get; private set; } + + protected override HttpRequestMessage CreateHttpRequestMessage(RequestData requestData) + { + LastRequest = base.CreateHttpRequestMessage(requestData); + return LastRequest; + } + + public override TResponse Request(RequestData requestData) + { + CreateHttpRequestMessage(requestData); + return new TResponse(); + } + } +} From ddb212c0182687d87359f54c6e731cb53f9b6e6b Mon Sep 17 00:00:00 2001 From: Thomas Farr Date: Wed, 10 Jan 2024 11:44:35 +1300 Subject: [PATCH 2/2] Fix test Signed-off-by: Thomas Farr --- .../AwsSigV4HttpConnectionTests.cs | 1 + .../Utils/TestableAwsSigV4HttpConnection.cs | 1 + .../Http}/HttpRequestMessageAssertions.cs | 7 +++- .../Http}/MockHttpMessageHandler.cs | 4 +- .../Connection/Http/HttpConnectionTests.cs | 38 ++++++++----------- .../Connection/Http/MockableHttpConnection.cs | 22 +++++++++++ 6 files changed, 46 insertions(+), 27 deletions(-) rename tests/{Tests.Auth.AwsSigV4/Utils => Tests.Core/Connection/Http}/HttpRequestMessageAssertions.cs (64%) rename tests/{Tests.Auth.AwsSigV4/Utils => Tests.Core/Connection/Http}/MockHttpMessageHandler.cs (86%) create mode 100644 tests/Tests/Connection/Http/MockableHttpConnection.cs diff --git a/tests/Tests.Auth.AwsSigV4/AwsSigV4HttpConnectionTests.cs b/tests/Tests.Auth.AwsSigV4/AwsSigV4HttpConnectionTests.cs index 1496020819..f81216af20 100644 --- a/tests/Tests.Auth.AwsSigV4/AwsSigV4HttpConnectionTests.cs +++ b/tests/Tests.Auth.AwsSigV4/AwsSigV4HttpConnectionTests.cs @@ -16,6 +16,7 @@ using OpenSearch.Client; using OpenSearch.OpenSearch.Xunit.XunitPlumbing; using Tests.Auth.AwsSigV4.Utils; +using Tests.Core.Connection.Http; using Xunit; namespace Tests.Auth.AwsSigV4; diff --git a/tests/Tests.Auth.AwsSigV4/Utils/TestableAwsSigV4HttpConnection.cs b/tests/Tests.Auth.AwsSigV4/Utils/TestableAwsSigV4HttpConnection.cs index d18e554a71..1950f4c63e 100644 --- a/tests/Tests.Auth.AwsSigV4/Utils/TestableAwsSigV4HttpConnection.cs +++ b/tests/Tests.Auth.AwsSigV4/Utils/TestableAwsSigV4HttpConnection.cs @@ -10,6 +10,7 @@ using Amazon.Runtime; using OpenSearch.Net; using OpenSearch.Net.Auth.AwsSigV4; +using Tests.Core.Connection.Http; namespace Tests.Auth.AwsSigV4.Utils; diff --git a/tests/Tests.Auth.AwsSigV4/Utils/HttpRequestMessageAssertions.cs b/tests/Tests.Core/Connection/Http/HttpRequestMessageAssertions.cs similarity index 64% rename from tests/Tests.Auth.AwsSigV4/Utils/HttpRequestMessageAssertions.cs rename to tests/Tests.Core/Connection/Http/HttpRequestMessageAssertions.cs index c6305d5524..567a263fa9 100644 --- a/tests/Tests.Auth.AwsSigV4/Utils/HttpRequestMessageAssertions.cs +++ b/tests/Tests.Core/Connection/Http/HttpRequestMessageAssertions.cs @@ -8,10 +8,13 @@ using System.Net.Http; using FluentAssertions; -namespace Tests.Auth.AwsSigV4.Utils; +namespace Tests.Core.Connection.Http; -internal static class HttpRequestMessageAssertions +public static class HttpRequestMessageAssertions { + public static void ShouldHaveMethod(this HttpRequestMessage request, string method) => + request.Method.Should().Be(new HttpMethod(method)); + public static void ShouldHaveHeader(this HttpRequestMessage request, string name, string value) => request.Headers.GetValues(name).Should().BeEquivalentTo(value); } diff --git a/tests/Tests.Auth.AwsSigV4/Utils/MockHttpMessageHandler.cs b/tests/Tests.Core/Connection/Http/MockHttpMessageHandler.cs similarity index 86% rename from tests/Tests.Auth.AwsSigV4/Utils/MockHttpMessageHandler.cs rename to tests/Tests.Core/Connection/Http/MockHttpMessageHandler.cs index 81199b740d..bbcd4a3152 100644 --- a/tests/Tests.Auth.AwsSigV4/Utils/MockHttpMessageHandler.cs +++ b/tests/Tests.Core/Connection/Http/MockHttpMessageHandler.cs @@ -9,9 +9,9 @@ using System.Threading; using System.Threading.Tasks; -namespace Tests.Auth.AwsSigV4.Utils; +namespace Tests.Core.Connection.Http; -internal class MockHttpMessageHandler : HttpMessageHandler +public class MockHttpMessageHandler : HttpMessageHandler { public delegate HttpResponseMessage Handler(HttpRequestMessage request); diff --git a/tests/Tests/Connection/Http/HttpConnectionTests.cs b/tests/Tests/Connection/Http/HttpConnectionTests.cs index 2b21eb997b..072be7d46f 100644 --- a/tests/Tests/Connection/Http/HttpConnectionTests.cs +++ b/tests/Tests/Connection/Http/HttpConnectionTests.cs @@ -1,15 +1,16 @@ /* SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ using System; using System.Net.Http; using FluentAssertions; using OpenSearch.Net; using OpenSearch.OpenSearch.Xunit.XunitPlumbing; +using Tests.Core.Connection.Http; using Xunit; using HttpMethod = OpenSearch.Net.HttpMethod; @@ -28,25 +29,16 @@ public static TheoryData HttpMethods() [MemberData(nameof(HttpMethods))] public void UsesCorrectHttpMethod(HttpMethod method) { - var mockHttpConnection = new MockHttpConnection(); - mockHttpConnection.Request(new RequestData(method, "", null, null, null, null)); - mockHttpConnection.LastRequest.Method.Should().Be(new System.Net.Http.HttpMethod(method.ToString())); - } - - private class MockHttpConnection : HttpConnection - { - public HttpRequestMessage LastRequest { get; private set; } - - protected override HttpRequestMessage CreateHttpRequestMessage(RequestData requestData) + HttpRequestMessage sentRequest = null; + var connection = new MockableHttpConnection(r => { - LastRequest = base.CreateHttpRequestMessage(requestData); - return LastRequest; - } + sentRequest = r; + return new HttpResponseMessage(); + }); + var client = new OpenSearchLowLevelClient(new ConnectionConfiguration(connection: connection)); - public override TResponse Request(RequestData requestData) - { - CreateHttpRequestMessage(requestData); - return new TResponse(); - } + client.DoRequest(method, "/"); + + sentRequest.ShouldHaveMethod(method.ToString()); } } diff --git a/tests/Tests/Connection/Http/MockableHttpConnection.cs b/tests/Tests/Connection/Http/MockableHttpConnection.cs new file mode 100644 index 0000000000..5588497059 --- /dev/null +++ b/tests/Tests/Connection/Http/MockableHttpConnection.cs @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +using System.Net.Http; +using OpenSearch.Net; +using Tests.Core.Connection.Http; + +namespace Tests.Connection.Http; + +public class MockableHttpConnection : HttpConnection +{ + private readonly MockHttpMessageHandler _handler; + + public MockableHttpConnection(MockHttpMessageHandler.Handler handler) => + _handler = new MockHttpMessageHandler(handler); + + protected override HttpMessageHandler CreateHttpClientHandler(RequestData requestData) => _handler; +}