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

[BUG] Unable to deserialize SearchRequest with _source field as an array #1087

Closed
chanon-onman opened this issue Jul 17, 2024 · 16 comments · Fixed by #1117
Closed

[BUG] Unable to deserialize SearchRequest with _source field as an array #1087

chanon-onman opened this issue Jul 17, 2024 · 16 comments · Fixed by #1117
Assignees
Labels
bug Something isn't working

Comments

@chanon-onman
Copy link

chanon-onman commented Jul 17, 2024

What is the bug?

The error org.opensearch.client.json.UnexpectedJsonEventException: Unexpected JSON event 'START_ARRAY' instead of '[START_OBJECT, KEY_NAME, VALUE_STRING, VALUE_TRUE, VALUE_FALSE] is raised when trying to deserialize following json payload to SearchRequest.

{
  "query": {
    "match_all": {}
  },
  "_source": ["_id"],
  "size": 1
}

Context: We are implementing a kind of proxy service which accepts any OpenSearch requests and overwrite some query logic before passing them to the OpenSearch cluster. Previously, we use opensearch-rest-high-level-client with XContentParser to parse incoming requests but now we would like to migrate to opensearch-java library.

Note that if _source is removed or change to string field, it can parse successfully but we cannot do this as a workaround as we expect to accept any form of valid requests from clients.

How can one reproduce the bug?

Execute following code and then will get org.opensearch.client.json.UnexpectedJsonEventException

import com.fasterxml.jackson.core.JsonFactory;
import org.opensearch.client.json.jackson.JacksonJsonpMapper;
import org.opensearch.client.json.jackson.JacksonJsonpParser;
import org.opensearch.client.opensearch.core.SearchRequest;

import java.io.IOException;

public class Test {
    public static void main(String[] args) throws IOException {
        String jsonPayload = "{\n" +
                "  \"query\": {\n" +
                "    \"match_all\": {}\n" +
                "  },\n" +
                "  \"_source\": [\"_id\"],\n" +
                "  \"size\": 1\n" +
                "}\n";

        SearchRequest searchRequest = SearchRequest._DESERIALIZER.deserialize(new JacksonJsonpParser(new JsonFactory().createParser(jsonPayload)), new JacksonJsonpMapper());
    }
}

What is the expected behavior?

We expect SearchRequest._DESERIALIZER can parse the json payload successfully.

What is your host/environment?

  • library = opensearch-java 2.11.1
  • SDK = Amazon Correto 11
  • OS = MacOS Somona 14.5
@dblock
Copy link
Member

dblock commented Jul 17, 2024

Indeed, looks like arrays of fields aren't properly deserializing. I fixed the spec in opensearch-project/opensearch-api-specification#430 and opened opensearch-project/documentation-website#7748 for the docs.

curl -k -X POST --json '{"director":"Bennett Miller","title":"Moneyball","year":2011}' -u admin:admin https://localhost:9200/movies/_doc

All these work:

$ curl -k -X POST --json '{"query":{"match_all":{}}}' -u admin:admin https://localhost:9200/movies/_search 
$ curl -k -X POST --json '{"_source":false,"query":{"match_all":{}}}' -u admin:admin https://localhost:9200/movies/_search 
$ curl -k -X POST --json '{"_source":["title"],"query":{"match_all":{}}}' -u admin:admin https://localhost:9200/movies/_search

@chanon-onman Add a (failing) unit test? Want to try to fix it?

@dblock dblock removed the untriaged label Jul 17, 2024
@chanon-onman
Copy link
Author

Thank @dblock for help looking into this. I can try fix it but I'm not sure if I can do it correctly or not. Can you help guide me? Do you have sample PR that does similar things?

@dblock
Copy link
Member

dblock commented Jul 17, 2024

I would start by seeing if the code generator that @Xtansia recently added would just do the right thing and add support for this after opensearch-project/opensearch-api-specification#430 is merged.

@chanon-onman
Copy link
Author

@dblock I run following command and got the below error, not sure how to fix it? Do you a document how to run codegen properly?

$ ./gradlew :java-codegen:downloadLatestSpec :java-codegen:run

> Configure project :java-client
=======================================
  Runtime JDK Version   : 11
  Gradle JDK Version    : 11
=======================================

> Configure project :java-codegen
=======================================
  Runtime JDK Version   : 11
  Gradle JDK Version    : 11
=======================================

> Task :java-codegen:downloadLatestSpec
Download https://github.com/opensearch-project/opensearch-api-specification/releases/download/main-latest/opensearch-openapi.yaml

> Task :java-codegen:run
[CodeGenerator] INFO  - Specification Location: /Users/chanononman/opensearch-java/java-codegen/opensearch-openapi.yaml
[CodeGenerator] INFO  - Eclipse Configuration: /Users/chanononman/opensearch-java/buildSrc/formatterConfig-generated.xml
[CodeGenerator] INFO  - Output Directory: /Users/chanononman/opensearch-java/java-client/src/generated/java
[OpenApiSpecification] INFO  - Parsing spec: /Users/chanononman/opensearch-java/java-codegen/opensearch-openapi.yaml
[CodeGenerator] FATAL - Unexpected Error
java.lang.IllegalArgumentException: Multiple types not yet supported: [boolean, null, number, object, string]
        at org.opensearch.client.codegen.openapi.OpenApiSchema.lambda$new$0(OpenApiSchema.java:89) ~[main/:?]
        at java.base/java.util.Optional.flatMap(Optional.java:294) ~[?:?]
        at org.opensearch.client.codegen.openapi.OpenApiSchema.<init>(OpenApiSchema.java:82) ~[main/:?]
        at org.opensearch.client.codegen.openapi.OpenApiElement.lambda$children$3(OpenApiElement.java:116) ~[main/:?]
        at org.opensearch.client.codegen.utils.Maps$EntryMapper.map(Maps.java:68) ~[main/:?]
        at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:178) ~[?:?]
        at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169) ~[?:?]
        at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133) ~[?:?]
        at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801) ~[?:?]
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913) ~[?:?]
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
        at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578) ~[?:?]
        at org.opensearch.client.codegen.utils.Maps.transform(Maps.java:58) ~[main/:?]
        at org.opensearch.client.codegen.openapi.OpenApiElement.children(OpenApiElement.java:113) ~[main/:?]
        at org.opensearch.client.codegen.openapi.OpenApiElement.children(OpenApiElement.java:96) ~[main/:?]
        at org.opensearch.client.codegen.openapi.OpenApiComponents.<init>(OpenApiComponents.java:30) ~[main/:?]
        at org.opensearch.client.codegen.openapi.OpenApiElement.child(OpenApiElement.java:67) ~[main/:?]
        at org.opensearch.client.codegen.openapi.OpenApiSpecification.<init>(OpenApiSpecification.java:68) ~[main/:?]
        at org.opensearch.client.codegen.openapi.OpenApiSpecification.retrieve(OpenApiSpecification.java:47) ~[main/:?]
        at org.opensearch.client.codegen.CodeGenerator.parseSpec(CodeGenerator.java:113) ~[main/:?]
        at org.opensearch.client.codegen.CodeGenerator.main(CodeGenerator.java:84) [main/:?]

> Task :java-codegen:run FAILED

FAILURE: Build failed with an exception.

@dblock
Copy link
Member

dblock commented Jul 22, 2024

@Xtansia ^

@chanon-onman
Copy link
Author

I can run codegen successfully when building opensearch-openapi.yaml locally from opensearch-api-specification commit e1cd369.

@dblock
Copy link
Member

dblock commented Jul 23, 2024

I kicked https://github.com/opensearch-project/opensearch-java/actions/runs/10050371147, and it failed, java.lang.IllegalArgumentException: Multiple types not yet supported: [boolean, null, number, object, string].

Care to open an issue for this @chanon-onman please? Or maybe fix it? :)

@Xtansia
Copy link
Collaborator

Xtansia commented Jul 23, 2024

Have made #1102 to fix the code generator error

@chanon-onman
Copy link
Author

Thank @Xtansia for your helps.
One thing I see when executing :java-codegen:run is it visits only info group. I'm not sure if it is supposed to generate code for every groups or not? And when I try changing OPERATION_MATCHER in CodeGenerator to search, I got this error.

Code:

private static final OperationGroup.Matcher OPERATION_MATCHER = OperationGroup.matcher().add(null, "search");

Error:

$ ./gradlew :java-codegen:run

> Configure project :java-client
=======================================
  Runtime JDK Version   : 11
  Gradle JDK Version    : 11
=======================================

> Configure project :java-codegen
=======================================
  Runtime JDK Version   : 11
  Gradle JDK Version    : 11
=======================================

> Task :java-codegen:run
[CodeGenerator] INFO  - Specification Location: /Users/chanononman/ips/opensearch-java/java-codegen/opensearch-openapi.yaml
[CodeGenerator] INFO  - Eclipse Configuration: /Users/chanononman/ips/opensearch-java/buildSrc/formatterConfig-generated.xml
[CodeGenerator] INFO  - Output Directory: /Users/chanononman/ips/opensearch-java/java-client/src/generated/java
[OpenApiSpecification] INFO  - Parsing spec: /Users/chanononman/ips/opensearch-java/java-codegen/opensearch-openapi.yaml
[SpecTransformer] INFO  - Visiting Specification: org.opensearch.client.codegen.openapi.OpenApiSpecification@6f1ff437[location=/Users/chanononman/ips/opensearch-java/java-codegen/opensearch-openapi.yaml]
[SpecTransformer] INFO  - Visiting Operation Group: search [4 variants]
[SpecTransformer] INFO  - Visiting Parameter: org.opensearch.client.codegen.openapi.OpenApiParameter@6f2c2e16[pointer=/components/parameters/search::path.index]
[SpecTransformer] INFO  - Visiting Parameter: org.opensearch.client.codegen.openapi.OpenApiParameter@4bb62c05[pointer=/components/parameters/search::query._source]
[CodeGenerator] FATAL - Unexpected Error
java.lang.UnsupportedOperationException: Can not get type name for oneOf: [org.opensearch.client.codegen.openapi.OpenApiSchema@45716ad8[pointer=/components/schemas/_core.search:SourceConfigParam/oneOf/0]]
        at org.opensearch.client.codegen.model.SpecTransformer.mapOneOf(SpecTransformer.java:375) ~[main/:?]
        at org.opensearch.client.codegen.model.SpecTransformer.mapTypeInner(SpecTransformer.java:329) ~[main/:?]
        at org.opensearch.client.codegen.model.SpecTransformer.mapType(SpecTransformer.java:304) ~[main/:?]
        at org.opensearch.client.codegen.model.SpecTransformer.mapType(SpecTransformer.java:298) ~[main/:?]
        at org.opensearch.client.codegen.model.SpecTransformer.mapTypeInner(SpecTransformer.java:315) ~[main/:?]
        at org.opensearch.client.codegen.model.SpecTransformer.mapType(SpecTransformer.java:304) ~[main/:?]
        at org.opensearch.client.codegen.model.SpecTransformer.mapType(SpecTransformer.java:298) ~[main/:?]
        at org.opensearch.client.codegen.model.SpecTransformer.visit(SpecTransformer.java:214) ~[main/:?]
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195) ~[?:?]
        at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177) ~[?:?]
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655) ~[?:?]
        at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:658) ~[?:?]
        at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:274) ~[?:?]
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655) ~[?:?]
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
        at java.base/java.util.stream.ReferencePipeline.forEachOrdered(ReferencePipeline.java:502) ~[?:?]
        at org.opensearch.client.codegen.model.SpecTransformer.visit(SpecTransformer.java:188) ~[main/:?]
        at org.opensearch.client.codegen.model.SpecTransformer.visit(SpecTransformer.java:97) ~[main/:?]
        at java.base/java.util.HashMap.forEach(HashMap.java:1337) ~[?:?]
        at org.opensearch.client.codegen.model.SpecTransformer.visit(SpecTransformer.java:89) ~[main/:?]
        at org.opensearch.client.codegen.CodeGenerator.parseSpec(CodeGenerator.java:115) ~[main/:?]
        at org.opensearch.client.codegen.CodeGenerator.main(CodeGenerator.java:84) [main/:?]

> Task :java-codegen:run FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':java-codegen:run'.
> Process 'command '/Users/chanononman/.gradle/jdks/eclipse_adoptium-11-x86_64-os_x.2/jdk-11.0.24+8/Contents/Home/bin/java'' finished with non-zero exit value 1

@Xtansia
Copy link
Collaborator

Xtansia commented Jul 24, 2024

@chanon-onman Yes the code generator is only recently introduced and have not yet started generating other operations yet. So there will be aspects of the more complicated operations such as search that's not yet implemented/handled by the generator. If you're interested in digging into this and working to get generation of search working, that'd be amazing, but unfortunately it's not going to be as simple as @dblock's comment might have suggested.

The easier of the two options immediately would be to just amend the current SearchRequest implementation.

@chanon-onman
Copy link
Author

@Xtansia Thanks for clarification. Can you help guide me how to modify SearchRequest deserializer? Does it need to use any code generator?

@chanon-onman
Copy link
Author

@Xtansia @dblock I am not sure I can fix this myself. Can you help fix this issue?

@Xtansia Xtansia self-assigned this Jul 31, 2024
@Xtansia
Copy link
Collaborator

Xtansia commented Jul 31, 2024

Apologies for the delay @chanon-onman, I'm taking a look into this now

@Xtansia
Copy link
Collaborator

Xtansia commented Aug 1, 2024

Have found the issue (and made a fix in #1117), the actual typings in SearchRequest were correct (so generator wouldn't have changed anything), this was actually an issue in the general deserialization logic of how we handle "shortcut" properties on objects.

ie. The real type is:

{
  "includes": ["_id"],
  "excludes": []
}

But we accept specifying just "_id" or ["_id"] as shortcutting to the includes property. The code was only handling the string case and not the array case.

@Xtansia
Copy link
Collaborator

Xtansia commented Aug 1, 2024

@chanon-onman The fix for this was just released as part of 2.13.0: https://central.sonatype.com/artifact/org.opensearch.client/opensearch-java/2.13.0

@chanon-onman
Copy link
Author

Thanks @Xtansia 👍

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

Successfully merging a pull request may close this issue.

3 participants