phiz71 / vertx-swagger

Swagger integration in Eclipse Vert.X world. A dynamic Vert.X router, configured with a swagger file and a swagger-codegen plugin to generate a server stub.
Apache License 2.0
86 stars 35 forks source link

fixing #79 #82

Closed phiz71 closed 7 years ago

javadch commented 7 years ago

I am going to test it and then update you.

javadch commented 7 years ago

It generates an implementation class for each xApi! These xApiImpl classes must not be generated because then it is not possible to write code in them! The next build will remove the implementation code!

Also, in the xVerticle classes, the default value for parameters is not taken into account.

javadch commented 7 years ago

When the generator adds a OffsetDateTime variable (mostly in the header classes), it should add a proper import import java.time.OffsetDateTime; to the class, too.

javadch commented 7 years ago

It works fine for Future.succeededFuture(resourceRespone) with all the headers and payload set, however, when I want to report a failure via Future.failedFuture(xApiException.excetopm_x_y), there is no way to set the headers. For example, when authentication fails, I want to not only set the response code to 401 (available via Exceptions) but also I want to set some headers (like WWW_Authenticate). The Exceptions need to also accept headers.

phiz71 commented 7 years ago

Lots of comment :)

It generates an implementation class for each xApi! These xApiImpl classes must not be generated because then it is not possible to write code in them! The next build will remove the implementation code!

A .swagger-codegen-ignore file is generated and it prevents *Impl files from being overwritten. However, I must admit I've never thought about your use-case when I merged this PR #72 . I think I will keep this feature (Impl generation) but I will make it optional ; a parameter will have to be set to activate it. And by default, Impl will not be generated.

Also, in the xVerticle classes, the default value for parameters is not taken into account.

The default value is managed in the AbstractSerializableParameterExtractor class, in the router. If a non-required parameter is not in the request and if it has a default value in the spec, then the router will work as if the default value was in the request :

if (!params.contains(name)) {
    if (abstractSerializableParameter.getRequired()) {
        throw new IllegalArgumentException("Missing required parameter: " + name);
    } else if (abstractSerializableParameter.getDefaultValue()!=null){
        return abstractSerializableParameter.getDefaultValue();
    } else {
        return null;
    }
}

When the generator adds a OffsetDateTime variable (mostly in the header classes), it should add a proper import import java.time.OffsetDateTime; to the class, too.

I've faced this problem too. I didn't find the right way to do this. Work in progress... 😅

It works fine for Future.succeededFuture(resourceRespone) with all the headers and payload set, however, when I want to report a failure via Future.failedFuture(xApiException.excetopm_x_y), there is no way to set the headers. For example, when authentication fails, I want to not only set the response code to 401 (available via Exceptions) but also I want to set some headers (like WWW_Authenticate). The Exceptions need to also accept headers.

Since Future.failedFuture can only use a Throwable, I think I will add a MultiMap in MainApiException to manage headers. However for the moment, all available headers will be generated in the xAPIHeader.

phiz71 commented 7 years ago

I think it's ok now :)

javadch commented 7 years ago

It is OK. Well done!

javadch commented 7 years ago

Just as a small cleanup, the code-gen puts the MainApiHeader and MainApiException java classes in the util folder, while they should be one level up, beside MainApiVerticle. the util folder currently should only contain ResourceResponse and SwaggerManager classes.