axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Davanum Srinivas (JIRA)" <j...@apache.org>
Subject [jira] Commented: (AXIS2-736) [ADB] Feedback from Wendy's POC
Date Mon, 15 May 2006 15:22:06 GMT
    [ http://issues.apache.org/jira/browse/AXIS2-736?page=comments#action_12402329 ] 

Davanum Srinivas commented on AXIS2-736:
----------------------------------------

Comments from Chuck on the mailing list:

> 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

> [ADB] Feedback from Wendy's POC
> -------------------------------
>
>          Key: AXIS2-736
>          URL: http://issues.apache.org/jira/browse/AXIS2-736
>      Project: Apache Axis 2.0 (Axis2)
>         Type: Bug

>     Reporter: Davanum Srinivas

>
> See original email here:
> http://mail-archives.apache.org/mod_mbox/ws-axis-dev/200605.mbox/%3cOF220CACBE.411DB84B-ON8525716B.0074F142-8525716B.007C5C4F@wendys.com%3e
> 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.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message