Let me add to the chorus that this is great feedback.

I've looked at some of the issues reported here in the code generator, but have am not submitting a patch to fix them since someone else might already be working on them.  Plus, there are some questions on one of them.  Here are notes on the issues and possible fixes.  I could make the fixes if they are not already underway:

4.   Bug:  GetAppReleasesType.java does not compile.
In the GetAppReleasesType#Factory.parse method, a reference is made to the nonexistent method "org.apache.axis2.databinding.utils.ConverterUtil.convertToNonNegativeInteger(content)".
The correct method name is "org.apache.axis2.databinding.utils.ConverterUtil.convertTononNegativeInteger(content)".  Note:  There is a correct reference to the org.apache.axis2.databinding.utils.ConverterUtil.convertTononNegativeInteger method in the AppRelease#Factory.parse method.  This leads me to conclude that you are not using common methods to determine which conversions to perform on input vs. output content, and so bugs that are caught in one class may still be unidentified in another class.
The underlying problem here is in JavaBeanWriter.getShortTypeName().  The shortTypeName is used in ADBBeanTemplate to look up the correct ConvertTo method.  These methods are (mostly) named like ConvertTo<xsd-type-name>.  However getShortTypeName() generates (the non-package non-array substring of) <java-class-name> instead of <xsd-type-name>.   I believe the correct fix is to have JavaTypeMap maintain the inverse type map (Java class name --> xsd type name) in addition to the forward type map, and then to have getShortTypeName() use this inverse map.  This will however lead to some secondary bug(s) because there is at least one exception in the ConvertTo naming convention, namely, ConvertToString is used instead of ConvertTostring, and ConvertToString is called explicitly in places.

The subissue, "This leads me to conclude that you are not using common methods to determine which conversions to perform on input vs. output content, and so bugs that are caught in one class may still be unidentified in another class. ", is mostly a non-issue.  The reason for the two different names in the generated code can be see in the two different declarations in the wsdl.  Here is the element that generates ConvertTononNegativeInteger properly:

          <xsd:element type="xsd:nonNegativeInteger" name="releaseDispOrder" />

and here is the element that generates ConvertToNonNegativeInteger improperly:

          <xsd:element type="acre:hierarchyNum" name="siteNum" />

where hierarchyNum is defined:

      <xsd:simpleType name="hierarchyNum">
        <xsd:restriction base='xsd:nonNegativeInteger'>
          <xsd:maxInclusive value='99999'/>
        </xsd:restriction>
      </xsd:simpleType>

The difference in these two cases is that the first is a direct reference to an xsd type, while the second is a reference to a generated Java type that requires decoding to obtain the underlying xsd type.  It is this map from Java type to underlying xsd type, i.e. JavaBeanWriter.getShortTypeName() that is the problem.

5.   Bug:  In AppRelease.java, the values for localAvailabilityStopTracker and localReleaseDescTracker are not set properly.
In the setters for the attributes that these Trackers track, the following code should be used to ensure that the Trackers are set properly:
For localAvailabilityStopTracker -
    public void setAvailabilityStop(java.util.Date param) {

        // update the setting tracker
        localAvailabilityStopTracker = (param != null);

        this.localAvailabilityStop = param;
    }

For localReleaseDescTracker -
    public void setReleaseDesc(java.lang.String param) {

        // update the setting tracker
        localReleaseDescTracker = true;

        this.localReleaseDesc = param;
    }

As generated by WSDL2Java, the Tracker is set to true even if the variable it is tracking is unset back to null.

The situation is more subtle than this.  The first question is whether we want to support unsetting a property.  Currently we support this in precisely one case, a <choice> particle.  In that case, the convention is that setting any property unsets all others.  This is the only case in which unsetting can always be supported.  In the minOccurs=0 case it is not possible to unset a primitive type property, for example.  Another interesting case is a nillable property.  I.e., if a property is both minOccurs=0 and nillable, what does it mean to set it to null?  Should this be unsetting the property or explicitly setting it to xsd:nil?

We could support unsetting of properties for non-primitive types that are not nillable and have the unset convention be setting to null as proposed.  However, if want to support unsetting, perhaps a better approach is to provide an explicit unset<property> method?

I'm not convinced this is a bug as reported.  It might be a good idea to generate the suggested code, but only in the cases for which it is valid, which is not all cases.

6.   Bug:  In the AppReleaseResponse.setReturn method, the value of local_returnTracker may be set to true, but it will never be unset.
Similar to Bug #3, the local_returnTracker should be set using the following code to ensure that it is unset when the supplied parm is null:
    public void set_return(com.wendys.ws.apprelease.AppRelease[] param) {

        validate_return(param);

        // update the setting tracker
        local_returnTracker = (param != null);
        local_return = param;
    }

This is the same as issue 5.

7.   Annoyance:  In the AppReleaseResponse.addReturn method, local_returnTracker only needs to be set to true when the local_return array is instantiated, not every time an AppRelease is added to the array.

The generated code supports adding values using only the add() method.  I.e., it is not necessary to first initialize an array with the set() method.  So, the suggested optimization would entail a test and is thus not any less cycles that just setting the tracker.  I think the current code is fine.

Chuck



[EMAIL PROTECTED] wrote on 05/11/2006 03:38 PM:

I am trying to create a Proof of Concept (POC) for Enterprise and B2B Services at Wendy's Int'l, Inc.

As part of this POC, I created a WSDL for an in-house service and then used the WSDL2Java tool to translate this WSDL to Java Classes for use with ADB binding.

In this document, I describe the issues I encountered setting up what I consider to be a simple POC.
Some, if not most, of these issues should be opened as JIRA Bugs and Enhancement Requests, but I didn't want to open a bunch of little requests that may seem disconnected, nor did I want to create one huge request with all this stuff in it.

I encountered most of these issues in Axis2 0.95 and all of them in 1.0.

I'm attaching my WSDL file so that you may generate the files and follow along.  In the WSDL I documented choices that I was forced to make (like not including our Global Schema in the WSDL).  These choices were forced by the limitations of the ADB binding.

I'm also including my build.xml so you too can build the aar.  However, without the business logic and our legacy code and tables the resulting application will not do anything.   Finally, I'm including my AppReleases.css so you can compare how the returned WSDL appears in the Browser and to the way I want it to appear (assuming JIRA issue Axis2-618 is resolved).

I should say here that I chose ADB because the application framework created under ADB is more intuitive and lighter weight than the framework created under XMLBeans.

Some preliminary information about my environment and what was created by WSDL2Java.

Environment:
        Windows XP Pro SP2
        Tomcat 5.0.28
        Java 1.4.2_10
        Axis2 1.0 (and Axis2 0.95 except where noted)

Service Name:
        AppRelease

Messages In:
        getAppReleasesRequest
        getAppReleasesSortedRequest

Message Out:
        getAppReleasesResponse

WSDL2Java command:
        wsdl2java -o ..\ -s -p com.wendys.ws -t -ss -sd -d adb -g -ssi -pn AppReleasePort -sn AppRelease -uri .\AppRelease.wsdl

Files created by WSDL2Java (I've identified one new file that was created because of the new -ssi runtime directive):
        <mypackagepath>/
                AppReleaseMessageReceiverInOut.java
                AppReleaseSkeleton.java
                AppReleaseSkeletonInterface.java  (new)
                AppReleaseStub.java
        <mypackagepath>/apprelease
                AppRelease.java
                GetAppReleasesRequest.java
                GetAppReleasesResponse.java
                GetAppReleasesSortedRequest.java
                GetAppReleasesType.java


The reason that the following issues are important is because if I modify my application to fix them, and if I subsequently have to modify my WSDL (perhaps to add a new Operation or to change the definition of an element) and run WSDL2Java again, the following files will be overwritten (thus necessitating some reapplying my changes):
        <mypackagepath>/apprelease/AppRelease.java
        <mypackagepath>/apprelease/GetAppReleasesRequest.java
        <mypackagepath>/apprelease/GetAppReleasesResponse.java
        <mypackagepath>/apprelease/GetAppReleasesSortedRequest.java
        <mypackagepath>/apprelease/GetAppReleasesType.java


Now, for the issues.

1.   Annoyance:  WSDL2Java.bat issues a warning because Log4J cannot be properly configured.
When running WSDL2Java.bat, the following warning is displayed:
        log4j:WARN No appenders could be found for logger (org.apache.axis2.i18n.ProjectResourceBundle).
        log4j:WARN Please initialize the log4j system properly.

To fix this, add the following line:
        set AXIS2_CLASS_PATH=%AXIS2_CLASS_PATH%;%AXIS2_HOME%\modules\core\conf

into the WSDL2Java.bat file, after these lines:
        rem loop through the libs and add them to the class path
        set AXIS2_CLASS_PATH=%AXIS2_HOME%
        FOR %%c in ("%AXIS2_HOME%\lib\*.jar") DO set AXIS2_CLASS_PATH=!AXIS2_CLASS_PATH!;%%c

%AXIS2_HOME%\modules\core\conf is the location of the Axis2-distributed log4j.properties file.

Once I made this change I learned that the Sourceforge Jalopy jar is not available, and so the output of my Source files is not is not tidied up.  I don't know if the tidying up of files was available in Axis2 0.95, so I've continued to work with untidy java files.


2.   Bug:  Jalopy jar is not available in the Axis2 binary distribution.


3.  Annoyance:  WSDL2Java has the capability to tidy up Java source using Jalopy, but no similar functionality has been provide for the XML files nor the generated WSDL using xmlTidy.


4.   Bug:  GetAppReleasesType.java does not compile.
In the GetAppReleasesType#Factory.parse method, a reference is made to the nonexistent method "org.apache.axis2.databinding.utils.ConverterUtil.convertToNonNegativeInteger(content)".
The correct method name is "org.apache.axis2.databinding.utils.ConverterUtil.convertTononNegativeInteger(content)".  Note:  There is a correct reference to the org.apache.axis2.databinding.utils.ConverterUtil.convertTononNegativeInteger method in the AppRelease#Factory.parse method.  This leads me to conclude that you are not using common methods to determine which conversions to perform on input vs. output content, and so bugs that are caught in one class may still be unidentified in another class.


5.   Bug:  In AppRelease.java, the values for localAvailabilityStopTracker and localReleaseDescTracker are not set properly.
In the setters for the attributes that these Trackers track, the following code should be used to ensure that the Trackers are set properly:
For localAvailabilityStopTracker -
    public void setAvailabilityStop(java.util.Date param) {

        // update the setting tracker
        localAvailabilityStopTracker = (param != null);

        this.localAvailabilityStop = param;
    }

For localReleaseDescTracker -
    public void setReleaseDesc(java.lang.String param) {

        // update the setting tracker
        localReleaseDescTracker = true;

        this.localReleaseDesc = param;
    }

As generated by WSDL2Java, the Tracker is set to true even if the variable it is tracking is unset back to null.


6.   Bug:  In the AppReleaseResponse.setReturn method, the value of local_returnTracker may be set to true, but it will never be unset.
Similar to Bug #3, the local_returnTracker should be set using the following code to ensure that it is unset when the supplied parm is null:
    public void set_return(com.wendys.ws.apprelease.AppRelease[] param) {

        validate_return(param);

        // update the setting tracker
        local_returnTracker = (param != null);
        local_return = param;
    }


7.   Annoyance:  In the AppReleaseResponse.addReturn method, local_returnTracker only needs to be set to true when the local_return array is instantiated, not every time an AppRelease is added to the array.


8.   Bug:  Under Axis2 1.0, only the system generated WSDL is served up in response to the Service?wsdl command.  Under Axis2 0.95 if a WSDL was located in the META-INF directory then that WSDL would be served up instead.  This has already been submitted as JIRA Bug Axis2-690.


9.   Annoyance:  WSDLs are not served up with the Stylesheet that is specified in the original WSDL.  In fact, under 0.95, even if the original WSDL is returned that WSDL has the stylesheet element removed.  This has been submitted as JIRA Enhancement Request Axis2-618.


10.   Annoyance:  In AppReleaseStub, the default constructor is not necessary.
This constructor is used only by the Test class, which can supply the necessary service endpoint URL when it instantiates the stub class.  This way, the URL of the Service is not hard-coded anywhere in the application.


11.   Annoyance:  WSDL2Java reverses the order of the Operations listed in the WSDL.
In my WSDL the Operations are listed in this order:  getAppReleases, getAppReleasesSorted.  Yet, in all the artifacts created by WSDL2Java, the operations, their messages, their associated methods, their entries in the services.xml file, their order in the generated WSDL file and their order in the axis2-admin/listServices page is reversed from what exists in my WSDL file.  I prefer to have messages and operations in WSDLs and attributes and methods in Classes in alphabetical order.  I prefer that my tools maintain the order that I have established.


12.  Annoyance:  WSDL2Java replaces my defined namespace prefix with "ns1".
See AppRelease, GetAppReleasesRequest, GetAppReleasesResponse, GetAppReleasesSortedRequest and GetAppReleasesType.
I see no reason not to use the namespace prefix specified in the WSDL that WSDL2Java reads, but if you MUST change the namespace prefix couldn’t you use the more traditional "tns"?


13.  Annoyance:  There are a large number of unused private methods, method parameters and method variables in the generated classes.
Eclipse can tell you about unused junk in the generated classes.  Perhaps these artifacts are no longer needed and can be removed?


14.  Annoyance:  The log4j.properties file shipped with Axis2 circumvents the ability for the developers to change the logging level of their classes.
I have included a modified log4j.properties file that actually allows me to put my classes into DEBUG level while leaving Axis2 and Tomcat logging at their levels.


15.  Annoyance:  Axis2 only allows for Log4J.properties to configure logging, not Log4J.xml
Log4J can be configured using either a properties or an xml file.  There is a growing number of Logging capabilities that are only configurable using the XML file.  The code that sets up configuration of Log4J in Axis2 should allow for both types of configuration files.


16.  Annoyance:  Axis2 does not use ConfigureAndWatch capabilities of Log4J
Log4J can be configured using a ConfigureAndWatch method that causes Log4J to monitor its configuration file and automatically apply changes made to that file to its configuration.  This means that if support personnel need to modify the logging configuration of a production application, only the log4j configuration file needs to be modified and the Application server does not need  to be restarted.

Following is a snippet of code for a Log4jUtils class that addresses Annoyances 15 and 16.
The configureAndWatch method of this utility class should be called by an initialization servelet or some other configuration and initialization class.

------

import org.apache.log4j.BasicConfigurator;
import org.apache.log4j.PropertyConfigurator;
import org.apache.log4j.helpers.FileWatchdog;
import org.apache.log4j.net.SyslogAppender;
import org.apache.log4j.xml.DOMConfigurator;

    .
    .
    .
/**
 * A collection of static utility methods for logging with Log4J.
 */
public final class Log4jUtils {
    private static final Logger LOG = Logger.getInstance(Log4jUtils.class);
    private static loggingConfigured = false;

    private Log4jUtils() {
        // All methods are static, this class should not be instantiated
    }

    .
    .
    .

    /**
     * Implements configureAndWatch for the current ClassLoader hierarchy using the Log4J
     * default number of seconds.
     */
    public static void configureAndWatch() {
        configureAndWatch(FileWatchdog.DEFAULT_DELAY);
    }
   
    /**
     * Implements configureAndWatch for the current ClassLoader hierarchy.
     *
     * @param watchMillisecs The number of milliseconds to wait between checking to see if the
     * log4j configuration file has changed.
     */
    public static void configureAndWatch(long watchMillisecs) {
        if (loggingConfigured) return;

        Class myClass = Log4jUtils.class;
        ClassLoader classLoader = myClass.getClassLoader();

        // Initialize LOG4J
        URL resourceUrl = classLoader.getResource("log4j.xml");
        if (resourceUrl != null) {
            String configFile = resourceUrl.getFile();
            DOMConfigurator.configureAndWatch(configFile, watchMillisecs);
        } else {
            resourceUrl = classLoader.getResource("log4j.properties");
            if (resourceUrl != null) {
                String configFile = resourceUrl.getFile();
                PropertyConfigurator.configureAndWatch(configFile, watchMillisecs);
            } else {
                // Since we aren't going to be able to pull our "real" settings,
                // let's at least make sure we get some output
                BasicConfigurator.configure();

                // Log an error with detail regarding the problem (and suspected cause)
                LOG.error(myClass.getName(), "Unable to find log4j.xml or log4j.properties. " +
                                             "Make sure the log4j configuration file is in the Classpath.");
            }
        }
        loggingConfigured = true;
    }


Hope this all helps.



Mike McAngus

Associate Chief Engineer, Enterprise Architecture
Wendy's International, Inc.




--
Chuck Williams
Manawiz
Principal
V: (808)885-8688
C: (415)846-9018
[EMAIL PROTECTED]
Skype: manawiz
AIM: hawimanawiz
Yahoo: jcwxx

Reply via email to