Closed jonalm closed 7 months ago
Could you give some more context, regarding what is the issue you faced?
Hi @tanmaykm , sorry for the delayed response. Here comes some more context.
The current implementation of set_param
fails when value
is of type Dict
. It hits string
directly. string(Dict("a"=>3))
results in Dict(\"a\" => 3)
. This string representation is further propagated to the query string.
I do not know if there is an official way to handle Dict
like objects by the OpenAPI spec, but the code generated here (https://github.com/jonalm/BenchlingRestApi.jl) seems to indicate that the proper way to handle the dict params in the query string is to set a paramete per item in the dict and to prepend the key value by the $(name).
value in the set_param
function. At least that is needed for the behavior of this function call https://github.com/jonalm/BenchlingRestApi.jl/blob/0294435467c1d57124d2d85956d383839fc3dd21/codegen/src/apis/api_CustomEntitiesApi.jl#L229C1-L229C99 to be consistent with the associated rest API.
For the particular function linked above, and this code example:
schema_id = "ts_GiD1yWEx"
schema_fields = Dict("from_entity"=> "bfi_mysPWbDJ", "type"=> "sfso_l5yLxYkM")
returning="customEntities.fields.to_entity.value, customEntities.fields.type.value"
api_return, http_resp = list_custom_entities(client; schema_id, schema_fields, returning)
The resulting query string is given by the code before this pr as (see the Dict
being interpolated into the string):
v2/custom-entities?schemaId=ts_GiD1yWEx&returning=customEntities.fields.to_entity.value%2C%20customEntities.fields.type.value&schemaField=Dict%28%22from_entity%22%20%3D%3E%20%22bfi_mysPWbDJ%22%2C%20%22type%22%20%3D%3E%20%22sfso_l5yLxYkM%22%29
which gives 400 Bad Request Error.
Whereas, with the changes proposed here, the resulting query string is:
api/v2/custom-entities?schemaId=ts_GiD1yWEx&schemaField.type=sfso_l5yLxYkM&returning=customEntities.fields.to_entity.value%2C%20customEntities.fields.type.value&schemaField.from_entity=bfi_mysPWbDJ
which works.
On second thought I am a bit unsure about the '$(name).' prefix I think it is specific to my case and not general (i.e. that prefix should be put explicitly into the input dict).
Attention: Patch coverage is 50.00000%
with 2 lines
in your changes are missing coverage. Please review.
Project coverage is 80.63%. Comparing base (
1916168
) to head (848c589
).
Files | Patch % | Lines |
---|---|---|
src/client.jl | 50.00% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I think what you describe fits into the deepObject
seriazation format? Then yes, OpenAPI.jl does not support that yet, it's in the TODO list.
The specification mentions this format though to serialize deepObjects
: https://swagger.io/docs/specification/serialization/
Dear @tanmaykm , I don't think that is right. The spec I used in my example does not specify style (https://github.com/jonalm/BenchlingRestApi.jl/blob/0294435467c1d57124d2d85956d383839fc3dd21/openapi.yaml#L26129) and should default to style=form
(not deepObject) according to the link you provided.
AFAIU the query parameters in an object (i.e. Dict in the julia generated code) should expand into ampersand (or comma) separated "key=value" elements (i.e. as the rightmost column of the first row in the table example under "Query Parameters" in the link you provided https://swagger.io/docs/specification/serialization/). This is what this PR fixes.
Yes, thanks for pointing out. I would like to leave a little comment in the code there to remind ourselves of what was done.
Also, do you intend to also add support for parsing back the same on the server side in this PR? Or add support for more formats? Happy to merge it as it is though, if you need only this much for your specific case.
Thanks @tanmaykm, it would be great if you could merge this minimal PR, even though it only contains a minor improvement. I figured this out by reverse engineering, and I have no prior expericence with OpenAPI so I wouldn't know where to start on the server side implementation.
I would be happy to contribute to more formats at a later stage, but I would need to grok the test setup first. How easy is it to create a minimal OpenAPI spec, simulate a server and test the generated code?
Sure! The CI I think was failing because of an unrelated issue, which I've fixed in master. Let's merge this one and also tag a release soon after.
There are some example specs and their client and server implementations in the test code, maybe those can serve as examples for writing new ones. Please feel free to ping me whenever you are planning to contribute next time.
Version v0.1.22 is now registered (https://github.com/JuliaRegistries/General/pull/102908). Thanks for the PR @jonalm !
I do not know how this is indended to work, so I don't know if this fix is general or only works in my case.