Re: JDOHelper patch
Hi Craig, this looks good! It makes JDOHelper.getPMF much easier. Regards Michael Hi, This patch fixes the exception thrown in case of a missing method. It also requires a JDO implementation to implement the static getPersistenceManagerFactory(Map props) method. And it adds two test cases to JDOHelperTest. Craig 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! -- Michael Bouschen[EMAIL PROTECTED] Engineering GmbH mailto:[EMAIL PROTECTED]http://www.tech.spree.de/ Tel.:++49/30/235 520-33 Buelowstr. 66 Fax.:++49/30/2175 2012 D-10783 Berlin
JDOHelper patch
Hi,This patch fixes the exception thrown in case of a missing method. It also requires a JDO implementation to implement the static getPersistenceManagerFactory(Map props) method. And it adds two test cases to JDOHelperTest.Craig jdohelper.patch Description: Binary data 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
Re: JDOHelper patch
Hi Erik,Thank you. You were so right.I've added your suggested catch(JDOException) and improved the JDOHelperTest to check for the proper nested exception. The original test only checked the outer exception which is correct, but the real problem is in the nested exception.Thanks for being persistent on this issue.Craig jdohelpertest.patch Description: Binary data On Jul 17, 2005, at 2:01 AM, [EMAIL PROTECTED] wrote:Quoting Craig Russell <[EMAIL PROTECTED]>: This exception is not thrown by Method.invoke. It will be caught bythe catch (InvocationTargetException). Reflective invocation wrapsall exceptions in InvocationTargetException which we then unwrap andrethrow JDOExceptions.I've added a new test case in JDOHelperTest to test that aJDOException thrown from getPersistenceManagerFactory will be thrownto the user. When the mapGetMethodException is thrown, it is inside the try catch blockcatching java.lang.Exception. The JDOException catch there fixes it. 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
Re: JDOHelper patch
Quoting Craig Russell <[EMAIL PROTECTED]>: > This exception is not thrown by Method.invoke. It will be caught by > the catch (InvocationTargetException). Reflective invocation wraps > all exceptions in InvocationTargetException which we then unwrap and > rethrow JDOExceptions. > > I've added a new test case in JDOHelperTest to test that a > JDOException thrown from getPersistenceManagerFactory will be thrown > to the user. > When the mapGetMethodException is thrown, it is inside the try catch block catching java.lang.Exception. The JDOException catch there fixes it.
Re: JDOHelper patch
Hi Erik,Thanks for the comments.I've updated the JDOHelperTest based on your comments:
jdohelpertest.patch
Description: Binary data
On Jul 16, 2005, at 9:43 AM, [EMAIL PROTECTED] wrote:Hi,There are two issues in JDOHelper.getPersistenceManagerFactory1 - the JDOHelper.getPMF raises will raise an exception if there is nogetPMF(Properties) or getPMF(Map) in the implementation. The exception for theproperties method is not raised, but in its place the exception for the mapmethod.I did consider this when I wrote the code, and decided that the Map exception was correct.The requirement in JDO 2.0 has changed from getPMF(Properties) to getPMF(Map), so a compliant implementation will recompile with Map as the formal parameter. A11.1-32 [An implementation must provide a method to construct a PersistenceManagerFactory by a Map instance. This static method is called by the JDOHelper method getPersistenceManagerFactory (Map props).static PersistenceManagerFactory getPersistenceManagerFactory (Map props);]A11.1-33 [The properties consist of: “javax.jdo.PersistenceManagerFactoryClass”, whose value is the name of the implementation class; any JDO vendor-specific properties; and the following standard property names, which correspond to the properties as documented in this chapter:"javax.jdo.option.Optimistic""javax.jdo.option.RetainValues"...Since Properties is a Map, applications need not change. By the way, the parameter being a Map instead of Properties was an excellent suggestion by you, IIRC.The exception is only raised if no getPMF method is found, and since the requirement is for getPMF(Map) it seems like this is the right exception to report.I included the JDOHelper code to allow getPMF(Properties) for convenience of implementations until they have compiled with the new API20 library. I don't expect that the Properties code will need to be there for the final release.2 - a try catch block is hidding JDOExceptions.Here is the patch:Index: C:/trunck/api20/test/java/javax/jdo/JDOHelperTest.java===--- C:/trunck/api20/test/java/javax/jdo/JDOHelperTest.java (revision 219338)+++ C:/trunck/api20/test/java/javax/jdo/JDOHelperTest.java (working copy)@@ -18,14 +18,14 @@ import java.io.File; import java.io.InputStream;-+import java.util.HashMap; import java.util.Map; import java.util.Properties; import javax.jdo.pc.PCPoint;+import javax.jdo.spi.I18NHelper; import javax.jdo.util.AbstractTest; import javax.jdo.util.BatchTestRunner;- import javax.naming.Context; import javax.naming.InitialContext; import javax.naming.NamingException;@@ -38,6 +38,9 @@ * TBD: getPMF for valid PMF class */ public class JDOHelperTest extends AbstractTest {+ /** The Internationalization message helper.+ */+ private final static I18NHelper msg = I18NHelper.getInstance("javax.jdo.Bundle"); //NOI18NThere is no specific error message in the specification, so it's not clear that we need to test for error message text. /** */ public static void main(String args[]) {@@ -130,6 +133,7 @@ // TBD test JDOHelper.isDeleted(pc) for persistent instance }+ /** Test null String resource with no class loader. */ public void testGetPMFNullResource() {@@ -401,7 +405,21 @@ catch (JDOFatalInternalException ex) { if (verbose) println("Caught expected exception " + ex);+ assertEquals(msg.msg("EXC_GetPMFNoSuchMethod"),ex.getMessage()); }Adding a new test method with a Map parameter is a good idea. But it should be a separate test case not added to testGetPMFNullResource.+ Map map = new HashMap();+ map.put("javax.jdo.PersistenceManagerFactoryClass",+ "javax.jdo.JDOHelperTest$BadPMFNoGetPMFMethod");+ try {+ pmf = JDOHelper.getPersistenceManagerFactory(map);+ fail("Bad PersistenceManagerFactoryClass should result inJDOFatalUserException ");+ }+ catch (JDOFatalInternalException ex) {+ if (verbose)+ println("Caught expected exception " + ex);+ assertEquals(msg.msg("EXC_GetPMFNoSuchMethod"),ex.getMessage());+ }+ } /** Test bad PMF class non-static getPMF method.Index: C:/trunck/api20/src/java/javax/jdo/JDOHelper.java===--- C:/trunck/api20/src/java/javax/jdo/JDOHelper.java (revision 219338)+++ C:/trunck/api20/src/java/javax/jdo/JDOHelper.java (working copy)@@ -24,26 +24,21 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException;-import java.io.InputStream; import java.io.IOException;--import java.lang.reflect.Method;+import java.io.InputStream; import java.lang.reflect.InvocationTargetException;-+import java.lang.reflect.Method;+import java.security.AccessController;+import java.security.PrivilegedAction; import java.util.Map; import java.util.Properties;-import java.security.AccessContr
JDOHelper patch
Hi,
There are two issues in JDOHelper.getPersistenceManagerFactory
1 - the JDOHelper.getPMF raises will raise an exception if there is no
getPMF(Properties) or getPMF(Map) in the implementation. The exception for the
properties method is not raised, but in its place the exception for the map
method.
2 - a try catch block is hidding JDOExceptions.
Here is the patch:
Index: C:/trunck/api20/test/java/javax/jdo/JDOHelperTest.java
===
--- C:/trunck/api20/test/java/javax/jdo/JDOHelperTest.java (revision
219338)
+++ C:/trunck/api20/test/java/javax/jdo/JDOHelperTest.java (working copy)
@@ -18,14 +18,14 @@
import java.io.File;
import java.io.InputStream;
-
+import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
import javax.jdo.pc.PCPoint;
+import javax.jdo.spi.I18NHelper;
import javax.jdo.util.AbstractTest;
import javax.jdo.util.BatchTestRunner;
-
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;
@@ -38,6 +38,9 @@
* TBD: getPMF for valid PMF class
*/
public class JDOHelperTest extends AbstractTest {
+/** The Internationalization message helper.
+ */
+private final static I18NHelper msg = I18NHelper.getInstance
("javax.jdo.Bundle"); //NOI18N
/** */
public static void main(String args[]) {
@@ -130,6 +133,7 @@
// TBD test JDOHelper.isDeleted(pc) for persistent instance
}
+
/** Test null String resource with no class loader.
*/
public void testGetPMFNullResource() {
@@ -401,7 +405,21 @@
catch (JDOFatalInternalException ex) {
if (verbose)
println("Caught expected exception " + ex);
+assertEquals(msg.msg("EXC_GetPMFNoSuchMethod"),ex.getMessage());
}
+Map map = new HashMap();
+map.put("javax.jdo.PersistenceManagerFactoryClass",
+"javax.jdo.JDOHelperTest$BadPMFNoGetPMFMethod");
+try {
+pmf = JDOHelper.getPersistenceManagerFactory(map);
+fail("Bad PersistenceManagerFactoryClass should result in
JDOFatalUserException ");
+}
+catch (JDOFatalInternalException ex) {
+if (verbose)
+println("Caught expected exception " + ex);
+assertEquals(msg.msg("EXC_GetPMFNoSuchMethod"),ex.getMessage());
+}
+
}
/** Test bad PMF class non-static getPMF method.
Index: C:/trunck/api20/src/java/javax/jdo/JDOHelper.java
===
--- C:/trunck/api20/src/java/javax/jdo/JDOHelper.java (revision 219338)
+++ C:/trunck/api20/src/java/javax/jdo/JDOHelper.java (working copy)
@@ -24,26 +24,21 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
-import java.io.InputStream;
import java.io.IOException;
-
-import java.lang.reflect.Method;
+import java.io.InputStream;
import java.lang.reflect.InvocationTargetException;
-
+import java.lang.reflect.Method;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
import java.util.Map;
import java.util.Properties;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
-
import javax.jdo.spi.I18NHelper;
import javax.jdo.spi.PersistenceCapable;
-import javax.jdo.spi.StateManager; // for javadoc
-
+import javax.jdo.spi.StateManager;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;
-
import javax.rmi.PortableRemoteObject;
@@ -338,7 +333,7 @@
} else if (propsMethod != null) {
pmfMethod = propsMethod;
} else {
-throw mapGetMethodException;
+throw propsGetMethodException;
}
}
return (PersistenceManagerFactory) pmfMethod.invoke (null, new
Object[] {props});
@@ -357,6 +352,8 @@
throw new JDOFatalInternalException
(msg.msg("EXC_GetPMFNullPointerException", pmfClassName), e); //NOI18N
} catch (ClassCastException e) {
throw new JDOFatalInternalException
(msg.msg("EXC_GetPMFClassCastException", pmfClassName), e); //NOI18N
+} catch (JDOException jdoex) {
+throw jdoex;
} catch (Exception e) {
throw new JDOFatalInternalException
(msg.msg("EXC_GetPMFUnexpectedException"), e); //NOI18N
}
