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

Initial invokeBindingList implementation #1068

Closed
wants to merge 5 commits into from

Conversation

salaboy
Copy link
Contributor

@salaboy salaboy commented Jul 8, 2024

Description

Add support for InvokeBindingList to enable binding returning a collection of typed objects.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1067

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@salaboy salaboy requested review from a team as code owners July 8, 2024 08:11
@salaboy salaboy force-pushed the invokeBindingList branch from 5f783aa to 338f286 Compare July 8, 2024 08:11
@salaboy salaboy force-pushed the invokeBindingList branch 3 times, most recently from 2d2bd1b to 33dc23f Compare July 8, 2024 09:04
@salaboy
Copy link
Contributor Author

salaboy commented Jul 8, 2024

After some weekend fun.. all test seems to be working :)

@salaboy
Copy link
Contributor Author

salaboy commented Jul 8, 2024

@artursouza @cicoyle please review :)

@salaboy
Copy link
Contributor Author

salaboy commented Jul 8, 2024

Interesting.. when testing this code with the Spring Boot integration I found that the PostgreSQL binding for some reason doesn't return an array of JSON objects.. but an array of arrays $). I am totally confused by that now:

{
  "metadata": {
    "operation": "query",
    "duration": "432µs",
    "start-time": "2020-09-24T11:13:46.405097Z",
    "end-time": "2020-09-24T11:13:46.420566Z",
    "sql": "SELECT * FROM foo WHERE id < $1"
  },
  "data": "[
    [0,\"test-0\",\"2020-09-24T04:13:46Z\"],
    [1,\"test-1\",\"2020-09-24T04:13:46Z\"],
    [2,\"test-2\",\"2020-09-24T04:13:46Z\"]
  ]"
}

This PR works based on the assumption that the return will be an array of objects serialized in JSON, but that is not the behavior of the postgresql.binding

@salaboy salaboy force-pushed the invokeBindingList branch from eb2a94b to 9b838d8 Compare July 9, 2024 10:03
@salaboy
Copy link
Contributor Author

salaboy commented Jul 9, 2024

Ok, I've updated this PR to include a test where I show how invokeBindingList saves the user to deserialize objects. Unfortunately, due the nature of output bindings, this works with PostgreSQL for now. If we are planning to standardize a bit more the "query" command for output bindings, we can make it work with other providers.

@salaboy
Copy link
Contributor Author

salaboy commented Jul 9, 2024

@artursouza @cicoyle please review :)

@artur-ciocanu
Copy link
Contributor

artur-ciocanu commented Jul 9, 2024

@salaboy I am wondering if invokeBindingList(...) is really required. I have looked at the existing DaprClient interface and we have the following method and its friends:

/**
   * Invokes a Binding operation.
   *
   * @param bindingName      The name of the biding to call.
   * @param operation The operation to be performed by the binding request processor.
   * @param data      The data to be processed, use byte[] to skip serialization.
   * @param type      The type being returned.
   * @param <T>       The type of the return
   * @return a Mono plan of type T.
   */
  <T> Mono<T> invokeBinding(String bindingName, String operation, Object data, TypeRef<T> type);

In order to deserialize into a List<T>, other Java collection or any other generic container you would use something like this:

TypeRef<List<String>> typeRef = new TypeRef<>() {};
Mono<List<String>> result = daprClient.invokeBinding("some-binding", "some-operation", null, typeRef);    

Please give it a try and let me know if it works for your particular use case. As far as I can tell TypeRef took some inspiration from: https://github.com/FasterXML/jackson-core/blob/2.18/src/main/java/com/fasterxml/jackson/core/type/TypeReference.java, which means that articles like: https://www.baeldung.com/java-deserialize-generic-type-with-jackson are somewhat applicable to Dapr TypeRef as well.

@salaboy
Copy link
Contributor Author

salaboy commented Jul 9, 2024

@artur-ciocanu thanks for checking this, I was trying to answer that same question, but I came to the conclusion that we need this.

Mostly because if not you need to then deserialize the content of the objects manually like it is being shown in this test:

public void invokeBindingResponseListObjectTypeRefTest() . The idea here is to simplify the users life by providing a helper method that will expect a list. This is specifically for "query" commands like the one provided by the PostgreSQL Binding that has a "row" style return type.

@salaboy
Copy link
Contributor Author

salaboy commented Jul 19, 2024

I will close this PR, because this is not needed right now. We can get back to this if we find enough use cases to require this.

@salaboy salaboy closed this Jul 19, 2024
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

Successfully merging this pull request may close these issues.

Add support for invokeBindings that return a list of typed objects
2 participants