juneau-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Bognar <jamesbog...@apache.org>
Subject Re: Swagger in RestMethod (7.2.0 -> 7.2.1 and onwards)
Date Fri, 26 Oct 2018 14:22:37 GMT
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 <goddard@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
> > <mailto:goddard@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
> >
> >
>
>

Mime
View raw message