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
|