HTTPCLIENT-1778: [OSGi] simplify handling of ManagedService based configurations

- simplify ProxyConfiguration handling

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/branches/4.5.x@1763268 13f79535-47bb-0310-9956-ffa450edef68

httpclient-osgi/src/main/java/org/apache/http/osgi/impl/HttpClientBuilderConfigurator.java

@@ -27,10 +27,9 @@
 package org.apache.http.osgi.impl;
 
 import org.apache.http.impl.client.HttpClientBuilder;
-import org.osgi.framework.BundleContext;
+import org.apache.http.osgi.services.ProxyConfiguration;
-import org.osgi.framework.ServiceRegistration;
 
-import java.util.Map;
+import java.util.List;
 
 final class HttpClientBuilderConfigurator {
 
@@ -38,11 +37,9 @@ final class HttpClientBuilderConfigurator {
 
     private final OSGiHttpRoutePlanner routePlanner;
 
-    HttpClientBuilderConfigurator(
+    HttpClientBuilderConfigurator(final List<ProxyConfiguration> proxyConfigurations) {
-            final BundleContext bundleContext,
+        credentialsProvider = new OSGiCredentialsProvider(proxyConfigurations);
-            final Map<String, ServiceRegistration> registeredConfigurations) {
+        routePlanner = new OSGiHttpRoutePlanner(proxyConfigurations);
-        credentialsProvider = new OSGiCredentialsProvider(bundleContext, registeredConfigurations);
-        routePlanner = new OSGiHttpRoutePlanner(bundleContext, registeredConfigurations);
     }
 
     <T extends HttpClientBuilder> T configure(final T clientBuilder) {

httpclient-osgi/src/main/java/org/apache/http/osgi/impl/HttpProxyConfigurationActivator.java

@@ -33,6 +33,7 @@ import java.util.Hashtable;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.osgi.services.CachingHttpClientBuilderFactory;
@@ -41,6 +42,7 @@ import org.apache.http.osgi.services.ProxyConfiguration;
 import org.osgi.framework.BundleActivator;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.Constants;
+import org.osgi.framework.ServiceReference;
 import org.osgi.framework.ServiceRegistration;
 import org.osgi.service.cm.ConfigurationException;
 import org.osgi.service.cm.ManagedServiceFactory;
@@ -72,6 +74,8 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M
 
     private final Map<String, ServiceRegistration> registeredConfigurations = new LinkedHashMap<String, ServiceRegistration>();
 
+    private final List<ProxyConfiguration> proxyConfigurations = new CopyOnWriteArrayList<ProxyConfiguration>();
+
     private final List<CloseableHttpClient> trackedHttpClients;
 
     public HttpProxyConfigurationActivator() {
@@ -93,8 +97,7 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M
 
         configurator = context.registerService(ManagedServiceFactory.class.getName(), this, props);
 
-        final HttpClientBuilderConfigurator configurator =
+        final HttpClientBuilderConfigurator configurator = new HttpClientBuilderConfigurator(proxyConfigurations);
-                new HttpClientBuilderConfigurator(context, registeredConfigurations);
 
         props.clear();
         props.put(Constants.SERVICE_PID, BUILDER_FACTORY_SERVICE_PID);
@@ -171,6 +174,7 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M
                                                                                     proxyConfiguration,
                                                                                     config);
             registeredConfigurations.put(pid, configurationRegistration);
+            proxyConfigurations.add(proxyConfiguration);
         } else {
             proxyConfiguration = (OSGiProxyConfiguration) context.getService(registration.getReference());
         }
@@ -186,10 +190,13 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M
      */
     @Override
     public void deleted(final String pid) {
-        final ServiceRegistration registeredConfiguration = registeredConfigurations.get(pid);
+        final ServiceRegistration registration = registeredConfigurations.remove(pid);
-        if (null != registeredConfiguration) {
+        if (registration != null) {
-            registeredConfiguration.unregister();
+            final ServiceReference ref = registration.getReference();
-            registeredConfigurations.remove(pid);
+            final ProxyConfiguration config = (ProxyConfiguration) context.getService(ref);
+            proxyConfigurations.remove(config);
+            context.ungetService(ref);
+            registration.unregister();
         }
     }
 

httpclient-osgi/src/main/java/org/apache/http/osgi/impl/OSGiCredentialsProvider.java

@@ -26,30 +26,23 @@
  */
 package org.apache.http.osgi.impl;
 
-import java.util.Map;
+import java.util.List;
 
 import org.apache.http.auth.AuthScope;
 import org.apache.http.auth.Credentials;
 import org.apache.http.auth.UsernamePasswordCredentials;
 import org.apache.http.client.CredentialsProvider;
 import org.apache.http.osgi.services.ProxyConfiguration;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceRegistration;
 
 /**
  * @since 4.3
  */
 final class OSGiCredentialsProvider implements CredentialsProvider {
 
-    private final BundleContext bundleContext;
+    private List<ProxyConfiguration> proxyConfigurations;
 
-    private final Map<String, ServiceRegistration> registeredConfigurations;
+    public OSGiCredentialsProvider(final List<ProxyConfiguration> proxyConfigurations) {
-
+        this.proxyConfigurations = proxyConfigurations;
-    public OSGiCredentialsProvider(
-            final BundleContext bundleContext,
-            final Map<String, ServiceRegistration> registeredConfigurations) {
-        this.bundleContext = bundleContext;
-        this.registeredConfigurations = registeredConfigurations;
     }
 
     /**
@@ -66,15 +59,13 @@ final class OSGiCredentialsProvider implements CredentialsProvider {
     @Override
     public Credentials getCredentials(final AuthScope authscope) {
         // iterate over all active proxy configurations at the moment of getting the credential
-        for (final ServiceRegistration registration : registeredConfigurations.values()) {
+        for (final ProxyConfiguration proxyConfiguration : proxyConfigurations) {
-            final Object proxyConfigurationObject = bundleContext.getService(registration.getReference());
-            if (proxyConfigurationObject != null) {
-                final ProxyConfiguration proxyConfiguration = (ProxyConfiguration) proxyConfigurationObject;
             if (proxyConfiguration.isEnabled()) {
                 final AuthScope actual = new AuthScope(proxyConfiguration.getHostname(), proxyConfiguration.getPort());
                 if (authscope.match(actual) >= 12) {
-                        return new UsernamePasswordCredentials(proxyConfiguration.getUsername(), proxyConfiguration.getPassword());
+                    final String username = proxyConfiguration.getUsername();
-                    }
+                    final String password = proxyConfiguration.getPassword();
+                    return new UsernamePasswordCredentials(username, password != null ? password : null);
                 }
             }
         }

httpclient-osgi/src/main/java/org/apache/http/osgi/impl/OSGiHttpRoutePlanner.java

@@ -26,8 +26,8 @@
  */
 package org.apache.http.osgi.impl;
 
+import java.util.List;
 import java.util.Locale;
-import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -37,8 +37,6 @@ import org.apache.http.HttpRequest;
 import org.apache.http.impl.conn.DefaultRoutePlanner;
 import org.apache.http.osgi.services.ProxyConfiguration;
 import org.apache.http.protocol.HttpContext;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceRegistration;
 
 /**
  * @since 4.3
@@ -55,16 +53,11 @@ final class OSGiHttpRoutePlanner extends DefaultRoutePlanner {
                                                                   "([01]?\\d\\d?|2[0-4]\\d|25[0-5])\\." +
                                                                   "([01]?\\d\\d?|2[0-4]\\d|25[0-5])$");
 
-    private final BundleContext bundleContext;
+    private List<ProxyConfiguration> proxyConfigurations;
 
-    private final Map<String, ServiceRegistration> registeredConfigurations;
+    public OSGiHttpRoutePlanner(final List<ProxyConfiguration> proxyConfigurations) {
-
-    public OSGiHttpRoutePlanner(
-            final BundleContext bundleContext,
-            final Map<String, ServiceRegistration> registeredConfigurations) {
         super(null);
-        this.bundleContext = bundleContext;
+        this.proxyConfigurations = proxyConfigurations;
-        this.registeredConfigurations = registeredConfigurations;
     }
 
     /**
@@ -72,12 +65,8 @@ final class OSGiHttpRoutePlanner extends DefaultRoutePlanner {
      */
     @Override
     protected HttpHost determineProxy(final HttpHost target, final HttpRequest request, final HttpContext context) throws HttpException {
-        ProxyConfiguration proxyConfiguration = null;
         HttpHost proxyHost = null;
-        for (final ServiceRegistration registration : registeredConfigurations.values()) {
+        for (final ProxyConfiguration proxyConfiguration : proxyConfigurations) {
-            final Object proxyConfigurationObject = bundleContext.getService(registration.getReference());
-            if (proxyConfigurationObject != null) {
-                proxyConfiguration = (ProxyConfiguration) proxyConfigurationObject;
             if (proxyConfiguration.isEnabled()) {
                 for (final String exception : proxyConfiguration.getProxyExceptions()) {
                     if (createMatcher(exception).matches(target.getHostName())) {
@@ -89,7 +78,6 @@ final class OSGiHttpRoutePlanner extends DefaultRoutePlanner {
                 }
             }
         }
-        }
 
         return proxyHost;
     }

httpclient-osgi/src/test/java/org/apache/http/osgi/impl/TestOSGiHttpRoutePlanner.java

@@ -26,22 +26,18 @@
  */
 package org.apache.http.osgi.impl;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 import java.net.InetAddress;
-import java.util.Map;
+import java.util.Hashtable;
-import java.util.TreeMap;
 
 import org.apache.http.HttpHost;
 import org.apache.http.osgi.services.ProxyConfiguration;
 import org.junit.Test;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceReference;
-import org.osgi.framework.ServiceRegistration;
 
 
 /**
@@ -50,48 +46,12 @@ import org.osgi.framework.ServiceRegistration;
 
 public class TestOSGiHttpRoutePlanner  {
 
-    final ProxyConfiguration pc1 = new ProxyConfiguration() {
+    private final ProxyConfiguration pc1 = proxy("proxy1", 8080, "localhost", "127.0.0.1", ".apache.org");
-        @Override
+    private final ProxyConfiguration pc2 = proxy("proxy2", 9090, "localhost", "127.0.0.1", ".oracle.com", "12.34.34.8");
-        public boolean isEnabled() {return true; }
-        @Override
-        public String getHostname() {return "proxy1"; }
-        @Override
-        public int getPort() { return 8080; }
-        @Override
-        public String getUsername() { return ""; }
-        @Override
-        public String getPassword() {return ""; }
-        @Override
-        public String[] getProxyExceptions() { return new String[]{"localhost", "127.0.0.1", ".apache.org"}; }
-    };
-
-    final ProxyConfiguration pc2 = new ProxyConfiguration() {
-        @Override
-        public boolean isEnabled() {return true; }
-        @Override
-        public String getHostname() {return "proxy2"; }
-        @Override
-        public int getPort() { return 9090; }
-        @Override
-        public String getUsername() { return ""; }
-        @Override
-        public String getPassword() {return ""; }
-        @Override
-        public String[] getProxyExceptions() { return new String[]{"localhost", "127.0.0.1", ".oracle.com", "12.34.34.8"}; }
-    };
 
     @Test
     public void testDeterminProxy() throws Exception {
-        final ServiceReference sRef1 = mock(ServiceReference.class);
+        OSGiHttpRoutePlanner planner = new OSGiHttpRoutePlanner(singletonList(pc1));
-        final ServiceRegistration sReg1 = mock(ServiceRegistration.class);
-        when(sReg1.getReference()).thenReturn(sRef1);
-        final BundleContext bc = mock(BundleContext.class);
-        when(bc.getService(sRef1)).thenReturn(this.pc1);
-
-        final Map<String, ServiceRegistration> registrations = new TreeMap<String, ServiceRegistration>(); // TreeMap for order
-        registrations.put("foo1", sReg1);
-
-        OSGiHttpRoutePlanner planner = new OSGiHttpRoutePlanner(bc, registrations);
 
         HttpHost proxy = planner.determineProxy(new HttpHost("localhost", 8090), null, null);
         assertNull(proxy);
@@ -113,13 +73,7 @@ public class TestOSGiHttpRoutePlanner  {
 
 
         // test with more than one registration of proxyConfiguration
-        final ServiceReference sRef2 = mock(ServiceReference.class);
+        planner = new OSGiHttpRoutePlanner(asList(pc1, pc2));
-        final ServiceRegistration sReg2 = mock(ServiceRegistration.class);
-        when(sReg2.getReference()).thenReturn(sRef2);
-        when(bc.getService(sRef2)).thenReturn(this.pc2);
-        registrations.put("foo2", sReg2);
-
-        planner = new OSGiHttpRoutePlanner(bc, registrations);
         proxy = planner.determineProxy(new HttpHost("localhost", 8090), null, null);
         assertNull(proxy);
 
@@ -139,15 +93,7 @@ public class TestOSGiHttpRoutePlanner  {
 
     @Test
     public void testMasking() throws Exception {
-        final ServiceReference sRef2 = mock(ServiceReference.class);
+        final OSGiHttpRoutePlanner planner = new OSGiHttpRoutePlanner(singletonList(pc2));
-        final ServiceRegistration sReg2 = mock(ServiceRegistration.class);
-        when(sReg2.getReference()).thenReturn(sRef2);
-        final BundleContext bc = mock(BundleContext.class);
-        when(bc.getService(sRef2)).thenReturn(this.pc2);
-        final Map<String, ServiceRegistration> registrations = new TreeMap<String, ServiceRegistration>();
-        registrations.put("foo2", sReg2);
-
-        final OSGiHttpRoutePlanner planner = new OSGiHttpRoutePlanner(bc, registrations);
 
         HttpHost proxy = planner.determineProxy(new HttpHost("12.34.34.2", 4554), null, null);
         assertNotNull(proxy);
@@ -157,4 +103,17 @@ public class TestOSGiHttpRoutePlanner  {
         assertNotNull(proxy);
     }
 
+    private ProxyConfiguration proxy(final String host, final int port, final String... exceptions) {
+        final OSGiProxyConfiguration proxyConfiguration = new OSGiProxyConfiguration();
+        final Hashtable<String, Object> config = new Hashtable<String, Object>();
+        config.put("proxy.enabled", true);
+        config.put("proxy.host", host);
+        config.put("proxy.port", port);
+        config.put("proxy.user", "");
+        config.put("proxy.password", "");
+        config.put("proxy.exceptions", exceptions);
+        proxyConfiguration.update(config);
+        return proxyConfiguration;
+    }
+
 }