[GitHub] rmannibucau commented on issue #4: GERONIMO-6670 - fixing text/html issue and tests (snake dependency wa…

[GitBox]

rmannibucau commented on issue #4: GERONIMO-6670 - fixing text/html issue and 
tests (snake dependency wa…
URL: https://github.com/apache/geronimo-openapi/pull/4#issuecomment-451571369
 
 
   It should be good on master.
   
   However I will take the opportunity to put there some notes for later and 
work considered within the fix and not in the PR:
   
   1. we likely don't want to add snakeyaml (you can reuse jackson if you 
absolutely need a mapper in tests)
   2. we can't ignore html just like that, then next issue would be xhtml, xml, 
webp, apng etc... so it doesnt fix the issue reliably but just moves it further
   3. this fix potentially breaks an app or server since it is fine to use html 
(tip: this is one of the most simplest and efficient way to implement a custom 
UI for openapi) so we can't hardcode any exclusion except for wildcard which 
means nothing. I insist on this one since it is one of the base of JAX-RS and 
Microprofile being based on JAX-RS it can't break its own feet.
   
   Master fixes uses the fact that when there is no "handler" we default on the 
openapi spec default. Side note being it sounds very acceptable like that but 
actually it can break clients. If I request a properties formatting and get a 
yaml i will not be able to deserialize whereas i could have handled the 415 to 
fallback on JSON (or Yaml). In other words we can revert this request later if 
users complain we don't return the right status. This is however a bug in 
OpenAPI spec since it shouldn't "default" to yaml (or anything). This is why 
the "fix" is acceptable at the end.
   
   Hope it is understable and complete ;), starts to be late to code here ;).


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: TOMEE-2289

[Romain Manni-Bucau]

Hi Ivan

For the browser usage /openapi.json or /openapi.yaml will solve it nicely

On master we ensure to respect the request expected type - it is fine to
serialize it as html, this is even a way to implement a UI for it ;) - but
we also ensure there is a writer instead of assuming the user request will
work (new). This has the drawback to be able to send back an invalid
response to the client (and break it) but since the spec doesn't define
what to do in that case (guess it should just do nothing and let JAX-RS
handle it otherwise microprofile wouldn't be JAX-RS based anymore) and
defines that yaml is supposed to be the default then we are in a grey
enough area to make you happy ;). Worse case we would revert it but it is
not a prod usage for sure so no big deal for now.

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le ven. 4 janv. 2019 à 21:04, Ivan Junckes Filho  a
écrit :

> FYI I opened a PR on geronimo to fix the issue with text/html.
>
> https://github.com/apache/geronimo-openapi/pull/4
>
> Postman was working because it used */* and the implementation was
> properly handling it, but not handling text/html.
>
> On Thu, Jan 3, 2019 at 2:34 PM Ivan Junckes Filho 
> wrote:
>
>> Yes, the default content-type for browsers is text/html. I will submit a
>> pr accepting that.
>>
>> On Thu, Jan 3, 2019 at 2:17 PM César Hernández Mendoza <
>> cesargu...@gmail.com> wrote:
>>
>>> This  "default" behavior is also present in Metrics geronimo
>>> implementation.
>>> The issue is that the spec doesn't explicitly indicate the default
>>> content-type requirement.
>>>
>>> El jue., 3 ene. 2019 a las 9:48, Ivan Junckes Filho (<
>>> ivanjunc...@gmail.com>)
>>> escribió:
>>>
>>> > I will take a look and help troubleshoot this.
>>> >
>>> > On Thu, Jan 3, 2019 at 12:08 PM Puneeth PS 
>>> wrote:
>>> >
>>> > > Hi Ivan,
>>> > >
>>> > > /openapi works in POSTMAN client but not on browser, so might not be
>>> an
>>> > > issue.
>>> > >
>>> > > On Thu, Jan 3, 2019 at 6:38 PM Ivan Junckes Filho <
>>> ivanjunc...@gmail.com
>>> > >
>>> > > wrote:
>>> > >
>>> > > > Hi Puneeth, I added a couple of more comments.
>>> > > >
>>> > > > There seems to have a couple of bugs in the openapi implementation.
>>> > > >
>>> > > > Json serialization bug and also /openapi is not working without the
>>> > > header.
>>> > > > So we may need to open bugs
>>> > > > in https://issues.apache.org/jira/projects/GERONIMO/issues
>>> > > > .
>>> > > >
>>> > > > I will try to help troubleshoot these issues.
>>> > > >
>>> > > >
>>> > > >
>>> > > >
>>> > > > On Thu, Jan 3, 2019 at 12:53 AM Puneeth PS 
>>> > wrote:
>>> > > >
>>> > > > > Hi Ivan,
>>> > > > >
>>> > > > > I have made the changes requested and I've some questions
>>> regarding
>>> > the
>>> > > > > spec which I've asked on the github thread, can you please
>>> clarify
>>> > > these?
>>> > > > >
>>> > > > > On Wed 2 Jan, 2019, 6:09 PM Ivan Junckes Filho <
>>> > ivanjunc...@gmail.com
>>> > > > > wrote:
>>> > > > >
>>> > > > > > Puneeth, I did some comments here
>>> > > > > >
>>> > https://github.com/apache/tomee/pull/340#pullrequestreview-188661807
>>> > > ,
>>> > > > > > please take a look.
>>> > > > > >
>>> > > > > > On Wed, Jan 2, 2019 at 9:51 AM Ivan Junckes Filho <
>>> > > > ivanjunc...@gmail.com
>>> > > > > >
>>> > > > > > wrote:
>>> > > > > >
>>> > > > > > > Hi Puneeth, I will review it for you.
>>> > > > > > >
>>> > > > > > > On Wed, Jan 2, 2019 at 8:39 AM Puneeth PS <
>>> puneeth...@gmail.com>
>>> > > > > wrote:
>>> > > > > > >
>>> > > > > > >> Hi,
>>> > > > > > >>
>>> > > > > > >> I have created a PR (
>>> https://github.com/apache/tomee/pull/340
>>> > )
>>> > > > for
>>> > > > > > >> TOMEE-2289. Can someone take a look and help me improve it?
>>> > > > > > >>
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> >
>>>
>>>
>>> --
>>> Atentamente:
>>> César Hernández Mendoza.
>>>
>>

[jira] [Updated] (GERONIMO-6670) When /openapi is requested with a mediatype the server does not support the response serialization fails

[Romain Manni-Bucau (JIRA)]

 [ 
https://issues.apache.org/jira/browse/GERONIMO-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Romain Manni-Bucau updated GERONIMO-6670:
-
Fix Version/s: (was: OpenAPI_1.0.3)
   OpenAPI_1.0.4

> When /openapi is requested with a mediatype the server does not support the 
> response serialization fails
> 
>
> Key: GERONIMO-6670
> URL: https://issues.apache.org/jira/browse/GERONIMO-6670
> Project: Geronimo
>  Issue Type: Improvement
>  Security Level: public(Regular issues) 
>Reporter: Ivan Junckes Filho
>Assignee: Romain Manni-Bucau
>Priority: Minor
> Fix For: OpenAPI_1.0.4
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Content-type text/html is returning this error when used:
> {code}
> No message body writer has been found for class 
> org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
> text/html
> {code}
> This content type is used by browsers, so if the user wants to do a GET 
> /openapi in the browser it will display this error.
> The ideal scenario is that user must be able to see the default openapi 
> document using the browser.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Resolved] (GERONIMO-6670) When /openapi is requested with a mediatype the server does not support the response serialization fails

[Romain Manni-Bucau (JIRA)]

 [ 
https://issues.apache.org/jira/browse/GERONIMO-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Romain Manni-Bucau resolved GERONIMO-6670.
--
   Resolution: Fixed
Fix Version/s: OpenAPI_1.0.3

The fix ensures there is a body writer for OpenAPI before returning, it 
guarantees it will be serializable from the mediatype sent or will default to 
the default mediatype (yaml if present, json otherwise).

This guarantees we respect the spec (JAX-RS and therefore Microprofile-OpenAPI) 
as much as possible and not violate it not respecting the requested media type 
intentionally.

> When /openapi is requested with a mediatype the server does not support the 
> response serialization fails
> 
>
> Key: GERONIMO-6670
> URL: https://issues.apache.org/jira/browse/GERONIMO-6670
> Project: Geronimo
>  Issue Type: Improvement
>  Security Level: public(Regular issues) 
>Reporter: Ivan Junckes Filho
>Assignee: Romain Manni-Bucau
>Priority: Minor
> Fix For: OpenAPI_1.0.3
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Content-type text/html is returning this error when used:
> {code}
> No message body writer has been found for class 
> org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
> text/html
> {code}
> This content type is used by browsers, so if the user wants to do a GET 
> /openapi in the browser it will display this error.
> The ideal scenario is that user must be able to see the default openapi 
> document using the browser.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Updated] (GERONIMO-6670) Content-type text/html is returning error on /open

[Romain Manni-Bucau (JIRA)]

 [ 
https://issues.apache.org/jira/browse/GERONIMO-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Romain Manni-Bucau updated GERONIMO-6670:
-
Priority: Minor  (was: Major)

> Content-type text/html is returning error on /open
> --
>
> Key: GERONIMO-6670
> URL: https://issues.apache.org/jira/browse/GERONIMO-6670
> Project: Geronimo
>  Issue Type: Improvement
>  Security Level: public(Regular issues) 
>Reporter: Ivan Junckes Filho
>Priority: Minor
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Content-type text/html is returning this error when used:
> No message body writer has been found for class 
> org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
> text/html
> This content type is used by browsers, so if the user wants to do a GET 
> /openapi in the browser it will display this error.
> The ideal scenario is that user must be able to see the default openapi 
> document using the browser.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Updated] (GERONIMO-6670) When /openapi is requested with a mediatype the server does not support the response serialization fails

[Romain Manni-Bucau (JIRA)]

 [ 
https://issues.apache.org/jira/browse/GERONIMO-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Romain Manni-Bucau updated GERONIMO-6670:
-
Description: 
Content-type text/html is returning this error when used:

{code}
No message body writer has been found for class 
org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
text/html
{code}

This content type is used by browsers, so if the user wants to do a GET 
/openapi in the browser it will display this error.

The ideal scenario is that user must be able to see the default openapi 
document using the browser.

  was:
Content-type text/html is returning this error when used:

No message body writer has been found for class 
org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
text/html

This content type is used by browsers, so if the user wants to do a GET 
/openapi in the browser it will display this error.

The ideal scenario is that user must be able to see the default openapi 
document using the browser.


> When /openapi is requested with a mediatype the server does not support the 
> response serialization fails
> 
>
> Key: GERONIMO-6670
> URL: https://issues.apache.org/jira/browse/GERONIMO-6670
> Project: Geronimo
>  Issue Type: Improvement
>  Security Level: public(Regular issues) 
>Reporter: Ivan Junckes Filho
>Priority: Minor
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Content-type text/html is returning this error when used:
> {code}
> No message body writer has been found for class 
> org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
> text/html
> {code}
> This content type is used by browsers, so if the user wants to do a GET 
> /openapi in the browser it will display this error.
> The ideal scenario is that user must be able to see the default openapi 
> document using the browser.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Assigned] (GERONIMO-6670) When /openapi is requested with a mediatype the server does not support the response serialization fails

[Romain Manni-Bucau (JIRA)]

 [ 
https://issues.apache.org/jira/browse/GERONIMO-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Romain Manni-Bucau reassigned GERONIMO-6670:


Assignee: Romain Manni-Bucau

> When /openapi is requested with a mediatype the server does not support the 
> response serialization fails
> 
>
> Key: GERONIMO-6670
> URL: https://issues.apache.org/jira/browse/GERONIMO-6670
> Project: Geronimo
>  Issue Type: Improvement
>  Security Level: public(Regular issues) 
>Reporter: Ivan Junckes Filho
>Assignee: Romain Manni-Bucau
>Priority: Minor
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Content-type text/html is returning this error when used:
> {code}
> No message body writer has been found for class 
> org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
> text/html
> {code}
> This content type is used by browsers, so if the user wants to do a GET 
> /openapi in the browser it will display this error.
> The ideal scenario is that user must be able to see the default openapi 
> document using the browser.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Updated] (GERONIMO-6670) When /openapi is requested with a mediatype the server does not support the response serialization fails

[Romain Manni-Bucau (JIRA)]

 [ 
https://issues.apache.org/jira/browse/GERONIMO-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Romain Manni-Bucau updated GERONIMO-6670:
-
Summary: When /openapi is requested with a mediatype the server does not 
support the response serialization fails  (was: Content-type text/html is 
returning error on /open)

> When /openapi is requested with a mediatype the server does not support the 
> response serialization fails
> 
>
> Key: GERONIMO-6670
> URL: https://issues.apache.org/jira/browse/GERONIMO-6670
> Project: Geronimo
>  Issue Type: Improvement
>  Security Level: public(Regular issues) 
>Reporter: Ivan Junckes Filho
>Priority: Minor
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Content-type text/html is returning this error when used:
> No message body writer has been found for class 
> org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
> text/html
> This content type is used by browsers, so if the user wants to do a GET 
> /openapi in the browser it will display this error.
> The ideal scenario is that user must be able to see the default openapi 
> document using the browser.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[GitHub] cesarhernandezgt commented on issue #4: GERONIMO-6670 - fixing text/html issue and tests (snake dependency wa…

[GitBox]

cesarhernandezgt commented on issue #4: GERONIMO-6670 - fixing text/html issue 
and tests (snake dependency wa…
URL: https://github.com/apache/geronimo-openapi/pull/4#issuecomment-451559048
 
 
   +1 build successfully locally.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: TOMEE-2289

[Ivan Junckes Filho]

FYI I opened a PR on geronimo to fix the issue with text/html.

https://github.com/apache/geronimo-openapi/pull/4

Postman was working because it used */* and the implementation was properly
handling it, but not handling text/html.

On Thu, Jan 3, 2019 at 2:34 PM Ivan Junckes Filho 
wrote:

> Yes, the default content-type for browsers is text/html. I will submit a
> pr accepting that.
>
> On Thu, Jan 3, 2019 at 2:17 PM César Hernández Mendoza <
> cesargu...@gmail.com> wrote:
>
>> This  "default" behavior is also present in Metrics geronimo
>> implementation.
>> The issue is that the spec doesn't explicitly indicate the default
>> content-type requirement.
>>
>> El jue., 3 ene. 2019 a las 9:48, Ivan Junckes Filho (<
>> ivanjunc...@gmail.com>)
>> escribió:
>>
>> > I will take a look and help troubleshoot this.
>> >
>> > On Thu, Jan 3, 2019 at 12:08 PM Puneeth PS 
>> wrote:
>> >
>> > > Hi Ivan,
>> > >
>> > > /openapi works in POSTMAN client but not on browser, so might not be
>> an
>> > > issue.
>> > >
>> > > On Thu, Jan 3, 2019 at 6:38 PM Ivan Junckes Filho <
>> ivanjunc...@gmail.com
>> > >
>> > > wrote:
>> > >
>> > > > Hi Puneeth, I added a couple of more comments.
>> > > >
>> > > > There seems to have a couple of bugs in the openapi implementation.
>> > > >
>> > > > Json serialization bug and also /openapi is not working without the
>> > > header.
>> > > > So we may need to open bugs
>> > > > in https://issues.apache.org/jira/projects/GERONIMO/issues
>> > > > .
>> > > >
>> > > > I will try to help troubleshoot these issues.
>> > > >
>> > > >
>> > > >
>> > > >
>> > > > On Thu, Jan 3, 2019 at 12:53 AM Puneeth PS 
>> > wrote:
>> > > >
>> > > > > Hi Ivan,
>> > > > >
>> > > > > I have made the changes requested and I've some questions
>> regarding
>> > the
>> > > > > spec which I've asked on the github thread, can you please clarify
>> > > these?
>> > > > >
>> > > > > On Wed 2 Jan, 2019, 6:09 PM Ivan Junckes Filho <
>> > ivanjunc...@gmail.com
>> > > > > wrote:
>> > > > >
>> > > > > > Puneeth, I did some comments here
>> > > > > >
>> > https://github.com/apache/tomee/pull/340#pullrequestreview-188661807
>> > > ,
>> > > > > > please take a look.
>> > > > > >
>> > > > > > On Wed, Jan 2, 2019 at 9:51 AM Ivan Junckes Filho <
>> > > > ivanjunc...@gmail.com
>> > > > > >
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Hi Puneeth, I will review it for you.
>> > > > > > >
>> > > > > > > On Wed, Jan 2, 2019 at 8:39 AM Puneeth PS <
>> puneeth...@gmail.com>
>> > > > > wrote:
>> > > > > > >
>> > > > > > >> Hi,
>> > > > > > >>
>> > > > > > >> I have created a PR (
>> https://github.com/apache/tomee/pull/340
>> > )
>> > > > for
>> > > > > > >> TOMEE-2289. Can someone take a look and help me improve it?
>> > > > > > >>
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>>
>> --
>> Atentamente:
>> César Hernández Mendoza.
>>
>

[GitHub] ivanjunckes opened a new pull request #4: GERONIMO-6670 - fixing text/html issue and tests (snake dependency wa…

[GitBox]

ivanjunckes opened a new pull request #4: GERONIMO-6670 - fixing text/html 
issue and tests (snake dependency wa…
URL: https://github.com/apache/geronimo-openapi/pull/4
 
 
   …s not there for testing purposes)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[jira] [Created] (GERONIMO-6670) Content-type text/html is returning error on /open

[Ivan Junckes Filho (JIRA)]

Ivan Junckes Filho created GERONIMO-6670:


 Summary: Content-type text/html is returning error on /open
 Key: GERONIMO-6670
 URL: https://issues.apache.org/jira/browse/GERONIMO-6670
 Project: Geronimo
  Issue Type: Improvement
  Security Level: public (Regular issues)
Reporter: Ivan Junckes Filho


Content-type text/html is returning this error when used:

No message body writer has been found for class 
org.apache.geronimo.microprofile.openapi.impl.model.OpenAPIImpl, ContentType: 
text/html

This content type is used by browsers, so if the user wants to do a GET 
/openapi in the browser it will display this error.

The ideal scenario is that user must be able to see the default openapi 
document using the browser.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: [VOTE] Apache Geronimo OpenAPI 1.0.3

[Ivan Junckes Filho]

+1

On Thu, Jan 3, 2019 at 6:30 PM Mark Struberg  wrote:

> +1
>
> LieGrue,
> strub
>
>
> > Am 02.01.2019 um 16:10 schrieb Romain Manni-Bucau  >:
> >
> > Hi guys,
> >
> > as mentionned here is the vote fore Geronimo OpenAPI 1.0.3 to ensure we
> don't need jackson when using it
> >
> > The staging repo is:
> https://repository.apache.org/content/repositories/orgapachegeronimo-1082
> > My keys is still available in
> http://svn.apache.org/repos/asf/geronimo/KEYS
> > Tag is on the main source repo and sources in the staging repo.
> >
> > This vote is open for 3 days as usual or untll it gets its 3 binding +1s.
> >
> > Romain Manni-Bucau
> > @rmannibucau |  Blog | Old Blog | Github | LinkedIn | Book
>
>

Re: [VOTE] Apache Geronimo Metrics 1.0.2

[Ivan Junckes Filho]

+1

On Thu, Jan 3, 2019 at 6:39 PM Mark Struberg  wrote:

> +1
>
> LieGrue,
> strub
>
>
> > Am 02.01.2019 um 16:18 schrieb Romain Manni-Bucau  >:
> >
> > Hi guys,
> >
> > As mentionned here is the vote for Geronimo Metrics 1.0.2 to ensure
> sigar extension does not register twice the same filesystem when multiple
> mount points are present - which prevents the startup of any application
> embedding it.
> >
> > The staging repo is:
> https://repository.apache.org/content/repositories/orgapachegeronimo-1083
> > My keys is still available in
> http://svn.apache.org/repos/asf/geronimo/KEYS
> > Tag is on the main source repo and sources in the staging repo.
> >
> > This vote is open for 3 days as usual or untll it gets its 3 binding +1s.
> >
> > Romain Manni-Bucau
> > @rmannibucau |  Blog | Old Blog | Github | LinkedIn | Book
>
>