Skip to content
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

JsonSerializerSettings not honored for Field Expressions #3926

Closed
octocat-mona opened this issue Jul 5, 2019 · 3 comments · Fixed by #3942
Closed

JsonSerializerSettings not honored for Field Expressions #3926

octocat-mona opened this issue Jul 5, 2019 · 3 comments · Fixed by #3942
Assignees

Comments

@octocat-mona
Copy link

NEST/Elasticsearch.Net version:
7.0.1

NEST.JsonNetSerializer version:
7.0.1

Elasticsearch version:
7.2.0

Description of the problem including expected versus actual behavior:
Expected: be able to query on a field using an Expression lambda which should honor the [JsonProperty] attributes when configured using JsonSerializerSettings just like in NEST 6.8.0.

Actual: No query results are returned.

Steps to reproduce:

  1. DELETE my-index
  2. Index a document and query it using a date range filter:
var pool = new SingleNodeConnectionPool(new Uri("https://localhost.fiddler:9200"));
var settings = new ConnectionSettings(pool, (serializer, connectionSettings)
        => new JsonNetSerializer(serializer, connectionSettings))
    .BasicAuthentication("elastic", "elastic");
var client = new ElasticClient(settings);
var doc = new DocumentModel { Timestamp = DateTime.UtcNow, DocumentName = "Doc 1" };

await client.IndexAsync(doc, descriptor => descriptor
    .Index("my-index")
    .Refresh(Refresh.True)
);

var countResponse = await client.CountAsync<DocumentModel>(descriptor => descriptor
    .Index("my-index")
);
long totalDocs = countResponse.Count; // totalDocs = 1

var searchResponse = await client.SearchAsync<DocumentModel>(descriptor => descriptor
    .Index("my-index")
    .Query(q => q
        .DateRange(range => range
            .Field(model => model.Timestamp)
            .GreaterThan("1970-01-01"))
    )
);
int count = searchResponse.Documents.Count; // count = 0

The poco model:

public class DocumentModel
{
    [JsonProperty("@timestamp")]
    public DateTime Timestamp { get; set; }

    [JsonProperty("document_name")]
    public string DocumentName { get; set; }
}
  1. Notice that the date range filter uses the lowercased Timestamp property name, not @timestamp from the [JsonProperty] attribute:
POST https://localhost:9200/my-index/_search?typed_keys=true HTTP/1.1
Accept: application/json
Content-Type: application/json
User-Agent: elasticsearch-net/7.0.1 (Microsoft Windows 10.0.17134; .NET Framework 4.7.3416.0; Nest)
Authorization: Basic ZWxhc3RpYzplbGFzdGlj
Host: localhost:9200
Content-Length: 53

{"query":{"range":{"timestamp":{"gt":"1970-01-01"}}}}
@octocat-mona
Copy link
Author

Also the documentation for 7.x seems not updated yet?
It still mentions Newtonsoft Json which is removed completely now?

@russcam
Copy link
Contributor

russcam commented Jul 8, 2019

@KPStolk You are correct that some of the documentation needs to be updated to reflect that 7.x no longer ships with an internalized version of Json.NET. The use of NEST.JsonNetSerializer nuget package is still valid however.

@codebrain codebrain mentioned this issue Jul 9, 2019
17 tasks
@russcam russcam self-assigned this Jul 11, 2019
russcam added a commit that referenced this issue Jul 11, 2019
This commit fixes a bug introduced with Diagnostics Source support. DiagnosticsSerializerProxy does not implement IPropertyMappingProvider, so if it wraps and delegates to a serializer that does implement IPropertyMappingProvider, such as JsonNetSerialzer, this is not honoured when assigning to _propertyMappingProvider. This commit uses the sourceSerializer when determining _propertyMappingProvider, and not the DiagnosticsSerializerProxy.

Fixes #3926
@russcam
Copy link
Contributor

russcam commented Jul 11, 2019

Opened #3942 to address

russcam added a commit that referenced this issue Jul 12, 2019
)

This commit fixes a bug introduced with Diagnostics Source support. DiagnosticsSerializerProxy does not implement IPropertyMappingProvider, so if it wraps and delegates to a serializer that does implement IPropertyMappingProvider, such as JsonNetSerialzer, this is not honoured when assigning to _propertyMappingProvider. This commit uses the sourceSerializer when determining _propertyMappingProvider, and not the DiagnosticsSerializerProxy.

Fixes #3926
russcam added a commit that referenced this issue Jul 12, 2019
)

This commit fixes a bug introduced with Diagnostics Source support. DiagnosticsSerializerProxy does not implement IPropertyMappingProvider, so if it wraps and delegates to a serializer that does implement IPropertyMappingProvider, such as JsonNetSerialzer, this is not honoured when assigning to _propertyMappingProvider. This commit uses the sourceSerializer when determining _propertyMappingProvider, and not the DiagnosticsSerializerProxy.

Fixes #3926

(cherry picked from commit 8e6afad)
russcam added a commit that referenced this issue Jul 18, 2019
)

This commit fixes a bug introduced with Diagnostics Source support. DiagnosticsSerializerProxy does not implement IPropertyMappingProvider, so if it wraps and delegates to a serializer that does implement IPropertyMappingProvider, such as JsonNetSerialzer, this is not honoured when assigning to _propertyMappingProvider. This commit uses the sourceSerializer when determining _propertyMappingProvider, and not the DiagnosticsSerializerProxy.

Fixes #3926

(cherry picked from commit 8e6afad)
codebrain pushed a commit that referenced this issue Jul 19, 2019
)

This commit fixes a bug introduced with Diagnostics Source support. DiagnosticsSerializerProxy does not implement IPropertyMappingProvider, so if it wraps and delegates to a serializer that does implement IPropertyMappingProvider, such as JsonNetSerialzer, this is not honoured when assigning to _propertyMappingProvider. This commit uses the sourceSerializer when determining _propertyMappingProvider, and not the DiagnosticsSerializerProxy.

Fixes #3926

(cherry picked from commit 8e6afad)
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 a pull request may close this issue.

2 participants