Hi Oshani ... On 7/12/06, Oshani Seneviratne <[EMAIL PROTECTED]> wrote:
if the readWSDL method is changed to accept URLs, this file path problem can be completely eliminated from the OM parser. Also from Axis2 point of view, if we can directly accept InputStreams it would be even more great :).
+1. If that helps Axis2 lets do it.
From a previous discussion I've learnt it is necessary for the WSDL that's read into Woden to have a base URI in order to interpret the relative URLs. But at the moment, Axis2 seems to be setting the base URI to the parent service archive. So, IMO it's just a matter of the user setting the base URI in order to resolve relative URIs.
We can use the jar URL [1] to refer to files within an aar file. This allows us to set the base URI to be the top level WSDL file in the aar (described by a jar URL). Then the usual rules for creating relative URLs for imports apply. So the base URI should be the jar URL of the top level WSDL and not the aar filename. [1] http://java.sun.com/j2se/1.4.2/docs/api/java/net/JarURLConnection.html
> Looking a bit further: line 91 of OMWSDLReader.readWSDL() declares the > url variable and is set at line 93 but never read. I think you copied > this method from DOMWSDLReader() and added the above code for some > reason - perhaps you had a problem with local files? There was a > recent problem with StringUtils.getURL() in WSDL4J which I don't think > is fixed in Woden. > I will remove the code for checking the ":" in the String URI, and modify the readWSDL signature to take in a URL. I should also probably check validity of this URL and not try to correct the URL to form a valid URL as Lawrence has pointed out. > Eclipse tells me there are some other places where local variables are > never read. There's also one in DOMWSDLReader so I'll open a JIRA for > fixing those. > Yes, there are unused variables. I'll fix those.
Ah sorry, I just went ahead and corrected DOMWSDLReader. There are other similar warnings throughout the Woden.
> Just something that might slightly speed things up is the number of > times the EndpointElementTest.wsdl file is read by multiple calls to > the OMEndpointElementTest.setUp() method. I don't think every test > case needs to re-read and test the WSDL - only the > > testSetAndGetBindingNameFromOM > testSetAndGetNameFromOM > > methods change the parsed WSDL in object form. Yes again! I think this should be changed in the EndpointElementTest as well.
Sure - please open a JIRA - and add a patch (until you get your Woden Karma :-)
> > And a small thing - we have been specifying implicit imports rather > than use * in import statements. This makes it easier to see which > package a class comes from if you're using a dumb editor like vi or > notepad. > Hmm.., I took some effort to make the code as clean as possible. But I see that 2 import *s, has slipped from my radar in the AllWodenTestsOM. So, my apologies.. Anyway, as a rule I have been following the coding conventions John proposed some time back [1]
Great thanks! We're not so good ourselves on this and the whole code probably needs reformatting with something like jalopy [2]. (there's a maven plugin for jalopy which is why I mention it). [2] http://jalopy.sourceforge.net/
.IMHO, it would be a good idea to document these in the project site. I'm willing to help, if required.
Great! We probably need to document how to update the project site too :-D
Please share if you have any other comments/concerns regarding the OM parser.
There seems to be a lot of common code between DOMWSDLReader and OMWSDLReader and probably other classes too. It would be great to refactor the common pieces out either using inheritance or delegation patterns. Also, from the test point of view we have AllWodenTestsDOM and AllWodenTestsOM but in fact the DOM tests include tests not specific to DOM. From the name of the classes it would easy to assume that comparing the performance of the two test cases is a valid thing to do. Perhaps we could split them out into DOM, OM and no-parse tests.
Thanks and Regards, Oshani.
Thanks again, Jeremy --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
