mmazi / rescu

A lightweight json Rest client utility for JAX-RS
MIT License
65 stars 60 forks source link

swagger jaxrs example #112

Open jnorthrup opened 6 years ago

jnorthrup commented 6 years ago

swagger-codegen has numerous java, rx, and jaxrs incarnations.

i have done some xchange modules with swagger-codegen and had success generating from yaml to java code

i would very much love to see a swagger-codegen jaxrs sample, and/or contribute same to expand the reach of code generation from swagger apis. So far, Xchange has had very narrow view of "improvement", so if it comes from the Master, all the better.

mmazi commented 6 years ago

Can you be more concrete? What exactly is your suggestion/question?

mmazi commented 6 years ago

Can you provide some links, examples, etc.?

jnorthrup commented 6 years ago

this is very forward looking...

I'm maintaining a seperate XChange fork for the moment, though i'd rather smooth swagger into rescu for now

https://github.com/jnorthrup/XChange/tree/jaxrs

jim@XChange$ cd ../Xpolyglot/ 
jim@Xpolyglot$ mkdir target
jim@Xpolyglot$ cd target; wget  https://oss.sonatype.org/content/repositories/snapshots/io/swagger/swagger-codegen-cli/3.0.0-SNAPSHOT/swagger-codegen-cli-3.0.0-20180328.154333-52.jar

2018-03-28 23:51:29 (741 KB/s) - ‘swagger-codegen-cli-3.0.0-20180328.154333-52.jar’ saved [14953421/14953421]

jim@target$ java -jar swagger-codegen-cli-3.0.0-20180328.154333-52.jar  langs
Available languages: [dynamic-html, html, html2, java, jaxrs-cxf-client, jaxrs-cxf, inflector, jaxrs-cxf-cdi, jaxrs-spec, jaxrs-jersey, jaxrs-resteasy-eap, jaxrs-resteasy]

config-help:

https://gist.github.com/jnorthrup/b974ea8ba905bcbe9810c713e59b7a02

jaxrs-spec generation https://gist.github.com/jnorthrup/80b0ede79a12ba28d27a00c94432181c

https://github.com/jnorthrup/XChange/blob/jaxrs/xchange-idexjaxrs/src/main/gen/java/org/knowm/xchange/idexjaxrs/service/ReturnTickerApi.java

shows a given generated api call -- idex in particular has no header or cookie signatures to contend with but other swagger specs do which i have to date implemented as an okhttp filterchain

mmazi commented 6 years ago

I'm still at a loss about what your proposal/question/goal is. Is it about creating JAX-RS-annotated Java interfaces from OpenAPI (e.g. if an OpenAPI specification of the API is provided by an exchange)? If so, how does this relate to rescu?

Again, please take a minute to explain what your goal is and what your proposal to achieve this is, and how this relates to rescu.

jnorthrup commented 6 years ago

i would very much love to see a swagger-codegen jaxrs sample adaptation by the guy who knows all the compromises and tradeoffs of the rescu library. What would take you 20 minutes to evaluate would take me a week

jnorthrup commented 6 years ago

@mmazi the first obstacle i've run across creating proxies for swagger generated jaxrs API's is an exception throw because the API declares "throws Exception"

am i missing an option that lets me generate proxies without restrictive choice of API Exceptions?

I prefer not to check code into git that requires generation then cut and paste or search and replace, but I don't see an alternative wihtout changing the final classes in rescu, on first look

mmazi commented 6 years ago

Rescu does not currently support checked exception on the interface methods (except IOExceptions). I remember making that decision years ago, but I don't remember the reasons, nor did I find any in related issues and commits, nor was I able to deduce them from a quick look at the code. (Sorry, I don't develop rescu actively any more, just maintain it).

Perhaps swagger has some config options to let you configure what exceptions it declares?

If you can push your code somewhere I might have a look.

jnorthrup commented 6 years ago
java.lang.IllegalArgumentException: Only IOExceptions and RuntimeExceptions are supported on API methods; this method doesn't comply: public abstract org.knowm.xchange.idexjaxrs.dto.ReturnTickerResponse org.knowm.xchange.idexjaxrs.service.ReturnTickerApi.ticker(org.knowm.xchange.idexjaxrs.dto.Market) throws java.lang.Exception
    at si.mazi.rescu.RestMethodMetadata.create(RestMethodMetadata.java:99)
    at si.mazi.rescu.RestInvocationHandler.getMetadata(RestInvocationHandler.java:181)
    at si.mazi.rescu.RestInvocationHandler.invoke(RestInvocationHandler.java:106)
    at com.sun.proxy.$Proxy8.ticker(Unknown Source)
    at org.knowm.xchange.idex.IdexExchange$2.getTicker(IdexExchange.java:62)
    at org.knowm.xchange.idex.IdexExchange.main(IdexExchange.java:85)

https://gist.github.com/jnorthrup/6529c1fe2d1621ae649f7d778e099a90 happens from https://github.com/jnorthrup/XChange/blob/e869e15ab6536fee58a52f428e20bbc1f0bc610c/xchange-idexjaxrs/src/main/java/org/knowm/xchange/idex/IdexExchange.java#L62

you can check out the branch and see the jars generator in action and run the main() method to duplicate with men install in the xchange-idexjaxrs module

jnorthrup commented 6 years ago

i would be glad to help maintain this. the basic intent would be to make enough things public or remove such exceptions until the problem goes away

jnorthrup commented 6 years ago

when i alter https://github.com/jnorthrup/rescu/blob/c769c6189e00fc8744ce105f1a021d42ea1aea78/src/main/java/si/mazi/rescu/RestMethodMetadata.java#L97

to test for

for (Class thrownException : thrownExceptions) {
            if (!Exception.class.isAssignableFrom(thrownException)) {
                if (!Exception.class.isAssignableFrom(thrownException)) {
                    throw new IllegalArgumentException("Only IOExceptions and RuntimeExceptions are supported on API methods; this method doesn't comply: " + method);
                }

i get the following opaque test failures... at least im not sure what I'm looking at yet.

image

what can be done as a workaround is to make the static methods less static, and make the methods less protected, until the factory methods can be duplicated and bypass the unit test coverage in the code that needs to avoid this check. im not aware of the factors that define this specific pair of exceptions being checked for, or why the tests fail if changed.

mmazi commented 6 years ago

I've created a quick patch and it seems throws Exception can be supported in rescu: #114.

Please try it and report if this helps.

jnorthrup commented 6 years ago

thank you this helps.

im not able to see the REST calls using logback.xml in the src/main/java

image

is there a programmatic feature to do same?

I am this far -- the call succeeds but returns a null i think

https://github.com/jnorthrup/XChange/blob/64149de07ff05070215366aa5dafc44de72d4bec/xchange-idexjaxrs/src/main/java/org/knowm/xchange/idex/IdexExchange.java#L99

mmazi commented 6 years ago

Move logback.xml to src/main/resources. Better still, move your main method to the xchange-examples module; logback is already configured there, and it will be more consistent.

Also, please continue communication regarding XChange in the appropriate repository (probably your XChange fork).

jnorthrup commented 6 years ago

my preferences run along the lines of https://12factor.net/logs personally. thus the question, "is there a programmatic means?"

src/main/resources doesn't change the outcome.

i just need one-time debugging im not big on editting three modules (parent, immediate, and examples modules) and a second github issue to test the first call of the first method of one.

mmazi commented 6 years ago

is there a programmatic feature to do same?

None that I know of.

In which branch in your repo can I find this code? Not in develop.

jnorthrup commented 6 years ago

so far the method appears to be working but the logging/debug traces dont happen when i uncommeent same from logback.xml

https://gist.github.com/jnorthrup/df64296c5f8a5a92ae7c57a9565231d1

this is the branch here https://github.com/jnorthrup/XChange/tree/develop-jaxrs

jnorthrup commented 6 years ago

btw, i was able to do roundtrip code generation from the rescu jaxrs in a majority of modules, https://github.com/jnorthrup/XChange/tree/develop-jaxrs/swagger-repo

the metadata extraction scripts are checked in to the same branch under etc as well. this could make for an interesting node.js or golang structure from the jaxrs metadata that is in use in java.

mmazi commented 6 years ago

src/main/resources doesn't change the outcome.

Perhaps you must mark the src/main/resources as resources root in IntelliJ. Or perhaps try a mvn package before running the main method (I assume you're running it from Idea). Check if the logback.xml file is copied by your build system (either mvn or Idea, whichever you're using) to the appropriate directory (probably should end up in target/classes).

jnorthrup commented 6 years ago

i have been running mvn install in the xchange-examples

my interest is in seeing the return json, since this incarnation does not yet define the error attribute returned in every dto response yet

jnorthrup commented 6 years ago

got the poms all lined up.

now im not sure why this POST method is sending an empty requestBody as shown below. "unannotatedmethodparameters is empty."

the two declarations, are shown as well.

image

jnorthrup commented 6 years ago

i appears that @Valid is preventing the parmeter from being created in the body. when i remove that, it creates a body

mmazi commented 6 years ago

@jnorthrup Please give me a hand here:

  1. Fork rescu
  2. Create a test in RestInvocationHandlerTest that demonstrates the @Valid problem:
    • Add a new method in ExampleService with a single @Valid parameter; if @Valid is not available, use an arbitrary non-JAX-RS-Param annotation.
  3. You can try fixing the problem yourself: it seems the problem is somewhere in lines 92-104 in RestInvocation. If you can't do this, just crate the test and I'll fix the code, but you might have to wait a bit for me to find the time.
  4. Create a pull request.
mmazi commented 6 years ago

So, to clarify: this is a bug in rescu, it should treat the parameter that is annotated only with @Valid (or any non-JAX-RS-parameter annotations) as the "unannotated" parameter (there can be only one such parameter).

jnorthrup commented 6 years ago

https://github.com/jnorthrup/XChange/blob/polyglot/xchange-idexjaxrs/pom.xml#L128 was my in-situ fix for now. since it's up to rescu to perform java validation, i figured it was a no-op

jnorthrup commented 6 years ago

i have done round-trip swagger extraction on the jaxrs for at least half of XChange modules which can then produce java service api interfaces against language "jaxrs-spec" with "interfacesOnly" config (the pom i showed you above is a reasonable maven generation workflow).

the pros are... jaxrs is a good and robust metadata format. the cons are... swagger generation for jaxrs-spec interfaces does not obey the "tags" hierarchy and only follows the paths hierarchy.

the ones that succeeded extraction are at https://github.com/jnorthrup/XChange/tree/swagger-extraction/swagger-repo and the script is in ../etc/ to recreate