IRequestConfiguration properties should be nullable to allow using IConnectionConfigurationValues by default #3231
Description
NEST/Elasticsearch.Net version: 6.1.0
Elasticsearch version: 6.1.1
Description of the problem including expected versus actual behavior:
ElasticLowLevelClient Cat* methods behavior is wrong regarding exception throwing.
Steps to reproduce:
- Assuming Elastic Search is running locally with default settings.
- Run NUnit tests:
[TestCase(false)]
[TestCase(true, ExpectedException = typeof(ElasticsearchClientException))]
public void CatIndicesUsesConnectionConfigWhenRequestConfigNotSetTest(bool connectionIsSetToThrow)
{
var config = new ConnectionConfiguration()
.ThrowExceptions(connectionIsSetToThrow);
var client = new ElasticLowLevelClient(config);
client.CatIndices<StringResponse>("non_existing_index");
}
[TestCase(false, false)]
[TestCase(false, true, ExpectedException = typeof(ElasticsearchClientException))]
[TestCase(true, false)]
[TestCase(true, true, ExpectedException = typeof(ElasticsearchClientException))]
public void CatIndicesUsesRequestConfigWhenItIsSetTest(bool connectionIsSetToThrow, bool requestIsSetToThrow)
{
var config = new ConnectionConfiguration()
.ThrowExceptions(connectionIsSetToThrow);
var client = new ElasticLowLevelClient(config);
var catIndicesRequestParameters = new CatIndicesRequestParameters{ RequestConfiguration = new RequestConfiguration { ThrowExceptions = requestIsSetToThrow } };
client.CatIndices<StringResponse>("non_existing_index", catIndicesRequestParameters);
}
- Verify that CatIndicesUsesConnectionConfigWhenRequestConfigNotSetTest(true)
fails because exception is not thrown.
Provide DebugInformation (if relevant):
The problem is that ElasticLowLevelClient._params method creates a new RequestConfiguration instance itself when null is passed to override two settings: ContentType and Accept.
IRequestConfiguration.ThrowExceptions property is not nullable so it is initialized with default(bool) value. Later this value is used in RequestData constructor to merge connection settings with request settings: ThrowExceptions = (local?.ThrowExceptions ?? global.ThrowExceptions);
Declaring IRequestParameters.ThrowExceptions as bool? would solve the issue and also would make it easier to write a code when only certain properties of IRequestConfiguration should be changed, and all the others should be taken from connection config. BTW, this already applied to some other properties of IRequestConfiguration:
- int? MaxRetries
- bool? DisableSniff
...
Please consider to make bool EnableHttpPipelining nullable as well.
P. S. Looking at constructor RequestConfigurationDescriptor(IRequestConfiguration config) it seems like property ThrowExceptions was forgotten there.