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

When deserializing to SearchResponse<JsonData> getting error for suggest field #1024

Open
AayushiJain012 opened this issue Jun 11, 2024 · 15 comments
Labels
bug Something isn't working

Comments

@AayushiJain012
Copy link

AayushiJain012 commented Jun 11, 2024

What is the bug?

While trying to deserialize this response I am getting StringOutofBoundException
Response -
{"took":4,"timed_out":false,"_shards":{"failed":0.0,"successful":1.0,"total":1.0,"skipped":0.0},"hits":{"total":{"relation":"eq","value":0},"hits":[]},"aggregations":{"cardinality#itemCount":{"value":0}},"suggest":{"correction":[{"length":5,"offset":0,"text":"talbe","options":[{"text":"table","freq":22,"score":0.8}]}]}}

Exception -
java.lang.StringIndexOutOfBoundsException: begin 0, end -1, length 10
at java.base/java.lang.String.checkBoundsBeginEnd(String.java:4608)
at java.base/java.lang.String.substring(String.java:2711)
at org.opensearch.client.json.ExternallyTaggedUnion.lambda$arrayDeserializer$0(ExternallyTaggedUnion.java:152)
at org.opensearch.client.json.JsonpDeserializer$3.deserialize(JsonpDeserializer.java:138)
at org.opensearch.client.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:87)
at org.opensearch.client.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:81)
at org.opensearch.client.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:185)
at org.opensearch.client.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:146)
at org.opensearch.client.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:87)
at org.opensearch.client.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:91)

Method to deserialize -

private static final SearchResponse deserializeSearchResponse(String astrJson) throws IOException {
		SearchResponse<JsonData> searchResponseCopy = null;
		JsonFactory jsonFactory = new JsonFactory();
		JsonParser jsonParser = jsonFactory.createParser(astrJson);
		JacksonJsonpParser jsonpParser = new JacksonJsonpParser(jsonParser);
		try {
			JsonpDeserializer<JsonData> jsonpDeserializer = JsonpDeserializer.of(JsonData.class);
		       JsonpDeserializer<SearchResponse<JsonData>> searchResponseDeserializer = SearchResponse.createSearchResponseDeserializer(jsonpDeserializer);
			searchResponseCopy = searchResponseDeserializer.deserialize(jsonpParser, new JacksonJsonpMapper());
		} catch (Exception ex) {
			LOGGER.warn("Failed to deserialize {}", astrJson, ex);
			throw ex;
		}
		return searchResponseCopy;
}

**Method used to serialize - **

private static final String serializeSearchRequest(SearchRequest searchRequest)
			throws IOException {
		String strJson = null;
		try {
			JacksonJsonpMapper jsonpMapper = new JacksonJsonpMapper();

			StringWriter writer = new StringWriter();
			JacksonJsonpGenerator generator = new JacksonJsonpGenerator(new JsonFactory().createGenerator(writer));

			searchRequest.serialize(generator, jsonpMapper);
			generator.flush();
			strJson = writer.toString();
		} catch (Exception ex) {
			LOGGER.warn("Failed to serialize {}", searchRequest, ex);
			throw ex;
		}
		return strJson;
	} 

How can one reproduce the bug?

Create a request with suggester, serialize the response and then deserialize it.

What is the expected behavior?

No Error and get deserialized response

What is your host/environment?

Operating system, version.

Do you have any screenshots?

If applicable, add screenshots to help explain your problem.

Do you have any additional context?

Java client version - 2.10.2

@AayushiJain012 AayushiJain012 added bug Something isn't working untriaged labels Jun 11, 2024
@dblock
Copy link
Member

dblock commented Jun 11, 2024

Want to turn this into a (failing) unit test?

@dblock dblock removed the untriaged label Jun 11, 2024
@AayushiJain012
Copy link
Author

This is not the only place where serialization is not happening properly.
If we have sub aggregation in that case SearchResponse.serialize() is not able to serialize sub aggregation (second level aggregation) and then throwing error when i deserialize it.

@dblock
Copy link
Member

dblock commented Jun 11, 2024

@AayushiJain012 You're absolutely right, all these instances need to be fixed :( We have recently merged a beginning of a code generator that aims to resolve this entire class of problems (#366). There are a few things you can do to help:

  1. Ensure that the specification for these APIs in https://github.com/opensearch-project/opensearch-api-specification is correct, and add tests for them in that repo.
  2. Write tests in this repo for the scenarios that are broken like the ones you're reporting and manually fix any of these bugs.

The combination will ensure that as we switch to the generator we're not introducing regressions.

@AayushiJain012
Copy link
Author

I dont understand how the changes you have mentioned is related to the problem I am facing. I am talking about this serialize method
https://github.com/Xtansia/opensearch-java/blob/13982b1ba1c89061c5886eb8232468c8798f3aec/java-client/src/main/java/org/opensearch/client/opensearch/core/search/SearchResult.java#L217C17-L217C26

@dblock
Copy link
Member

dblock commented Jun 12, 2024

Generally these problems are trying to deserialize something that's the wrong type in code vs. the response. Start by writing a unit test for this problem, should point exactly to the field that is the issue.

@AayushiJain012
Copy link
Author

AayushiJain012 commented Jun 12, 2024

The problem is with serialize method not properly serializing suggest and nested-aggregation

Can you help me with the logic how to correctly serialize search response in Java client

@dblock
Copy link
Member

dblock commented Jun 12, 2024

Can you help me with the logic how to correctly serialize search response in Java client

Probably. Write a failing unit test for it? @Xtansia can point you to something similar.

@Xtansia
Copy link
Collaborator

Xtansia commented Jun 12, 2024

The failure is happening here: https://github.com/opensearch-project/opensearch-java/blob/main/java-client/src/main/java/org/opensearch/client/json/ExternallyTaggedUnion.java#L150-L152

It's missing the check from this other path: https://github.com/opensearch-project/opensearch-java/blob/main/java-client/src/main/java/org/opensearch/client/json/ExternallyTaggedUnion.java#L126-L131

Which is to say, you need to enable typed_keys on your search request for the deserializer to be happy. The Java client does this automatically when using the client.search() method.

@AayushiJain012
Copy link
Author

I am using client.search()
The issue is during serialization. Like aggregation, in suggest field we dont add # and during deserialization it throws an error.
Please check serialization logic for suggest field here -

@Xtansia
Copy link
Collaborator

Xtansia commented Jun 13, 2024

I am using client.search()
The issue is during serialization. Like aggregation, in suggest field we dont add # and during deserialization it throws an error.
Please check serialization logic for suggest field here -

@AayushiJain012 I see, that is a problem types should be able to round-trip if they implement both serialize & deserialize. However, could you please explain a bit your use case for why you need to re-serialize & deserialize the response object rather than using it directly or mapping into your own object structure?

@AayushiJain012
Copy link
Author

@Xtansia We are getting the response as JsonData and then serializing it to store in cache. For second request if data is in cache we simply deserialize it and send back, instead of making call to opensearch.

@AayushiJain012
Copy link
Author

@Xtansia @dblock
Are we going to fix this serialization issue? Or can you point me how we serialize the opensearch response which is then deserialize by java-client. I can also try to do something same and write my own code to serialize.

It will be good if we fix these issue in code itself.

@dblock
Copy link
Member

dblock commented Jun 21, 2024

Would love a PR that fixes the serialization issue.

@AayushiJain012
Copy link
Author

who will work on that PR and in when we can get the fix?

@dblock
Copy link
Member

dblock commented Jun 24, 2024

who will work on that PR and in when we can get the fix?

Nobody is working on it right now. Maybe you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants