Another issue is that the HTTP 1.1 defined set of status codes is
extensible and I wouldn't want to litter the enum with lots of unused
values just in case someone comes up with some new code or class of
codes like WebDAV does:
http://www.webdav.org/specs/rfc4918.html#status.code.extensions.to.http11
Maybe we should drop the ClientResponse.getStatus method in favor of
the alternative that just returns an int ?
Marc.
On Jan 19, 2009, at 8:56 AM, Paul Sandoz wrote:
>
> On Jan 19, 2009, at 2:43 PM, Gili wrote:
>
>>
>> Why can't #2 be done? It would be a backwards compatible change.
>
> Because i am not sure we can add such things to a maintenance
> release. Need to discuss with Marc. Not sure it is completely
> backwards compatible because one is adding new cases. Existing code
> may assume that it has covered all cases.
>
> Paul.
>
>>
>> Long term this is the direction we want to go anyway.
>>
>> Gili
>>
>> Paul Sandoz (via Nabble) wrote:
>>
>> > HI Gili,
>> >
>> > Unfortunately enum classes cannot be extended.
>> >
>> > I think the cleanest solution is one of the following:
>> >
>> > 1) Use a new Jersey enum class that contains all known status
>> > codes; or
>> >
>> > 2) Update JSR 311 to declare all known status codes.
>> >
>> > where the client API uses one or the other depending on which gets
>> > implemented.
>> >
>> > I am not sure we can do 2) because of the nature of the change.
>> Thus i
>> > think 1) may be the better solution.
>> >
>> > Paul.
>> >
>> >
>> > On Jan 19, 2009, at 5:32 AM, Gili wrote:
>> >
>> > >
>> > > As we discussed before, Response.Status is missing quite a few
>> > > status codes.
>> > > As a result, it isn't rare to see ClientResponse.getStatus()
>> return
>> > > null.
>> > > This is problematic because we then end up with the following
>> code:
>> > >
>> > > if (response.getStatus()==null)
>> > > throw new AssertionException("unexpected return code: " +
>> > > response.getStatus());
>> > > switch(response.getStatus())
>> > > {
>> > > case NOT_FOUND:
>> > > ...
>> > > case NOT_MODIFIED:
>> > > ...
>> > > default:
>> > > throw new AssertionException("unexpected return code: " +
>> > > response.getStatus());
>> > > }
>> > >
>> > >
>> > > 1) We have to check for "unexpected return codes" twice.
>> > > 2) Ideally we want to use enum-style return codes inside switch
>> > > statements
>> > > but we end up having to use integer return codes instead because
>> > > Response.Status is missing a lot of important codes.
>> > > 3) Response.Status.fromStatusCode(int) returns null while
>> > > Response.Status.valueOf() throws IllegalArgumentException. This
>> is
>> > > inconsistent in my opinion. Why doesn't
>> > > Response.Status.fromStatusCode(int)
>> > > throw IllegalArgumentException as well?
>> > >
>> > > Whatever solution we come up with has to:
>> > >
>> > > 1) Provide enums for all HTTP return codes
>> > > 2) Handle unexpected return codes using a single switch branch
>> > > 3) Retain backwards compatibility as new HTTP codes are added
>> in the
>> > > future
>> > >
>> > > What do you guys suggest?
>> > >
>> > > My proposal is:
>> > >
>> > > 1) Update JSR311 so all HTTP codes show up in Response.Status, or
>> > > define a
>> > > new ClientStatus that would do the same and have jersey return
>> that
>> > > instead.
>> > > 2) ClientStatus.fromStatusCode(int) will need to return some
>> catch-
>> > > all enum
>> > > value (say STATUS_UNKNOWN) on unknown status codes. Developers
>> will be
>> > > instructed to never catch this code, instead they should catch
>> > > "default:".
>> > > This will allow us to add new enum values in the future without
>> > > breaking
>> > > backwards compatibility.
>> > >
>> > > This is far from perfect but it's the best I could come up
>> with...
>> > >
>> > > Gili
>> > > --
>> > > View this message in context:
>> > http://n2.nabble.com/ClientResponse.getStatus%28%29-shouldn%27t-return-null-tp2179833p2179833.html
>> > > Sent from the Jersey mailing list archive at Nabble.com.
>> > >
>> > >
>> > >
>> ---------------------------------------------------------------------
>> > > To unsubscribe, e-mail: users-unsubscribe@...
>> > <http://n2.nabble.com/user/SendEmail.jtp?
>> type=node&node=2180823&i=0>
>> > > For additional commands, e-mail: users-help@...
>> > <http://n2.nabble.com/user/SendEmail.jtp?
>> type=node&node=2180823&i=1>
>> > >
>> >
>> >
>> >
>> ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: users-unsubscribe@...
>> > <http://n2.nabble.com/user/SendEmail.jtp?
>> type=node&node=2180823&i=2>
>> > For additional commands, e-mail: users-help@...
>> > <http://n2.nabble.com/user/SendEmail.jtp?
>> type=node&node=2180823&i=3>
>> >
>> >
>> >
>> >
>> ------------------------------------------------------------------------
>> > This email is a reply to your post @
>> > http://n2.nabble.com/ClientResponse.getStatus%28%29-shouldn%27t-return-null-tp2179833p2180823.html
>> > You can reply by email or by visting the link above.
>> >
>>
>> View this message in context: Re: [Jersey]
>> ClientResponse.getStatus() shouldn't return null
>> Sent from the Jersey mailing list archive at Nabble.com.
>
---
Marc Hadley <marc.hadley at sun.com>
CTO Office, Sun Microsystems.