You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by Alessio Soldano <as...@redhat.com> on 2014/04/04 11:35:07 UTC
Setting custom DestinationRegistry in HTTPTransportFactory (CXF 3.0)
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
Re: Setting custom DestinationRegistry in HTTPTransportFactory (CXF
3.0)
Posted by Alessio Soldano <as...@redhat.com>.
Cool, thanks for the feedback.
Will work on this on Monday.
Cheers
Alessio
On 04/04/14 17:49, Daniel Kulp wrote:
> This seems like a good solution/idea….. wish I would have thought of it. :-)
>
> Using the configurer to configure these things is definitely the right approach.
>
> Dan
>
>
>
> On Apr 4, 2014, at 5:35 AM, Alessio Soldano <as...@redhat.com> wrote:
>
>> 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
>>
--
Alessio Soldano
Web Service Lead, JBoss
Re: Setting custom DestinationRegistry in HTTPTransportFactory (CXF 3.0)
Posted by Daniel Kulp <dk...@apache.org>.
This seems like a good solution/idea….. wish I would have thought of it. :-)
Using the configurer to configure these things is definitely the right approach.
Dan
On Apr 4, 2014, at 5:35 AM, Alessio Soldano <as...@redhat.com> wrote:
> 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
>
--
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com