Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve conflicting XMLNS imports #23

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

veewee
Copy link
Member

@veewee veewee commented Sep 27, 2024

Q A
Type bug/
BC Break no
Fixed issues

Summary

Cross-import schemas can contain namespace conflicts.

For example: import1 requires import2:

  • Import 1 specifies xmlns:ns1="urn:1"
  • Import 2 specifies xmlns:ns1="urn:2".

This method will detect conflicting namespaces and resolve them.
Namespaces will be renamed to a unique name and the "type" argument with QName's will be re-prefixed.

/cc @rauanmayemir This should fix the flattening of your sync API ;) Can you give it a try?

@veewee veewee force-pushed the resolve-conflicting-xmlns branch 2 times, most recently from ca4821c to 9de8b26 Compare September 27, 2024 06:05
@rauanmayemir
Copy link

@veewee Types are being generated corrected now. 👍

Jetbrains idea view can't recognize the flattened wsdl properly, I wonder if generated dump could be more explicit?
image

@veewee veewee force-pushed the resolve-conflicting-xmlns branch 2 times, most recently from 78aee04 to 50711a0 Compare September 27, 2024 07:25
@veewee veewee force-pushed the resolve-conflicting-xmlns branch from 50711a0 to c0fc5a5 Compare September 27, 2024 07:44
@veewee
Copy link
Member Author

veewee commented Sep 27, 2024

@rauanmayemir

I don't know why PHPStorm is not able to resolve this type declaration.
Everything seems to be fine XSD-wise, so I suppose it's an issue in how PHPStorm parses these flattened schema's in comparison to how it does it when they are in separate files.
Although: When I copy the file to a new project, PHPStorm is able to understand the file correctly. It seems like it is searching in the non-flattened files for some reason.

In any case: Both the wsdl-reader package and SoapUI is able to read and understand the document without errors. That was the main goal of this PR.

About:

I wonder if generated dump could be more explicit?

What do you mean exactly?

@rauanmayemir
Copy link

It seems like it is searching in the non-flattened files for some reason.

Ah, that would make the most sense.

What do you mean exactly?

Basically, I mean what if all xmlns were rewritten to avoid the namespace clashes, not just conflicting ones.

Usually I follow this convention where I have a local tns which could clash when the schema is flattened.

@veewee
Copy link
Member Author

veewee commented Sep 27, 2024

Basically, I mean what if all xmlns were rewritten to avoid the namespace clashes, not just conflicting ones.

So far, I haven't seen that many conflicting situations in real-world examples. This one was the first.
The service you provided had some namespace optimization done, which was just an increment of bons[i++].

A lot of other services give some more meaningful prefix so that you get a clue of where the data is being linked to.
These namespaces are also being used by the encoding package to prefix the generated XML during encoding.
As a human, this gives you an instant idea of what xsd the generated XML is being linked to.

For that reason, I don't think the flattening process should rewrite ALL namespaces, at least not by default.
Another reason would be the technical reason: Re-prefixing in XSDs is a matter of replacing type arguments. But in WSDL context, QName's are being used all over the place. It is a lot harder to replace them there.

For now, I suppose this PR unblocks your issue?

@rauanmayemir
Copy link

You're right, most of the cases are simpler.
Yes, this has definitely unblocked our issue. 🙌

@veewee veewee merged commit 8975e4f into php-soap:main Sep 27, 2024
9 checks 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.

2 participants