[FEATURE] Add builder.copy for external retry logic

[injae-kim]

Is your feature request related to a problem?

index = ..;
targetId = ..;
Bool.QueryBuilder queryBuilder = builder(index, targetId, ..);

return search(queryBuilder);

--

public doc search(queryBuilder) { 
  RxJava.retry(retry -> {
    opensearchClient.search(queryBuilder, ..); // ⭐ on retry, re-use same query builder
  });
}

opensearch-java/java-client/src/main/java/org/opensearch/client/util/ObjectBuilderBase.java

Lines 46 to 49 in dbd93c1

	protected void _checkSingleUse() {
	if (this._used) {
	throw new IllegalStateException("Object builders can only be used once");
	}

java.lang.IllegalStateException: Object builders can only be used once

Users can use external retry logic with query builder like above.
But we can't re-use the same query builder on open search so IllegalStateException exception thrown.

What solution would you like?

Bool.QueryBuilder queryBuilder = builder(index, targetId, ..);
return search(queryBuilder);

--

public doc search(queryBuilder) { 
  RxJava.retry(retry -> {
    opensearchClient.search(queryBuilder.copy(), ..); // ⭐ add builder.copy()
  });
}

If we can use queryBudiler.copy(), it's really useful for several cases!

[injae-kim]

I think builder.copy() is usually supported on many other db-client library so it's okay to add it on openserach java client too. and I think many opensearch user need this feature.

I'm contributor on other opensource so I can make PR so PTAL 🙇

[Xtansia]

Thanks for bringing this up. It should be relatively easy to implement on the generated code. Some things to consider:

• Would you expect this to be a surface-level copy or a clone/deep-copy? Lists & Maps would need to be properly copied as we append to them in builders. But what do we do with nested objects.
• Which use-cases require the builder implementing copy versus just re-using the built object from the builder?

[injae-kim]

Would you expect this to be a surface-level copy or a clone/deep-copy?

opensearch-java/java-client/src/main/java/org/opensearch/client/util/ObjectBuilderBase.java

Lines 46 to 49 in dbd93c1

	protected void _checkSingleUse() {
	if (this._used) {
	throw new IllegalStateException("Object builders can only be used once");
	}

For now I simply want to pass above _checkSingleUse(), so surface-level copy + set used field as false is what I expect on builder.copy()!

Which use-cases require the builder implementing copy versus just re-using the built object from the builder?

index = ..;
targetId = ..;
Bool.QueryBuilder queryBuilder = builder(index, targetId, ..);

return search(queryBuilder);

--
// what I want
public doc search(queryBuilder) { 
  RxJava.retry(retry -> {
    opensearchClient.search(queryBuilder, ..); // ⭐ on retry, re-use same query builder
  });
}

--
// but now
public doc search(paramA, paramB, paramC, ..) {
newBuilder = builder.of(paramA, paramB, ..); // to build new builder, we need to pass all parameters!

  RxJava.retry(retry -> {
    opensearchClient.search(newBuilder, ..);
  });
}

To retry like above case, we need to pass all params to create builder on each retry 😢

or, I think we can easily add builder.setUsed(false) method to solve this situation.

@Xtansia wait your opinion~! thanks! 🙇

[Xtansia]

@injae-kim My question is, client.search() accepts a SearchRequest not a SearchRequest.Builder, which is the result of calling build(), so are you not able to just re-use the SearchRequest instance after calling build once?

[injae-kim]

public doSearch(BoolQuery.Builder queryBuilder) {
  SearchRequset.Builder requestBuilder = new SearchRequest.builder().index(..).size(..)..;

  RxJava.retry(() -> {
    requestBuilder.query(q -> q.bool(b -> queryBuilder)); // here! re-use same queryBuilder on retry
  });
}

@Xtansia , aha~ I want to re-use BoolQuery.Builder, not SearchRequest.Builder.
We pass BoolQuery.Builder as param like above code and re-use it on retry.

[Xtansia]

@injae-kim Well the same question applies, do you need to pass around BoolQuery.Builder, is there a reason you're not just passing around a BoolQuery instance?

Are you varying the search request on each retry? Otherwise it should be built once outside the retry and just call client.search(req) inside

[injae-kim]

is there a reason you're not just passing around a BoolQuery instance?

searchAPI(params..) {
  BoolQuery.Builder builder = newBuilder(params..);
  search(builder);
}

search(builder) {
  if (some condition) {
    builder.filter(..); // add filter()
  }

  doSearch(builder);
}

doSearch(builder) {
  if (some condition) {
    builder.should(..); // add should()
  }

  RxJava.retry(() -> {
    SearchRequest req = new Request(builder); // reuse same builder on retry
    return client.search(req)
  });
}

@Xtansia yes. on our service, we add some more filtering on internal layer like above.
So we prefer to pass BoolQuery.Builder for flexibility, not passing BoolQuery directly!

I think it's common usage for other user too :)