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

Named XSD Element with unnamed complexType child generates invalid python. #97

Open
techdragon opened this issue May 2, 2017 · 4 comments

Comments

@techdragon
Copy link
Contributor

techdragon commented May 2, 2017

I'm dealing with a 3rd party vendor and their XSD schema seems to have revealed a shortcoming of soapfish's xsd2py functionality, and by extension, its wsdl2py functionality as well.

The trouble seems to be when an element and complex type are used like this in an XSD.

<xs:element name="fooCreateRequest">
    <xs:complexType>
        <xs:complexContent>
            <xs:extension base="FooCreateRequestBaseType"/>
        </xs:complexContent>
    </xs:complexType>
</xs:element>

<xs:complexType name="FooCreateRequestType">
    <xs:annotation>
        <xs:documentation>Foo Details to be created</xs:documentation>
    </xs:annotation>
    <xs:complexContent>
        <xs:extension base="FooCreateRequestBaseType">
            <xs:attribute name="version" type="xs:decimal" default="1" />
        </xs:extension>
    </xs:complexContent>
</xs:complexType>

Initially this was just failing to generate at all, throwing a TypeError: 'NoneType' object is not subscriptable, one quick edit to the capitalize function in soapfish/utils.py and I was able to get it to generate the following python.

class FooCreateRequestType(FooCreateRequestBaseType):
    XSI_TYPE = xsd_types.XSDQName('https://www.example.com/ws', 'FooCreateRequestType')
    INHERITANCE = xsd.Inheritance.EXTENSION
    version = xsd.Attribute(xsd.Decimal, use=xsd.Use.OPTIONAL)


class (FooCreateRequestType):
    XSI_TYPE = xsd_types.XSDQName('https://www.example.com/ws', 'fooCreateRequest')
    INHERITANCE = xsd.Inheritance.EXTENSION

So it's not generating the classname.

I've cooked up a quick fix to this in my own local fork, but before I create a pull request that might mess up some of the work that is going on in @oskrkal 's branch over here in https://github.com/oskrkal/soapfish/tree/references , it seemed like I should check that I have actually fixed it correctly in the eyes of some people more familiar with what the code should be doing, not just written something that generates valid python syntax that won't do what it needs to do later.

My implemented fix generates the following code by checking for the existence of the complex type object name and using some extra logic to handle the case when the complex type object name is None

class FooCreateRequestType(FooCreateRequestBaseType):
    XSI_TYPE = xsd_types.XSDQName('https://www.example.com/ws', 'FooCreateRequestType')
    INHERITANCE = xsd.Inheritance.EXTENSION
    version = xsd.Attribute(xsd.Decimal, use=xsd.Use.OPTIONAL)


class FooCreateRequest(FooCreateRequestType):
    XSI_TYPE = xsd_types.XSDQName('https://www.example.com/ws', 'fooCreateRequest')
    INHERITANCE = xsd.Inheritance.EXTENSION

Does this look correct? I'm a little unsure about the capitalisation of the classname FooCreateRequest since it seems like it could be meant to match up with one of the names generated in the operations section, but it could just as easily not be meant to since that part also says "Put your own implementation here."

def FooCreate(request, fooCreateRequest):
    # TODO: Put your implementation here.
    return fooCreateResponse

With fooCreateResponse being undefined, but a FooCreateResponse has been generated.

@ngnpope
Copy link
Member

ngnpope commented May 2, 2017

@techdragon: Could you attach a full *.xsd and *.wsdl? Also if you could submit a pull request, then I can see what changes you are trying to make and better understand the problem. Thanks.

@oskrkal: Just been made aware of your fork. Were you planning to submit a pull request for your fixes? Or are you happy for me to pull in some of the changes?

@iurisilvio
Copy link
Contributor

@techdragon
Copy link
Contributor Author

@pope1ni I'm not able to submit the actual WSDL and XSD files, corporate confidentiality reasons. I'll try and make a minimal test case XSD & WSDL at work tomorrow, worst case I can post the full XSD & WSDL after I sanitise them. I'll also make a pull request so you can see the changes.

@pope1ni If I'm going to add some test coverage for the pull request... would it be better to modify one of the existing tests/assets/generation/*.wsdl files to cover the 'test case' in the wsdl, or create a new test with its own separate wsdl file?

@ngnpope
Copy link
Member

ngnpope commented May 3, 2017

Thanks. A minimal test case would be useful. I'd add separate test resources for now. We can always combine them later if it makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants