Skip to content

fix #1930 unwrap .Terms(new List<T>) to just T #1948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 23, 2016

Conversation

Mpdreamz
Copy link
Member

When calling

.Terms(new List<T>());

The user expects T[] to be written to elasticsearch but due to C#
resolution they end up calling

.Terms<List<T>>(params List<T>)

and we serialize to T[][] which is not a valid terms query.

The fluent Terms() method now tries to preempt these cases and unwraps
to T[] when a single IEnumerable is passed as T, special casing string
to be exluded from the unwrapping behavior so we do not unwrap string to
char[]

This PR also introduces QueryDslIntegrationTestsBase, all of our QueryDSL tests are unit tests but with this new base class we can start introducing more integration tests for the query dsl over time /cc @gmarz @russcam

When calling

```csharp
.Terms(new List<T>());
```

The user expects T[] to be written to elasticsearch but due to C#
resolution they end up calling

```csharp
.Terms<List<T>>(params List<T>)
```

 and we serialize to T[][] which is not a valid terms query.

The fluent Terms() method now tries to preempt these cases and unwraps
to T[] when a single IEnumerable is passed as T, special casing `string`
to be exluded from the unwrapping behavior so we do not unwrap string to
char[]
namespace Tests.QueryDsl
{
[Collection(IntegrationContext.ReadOnly)]
public abstract class QueryDslIntegrationTestsBase : ApiIntegrationTestBase<ISearchResponse<Project>, ISearchRequest, SearchDescriptor<Project>, SearchRequest<Project>>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@gmarz
Copy link
Contributor

gmarz commented Mar 23, 2016

LGTM

@gmarz gmarz merged commit 3c3518d into 2.x Mar 23, 2016
@gmarz gmarz deleted the fix/support-list-of-terms-natively branch March 23, 2016 19:04
@gmarz
Copy link
Contributor

gmarz commented Mar 23, 2016

merged to 2.x and master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants