savonrb / wasabi

A simple WSDL parser
MIT License
90 stars 84 forks source link

fixed problem with port_message part in case there is no namespace set #38

Closed dabooze closed 10 years ago

dabooze commented 10 years ago

Some WSDL's don't have a namespace set (separated by a colon) in the operation names and hence this needs to be taken care if in order to successfully determine the correct message name for sending out the SOAP request.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 2211f2e5b1dfce71f3f1c4cb677245bfea4d809a on dabooze:master into 5a03d45c90350a6c02986c0c94ab2c32b91f81f3 on savonrb:master.

tjarratt commented 10 years ago

Hey @dabooze your change looks good, it's fairly straightforward. Would you mind adding a simple test for this?

tjarratt commented 10 years ago

Actually, this part of the code is currently causing problems for Savon, see https://github.com/savonrb/wasabi/issues/32 and https://github.com/savonrb/savon/issues/520

I'd hold off on writing any tests until we get this sorted out.

dabooze commented 10 years ago

Interesting. Ok I'll wait until it's clear whether the Wasabi patch should be rolled back or Savon code needs changes.

tjarratt commented 10 years ago

My impression is that this fix is valid, but that we need to be more careful when we use the operation name rather than the port_message name. Each time I look into this, I understand the issue a little more, but it's still rather opaque as to what the correct behavior is in all cases.

Thank you for understanding!

tjarratt commented 10 years ago

I had to merge this in by hand because of the recent changes to Wasabi::Parser.

Thank you so much for issuing this pull request @dabooze. If you can, and you'd like to add your WSDL (or a similar WSDL) as a fixture and write some better tests for this behavior, I'd be more than happy to merge that in.

The tests I added in https://github.com/savonrb/wasabi/commit/64fe56a5de9cded71f50b8e755f082eeb7d2f774 aren't the best. There was an example WSDL already in the project and I made a new operation that matched what I expected an operation with a port message but no colon for a namespace would look like (based on some other WSDLs I've seen).