Hi Paul,
Sorry for the late reply I've been really busy today.
I should have been more specific in my e-mail. Requests are thread-safe
you can have multiple connections and they should be handled in a
thread-safe manner. Multiple-threads can share connections in fact. But
you're right, you can't change the configuration in a thread-safe manner.
I was under the impression, from reading the JavaDocs, that this was also
the case with the URLConnection backend so I didn't worry to much about
it.
Can you give me an example of when a WebResource would configure a
property? I'm just curious if this is a common case.
-jOrGe W.
Paul Sandoz <Paul.Sandoz_at_Sun.COM>
Sent by: Paul.Sandoz_at_Sun.COM
11/25/2008 05:08 AM
Please respond to
users_at_jersey.dev.java.net
To
users_at_jersey.dev.java.net
cc
Subject
Re: [Jersey] Patch: HttpClient backend to Jersey Client
Hi Jorge,
I have made some more changes along what we agreed.
Are you sure the ApacheHttpClientHandler is thread safe?
The reason is properties on the HttpClient instance are set by the
handler. A WebResource can modify the properties configured from the
client configuration so the properties are not guaranteed to be the
same for each and every invocation of handle. So we may need to
rethink how we go about this integration and perhaps change the client
API to support this more efficiently.
Paul.
On Nov 21, 2008, at 5:27 PM, Paul Sandoz wrote:
> I have done an initial commit but there are no changes yet w.r.t.
> modifications of the API as we have discussed.
>
> I look the liberty of creating a separate module in the contribs
> section and rearranged some of the package/class names.
>
> I also wanted to explore something like this:
>
> Client c = ApacheHttpClient.create();
>
> so that one does not have to refer to directly to the handler and
> the pattern is similar.
>
> Paul.
>
> On Nov 21, 2008, at 3:18 PM, Jorge L. Williams wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>>> On Nov 20, 2008, at 6:30 PM, Jorge L Williams wrote:
>>>
>>>>
>>>>
>>>> Paul.Sandoz_at_Sun.COM wrote on 11/20/2008 08:08:05 AM:
>>>>
>>>>>
>>>>> On Nov 20, 2008, at 3:42 PM, Jorge L. Williams wrote:
>>>>>
>>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>>> Hash: SHA1
>>>>>>
>>>>>> Paul Sandoz wrote:
>>>>>>> Hi Jorge,
>>>>>>>
>>>>>>> I have looked more closely at the patch, some comments and
>>>> proposed
>>>>>>> improvements:
>>>>>>>
>>>>>>> - Alex pointed out to me that by default interactive prompting
>>>> for
>>>>>>> credentials occurs. I think the default value for
>>>>>>> PROPERTY_INTERACTIVE and PROPERTY_PREEMPTIVE_AUTHENTICATION
>>>> should
>>>>>>> be false, since by client API is likely to be used in
>>>>>>> non-interactive environments.
>>>>>>>
>>>>>>
>>>>>> The default for PROPERTY_PREEMPTIVE_AUTHENTICATION is already
>>>> false.
>>>>>>
>>>>>> As for, PROPERTY_INTERACTIVE, I distinctly remember that
>>>> prompting
>>>>>> for credentials was the default behavior when using
>>>> URLConnection and
>>>>>> I wanted to match the behavior in case people were relying on
>>>> it. Now
>>>>>> that I look at the documentation, I guess I must have dreamt
>>>>>> that
>>>>>> behavior because I can't find a reference to it. Keep in mind
>>>> that,
>>>>>> if you set interactive to true, but also set the credentials in
>>>> the
>>>>>> Config you will *not* be prompt.
>>>>>
>>>>> OK.
>>>>>
>>>>>
>>>>
>>>> Just to clarify: Does this mean you're okay with the current
>>>> default
>>>> of setting interactive to true??
>>>>
>>>
>>> No. It means that i understood what you said.
>>>
>>>
>>
>> Okay, gotcha.
>>>>
>>>>>> None the less, I really don't have
>>>>>> a problem with setting the default to false.
>>>>>>
>>>>>>> - Rename: PROPERTY_INTERACTIVE ->
>>>>>>> PROPERTY_AUTHENTICATION_INTERACTIVE
>>>>>>> PROPERTY_PREEMPTIVE_AUTHENTICATION ->
>>>>>>> PROPERTY_AUTHENTICATION_PREEMPTIVE
>>>>>>>
>>>>>> Good idea, no problem.
>>>>>>
>>>>>>> - Change the properties PROPERTY_PROXY_SET, PROPERTY_PROXY_HOST,
>>>>>>> PROPERTY_PROXY_PORT to one property PROPERTY_PROXY that is a
>>>> string
>>>>>>> or URI, which if the port is absent then 8080 is used.
>>>>>>>
>>>>>> Another good idea. Here, again, I was trying to match the
>>>> behavior
>>>>>> of the java.net package.
>>>>>>> - Have a method HttpClientConfig.getHttpState() thus the
>>>>>>> credential-based methods could be re-factored to a helper class
>>>> or
>>>>>>> just reside on the DefaultHttpClientConfig.
>>>>>>>
>>>>>> Okay, here I disagree. I really don't like the developer having
>>>> to
>>>>>> know anything about HttpClient in the typical case. Not
>>>> having to
>>>>>> visit the Javadocs for the HttpClient api just makes the backend
>>>>>> easier to use. I think setting credentials will occur quite
>>>> frequently
>>>>>> so I'm a little reluctant to have the user have to manage an
>>>> HttpState
>>>>>> or have to know anything about it to work with the ClientHandler.
>>>>>>
>>>>>> Of course, I'm open to the idea, but I guess I don't understand
>>>> why
>>>>>> you're making this suggestion(?) Is it because you feel it will
>>>>>> address a security issue? Please explain.
>>>>>
>>>>> I am guessing that there may be other ways to configure the
>>>> HttpState
>>>>> beyond the common cases.
>>>>>
>>>>> A way to get or set the HttpState might be useful. A filter could
>>>>> create/use a different HttpState and set it on the properties
>>>>> e.g. i
>>>>> could have credentials set up for a particular WebResource using a
>>>>> filter to add the HttpState to the properties.
>>>>>
>>>>> I was not proposing to remove the methods or for the ClientHandler
>>>> to
>>>>> be utilized. I think they are useful, and maybe so in a different
>>>>> context.
>>>>>
>>>>>
>>>>
>>>> Okay. So long as we keep the simple credential setting methods on
>>>> HttpClientConfig, then I don't have a problem exposing the
>>>> HttpState
>>>> through a property or by adding a getHttpState() method. Keep in
>>>> mind that getHttpState() and the property PROPERTY_HTTP_STATE
>>>> already exists in DefaultHttpClientConfig -- we'll just need to
>>>> move
>>>> these over.
>>>>
>>>
>>> What about instead if we have a class HttpClientState that
>>> encapsulates HttpState and has those helper methods on it. Then we
>>> can
>>> expose a property for HttpClientState and getters/setters. That
>>> way it
>>> is more resuable and with a bit of JavaDoc we can easily reference
>>> how
>>> to set credentials, for example:
>>>
>>> HttpClientConfig.getState().setCredentials(...);
>>>
>>>
>>>
>>
>>
>> That sounds reasonable. I'm okay with that.
>>
>>>>
>>>>>>
>>>>>>> - I think we need to implement a RequestEntity specific to
>>>> writing
>>>>>>> so that we do not buffer the request.
>>>>>>>
>>>>>> We are in accord on this.
>>>>>>
>>>>>> I'm spread a bit thin these days and won't be able to address
>>>> these
>>>>>> issues until sometime next week.
>>>>>>
>>>>>
>>>>>
>>>>> I will try and do something tomorrow,
>>>>> Paul.
>>>>>
>>>>
>>>> Thank you. Keep me posted, and if there's a chance I'll contribute
>>>> some next week.
>>>>
>>>
>>> Some unit tests would be most helpful.
>> Can't make promises, but I'll see if I can fit that in.
>>
>>
>> - -jOrGe W.
>>
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v2.0.9 (GNU/Linux)
>>
>> iEYEARECAAYFAkkmwx0ACgkQ72r0i/n44hOmTgCgnmuxKlQEbNSp744L2Rv67DTH
>> dPwAn2N1bmWYbGr18o6RVNcMZaVaJuCI
>> =ovQ0
>> -----END PGP SIGNATURE-----
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
>> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
For additional commands, e-mail: users-help_at_jersey.dev.java.net