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