HttpComponents HttpClient
  1. HttpComponents HttpClient
  2. HTTPCLIENT-1238

Contribute Bundle Activator And Central Proxy Configuration

    Details

    • Type: Wish Wish
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 4.2.1
    • Fix Version/s: 4.3 Beta2
    • Component/s: HttpClient
    • Labels:
      None

      Description

      as discussed at [0] i'd like to contribute the bundle activator and central proxy configuration.

      the attached patch may need some cleanup on your side, as only assumed locations where to put some classes or in which pom.xml to put dependencies.

      i kindly ask you to review the patch in if possible integrate it in a future release.

      Adobe (and i as its employee) is available for assistance, explanations, or further dev work required in the context of the patch.

      [0] http://mail-archives.apache.org/mod_mbox/hc-dev/201209.mbox/%3cCACMijv-21S4+Jw_A=jDFHeVB9cMt4KNi5p3jAHZFh3kvu4btdw@mail.gmail.com%3e

      1. HTTPCLIENT-1238.patch
        20 kB
        Dominique Jäggi
      2. HTTPCLIENT-1238-2.patch
        55 kB
        Dominique Jäggi
      3. HTTPCLIENT-1238-3.patch
        55 kB
        Dominique Jäggi
      4. HTTPCLIENT-1238-4.patch
        67 kB
        Simone Tripodi
      5. HTTPCLIENT-1238-5.patch
        67 kB
        Simone Tripodi
      6. HTTPCLIENT-1238-6.patch
        66 kB
        Simone Tripodi

        Activity

        Hide
        Simone Tripodi added a comment -

        Hey Oleg Kalnichevski thanks a lot for the kind proposal, very appreciated!
        I think that becoming a HC committer would help me on keeping the OSGi documentation in sync - I am already an ASF member, my id is simonetripodi, that should help on speeding the procedures up...
        Thanks once again for the proposal, you made my day!

        Show
        Simone Tripodi added a comment - Hey Oleg Kalnichevski thanks a lot for the kind proposal, very appreciated! I think that becoming a HC committer would help me on keeping the OSGi documentation in sync - I am already an ASF member, my id is simonetripodi , that should help on speeding the procedures up... Thanks once again for the proposal, you made my day!
        Hide
        Oleg Kalnichevski added a comment -

        Hi Simone.
        If you are interested in HC committership, I can immediately put your candidacy on vote. The whole process is likely to require up to a week, though. If not, a patch against the latest SVN trunk will do just fine.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Hi Simone. If you are interested in HC committership, I can immediately put your candidacy on vote. The whole process is likely to require up to a week, though. If not, a patch against the latest SVN trunk will do just fine. Oleg
        Hide
        Simone Tripodi added a comment -

        Hi there Oleg Kalnichevski,
        of course I can contribute documentation as well - where/how can I check it in?
        TIA, all the best!

        Show
        Simone Tripodi added a comment - Hi there Oleg Kalnichevski , of course I can contribute documentation as well - where/how can I check it in? TIA, all the best!
        Hide
        Oleg Kalnichevski added a comment -

        @Dominique, @Simone: I am going to put HC 4.3 release on vote really soon (within 7 to 10 days). This is the last chance to make any adjustments to the new APIs. Another thing is that none of present HC committers appears to be a big time OSGi user. It would be awesome if you could contribute some basic documentation (javadocs at least, and ideally a short OSGi tutorial / user guide).

        Oleg

        Show
        Oleg Kalnichevski added a comment - @Dominique, @Simone: I am going to put HC 4.3 release on vote really soon (within 7 to 10 days). This is the last chance to make any adjustments to the new APIs. Another thing is that none of present HC committers appears to be a big time OSGi user. It would be awesome if you could contribute some basic documentation (javadocs at least, and ideally a short OSGi tutorial / user guide). Oleg
        Hide
        Morten Christensen added a comment -

        It would be nice with some documentation of what the osgi-specific modules does different from the non-osgi httpclient/core modules.

        Show
        Morten Christensen added a comment - It would be nice with some documentation of what the osgi-specific modules does different from the non-osgi httpclient/core modules.
        Hide
        Sebb added a comment -

        Could you create a JIRA for this?

        If you have a full implementation, great, otherwise just specify the method API and Javadoc, ideally with some examples.

        Show
        Sebb added a comment - Could you create a JIRA for this? If you have a full implementation, great, otherwise just specify the method API and Javadoc, ideally with some examples.
        Hide
        Simone Tripodi added a comment -

        Just a minor note: I just had a look at the InetAddressUtils class, looks like it only has a method to check if input strings are IPv4 or IPv6 compliant, while in the OSGiHttpRoutePlanner class we need to calculate the subnet mask, accessing to single IP elements

        Show
        Simone Tripodi added a comment - Just a minor note: I just had a look at the InetAddressUtils class, looks like it only has a method to check if input strings are IPv4 or IPv6 compliant, while in the OSGiHttpRoutePlanner class we need to calculate the subnet mask, accessing to single IP elements
        Hide
        Simone Tripodi added a comment -

        thanks a lot Oleg for checking in our contribution, that is really a very good news that makes me happy!

        I'll open a new issue about InetAddressUtils usage - and thanks a lot also for checkin the code format

        Simo

        Show
        Simone Tripodi added a comment - thanks a lot Oleg for checking in our contribution, that is really a very good news that makes me happy! I'll open a new issue about InetAddressUtils usage - and thanks a lot also for checkin the code format Simo
        Hide
        Oleg Kalnichevski added a comment -

        Patch committed to SVN trunk with some minor formatting changes. Many thanks for contributing it to all people involved and especially to Simone (sorry about referring to you by the wrong id).

        There is one thing I noticed that probably could be optimized. OSGiHttpRoutePlanner makes use of some regular expressions to match IPv4 addresses. It probably could re-use methods from InetAddressUtils.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Patch committed to SVN trunk with some minor formatting changes. Many thanks for contributing it to all people involved and especially to Simone (sorry about referring to you by the wrong id). There is one thing I noticed that probably could be optimized. OSGiHttpRoutePlanner makes use of some regular expressions to match IPv4 addresses. It probably could re-use methods from InetAddressUtils. Oleg
        Hide
        Simone Tripodi added a comment -

        The latest attached proposal integrates Oleg's suggestions - moreover I verified that deploying the HttpCore OSGi bundle first, it correctly exports all required packages by HttpClient OSG bundle, so I reverted my fixes and no actions are required.

        Like always, suggestions/comments/feedbacks/hints/... are very appreciated!

        TIA, all the best!

        Show
        Simone Tripodi added a comment - The latest attached proposal integrates Oleg's suggestions - moreover I verified that deploying the HttpCore OSGi bundle first, it correctly exports all required packages by HttpClient OSG bundle, so I reverted my fixes and no actions are required. Like always, suggestions/comments/feedbacks/hints/... are very appreciated! TIA, all the best!
        Hide
        Simone Tripodi added a comment -

        Thanks a lot Oleg Kalnichevski for reviewing my patch!

        let's discuss points one by one:

        (1) My understanding is that HttpClient OSGi bundle is dependent on HttpCore OSGi bundle and it is responsibility of HttpCore bundle to export core packages. Can this confusion be due to the fact that HttpClient OSGi does not correctly declare its dependency on HttpCore bundle?

        Sorry, my bad - I didn't notice there is an HttpCore OSGi bundle, that makes my patch invalid - the right way should be importing in the OSGi runtime environment the HttpCore OSGi bundle first, then the HttpClient OSGi one.
        I am honestly not a fan of declaring bundles dependencies in OSGi metadata, it is IMHO a nice workaround for particular situations - I'll make a test with the HttpCore bundle in my runtime and let you know

        (2) HttpClients interface sounds like a misnomer to me. This looks more like HttpClientBuilderFactory. The name is uglier but more precise

        Agreed, I didn't find a better name for that interface indeed and your feedback is much more than appreciated!

        (3) Can we put something in the name of the new classes to stress out their purpose and make them more distinguishable? DefaultHttpRoutePlanner -> OSGiHttpRoutePlanner, DefaultCredentialsProvider -> OSGiCredentialsProvider?

        Yes sure, I think so, even if, since they are already under osgi package, I was looking for a less redundant name, but having that taxonomy works for me

        Shall I commit the patch as is or do you want to submit another patch?

        I think it is reasonable I submit a new patch, applying your feedbacks

        Thanks!!!
        -Simo

        PS: I'd prefer be mentioned with the simone.tripodi id, simao is a mistake I was not able to delete

        Show
        Simone Tripodi added a comment - Thanks a lot Oleg Kalnichevski for reviewing my patch! let's discuss points one by one: (1) My understanding is that HttpClient OSGi bundle is dependent on HttpCore OSGi bundle and it is responsibility of HttpCore bundle to export core packages. Can this confusion be due to the fact that HttpClient OSGi does not correctly declare its dependency on HttpCore bundle? Sorry, my bad - I didn't notice there is an HttpCore OSGi bundle, that makes my patch invalid - the right way should be importing in the OSGi runtime environment the HttpCore OSGi bundle first, then the HttpClient OSGi one. I am honestly not a fan of declaring bundles dependencies in OSGi metadata, it is IMHO a nice workaround for particular situations - I'll make a test with the HttpCore bundle in my runtime and let you know (2) HttpClients interface sounds like a misnomer to me. This looks more like HttpClientBuilderFactory. The name is uglier but more precise Agreed, I didn't find a better name for that interface indeed and your feedback is much more than appreciated! (3) Can we put something in the name of the new classes to stress out their purpose and make them more distinguishable? DefaultHttpRoutePlanner -> OSGiHttpRoutePlanner, DefaultCredentialsProvider -> OSGiCredentialsProvider? Yes sure, I think so, even if, since they are already under osgi package, I was looking for a less redundant name, but having that taxonomy works for me Shall I commit the patch as is or do you want to submit another patch? I think it is reasonable I submit a new patch, applying your feedbacks Thanks!!! -Simo PS: I'd prefer be mentioned with the simone.tripodi id, simao is a mistake I was not able to delete
        Hide
        Oleg Kalnichevski added a comment -

        Simone Tripodi The patch is exactly what I felt what was the right approach. All OSGi specific aspects are kept inside the OSGi module without spilling any of its specific aspects into other modules of HttpClient. Great gob!

        Minor nitpicks
        (1) My understanding is that HttpClient OSGi bundle is dependent on HttpCore OSGi bundle and it is responsibility of HttpCore bundle to export core packages. Can this confusion be due to the fact that HttpClient OSGi does not correctly declare its dependency on HttpCore bundle?
        (2) HttpClients interface sounds like a misnomer to me. This looks more like HttpClientBuilderFactory. The name is uglier but more precise
        (3) Can we put something in the name of the new classes to stress out their purpose and make them more distinguishable? DefaultHttpRoutePlanner -> OSGiHttpRoutePlanner, DefaultCredentialsProvider -> OSGiCredentialsProvider?

        Shall I commit the patch as is or do you want to submit another patch?

        Oleg

        Show
        Oleg Kalnichevski added a comment - Simone Tripodi The patch is exactly what I felt what was the right approach. All OSGi specific aspects are kept inside the OSGi module without spilling any of its specific aspects into other modules of HttpClient. Great gob! Minor nitpicks (1) My understanding is that HttpClient OSGi bundle is dependent on HttpCore OSGi bundle and it is responsibility of HttpCore bundle to export core packages. Can this confusion be due to the fact that HttpClient OSGi does not correctly declare its dependency on HttpCore bundle? (2) HttpClients interface sounds like a misnomer to me. This looks more like HttpClientBuilderFactory. The name is uglier but more precise (3) Can we put something in the name of the new classes to stress out their purpose and make them more distinguishable? DefaultHttpRoutePlanner -> OSGiHttpRoutePlanner, DefaultCredentialsProvider -> OSGiCredentialsProvider? Shall I commit the patch as is or do you want to submit another patch? Oleg
        Hide
        Simone Tripodi added a comment -

        Thanks a lot Felix Meschberger for reviewing the patch!

        I just applied the modifications you suggested in a new patch which exports the services only package, where I moved both HttpClients and ProxyConfiguration, then moved the HttpProxyConfigurationActivator in the impl package - that should be more canonical according to OSGi best practices!

        Show
        Simone Tripodi added a comment - Thanks a lot Felix Meschberger for reviewing the patch! I just applied the modifications you suggested in a new patch which exports the services only package, where I moved both HttpClients and ProxyConfiguration , then moved the HttpProxyConfigurationActivator in the impl package - that should be more canonical according to OSGi best practices!
        Hide
        Felix Meschberger added a comment -

        Simone Tripodi If I interpret the patch correctly, the osgi package with the BundleActivator and the ProxyConfig interfaces is exported but the osgi.services patch with the HttpClients service interface is not exported. I think the HttpClients service interface must be exported for it to be used. On the other hand the BundleActivator should not be exported and can as well be moved to the osgi.impl package.

        Show
        Felix Meschberger added a comment - Simone Tripodi If I interpret the patch correctly, the osgi package with the BundleActivator and the ProxyConfig interfaces is exported but the osgi.services patch with the HttpClients service interface is not exported. I think the HttpClients service interface must be exported for it to be used. On the other hand the BundleActivator should not be exported and can as well be moved to the osgi.impl package.
        Hide
        Simone Tripodi added a comment -

        Oh, forgot to mention that the bundle keeps track of all HttpClient instances created by the custom builder, in order that when shutting it down it tries to close all active clients.

        Show
        Simone Tripodi added a comment - Oh, forgot to mention that the bundle keeps track of all HttpClient instances created by the custom builder, in order that when shutting it down it tries to close all active clients.
        Hide
        Simone Tripodi added a comment -

        Hi all/Oleg,

        please find in attachment a new proposal which makes the Proxy Configuration feature, as requested, less intrusive in the core codebase.

        Two important notes:

        • OSGi users now can configure Proxy Configuration(s) via the MetaType standard service and, in order to obtain HttpClient instances have to request the org.apache.http.osgi.services.HttpClients service which is able to create OSGi-aware org.apache.http.impl.client.HttpClientBuilder, that at its time configures the HttpClient with org.apache.http.conn.routing.HttpRoutePlanner and org.apache.http.client.CredentialsProvider instances backed by a data structure able to provide route and credentials according to active Proxy configurations, all made transparent to final users.
        • The httpclient-osgi bundle was not a complete working bundle, when installing it in an OSGi runtime, it claimed the missing httpcore packages; so the patch provides the addition of missing dependency and OSGi exports

        Please let me know if something more is required in order to get that patch applied.
        Many thanks in advance, all the best!
        -Simo

        Show
        Simone Tripodi added a comment - Hi all/Oleg, please find in attachment a new proposal which makes the Proxy Configuration feature, as requested, less intrusive in the core codebase. Two important notes: OSGi users now can configure Proxy Configuration(s) via the MetaType standard service and, in order to obtain HttpClient instances have to request the org.apache.http.osgi.services.HttpClients service which is able to create OSGi-aware org.apache.http.impl.client.HttpClientBuilder , that at its time configures the HttpClient with org.apache.http.conn.routing.HttpRoutePlanner and org.apache.http.client.CredentialsProvider instances backed by a data structure able to provide route and credentials according to active Proxy configurations, all made transparent to final users. The httpclient-osgi bundle was not a complete working bundle, when installing it in an OSGi runtime, it claimed the missing httpcore packages; so the patch provides the addition of missing dependency and OSGi exports Please let me know if something more is required in order to get that patch applied. Many thanks in advance, all the best! -Simo
        Hide
        Oleg Kalnichevski added a comment -

        Dominique,
        I am pretty much done with the most radical changes for 4.3. I expect changes in the future to be more incremental. My suggest would be to use a custom, OSGi runtime aware HttpClient build which could construct HttpClient instances with a different route planner and would also maintain a global structure to keep track of all active connection pools. Please have a look at the caching module in the latest SVN snapshot as an example of a context specific (caching in this particular case) HttpClient implementations.

        Cheers

        Oleg

        Show
        Oleg Kalnichevski added a comment - Dominique, I am pretty much done with the most radical changes for 4.3. I expect changes in the future to be more incremental. My suggest would be to use a custom, OSGi runtime aware HttpClient build which could construct HttpClient instances with a different route planner and would also maintain a global structure to keep track of all active connection pools. Please have a look at the caching module in the latest SVN snapshot as an example of a context specific (caching in this particular case) HttpClient implementations. Cheers Oleg
        Hide
        Oleg Kalnichevski added a comment -

        Dominique
        Sorry about the delay. I am currently in the process of making significant changes in trunk (4.3) that directly impact the area you are working on. I was hoping to be able to commit my changes soon but right now it looks like it is going to take a few more weeks to complete. Please bear with me.

        One thing I can tell you right way though. Inclusion of any processing logic to HttpRoute class is a no go and a clear show stopper for me. HttpRoute should remain a plain java bean. Route computation logic should go into one of the HttpRoutePlanner implementations.

        I am also not very hot about maintaining a global structure to keep track of all connection manager instances. I understand it is clearly a good thing in the OSGi environment but am a little worried it may lead to GC problems in other managed environments or containers. We had such issues in the past. So, ideally I would prefer to have all connection manager tracking code moved to the OSGi module while HttpClient main module would provide an abstract callback mechanism without any implementation by default.

        Once again, I think things will get easier if you submit your changes in smaller chunks.

        Oleg

        PS: for all the pain of having to put up with my grumblings I'll happily compensate by buying you a drink or two next time you happen to be in Zurich.

        Show
        Oleg Kalnichevski added a comment - Dominique Sorry about the delay. I am currently in the process of making significant changes in trunk (4.3) that directly impact the area you are working on. I was hoping to be able to commit my changes soon but right now it looks like it is going to take a few more weeks to complete. Please bear with me. One thing I can tell you right way though. Inclusion of any processing logic to HttpRoute class is a no go and a clear show stopper for me. HttpRoute should remain a plain java bean. Route computation logic should go into one of the HttpRoutePlanner implementations. I am also not very hot about maintaining a global structure to keep track of all connection manager instances. I understand it is clearly a good thing in the OSGi environment but am a little worried it may lead to GC problems in other managed environments or containers. We had such issues in the past. So, ideally I would prefer to have all connection manager tracking code moved to the OSGi module while HttpClient main module would provide an abstract callback mechanism without any implementation by default. Once again, I think things will get easier if you submit your changes in smaller chunks. Oleg PS: for all the pain of having to put up with my grumblings I'll happily compensate by buying you a drink or two next time you happen to be in Zurich.
        Hide
        Sebb added a comment -

        A few comments from looking at the code in isolation:

        public static ClientConnectionManagerTracker instance; // should be private??
        ...
        public static ClientConnectionManagerTracker getTracker()

        Multiple threads can (re)create the tracker, so it won't be a singleton.
        If that does not matter, it should be documented.
        The field is public so can be updated independently - mutable static fields are inherently not thread-safe (in fact they are generally thread-hostile).

        If it is intended to be a singleton, the code should sync the getTracker() method, or use an IODH.

        The same remarks apply to ConfigurableProxySelector.

        The ProxyConfiguration interface needs Javadoc for the methods.
        The failed() method in particular needs to be carefully documented as it has side effects.

        Javadoc elsewhere is also lacking.

        TransparentProxySelector.FAILURES should be final.

        PropertiesUtil has @since 2.1, which looks wrong.

        Not clear why the code throws UndeclaredThrowableException; this should be documented.

        Sorry, cannot comment on the code in relation to HttpComponents itself.

        Show
        Sebb added a comment - A few comments from looking at the code in isolation: public static ClientConnectionManagerTracker instance; // should be private?? ... public static ClientConnectionManagerTracker getTracker() Multiple threads can (re)create the tracker, so it won't be a singleton. If that does not matter, it should be documented. The field is public so can be updated independently - mutable static fields are inherently not thread-safe (in fact they are generally thread-hostile). If it is intended to be a singleton, the code should sync the getTracker() method, or use an IODH. The same remarks apply to ConfigurableProxySelector. The ProxyConfiguration interface needs Javadoc for the methods. The failed() method in particular needs to be carefully documented as it has side effects. Javadoc elsewhere is also lacking. TransparentProxySelector.FAILURES should be final. PropertiesUtil has @since 2.1, which looks wrong. Not clear why the code throws UndeclaredThrowableException; this should be documented. Sorry, cannot comment on the code in relation to HttpComponents itself.
        Hide
        Dominique Jäggi added a comment -

        could someone please review the patch?

        Show
        Dominique Jäggi added a comment - could someone please review the patch?
        Hide
        Dominique Jäggi added a comment -

        still, authentication support for the transparent proxies is missing - after your review, maybe you'd have some suggestions on how to properly implement that.

        Show
        Dominique Jäggi added a comment - still, authentication support for the transparent proxies is missing - after your review, maybe you'd have some suggestions on how to properly implement that.
        Hide
        Dominique Jäggi added a comment -

        attached latest version of patch.

        • httpclient-osgi now only contains bundle activator and metatype information, registering itself as a managed service factory for multiple proxy configs
        • httclient received client connection manager tracker and configurable proxy selector
        Show
        Dominique Jäggi added a comment - attached latest version of patch. httpclient-osgi now only contains bundle activator and metatype information, registering itself as a managed service factory for multiple proxy configs httclient received client connection manager tracker and configurable proxy selector
        Hide
        Oleg Kalnichevski added a comment -

        Dominique

        Speaking from personal experience both contributing and reviewing / accepting large change sets it is often much easier and productive to feed things in as a series of smaller incremental patches instead of trying to cover everything with one mega patch. Consider splitting things up into more granular change sets and submit least complicated / intrusive first.

        > for this i request your ideas, if it is at all worth the effort. non-system-properties and non-osgi configuration would most likely result in a new dependency

        Let's focus on system properties based / OSGi configuration first and see where it is going to take us.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Dominique Speaking from personal experience both contributing and reviewing / accepting large change sets it is often much easier and productive to feed things in as a series of smaller incremental patches instead of trying to cover everything with one mega patch. Consider splitting things up into more granular change sets and submit least complicated / intrusive first. > for this i request your ideas, if it is at all worth the effort. non-system-properties and non-osgi configuration would most likely result in a new dependency Let's focus on system properties based / OSGi configuration first and see where it is going to take us. Oleg
        Hide
        Dominique Jäggi added a comment - - edited

        currently there's a dependency on OSGi declarative services. the new patch should be rewritten to eliminate that need. a dependency on the configuration admin is essential though.

        Show
        Dominique Jäggi added a comment - - edited currently there's a dependency on OSGi declarative services. the new patch should be rewritten to eliminate that need. a dependency on the configuration admin is essential though.
        Hide
        Dominique Jäggi added a comment -

        > * central proxies can be configured outside OSGi (method to be determined)

        for this i request your ideas, if it is at all worth the effort. non-system-properties and non-osgi configuration would most likely result in a new dependency (e.g. apache commons configuration) or several dependencies (other config frameworks).

        Show
        Dominique Jäggi added a comment - > * central proxies can be configured outside OSGi (method to be determined) for this i request your ideas, if it is at all worth the effort. non-system-properties and non-osgi configuration would most likely result in a new dependency (e.g. apache commons configuration) or several dependencies (other config frameworks).
        Hide
        Dominique Jäggi added a comment -

        upon further consideration, i would like to create a third patch (making the previous two patches completely obsolete). the current approach has issues:

        • setting a proxy selector via a java.net.ProxySelector#setDefault() in the OSGi bundle is dangerous:
          • it sets the proxy selector JVM-wide, not only in the context of the application
          • if multiple instances of httpcomponents are used within the JVM, they overwrite each other's proxy selectors
        • a transparent, central proxy configuration should also be supported outside the context of OSGi (and not only via system properties - an obsolete way)

        thus the suggestions for the next patch are:

        • introduce central proxy configuration facility (setting of multiple proxies) - move proxy selection code from httpclient-osgi to httpclient
        • central proxies can be configured outside OSGi (method to be determined)
        • if running in an OSGi context, the OSGi bundle provides a configuration bridge to the OSGi configuration admin proxy configurations
        • configurations are looked for in the sequence: OSGi (if in container) - non-OSGi configuration - system properties
        • patch HttpClientBuilder to use a central proxy selector (non java.net.ProxySelector), using configurations from sequence above, setting proxies and proxy authentication

        as such please disregard the most recent attache patch and let me know your thoughts.

        Show
        Dominique Jäggi added a comment - upon further consideration, i would like to create a third patch (making the previous two patches completely obsolete). the current approach has issues: setting a proxy selector via a java.net.ProxySelector#setDefault() in the OSGi bundle is dangerous: it sets the proxy selector JVM-wide, not only in the context of the application if multiple instances of httpcomponents are used within the JVM, they overwrite each other's proxy selectors a transparent, central proxy configuration should also be supported outside the context of OSGi (and not only via system properties - an obsolete way) thus the suggestions for the next patch are: introduce central proxy configuration facility (setting of multiple proxies) - move proxy selection code from httpclient-osgi to httpclient central proxies can be configured outside OSGi (method to be determined) if running in an OSGi context, the OSGi bundle provides a configuration bridge to the OSGi configuration admin proxy configurations configurations are looked for in the sequence: OSGi (if in container) - non-OSGi configuration - system properties patch HttpClientBuilder to use a central proxy selector (non java.net.ProxySelector), using configurations from sequence above, setting proxies and proxy authentication as such please disregard the most recent attache patch and let me know your thoughts.
        Hide
        Oleg Kalnichevski added a comment -

        Dominique,

        I'll do a proper review of the patch in the coming days

        > handling proxy authentication - what would be the best location and way to inject the authentication needs of a proxy?

        Custom HttpRoutePlanner seems like the best option.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Dominique, I'll do a proper review of the patch in the coming days > handling proxy authentication - what would be the best location and way to inject the authentication needs of a proxy? Custom HttpRoutePlanner seems like the best option. Oleg
        Hide
        Dominique Jäggi added a comment -

        the installation of httpclient-osgi so far has the following dependencies that must be available in the OSGi container:

        • httpcomponents core
        • OSGi declarative services
        • apache commons logging
        Show
        Dominique Jäggi added a comment - the installation of httpclient-osgi so far has the following dependencies that must be available in the OSGi container: httpcomponents core OSGi declarative services apache commons logging
        Hide
        Dominique Jäggi added a comment -

        i have created a second patch based on 4.3 trunk of httpclient.

        the patch contains:

        • bundle activator
        • proxy configuration mechanism (osgi config service/config factory with multiple support)
        • proxy selector (registering itself via java.net.ProxySelector.setDefault())
        • client manager tracker for central shutdown of all clients
        • patched HttpRoute for default proxy selection
        • patched client managers to track themselves in the client manager tracker

        what is still missing:

        • handling proxy authentication - what would be the best location and way to inject the authentication needs of a proxy?
        Show
        Dominique Jäggi added a comment - i have created a second patch based on 4.3 trunk of httpclient. the patch contains: bundle activator proxy configuration mechanism (osgi config service/config factory with multiple support) proxy selector (registering itself via java.net.ProxySelector.setDefault()) client manager tracker for central shutdown of all clients patched HttpRoute for default proxy selection patched client managers to track themselves in the client manager tracker what is still missing: handling proxy authentication - what would be the best location and way to inject the authentication needs of a proxy?
        Hide
        Dominique Jäggi added a comment -

        thanks for your feedback - i will look at your suggestions and deliver a new proposition based on http-client trunk.

        Show
        Dominique Jäggi added a comment - thanks for your feedback - i will look at your suggestions and deliver a new proposition based on http-client trunk.
        Hide
        Oleg Kalnichevski added a comment -

        Dominique

        I took a cursory look at the patch and can give you some feedback right away.

        (1) There is no way we could add OSGi dependencies directly to the base module. That would break pretty much every instance of HttpClient used outside an OSGi container. Whatever changes we do to make HttpClient more OSGi friendly they need to be confined to the httpclient-osgi module.

        I do not know if this is easy or possible at all but this is a few option that I can see. One possibility would be overriding the stock versions of DefaultHttpRoutePlanner and DefaultHttpClient through some sort of class loader magic. Alternatively we could provide an OSGi aware HttpClientBuilder implementation (only available in 4.3 version on trunk) in the httpclient-osgi module. The downside of the latter would be having to instruct the users to explicitly use that custom builder when targeting OSGi environments.

        (2) Is dependency on Sling Commons really necessary? Could we do without it? I could imagine moving ProxySelector to the base module if it did not come with additional external dependencies.

        (3) All this stuff should probably go into 4.3 in order to go through proper ALPHA / BETA / GA QA cycle. So, ideally, it should not be using any deprecated code.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Dominique I took a cursory look at the patch and can give you some feedback right away. (1) There is no way we could add OSGi dependencies directly to the base module. That would break pretty much every instance of HttpClient used outside an OSGi container. Whatever changes we do to make HttpClient more OSGi friendly they need to be confined to the httpclient-osgi module. I do not know if this is easy or possible at all but this is a few option that I can see. One possibility would be overriding the stock versions of DefaultHttpRoutePlanner and DefaultHttpClient through some sort of class loader magic. Alternatively we could provide an OSGi aware HttpClientBuilder implementation (only available in 4.3 version on trunk) in the httpclient-osgi module. The downside of the latter would be having to instruct the users to explicitly use that custom builder when targeting OSGi environments. (2) Is dependency on Sling Commons really necessary? Could we do without it? I could imagine moving ProxySelector to the base module if it did not come with additional external dependencies. (3) All this stuff should probably go into 4.3 in order to go through proper ALPHA / BETA / GA QA cycle. So, ideally, it should not be using any deprecated code. Oleg

          People

          • Assignee:
            Unassigned
            Reporter:
            Dominique Jäggi
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development