I think this is good to check in. We can resolve details with patches.
1. I'd like to have a name attribute for the PMF. Seems like this is a top level concept when finding PMF by name. This would be equivalent to the PUName that we've defined already. I'm not sure what we should do with the existing PUName. We could allow the user to specify the name as an attribute and check it where we check for PUName in attribute or property and allow only one of them.
2. The same comment applies to the provider class. We might consider promoting the javax.jdo.PersistenceManagerFactoryClass to "provider". Similarly, let's look at all the elements of persistence-unit in persistence.xml.
3. + catch (ParserConfigurationException e) {
+ throw new JDOFatalUserException(
+ msg.msg("EXC_ParserConfigException"),
+ e);
+ }
Are you sure this is a user error? Could be a
JDOFatalInternalException that should be reported to
([EMAIL PROTECTED]).
4. + catch (SAXException e) {
+ throw new JDOFatalUserException(
+ msg.msg("EXC_ParsingException", url.toExternalForm()),
+ e);
The SAX parser exception message should include the line number and
column number of the error. This should be available from the
exception itself but for some reason is not in the message text.
5. + catch (IOException ioe) { /* gulp */ }
I don't remember talking about this. Shouldn't we wrap this in a
JDOSomethingException?
6. Not that there's anything intrinsically wrong with your implementation, but I'd like to have a way for the jdo implementation to provide a different persistence.xml handler. Like add a method to JDOImplHelper to registerJDOConfigHandler, and refactor the existing handler to statically register itself. To do this, the jdoconfig handler needs to have a single instance with the methods you've created to do the parsing.
Craig Russell Architect, Sun Java Enterprise System http://java.sun.com/products/jdo 408 276-5638 mailto:[EMAIL PROTECTED] P.S. A good JDO? O, Gasp!
smime.p7s
Description: S/MIME cryptographic signature
