Re: svn commit: r607390 - in /incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/apache/cxf/maven_plugin: WSDL2JavaMojo.java WsdlOptionLoader.java

[James Mao]

Let's off this thread, and put the comments in the Jira CXF-1246, some 
points we made here are valuable, we should log that in the jira as well
Make sense?

James

James Mao wrote:
Hi Dan,

On Thursday 03 January 2008, James Mao wrote:
 
This commit is breaks all kinds of things and I think needs to be
rolled back.   I certainly need to roll it back off the 2.0.4
branch.

Problems I see:
1) By default, it grabs *.wsdl from src/main/resources/wsdl to
process. This isn't backwords compatible at all so I need to roll
that off the 2.0.4 branch.  For example, the 2.0.4 testutils fails
to build with it as there are wsdls in there (the catalog stuff)
that cannot/shouldnot be processed.
      
By default, it grabs *.wsdl from the $wsdlRoot, unless it's not set
    

No, it grabs it from src/main/resources/wsdl if it's not set.   
That's the problem.   If it didn't do anything if wsdlRoot wasn't 
set, I'd be OK with it.
  

Previously, yes, it won't do anything if wsdlRoot is not set.
but in this case if you do mvn wsdl2java:wsdl2java, nothing will happen.
and this is what I need to fix for the CXF-1246

 
I have moved those shouldnot/cannot be processed wsdls out of
testutils to the places that they should belong,
To put all of the wsdls in testutils does not make any sense to me.
    

Which is fine for 2.1, but not for 2.0.4.   If the change causes our 
own build to break, it's not a backword compatible change and not 
something we should put on the 2.0.x patches branch.    Our users 
could have similar setups and updating to 2.0.4 should cause strange 
breakages.
  

Sorry, I probably will not care too much about the 2.0.4, as I 
commented in the CXF-1246, it is for the CXF 2.1 release.
So, please do merge anything to 2.0.4 branch, that definitely will 
have a lot of work if we do so.

Oops, I mean *do not*




 
I guess 2.0.4 could do the same thing? I can merge the commits to
2.0.4 if we would like to do that.

If not, just use the <wsdlOption/>s to specify the wsdls you want to
build, and set the wsdlRoot to '/dev/null' to turn off the auto
loading.
    

Again, that's not a backword compatible thing. I think what we need 
to do for 2.0.4 is make it NOT scan anything if wsdlRoot is not 
set.  Provide an expression to set it when calling from the command 
line.

Basically just:
   /**
     * @parameter expression="${wsdl2java.wsdlRoot}"
     */
    File wsdlRoot;


 
3) defaults should be set with default-value tags on the @parameter
so they are documented.   They shouldnt be in the code.   Probably
should have a separate var for testWsdlRoot.
      
I will think about it, but since we are using maven, and so I prefer
the so-called CoC principle,
Hard code is not always a bad thing. I personally prefer less options
    

Umm....  we are using maven and thus we should be following the same 
practices that maven uses, which is to use the flags on the 
@parameter stuff and let plexus inject it.    

But no default value should be set on the @parameter, we should hard 
code it if it's null, right?

James

Dan

 
4) They probably should be a File object, not a String.


For example:
    /**
     * @parameter expression="${wsdl2java.wsdlRoot}"
default-value="${basedir}/src/main/resources/wsdl"
     */
    File wsdlRoot;

That would provide a good default, but also also setting it via a
command line:

mvn codegen-plugin:wsdl2java -Dwsdl2java.wsdlRoot=target/foo

Actually, sourceRoot and testSourceRoot should probably have
expressions to make them settable on the command line as well.


Looking at it, I'm wondering if we should make the mojo abstract and
make two subclasses: one for source and one for tests.
wsdl2java
wsdl2javatests
or similar.   It would definitely simplify it quite a bit.

Dan

mmao wrote:
    
Author: mmao
Date: Fri Dec 28 23:18:46 2007
New Revision: 607390

URL: http://svn.apache.org/viewvc?rev=607390&view=rev
Log:
CXF-1246
  Support run wsdl2java in the style of
     mvn
org.apache.cxf:cxf-codegen-plugin:2.1-incubator-SNAPSHOT:wsdl2java


Modified:
        
incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/a
pache/cxf/maven_plugin/WSDL2JavaMojo.java




incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/a
pache/cxf/maven_plugin/WsdlOptionLoader.java

    
Modified:
        
incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/a
pache/cxf/maven_plugin/WSDL2JavaMojo.java

    
URL:
        
http://svn.apache.org/viewvc/incubator/cxf/trunk/maven-plugins/codeg
en-plugin/src/main/java/org/apache/cxf/maven_plugin/WSDL2JavaMojo.jav
a?rev=607390&r1=607389&r2=607390&view=diff

====================================================================
==========

    
---
        
incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/a
pache/cxf/maven_plugin/WSDL2JavaMojo.java

    
(original)
+++
        
incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/a
pache/cxf/maven_plugin/WSDL2JavaMojo.java

    
Fri Dec 28 23:18:46 2007
@@ -87,6 +87,16 @@
      */
     boolean useCompileClasspath;

+    private List<WsdlOption> getWsdlOptionsFromDir(final File
root) throws MojoExecutionException {
+        List<WsdlOption> options = new ArrayList<WsdlOption>();
+        for (WsdlOption o : new WsdlOptionLoader().load(root)) {
+            if (!options.contains(o)) {
+                options.add(o);
+            }
+        }
+        return options;
+    }
+
     public void execute() throws MojoExecutionException {
         String outputDir = testSourceRoot == null ? sourceRoot :
testSourceRoot;
         File outputDirFile = new File(outputDir);
@@ -95,22 +105,25 @@
         File classesDir = new File(classesDirectory);
         classesDir.mkdirs();

-
-        if (wsdlRoot != null) {
-            List<WsdlOption> options = new
ArrayList<WsdlOption>(); -            if (wsdlOptions != null) {
-                options.addAll(Arrays.asList(wsdlOptions));
+        List<WsdlOption> options = new ArrayList<WsdlOption>();
+        if (wsdlRoot == null) {
+            File sourceWsdlRoot = new File(project.getBasedir(),
"/src/main/resources/wsdl");
+            if (sourceWsdlRoot.exists()) {
+               
options.addAll(getWsdlOptionsFromDir(sourceWsdlRoot)); }
-
-            for (WsdlOption o : new
WsdlOptionLoader().load(wsdlRoot))
        
{

    
-                if (!options.contains(o)) {
-                    options.add(o);
-                }
+
+            File testWsdlRoot = new File(project.getBasedir(),
"/src/test/resources/wsdl");
+            if (testWsdlRoot.exists()) {
+               
options.addAll(getWsdlOptionsFromDir(testWsdlRoot)); }
-            wsdlOptions = options.toArray(new
WsdlOption[options.size()]);
         }

-        if (wsdlOptions == null) {
+        if (wsdlOptions != null) {
+            options.addAll(Arrays.asList(wsdlOptions));
+        }
+        wsdlOptions = options.toArray(new
WsdlOption[options.size()]); +
+        if (wsdlOptions.length == 0) {
             getLog().info("Nothing to generate");
             return;
         }

Modified:
        
incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/a
pache/cxf/maven_plugin/WsdlOptionLoader.java

    
URL:
        
http://svn.apache.org/viewvc/incubator/cxf/trunk/maven-plugins/codeg
en-plugin/src/main/java/org/apache/cxf/maven_plugin/WsdlOptionLoader.
java?rev=607390&r1=607389&r2=607390&view=diff

====================================================================
==========

    
---
        
incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/a
pache/cxf/maven_plugin/WsdlOptionLoader.java

    
(original)
+++
        
incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/a
pache/cxf/maven_plugin/WsdlOptionLoader.java

    
Fri Dec 28 23:18:46 2007
@@ -38,14 +38,16 @@
     private static final String WSDL_BINDINGS =
"-binding-?\\d*.xml$";

     public List<WsdlOption> load(String wsdlRoot) throws
MojoExecutionException {
-        File wsdlBasedir;
-        if (wsdlRoot == null) {
+        return load(new File(wsdlRoot));
+    }
+
+    public List<WsdlOption> load(File wsdlBasedir) throws
MojoExecutionException {
+        if (wsdlBasedir == null) {
             return new ArrayList<WsdlOption>();
         }
-        wsdlBasedir = new File(wsdlRoot);

         if (!wsdlBasedir.exists()) {
-            throw new MojoExecutionException(wsdlRoot + " not
        
exists");

    
+            throw new MojoExecutionException(wsdlBasedir + " not
exists");
         }

         return findJobs(wsdlBasedir, getWsdlFiles(wsdlBasedir));