From fc70b74745dc52fef5e5e92d8d52f0e1b1f618db Mon Sep 17 00:00:00 2001 From: Thomas Farr Date: Fri, 12 Jan 2024 03:51:29 +1300 Subject: [PATCH] Add support for `PATCH` in `HttpConnection.ConvertHttpMethod` (#489) (#507) * Add support for PATCH in HttpConnection.ConvertHttpMethod Signed-off-by: Assaf Tirangel * Fix test Signed-off-by: Thomas Farr --------- Signed-off-by: Assaf Tirangel Signed-off-by: Thomas Farr Co-authored-by: Assaf Tirangel Co-authored-by: Thomas Farr (cherry picked from commit 4e721ee854f90fb66f05002c991492c081b68bf0) Co-authored-by: assaftir --- CHANGELOG.md | 3 ++ .../Connection/HttpConnection.cs | 25 +++++------ .../AwsSigV4HttpConnectionTests.cs | 1 + .../Utils/TestableAwsSigV4HttpConnection.cs | 1 + .../Http}/HttpRequestMessageAssertions.cs | 7 ++- .../Http}/MockHttpMessageHandler.cs | 4 +- .../Connection/Http/HttpConnectionTests.cs | 44 +++++++++++++++++++ .../Connection/Http/MockableHttpConnection.cs | 22 ++++++++++ 8 files changed, 90 insertions(+), 17 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/HttpConnectionTests.cs create mode 100644 tests/Tests/Connection/Http/MockableHttpConnection.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 777320fb97..4822fee359 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] +### Fixed +- Fix `HttpConnection.ConvertHttpMethod` to support `Patch` method ([#489](https://github.com/opensearch-project/opensearch-net/pull/489)) + ### Dependencies - Bumps `BenchMarkDotNet` from 0.13.10 to 0.13.11 - Bumps `xunit.runner.visualstudio` from 2.5.4 to 2.5.6 diff --git a/src/OpenSearch.Net/Connection/HttpConnection.cs b/src/OpenSearch.Net/Connection/HttpConnection.cs index a41bea5c5e..25a2b0a839 100644 --- a/src/OpenSearch.Net/Connection/HttpConnection.cs +++ b/src/OpenSearch.Net/Connection/HttpConnection.cs @@ -28,7 +28,6 @@ #if DOTNETCORE using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; @@ -71,6 +70,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; @@ -431,19 +432,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.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 new file mode 100644 index 0000000000..072be7d46f --- /dev/null +++ b/tests/Tests/Connection/Http/HttpConnectionTests.cs @@ -0,0 +1,44 @@ +/* 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 Tests.Core.Connection.Http; +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) + { + HttpRequestMessage sentRequest = null; + var connection = new MockableHttpConnection(r => + { + sentRequest = r; + return new HttpResponseMessage(); + }); + var client = new OpenSearchLowLevelClient(new ConnectionConfiguration(connection: connection)); + + 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; +}