fix set_param

[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.

[tanmaykm]

Could you give some more context, regarding what is the issue you faced?

[jonalm]

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.

[jonalm]

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).

[codecov-commenter]

Codecov Report

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 ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
- Coverage   81.74%   80.63%   -1.11%     
==========================================
  Files           7        6       -1     
  Lines         619      594      -25     
==========================================
- Hits          506      479      -27     
- Misses        113      115       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tanmaykm]

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/

[jonalm]

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.

[tanmaykm]

Yes, thanks for pointing out. I would like to leave a little comment in the code there to remind ourselves of what was done.

[tanmaykm]

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.

[jonalm]

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?

[tanmaykm]

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.

[tanmaykm]

Version v0.1.22 is now registered (JuliaRegistries/General#102908).
Thanks for the PR @jonalm !