MountainClimb / datanucleus-appengine

Automatically exported from code.google.com/p/datanucleus-appengine
0 stars 0 forks source link

Can't store null value in LinkedList<Integer> #234

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Define an entity that contains a LinkedList<Integer> (or any other supported 
java.util.Collection)
2. Create a list and store a null value in there.
3. persist() the entity in a transaction and attempt to commit the transaction

What is the expected output? What do you see instead?
I expect to see a null being stored in the entity. Which is exactly what 
happens when I store a null in a LinkedList<Long>. However, 
datanucleus-appengine attempts to convert the Integer to a Long without a null 
check. The offending line is 
http://code.google.com/p/datanucleus-appengine/source/browse/trunk/src/org/datan
ucleus/store/appengine/TypeConversionUtils.java#111

A stack trace is attached:
java.lang.NullPointerException
    at org.datanucleus.store.appengine.TypeConversionUtils$7.apply(TypeConversionUtils.java:111)
    at org.datanucleus.store.appengine.TypeConversionUtils$7.apply(TypeConversionUtils.java:110)
    at org.datanucleus.store.appengine.Utils.transform(Utils.java:62)
    at org.datanucleus.store.appengine.TypeConversionUtils.convertPojoCollectionToDatastoreValue(TypeConversionUtils.java:643)
    at org.datanucleus.store.appengine.TypeConversionUtils.pojoValueToDatastoreValue(TypeConversionUtils.java:619)
    at org.datanucleus.store.appengine.DatastoreFieldManager.storeObjectField(DatastoreFieldManager.java:806)
    at org.datanucleus.state.AbstractStateManager.providedObjectField(AbstractStateManager.java:1037)
    at my.package.Campaign.jdoProvideField(Campaign.java)
    at my.package.Campaign.jdoProvideFields(Campaign.java)
    at org.datanucleus.state.JDOStateManagerImpl.provideFields(JDOStateManagerImpl.java:2715)
    at org.datanucleus.store.appengine.DatastorePersistenceHandler.insertPreProcess(DatastorePersistenceHandler.java:341)
    at org.datanucleus.store.appengine.DatastorePersistenceHandler.insertObjects(DatastorePersistenceHandler.java:251)
    at org.datanucleus.store.appengine.DatastorePersistenceHandler.insertObject(DatastorePersistenceHandler.java:240)
    at org.datanucleus.state.JDOStateManagerImpl.internalMakePersistent(JDOStateManagerImpl.java:3185)
    at org.datanucleus.state.JDOStateManagerImpl.flush(JDOStateManagerImpl.java:4513)
    at org.datanucleus.ObjectManagerImpl.flushInternal(ObjectManagerImpl.java:2814)
    at org.datanucleus.ObjectManagerImpl.flush(ObjectManagerImpl.java:2754)
    at org.datanucleus.ObjectManagerImpl.preCommit(ObjectManagerImpl.java:2893)
    at org.datanucleus.TransactionImpl.internalPreCommit(TransactionImpl.java:369)
    at org.datanucleus.TransactionImpl.commit(TransactionImpl.java:256)
    at org.datanucleus.jpa.EntityTransactionImpl.commit(EntityTransactionImpl.java:104)
    at org.datanucleus.store.appengine.jpa.DatastoreEntityTransactionImpl.commit(DatastoreEntityTransactionImpl.java:55)
    at my.package.TransactionImpl.commit(TransactionImpl.java:33)

What version of the product are you using? On what operating system?
Appengine 1.4.0 on JDK 1.6.0_22 on Windows 7. However, the bug exists in 
production code.

Please provide any additional information below.

A simple fix would be to modify the line linked to above to:

  return in==null ? null : ((Integer) in).longValue();

Original issue reported on code.google.com by manish.a...@gmail.com on 6 Jun 2011 at 5:46

GoogleCodeExporter commented 8 years ago
Looks like a very easy fix.  Could I interest you in submitting a patch?

Original comment by max.r...@gmail.com on 6 Jun 2011 at 6:09

GoogleCodeExporter commented 8 years ago
Certainly, I can submit a patch.

There is also the alternative fix of changing line 
http://code.google.com/p/datanucleus-appengine/source/browse/trunk/src/org/datan
ucleus/store/appengine/Utils.java#62 to:
to.add(fromElement==null ? null : func.apply(fromElement));

The orginal suggested fix would have to have the same solution replicated 
across multiple Functions, this new one wouldn't.

Any opinions? Thanks.

Original comment by manish.a...@gmail.com on 6 Jun 2011 at 6:41

GoogleCodeExporter commented 8 years ago
I like the original proposal better, since it should really be up to the 
function to decide how to handle a null argument.

Original comment by max.r...@gmail.com on 6 Jun 2011 at 7:51

GoogleCodeExporter commented 8 years ago
Hi,

I couldn't figure out:

   1. How to attach a patch file to the issue
   2. How to run tests on datanucleus-appengine

I've attached the patch file. Hopefully its ok.

Original comment by manish.a...@gmail.com on 7 Jun 2011 at 12:45

GoogleCodeExporter commented 8 years ago
Patch attached. Couldn't manage to run tests, so please check.

Original comment by manish.a...@gmail.com on 7 Jun 2011 at 12:46

Attachments:

GoogleCodeExporter commented 8 years ago
Sorry! I hadn't scrolled my browser window far enough to see the attach file
link. [?]

On Mon, Jun 6, 2011 at 5:45 PM, Manish Ahluwalia <manish.ahluwalia@gmail.com

Original comment by manish.a...@gmail.com on 7 Jun 2011 at 12:49

GoogleCodeExporter commented 8 years ago
Thanks for the patch!  It turns out in order to accept this I need you to sign 
a Contributor License Agreement:
http://code.google.com/legal/individual-cla-v1.0.html

It can all be done electronically.  Hopefully that's not a problem.

Max

Original comment by max.r...@gmail.com on 7 Jun 2011 at 3:24

GoogleCodeExporter commented 8 years ago
Done

Original comment by manish.a...@gmail.com on 7 Jun 2011 at 5:47

GoogleCodeExporter commented 8 years ago
Thanks Manish!

Original comment by max.r...@gmail.com on 8 Jun 2011 at 4:01