Possible fix for #162: incomplete schema compare when using Element refs

[ithena]

When using ref constructs for elements the name of the element is always null. In the compare algorithm a.name != b.name is regularly used to find differences, but when using ref's no change is found (since null == null).

This fix overwrites the Element getName() method to return the ref (as String) when no name has been set. This way the compare algorithm does find differences where a.name != b.name is used.

Overwriting the getName() method is probably a shortcut that causes other inconsistencies (although only one existing test fails, details below). In this case a correcter approach would be to extend the compare algorithm to add a.ref != b.ref statements wherever applicable. Could you advise on this?

After this change one assertion in one test case failed:

OperationUsageAnalyzer4OTATest.testAnalyzeOperationUsage() {
    assert 150 == OperationUsageAnalyzer.analyzeOperationUsage(wsdl).elementsInfo.size()
}

after the update 155 usages were found instead of the expected 150. These 5 extra usages found are Elements that use ref instead of name+type. Could you validate if this result (the 5 extra usages) is correct or incorrect?

Println of the toString() for the extra 5 elements:**

element[name={http://www.opentravel.org/OTA/2003/05}TPA_Extensions,type=null,ref={http://www.opentravel.org/OTA/2003/05}TPA_Extensions,embeddedType=null]=[PortType: hotelPortType, Operation: otaHotelResNotif (input: true, output: false, fault: false)]
element[name={http://www.opentravel.org/OTA/2003/05}RateAmountMessages,type=null,ref={http://www.opentravel.org/OTA/2003/05}RateAmountMessages,embeddedType=null]=[PortType: hotelPortType, Operation: otaHotelRateAmountNotif (input: true, output: false, fault: false)]
element[name={http://www.opentravel.org/OTA/2003/05}TPA_Extensions,type=null,ref={http://www.opentravel.org/OTA/2003/05}TPA_Extensions,embeddedType=null]=[PortType: hotelPortType, Operation: otaCancel (input: true, output: false, fault: false)]
element[name={http://www.opentravel.org/OTA/2003/05}TPA_Extensions,type=null,ref={http://www.opentravel.org/OTA/2003/05}TPA_Extensions,embeddedType=null]=[PortType: hotelPortType, Operation: otaCancel (input: true, output: false, fault: false)]
element[name={http://www.opentravel.org/OTA/2003/05}TPA_Extensions,type=null,ref={http://www.opentravel.org/OTA/2003/05}TPA_Extensions,embeddedType=null]=[PortType: hotelPortType, Operation: otaCancel (input: false, output: true, fault: false)]

[keshavarzi]

Thank you for your pull request. It would fix the current bug but I would rather add some constraints to dffGenerator instead and left the getName() as it is.

[ithena]

That indeed seems to be a safer and more appropriate fix. I will have a look and update the pull request when ready.

[ithena]

@keshavarzi , What would be a good name for where the name or ref is used for element identification. For most schema components the name will be used, but for elements the ref will be used if no name is set.

In the code a new method, e.g. String getIdentification(), would be introduced in the SchemaComponent class that simply returns the name. On Element level this method would be overwritten to return the name, and if not set ref.toString().

The different algorithms, e.g. the Schema compare algorithm, would then use this new method everywhere .name is currently used.

But what would be a correct and descriptive name for this method: getIdentification(), ... ?

And should this method always use the name, except for the specified case in Element? Or would it be correcter to use qName for declarations, ... ?

[ithena]

@keshavarzi , I updated the fix so that there are only changes in the compare algorithm. Still searching for an appropriate name for the currently called getIdentification() method.

[keshavarzi]

Thanks a lot Ruben. Based on your idea we are discussing and implementing a fix here, which would be applied in the next few days.

[ithena]

Any idea on planning when your change for the refs will be implemented?

[keshavarzi]

Maybe tomorrow (but no guaranty). It should be done by the end of this week ;)

[ithena]

ok, thx for the info

[keshavarzi]

I've just committed my changes to SeqenceDiffGenerator. Please check it out and let me know if any thing doesn't work as expected.

[ithena]

keshavarzi, Some of the Difference descriptions still exclusively use Element.name, resulting in descriptions like "Position of element null changed from 4 to 6."

[keshavarzi]

@ithena please check the latest commit out and have a new try!