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

[REQ] Add a Kotlin client Volley library #10011

Open
alisters opened this issue Jul 22, 2021 · 9 comments
Open

[REQ] Add a Kotlin client Volley library #10011

alisters opened this issue Jul 22, 2021 · 9 comments

Comments

@alisters
Copy link
Contributor

alisters commented Jul 22, 2021

Is your feature request related to a problem? Please describe.

We use android and volley, but with Kotlin. Presently there is

  • A volley client library in the android generator, which generates java.
  • A kotlin client generator with various libraries but not Volley.

Searching the issues shows the volley client may need attention - #9293

Additionally we use co-routines, but my understanding (limited!) of the implementation of co-routines in both these libraries is that whilst they add a suspending function, they then block on it's return. I'd like to see suspendCoroutine, adding the callback listeners to the co-routine continuation, and suspending the co-routine until a value has returned (avoiding tying up a thread until the response arrives). This is the documented way of adapting existing callback based api's to co-routines - though the best explanation of it i've found is in this video, at this point - https://youtu.be/YrrUCSi72E8?t=982

Describe the solution you'd like

I'd like to leverage the model generation in the kotlin client generator, but add a library to for volley

Describe alternatives you've considered

Using one of the existing two generators / libraries. See some of the shortcomings outlined above

  • We could have added it as a new library in java to the android client, but java, to the best of my knowledge would have had less functionality around co-routines / or require interop.
  • The existing libraries in the kotlin client have files we don't need - ApiInvoker.kt.mustache, and we needed to add our own (GsonRequest, RequestFactory). We tried adding custom templates to begin with but this was complicated, and didn't seem to fit well with calling volley with co-routines.

Additional context

We've started work on it, and should have a PR ready with basic functionality ready in a week or two. It has some obvious extension points with the ability to pass your own configured requestQueue, or your own factory for mapping from the various http call fields to volley request classes.

@wing328
Copy link
Member

wing328 commented Aug 1, 2021

@alisters thanks for offering contributions on this. We definitely welcome Volley support via the --library option in the Kotlin client generator.

Let us know if you need any help.

@alisters
Copy link
Contributor Author

alisters commented Aug 9, 2021

@wing328 - great. This is progressing, but as always more slowly than hoped ! We are using it in house, and are adding some auth to it. Next update will hopefully be a PR and any/all help and feedback welcomed then.

@alisters
Copy link
Contributor Author

alisters commented Aug 25, 2021

@wing328

  • i've created this pull request - Kotlin client: add volley library support  #10253.
  • i still need to understand the scripts to run to properly prepare the PR, and do additional testing, ...
  • I'm working through some of the auth related functionality, which we don't actually use in my organisation.
  • I added some design notes in the readme but i will copy them below for ease.

@alisters
Copy link
Contributor Author

alisters commented Aug 25, 2021

Design decisions for the Android Kotlin library

A kotlin client for Android using the currently recommended http client, Volley. See https://developer.android.com/training/volley

  • Currently sends GsonRequests - volley requests where the json payload is de/serialized using gson

  • Currently only supports Gson as a serializer

  • Defaults the source location to src/main/java as per standard Android builds

  • Reuse model class and other jvm common infrastructure from the kotlin client

  • Api calls use co-routines, the suspendCoroutine builder function and execute them using volley callbacks to avoid tying up a thread.

  • An api "client" stub is generated by tag for a set of operations as per the base generator

  • A RequestFactory is injected into each client stub - facilitating extension if required, with a default implementation provided.

  • The request factory can be passed per tag (api client) or per operation (an extra parameter is created on operations with non-global security), when per operation auth overriding global security.

  • Since the request factory is injected, appropriate DI container based scoping of the Request Factory and pre-generated auth header factories allow for thread safe and secure setting of credentials.

  • The request factory is passed header factories that optionally late bind or lazily provide header values allow for refreshing tokens etc

  • Factoring of header factories to the Request Factory allow ambient provision of credentials. Code gen library is credential storage agnostic.

  • Header factories allow the merging of generated headers from open api spec with dynamically added headers

  • Injection of http url stack to allow custom http stacks

  • Optional generation of room database models, and transform methods to these from open api models

  • In our own code, the generated room and api models are extended with additional extension properties - while code gen consumers get their clients generated for free the resulting code is still extensible using kotlin. I'll provide some sample code.

  • Additional supporting files ??

Future improvements

  • Option to generate image requests on certain conditionals e.g content-type gif etc
  • Support for kotlin serialization.
  • Form support ?
  • Support for file inputs/uploads
  • More auth support
  • Improvements to the generated gradle build file ? Do we need to generate an aar or just a jar.
  • We build this as a library presently - so more testing of the maven and gradle plugins required. Do we need to support the maven plugin for android as, AFAIK, android builds are almost exclusively gradle (?)

Consumption of the client

Hilt Dependency injection example - when constructing our clients, we inject the appropriate header factories. We don't use auth from the api, but in this sample i also inject a basic auth header factory and bind it's credentials
return SomeApi (context = context, requestQueue = restService.getRequestQueue(), requestFactory = RequestFactory(sessionHeaderFactory, traceHeaderFactory, RequestFactory.basicAuthHeaderFactoryBuilder("username", "password")), basePath = configurationService.getBaseUrl() )

  • TODO: Explain more here - exception handling pattern used when calling coroutines, and the design behind our call back function. We use a wrapper function that calls the api client but catches volley exceptions. This then returns a service result. Explain how the client might be used in a "standard" Android MVVM with repository architected application.

@alisters
Copy link
Contributor Author

I've attempted to add per operation security but can't see an easy way to do it presently.

  • Output the operation Json Model using --global-property debugOperations and don't see a property indicating that there is per operation security, or even generating an entry under the authMethods array.

Have seen that it was done for the python client here - https://github.com/OpenAPITools/openapi-generator/pull/6569/files - but am wondering if this is commonly implemented ?

@alisters
Copy link
Contributor Author

@wing328 We've updated the PR after generating the required samples and docs. Could you indicate

  • If we need to squash these commits or can we do that on merge ?
  • Is there anything else that needs to be done ?
  • When are releases done ? And what is the patching process ?

I was unable to tag the issue as Kotlin client related when i raised it. I'm happy to be tagged on bugs raised, and to monitor the appropriate tags if you can tell me what they will be.

@alisters
Copy link
Contributor Author

alisters commented Aug 30, 2021

@wing328 Some help is required please. I'm unsure why the circle:ci and bitrise builds are failing.

  • Is the --dry-run that appears to be run by the bitrise build failing for our generator /library ? It's hard to tell from the output, or what might be causing the build to fail (the generator names are printed). I've run it locally, and while there are two files in the dry run status map, debugging.md seems to indicate that this is not necessarily a problem ? Exit code of the command is 0 as well. Local output is
  • [main] ERROR o.o.codegen.DefaultGenerator -

Dry Run Results:

k C:\dev...-kotlin-volley.openapi-generator-ignore
Skipped by user option(s) Skipped by supportingFiles options supplied by user.
w C:\dev...-integration-kotlin-volley.openapi-generator\VERSION
Write............... File will be written.

States:

  • w Write

Circle:ci circle_parallel seems to be failing on the typescript client ?

  • apis/PetApi.ts(360,59): error TS2554: Expected 2 arguments, but got 3.

@wing328
Copy link
Member

wing328 commented Aug 30, 2021

Please ignore those CI failure for the time being. I'll take a look at your PR later today.

@alisters
Copy link
Contributor Author

Ok and thank you.

If you're not familiar with volley, the major aspect i didn't really call out in the above comments is that volley doesn't directly communicate with an http client - it adds requests to a queue which then interacts with an http client. So in our code gen setup we replaced an apiclient or api invoker that the other kotlin and java clients had with just the RequestFactory. In turn this changes the way you might interact with the code gen in the course of making a given request - how do you set the username and password for a single operation that has overridden global security ? We could put these calls on the request factory but it didn't feel great. TBD further if you wish to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants