axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Amila Suriarachchi <amilasuriarach...@gmail.com>
Subject Re: -1 for SVN revision 727413 (WSDL2Java change)
Date Sun, 01 Feb 2009 14:27:11 GMT
On Sat, Jan 31, 2009 at 9:20 PM, Glen Daniels <glen@thoughtcraft.com> wrote:

> Hey Amila, all:
>
> Apologies if the tone of that last mail was a bit intense. :)  I don't
> mean to be confrontational here, clearly we all want things to work.

definitely +1 :)

>
> I'm just having trouble understanding what you want here.  Maybe you
> could just answer a couple of questions:
>
> It seems like your changes broke the Rampart build.  That's issue #1.
> Do you think Rampart is wrong in mapping WSDL "Ping" to Java "ping()"?
> If so, why?

Depends on the way they use wsdl2java tool. if they use -lcmn option this is
correct. otherwise have to change the method name to "Ping". Not only
rampart but also any module which uses wsdl2java tool (or any other code
generation tool ) has to write code compatible with the generated code.

Let me explain you the situation.

Earlier Rampart has matched the WSDL "Ping" to java "Ping". Then once I did
the first change this did not get compiled since generated code was "ping".
To fix this it was changed (by nandana I think) to "ping".

Then I changes code again (which you had put -1) so that by default it
generates method name as "Ping". Now again rampart had a problem. This can
be solved by either converting back the name as "Ping" or by using the -lcmn
option when generating the code.

IMHO Rampart is a WS-Security implementation and it has nothing to worry
about the method names of the stub. The only thing it has to do is to write
the stub method names which are compatible with the generated wsdl2java
code. (Again this can be done by either using "Ping" or using -lcmn option).



>
> Issue #2 is why you think the option is necessary at all.  Could you
> please explain that?


just think what we had on the time Axis2 1.4 release. At that time the
Operation name "Foo" mapped to java method name "Foo".  This has no any
technical problem. But java coding convention requires method names to start
with lower case letters. So I wanted to generate method names according to
that (i.e. "foo")

Just changing wsdl2java tool to generate such method name (which I did
earlier) has two problems.
1. WSDL allows to have two operation names as "Foo" and "foo". Generated
code won't compile for such WSDL files.
2. the generated code is not backward compatible with Axis2 1.4 release
code.

To over come these two problems I make it as an option. So that the people
do not have such operation names and do not consider about the backward
compatibility can use that option.

Could you please explain why you want to remove this option and not
solve this problem by either changing rampart method name or using -lcmn
option?

As I saw in a rampart list discussion[1] Andreas has already start using
-lcmn option. The problem you have mentioned there I have fixed by calling
to xmlNameToJava.


[1]
http://mail-archives.apache.org/mod_mbox/ws-rampart-dev/200901.mbox/browser


thanks,
Amila.


>
> Thanks,
> --Glen
>
> Glen Daniels wrote:
> > Hi Amila:
> >
> > Sorry for the late reply, I had a logjam with my axis-dev mail for a few
> > days (I *thought* it was weird that nothing was coming in).
> >
> > I don't know how much more clearly I can put this than [1], but I'll try.
> >
> > -1 to the change you just made, Amila, and -1 to the option.  I never
> > withdrew my previous -1, so that's why I reverted your change after you
> > didn't.  Sorry, but what you're trying to do here is just wrong as far
> > as I can tell, and until you explain exactly what you're trying to do
> > and why we need it I'm not going to accept a change which BREAKS
> > previous codegen behavior in incorrect ways.  I'm going to ask that you
> > please revert 739189 and 739190, and I will once again explain my
> reasoning.
> >
> > Let's forget, for the moment, what previous revisions did or did not do,
> > and focus on the way it SHOULD work, ok?  We'll get back to history
> later.
> >
> > * We MUST call xmlNameToJavaIdentifier() when translating XML stuff
> > (i.e. WSDL) to Java stuff.  This is not negotiable.  XML's lexical
> > identifier space is simply not compatible with Java's, and we therefore
> > must do some mapping in order to guarantee compilable Java code.  The
> > simple examples here are a) a WSDL operation "foo-bar", and b) multiple
> > XML elements/operations that map to the same Java (for instance "Foo",
> > "foo", and "FoO").
> >
> > * We MUST guarantee compilable Java code from *any* valid WSDL.
> >
> > * The mapping SHOULD be exactly the same as the standard JAX-RPC/JAX-WS
> > mapping rules.  Why would it ever be otherwise?
> >
> > Please let me know if you have any issues with this analysis.  And
> > others, please feel free to chime in here!
> >
> > So, a few results from this:
> >
> > - If we weren't doing things RIGHT before, then we should fix the
> > default behavior.  But fixing a bug is the *only* reason to change the
> > default.
> >
> > - We should have a set of tests that confirms the correct behavior.
> > Have a WSDL that demonstrates all the conversions, including generations
> > for "foo1()", "foo2()" etc. when there are identical mapping results.
> > Axis1 did this right, Axis2 should too.
> >
> > - Why would we ever need an option to change this behavior?
> >
> > As I understand it, we *were* doing the right thing - mostly - before
> > you added the option.  That's why Rampart assumes the WSDL "Ping"
> > operation generates a Java "ping()", which is correct.  When you added
> > the option, you changed the default behavior and broke things.
> >
> > Please respect the -1 and revert your changes.  If you can explain
> > (preferably with a concrete example/test) why you still think we need an
> > option, I'm happy to discuss it.
> >
> > Thanks,
> > --Glen
> >
> > [1] http://markmail.org/message/7iq6wrgxdtxg5wfu
> >
> > Amila Suriarachchi wrote:
> >> Added an option to make the method name lower case. By default wsdl2java
> >> generates the code using the operation name of the wsdl. Rampart can
> >> either add this option (i.e -lcmn ) or revert the earlier change.
> >>
> >> thanks,
> >> Amila.
> >>
> >> On Mon, Jan 26, 2009 at 10:51 AM, Amila Suriarachchi
> >> <amilasuriarachchi@gmail.com <mailto:amilasuriarachchi@gmail.com>>
> wrote:
> >>
> >>     hi Glen,
> >>
> >>     What is the motivation behind this change?
> >>
> >>     http://svn.apache.org/viewvc?view=rev&revision=733776
> >>     <http://svn.apache.org/viewvc?view=rev&revision=733776>
> >>
> >>     1. You have done this after my fix for this issue which has been
> >>     discussed this this thread without any further discussions. IMHO you
> >>     should have send a notification to this thread at least after doing
> >>     this change.
> >>
> >>     2. -lcmn is an option. IMHO there is no reason to revert an option.
> >>
> >>     3. What is the best way you suggest?
> >>
> >>     thanks,
> >>     Amila.
> >>
> >>
> >>
> >>
> >>     On Fri, Jan 9, 2009 at 10:54 AM, Amila Suriarachchi
> >>     <amilasuriarachchi@gmail.com <mailto:amilasuriarachchi@gmail.com>>
> >>     wrote:
> >>
> >>
> >>
> >>         On Thu, Jan 8, 2009 at 8:22 PM, Glen Daniels
> >>         <glen@thoughtcraft.com <mailto:glen@thoughtcraft.com>> wrote:
> >>
> >>             Hi Amila:
> >>
> >>             Amila Suriarachchi wrote:
> >>             >     > Originally (i.e. even for Axis2 1.4 ) the generated
> >>             method names were
> >>             >     > exactly same as the Operation
> >>             >
> >>             > This has no compilation problem. since port type operation
> >>             names differ
> >>             > from each other.
> >>
> >>             Let me put this as directly as possible.  If we don't call
> >>             xmlNameToJavaIdentifier() when translating names, we can end
> >>             up with
> >>             uncompilable stuff.  Just run WSDL2Java on a WSDL with an
> >>             operation name
> >>             containing a dash or any other illegal Java identifier
> >>             character.  That
> >>             MUST be fixed.
> >>
> >>             If we don't have some way of munging names so that we handle
> >>             the case
> >>             where multiple XML names map to the same Java name, we will
> >>             also likely
> >>             end up with uncompilable code (due to duplicate method
> >>             names).  That
> >>             MUST be fixed.
> >>
> >>
> >>         Here I have made a mistake of reverting the first commit. see
> >>         the initial commit[1]
> >>         it was calling to xmlToJava method to avoid the problem you have
> >>         mentioned. I changed the current accordingly.
> >>         thanks for pointing out this.
> >>
> >>         thanks,
> >>         Amila.
> >>
> >>         [1]
> >>
> http://svn.apache.org/viewvc/webservices/axis2/trunk/java/modules/codegen/src/org/apache/axis2/wsdl/codegen/emitter/AxisServiceBasedMultiLanguageEmitter.java?r1=644817&r2=660424
> >>         <
> http://svn.apache.org/viewvc/webservices/axis2/trunk/java/modules/codegen/src/org/apache/axis2/wsdl/codegen/emitter/AxisServiceBasedMultiLanguageEmitter.java?r1=644817&r2=660424
> >
> >>
> >>
> >>
> >>
> >>             >     So you're telling me that Axis2 1.4 would generate
> >>             uncompilable names?
> >>             >
> >>             > No. It generates the names correctly. please see above.
> >>             The only thing
> >>             > is if a wsdl operation name starts with
> >>             > some thing like 'Foo' then the generated method name is
> >>             also 'Foo' which
> >>             > is not the java convention.
> >>
> >>             And if an operation name is "Do-Something"?
> >>
> >>             >     If there are two or more XML names which map to a
> >>             common Java name, then
> >>             >     it's our responsibility to disambiguate them,
> >>             typically by adding a
> >>             >     suffix.  So for XML operation names "Foo" "F-oO" and
> >>             "foo", you'd get
> >>             >     Java names "foo()", "foo2()", and "foo3()".  The
> >>             metadata/code should
> >>             >     handle making sure that each method correctly
> >>             serializes/deserializes
> >>             >     the appropriate XML.  This is the way Axis1 works, and
> >>             I believe it is
> >>             >     the way most other Java toolkits work.
> >>             >
> >>             > this is also an option. But isn't it better to go with the
> >>             way we were
> >>             > in the Axis2 1.4. Otherwise
> >>             > as you have mentioned this may cause problems to users.
> >>
> >>             However things were, we must do things the right way - if we
> >>             were doing
> >>             this in a buggy/wrong way before, then fixing that is a good
> >>             thing.
> >>
> >>             >     I disagree.  If ANY valid WSDL can produce Java code
> >>             that does not
> >>             >     compile, then we have a bug.  Just because we had a
> >>             bug before doesn't
> >>             >     mean it's ok to exchange one bug for another.
> >>             >
> >>             > I may not have made this clear.
> >>             > First Axis2 1.4 worked fine and Rampart was also worked
> >>             accordingly and
> >>             > worked fine.
> >>             >
> >>             > Then I made the change I have mentioned and *Rampart has
> >>             also changed*
> >>             > accordingly.
> >>             > Therefore now rampart is compatible with the first change.
> >>             >
> >>             > Then I reverted the first change and made this as an
> >>             option.  Since
> >>             > rampart is compatible with the first change now it is
> failing.
> >>             > Therefore basically reverting the change made to the
> >>             rampart (so that it
> >>             > looks like at the Axis2 1.4 release ) fix this issue.
> >>
> >>             As far as I can tell, we are still doing things wrong.
> >>              There should be
> >>             NO way, with an option or without, to ever generate
> >>             uncompilable Java
> >>             code from a valid WSDL.  As I understand it right now if you
> >>             do not
> >>             specify the "-lcmn" option we will not do XML->Java name
> >>             translation,
> >>             and if so, that is just broken.
> >>
> >>             Thanks,
> >>             --Glen
> >>
> >>
> >>
> >>
> >>         --
> >>         Amila Suriarachchi
> >>         WSO2 Inc.
> >>         blog: http://amilachinthaka.blogspot.com/
> >>
> >>
> >>
> >>
> >>     --
> >>     Amila Suriarachchi
> >>     WSO2 Inc.
> >>     blog: http://amilachinthaka.blogspot.com/
> >>
> >>
> >>
> >>
> >> --
> >> Amila Suriarachchi
> >> WSO2 Inc.
> >> blog: http://amilachinthaka.blogspot.com/
>



-- 
Amila Suriarachchi
WSO2 Inc.
blog: http://amilachinthaka.blogspot.com/

Mime
View raw message