Add support for PATCH in HttpConnection.ConvertHttpMethod

[assaftir]

Description

Add support for PATCH in HttpConnection.ConvertHttpMethod.

Issues Resolved

Fixes #488

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[dblock]

Thanks. Add a test please?

[assaftir]

Thanks. Add a test please?

Sure, will do asap.
Any guidance for expected test?

[dblock]

@assaftir No particular guidance, see if there are any existing tests with verbs. If not add something. I'm happy if the code path is exercised and tests would fail without this change.

[Xtansia]

Hi @assaftir,

Thanks for catching this, don't know how I missed it in the initial feature PR!

I've just got one minor nit about using a const.

As far as testing this, I think the simplest possible would be something like the following added to the Tests project:

/* 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 FluentAssertions;
using OpenSearch.Net;
using OpenSearch.OpenSearch.Xunit.XunitPlumbing;
using Xunit;

namespace Tests.Connection.Http;

public class HttpConnectionTests
{
	public static TheoryData<HttpMethod> HttpMethods()
	{
		var data = new TheoryData<HttpMethod>();
		foreach (var httpMethod in Enum.GetValues<HttpMethod>()) data.Add(httpMethod);
		return data;
	}

	[TU]
	[MemberData(nameof(HttpMethods))]
	public void ConvertHttpMethod_ConvertsMethod(HttpMethod method)
	{
		var converted = HttpConnection.ConvertHttpMethod(method);
		converted.ToString().Should().BeEquivalentTo(method.ToString());
	}
}

But it's debatable whether this is a good test as it's directly prodding an implementation detail of HttpConnection. Otherwise could do something like https://github.com/opensearch-project/opensearch-net/blob/main/tests/Tests.Auth.AwsSigV4/AwsSigV4HttpConnectionTests.cs and create a mockable subclass of HttpConnection, then using same theory data and calling low-level HTTP method then asserting on the constructed HttpRequestMessage

[Xtansia]

This will also need an entry added to the Unreleased section of the CHANGELOG.md under a Fixed heading

[assaftir]

Hi @assaftir,

Thanks for catching this, don't know how I missed it in the initial feature PR!

I've just got one minor nit about using a const.

As far as testing this, I think the simplest possible would be something like the following added to the Tests project:

/* 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 FluentAssertions;

using OpenSearch.Net;

using OpenSearch.OpenSearch.Xunit.XunitPlumbing;

using Xunit;



namespace Tests.Connection.Http;



public class HttpConnectionTests

{

	public static TheoryData<HttpMethod> HttpMethods()

	{

		var data = new TheoryData<HttpMethod>();

		foreach (var httpMethod in Enum.GetValues<HttpMethod>()) data.Add(httpMethod);

		return data;

	}



	[TU]

	[MemberData(nameof(HttpMethods))]

	public void ConvertHttpMethod_ConvertsMethod(HttpMethod method)

	{

		var converted = HttpConnection.ConvertHttpMethod(method);

		converted.ToString().Should().BeEquivalentTo(method.ToString());

	}

}

But it's debatable whether this is a good test as it's directly prodding an implementation detail of HttpConnection. Otherwise could do something like https://github.com/opensearch-project/opensearch-net/blob/main/tests/Tests.Auth.AwsSigV4/AwsSigV4HttpConnectionTests.cs and create a mockable subclass of HttpConnection, then using same theory data and calling low-level HTTP method then asserting on the constructed HttpRequestMessage

I've implemented a test, let me know if it works, otherwise I'll go the route you took with the 'AwsSigV4' tests.

I'm not a huge fan of unit tests and not familiar with the xUnit framework, so forgive me if I haven't done that right.

[opensearch-trigger-bot]

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.x 1.x
# Navigate to the new working tree
cd .worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-489-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4e721ee854f90fb66f05002c991492c081b68bf0
# Push it to GitHub
git push --set-upstream origin backport/backport-489-to-1.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-489-to-1.x.