Merge branch 'fix/3378'

Author: russcam

src/Elasticsearch.Net/Responses/ElasticsearchResponse.cs

@@ -8,32 +8,43 @@ namespace Elasticsearch.Net
 	/// </summary>
 	public abstract class ElasticsearchResponseBase : IApiCallDetails, IElasticsearchResponse
 	{
+		/// <inheritdoc />
 		public IApiCallDetails ApiCall { get; set; }
-		public List<Audit> AuditTrail => ApiCall.AuditTrail;
 
+		/// <inheritdoc cref="IApiCallDetails.DebugInformation"/>
 		public string DebugInformation => ApiCall.DebugInformation;
-		public IEnumerable<string> DeprecationWarnings => ApiCall.DeprecationWarnings;
+		/// <inheritdoc cref="IApiCallDetails.HttpMethod"/>
 		public HttpMethod HttpMethod => ApiCall.HttpMethod;
+		/// <inheritdoc cref="IApiCallDetails.AuditTrail"/>
+		public List<Audit> AuditTrail => ApiCall.AuditTrail;
+		/// <inheritdoc cref="IApiCallDetails.DeprecationWarnings"/>
+		public IEnumerable<string> DeprecationWarnings => ApiCall.DeprecationWarnings;
+		/// <inheritdoc cref="IApiCallDetails.SuccessOrKnownError"/>
+		public bool SuccessOrKnownError => ApiCall.SuccessOrKnownError;
+		/// <inheritdoc cref="IApiCallDetails.HttpStatusCode"/>
 		public int? HttpStatusCode => ApiCall.HttpStatusCode;
-		public Exception OriginalException => ApiCall.OriginalException;
 
-		/// <summary>The raw byte request message body, only set when DisableDirectStreaming() is set on Connection configuration</summary>
-		public byte[] RequestBodyInBytes => ApiCall.RequestBodyInBytes;
+		/// <inheritdoc cref="IApiCallDetails.Success"/>
+		public bool Success => ApiCall.Success;
+		/// <inheritdoc cref="IApiCallDetails.OriginalException"/>
+		public Exception OriginalException => ApiCall.OriginalException;
+		/// <inheritdoc cref="IApiCallDetails.ResponseMimeType"/>
+		public string ResponseMimeType => ApiCall.ResponseMimeType;
+		/// <inheritdoc cref="IApiCallDetails.Uri"/>
+		public Uri Uri => ApiCall.Uri;
 
-		/// <summary>The raw byte response message body, only set when DisableDirectStreaming() is set on Connection configuration</summary>
+		/// <inheritdoc cref="IApiCallDetails.ResponseBodyInBytes"/>
 		public byte[] ResponseBodyInBytes => ApiCall.ResponseBodyInBytes;
 
-		public string ResponseMimeType => ApiCall.ResponseMimeType;
+		/// <inheritdoc cref="IApiCallDetails.RequestBodyInBytes"/>
+		public byte[] RequestBodyInBytes => ApiCall.RequestBodyInBytes;
 
-		public bool Success => ApiCall.Success;
-		public bool SuccessOrKnownError => ApiCall.SuccessOrKnownError;
-		public Uri Uri => ApiCall.Uri;
+		bool IElasticsearchResponse.TryGetServerErrorReason(out string reason) => TryGetServerErrorReason(out reason);
 
+		// TODO: Remove?
 		//ignored
 		List<Audit> IApiCallDetails.AuditTrail { get; set; }
 
-		bool IElasticsearchResponse.TryGetServerErrorReason(out string reason) => TryGetServerErrorReason(out reason);
-
 		protected virtual bool TryGetServerErrorReason(out string reason)
 		{
 			reason = null;
@@ -45,7 +56,7 @@ protected virtual bool TryGetServerErrorReason(out string reason)
 
 	/// <summary>
 	/// A response from Elasticsearch including details about the request/response life cycle. Base class for the built in low level response
-	/// types: <see cref="StringResponse" /> <see cref="BytesResponse" /> and <see cref="DynamicResponse" />
+	/// types, <see cref="StringResponse"/>, <see cref="BytesResponse"/>, <see cref="DynamicResponse"/> and <see cref="VoidResponse"/>
 	/// </summary>
 	public abstract class ElasticsearchResponse<T> : ElasticsearchResponseBase
 	{

src/Elasticsearch.Net/Responses/HttpDetails/IApiCallDetails.cs

@@ -4,19 +4,23 @@
 
 namespace Elasticsearch.Net
 {
+	/// <summary>
+	/// Details about the API call
+	/// </summary>
 	public interface IApiCallDetails
 	{
-		//TODO Get rid of setter
-		/// <summary>
-		/// An audit trail of requests made to nodes within the cluster
-		/// </summary>
-		List<Audit> AuditTrail { get; set; }
 
-		/// <summary>
-		/// A lazy human readable string representation of what happened during this request for both successful and
-		/// failed requests.
-		/// </summary>
-		string DebugInformation { get; }
+		//TODO: Get rid of setter
+        /// <summary>
+        /// An audit trail of requests made to nodes within the cluster
+        /// </summary>
+        List<Audit> AuditTrail { get; set; }
+
+        /// <summary>
+        /// A lazy human readable string representation of what happened during this request for both successful and
+        /// failed requests.
+        /// </summary>
+        string DebugInformation { get; }
 
 		/// <summary>
 		/// A collection of deprecation warnings returned from Elasticsearch.
@@ -35,28 +39,27 @@ public interface IApiCallDetails
 		int? HttpStatusCode { get; }
 
 		/// <summary>
-		/// If <see cref="Success" /> is <c>false</c>, this will hold the original exception.
-		/// This will be the orginating CLR exception in most cases.
+		/// If <see cref="Success"/> is <c>false</c>, this will hold the original exception.
+		/// This will be the originating CLR exception in most cases.
 		/// </summary>
 		Exception OriginalException { get; }
 
-		/// <summary>
-		/// The request body bytes.
-		/// <para>NOTE: Only set when disable direct streaming is set for the request</para>
-		/// </summary>
-		[DebuggerDisplay("{RequestBodyInBytes != null ? System.Text.Encoding.UTF8.GetString(RequestBodyInBytes) : null,nq}")]
+        /// <summary>
+        /// The request body bytes.
+        /// <para>NOTE: Only set when disable direct streaming is set for the request</para>
+        /// </summary>
+        [DebuggerDisplay("{RequestBodyInBytes != null ? System.Text.Encoding.UTF8.GetString(RequestBodyInBytes) : null,nq}")]
 		byte[] RequestBodyInBytes { get; }
 
-		/// <summary>
-		/// The response body bytes.
-		/// <para>NOTE: Only set when disable direct streaming is set for the request</para>
-		/// </summary>
-		[DebuggerDisplay("{ResponseBodyInBytes != null ? System.Text.Encoding.UTF8.GetString(ResponseBodyInBytes) : null,nq}")]
+        /// <summary>
+        /// The response body bytes.
+        /// <para>NOTE: Only set when disable direct streaming is set for the request</para>
+        /// </summary>
+        [DebuggerDisplay("{ResponseBodyInBytes != null ? System.Text.Encoding.UTF8.GetString(ResponseBodyInBytes) : null,nq}")]
 		byte[] ResponseBodyInBytes { get; }
 
-		/// <summary>The response mime type </summary>
+		/// <summary>The response MIME type </summary>
 		string ResponseMimeType { get; }
-
 		/// <summary>
 		/// The response status code is in the 200 range or is in the allowed list of status codes set on the request.
 		/// </summary>

src/Elasticsearch.Net/Responses/ServerException/ServerError.cs

@@ -18,9 +18,24 @@ public ServerError(Error error, int? statusCode)
 		public Error Error { get; }
 		public int Status { get; }
 
+		public static bool TryCreate(Stream stream, out ServerError serverError)
+		{
+			try
+			{
+				serverError = Create(stream);
+				return true;
+			}
+			catch
+			{
+				serverError = null;
+				return false;
+			}
+		}
+
 		public static ServerError Create(Stream stream) =>
 			LowLevelRequestResponseSerializer.Instance.Deserialize<ServerError>(stream);
 
+		// TODO: make token default parameter in 7.x
 		public static Task<ServerError> CreateAsync(Stream stream, CancellationToken token) =>
 			LowLevelRequestResponseSerializer.Instance.DeserializeAsync<ServerError>(stream, token);
 

src/Elasticsearch.Net/Responses/Special/BytesResponse.cs

@@ -1,4 +1,5 @@
-using System.IO;
+using System;
+using System.IO;
 
 namespace Elasticsearch.Net
 {
@@ -11,11 +12,11 @@ public BytesResponse() { }
 		public bool TryGetServerError(out ServerError serverError)
 		{
 			serverError = null;
-			if (Body == null || Body.Length == 0) return false;
+			if (Body == null || Body.Length == 0 || ResponseMimeType != RequestData.MimeType)
+				return false;
 
-			using (var stream = new MemoryStream(Body))
-				serverError = ServerError.Create(stream);
-			return true;
+			using(var stream = new MemoryStream(Body))
+				return ServerError.TryCreate(stream, out serverError);
 		}
 
 		protected override bool TryGetServerErrorReason(out string reason)

src/Elasticsearch.Net/Responses/Special/StringResponse.cs

@@ -1,4 +1,5 @@
-using System.IO;
+using System;
+using System.IO;
 using System.Text;
 
 namespace Elasticsearch.Net
@@ -12,11 +13,11 @@ public StringResponse() { }
 		public bool TryGetServerError(out ServerError serverError)
 		{
 			serverError = null;
-			if (string.IsNullOrEmpty(Body)) return false;
+			if (string.IsNullOrEmpty(Body) || ResponseMimeType != RequestData.MimeType)
+				return false;
 
-			using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(Body)))
-				serverError = ServerError.Create(stream);
-			return true;
+			using(var stream = new MemoryStream(Encoding.UTF8.GetBytes(Body)))
+				return ServerError.TryCreate(stream, out serverError);
 		}
 
 		protected override bool TryGetServerErrorReason(out string reason)

src/Tests/Tests.Core/Client/FixedResponseClient.cs

@@ -28,12 +28,25 @@ public static ConnectionSettings CreateConnectionSettings(
 		)
 		{
 			var serializer = TestClient.Default.RequestResponseSerializer;
-			byte[] fixedResult = null;
-			if (response is string s) fixedResult = Encoding.UTF8.GetBytes(s);
-			else if (contentType == RequestData.MimeType) fixedResult = serializer.SerializeToBytes(response);
-			else fixedResult = Encoding.UTF8.GetBytes(response.ToString());
+			byte[] responseBytes;
+			switch (response)
+			{
+				case string s:
+					responseBytes = Encoding.UTF8.GetBytes(s);
+					break;
+				case byte[] b:
+					responseBytes = b;
+					break;
+				default:
+				{
+					responseBytes = contentType == RequestData.MimeType
+						? serializer.SerializeToBytes(response)
+						: Encoding.UTF8.GetBytes(response.ToString());
+					break;
+				}
+			}
 
-			var connection = new InMemoryConnection(fixedResult, statusCode, exception, contentType);
+			var connection = new InMemoryConnection(responseBytes, statusCode, exception, contentType);
 			var connectionPool = new SingleNodeConnectionPool(new Uri("http://localhost:9200"));
 			var defaultSettings = new ConnectionSettings(connectionPool, connection)
 				.DefaultIndex("default-index");

src/Tests/Tests/ClientConcepts/ConnectionPooling/Exceptions/UnrecoverableExceptions.doc.cs

@@ -9,6 +9,7 @@
 using Nest;
 using Tests.Domain;
 using Tests.Framework;
+using Tests.Framework.SerializationTests;
 
 namespace Tests.ClientConcepts.ConnectionPooling.Exceptions
 {
@@ -99,14 +100,7 @@ [U] public async Task BadAuthenticationIsUnrecoverable()
 			);
 		}
 
-		private static byte[] HtmlNginx401Response = Encoding.UTF8.GetBytes(@"<html>
-<head><title>401 Authorization Required</title></head>
-<body bgcolor=""white"">
-<center><h1>401 Authorization Required</h1></center>
-<hr><center>nginx/1.4.6 (Ubuntu)</center>
-</body>
-</html>
-");
+		private static byte[] HtmlNginx401Response = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);
 
 		/**
 		 * When a bad authentication response occurs, the client attempts to deserialize the response body returned;

src/Tests/Tests/Framework/SerializationTests/BytesResponseTests.cs

@@ -0,0 +1,73 @@
+using System;
+using System.Linq;
+using System.Text;
+using Elastic.Xunit.XunitPlumbing;
+using Elasticsearch.Net;
+using FluentAssertions;
+using Tests.Core.Client;
+using Tests.Core.ManagedElasticsearch.Clusters;
+
+namespace Tests.Framework.SerializationTests
+{
+	public class BytesResponseTests : IClusterFixture<ReadOnlyCluster>
+	{
+		private readonly ReadOnlyCluster _cluster;
+
+		public BytesResponseTests(ReadOnlyCluster cluster) => _cluster = cluster;
+
+		[I] public void NonNullBytesResponse()
+		{
+			var client = _cluster.Client;
+
+			var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
+			bytesResponse.Body.Should().NotBeNull();
+			bytesResponse.Body.Should().BeEquivalentTo(bytesResponse.ResponseBodyInBytes);
+		}
+
+		[I] public void NonNullBytesLowLevelResponse()
+		{
+			var settings  = new ConnectionConfiguration(new Uri($"http://localhost:{_cluster.Nodes.First().Port ?? 9200}"));
+			var lowLevelClient = new ElasticLowLevelClient(settings);
+
+			var bytesResponse = lowLevelClient.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
+
+			bytesResponse.Body.Should().NotBeNull();
+			bytesResponse.Body.Should().BeEquivalentTo(bytesResponse.ResponseBodyInBytes);
+		}
+
+		[U]
+		public void TryGetServerErrorDoesNotThrowException()
+		{
+			var responseBytes = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);
+
+			var client = FixedResponseClient.Create(responseBytes, 401,
+				modifySettings: s => s.DisableDirectStreaming(),
+				contentType: "text/html",
+				exception: new Exception("problem with the request as a result of 401")
+			);
+
+			var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
+
+			bytesResponse.Body.Should().NotBeNull();
+			bytesResponse.Body.Should().BeEquivalentTo(responseBytes);
+			bytesResponse.TryGetServerError(out var serverError).Should().BeFalse();
+			serverError.Should().BeNull();
+		}
+
+		[U] public void SkipDeserializationForStatusCodesSetsBody()
+		{
+			var responseBytes = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);
+
+			var client = FixedResponseClient.Create(responseBytes, 401,
+				modifySettings: s => s.DisableDirectStreaming().SkipDeserializationForStatusCodes(401),
+				contentType: "text/html",
+				exception: new Exception("problem with the request as a result of 401")
+			);
+
+			var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
+
+			bytesResponse.Body.Should().NotBeNull();
+			bytesResponse.Body.Should().BeEquivalentTo(responseBytes);
+		}
+	}
+}

src/Tests/Tests/Framework/SerializationTests/StringResponseTests.cs

@@ -0,0 +1,42 @@
+using System;
+using System.Text;
+using Elastic.Xunit.XunitPlumbing;
+using Elasticsearch.Net;
+using FluentAssertions;
+using Tests.Core.Client;
+
+namespace Tests.Framework.SerializationTests
+{
+	public class StringResponseTests
+	{
+		[U] public void TryGetServerErrorDoesNotThrowException()
+		{
+			var client = FixedResponseClient.Create(StubResponse.NginxHtml401Response, 401,
+				modifySettings: s => s.DisableDirectStreaming(),
+				contentType: "text/html",
+				exception: new Exception("problem with the request as a result of 401")
+			);
+
+			var stringResponse = client.LowLevel.Search<StringResponse>("project", "project", PostData.Serializable(new { }));
+
+			stringResponse.Body.Should().NotBeNull();
+			stringResponse.Body.Should().Be(StubResponse.NginxHtml401Response);
+			stringResponse.TryGetServerError(out var serverError).Should().BeFalse();
+			serverError.Should().BeNull();
+		}
+
+		[U] public void SkipDeserializationForStatusCodesSetsBody()
+		{
+			var client = FixedResponseClient.Create(StubResponse.NginxHtml401Response, 401,
+				modifySettings: s => s.DisableDirectStreaming().SkipDeserializationForStatusCodes(401),
+				contentType: "text/html",
+				exception: new Exception("problem with the request as a result of 401")
+			);
+
+			var stringResponse = client.LowLevel.Search<StringResponse>("project", "project", PostData.Serializable(new { }));
+
+			stringResponse.Body.Should().NotBeNull();
+			stringResponse.Body.Should().Be(StubResponse.NginxHtml401Response);
+		}
+	}
+}