Hi Tom,
This is really interesting, and explains why Petr reported that it
worked before.
While most of the cases used correctly file.toURI().toURL(),
this particular place (processDirectory()) called file.toURL().
The changes in v1.6 replaced all file/url access, fixing one case
but causing regression in the others.
Now I have follow up questions:
1) null/empty-string path comparison.
Do you think we should still keep them? This assertion was added
as part of the changes 1.5->1.6:
http://fisheye5.cenqua.com/viewrep/glassfish/entity-persistence/src/java/oracle/toplink/essentials/ejb/cmp3/persistence/PersistenceUnitProcessor.java?r1=1.5&r2=1.6
2) there are several unnecessary assertions and conversions
when one method calls another in the same class, e.g:
processPersistenceArchive -> processDirectory
getClassNamesFromURL-> getClassNamesFromJar
getPersistentClassNamesFromJar-> getClassNamesFromJar
Was this done on purpose? Or should I clean this up as part
of my changes?
3) what is the reason for keeping such internal methods public
or protected?
thanks,
-marina
Tom Ware wrote On 02/22/06 06:19,:
> Hi Marina,
>
> When we fixed a similar issue (184) a while ago, we were having
> problems with the URI class handling directories with spaces. If you
> take a look at the difference between version 1.5 and version 1.6 in
> CVS, you will be able to see the changes we made.
>
> If you can make the URI version work for both issues, I think the
> suggested fix should be fine.
>
> If there are appropriate PersistenceUnitLoadingExceptions, feel free
> to reuse them. Otherwise, adding a new error code is a good idea.
>
> -Tom
>
> Marina Vatkina wrote:
>
>
>>Hi Tom,
>>
>>To fix issue 228 in all of the PersistenceUnitProcessor.java, I need
>>a) to change in PersistenceUnitProcessor.java the code from:
>>
>> String filePath = url.getFile();
>> ...
>> File file = new File(filePath);
>>
>>to
>>
>> File file = new File(url.toURI());
>>
>>b) Wrap URISyntaxException that can be thrown from URL.toURI().
>>
>>See the latest comments to the issue
>>https://glassfish.dev.java.net/issues/show_bug.cgi?id=228
>>and the attached file as the 1st cut, where duplicate code is moved to a
>>separate private static method.
>>
>>Should I reuse any of the existing PersistenceUnitLoadingException exceptions or
>>add a new one?
>>
>>Do you see any problems with the proposed changes?
>>
>>thanks,
>>-marina
>>
>>
>>
>
>