Return empty metadata for key not found + add IT test.

[artursouza]

Description

Return empty metadata for key not found + add IT test.

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: N/A

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

[halspang]

Couple things, some potentially may not be related to this PR:

• Should we change the other State constructors to return an empty map instead of null for metadata? Is there a case for null metadata?
• Based on the build state method, is there a time when we actually assign the metadata? Looking through the http client, it doesn't seem like it's assigned during any gets. Is it saved as part of the value?
• Only the GRPC client has tests for metadata, is this intended?
• Is this a breaking change? Do we think customers rely on the null metadata?

[artursouza]

Couple things, some potentially may not be related to this PR:

• Should we change the other State constructors to return an empty map instead of null for metadata? Is there a case for null metadata?

I thought about that. We would need to see where it is being used and see if it makes sense.

• Based on the build state method, is there a time when we actually assign the metadata? Looking through the http client, it doesn't seem like it's assigned during any gets. Is it saved as part of the value?

We don't have that in http yet, you are right. We would need to see how the API returns metadata in Dapr API.

• Only the GRPC client has tests for metadata, is this intended?

We need to see how Dapr API handles metadata response in HTTP - it might not be supported.

• Is this a breaking change? Do we think customers rely on the null metadata?

Well, it can be depending on the logic the customer has. On the other hand, it is odd that HTTP has a different behavior than GRPC. Since GRPC is the default client, it looks like a bug in the HTTP client.

[artursouza]

HTTP API does not return metadata, it only returns ETag: https://github.com/dapr/dapr/blob/39b076c273126791a72bc952f4a3805841b987fe/pkg/http/responses.go#L45

If HTTP were to return metadata too, it would need to be handled with the "metadata." prefix in the header, similar to how it handles incoming metadata: https://github.com/dapr/dapr/blob/39b076c273126791a72bc952f4a3805841b987fe/pkg/http/api.go#L1356

I suggest we merge this PR because the behavior is aligned to how the runtime is implemented. Also, it makes the HTTP implementation behave like the gRPC implementation for this example and the test confirms both are working the same way.

[codecov]

Codecov Report

Merging #528 (c9a030f) into master (dfd0d5c) will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #528      +/-   ##
============================================
+ Coverage     80.52%   80.65%   +0.13%     
- Complexity      981      982       +1     
============================================
  Files            88       88              
  Lines          3024     3024              
  Branches        331      331              
============================================
+ Hits           2435     2439       +4     
+ Misses          423      420       -3     
+ Partials        166      165       -1     

Impacted Files	Coverage Δ	Complexity Δ
...k/src/main/java/io/dapr/client/DaprClientHttp.java	91.89% <100.00%> (ø)	98.00 <0.00> (ø)
sdk/src/main/java/io/dapr/utils/Retry.java	75.00% <0.00%> (+25.00%)	2.00% <0.00%> (+1.00%)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfd0d5c...c9a030f. Read the comment docs.

[mukundansundar]

I think for the key not found scenario, the empty metadata makes sense ... I think HTTP API in runtime should be enhanced to return metadata also and the implementations in components-contrib need to be enhanced to return any metadata or empty map.

[artursouza]

Issue to follow up in the runtime: dapr/dapr#3172