cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Kulp <dk...@apache.org>
Subject Re: svn commit: r607390 - in /incubator/cxf/trunk/maven-plugins/codegen-plugin/src/main/java/org/apache/cxf/maven_plugin: WSDL2JavaMojo.java WsdlOptionLoader.java
Date Fri, 04 Jan 2008 03:31:21 GMT
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.

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


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

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));



-- 
J. Daniel Kulp
Principal Engineer, IONA
dkulp@apache.org
http://www.dankulp.com/blog

Mime
View raw message