Demodulize names of Reusable types if they are located in a module

[jochemsiegel]

Added a demodulize to the wash_out_param_name function for types, to avoid a complexType being generated with a "module/class_name" structure.

I'm not sure if everyone considers this desired functionality, but I intend to use quite a few reusable types, and want to group them in a separate module. This fix insures the complexType name generated won't include the module name (which as far as I can see is not supported in WSDL's anyway).

Example:

soap_action :test_action,
            args: TestModule::Request,
            return: TestModule::Response

will currently generate the following complexType name in the WSDL:

<xsd:complexType name="test_module/request">

Which isn't supported in a WSDL.

The fix changes it tot this:

<xsd:complexType name="request">

[inossidabile]

The reason to make this is clear. But on the other hand won't it bring you possible collisions? I mean if you use modules for complex types, it's easily possible to duplicate names on the level of Ruby. This will lead to a bug that will be a huge pain to debug.

Shouldn't we just replace / with _ to give it a long but consistent name?

[jochemsiegel]

As far as I can see, the wash_out_param_name function is currently only used directly for WSDL generation. The only collision issue i can see that would occur would be that it would only generate one of the identically named types that are in a different module. I think this would only occur in the WSDL though, but not on class level. Also this would only occur if you use these identically named classes in the same soap_action, which doesn't sound very likely to me.

Perhaps i'm missing something though.

[inossidabile]

This is exactly the case that I'm affraid of. And it's very likely, that's what I believe in. You are right in that it's only used directly for WSDL generation. Therefore it doesn't make a lot of sense to save name length, right? And if we have a free way to avoid such collision – we don't have an option to not do that.

Please change demodulize to .gsub '/', '_' and I will merge.

[inossidabile]

@jochemsiegel ping? :)

[jochemsiegel]

Sorry, I was abroad this weekend. I will take one more shot at trying to convince you, so i hope you will bear with me. If you're not convinced, will change the pull request :)

I'm struggling with including the module name in the WSDL, for all your clients to see it as a request. The modularization to me is more a coding best practice, to keep the code readable and easily understandable, and for me should not be exposed to the clients of my SOAP service.

I'm not saying this is not an issue though, because it would indeed be hard to debug.

For this reason however, i think merely converting the module name would pollute the interface, and I think isn't the right solution. As a safety for the collision I could build a safety that catches this in the soap_action method?

[inossidabile]

and for me should not be exposed to the clients of my SOAP service.

I however can't see any issues with the fact that consumers will see module path. You can always manually hide it right?

So to finalize – would you agree that's it's much more a case when you don't care what exactly gets exported to WSDL? And in cases when you do – you just have a way to affect it (which you actually have in WashOut).

Considering this I'm absolutely sure we should go the way when things just work and don't make you think once again. Even in edge cases. So I'd still ask you to modify the PR. And thank you very much for detailed responses 👍

[jochemsiegel]

You can always manually hide it right?

Hmm. Reason i was so adamant is because i didnt find any possiblity to change the name of the complexType in the WSDL. I've been going through the source but can't find this functionality. How would I do this? Make my own builder? Sorry for my ignorance...

If that is indeed the case, one last thing. Is it an idea to make it .gsub '/', '.' instead? To avoid collisions of classes that have the same name as a module + class.

[inossidabile]

I would be happy with '.'.

[inossidabile]

Regarding changing the name – just override the self.wash_out_param_name method on the level of descendant. And that's pretty much it ;)

[jochemsiegel]

Wow... totally missed that :) Will change the pull request. Thx for the in depth responses!

[inossidabile]

ehm. Shouldn't it be name.underscore.gsub('/', '.')? You missed the order of calls :)