[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
GitHub user puneethps opened a pull request:

    https://github.com/apache/tomee/pull/340

    TOMEE-2289 microprofile openAPI example

    The JIRA status for TOMEE-2289 hasn't changed for quite sometime so I felt I could add a new project for this. Can someone please  review the PR and suggest changes required?

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/puneethps/tomee master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/tomee/pull/340.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #340
   
----
commit cb3cc174ea9e66a63629b46f33a7f88b2bcedfd0
Author: Puneeth PS <puneeth.ps@...>
Date:   2019-01-01T11:34:21Z

    Added MicroProfile OpenAPI Example.(TOMEE-2289)
   
    There was no progress on TOMEE-2289 so I created the example project.

commit df1920931e0a19f1dc41edc6170f87ba42feadf2
Author: Puneeth PS <puneeth.ps@...>
Date:   2019-01-01T12:09:50Z

    added README.adoc
   
    added README.adoc

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
Github user ivanjunckes commented on a diff in the pull request:

    https://github.com/apache/tomee/pull/340#discussion_r244725387
 
    --- Diff: examples/mp-openapi/src/main/java/org/superbiz/openapi/WeatherService.java ---
    @@ -0,0 +1,52 @@
    +package org.superbiz.openapi;
    +
    +
    +
    +import javax.enterprise.context.ApplicationScoped;
    +
    +import javax.ws.rs.GET;
    +import javax.ws.rs.Path;
    +import javax.ws.rs.PathParam;
    +import javax.ws.rs.core.Response;
    +
    +import org.eclipse.microprofile.openapi.annotations.Operation;
    +import org.eclipse.microprofile.openapi.annotations.parameters.Parameter;
    +import org.eclipse.microprofile.openapi.annotations.responses.APIResponse;
    +import org.eclipse.microprofile.openapi.annotations.media.Content;
    +import org.eclipse.microprofile.openapi.annotations.media.Schema;
    +
    --- End diff --
   
    Can you be careful formatting the class? There are a lot of unnecessary spaces.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
In reply to this post by cotnic
Github user ivanjunckes commented on a diff in the pull request:

    https://github.com/apache/tomee/pull/340#discussion_r244725584
 
    --- Diff: examples/mp-openapi/src/main/java/org/superbiz/openapi/Weather.java ---
    @@ -0,0 +1,25 @@
    +package org.superbiz.openapi;
    +
    +import org.eclipse.microprofile.openapi.annotations.media.Schema;
    +
    +@Schema(name="Weather", description="POJO that represents weather.")
    --- End diff --
   
    Can you follow a pattern with spaces? name="Wather" has no space between name and the value. But for @Schema the variables have space. Can you make sure to format them correctly having one space between the variable and the value?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
In reply to this post by cotnic
Github user ivanjunckes commented on a diff in the pull request:

    https://github.com/apache/tomee/pull/340#discussion_r244724535
 
    --- Diff: examples/mp-openapi/README.adoc ---
    @@ -0,0 +1,112 @@
    +:index-group: MicroProfile
    +:jbake-type: page
    +:jbake-status: unpublished
    +
    +# Microprofile OpenAPI
    +This is an example on how to use microprofile OpenAPI in TomEE.
    +
    +== Run the application:
    +mvn clean install tomee:run
    +
    +Within the application there is an endpoint that will give you weather based on day.
    +
    +== Request
    +....
    +GET http://localhost:8080/mp-openapi/weather/status/{today}
    +
    +
    +== Response
    +....
    +{today} is a sunny day.
    +
    +
    +== OpenAPI
    +
    +OpenAPI is a spec for natively producing OpenAPI v3 documents from JAX-RS applications.
    +There exists an endpoint /openApi which gives the OpenAPI document based on Accept request header.
    +
    +
    +== Request
    +....
    +GET http://localhost:8080/mp-openapi/openapi
    --- End diff --
   
    It does not work without content-type as application/json so it is worth mention it. This is a bug in openapi I think. Because it should have a default message body reader/writer and it should be yaml. But it can be fixed later, there is a discussion on the list about it.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
In reply to this post by cotnic
Github user ivanjunckes commented on a diff in the pull request:

    https://github.com/apache/tomee/pull/340#discussion_r244723832
 
    --- Diff: examples/mp-openapi/src/main/java/org/superbiz/openapi/WeatherService.java ---
    @@ -0,0 +1,52 @@
    +package org.superbiz.openapi;
    +
    +
    +
    +import javax.enterprise.context.ApplicationScoped;
    +
    +import javax.ws.rs.GET;
    +import javax.ws.rs.Path;
    +import javax.ws.rs.PathParam;
    +import javax.ws.rs.core.Response;
    +
    +import org.eclipse.microprofile.openapi.annotations.Operation;
    +import org.eclipse.microprofile.openapi.annotations.parameters.Parameter;
    +import org.eclipse.microprofile.openapi.annotations.responses.APIResponse;
    +import org.eclipse.microprofile.openapi.annotations.media.Content;
    +import org.eclipse.microprofile.openapi.annotations.media.Schema;
    +
    +
    +
    +
    +
    +@Path("/weather")
    +@ApplicationScoped
    +public class WeatherService {
    +
    +    @Path("/status/{day}/")  
    +    @GET
    +    @Operation(summary = "Finds weather for day specified in the URL ", description = "Describes how the day will be.")
    +    @APIResponse(responseCode = "400", description = "Weather for this day not found")
    +    public Response dayStatus( @Parameter(description = "The day for which the weather needs to be fetched.", required = true) @PathParam("day") String day ) {
    +    
    +     if(day.equalsIgnoreCase("tomorrow"))
    +     return Response.status(Response.Status.NOT_FOUND).build();
    +     String message=day+ "is a sunny day.";
    --- End diff --
   
    It is missing a space. Output is "Mondayis a sunny day." and should be "Monday is a sunny day."


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
In reply to this post by cotnic
Github user ivanjunckes commented on a diff in the pull request:

    https://github.com/apache/tomee/pull/340#discussion_r244726062
 
    --- Diff: examples/mp-openapi/src/test/java/org/superbiz/openapi/WeatherServiceTest.java ---
    @@ -0,0 +1,208 @@
    +package org.superbiz.openapi;
    +
    +import org.jboss.arquillian.container.test.api.Deployment;
    +import org.jboss.arquillian.junit.Arquillian;
    +import org.jboss.arquillian.test.api.ArquillianResource;
    +import org.jboss.shrinkwrap.api.ShrinkWrap;
    +import org.jboss.shrinkwrap.api.asset.StringAsset;
    +import org.jboss.shrinkwrap.api.spec.WebArchive;
    +
    +import org.junit.After;
    +import org.junit.Before;
    +import org.junit.Test;
    +import org.junit.runner.RunWith;
    +
    +import javax.json.Json;
    +import javax.json.JsonObject;
    +
    +import javax.ws.rs.client.Client;
    +import javax.ws.rs.client.ClientBuilder;
    +import javax.ws.rs.client.WebTarget;
    +import javax.ws.rs.core.MediaType;
    +import javax.ws.rs.core.Response;
    +
    +import java.io.StringReader;
    +
    +import java.net.URL;
    +import java.util.stream.Stream;
    +
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +@RunWith(Arquillian.class)
    +public class WeatherServiceTest {
    +
    +    @Deployment(testable = false)
    +    public static WebArchive createDeployment() {
    +        final WebArchive webArchive = ShrinkWrap.create(WebArchive.class, "test.war")
    +                .addClass(WeatherService.class)
    +                .addClass(Weather.class)
    +                .addAsWebInfResource(new StringAsset("<beans/>"), "beans.xml");
    +        return webArchive;
    +    }
    +
    +    @ArquillianResource
    +    private URL base;
    +
    +    private Client client;
    +
    +    @Before
    +    public void before() {
    +        this.client = ClientBuilder.newClient();
    +    }
    +
    +    @After
    +    public void after() {
    +        this.client.close();
    +    }
    +
    +    
    +    @Test
    +    public void dayStatus() {
    +        WebTarget webTarget = this.client.target(this.base.toExternalForm());
    +        final Response message =  webTarget.path("/openapi")
    +                .request()
    +                .accept(MediaType.APPLICATION_JSON)
    +                .get();
    +        final String metaData = message.readEntity(String.class);
    +        JsonObject jsonObject = Json.createReader(new StringReader(metaData)).readObject();
    +        JsonObject response=jsonObject.get("paths").asJsonObject().get("/weather/status/{day}").asJsonObject();
    +        final String expected = "{\r\n" +
    +         "            \"get\": {\r\n" +
    +         "                \"deprecated\": false,\r\n" +
    +         "                \"description\": \"Describes how the day will be.\",\r\n" +
    +         "                \"operationId\": \"dayStatus\",\r\n" +
    +         "                \"parameters\": [\r\n" +
    +         "                    {\r\n" +
    +         "                        \"allowEmptyValue\": false,\r\n" +
    +         "                        \"allowReserved\": false,\r\n" +
    +         "                        \"description\": \"The day for which the weather needs to be fetched.\",\r\n" +
    +         "                        \"in\": \"path\",\r\n" +
    +         "                        \"name\": \"\",\r\n" +
    +         "                        \"required\": true,\r\n" +
    +         "                        \"schema\": {\r\n" +
    +         "                            \"type\": \"string\"\r\n" +
    +         "                        }\r\n" +
    +         "                    }\r\n" +
    +         "                ],\r\n" +
    +         "                \"responses\": {\r\n" +
    +         "                    \"400\": {\r\n" +
    +         "                        \"content\": {\r\n" +
    +         "                            \"200\": {}\r\n" +
    +         "                        },\r\n" +
    +         "                        \"description\": \"Weather for this day not found\"\r\n" +
    +         "                    }\r\n" +
    +         "                },\r\n" +
    +         "                \"summary\": \"Finds weather for day specified in the URL \"\r\n" +
    +         "            }\r\n" +
    +         "        }";
    +        JsonObject expectedJson = Json.createReader(new StringReader(expected)).readObject();
    +        assertEquals(expectedJson.keySet().size(), response.keySet().size());
    +        String[] expectedKeys = new String[]{"deprecated", "description", "operationId", "parameters", "responses", "summary"};
    +        Stream.of(expectedKeys).forEach((text) -> {
    +          assertTrue("Expected: " + text
    +                  + " to be present in " + expected,
    +                  expectedJson.getJsonObject("get").get(text) != null);
    +        });
    +        
    +
    +    }
    +    
    +    @Test
    +    public void detailedDayStatus() {
    +        WebTarget webTarget = this.client.target(this.base.toExternalForm());
    +        final Response message =  webTarget.path("/openapi")
    +                .request()
    +                .accept(MediaType.APPLICATION_JSON)
    +                .get();
    +        final String metaData = message.readEntity(String.class);
    +        JsonObject jsonObject = Json.createReader(new StringReader(metaData)).readObject();
    +        JsonObject response=jsonObject.get("paths").asJsonObject().get("/weather/detailedWeather/{day}").asJsonObject();
    +        final String expected = "{\r\n" +
    +         "            \"get\": {\r\n" +
    +         "                \"operationId\": \"getDetailedWeather\",\r\n" +
    +         "                \"parameters\": [\r\n" +
    +         "                    {\r\n" +
    +         "                        \"allowEmptyValue\": false,\r\n" +
    --- End diff --
   
    There are a lot of values in the response that are not in the Weather pojo. This seems to me like a bug in the implementation. Can you evaluate and raise this bug on the list?
    allowEmptyValue
    allowReserved
    required
    ... are not part of the pojo.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
In reply to this post by cotnic
Github user ivanjunckes commented on a diff in the pull request:

    https://github.com/apache/tomee/pull/340#discussion_r244725773
 
    --- Diff: examples/mp-openapi/src/main/java/org/superbiz/openapi/WeatherService.java ---
    @@ -0,0 +1,52 @@
    +package org.superbiz.openapi;
    +
    +
    +
    +import javax.enterprise.context.ApplicationScoped;
    +
    +import javax.ws.rs.GET;
    +import javax.ws.rs.Path;
    +import javax.ws.rs.PathParam;
    +import javax.ws.rs.core.Response;
    +
    +import org.eclipse.microprofile.openapi.annotations.Operation;
    +import org.eclipse.microprofile.openapi.annotations.parameters.Parameter;
    +import org.eclipse.microprofile.openapi.annotations.responses.APIResponse;
    +import org.eclipse.microprofile.openapi.annotations.media.Content;
    +import org.eclipse.microprofile.openapi.annotations.media.Schema;
    +
    +
    +
    +
    +
    +@Path("/weather")
    +@ApplicationScoped
    +public class WeatherService {
    +
    +    @Path("/status/{day}/")  
    +    @GET
    +    @Operation(summary = "Finds weather for day specified in the URL ", description = "Describes how the day will be.")
    +    @APIResponse(responseCode = "400", description = "Weather for this day not found")
    +    public Response dayStatus( @Parameter(description = "The day for which the weather needs to be fetched.", required = true) @PathParam("day") String day ) {
    +    
    +     if(day.equalsIgnoreCase("tomorrow"))
    +     return Response.status(Response.Status.NOT_FOUND).build();
    +     String message=day+ "is a sunny day.";
    +        return Response.status(Response.Status.OK)
    +              .entity(message)
    +              .build();                
    +    }
    +
    +    @Path("/detailedWeather/{day}/")  
    +    @GET
    +    @APIResponse(description = "Detailed Weather", content=@Content(mediaType="application/json", schema= @Schema(implementation=Weather.class)))
    +    public Weather getDetailedWeather( @Parameter(description = "The day for which the detailed weather needs to be fetched.", required = true) @PathParam("day") String day) {
    +    
    +        Weather w=new Weather();
    --- End diff --
   
    Space is missing in several places, between variable and value assingments. Please format the class correctly.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
In reply to this post by cotnic
Github user ivanjunckes commented on a diff in the pull request:

    https://github.com/apache/tomee/pull/340#discussion_r244724385
 
    --- Diff: examples/mp-openapi/README.adoc ---
    @@ -0,0 +1,112 @@
    +:index-group: MicroProfile
    +:jbake-type: page
    +:jbake-status: unpublished
    +
    +# Microprofile OpenAPI
    +This is an example on how to use microprofile OpenAPI in TomEE.
    +
    +== Run the application:
    +mvn clean install tomee:run
    +
    +Within the application there is an endpoint that will give you weather based on day.
    +
    +== Request
    +....
    +GET http://localhost:8080/mp-openapi/weather/status/{today}
    +
    +
    +== Response
    +....
    +{today} is a sunny day.
    +
    +
    +== OpenAPI
    +
    +OpenAPI is a spec for natively producing OpenAPI v3 documents from JAX-RS applications.
    +There exists an endpoint /openApi which gives the OpenAPI document based on Accept request header.
    --- End diff --
   
    "/openApi" is not a valid path, it must be "/openapi". Path is casesensitive.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee pull request #340: TOMEE-2289 microprofile openAPI example

cotnic
In reply to this post by cotnic
Github user puneethps commented on a diff in the pull request:

    https://github.com/apache/tomee/pull/340#discussion_r244777643
 
    --- Diff: examples/mp-openapi/README.adoc ---
    @@ -0,0 +1,112 @@
    +:index-group: MicroProfile
    +:jbake-type: page
    +:jbake-status: unpublished
    +
    +# Microprofile OpenAPI
    +This is an example on how to use microprofile OpenAPI in TomEE.
    +
    +== Run the application:
    +mvn clean install tomee:run
    +
    +Within the application there is an endpoint that will give you weather based on day.
    +
    +== Request
    +....
    +GET http://localhost:8080/mp-openapi/weather/status/{today}
    +
    +
    +== Response
    +....
    +{today} is a sunny day.
    +
    +
    +== OpenAPI
    +
    +OpenAPI is a spec for natively producing OpenAPI v3 documents from JAX-RS applications.
    +There exists an endpoint /openApi which gives the OpenAPI document based on Accept request header.
    +
    +
    +== Request
    +....
    +GET http://localhost:8080/mp-openapi/openapi
    --- End diff --
   
    I think it is working as expected now, without the Accept:application/json header it is outputting a valid YAML. I think I should document that in the README file.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] tomee issue #340: TOMEE-2289 microprofile openAPI example

cotnic
In reply to this post by cotnic
Github user puneethps commented on the issue:

    https://github.com/apache/tomee/pull/340
 
    I noticed one more difference from the spec and the implementation, what I understood from the spec was that there is supposed to be a components field in the yaml or the json if there is a schema annotation which I don't see in the generated output. I think this should also be raised as an issue.


---