Mailing-List: contact dev-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Received-SPF: pass (nike.apache.org: domain of asoldano@redhat.com designates
 209.132.183.28 as permitted sender)
Message-ID: <533E7CCB.6000504@redhat.com>
Date: Fri, 04 Apr 2014 11:35:07 +0200
From: Alessio Soldano <asoldano@redhat.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:17.0) Gecko/20130805 Thunderbird/17.0.8
MIME-Version: 1.0
To: dev@cxf.apache.org
Subject: Setting custom DestinationRegistry in HTTPTransportFactory (CXF 3.0)
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi,
I've been mentioning this to Dan on IRC yesterday, but I'd like to bring 
the topic back here as I have a proposal now.

I need to set a custom DestinationRegistry implementation in the 
HTTPTransportFactory (as I override a method in the registry). With CXF 
2.7.x, I used to set an instance of my registry impl in the bus:

bus.setExtension(new JBossWSDestinationRegistryImpl(), 
DestinationRegistry.class);

however that does not work anymore with CXF 3.0 because the 
HTTPTransportFactory does not hold a reference to the bus anymore, hence 
it does not look for configured registry in it, and simply creates the 
default DestinationRegistryImpl.
My idea would be to rely on the optional Configurer which could be 
installed in the bus (I'm already setting a custom Configurer) to 
configure the HTTPTransportFactory before it's actually used. The 
factory is already passed to the configurer afaics. So, we'd need to 
allow changing the reference to the registry in the factory. If you have 
nothing against that, I'd create a jira and commit the following changes 
(with proper logging i18n, of course):

diff --git 
a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPTransportFactory.java 
b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPTransportFactory.java
index 44b4592..bcf75c3 100644
--- 
a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPTransportFactory.java
+++ 
b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPTransportFactory.java
@@ -27,6 +27,9 @@ import java.util.HashSet;
  import java.util.Iterator;
  import java.util.List;
  import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
  import java.util.logging.Level;
  import java.util.logging.Logger;

@@ -75,7 +78,11 @@ public class HTTPTransportFactory
          URI_PREFIXES.add("https://");
      }

-    protected final DestinationRegistry registry;
+    protected DestinationRegistry registry;
+
+    private final ReadWriteLock lock = new ReentrantReadWriteLock();
+    private final Lock r = lock.readLock();
+    private final Lock w = lock.writeLock();

      public HTTPTransportFactory() {
          this(new DestinationRegistryImpl());
@@ -91,6 +98,19 @@ public class HTTPTransportFactory
          return registry;
      }

+    public void setRegistry(DestinationRegistry newRegistry) {
+        w.lock();
+        try {
+            if (registry.getDestinations().isEmpty()) {
+                this.registry = newRegistry;
+            } else {
+                throw new RuntimeException("Cannot change registry 
already in use!");
+            }
+        } finally {
+            w.unlock();
+        }
+    }
+
      /**
       * This call is used by CXF ExtensionManager to inject the 
activationNamespaces
       * @param ans The transport ids.
@@ -231,31 +251,36 @@ public class HTTPTransportFactory
          if (endpointInfo == null) {
              throw new IllegalArgumentException("EndpointInfo cannot be 
null");
          }
-        synchronized (registry) {
-            AbstractHTTPDestination d = 
registry.getDestinationForPath(endpointInfo.getAddress());
-            if (d == null) {
-                HttpDestinationFactory jettyFactory = 
bus.getExtension(HttpDestinationFactory.class);
-                String addr = endpointInfo.getAddress();
-                if (jettyFactory == null && addr != null && 
addr.startsWith("http")) {
-                    String m =
-                        new 
org.apache.cxf.common.i18n.Message("NO_HTTP_DESTINATION_FACTORY_FOUND"
-                                                               , 
LOG).toString();
-                    LOG.log(Level.SEVERE, m);
-                    throw new IOException(m);
-                }
-                HttpDestinationFactory factory = null;
-                if (jettyFactory != null && (addr == null || 
addr.startsWith("http"))) {
-                    factory = jettyFactory;
-                } else {
-                    factory = new ServletDestinationFactory();
+        r.lock();
+        try {
+            synchronized (registry) {
+                AbstractHTTPDestination d = 
registry.getDestinationForPath(endpointInfo.getAddress());
+                if (d == null) {
+                    HttpDestinationFactory jettyFactory = 
bus.getExtension(HttpDestinationFactory.class);
+                    String addr = endpointInfo.getAddress();
+                    if (jettyFactory == null && addr != null && 
addr.startsWith("http")) {
+                        String m =
+                            new 
org.apache.cxf.common.i18n.Message("NO_HTTP_DESTINATION_FACTORY_FOUND"
+ , LOG).toString();
+                        LOG.log(Level.SEVERE, m);
+                        throw new IOException(m);
+                    }
+                    HttpDestinationFactory factory = null;
+                    if (jettyFactory != null && (addr == null || 
addr.startsWith("http"))) {
+                        factory = jettyFactory;
+                    } else {
+                        factory = new ServletDestinationFactory();
+                    }
+
+                    d = factory.createDestination(endpointInfo, bus, 
registry);
+                    registry.addDestination(d);
+                    configure(bus, d);
+                    d.finalizeConfig();
                  }
-
-                d = factory.createDestination(endpointInfo, bus, registry);
-                registry.addDestination(d);
-                configure(bus, d);
-                d.finalizeConfig();
+                return d;
              }
-            return d;
+        } finally {
+            r.unlock();
          }
      }


The read/write lock stuff is required to prevent anybody from trying to 
use the registry while its reference is being modified, but might be 
considered too much of a preventive measure and avoided if we simply 
assume and document that the registry can be modified only before 
starting using it.
WDYT? Perhaps is this "issue" going to be addressed in a more general way?

Thanks
Alessio

-- 
Alessio Soldano
Web Service Lead, JBoss