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

Response code with headers are not supported #79

Closed javadch closed 7 years ago

javadch commented 7 years ago

Assuming I have the following resource/path:

        "get": {
          "tags": [
            "Users"
          ],
          "summary": "Logs user into the system",
          "description": "",
          "operationId": "loginUser",
          "parameters": [
            {
              "name": "username",
              "in": "query",
              "description": "The user name for login",
              "required": true,
              "type": "string"
            },
            {
              "name": "password",
              "in": "query",
              "description": "The password for login in clear text",
              "required": true,
              "type": "string"
            }
          ],
          "responses": {
            "200": {
              "description": "successful operation",
              "schema": {
                "type": "string"
              },
              "headers": {
                "X-Rate-Limit": {
                  "type": "integer",
                  "format": "int32",
                  "description": "calls per hour allowed by the user"
                },
                "X-Expires-After": {
                  "type": "string",
                  "format": "date-time",
                  "description": "date in UTC when token expires"
                }
              }
            },
            "400": {
              "description": "Invalid username/password supplied"
            }
          }
        }
      }

On successful authentication, I want a way to access at set X-Rate-Limit and X-Expires-After response headers or there should be a mechanism to automatically set them. The generated code relies on the XXXApiImpl service class by passing a handler object to its methods; here loginUser method. But there is nothing on the handler that I can set response headers. Also, on the failure, the only option is to perform Future.failedfuture() and pass a throwable! So, the question is how to access and set response headers from inside service classes? Or, what would be the best practice to do that?

Also, in the snippet, the router does not provide a way to set the response header, nor does it automatically!

            "401": {
              "description" : "Authentication information is missing or invalid",
              "headers": {
                "WWW_Authenticate": {
                    "type": "string"
                }
              }        
            }                  
javadch commented 7 years ago

I am thinking of having a mechanism similar operationId for responseId. The code-gen generates proper classes for responses (similar to the exceptions that are generated now) and assigns an ID to each response. For instance, in the example above, the two responseIds would be get_userLogin_200 and get_userLogin_400. Having this Ids, I would be able to pass them to the handler.handle from inside the methods of the ApiImpl classes. They can be passed with either Future.failedFuture or Future.succeededFuture. All the information needed to completely produce each response, including message, headers, etc. are encapsulated in the response class. Then the router not only sends the actual business data to the client, it also sends the headers, etc.

javadch commented 7 years ago

I would suggest that the vertx-swagger/modules/vertx-swagger-codegen/src/main/resources/javaVertXServer/AsyncMethod.mustache template provides a way to send a response/ a response id, or a set of headers back. So that they are added to the response header in the vertx-swagger/modules/vertx-swagger-codegen/src/main/resources/javaVertXServer/AsyncCall.mustache template. A similar technique is currenty in use in the Async.mustache template to set the Content-Type header. See Lines 4: DeliveryOptions deliveryOptions = new DeliveryOptions().addHeader("Content-Type", "application/json")

phiz71 commented 7 years ago

I think I've found a way to fix this issue. I will create a new class like this :

public class ResourceResponse<T> {
    private MultiMap headers;
    private T response;
    ....
}

Instead of generating a Handler<AsyncResult<T>> with T as the response type, the codegen will create a Handler<AsyncResult<ResourceResponse<T>>>.

As a consequence, APIImpl would be able to return headers and APIVerticle would be able to send these headers in the DeliveryOptions

What do you think of this proposition ?

javadch commented 7 years ago

I like it. Its a clear solution. However, what about generating the response headers like ApiExceptions?

On Sep 30, 2017 11:10 PM, "Florent Chamfroy" notifications@github.com wrote:

I think I've found a way to fix this issue. I will create a new class like this :

public class ResourceResponse { private MultiMap headers; private T response; .... }

Instead of generating a Handler<AsyncResult> with T as the response type, the codegen will create a Handler<AsyncResult<ResourceResponse>>.

As a consequence, APIImpl would be able to return headers and APIVerticle would be able to send these headers in the DeliveryOptions

What do you think of this proposition ?

β€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/phiz71/vertx-swagger/issues/79#issuecomment-333336149, or mute the thread https://github.com/notifications/unsubscribe-auth/ABeCLBQSvjMWBsKaq-XXSwfwOT5VVC35ks5snq5NgaJpZM4Pn3KT .

phiz71 commented 7 years ago

I'm working on it. I'm preparing a PR : it will be clearer than comments πŸ˜„ .

javadch commented 7 years ago

Thanks. Waiting for itπŸ˜‰

On Oct 1, 2017 12:04 AM, "Florent Chamfroy" notifications@github.com wrote:

I'm working on it. I'm preparing a PR : it will be clearer than comments πŸ˜„ .

β€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/phiz71/vertx-swagger/issues/79#issuecomment-333338945, or mute the thread https://github.com/notifications/unsubscribe-auth/ABeCLP-bvgQWnrpW8T-w4naSzAA5CP98ks5snrrSgaJpZM4Pn3KT .

javadch commented 7 years ago

What about a solution like this (not tested!):

public final class LoansApiResourceResponse extends ResourceResponse<Loan> {  
    public LoansApiResourceResponse(int code, Header... headers) {
        super(code, headers);
    }

    public static final LoansApiResourceResponse LoansApi_Get_getLoan_200 = new LoansApiResourceResponse(200, new Header("Header1", "String", ""), new Header("Header2","Integer","10"), ...);
    public static final LoansApiResourceResponse LoansApi_Get_getLoan_400 = new LoansApiResourceResponse(400, new Header("Header3", "String", ""), new Header("Header2","Integer","10"), ...);
    public static final LoansApiResourceResponse LoansApi_Get_getLoan_401 = new LoansApiResourceResponse(401, new Header("Header3", "String", ""), new Header("Header2","Integer","10"), ...);

    public static final LoansApiResourceResponse LoansApi_Post_getLoan_201 = new LoansApiResourceResponse(201, new Header("Location", "String", ""));
    public static final LoansApiResourceResponse LoansApi_Post_getLoan_401 = new LoansApiResourceResponse(401, new Header("Error-Cause", "String", ""));

  }

public class ResourceResponse<T> {
    protected int code;
    protected MultiMap headers;
    protected T payload;

    public ResourceResponse<T>(int code, Header... headers){
      // set the headers and the code
    }

    public void popluate(T payload, String... headerValues){
      // set the payload and the heade values, 
    }
}

//service implementation
public void getLoan(String id, Handler<AsyncResult<ResourceResponse<Loan>>> handler){
  Loan ln = repo.getLoan(id);
  LoansApiResourceResponse res = LoansApiResourceResponse.LoansApi_Get_getLoan_200;
  res.populate(ln, "Header1_Value", "Header2_Value")
  handler.handle(Future.succeededFuture(res));
}

public void postLoan(Loan body, Handler<AsyncResult<ResourceResponse<Loan>>> handler) {
  try{
    repo.saveLoan(body);
    LoansApiResourceResponse res = LoansApiResourceResponse.LoansApi_Post_getLoan_201;
    res.populate(ln, buildUrl(body.getId()));
    handler.handle(Future.succeededFuture(res));  
  } catch(Exception ex){
    LoansApiResourceResponse res = LoansApiResourceResponse.LoansApi_Post_getLoan_401;
    res.populate(null, ex.getMessage());
    handler.handle(Future.failedFuture(res));        
  }
}
phiz71 commented 7 years ago

Hi, I've just pushed a PR #82 . I've done a lot of modification, so looking at files changes might be difficult.

phiz71 commented 7 years ago

@javadch : can I close this issue ?

javadch commented 7 years ago

Sure.