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
> <[email protected] <mailto:[email protected]>> 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
>     <[email protected] <mailto:[email protected]>>
>     wrote:
> 
> 
> 
>         On Thu, Jan 8, 2009 at 8:22 PM, Glen Daniels
>         <[email protected] <mailto:[email protected]>> 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/

Reply via email to