Operation output

[leoping]

This pull request addresses two issues:

1. Fixed bug where the fallback logic for operation's input message type was not correct (it was looking at operation_name instead of port_mesage_type). For example, see fixed specs where it was (incorrectly) identifying "sendsms" as the input name, instead of the correct "sendsmsRequest".
2. Included operation's output to the operations hash, to be consistent with the already-included operations input.

We have tested our changes against the wasabi specs, as well as the specs of savon and savon-multipart, and they all pass (on a local test machine). We will look into the Travis build report ASAP.

[egilburg]

Hi, this is a co-worker of leoping. I looked at the Travis build and it seems to be passing on all platforms except jruby, which fails with a gem install error (nokogiri). Since we haven't changed any gem dependencies in our pull request, we are not sure why this is happening or how to fix it, but it seems to not be related to our changes.

[timabdulla]

hm. that's a strange reason for failure. i'll investigate further when i have a chance.

[timabdulla]

@rubiii - do you think that the operation output should be included in the return value of operations? i believe that this would actually be quite useful.

[timabdulla]

that said, i'm not sure that i agree with the implementation of the feature in this PR.

[rubiii]

@timabdulla well, basically the code was moved from input_for to input_output_for, right? are you talking about the second argument for that method?

i'm not sure whether this new piece of information can be used to improve savon, but it looks like a nice addition.

[leoping]

Thanks @timabdulla and @rubiii for looking into this PR, and thanks @timabdulla for investigating the failed Travis build for jRuby. @timabdulla, could you please provide a bit more details on your comments about not agreeing to the implementation of the feature at your earliest convenience? We'll be glad to make any changes if necessary. Thanks!

[rubiii]

this should be fixed on master. sorry that this took me so long. i updated the changelog to reflect the changes on master. if you would like to test the new code before the release, please give it a try and let me know if it works for you. it's probably still weeks until 4.0.0, but the sooner i can get feedback, the better. thanks.

[leoping]

Thanks @rubiii. We'll test the new code asap and let you know.

[alvinj01]

@rubiii We cannot test Wasabi 4.0.0 because we also need a version of Savon that uses Wasabi 4.0.0. Is there something else that we can work with?

[rubiii]

hey @leoping,

my plans for savon version 3 changed to not use wasabi, but to include the code for the new parser into savon. since savon version 2 will keep using wasabi, i merged your changes and i'm planning to release them with savon v2.3.0.

hope you can test your app against wasabi master (and the savon version2 branch) if you can.

[leoping]

Thanks @rubiii! We'll test our code with savon version2 branch and wasabi master and let you know. For Savon version 3, what do you mean by "include the code for new parser into savon"? Which new parser are you referring to? Thanks.

[leoping]

@rubiii, nvm, i guess you mean a new parser similar to lib/wasabi/parser.rb right?

[rubiii]

right. savon master is what will become savon version 3. if you look at the code, you can see that it doesn't depend on wasabi any longer, but ships with all the code i've been working on for wasabi 4.

[paracycle]

I think part of this patch caused a regression in Savon. The first item in the list of the original PR message is not valid. The name of the operation (and thus the message_tag) should be read from operation[@name] and not from input[@message].

With this patch the behaviour of Savon is not inline with other tools (for example SoapUI).

For reference, see my comments in savonrb/savon#520.

Thus either this patch should be partially rolled back, or Savon should implement a different logic when deciding on the message tag name.

[tjarratt]

I'm looking into this issue now. My current thoughts are that @paracycle's analysis is spot on, and that we'll need to revert this issue and figure out which part of this PR caused the regression in Savon.

[coveralls]

Coverage remained the same when pulling 8413ba8b41cd26ece1fc5d6ca69d0dea239ac8e3 on Sage:operation_output into e080f40c272df66f5c4c9dac88c15b8940ded59c on savonrb:master.