ws-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Kulp <dk...@apache.org>
Subject Re: [Neethi] More updates, much easier to port from 2 -> 3
Date Sat, 19 Feb 2011 19:32:07 GMT

Andreas,

Major thanks for looking at this.   It's a big help since you obviously have 
found some issues that need to get fixed before the release.

#2 is definitely a problem I overlooked.   I'll get that fixed on Monday. 

#1 is a bigger....


On Saturday 19 February 2011 10:30:59 AM Andreas Veithen wrote:
> 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.

Well, I debated removing the XmlPrimitiveAssertion class entirely as it really 
cannot be used for much other than parsing.   It doesn't support normalization 
properly nor can it really handle intersections.   It could definitely extend 
the PrimitiveAssertion, but the normalization/intersetion issues would remain.


> 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.

Well, I generatlly consider PrimitiveAssertion and the 
PolicyContainingPrimitiveAssertion as more or less possible base classes that 
customs could subclass.  I've done that in CXF.   

> 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.

Well, one of the things I'm trying to do is make it so a "generic" policy 
editor (maybe a GUI or something), can parse/process policies in a very 
generic way without the custom parsers and such.   Thus, Neethi should at 
least be able to parse/process everything in a generic way without all the 
custom builders and such.   Thus, if info is being lost, that is a problem.  
Thanks for looking at it and finding some of those issues.   I'll look at them 
on Monday.

Obviously, runtimes that need more complete control over the parsing and types 
should provide custom builders and types and such.

Dan

> 
> 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:MaximumRetransmissionCo
> unt>
> 
> 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/conver
> >> > ter /JavaMethodsToMDCConverter.java
> >> > ===================================================================
> >> > ---
> >> > 
> >> > modules/metadata/src/org/apache/axis2/jaxws/description/builder/conver
> >> > ter /JavaMethodsToMDCConverter.java (revision 1071847)
> >> > +++
> >> > 
> >> > modules/metadata/src/org/apache/axis2/jaxws/description/builder/conver
> >> > ter /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.jav
> >> > a ===================================================================
> >> > ---
> >> > modules/mtompolicy/src/org/apache/axis2/policy/model/MTOMAssertion.jav
> >> > a (revision 1071847)
> >> > +++
> >> > modules/mtompolicy/src/org/apache/axis2/policy/model/MTOMAssertion.jav
> >> > a (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/MTOM10Assertio
> >> > nBu ilder.java
> >> > ===================================================================
> >> > ---
> >> > 
> >> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM10Assertio
> >> > nBu ilder.java (revision 1071847)
> >> > +++
> >> > 
> >> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM10Assertio
> >> > nBu 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/MTOM11Assertio
> >> > nBu ilder.java
> >> > ===================================================================
> >> > ---
> >> > 
> >> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM11Assertio
> >> > nBu ilder.java (revision 1071847)
> >> > +++
> >> > 
> >> > modules/mtompolicy/src/org/apache/axis2/policy/builders/MTOM11Assertio
> >> > nBu 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

-- 
Daniel Kulp
dkulp@apache.org
http://dankulp.com/blog
Talend - http://www.talend.com

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


Mime
View raw message