j256 / ormlite-android

ORMLite Android functionality used in conjunction with ormlite-core
http://ormlite.com/
ISC License
1.59k stars 367 forks source link

Add an Android CursorLoader and CursorAdapter implementation #8

Closed emmby closed 10 years ago

emmby commented 10 years ago

Using OrmLite on Android requires more code than it seems like it should. This pull request implements a fairly standard Loader and Adapter to make ormlite easy to use with contemporary Android paradigms like Fragments.

It relies on PR https://github.com/j256/ormlite-core/pull/17 in ormlite-core to facilitate the notification aspect of Android Loaders and Adapters.

Sample app is available here: https://github.com/emmby/ormlite-pr8-sample

emmby commented 10 years ago

Just added a sample app in the PR description.

pelotasplus commented 10 years ago

i'm using your changes in my app.

data being fetched from the backend using IntentService, written to ormlite database using batch mode. after that one single modelDao.notifyContentChange() call notitifies loaders about new data.

works like a charm. i'd love to see this PR accepted!

pjakubczyk commented 10 years ago

@emmby when I could expect this PR to be accepted :)? It works great but having all ormlite related source code in my app doesn't look great :D

Looking forward to maven publish.

emmby commented 10 years ago

Glad you like it. I'd love to see it merged in as well. @kpgalligan were you able to build and try out the change? Any thoughts on what (if anything) would need to be done to the PR before it could be merged? I'm happy to help.

pjakubczyk commented 10 years ago

@emmby hey! do you have any updates from @kpgalligan ?

emielvanliere commented 10 years ago

@emmby this is a very useful PR, thanks!

emmby commented 10 years ago

Great, glad you like it! @kpgalligan @j256 Any thing I can do to help get this PR merged in?

emmby commented 10 years ago

ping

j256 commented 10 years ago

Sorry for the delay on this. What versions of Android is this compatible with? Is there anything that will cause compilation errors in older versions?

mseroczynski commented 10 years ago

How long will it take to merge it? I'm using temporary solution now but it would be really nice if You'd ensure us that it will be done soon. Btw, thanks for OrmLite :).

pjakubczyk commented 10 years ago

now ping to @emmby :D

emmby commented 10 years ago

It will compile and run down to API level 11 (3.0), which is when CursorLoader was first introduced. Apps supporting earlier API levels should be able to link to ormlite-android and run without problems, I believe.

mseroczynski commented 10 years ago

how you doin' @j256

emmby commented 10 years ago

@j256 @kpgalligan I'm starting work on the 3rd edition of Android for Dummies. I'd love to be able to reference Ormlite. What can we do to release a version of ormlite for android that supports loaders and adapters in time for publication deadlines?

simonides commented 10 years ago

I've been searching around for a few hours now just because I couldn't believe that Ormlite hasn't got these Classes already built in!; I'm glad I stumbled upon this page. @j256 @kpgalligan How does it come that such important classes do not find their way into Ormlite faster?

@emmby Can't you release an extra bundle with these classes in the meantime?

mseroczynski commented 10 years ago

I don't recommend waiting for any action, I've waited for 3 months and gave up, now I've got modified code in my project and I regret not doing it right when it was needed.

simonides commented 10 years ago

@mseroczynski I am not sure if I understood you right. Do you use ormlight in your project now or not? What exactly do you regret? Did you build the lib yourself?

emmby commented 10 years ago

Woo! Great news! Thanks @j256 . Don't forget that this PR also depends on https://github.com/j256/ormlite-core/pull/17 in ormlite-core

j256 commented 10 years ago

Sorry for the delay on this guys. I should have merged it earlier because it only adds files. My worry was that this would invalidate support for older versions of Android. In general I don't have a lot of time for ORMLite and I really don't have the ability to validate this code.... But I'll take a chance given the multiple interest.

j256 commented 10 years ago

Shit and of course I'd forgotten that part. That's going to take a bunch of time for me to verify. I don't like some of that merge. Knew I should have taken the time to look at this more closely. Dammit.

emmby commented 10 years ago

I can hop on and address any concerns on that side. Ultimately it's functionally a pretty simple change (just maintain a list of listeners), but agreed it wasn't clear to me the best way to implement it and so there's likely opportunities for improvement

simonides commented 10 years ago

Wow didn't expect this is ging to happen so fast. The only thing that I didn't understand is why you don't or didn't express your concerns because obviously emmby is willing to help out on this one and I could imagine that some more people could be found to do so. But I respect it if you want to keep your hand over it and thanks to both of you for your great work.

emmby commented 10 years ago

These things take a little time sometimes. Anyway, hopefully we can find the best way forward.

kpgalligan commented 10 years ago

Sorry all. Notifications go to (apparently) non-priority mail on gmail. I'd love to spend more time on this, but to say I'm swamped would be putting it mildly. We use ormlite with loaders. Just didn't build anything specifically to merge the two. By way of apology, all I can say is besides an Android consulting company I'm running this: http://nyc.droidcon.com/2014/. However, if anybody wants to dig deep on orm and Android, submit a talk!

j256 commented 10 years ago

Wait, I see two versions of OrmLiteCursorLoader. One in apptools and one in apptools/support. Which one is the right one?

kpgalligan commented 10 years ago

There need to be two versions. One is "standard" Android, and the other is for the support package. Loaders (Fragments, other things) exist in a weird quantum state at the moment (and probably for a long time).

j256 commented 10 years ago

So I don't think its a problem to have these classes in the ormlite-android project @kpgalligan. They aren't required so you won't get a problem unless you reference them, right? The DaoObserver pattern in ormlite-core is generic and not android specific.

j256 commented 10 years ago

Ok. I've made some refactoring and reformatting changes to the code @emmby. Thanks much for the suggestions. Let me know if you have any feedback. I've also checked in the code into -core although I did not take your other pull.

kpgalligan commented 10 years ago

You mean the two versions in the build, even if one or the other's dependency isn't in the user's project? You should be OK, assuming of course the code doesn't reference the incorrect one (or either, if you're not using either). I would not put money on that till I'd tried it, but should be OK.

I think I'm reading the pull request wrong, but I don't see the added dependency to the support libs in the maven pom file (but the compile would fail, so I must be looking at it wrong. I don't do much with the pull requests).

emmby commented 10 years ago

Wonderful! Thanks everyone for making this happen!

simonides commented 10 years ago

Sorry to ask but does that mean that there is already a build to download?

VincentJousse commented 10 years ago

I would also be very interested in a new ORMLite build with these commits embedded. @j256 is it planned in the comming days or weeks ?

efung commented 10 years ago

@j256 Unfortunately, the changes you made in https://github.com/j256/ormlite-core/commit/c16504c87ec77f6da7eaccdf931b36993efaffff don't work.

ConcurrentHashMap.put(k,v) does not allow null values, whereas WeakHashMap.put(k,v) does, so it throws an exception now. I discovered this when running the tests.

A map isn't needed here, is it? Would a HashSet (possibly wrapped in Collections.synchronizedSet()) work?

cc @emmby

ghost commented 10 years ago

I can confirm @efung here's is the full stacktrace

java.lang.NullPointerException
at  java.util.concurrent.ConcurrentHashMap.put (ConcurrentHashMap.java : 1017)
at  com.j256.ormlite.dao.BaseDaoImpl.registerObserver (BaseDaoImpl.java : 848)
at  com.j256.ormlite.android.apptools.OrmLiteCursorLoader. (OrmLiteCursorLoader.java : 32)
at  com.example.ui.BlockScheduleItemsFragment.onCreateLoader (BlockScheduleItemsFragment.java : 401)
at  android.app.LoaderManagerImpl.createLoader (LoaderManager.java : 546)
at  android.app.LoaderManagerImpl.createAndInstallLoader (LoaderManager.java : 555)
at  android.app.LoaderManagerImpl.restartLoader (LoaderManager.java : 704)
at  com.example.ui.BlockScheduleItemsFragment.onResume (BlockScheduleItemsFragment.java : 167)
at  android.app.Fragment.performResume (Fragment.java : 1743)
at  android.app.FragmentManagerImpl.moveToState (FragmentManager.java : 924)
at  android.app.FragmentManagerImpl.moveToState (FragmentManager.java : 1062)
at  android.app.BackStackRecord.run (BackStackRecord.java : 684)
at  android.app.FragmentManagerImpl.execPendingActions (FragmentManager.java : 1447)
at  android.app.FragmentManagerImpl.executePendingTransactions (FragmentManager.java : 479)
at  android.support.v13.app.FragmentStatePagerAdapter.finishUpdate (FragmentStatePagerAdapter.java : 167)
at  android.support.v4.view.ViewPager.populate (ViewPager.java : 1073)
at  android.support.v4.view.ViewPager.populate (ViewPager.java : 919)
at  android.support.v4.view.ViewPager.onMeasure (ViewPager.java : 1441)
at  android.view.View.measure (View.java : 16497)
at  android.widget.RelativeLayout.measureChildHorizontal (RelativeLayout.java : 719)
at  android.widget.RelativeLayout.onMeasure (RelativeLayout.java : 455)
at  android.view.View.measure (View.java : 16497)
at  android.widget.RelativeLayout.measureChildHorizontal (RelativeLayout.java : 719)
at  android.widget.RelativeLayout.onMeasure (RelativeLayout.java : 455)
at  android.view.View.measure (View.java : 16497)
at  android.view.ViewGroup.measureChildWithMargins (ViewGroup.java : 5125)
at  android.widget.FrameLayout.onMeasure (FrameLayout.java : 310)
at  android.view.View.measure (View.java : 16497)
at  android.support.v4.widget.DrawerLayout.onMeasure (DrawerLayout.java : 762)
at  android.view.View.measure (View.java : 16497)
at  android.view.ViewGroup.measureChildWithMargins (ViewGroup.java : 5125)
at  android.widget.FrameLayout.onMeasure (FrameLayout.java : 310)
at  android.view.View.measure (View.java : 16497)
at  android.view.ViewGroup.measureChildWithMargins (ViewGroup.java : 5125)
at  com.android.internal.widget.ActionBarOverlayLayout.onMeasure (ActionBarOverlayLayout.java : 327)
at  android.view.View.measure (View.java : 16497)
at  android.view.ViewGroup.measureChildWithMargins (ViewGroup.java : 5125)
at  android.widget.FrameLayout.onMeasure (FrameLayout.java : 310)
at  com.android.internal.policy.impl.PhoneWindow$DecorView.onMeasure (PhoneWindow.java : 2291)
at  android.view.View.measure (View.java : 16497)
at  android.view.ViewRootImpl.performMeasure (ViewRootImpl.java : 1912)
at  android.view.ViewRootImpl.measureHierarchy (ViewRootImpl.java : 1109)
at  android.view.ViewRootImpl.performTraversals (ViewRootImpl.java : 1291)
at  android.view.ViewRootImpl.doTraversal (ViewRootImpl.java : 996)
at  android.view.ViewRootImpl$TraversalRunnable.run (ViewRootImpl.java : 5600)
at  android.view.Choreographer$CallbackRecord.run (Choreographer.java : 761)
at  android.view.Choreographer.doCallbacks (Choreographer.java : 574)
at  android.view.Choreographer.doFrame (Choreographer.java : 544)
at  android.view.Choreographer$FrameDisplayEventReceiver.run (Choreographer.java : 747)
at  android.os.Handler.handleCallback (Handler.java : 733)
at  android.os.Handler.dispatchMessage (Handler.java : 95)
at  android.os.Looper.loop (Looper.java : 136)
at  android.app.ActivityThread.main (ActivityThread.java : 5001)
at  java.lang.reflect.Method.invokeNative Method
at  com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java : 785)
at  com.android.internal.os.ZygoteInit.main (ZygoteInit.java : 601)
ghost commented 10 years ago

And why is the support.OrmLiteCursorLoader not merged? Because when using import android.support.v4.content.Loader; instead of import android.content.Loader; I get this lint error: Type mismatch: cannot convert from OrmLiteCursorLoader<Event> to Loader<Cursor>.

j256 commented 10 years ago

Oh, in terms of a HashSet versus a HashMap @efung , if you actually look at the HashSet code, it uses and internal HashMap with constant values. :-) ConcurrentHashMap is the way to go instead of a synchronized wrapper around HashSet.

efung commented 10 years ago

Actually, to echo what @GerritHoekstra mentioned above, I just noticed that 3f490869ca2c52b533e71b3f797e3d692dc4220a removes the support.OrmLiteCursorLoader class. I'm stuck supporting Android 4.0.4 and would like to use the compatibility package since its AsyncTaskLoader supports cancelling, which was not added until API Level 16.

j256 commented 10 years ago

I think it just moved it to another package @efung. Am I missing something? See: https://github.com/j256/ormlite-android/blob/master/src/main/java/com/j256/ormlite/android/apptools/OrmLiteCursorLoader.java

efung commented 10 years ago

No, if you look at the original PR #8 , you'll see there are two versions of the class. The difference is whether it extends android.content.AsyncTaskLoader or android.support.v4.content.AsyncTaskLoader. See this previous comment

j256 commented 10 years ago

Huh. I don't fully understand this. My apologies. Do I really need to have 2 versions of the class or can I just use the support class which hopefully extends android.content.AsyncTaskLoader if in v4?

efung commented 10 years ago

You do need to have two versions, because otherwise you'd force people to link in the support library, even if they were on the latest version of Android.

j256 commented 10 years ago

Wow. What a mess. Sigh.

efung commented 10 years ago

Yeah, indeed. You see multiple versions of stuff in everyone's library projects whenever Fragments or Loaders are involved, e.g.:

https://github.com/commonsguy/cwac-pager/tree/master/pager/src/com/commonsware/cwac/pager

or you force them to include the compat library, e.g.:

https://github.com/JakeWharton/Android-ViewPagerIndicator/tree/master/library

atali commented 10 years ago

Is there any downloable package ?

VincentJousse commented 9 years ago

Hi all,

here is an issue I've added these days: #30 . @j256 would like me to query your opinion about it. Please take a look, it's describing a very common and simple use case of OrmLite loaders. Do you think the issue should be resolved inside OrmLite or do you think it's the application's responsability ?

Thanks !

VincentJousse commented 9 years ago

Hi again,

I'm facing a database connection issue after configuration changes. I'm using a single activity which extends OrmLiteBaseActivity. I also use an OrmLiteQueryForAllLoader in this activity to load my persisted objects. At configuration changes, the activity is destroyed, so the database helper close the database connection since nobody use it anymore. Then the activity is recreated, leading to a new database connection. The problem is that OrmLiteQueryForAllLoader (which is kept alive even at configuration changes) retains a Dao object with the first created dabase connection. After a configuration change, this connection is closed and this leads to a IllegalStateException since the retained Dao tries to use a closed connection.

We need a way to keep the connection opened even when all activities are destroyed, maybe by retaining it in the loaders ? What do you think of this ?

martinrevert commented 9 years ago

Hi! Is this already merged on stable?

guilhermekrz commented 9 years ago

I don't think so, I am grabbing it from http://ormlite.com/releases/, version 4.49-SNAPSHOT

guilhermekrz commented 9 years ago

Unfortunately this don't work with RecyclerView. Using the following approach: https://gist.github.com/guilhermekrz/504ea775ad3e15932836 Anyone have a better idea? Could this be incorporated into ormlite-android?