Tuscany
  1. Tuscany
  2. TUSCANY-3943

JAXWS Elements are not processed by the WSDLModelResolver - enableWrapperStyle elements are ignored

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: Java-SCA-2.x
    • Component/s: SCA Java Runtime
    • Labels:
      None
    • Environment:
      All systems

      Description

      Tuscany doesn't process any jaxws tags that are included in WSDL files. This causes any enableWrapperStyle elements to be ignored, which causes a logic failure in the wrapping logic when Tuscany expects wrapped objects, and Java classes generated by external tools such as wsimport are not wrapped.

      1. binding-ws-wsdlgen.patch
        4 kB
        Eric Larsen
      2. interface-wsdl.patch
        8 kB
        Eric Larsen
      3. itest.patch
        42 kB
        Eric Larsen

        Activity

        Hide
        Scott Kurz added a comment -

        After thinking some more I think Eric you've got the right idea as far as the wsdlgen, which is to simply map:

        @SOAPBinding(parameterStyle = SOAPBinding.ParameterStyle.BARE)

        to:
        <jaxws:enableWrapperStyle>false</jaxws:enableWrapperStyle>

        thereby enabling the new WSDL introspector logic to produce a match when interface compatibility is tested.

        Let me just explain the case that falls through the cracks.

        1. Take a WSDL with <jaxws:enableWrapperStyle>false</jaxws:enableWrapperStyle> and generate Java.
        2. Later, take the generated Java with @SOAPBinding(parameterStyle = SOAPBinding.ParameterStyle.BARE) and (re)generate a WSDL using a tool that does not gen the WSDL with the enableWrapperStyle set to 'false'.
        3. Package the new WSDL and Java in a Tuscany contribution and get an interface mismatch error.

        I tried step 2 with wsgen and it did indeed fail to generate the WSDL with enableWrapperStyle set to 'false'. (I wonder what CXF would do?)

        I think this is a restriction that we can live with. You'd either have to package the original WSDL or simply package no WSDL at all and let Tuscany generate one for you.

        So the only thing left would be to fix the issue with scope/granularity of the reading of this customization in the WSDL (which I mentioned earlier) as well as the DOM namespace tweak I mentioned (and maybe fleshing out the 2nd test in this new itest?).

        Eric, are you still interested in completing this?

        Show
        Scott Kurz added a comment - After thinking some more I think Eric you've got the right idea as far as the wsdlgen, which is to simply map: @SOAPBinding(parameterStyle = SOAPBinding.ParameterStyle.BARE) to: <jaxws:enableWrapperStyle>false</jaxws:enableWrapperStyle> thereby enabling the new WSDL introspector logic to produce a match when interface compatibility is tested. Let me just explain the case that falls through the cracks. 1. Take a WSDL with <jaxws:enableWrapperStyle>false</jaxws:enableWrapperStyle> and generate Java. 2. Later, take the generated Java with @SOAPBinding(parameterStyle = SOAPBinding.ParameterStyle.BARE) and (re)generate a WSDL using a tool that does not gen the WSDL with the enableWrapperStyle set to 'false'. 3. Package the new WSDL and Java in a Tuscany contribution and get an interface mismatch error. I tried step 2 with wsgen and it did indeed fail to generate the WSDL with enableWrapperStyle set to 'false'. (I wonder what CXF would do?) I think this is a restriction that we can live with. You'd either have to package the original WSDL or simply package no WSDL at all and let Tuscany generate one for you. So the only thing left would be to fix the issue with scope/granularity of the reading of this customization in the WSDL (which I mentioned earlier) as well as the DOM namespace tweak I mentioned (and maybe fleshing out the 2nd test in this new itest?). Eric, are you still interested in completing this?
        Hide
        Scott Kurz added a comment -

        The tests work for me, when I:
        1) run with the mods to binding-ws-wsdlgen, interface-wsdl outlined above (i.e. Eric's patch plus the change I identified in my comment)
        AND
        2) Change the new testGetGreetings()
        from:
        assertEquals("Winnder!", response.getPerson().getGreeting());
        to:
        assertEquals("Winner!", response.getPerson().getGreeting());

        That's what you meant Eric, right?

        I'm giving some more thought to the round-trip issue..

        Show
        Scott Kurz added a comment - The tests work for me, when I: 1) run with the mods to binding-ws-wsdlgen, interface-wsdl outlined above (i.e. Eric's patch plus the change I identified in my comment) AND 2) Change the new testGetGreetings() from: assertEquals("Winnder!", response.getPerson().getGreeting()); to: assertEquals("Winner!", response.getPerson().getGreeting()); That's what you meant Eric, right? I'm giving some more thought to the round-trip issue..
        Hide
        Simon Laws added a comment -

        I've committed the patch Eric but it fails. Judging by comments here I'm assuming that that is expected so I've marked the two tests as @Ignore.

        To get the test to run I had to replace the contribution location string

        target/itest-contribution-doc-lit-disablewrapped.jar

        with

        ../disablewrapped-app/target/disablewrapped-app.jar

        Is my change correct?

        Also most of the XML artifacts in the patch were missing license headers. I've added them.

        Show
        Simon Laws added a comment - I've committed the patch Eric but it fails. Judging by comments here I'm assuming that that is expected so I've marked the two tests as @Ignore. To get the test to run I had to replace the contribution location string target/itest-contribution-doc-lit-disablewrapped.jar with ../disablewrapped-app/target/disablewrapped-app.jar Is my change correct? Also most of the XML artifacts in the patch were missing license headers. I've added them.
        Hide
        Eric Larsen added a comment -

        Patch made against the itest directory, creates a new component, with two sub-components, an test application and the test case.

        Show
        Eric Larsen added a comment - Patch made against the itest directory, creates a new component, with two sub-components, an test application and the test case.
        Hide
        Scott Kurz added a comment -

        Gave this some more thought and realized two things:

        1) The scope/granularity of <jaxws:enableWrapperStyle> is a bit more complicated, according to the JAX-WS spec.

        Sec. 8.6:
        ... when determining the value of the jaxws:enableWrapperStyle customization parameter for a
        portType operation, binding declarations MUST be processed in the following order, according to the element
        they pertain to: (1) the portType operation in question, (2) its parent portType, (3) the definitions element.

        I guess to implement it right we'd need to do that, even though I'd guess it would be awhile before we'd ever come across a need for this.

        2) I can see why you raised the subject of wsdlgen. There are some round-tripping issues here. If I start with a WSDL op that otherwise qualifies for wrapper-style mapping but has <jaxws:enableWrapperStyle> and generate Java I'll end up with a "bare" mapping and the Java will have:

        @SOAPBinding(parameterStyle = SOAPBinding.ParameterStyle.BARE)

        However, I don't see that the JAX-WS spec says anything in the other direction, about mapping this (generated) Java to WSDL. So I might end up with an equivalent WSDL, but without the <jaxws:enableWrapperStyle> customization. The next person to generate Java might just then (assuming the toolset doesn't "remember somehow) get a wrapped-style Java interface.

        Now... I still think it would help greatly to have a test to talk to before going further... but I thought it might be helpful to write down that thought after having it.

        It might be that, after considering the second point, we need to take a completely different approach than simply reading in the <jaxws:enableWrapperStyle> customization like in Eric's patch, a modified version of which I used to pass the interface-wsdl test I added.

        Scott

        Show
        Scott Kurz added a comment - Gave this some more thought and realized two things: 1) The scope/granularity of <jaxws:enableWrapperStyle> is a bit more complicated, according to the JAX-WS spec. Sec. 8.6: ... when determining the value of the jaxws:enableWrapperStyle customization parameter for a portType operation, binding declarations MUST be processed in the following order, according to the element they pertain to: (1) the portType operation in question, (2) its parent portType, (3) the definitions element. I guess to implement it right we'd need to do that, even though I'd guess it would be awhile before we'd ever come across a need for this. 2) I can see why you raised the subject of wsdlgen. There are some round-tripping issues here. If I start with a WSDL op that otherwise qualifies for wrapper-style mapping but has <jaxws:enableWrapperStyle> and generate Java I'll end up with a "bare" mapping and the Java will have: @SOAPBinding(parameterStyle = SOAPBinding.ParameterStyle.BARE) However, I don't see that the JAX-WS spec says anything in the other direction, about mapping this (generated) Java to WSDL. So I might end up with an equivalent WSDL, but without the <jaxws:enableWrapperStyle> customization. The next person to generate Java might just then (assuming the toolset doesn't "remember somehow) get a wrapped-style Java interface. Now... I still think it would help greatly to have a test to talk to before going further... but I thought it might be helpful to write down that thought after having it. It might be that, after considering the second point, we need to take a completely different approach than simply reading in the <jaxws:enableWrapperStyle> customization like in Eric's patch, a modified version of which I used to pass the interface-wsdl test I added. Scott
        Hide
        Scott Kurz added a comment -

        Eric,

        Thanks for raising the issue and the patch. In r1166896, I added a test to interface-wsdl showing this issue, @Ignore(d) for now.

        A couple comments on your patch. It's a good start, I did have to tweak it a bit since it assumes I'm going to have a <jaxws:package> customization in my WSDL
        along with the <jaxws:enableWrapperStyle> customization (which my test in particular did not have).

        But I basically made my test pass by tweaking the one line to:

        Node wrapperElem = element.getElementsByTagNameNS(jaxwsNS,wrapperTagName).item(0);

        I'm not sure at this point what to make of your comment about needing to tweak wsdlgen. Can you outline at least (if not post) the test you used to reach this conclusion?

        Also, while we might want to read and honor the <jaxws:package> customization (as well as others) at some point, my preference would be to leave this out altogether unless we are really going to make use of this in a real scenario. If we don't have a test which makes uses of this at least, let's cut that out so any maintainers of this code have less to worry about.

        If you can resubmit a trimmed-down patch which passes the new test, I can commit it.

        I'll let you decide whether you want to use this or another JIRA to address the wsdlgen issue as well.

        Thanks,
        Scott

        Show
        Scott Kurz added a comment - Eric, Thanks for raising the issue and the patch. In r1166896, I added a test to interface-wsdl showing this issue, @Ignore(d) for now. A couple comments on your patch. It's a good start, I did have to tweak it a bit since it assumes I'm going to have a <jaxws:package> customization in my WSDL along with the <jaxws:enableWrapperStyle> customization (which my test in particular did not have). But I basically made my test pass by tweaking the one line to: Node wrapperElem = element.getElementsByTagNameNS(jaxwsNS,wrapperTagName).item(0); I'm not sure at this point what to make of your comment about needing to tweak wsdlgen. Can you outline at least (if not post) the test you used to reach this conclusion? Also, while we might want to read and honor the <jaxws:package> customization (as well as others) at some point, my preference would be to leave this out altogether unless we are really going to make use of this in a real scenario. If we don't have a test which makes uses of this at least, let's cut that out so any maintainers of this code have less to worry about. If you can resubmit a trimmed-down patch which passes the new test, I can commit it. I'll let you decide whether you want to use this or another JIRA to address the wsdlgen issue as well. Thanks, Scott
        Hide
        Eric Larsen added a comment -

        I have implemented a simple tag model which handles the jaxws:enableWrappedStyle tag, and then uses the model extension in the wrapper logic.

        In addition to the WSDL processing side, a modification was needed for the Java -> WSDL logic due to contract matching. All non-wrapped operations now output a jaxws:elementWrappedStyle false tag, since you cannot distinguish between non-wrapped and disable-wrapped cases at that point, and adding the tag does not impact the non-wrapped logic.

        Show
        Eric Larsen added a comment - I have implemented a simple tag model which handles the jaxws:enableWrappedStyle tag, and then uses the model extension in the wrapper logic. In addition to the WSDL processing side, a modification was needed for the Java -> WSDL logic due to contract matching. All non-wrapped operations now output a jaxws:elementWrappedStyle false tag, since you cannot distinguish between non-wrapped and disable-wrapped cases at that point, and adding the tag does not impact the non-wrapped logic.

          People

          • Assignee:
            Unassigned
            Reporter:
            Eric Larsen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development