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 > 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 > >