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] BulkRequest with scripted noop upsert UpdateOperation fails with "missing required property" #1042

Closed
BrendonFaleiro opened this issue Jun 20, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@BrendonFaleiro
Copy link
Contributor

BrendonFaleiro commented Jun 20, 2024

What is the bug?

Similar to Elasticsearch bug: elastic/elasticsearch-java#403.
When you do a scripted upsert with a script that does a "noop", there is no source in the response. But the InlineGet has source as a required field.
https://github.com/opensearch-project/opensearch-java/blob/main/java-client/src/main/java/org/opensearch/client/opensearch/_types/InlineGet.java#L141-L146

A bulk request with an update operation set to do scripted upsert where the script chooses to set ctx.op = 'none' currently fails with

Missing required property 'InlineGet.source': org.opensearch.client.util.MissingRequiredPropertyException
org.opensearch.client.util.MissingRequiredPropertyException: Missing required property 'InlineGet.source'
	at org.opensearch.client.util.ApiTypeHelper.requireNonNull(ApiTypeHelper.java:89)
	at org.opensearch.client.opensearch._types.InlineGet.<init>(InlineGet.java:87)
	at org.opensearch.client.opensearch._types.InlineGet.<init>(InlineGet.java:55)
	at org.opensearch.client.opensearch._types.InlineGet$Builder.build(InlineGet.java:326)
	at org.opensearch.client.opensearch._types.InlineGet$Builder.build(InlineGet.java:205)
	at org.opensearch.client.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:92)
	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.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:97)
	at org.opensearch.client.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:60)
	at org.opensearch.client.json.JsonpDeserializerBase$ArrayDeserializer.deserialize(JsonpDeserializerBase.java:343)
	at org.opensearch.client.json.JsonpDeserializerBase$ArrayDeserializer.deserialize(JsonpDeserializerBase.java:308)
	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)
	at org.opensearch.client.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:55)

How can one reproduce the bug?

BulkResponse response = client.bulk(
        BulkRequest.of(bulkRequest -> bulkRequest
                .index("test-index")
                .operations(bulkOperation -> bulkOperation
                        .update(updateOperation -> updateOperation
                            .index("indexName")
                            .id("myId")
                            .script(new Script.Builder()
                                .inline(new InlineScript.Builder()
                                    .lang("painless")
                                    .source("ctx.op = 'none';")
                                    .build())
                            .scriptedUpsert(true)
                            .upsert(ImmutableMap.of())
                        )
               )
        )
);

What is the expected behavior?

This should succeed.

The response from Opensearch that reaches the Transport is

{
    "took": 2,
    "errors": false,
    "items": [{
        "update": {
            "_index": "test-index",
            "_id": "myId",
            "_version": -1,
            "result": "noop",
            "_shards": {
                "total": 3,
                "successful": 3,
                "failed": 0
            },
            "get": {
                "found": false
            },
            "status": 200
        }
    }]
}

Do you have any additional context?

Similar to Elasticsearch bug: elastic/elasticsearch-java#403.

@BrendonFaleiro
Copy link
Contributor Author

@dblock
Copy link
Member

dblock commented Jun 20, 2024

I think this will also require a change in the api spec: https://github.com/opensearch-project/opensearch-api-specification/blob/33aa91a95dcb2155cc05b9917a790ca46aa7c35e/spec/schemas/_common.yaml#L1258

Yes, please! With a test.

@BrendonFaleiro
Copy link
Contributor Author

BrendonFaleiro commented Jun 20, 2024

@dblock

With a test.

Did you mean in opensearch-java or in the api-spec? I have added a commit with the test.

I have sent the following requests:

Please let me know if anything else needs to be done.

@BrendonFaleiro
Copy link
Contributor Author

Closed the previous pull requests. Here are the updated requests:

@dblock
Copy link
Member

dblock commented Jun 21, 2024

Closed the previous pull requests.

You can amend and force push btw, less work.

dblock pushed a commit that referenced this issue Jun 21, 2024
Addresses bug in #1042

Signed-off-by: Brendon Faleiro <[email protected]>
Co-authored-by: Brendon Faleiro <[email protected]>
BrendonFaleiro pushed a commit to BrendonFaleiro/opensearch-java that referenced this issue Jun 21, 2024
Addresses bug in opensearch-project#1042

Signed-off-by: Brendon Faleiro <[email protected]>
VachaShah pushed a commit that referenced this issue Jun 21, 2024
Addresses bug in #1042

Signed-off-by: Brendon Faleiro <[email protected]>
Co-authored-by: Brendon Faleiro <[email protected]>
@dblock dblock removed the untriaged label Jun 21, 2024
Xtansia pushed a commit to Xtansia/opensearch-java that referenced this issue Jun 26, 2024
Addresses bug in opensearch-project#1042

Signed-off-by: Brendon Faleiro <[email protected]>
Co-authored-by: Brendon Faleiro <[email protected]>
(cherry picked from commit 69aa51c)
Xtansia added a commit that referenced this issue Jun 27, 2024
* Made InlineGet source field nullable (#1046)

Addresses bug in #1042

Signed-off-by: Brendon Faleiro <[email protected]>
Co-authored-by: Brendon Faleiro <[email protected]>
(cherry picked from commit 69aa51c)

* Fix changelog

Signed-off-by: Thomas Farr <[email protected]>

---------

Signed-off-by: Thomas Farr <[email protected]>
Co-authored-by: Brendon Faleiro <[email protected]>
@Xtansia
Copy link
Collaborator

Xtansia commented Jun 28, 2024

This fix was released in v2.11.1

@Xtansia Xtansia closed this as completed Jun 28, 2024
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