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: local policy includes SPF record at
 spf.trusted-forwarder.org)
Content-Type: text/plain; charset=windows-1252
Mime-Version: 1.0 (Mac OS X Mail 7.2 \(1874\))
Subject: Re: Setting custom DestinationRegistry in HTTPTransportFactory (CXF
 3.0)
From: Daniel Kulp <dkulp@apache.org>
In-Reply-To: <533E7CCB.6000504@redhat.com>
Date: Fri, 4 Apr 2014 11:49:24 -0400
Content-Transfer-Encoding: quoted-printable
Message-Id: <1CA00526-62DC-4BAE-AB23-7984B5F2D5C3@apache.org>
References: <533E7CCB.6000504@redhat.com>
To: dev@cxf.apache.org,
 Alessio Soldano <asoldano@redhat.com>


This seems like a good solution/idea=85.. 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 <asoldano@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.
>=20
> 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:
>=20
> bus.setExtension(new JBossWSDestinationRegistryImpl(), =
DestinationRegistry.class);
>=20
> 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):
>=20
> diff --git =
a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPTrans=
portFactory.java =
b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPTrans=
portFactory.java
> index 44b4592..bcf75c3 100644
> --- =
a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPTrans=
portFactory.java
> +++ =
b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPTrans=
portFactory.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;
>=20
> @@ -75,7 +78,11 @@ public class HTTPTransportFactory
>         URI_PREFIXES.add("https://");
>     }
>=20
> -    protected final DestinationRegistry registry;
> +    protected DestinationRegistry registry;
> +
> +    private final ReadWriteLock lock =3D new =
ReentrantReadWriteLock();
> +    private final Lock r =3D lock.readLock();
> +    private final Lock w =3D lock.writeLock();
>=20
>     public HTTPTransportFactory() {
>         this(new DestinationRegistryImpl());
> @@ -91,6 +98,19 @@ public class HTTPTransportFactory
>         return registry;
>     }
>=20
> +    public void setRegistry(DestinationRegistry newRegistry) {
> +        w.lock();
> +        try {
> +            if (registry.getDestinations().isEmpty()) {
> +                this.registry =3D 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 =3D=3D null) {
>             throw new IllegalArgumentException("EndpointInfo cannot be =
null");
>         }
> -        synchronized (registry) {
> -            AbstractHTTPDestination d =3D =
registry.getDestinationForPath(endpointInfo.getAddress());
> -            if (d =3D=3D null) {
> -                HttpDestinationFactory jettyFactory =3D =
bus.getExtension(HttpDestinationFactory.class);
> -                String addr =3D endpointInfo.getAddress();
> -                if (jettyFactory =3D=3D null && addr !=3D null && =
addr.startsWith("http")) {
> -                    String m =3D
> -                        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 =3D null;
> -                if (jettyFactory !=3D null && (addr =3D=3D null || =
addr.startsWith("http"))) {
> -                    factory =3D jettyFactory;
> -                } else {
> -                    factory =3D new ServletDestinationFactory();
> +        r.lock();
> +        try {
> +            synchronized (registry) {
> +                AbstractHTTPDestination d =3D =
registry.getDestinationForPath(endpointInfo.getAddress());
> +                if (d =3D=3D null) {
> +                    HttpDestinationFactory jettyFactory =3D =
bus.getExtension(HttpDestinationFactory.class);
> +                    String addr =3D endpointInfo.getAddress();
> +                    if (jettyFactory =3D=3D null && addr !=3D null && =
addr.startsWith("http")) {
> +                        String m =3D
> +                            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 =3D null;
> +                    if (jettyFactory !=3D null && (addr =3D=3D null =
|| addr.startsWith("http"))) {
> +                        factory =3D jettyFactory;
> +                    } else {
> +                        factory =3D new ServletDestinationFactory();
> +                    }
> +
> +                    d =3D factory.createDestination(endpointInfo, =
bus, registry);
> +                    registry.addDestination(d);
> +                    configure(bus, d);
> +                    d.finalizeConfig();
>                 }
> -
> -                d =3D factory.createDestination(endpointInfo, bus, =
registry);
> -                registry.addDestination(d);
> -                configure(bus, d);
> -                d.finalizeConfig();
> +                return d;
>             }
> -            return d;
> +        } finally {
> +            r.unlock();
>         }
>     }
>=20
>=20
> 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?
>=20
> Thanks
> Alessio
>=20
> --=20
> Alessio Soldano
> Web Service Lead, JBoss
>=20

--=20
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com