You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@juneau.apache.org by David Goddard <go...@acm.org> on 2018/10/25 18:32:09 UTC
Swagger in RestMethod (7.2.0 -> 7.2.1 and onwards)
Hi,
I'm catching up with some of the 7.2.0 changes and I'm hitting some
issues updating some pre-7.2.0 code (specifically I'm having problems
similar to JUNEAU-85).
I note that org.apache.juneau.rest.annotation.Parameter was removed from
7.2.0 as part of the Swagger changes (although not all of the
documentation seems to have caught up:
http://juneau.apache.org/site/apidocs/org/apache/juneau/rest/annotation/MethodSwagger.html#parameters--)
So my original code was like:
swagger = @MethodSwagger(
parameters= {
@Parameter(in="path", name="somePathParam", description="xxx"),
}
),
This would obviously fail in 7.2.0 as @Parameter is absent.
However, I'm hitting some issues getting it working with the
SimplifiedJson syntax described in
http://juneau.apache.org/site/apidocs/org/apache/juneau/rest/annotation/RestMethod.html#swagger--,
which is of this format as per the docs:
swagger={
"parameters:[",
"{name:'propertyName',in:'path',description:'The system property
name.'},",
"{in:'body',description:'The new system property value.'}",
"],",
"responses:{",
"302: {headers:{Location:{description:'The root URL of this
resource.'}}},",
"403: {description:'User is not an admin.'}",
"}"
}
So my method updates to something like this:
@RestMethod(name="GET", path="/foo/{somePathParam}/*",
guards=FooGuard.class,
swagger = {
"parameters:[",
"{name:'somePathParam',in:'path',description:'xxx',required:true}",
"],"
},
properties = { // some props }
)
public ServiceResult someMethod(RestRequest request, RestResponse
response, @Path String somePathParam) {
// etc.
}
However, this gives me a syntax error in Eclipse:
Type mismatch: cannot convert from String[] to MethodSwagger
Any idea what I'm doing wrong here? (I am assuming I'm doing something
wrong)
Looking at the Pet Store example code, I see a different syntax in
PetStoreResource, leading me to change my method to this:
@RestMethod(name="GET",
path="/foo/{somePathParam}/*",
properties ={
@Property(name=HTMLDOC_header, value="$L{title}"),
},
swagger = @MethodSwagger(
parameters= {
"{name:'somePathParam',in:'path',description:'xxx'}",
}
)
)
public ServiceResult someMethod(RestRequest request,
RestResponse response,
@Path String somePathParam) {
return null; //(Not really)
}
Do we have a documentation issue, or an understanding issue on my part?
However, in any case I get a runtime error at the moment:
#######
Exception occurred while initializing method
'some.package.ServiceREST.someMethod'
...
Caused by: @Path used without name or value on method
'some.package.ServiceREST.someMethod(RestRequest,RestResponse,String)'
parameter '2'.
#######
What am I missing?
....
More generally, @Parameter is back in 7.2.1-RC, but it's unclear to me
at least what the direction is for this. Are we now looking to remove
these annotations again in a future (major) release, with the favoured
approach being the SimplifiedJson syntax? However, @Parameter is not
marked as deprecated in the current RC. I don't get a clear sense from
the docs which way this is going (or indeed where it exactly is now).
I'd agree with Gary's sentiment from JUNEAU-85 that 7.1 -> 7.x should be
a non-breaking change, but it is the case that having multiple
concurrently valid techniques is confusing at present (at least to me);
this could be a documentation fix though.
Thanks
David
Re: Swagger in RestMethod (7.2.0 -> 7.2.1 and onwards)
Posted by James Bognar <ja...@apache.org>.
Thanks David. You're right...it's a doc error. I'll fix it.
On Fri, Oct 26, 2018 at 5:03 AM David Goddard <go...@acm.org> wrote:
> On 25/10/2018 22:52, James Bognar wrote:
>
> > I could use some guidance here actually on whether to keep
> > the @Parameter annotation. The plan was to deprecate it, but I'm
> > willing to keep it in.
>
> I prefer to remove ambiguity so in the long term support removing it.
>
> > The "new" approach is to annotate your parameters on the method
> > arguments like so:
> >
> > @RestMethod(name="GET",
> > path="/foo/{somePathParam}/*",
> > properties ={
> > @Property(name=HTMLDOC_header, value="$L{title}")
> > }
> > )
> > public ServiceResult someMethod(
> > RestRequest request,
> > RestResponse response,
> > @Path(
> > name="somePathParam",
> > description="xxx"
> > )
> > String somePathParam) {
> > ...
> > }
> >
>
> OK, this is good - works for me and is a clean approach, thanks.
>
> [...]
> > As far as keeping @Parameter, I could re-add it using a separate
> > "parameters2" attribute like so:
> > @RestMethod(name="GET",
> > swagger=@MethodSwagger(
> > parameters2={
> > @Parameter(....)
> > }
> > )
> > )
> >
> > My concerns with that though are that we're defining two separate ways
> > of defining free-form parameters. And the existing @Parameter
> > annotation didn't get the Swagger spec exactly correct (it was an
> > amalgam of "body" and the other parameter types which only has some
> > overlap in Swagger).
>
> This would still be a breaking change though? As it understand it,
> @Parameter is already back in the 7.2.1 code, so as not to break old
> code. For a future major release, I would support removing it. I agree
> that we should not have too many ways of defining these params so it
> should go entirely, as long as it's a major release with a release note
> describing an 'old' and 'new' way of doing it, so migration is clear.
>
> > I'm not sure why you're seeing that "@Path used without name or value"
> > error. I'll see if I can reproduce that. The workaround is to add the
> > name of your path parameter like so:
> >
> > public ServiceResult someMethod(
> > RestRequest request,
> > RestResponse response,
> > @Path("somePathParam") String somePathParam) {
> > ...
> > }
>
> This is my fault, sorry. Revisiting the code today I see that I had
> left a method with broken syntax in the class (ironically that I'd added
> to test out this problem); removing this fixed the problem.
>
> Incidentally, I think there is a documentation error in
>
> http://juneau.apache.org/site/apidocs/overview-summary.html#juneau-rest-server.HttpPartAnnotations.Path
>
> One of the examples suggests 'required' is an allowed attribute of @Path
> but this is not supported in the code:
>
> // Normal
> @Path(
> name="name",
> description="Pet name",
> required=true,
> example="Doggie"
> )
>
> (I think logically 'required' should not be supported, so it seems a
> documentation rather than code error)
>
> Thanks,
>
> David
>
>
>
> > On Thu, Oct 25, 2018 at 2:32 PM David Goddard <goddard@acm.org
> > <ma...@acm.org>> wrote:
> >
> > Hi,
> >
> > I'm catching up with some of the 7.2.0 changes and I'm hitting some
> > issues updating some pre-7.2.0 code (specifically I'm having problems
> > similar to JUNEAU-85).
> >
> > I note that org.apache.juneau.rest.annotation.Parameter was removed
> > from
> > 7.2.0 as part of the Swagger changes (although not all of the
> > documentation seems to have caught up:
> >
> http://juneau.apache.org/site/apidocs/org/apache/juneau/rest/annotation/MethodSwagger.html#parameters--
> )
> >
> > So my original code was like:
> >
> > swagger = @MethodSwagger(
> > parameters= {
> > @Parameter(in="path", name="somePathParam",
> description="xxx"),
> > }
> > ),
> >
> > This would obviously fail in 7.2.0 as @Parameter is absent.
> >
> > However, I'm hitting some issues getting it working with the
> > SimplifiedJson syntax described in
> >
> http://juneau.apache.org/site/apidocs/org/apache/juneau/rest/annotation/RestMethod.html#swagger--
> ,
> >
> > which is of this format as per the docs:
> >
> > swagger={
> > "parameters:[",
> > "{name:'propertyName',in:'path',description:'The system
> > property
> > name.'},",
> > "{in:'body',description:'The new system property value.'}",
> > "],",
> > "responses:{",
> > "302: {headers:{Location:{description:'The root URL of this
> > resource.'}}},",
> > "403: {description:'User is not an admin.'}",
> > "}"
> > }
> >
> > So my method updates to something like this:
> >
> > @RestMethod(name="GET", path="/foo/{somePathParam}/*",
> > guards=FooGuard.class,
> > swagger = {
> > "parameters:[",
> >
> > "{name:'somePathParam',in:'path',description:'xxx',required:true}",
> > "],"
> > },
> > properties = { // some props }
> > )
> > public ServiceResult someMethod(RestRequest request, RestResponse
> > response, @Path String somePathParam) {
> > // etc.
> > }
> >
> > However, this gives me a syntax error in Eclipse:
> >
> > Type mismatch: cannot convert from String[] to MethodSwagger
> >
> > Any idea what I'm doing wrong here? (I am assuming I'm doing
> something
> > wrong)
> >
> > Looking at the Pet Store example code, I see a different syntax in
> > PetStoreResource, leading me to change my method to this:
> >
> > @RestMethod(name="GET",
> > path="/foo/{somePathParam}/*",
> > properties ={
> > @Property(name=HTMLDOC_header, value="$L{title}"),
> > },
> > swagger = @MethodSwagger(
> > parameters= {
> > "{name:'somePathParam',in:'path',description:'xxx'}",
> > }
> > )
> > )
> > public ServiceResult someMethod(RestRequest request,
> > RestResponse response,
> > @Path String somePathParam) {
> > return null; //(Not really)
> > }
> >
> > Do we have a documentation issue, or an understanding issue on my
> part?
> >
> > However, in any case I get a runtime error at the moment:
> >
> > #######
> > Exception occurred while initializing method
> > 'some.package.ServiceREST.someMethod'
> > ...
> > Caused by: @Path used without name or value on method
> >
> 'some.package.ServiceREST.someMethod(RestRequest,RestResponse,String)'
> > parameter '2'.
> > #######
> >
> > What am I missing?
> >
> > ....
> >
> > More generally, @Parameter is back in 7.2.1-RC, but it's unclear to
> me
> > at least what the direction is for this. Are we now looking to
> remove
> > these annotations again in a future (major) release, with the
> favoured
> > approach being the SimplifiedJson syntax? However, @Parameter is not
> > marked as deprecated in the current RC. I don't get a clear sense
> from
> > the docs which way this is going (or indeed where it exactly is now).
> >
> > I'd agree with Gary's sentiment from JUNEAU-85 that 7.1 -> 7.x
> > should be
> > a non-breaking change, but it is the case that having multiple
> > concurrently valid techniques is confusing at present (at least to
> me);
> > this could be a documentation fix though.
> >
> > Thanks
> >
> > David
> >
> >
>
>
Re: Swagger in RestMethod (7.2.0 -> 7.2.1 and onwards)
Posted by David Goddard <go...@acm.org>.
On 25/10/2018 22:52, James Bognar wrote:
> I could use some guidance here actually on whether to keep
> the @Parameter annotation. The plan was to deprecate it, but I'm
> willing to keep it in.
I prefer to remove ambiguity so in the long term support removing it.
> The "new" approach is to annotate your parameters on the method
> arguments like so:
>
> @RestMethod(name="GET",
> path="/foo/{somePathParam}/*",
> properties ={
> @Property(name=HTMLDOC_header, value="$L{title}")
> }
> )
> public ServiceResult someMethod(
> RestRequest request,
> RestResponse response,
> @Path(
> name="somePathParam",
> description="xxx"
> )
> String somePathParam) {
> ...
> }
>
OK, this is good - works for me and is a clean approach, thanks.
[...]
> As far as keeping @Parameter, I could re-add it using a separate
> "parameters2" attribute like so:
> @RestMethod(name="GET",
> swagger=@MethodSwagger(
> parameters2={
> @Parameter(....)
> }
> )
> )
>
> My concerns with that though are that we're defining two separate ways
> of defining free-form parameters. And the existing @Parameter
> annotation didn't get the Swagger spec exactly correct (it was an
> amalgam of "body" and the other parameter types which only has some
> overlap in Swagger).
This would still be a breaking change though? As it understand it,
@Parameter is already back in the 7.2.1 code, so as not to break old
code. For a future major release, I would support removing it. I agree
that we should not have too many ways of defining these params so it
should go entirely, as long as it's a major release with a release note
describing an 'old' and 'new' way of doing it, so migration is clear.
> I'm not sure why you're seeing that "@Path used without name or value"
> error. I'll see if I can reproduce that. The workaround is to add the
> name of your path parameter like so:
>
> public ServiceResult someMethod(
> RestRequest request,
> RestResponse response,
> @Path("somePathParam") String somePathParam) {
> ...
> }
This is my fault, sorry. Revisiting the code today I see that I had
left a method with broken syntax in the class (ironically that I'd added
to test out this problem); removing this fixed the problem.
Incidentally, I think there is a documentation error in
http://juneau.apache.org/site/apidocs/overview-summary.html#juneau-rest-server.HttpPartAnnotations.Path
One of the examples suggests 'required' is an allowed attribute of @Path
but this is not supported in the code:
// Normal
@Path(
name="name",
description="Pet name",
required=true,
example="Doggie"
)
(I think logically 'required' should not be supported, so it seems a
documentation rather than code error)
Thanks,
David
> On Thu, Oct 25, 2018 at 2:32 PM David Goddard <goddard@acm.org
> <ma...@acm.org>> wrote:
>
> Hi,
>
> I'm catching up with some of the 7.2.0 changes and I'm hitting some
> issues updating some pre-7.2.0 code (specifically I'm having problems
> similar to JUNEAU-85).
>
> I note that org.apache.juneau.rest.annotation.Parameter was removed
> from
> 7.2.0 as part of the Swagger changes (although not all of the
> documentation seems to have caught up:
> http://juneau.apache.org/site/apidocs/org/apache/juneau/rest/annotation/MethodSwagger.html#parameters--)
>
> So my original code was like:
>
> swagger = @MethodSwagger(
> parameters= {
> @Parameter(in="path", name="somePathParam", description="xxx"),
> }
> ),
>
> This would obviously fail in 7.2.0 as @Parameter is absent.
>
> However, I'm hitting some issues getting it working with the
> SimplifiedJson syntax described in
> http://juneau.apache.org/site/apidocs/org/apache/juneau/rest/annotation/RestMethod.html#swagger--,
>
> which is of this format as per the docs:
>
> swagger={
> "parameters:[",
> "{name:'propertyName',in:'path',description:'The system
> property
> name.'},",
> "{in:'body',description:'The new system property value.'}",
> "],",
> "responses:{",
> "302: {headers:{Location:{description:'The root URL of this
> resource.'}}},",
> "403: {description:'User is not an admin.'}",
> "}"
> }
>
> So my method updates to something like this:
>
> @RestMethod(name="GET", path="/foo/{somePathParam}/*",
> guards=FooGuard.class,
> swagger = {
> "parameters:[",
>
> "{name:'somePathParam',in:'path',description:'xxx',required:true}",
> "],"
> },
> properties = { // some props }
> )
> public ServiceResult someMethod(RestRequest request, RestResponse
> response, @Path String somePathParam) {
> // etc.
> }
>
> However, this gives me a syntax error in Eclipse:
>
> Type mismatch: cannot convert from String[] to MethodSwagger
>
> Any idea what I'm doing wrong here? (I am assuming I'm doing something
> wrong)
>
> Looking at the Pet Store example code, I see a different syntax in
> PetStoreResource, leading me to change my method to this:
>
> @RestMethod(name="GET",
> path="/foo/{somePathParam}/*",
> properties ={
> @Property(name=HTMLDOC_header, value="$L{title}"),
> },
> swagger = @MethodSwagger(
> parameters= {
> "{name:'somePathParam',in:'path',description:'xxx'}",
> }
> )
> )
> public ServiceResult someMethod(RestRequest request,
> RestResponse response,
> @Path String somePathParam) {
> return null; //(Not really)
> }
>
> Do we have a documentation issue, or an understanding issue on my part?
>
> However, in any case I get a runtime error at the moment:
>
> #######
> Exception occurred while initializing method
> 'some.package.ServiceREST.someMethod'
> ...
> Caused by: @Path used without name or value on method
> 'some.package.ServiceREST.someMethod(RestRequest,RestResponse,String)'
> parameter '2'.
> #######
>
> What am I missing?
>
> ....
>
> More generally, @Parameter is back in 7.2.1-RC, but it's unclear to me
> at least what the direction is for this. Are we now looking to remove
> these annotations again in a future (major) release, with the favoured
> approach being the SimplifiedJson syntax? However, @Parameter is not
> marked as deprecated in the current RC. I don't get a clear sense from
> the docs which way this is going (or indeed where it exactly is now).
>
> I'd agree with Gary's sentiment from JUNEAU-85 that 7.1 -> 7.x
> should be
> a non-breaking change, but it is the case that having multiple
> concurrently valid techniques is confusing at present (at least to me);
> this could be a documentation fix though.
>
> Thanks
>
> David
>
>
Re: Swagger in RestMethod (7.2.0 -> 7.2.1 and onwards)
Posted by James Bognar <ja...@gmail.com>.
Hi David,
I could use some guidance here actually on whether to keep the @Parameter
annotation. The plan was to deprecate it, but I'm willing to keep it in.
The "new" approach is to annotate your parameters on the method arguments
like so:
@RestMethod(name="GET",
path="/foo/{somePathParam}/*",
properties ={
@Property(name=HTMLDOC_header, value="$L{title}")
}
)
public ServiceResult someMethod(
RestRequest request,
RestResponse response,
@Path(
name="somePathParam",
description="xxx"
)
String somePathParam) {
...
}
The @MethodSwagger(parameters) annotation is thus really not needed
anymore, but the Simplified-JSON syntax for defining it is provided as a
free-form way of doing so. The @Parameter annotation didn't include all
possible Swagger values, so this was a way to "keep up with the spec" if
you wanted to define the parameters in the @MethodSwagger like so:
@RestMethod(name="GET",
swagger=@MethodSwagger(
parameters={
"{name:'somePathParam',in:'path',description:'xxx'}"
}
)
)
However, parameters defined like this are considered "documentation only",
meaning you don't get auto-validation of parameter values. That's actually
part of the design because it allows you to provide either validating or
non-validating Swagger. For example,
// Validating
@Path(
name="somePathParam",
description="xxx",
minLength=10, maxLength=20
)
// Non-validating
@RestMethod(name="GET",
swagger=@MethodSwagger(
parameters={
"{name:'somePathParam',in:'path',description:'xxx',minLength:10,maxLength:20}"
}
)
)
As far as keeping @Parameter, I could re-add it using a separate
"parameters2" attribute like so:
@RestMethod(name="GET",
swagger=@MethodSwagger(
parameters2={
@Parameter(....)
}
)
)
My concerns with that though are that we're defining two separate ways of
defining free-form parameters. And the existing @Parameter annotation
didn't get the Swagger spec exactly correct (it was an amalgam of "body"
and the other parameter types which only has some overlap in Swagger).
I'm not sure why you're seeing that "@Path used without name or value"
error. I'll see if I can reproduce that. The workaround is to add the
name of your path parameter like so:
public ServiceResult someMethod(
RestRequest request,
RestResponse response,
@Path("somePathParam") String somePathParam) {
...
}
On Thu, Oct 25, 2018 at 2:32 PM David Goddard <go...@acm.org> wrote:
> Hi,
>
> I'm catching up with some of the 7.2.0 changes and I'm hitting some
> issues updating some pre-7.2.0 code (specifically I'm having problems
> similar to JUNEAU-85).
>
> I note that org.apache.juneau.rest.annotation.Parameter was removed from
> 7.2.0 as part of the Swagger changes (although not all of the
> documentation seems to have caught up:
>
> http://juneau.apache.org/site/apidocs/org/apache/juneau/rest/annotation/MethodSwagger.html#parameters--
> )
>
> So my original code was like:
>
> swagger = @MethodSwagger(
> parameters= {
> @Parameter(in="path", name="somePathParam", description="xxx"),
> }
> ),
>
> This would obviously fail in 7.2.0 as @Parameter is absent.
>
> However, I'm hitting some issues getting it working with the
> SimplifiedJson syntax described in
>
> http://juneau.apache.org/site/apidocs/org/apache/juneau/rest/annotation/RestMethod.html#swagger--,
>
> which is of this format as per the docs:
>
> swagger={
> "parameters:[",
> "{name:'propertyName',in:'path',description:'The system property
> name.'},",
> "{in:'body',description:'The new system property value.'}",
> "],",
> "responses:{",
> "302: {headers:{Location:{description:'The root URL of this
> resource.'}}},",
> "403: {description:'User is not an admin.'}",
> "}"
> }
>
> So my method updates to something like this:
>
> @RestMethod(name="GET", path="/foo/{somePathParam}/*",
> guards=FooGuard.class,
> swagger = {
> "parameters:[",
>
> "{name:'somePathParam',in:'path',description:'xxx',required:true}",
> "],"
> },
> properties = { // some props }
> )
> public ServiceResult someMethod(RestRequest request, RestResponse
> response, @Path String somePathParam) {
> // etc.
> }
>
> However, this gives me a syntax error in Eclipse:
>
> Type mismatch: cannot convert from String[] to MethodSwagger
>
> Any idea what I'm doing wrong here? (I am assuming I'm doing something
> wrong)
>
> Looking at the Pet Store example code, I see a different syntax in
> PetStoreResource, leading me to change my method to this:
>
> @RestMethod(name="GET",
> path="/foo/{somePathParam}/*",
> properties ={
> @Property(name=HTMLDOC_header, value="$L{title}"),
> },
> swagger = @MethodSwagger(
> parameters= {
> "{name:'somePathParam',in:'path',description:'xxx'}",
> }
> )
> )
> public ServiceResult someMethod(RestRequest request,
> RestResponse response,
> @Path String somePathParam) {
> return null; //(Not really)
> }
>
> Do we have a documentation issue, or an understanding issue on my part?
>
> However, in any case I get a runtime error at the moment:
>
> #######
> Exception occurred while initializing method
> 'some.package.ServiceREST.someMethod'
> ...
> Caused by: @Path used without name or value on method
> 'some.package.ServiceREST.someMethod(RestRequest,RestResponse,String)'
> parameter '2'.
> #######
>
> What am I missing?
>
> ....
>
> More generally, @Parameter is back in 7.2.1-RC, but it's unclear to me
> at least what the direction is for this. Are we now looking to remove
> these annotations again in a future (major) release, with the favoured
> approach being the SimplifiedJson syntax? However, @Parameter is not
> marked as deprecated in the current RC. I don't get a clear sense from
> the docs which way this is going (or indeed where it exactly is now).
>
> I'd agree with Gary's sentiment from JUNEAU-85 that 7.1 -> 7.x should be
> a non-breaking change, but it is the case that having multiple
> concurrently valid techniques is confusing at present (at least to me);
> this could be a documentation fix though.
>
> Thanks
>
> David
>
>
>