Patch: NPE single values and @Embeddable

[Nestor Urquiza]

There are two unrelated issues for which I have provided a patch myself. I am unsure if you want to take another route to correct the problems:

1. NPE when updating entities with non complex null fields and using Hibernate Persistance Provider.
2. @Embeddable annotation is not supported and an Exception is thrown for them as they do not have ID.

My quick and dirty  fix for the first was just to skip null single values..

My workaround about ignoring @Embeddable entities is perhaps correct as per JDK5 javadocs ("Only Basic, Column, Lob, Temporal, and Enumerated mapping annotations may portably be used to map the persistent fields or properties of classes annotated as Embeddable.") but I see no restrictions on JDK6 (http://download.oracle.com/javaee/6/api/javax/persistence/Embeddable.html) which means that probably supporting @Embeddable in terms of security should be a task to take into consideration.

Let me know if I should submit a bug (or more) fix or feature request.

Here is a suggested patch:

Index: pom.xml
===================================================================
--- pom.xml (revision 852)
+++ pom.xml (working copy)
@@ -4,7 +4,7 @@
     <groupId>net.sf.jpasecurity</groupId>
     <artifactId>jpasecurity-modules</artifactId>
     <version>0.4.0-SNAPSHOT</version>
-    <relativePath>../pom.xml</relativePath>
+    <relativePath>../jpasecurity-modules/pom.xml</relativePath>
   </parent>
   <modelVersion>4.0.0</modelVersion>
   <artifactId>jpasecurity-core</artifactId>
Index: src/main/java/net/sf/jpasecurity/entity/EntityPersister.java
===================================================================
--- src/main/java/net/sf/jpasecurity/entity/EntityPersister.java    (revision 842)
+++ src/main/java/net/sf/jpasecurity/entity/EntityPersister.java    (working copy)
@@ -395,30 +395,32 @@
                 && (propertyMapping.getCascadeTypes().contains(cascadeType)
                     || propertyMapping.getCascadeTypes().contains(CascadeType.ALL))) {
                 Object secureValue = propertyMapping.getPropertyValue(secureEntity);
-                if (propertyMapping.isSingleValued()) {
-                    initialize(secureValue,
-                               getUnsecureObject(secureValue),
-                               isNew(secureValue),
-                               cascadeType,
-                               alreadyInitializedEntities);
-                } else {
-                    if (secureValue instanceof AbstractSecureCollection) {
-                        AbstractSecureCollection<?, Collection<?>> secureCollection
-                            = (AbstractSecureCollection<?, Collection<?>>)secureValue;
-                        secureCollection.initialize(!isNew);
-                    } else if (secureValue instanceof SecureList) {
-                        SecureList<?> secureList = (SecureList<?>)secureValue;
-                        secureList.initialize(!isNew);
-                    } else if (secureValue instanceof DefaultSecureMap) {
-                        DefaultSecureMap<?, ?> secureMap = (DefaultSecureMap<?, ?>)secureValue;
-                        secureMap.initialize(!isNew);
-                    }
-                    for (Object secureEntry: ((Collection<Object>)secureValue)) {
-                        initialize(secureEntry,
-                                   getUnsecureObject(secureEntry),
-                                   isNew(secureEntity),
-                                   cascadeType,
-                                   alreadyInitializedEntities);
+                if (secureValue != null) {
+                    if (propertyMapping.isSingleValued()) {
+                        initialize(secureValue,
+                                getUnsecureObject(secureValue),
+                                isNew(secureValue),
+                                cascadeType,
+                                alreadyInitializedEntities);
+                    } else {
+                        if (secureValue instanceof AbstractSecureCollection) {
+                            AbstractSecureCollection<?, Collection<?>> secureCollection
+                                = (AbstractSecureCollection<?, Collection<?>>)secureValue;
+                            secureCollection.initialize(!isNew);
+                        } else if (secureValue instanceof SecureList) {
+                            SecureList<?> secureList = (SecureList<?>)secureValue;
+                            secureList.initialize(!isNew);
+                        } else if (secureValue instanceof DefaultSecureMap) {
+                            DefaultSecureMap<?, ?> secureMap = (DefaultSecureMap<?, ?>)secureValue;
+                            secureMap.initialize(!isNew);
+                        }
+                        for (Object secureEntry: ((Collection<Object>)secureValue)) {
+                            initialize(secureEntry,
+                                       getUnsecureObject(secureEntry),
+                                       isNew(secureEntity),
+                                       cascadeType,
+                                       alreadyInitializedEntities);
+                        }
                     }
                 }
             }
Index: src/main/java/net/sf/jpasecurity/mapping/DefaultClassMappingInformation.java
===================================================================
--- src/main/java/net/sf/jpasecurity/mapping/DefaultClassMappingInformation.java    (revision 850)
+++ src/main/java/net/sf/jpasecurity/mapping/DefaultClassMappingInformation.java    (working copy)
@@ -24,6 +24,7 @@
 import java.util.Map;
 import java.util.Set;

+import javax.persistence.Embeddable;
 import net.sf.jpasecurity.ExceptionFactory;
 import net.sf.jpasecurity.util.ReflectionUtils;

@@ -211,6 +212,10 @@

     public Object getId(Object entity) {
         List<PropertyMappingInformation> idProperties = getIdPropertyMappings();
+        boolean isEmbeddable = entity.getClass().getAnnotation(Embeddable.class) != null;
+        if (isEmbeddable) {
+            return null;
+        }
         if (idProperties.size() == 0) {
             String error = "Id property required for class " + getEntityType().getName();
             throw exceptionFactory.createMappingException(error);

[Arne Limburg]

Hi Nestor,

I have added the first patch to trunk and to the 0.3-branch. However the second needs some more work: It seems reasonable to return null on getId(…) when the type is no entity, however the detection whether we have an entity or an embeddable has to go to another place: Embeddables could be defined either via annotations or via XML. The detection of embeddables has to go somewhere into the mapping-parser. I'll take a closer look into it.

Btw. Most of the code should support embeddables. Can you provide a stack-trace, where the error occures?

[Nestor Urquiza]

Here is the stacktrace for this one:

com.nestorurquiza.service.impl.ErrorEnabledRuntimeException: com.nestorurquiza.dao.DaoException: javax.persistence.PersistenceException: Id property required for class com.nestorurquiza.model.ContactDetail
    at com.nestorurquiza.service.impl.GenericEntityCrudServiceImpl.create(GenericEntityCrudServiceImpl.java:56)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:309)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:183)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:150)
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:110)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:202)
    at $Proxy147.create(Unknown Source)
    at com.nestorurquiza.service.impl.ServiceProviderContactServiceImplTest.setUpTestDataWithinTransaction(ServiceProviderContactServiceImplTest.java:76)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:27)
    at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:74)
    at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:82)
    at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:72)
    at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:240)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
    at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:180)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:49)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
Caused by: com.nestorurquiza.dao.DaoException: javax.persistence.PersistenceException: Id property required for class com.nestorurquiza.model.ContactDetail
    at com.nestorurquiza.dao.GenericEntityDaoImpl.processException(GenericEntityDaoImpl.java:124)
    at com.nestorurquiza.dao.GenericEntityDaoImpl.create(GenericEntityDaoImpl.java:102)
    at com.nestorurquiza.service.impl.GenericEntityCrudServiceImpl.create(GenericEntityCrudServiceImpl.java:50)
    ... 40 more
Caused by: javax.persistence.PersistenceException: Id property required for class com.nestorurquiza.model.ContactDetail
    at net.sf.jpasecurity.persistence.JpaExceptionFactory.createRuntimeException(JpaExceptionFactory.java:28)
    at net.sf.jpasecurity.configuration.AbstractExceptionFactory.createRuntimeException(AbstractExceptionFactory.java:59)
    at net.sf.jpasecurity.configuration.AbstractExceptionFactory.createMappingException(AbstractExceptionFactory.java:34)
    at net.sf.jpasecurity.mapping.DefaultClassMappingInformation.getId(DefaultClassMappingInformation.java:221)
    at net.sf.jpasecurity.entity.EntityPersister.isNew(EntityPersister.java:263)
    at net.sf.jpasecurity.entity.EntityPersister.createUnsecureObject(EntityPersister.java:247)
    at net.sf.jpasecurity.entity.AbstractSecureObjectManager.getUnsecureObject(AbstractSecureObjectManager.java:140)
    at net.sf.jpasecurity.entity.EntityPersister.getUnsecureObject(EntityPersister.java:240)
    at net.sf.jpasecurity.entity.AbstractSecureObjectManager.unsecureCopy(AbstractSecureObjectManager.java:191)
    at net.sf.jpasecurity.entity.EntityPersister.createUnsecureObject(EntityPersister.java:253)
    at net.sf.jpasecurity.entity.AbstractSecureObjectManager.getUnsecureObject(AbstractSecureObjectManager.java:140)
    at net.sf.jpasecurity.entity.EntityPersister.getUnsecureObject(EntityPersister.java:240)
    at net.sf.jpasecurity.entity.EntityPersister.persist(EntityPersister.java:63)
    at net.sf.jpasecurity.persistence.DefaultSecureEntityManager.persist(DefaultSecureEntityManager.java:120)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.springframework.orm.jpa.SharedEntityManagerCreator$SharedEntityManagerInvocationHandler.invoke(SharedEntityManagerCreator.java:240)
    at $Proxy84.persist(Unknown Source)
    at com.nestorurquiza.dao.GenericEntityDaoImpl.create(GenericEntityDaoImpl.java:98)
    ... 41 more

[Arne Limburg]

Thank you, I commited a fix for this that is based on your proposed fix

[Nestor Urquiza]

Thanks! My tests passed with this fix.