Was there anywhere else you feel changes need to be made? On the whole I think this type-safe approach is better than the so-called 'user-friendly' approach of just allowing a String and we do the rest.
Cheers,
Jeremy
On 7/11/06, Lawrence Mandel <[EMAIL PROTECTED]> wrote:
I think it's a bad idea for Woden to start trying to correct input strings to form valid URLs. We should rather just require that a valid URL is passed in. (This means Woden will likely choke on c:\somefile.wsdl as it requires the file:///.)
The justification for my suggestion comes from the Xerces team, who have a lot of experience with parsers. I spoke with the Xerces team about this a while back and they told their story of bug report after bug report being filed from users that expected their entry to be resolved correctly. Let's learn from the Xerces experience and leave this logic up to the Woden consumer.
Lawrence
woden-dev@ws.apache.org Re: Tests for the StAX/OM parser
Thanks very much for your recent patch. I have applied it in revision
420800 with some small changes:
I had a problem at OMWSDLReader line 76. This assumes if there is a :
in the input string then it is a URL. However, on windows c:\foo.wsdl
is a file and not a URL. So this logic:
int index = wsdlURI.indexOf(':');
String wsdlURL = (index != -1)
? wsdlURI
: ("file://"+wsdlURI);
yeilds c:\foo.wsdl instead of file://c:\foo.wsdl. The problem showed
up because W3CFileRepository.getFilePath() was returning a file path
instead of a URL - so I fixed that. However the problem remains in
readWSDL() should someone pass in the local string path of a WSDL on a
Windows machine - would be good to fix.
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.
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.
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.
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.
Thanks again.
Jeremy
On 7/10/06, Oshani Seneviratne <[EMAIL PROTECTED]> wrote:
> Hi Jeremy,
>
> I'm terribly sorry for the trouble that you had to undergo in applying
> my 2 previous patches! And also, thanks a lot for pointing out the
> correct way of submitting patches.
>
> I have attached a new patch at [1], which you could apply at the Woden
> root. Please disregard those 2 earlier patches, i.e.
> 'TestsForOMParser.patch' and 'OMWSDLReader-additions.patch' that I
> have attached with WODEN-37.
>
> I documented all the changes that are there with the new patch at [2].
> Basically I have modified the classpath, project.properties and
> changed the code & the ant scripts so that you would be able to run
> tests for both DOM and StAX/OM parsers.
>
> So, after applying the patch you can run:
> 'ant runTestsDOM' to see tests for DOM,
> 'ant runTestsOM' to see tests for StAX/OM, and
> 'ant runTests' to see tests for both.
>
> [1] http://issues.apache.org/jira/secure/attachment/12336594/TestsAndAdditions.patch
> [2]
> http://issues.apache.org/jira/browse/WODEN-37#action_12420090
>
> Thanks and Regards,
> Oshani.
>
>
>
> On 7/8/06, Jeremy Hughes <[EMAIL PROTECTED]> wrote:
> > Hi Oshani, thanks for your patches. I started to apply them, but I
> > came up against a few problems. Firstly the TestsForOMParser.patch was
> > created when you were in the test/org/apache/woden directory. This I
> > could workaround - it is normal (although I have to admit we haven't
> > documented this on the Woden site) to create the patch while in the
> > project root dir. The OMWSDLReader-additions.patch had a similar
> > problem. Anyway, I worked around this.
> >
> > The main issue is that I can't run the tests without making changes to
> > the classpath and I ran out of time here figuring it out. It would be
> > great if I could take your patch and just like the in the
> > contributors' FAQ [1], run the 'patch' command from the command line
> > in the root dir of Woden. Then if I do 'ant runTests' all the new
> > tests should get run as well as the old ones - and of course for all
> > the tests to pass :-)
> >
> > [1] http://www.apache.org/dev/contributors.html#patches
> >
> > Many thanks,
> > Jeremy
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
