Skip to content

Conversation

@russcam
Copy link
Contributor

@russcam russcam commented Aug 11, 2018

When the join field is a string and using JsonNetSerializer, the string
conversion should not emit the surrounding double quotes

Closes #3356

When the join field is a string and using JsonNetSerializer, the string
conversion should not emit the surrounding double quotes

Closes #3356
[U]
public void JoinFieldDeserializedCorrectly()
{
var pool = new SingleNodeConnectionPool(new Uri("http://localhost:9200"));
Copy link
Member

Choose a reason for hiding this comment

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

LGTM small suggestion though is to use the recently centralized json serialization assertions.

var tester = SerializationTester.DefaultWithSourceSerializer;
var response = tester.Client.IndexDocument(doc);
tester.AssertSerialize(response.ApiCall.RequestBodyInBytes, new { join = "parent" });
var doc = tester.AssertDeserialize<MyDocument>(response.ApiCall.RequestBodyInBytes);

Copy link
Member

Choose a reason for hiding this comment

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

SerializationTester.DefaultWithSourceSerializer;

Would need to be added though

Copy link
Member

@Mpdreamz Mpdreamz Aug 12, 2018

Choose a reason for hiding this comment

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

Could even use only AssertRoundTrip<MyDocument>() rather than AssertSerialize and AssertDeserialize separately

Update following PR
@Mpdreamz Mpdreamz merged commit 8d4cf41 into master Aug 14, 2018
@Mpdreamz Mpdreamz deleted the fix/3356 branch August 14, 2018 12:02
@Mpdreamz Mpdreamz mentioned this pull request Sep 10, 2018
45 tasks
russcam added a commit that referenced this pull request Sep 14, 2018
* Correctly deserialize JoinField

When the join field is a string and using JsonNetSerializer, the string
conversion should not emit the surrounding double quotes

Closes #3356

* Use SerializationTester

Update following PR

(cherry picked from commit 8d4cf41)
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.

3 participants