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

[v5] UpdateResponse doesn't match the actual response json from elasticsearch. #541

Closed
yizha opened this issue Jun 12, 2017 · 8 comments
Closed
Assignees

Comments

@yizha
Copy link

yizha commented Jun 12, 2017

Please use the following questions as a guideline to help me answer
your issue/question without further inquiry. Thank you.

Which version of Elastic are you using?

[ ] elastic.v2 (for Elasticsearch 1.x)
[ ] elastic.v3 (for Elasticsearch 2.x)
[X] elastic.v5 (for Elasticsearch 5.x)

Please describe the expected behavior

UpdateResponse struct should match the response from Elasticsearch v5.4, but it doesn't now.

UpdateResponse definition from update.go:

// UpdateResponse is the result of updating a document in Elasticsearch.
type UpdateResponse struct {
	Index     string     `json:"_index"`
	Type      string     `json:"_type"`
	Id        string     `json:"_id"`
	Version   int        `json:"_version"`
	Created   bool       `json:"created"`
	GetResult *GetResult `json:"get"`
}

Actual response from Elasticsearch v5.4

{
  "_index": "test",
  "_type": "test",
  "_id": "AVyP-TFcWnJHS5P8SKUK",
  "_version": 3,
  "result": "updated",
  "_shards": {
    "total": 2,
    "successful": 1,
    "failed": 0
  }
}

Please describe the actual behavior

UpdateResponse struct should look like:

type UpdateResponse struct {
	Index     string     `json:"_index"`
	Type      string     `json:"_type"`
	Id        string     `json:"_id"`
	Version   int        `json:"_version"`
	Result    string     `json:"result"`
}

Any steps to reproduce the behavior?

N/A.

@olivere
Copy link
Owner

olivere commented Jun 12, 2017

Yes, that structure changed recently. I will change it to reflect the current version. Thanks.

@olivere olivere self-assigned this Jun 12, 2017
@olivere
Copy link
Owner

olivere commented Jun 16, 2017

The returned structures for Update and Delete in ES 5.4.1 are actually:

// UpdateResponse is the result of updating a document in Elasticsearch.
type UpdateResponse struct {
	Index         string      `json:"_index"`
	Type          string      `json:"_type"`
	Id            string      `json:"_id"`
	Version       int         `json:"_version"`
	Shards        *shardsInfo `json:"_shards"`
	Result        bool        `json:"string,omitempty"`
	ForcedRefresh bool        `json:"forced_refresh,omitempty"`
	GetResult     *GetResult  `json:"get,omitempty"`
}

// DeleteResponse is the outcome of running DeleteService.Do.
type DeleteResponse struct {
	Index         string      `json:"_index"`
	Type          string      `json:"_type"`
	Id            string      `json:"_id"`
	Version       int64       `json:"_version"`
	Shards        *shardsInfo `json:"_shards"`
	Result        bool        `json:"string,omitempty"`
	ForcedRefresh bool        `json:"forced_refresh,omitempty"`
	Found         bool        `json:"found"`
}

I will fix that in 5.0.41.

@olivere olivere closed this as completed Jun 16, 2017
@olivere
Copy link
Owner

olivere commented Jun 16, 2017

Ref: e3b7c0

@claudiuolteanu
Copy link
Contributor

Can you please detail why the Result field from DeleteResponse and UpdateResponse is represented as a boolean? From what I saw in the documentation of ES 5.4.1 it is represented as a string and its possible values are represented in this enum:
https://github.com/elastic/elasticsearch/blob/5.4/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java#L64
Here you can find the implementation of UpdateResponse class which extends the DocWriteResponse:
https://github.com/elastic/elasticsearch/blob/v5.4.1/core/src/main/java/org/elasticsearch/action/update/UpdateResponse.java#L35

If we represent the Result field as bool, it will alway be False.

@olivere
Copy link
Owner

olivere commented Sep 12, 2017

@claudiuolteanu It's clearly wrong for UpdateResponse. But for DeleteResponse, it is correct.

Thanks for addressing this. Maybe you can write a PR?

Tracked in #599

@claudiuolteanu
Copy link
Contributor

claudiuolteanu commented Sep 14, 2017

Can you please take a look at #600 ? thanks

@olivere
Copy link
Owner

olivere commented Sep 14, 2017

@claudiuolteanu I already did. However, I need to find some time first. I hope to work on Elastic this weekend, and include some of the outstanding PRs.

@claudiuolteanu
Copy link
Contributor

Ok, thanks. If I can help you with something, please let me know.

Have a nice week

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

No branches or pull requests

3 participants