ws-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andreas Veithen <andreas.veit...@gmail.com>
Subject Re: [Neethi] More updates, much easier to port from 2 -> 3
Date Sat, 19 Feb 2011 15:30:59 GMT
I took over the task of porting Sandesha2 to Neethi 3. That turned out
to be trickier than Axis2 and Rampart. I have an issue with the
following code in XMLPrimitiveAssertionBuilder:

        if (count == 0) {
            return newPrimitiveAssertion(element, atts.isEmpty() ? null : atts);
        } else if (policyCount == 1 && count == 1) {
            Policy policy = factory.getPolicyEngine().getPolicy(policyEl);
            return newPolicyContainingAssertion(element,
atts.isEmpty() ? null : atts, policy);
        }
        return new XmlPrimitiveAssertion(element);

There are two problems with that logic:

1. For assertions that have an optional child element, this logic
implies that the builder returns two fairly different types of objects
depending on the actual content of the assertion: a PrimitiveAssertion
if there is no child element and an XmlPrimitiveAssertion if there is
one. Note that in contrast to what the names may suggest,
XmlPrimitiveAssertion doesn't extend PrimitiveAssertion and they only
have the Assertion interface in common. That doesn't make sense to me;
two assertions differing only by the number of child elements should
not be represented by two completely different classes.

Of course one can argue that PrimitiveAssertion and
XmlPrimitiveAssertion should be considered as opaque, i.e. that they
are used only to ensure round-trippability, but that client code
should not attempt to parse the content of these types of assertions.
In fact one can consider that if the client code needs to parse an
assertion, it should have an appropriate assertion builder.

That is actually the point of view I took when porting Sandesha2.
Previously it attempted to parse the content of
XmlPrim(i)tiveAssertion objects, and now I've changed the code so that
there is an assertion builder for each type of assertion. However,
this causes a huge increase of the volume of code necessary to parse
assertions.

In addition, if one considers PrimitiveAssertion and
XmlPrimitiveAssertion as opaque, then the logical consequence would be
that these classes should not even have public methods allowing to
retrieve the content.

2. The code fragment shown above neglects the existence of assertions
that have no child elements, but that contain character data. Not sure
if this kind of assertions conform to the specs and/or best practices,
but they are used in Sandesha2. Example:

<sandesha2:MaximumRetransmissionCount>10</sandesha2:MaximumRetransmissionCount>

Currently, the code will create a PrimitiveAssertion and drop the
content of the assertion (only attributes are stored). That issue is
more serious than the first one because it breaks round-trippability.

Andreas

On Fri, Feb 18, 2011 at 17:18, Daniel Kulp <dkulp@apache.org> wrote:
> On Friday 18 February 2011 12:12:25 AM Afkham Azeez wrote:
>> Dan,
>> We will need to organize a hackathon to migrate Axis2 to use the new Neethi
>> APIs. Will you be able to assist during this?
>
> I'm not really sure you would need a Hackathon for it.   With the latest
> updates, it will probably take a single person longer to run the tests
> afterword than it would take to update the code.
>
> I just created RAMPART-322 and attached a patch that completely updates it to
> Neethi3.   Took about 30 minutes of work to do, and that's mostly cause I hit
> a bug in the new Neethi code that needed to get fixed first.
>
> Yes, if you wanted to update all of Axis/Rampart/etc... to not use the static
> PolicyEngine, that would require a lot more work.  However, that isn't
> required.
>
> Dan
>
>
>>
>> Azeez
>>
>> On Fri, Feb 18, 2011 at 10:04 AM, Daniel Kulp <dkulp@apache.org> wrote:
>> > I just committed a bunch more updates to Neethi that cleanup a few
>> > things, but
>> > also will end up making it much easier to port from Neeth 2 to 3.
>> >
>> > 1) I renamed PolicyEngine to PolicyBuilder.  It really is just a Policy
>> > builder.   CXF and Axis (and others) really have the "engine" for
>> > asserting the policies and such, thus the rename kind of makes sense.
>> >
>> > 2) With that renamed, I re-created the PolicyEngine with all the static
>> > methods.  It just wrappers a static instance of PolicyBuilder.    I'd
>> > LIKE to
>> > mark PolicyEngine as Deprecated as it's definitely better to not use the
>> > statics, but I'm also OK just leaving it.
>> >
>> > 3) I also changed the return of getAlternatives to match what the javadoc
>> > said
>> > which also matches what Axis is casting it to.  (simplified some CXF code
>> > as
>> > well)
>> >
>> >
>> > The result is that it's now pretty easy to update from 2.x to 3.x.   The
>> > changes required really are just:
>> >
>> > 1) Anything that implements AssertionBuilder will neet to be updated to
>> > implement AssertionBuilder<OMElement>.   Fairly simple.
>> >
>> > 2) All policies will need to have a "boolean isIgnorable()" method added.
>> >
>> >  You
>> >
>> > can just add a default method that returns false to keep the old
>> > behavior. However, you could also update the builders to record the
>> > ignorable flag properly so the intersections would work.
>> >
>> > I've included a patch below that would update the axis2 trunk.
>> > Obviously, things like Rampart would be more involved, but it does show
>> > how simple it is.
>> >
>> > Once again, I'd appreciate anyone else looking at it and any thoughts
>> > folks may have.
>> >
>> > --
>> > Daniel Kulp
>> > dkulp@apache.org
>> > http://dankulp.com/blog
>> >
>> >
>> >
>> >
>> >
>> > Index: modules/parent/pom.xml
>> > ===================================================================
>> > --- modules/parent/pom.xml      (revision 1071847)
>> > +++ modules/parent/pom.xml      (working copy)
>> > @@ -69,7 +69,7 @@
>> >
>> >        <!-- Tracking SNAPSHOT(s) of a few projects -->
>> >
>> >         <axiom.version>1.2.12-SNAPSHOT</axiom.version>
>> >         <!--TODO: fix the compatibility issues with the Neethi trunk
and
>> >
>> > bring back to 3.0.0-SNAPSHOT-->
>> > -        <neethi.version>2.0.4</neethi.version>
>> > +        <neethi.version>3.0.0-SNAPSHOT</neethi.version>
>> >
>> >         <woden.version>1.0-SNAPSHOT</woden.version>
>> >
>> >        <!-- Use released versions for these projects -->
>> >
>> > Index:
>> >
>> > modules/metadata/src/org/apache/axis2/jaxws/description/builder/converter
>> > /JavaMethodsToMDCConverter.java
>> > =================================================================== ---
>> >
>> > modules/metadata/src/org/apache/axis2/jaxws/description/builder/converter
>> > /JavaMethodsToMDCConverter.java (revision 1071847)
>> > +++
>> >
>> > modules/metadata/src/org/apache/axis2/jaxws/description/builder/converter
>> > /JavaMethodsToMDCConverter.java (working copy)
>> > @@ -28,7 +28,6 @@
>> >
>> >  import org.apache.axis2.jaxws.description.builder.WebEndpointAnnot;
>> >  import org.apache.axis2.jaxws.description.builder.WebMethodAnnot;
>> >  import org.apache.axis2.jaxws.description.builder.WebResultAnnot;
>> >
>> > -import sun.reflect.generics.reflectiveObjects.GenericArrayTypeImpl;
>> >
>> >  import javax.jws.Oneway;
>> >  import javax.jws.WebMethod;
>> >
>> > @@ -39,6 +38,7 @@
>> >
>> >  import javax.xml.ws.ResponseWrapper;
>> >  import javax.xml.ws.WebEndpoint;
>> >  import java.lang.reflect.Constructor;
>> >
>> > +import java.lang.reflect.GenericArrayType;
>> >
>> >  import java.lang.reflect.Method;
>> >  import java.lang.reflect.Modifier;
>> >  import java.lang.reflect.ParameterizedType;
>> >
>> > @@ -347,7 +347,7 @@
>> >
>> >             mdc.setReturnType(fullType);
>> >
>> >         } else if (type instanceof Class) {
>> >
>> >             mdc.setReturnType(((Class)type).getName());
>> >
>> > -        } else if (type instanceof GenericArrayTypeImpl) {
>> > +        } else if (type instanceof GenericArrayType) {
>> >
>> >             mdc.setReturnType(type.getClass().getName());
>> >
>> >         }
>> >
>> >     }
>> >
>> > Index:
>> > modules/mtompolicy/src/org/apache/axis2/policy/model/MTOMAssertion.java
>> > ===================================================================
>> > ---
>> > modules/mtompolicy/src/org/apache/axis2/policy/model/MTOMAssertion.java
>> > (revision 1071847)
>> > +++
>> > modules/mtompolicy/src/org/apache/axis2/policy/model/MTOMAssertion.java
>> > (working copy)
>> > @@ -52,5 +52,10 @@
>> >
>> >         this.optional = isOptional;
>> >
>> >     }
>> >
>> > +
>> > +    public boolean isIgnorable() {
>> > +        return false;
>> > +    }
>> > +
>> >
>> >  }
>> >
>> > \ No newline at end of file
>> > Index:
>> >
>> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM10AssertionBu
>> > ilder.java
>> > =================================================================== ---
>> >
>> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM10AssertionBu
>> > ilder.java (revision 1071847)
>> > +++
>> >
>> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM10AssertionBu
>> > ilder.java (working copy)
>> > @@ -43,7 +43,7 @@
>> >
>> >  * The builder will be picked by the
>> >  * "org.apache.neethi.AssertionBuilderFactory".
>> >  */
>> >
>> > -public class MTOM10AssertionBuilder implements AssertionBuilder {
>> > +public class MTOM10AssertionBuilder implements
>> > AssertionBuilder<OMElement> {
>> >
>> >     private static Log log =
>> >
>> > LogFactory.getLog(MTOM10AssertionBuilder.class);
>> >
>> > Index:
>> >
>> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM11AssertionBu
>> > ilder.java
>> > =================================================================== ---
>> >
>> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM11AssertionBu
>> > ilder.java (revision 1071847)
>> > +++
>> >
>> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM11AssertionBu
>> > ilder.java (working copy)
>> > @@ -39,7 +39,7 @@
>> >
>> >  * "org.apache.neethi.AssertionBuilderFactory".
>> >  */
>> >
>> > -public class MTOM11AssertionBuilder implements AssertionBuilder{
>> > +public class MTOM11AssertionBuilder implements
>> > AssertionBuilder<OMElement> {
>> >
>> >     private static Log log =
>> >
>> > LogFactory.getLog(MTOM10AssertionBuilder.class);
>> >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@ws.apache.org
>> > For additional commands, e-mail: dev-help@ws.apache.org
>
> --
> Daniel Kulp
> dkulp@apache.org
> http://dankulp.com/blog
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: dev-help@ws.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ws.apache.org
For additional commands, e-mail: dev-help@ws.apache.org


Mime
View raw message