FroMage / redpipe

Redpipe Web Framework
Apache License 2.0
70 stars 10 forks source link

Swagger handling of `Single`, `Observable` etc. #36

Closed aesteve closed 6 years ago

aesteve commented 6 years ago

Use case :

@Path("/user")
public class UserController {

    public static class Person {
        private String name;

        public Person(String name) {
            this.name = name;
        }

        public String getName() {
            return name;
        }
    }

    @GET
    @Path("/me")
    public Single<Person> myInfos() {
        return Single.just(new Person("toto"));
    }
}

This is going to create a SinglePerson model in Swagger.

If I'm just using Person and not Single<Person>, then a (good) Person model is generated in Swagger.

I can contribute if you want, just give me some pointers in order to know where the Swagger generation happens.

Thanks.

FroMage commented 6 years ago

Swagger setup is done purely in Server.setupSwagger(), but that means that it's really in Swagger that we'll need to hack or do a PR. It's done by the io.swagger:swagger-jaxrs project.

ATM it's done in https://github.com/swagger-api/swagger-core/blob/v1.5.16/modules/swagger-jaxrs/src/main/java/io/swagger/jaxrs/Reader.java but there's a 2.0 branch where it's done in https://github.com/swagger-api/swagger-core/blob/master/modules/swagger-jaxrs2/src/main/java/io/swagger/v3/jaxrs2/Reader.java

For the 1.5 branch (the one we're currently using), it appears to be done in https://github.com/swagger-api/swagger-core/blob/v1.5.16/modules/swagger-jaxrs/src/main/java/io/swagger/jaxrs/Reader.java#L805 and more specifically in https://github.com/swagger-api/swagger-core/blob/v1.5.16/modules/swagger-jaxrs/src/main/java/io/swagger/jaxrs/Reader.java#L893

If we can find a way to hook into there, then great. If not, we'll probably have to send them a PR. I suppose they're not yet in line with the JAX-RS 2.1 addition of CompletionStage, and therefore can't be in line with the latest RESTEasy additions of support for RxJava.

I wouldn't mind switching to 2.0 if you figure out how to upgrade and it works ;)

FroMage commented 6 years ago

Perhaps with SwaggerExtension?

aesteve commented 6 years ago

OK, not sure what SwaggerExtension looks like, but I'll search, no problem.

So, no testing for Swagger in Redpipe's souce code, since it's delegated directly. Am I right ?

(I might add some, in order to check that a 2.0 migration won't break anything)

FroMage commented 6 years ago

Apparently, there's a plug-in mechanism, if we just register our extension: https://github.com/swagger-api/swagger-core/tree/v1.5.16/modules/swagger-jaxrs/src/main/java/io/swagger/jaxrs/ext and then we can surely adapt the return types there. Looks easy :)

No, no testing ATM, but I'd welcome adding tests, of course. Especially if you're a Swagger user, because I'm not very good at it, so you'll know what to test better than I will.

aesteve commented 6 years ago

I'm a Swagger (UI) "user" yeah, but I don't know any stuff about the Swagger internals. Time to dive into it I guess.

Thanks for the pointers, I'm giving a look at how the tests work atm.

FroMage commented 6 years ago

I think the swagger tests should test the generated yaml file. As for the UI, frankly I think it should either be provided standard on redpipe, or it should be a redpipe plugin. Perhaps we can move the base swagger support to a new swagger plugin which would have the UI in there too, but I think there's value in having the UI by default in there, without requiring any plugin.

aesteve commented 6 years ago

For further ref.


redpipe/redpipe-engine/src/main/java/net/redpipe/engine/core/MyBeanConfig.java
Error:(16, 18) java: package io.swagger does not exist
Error:(17, 30) java: package io.swagger.annotations does not exist
Error:(18, 31) java: package io.swagger.jaxrs.config does not exist
Error:(25, 35) java: cannot find symbol
  symbol: class BeanConfig
Error:(26, 9) java: method does not override or implement a method from a supertype
Error:(33, 34) java: cannot find symbol
  symbol:   method getResourcePackage()
  location: class net.redpipe.engine.core.MyBeanConfig
Error:(65, 78) java: cannot find symbol
  symbol:   class SwaggerDefinition
  location: class net.redpipe.engine.core.MyBeanConfig
Error:(73, 63) java: cannot find symbol
  symbol:   class Api
  location: class net.redpipe.engine.core.MyBeanConfig
----------
redpipe/redpipe-engine/src/main/java/net/redpipe/engine/core/Server.java
Error:(26, 31) java: package io.swagger.jaxrs.config does not exist
Error:(27, 31) java: package io.swagger.jaxrs.config does not exist
Error:(28, 32) java: package io.swagger.jaxrs.listing does not exist
Error:(29, 32) java: package io.swagger.jaxrs.listing does not exist
Error:(414, 17) java: cannot find symbol
  symbol:   variable ReaderConfigUtils
  location: class net.redpipe.engine.core.Server
Error:(416, 17) java: cannot find symbol
  symbol:   class BeanConfig
  location: class net.redpipe.engine.core.Server
Error:(429, 65) java: cannot find symbol
  symbol:   class ApiListingResource
  location: class net.redpipe.engine.core.Server
Error:(430, 58) java: cannot find symbol
  symbol:   class SwaggerSerializers
  location: class net.redpipe.engine.core.Server

``` 
FroMage commented 6 years ago

WDYM? It doesn't compile from the command-line? Or is this an IDE issue you're having?

aesteve commented 6 years ago

No don't worry, it was just "the amount of stuff to change" to switch to swagger-jaxrs v2.

Decent I guess :) but it requires to look at migration guide for 1.5 to 2.x, but they seem to be in the process of writing it.

FroMage commented 6 years ago

Ah OK. You had me worried :)

FroMage commented 6 years ago

Unfortunately, it appears it will be harder to support the new RESTEasy @PathParam annotation that doesn't have a value parameter, because the swagger-jaxrs methods that process these parameters don't hang on to the reflection info that has the method names…