Skip to content

Conversation

@smokhov
Copy link
Contributor

@smokhov smokhov commented Aug 18, 2025

This PR supersedes PR #1226 (by @paulish). For now simply sync'ing it with master. Unsure yet what type of test it would have... It seems covered? But I don't have a concrete example in mind.

@w666 w666 self-requested a review September 2, 2025 09:46
@w666
Copy link
Collaborator

w666 commented Sep 2, 2025

Will have a look in this one later. It is in my backlog.

@w666
Copy link
Collaborator

w666 commented Sep 26, 2025

Will have a look in this one later. It is in my backlog.

fyi: I am working on a test for this one when have time, but due to other issues in this project this is not a first priority.

@w666
Copy link
Collaborator

w666 commented Oct 20, 2025

Was looking into this past week. Still not sure I understand the problem because I managed to create a WSDL file that breaks it.
Basically, when there are two services and port bindings are in order of appearance it works, but once I change, for example, port order it breaks parsing. I assume this is what this PR was about. But it does not work, moreover it works same way before the fix.

Anyway, I continue investigating this. At least I have a good WSDL and a template for a test.

@w666 w666 force-pushed the paulish-service-name-port-binding-1226 branch from 4802783 to 2b39cf4 Compare October 22, 2025 04:54
@w666
Copy link
Collaborator

w666 commented Oct 23, 2025

Reverted original change, I think it is not correct, moreover it does not fix anything. Instead fixed looked through service->port bindings. Added new multi-service/multi-port WSDL and new tests.

@smokhov would you be able to review this, in case I overlooked something?

@smokhov
Copy link
Contributor Author

smokhov commented Oct 24, 2025

Reverted original change, I think it is not correct, moreover it does not fix anything. Instead fixed looked through service->port bindings. Added new multi-service/multi-port WSDL and new tests.

@smokhov would you be able to review this, in case I overlooked something?

I had a look at the changes, you've done quite a comprehensive test coverage for the issue, so I think it's good to go. I guess the code has improved since the times of the original PR so the original change became redundant, but it's nice now to have it fully covered as well.

@w666 w666 changed the title fix: serviceName and portName must correspond to port binding (take 2) Add support for multi-service and multi-port binding WSDL files Oct 24, 2025
@w666 w666 merged commit 95fd44a into vpulim:master Oct 24, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants