asolfre / objectify-appengine

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

Wrapper for Saver & Deleter #131

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I've noticed that while Loader has a Wrapper implementation, neither Saver nor 
Deleter have one.
So, here's a patch proposing a SaverWrapper and a Deleter Wrapper.

Here is the exact description of what the patch does :

 - Saver.java: Adds the method setWrapper(Saver)
 - SaverImpl.java: Every call to a this method is now a call to this.wrapper
 - SaverWrapper.java: Simple Wrapper for the Saver interface

 - Deleter.java: Adds the method setWrapper(Deleter)
 - DeleterImpl.java: Every call to a this method is now a call to this.wrapper
 - DeleterTypeImpl.java: Instead of using WriteEngine.delete, uses Deleter.keys (using the Deleter Wrapper)
 - DeleterWrapper.java: Simple Wrapper for the Deleter interface

 - DatastoreUtils: Adds the method public static List<Key<T>> createKeys(Key<?>, Class<T>, Iterable<?>) that create multiple Key<?> with the same parent and type, and multiple ids.

The only downside I see with this patch is that DeleterTypeImpl is a bit less 
efficient : It used to direclty create Raw datastore keys and pass them to 
WriteEngine. Now it creates Objectify keys, which are then transformed to raw 
datastore keys to be passed to WriteEngine. There is an additional treatment.
While I totaly understand the need for Objectify to be efficient, I don't think 
that, in this particular case, this is a great matter. I think it is best like 
that because:
 * DeleterType does not need a wrapper as DeleterTypeImpl simply leads back to Deleter (it is a shortcut to create keys manually)
 * The only real interesting method to override on DeleterWrapper is keys(Iterable<? extends Key<?>>) and entities(Iterable<?>) as every other method of deletion leads back to one of those two methods.
It is therefore far easier to hook on a deletion.

Original issue reported on code.google.com by salo...@kickyourapp.com on 24 Jul 2012 at 10:48

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by lhori...@gmail.com on 24 Jul 2012 at 4:48

GoogleCodeExporter commented 9 years ago
This has been more-or-less applied to master.  Discussion can continue on the 
GAE list.  Thanks for the patch!

Original comment by lhori...@gmail.com on 28 Jul 2012 at 1:18