22

I am trying to asynchronously query a provider by using a CursorLoader with a SimpleCursorTreeAdapter

Here is my Fragment class which implements the CursorLoader

public class GroupsListFragment extends ExpandableListFragment implements
  LoaderManager.LoaderCallbacks<Cursor> {

  private final String DEBUG_TAG = getClass().getSimpleName().toString();      

  private static final String[] CONTACTS_PROJECTION = new String[] {
    ContactsContract.Contacts._ID,
    ContactsContract.Contacts.DISPLAY_NAME };  

  private static final String[] GROUPS_SUMMARY_PROJECTION = new String[] {
    ContactsContract.Groups.TITLE, ContactsContract.Groups._ID,
    ContactsContract.Groups.SUMMARY_COUNT,
    ContactsContract.Groups.ACCOUNT_NAME,
    ContactsContract.Groups.ACCOUNT_TYPE,
    ContactsContract.Groups.DATA_SET };

  GroupsAdapter mAdapter;

  @Override
  public void onActivityCreated(Bundle savedInstanceState) {
    super.onActivityCreated(savedInstanceState);

    populateContactList();

    getLoaderManager().initLoader(-1, null, this);
  } 

  public Loader<Cursor> onCreateLoader(int id, Bundle args) {
    // This is called when a new Loader needs to be created.
    Log.d(DEBUG_TAG, "onCreateLoader for loader_id " + id);
    CursorLoader cl;
    if (id != -1) {
      // child cursor
      Uri contactsUri = ContactsContract.Data.CONTENT_URI;
      String selection = "(("
        + ContactsContract.CommonDataKinds.GroupMembership.DISPLAY_NAME
        + " NOTNULL) AND ("
        + ContactsContract.CommonDataKinds.GroupMembership.HAS_PHONE_NUMBER
        + "=1) AND ("
        + ContactsContract.CommonDataKinds.GroupMembership.DISPLAY_NAME
        + " != '') AND ("
        + ContactsContract.CommonDataKinds.GroupMembership.GROUP_ROW_ID
        + " = ? ))";
      String sortOrder = ContactsContract.CommonDataKinds.GroupMembership.DISPLAY_NAME
        + " COLLATE LOCALIZED ASC";
      String[] selectionArgs = new String[] { String.valueOf(id) };

      cl = new CursorLoader(getActivity(), contactsUri,
        CONTACTS_PROJECTION, selection, selectionArgs, sortOrder);
    } else {
      // group cursor
      Uri groupsUri = ContactsContract.Groups.CONTENT_SUMMARY_URI;
      String selection = "((" + ContactsContract.Groups.TITLE
        + " NOTNULL) AND (" + ContactsContract.Groups.TITLE
        + " != '' ))";
      String sortOrder = ContactsContract.Groups.TITLE
        + " COLLATE LOCALIZED ASC";
      cl = new CursorLoader(getActivity(), groupsUri,
        GROUPS_SUMMARY_PROJECTION, selection, null, sortOrder);
    }

    return cl;
  }

  public void onLoadFinished(Loader<Cursor> loader, Cursor data) {
    // Swap the new cursor in. 
    int id = loader.getId();
    Log.d(DEBUG_TAG, "onLoadFinished() for loader_id " + id);
    if (id != -1) {
      // child cursor
      if (!data.isClosed()) {
        Log.d(DEBUG_TAG, "data.getCount() " + data.getCount());
        try {
          mAdapter.setChildrenCursor(id, data);
        } catch (NullPointerException e) {
          Log.w("DEBUG","Adapter expired, try again on the next query: "
            + e.getMessage());
        }
      }
    } else {
      mAdapter.setGroupCursor(data);
    }

  }

  public void onLoaderReset(Loader<Cursor> loader) {
    // This is called when the last Cursor provided to onLoadFinished()
    // is about to be closed.
    int id = loader.getId();
    Log.d(DEBUG_TAG, "onLoaderReset() for loader_id " + id);
    if (id != -1) {
      // child cursor
      try {
        mAdapter.setChildrenCursor(id, null);
      } catch (NullPointerException e) {
        Log.w("TAG", "Adapter expired, try again on the next query: "
          + e.getMessage());
      }
    } else {
      mAdapter.setGroupCursor(null);
    }
  }

  /**
  * Populate the contact list
  */
  private void populateContactList() {
    // Set up our adapter
    mAdapter = new GroupsAdapter(getActivity(),this,
      android.R.layout.simple_expandable_list_item_1,
      android.R.layout.simple_expandable_list_item_1,
      new String[] { ContactsContract.Groups.TITLE }, // Name for group layouts
      new int[] { android.R.id.text1 },
      new String[] { ContactsContract.Contacts.DISPLAY_NAME }, // Name for child layouts
      new int[] { android.R.id.text1 });

    setListAdapter(mAdapter);
  }
}

And here is my adapter which subclasses SimpleCursorTreeAdapter

public class GroupsAdapter extends SimpleCursorTreeAdapter {

  private final String DEBUG_TAG = getClass().getSimpleName().toString();

  private ContactManager mActivity;
  private GroupsListFragment mFragment;

  // Note that the constructor does not take a Cursor. This is done to avoid
  // querying the database on the main thread.
  public GroupsAdapter(Context context, GroupsListFragment glf,
    int groupLayout, int childLayout, String[] groupFrom,
    int[] groupTo, String[] childrenFrom, int[] childrenTo) {

    super(context, null, groupLayout, groupFrom, groupTo, childLayout,
      childrenFrom, childrenTo);
    mActivity = (ContactManager) context;
    mFragment = glf;
  }

  @Override
  protected Cursor getChildrenCursor(Cursor groupCursor) {
    // Given the group, we return a cursor for all the children within that group
    int groupId = groupCursor.getInt(groupCursor
      .getColumnIndex(ContactsContract.Groups._ID));

    Log.d(DEBUG_TAG, "getChildrenCursor() for groupId " + groupId);

    Loader loader = mActivity.getLoaderManager().getLoader(groupId); 
    if ( loader != null && loader.isReset() ) { 
      mActivity.getLoaderManager().restartLoader(groupId, null, mFragment); 
    } else { 
      mActivity.getLoaderManager().initLoader(groupId, null, mFragment); 
    } 

  }

}

The problem is that when i click one of the parent groups one of three things happens in what appears to be a inconsistent fashion.

1) Either the group opens up and the children appear below it

2) The group does not open and the setChildrenCursor() call throws an NullPointerException error which gets caught in the try catch block

3) The group does not open and no error is thrown

Here is some debugging output in a scenario in which a group is expanded and showing the children:

When all groups are displayed it ouputs:

05-20 10:08:22.765: D/GroupsListFragment(22132): onCreateLoader for loader_id -1
05-20 10:08:23.613: D/GroupsListFragment(22132): onLoadFinished() for loader_id -1

-1 is the loader_id of the group cursor

Then if i select one group in particular (let's just call it group A) it outputs:

05-20 23:22:31.140: D/GroupsAdapter(13844): getChildrenCursor() for groupId 67
05-20 23:22:31.140: D/GroupsListFragment(13844): onCreateLoader for loader_id 67
05-20 23:22:31.254: D/GroupsListFragment(13844): onLoadFinished() for loader_id 67
05-20 23:22:31.254: D/GroupsListFragment(13844): data.getCount() 4
05-20 23:22:31.254: W/GroupsListFragment(13844): Adapter expired, try again on the next query: null

The group does not expand and the NullPointerException is caught. Then if i select another group (let's just call it group B) it outputs:

05-20 23:25:38.089: D/GroupsAdapter(13844): getChildrenCursor() for groupId 3
05-20 23:25:38.089: D/GroupsListFragment(13844): onCreateLoader for loader_id 3
05-20 23:25:38.207: D/GroupsListFragment(13844): onLoadFinished() for loader_id 3
05-20 23:25:38.207: D/GroupsListFragment(13844): data.getCount() 6

This time, the NullPointerException is not thrown. And instead of group B expanding, group A is expanded.

Can anyone explain the behavior that the setChildrenCursor() call is exhibiting?

I am thinking there is a problem with how the group/child CursorLoaders are instantiated in onCreateLoader(). For the group CursorLoader i just want all groups in my phone. The child CursorLoader should contain all contacts within a group. Does anyone have any ideas what could be the issue?

UPDATE

Thanks to @Yam's advice I have now modified the getChildrenCursor() method. I am now selecting the groupCursor position not the value of ContactsContract.Groups._ID to pass into the initLoader() call. I also changed the logic to call restartLoader() only when loader is not null and loader isReset is false.

protected Cursor getChildrenCursor(Cursor groupCursor) {
  // Given the group, we return a cursor for all the children within that
  // group
  int groupPos = groupCursor.getPosition();
  Log.d(DEBUG_TAG, "getChildrenCursor() for groupPos " + groupPos);

  Loader loader = mActivity.getLoaderManager().getLoader(groupPos);
  if (loader != null && !loader.isReset()) {
    mActivity.getLoaderManager().restartLoader(groupPos, null, mFragment);
  } else {
    mActivity.getLoaderManager().initLoader(groupPos, null, mFragment);
  }

  return null;
}

This definitely makes more sense and does not exhibit some of the erratic behavior of a group expanding sometimes and not other times.

However, there are contacts that are being displayed under a group that they don't belong to. And also some groups that do have contacts in them but it won't show any contacts. So it seems that the getChildrenCursor() issues may now be resolved.

But now it looks to be an issue of how the CursorLoaders are instantiated in the onCreateLoader() method. Is the CursorLoader returned in the onCreateLoader() method for the child cursor being instantiated improperly?

UPDATE

So I have identified one of my issues. In the getChildrenCursor() method if I pass the groupId into the initLoader() method then in the onCreateLoader() method, when the CursorLoader is created it will get the correct groupid parameter for the query. However, in the onLoadFinished() the call to setChildrenCursor() is getting passed the loader id for the first parameter not the groupPosition. I'm guessing i have to map loader ids to group positions in some data structure. But i'm not sure if this is the best approach. Does anyone have any suggestions?

toobsco42
  • 6,131
  • 16
  • 73
  • 85
  • I've just done this, but it wasn't using a CursorLoader, so that's throwing me... In my implemmentation, the getChildrenCursor returns a cursor. With the loadermanager, where is that cursor/data actually going? If you are not feeding a cursor into the constructor, what is being fed into `getChildrenCursor` as the groupCursor? – Barak May 16 '12 at 04:41
  • The groupCursor has been set in the onLoadFinished() method of the LoaderManager. I have stepped through the code with a debugger and in the getChildenCursor() method, the groupCursor is always defined. – toobsco42 May 16 '12 at 04:47
  • I dunno, it really sounds like you aren't always getting a populated childCursor... – Barak May 16 '12 at 04:53
  • What exactly do you mean by a populated childCursor? How can I check for something like that? – toobsco42 May 16 '12 at 07:00

3 Answers3

16

So i figured out that I needed to map loaderids to groupPositions and this solved my issue:

Here is my Fragment class which implements the CursorLoader

public class GroupsListFragment extends ExpandableListFragment implements
  LoaderManager.LoaderCallbacks<Cursor> {

  private final String DEBUG_TAG = getClass().getSimpleName().toString();      

  private static final String[] CONTACTS_PROJECTION = new String[] {
    ContactsContract.Contacts._ID,
    ContactsContract.Contacts.DISPLAY_NAME };  

  private static final String[] GROUPS_PROJECTION = new String[] {
    ContactsContract.Groups.TITLE, ContactsContract.Groups._ID };

  GroupsAdapter mAdapter;

  @Override
  public void onActivityCreated(Bundle savedInstanceState) {
    super.onActivityCreated(savedInstanceState);

    populateContactList();

    // Prepare the loader. Either re-connect with an existing one,
    // or start a new one.
    Loader loader = getLoaderManager().getLoader(-1);
    if (loader != null && !loader.isReset()) {
      getLoaderManager().restartLoader(-1, null, this);
    } else {
      getLoaderManager().initLoader(-1, null, this);
    }
  } 

  public Loader<Cursor> onCreateLoader(int id, Bundle args) {
    // This is called when a new Loader needs to be created.
    Log.d(DEBUG_TAG, "onCreateLoader for loader_id " + id);
    CursorLoader cl;
    if (id != -1) {
      // child cursor
      Uri contactsUri = ContactsContract.Data.CONTENT_URI;
      String selection = "((" + ContactsContract.Contacts.DISPLAY_NAME
        + " NOTNULL) AND ("
        + ContactsContract.Contacts.HAS_PHONE_NUMBER + "=1) AND ("
        + ContactsContract.Contacts.DISPLAY_NAME + " != '') AND ("
        + ContactsContract.CommonDataKinds.GroupMembership.GROUP_ROW_ID
        + " = ? ))";
      String sortOrder = ContactsContract.Contacts.DISPLAY_NAME
        + " COLLATE LOCALIZED ASC";
      String[] selectionArgs = new String[] { String.valueOf(id) };

      cl = new CursorLoader(getActivity(), contactsUri,
        CONTACTS_PROJECTION, selection, selectionArgs, sortOrder);
    } else {
      // group cursor
      Uri groupsUri = ContactsContract.Groups.CONTENT_URI;
      String selection = "((" + ContactsContract.Groups.TITLE
        + " NOTNULL) AND (" + ContactsContract.Groups.TITLE
        + " != '' ))";
      String sortOrder = ContactsContract.Groups.TITLE
        + " COLLATE LOCALIZED ASC";
      cl = new CursorLoader(getActivity(), groupsUri,
        GROUPS_PROJECTION, selection, null, sortOrder);
    }

    return cl;
  }

  public void onLoadFinished(Loader<Cursor> loader, Cursor data) {
    // Swap the new cursor in. 
    int id = loader.getId();
    Log.d(DEBUG_TAG, "onLoadFinished() for loader_id " + id);
    if (id != -1) {
      // child cursor
      if (!data.isClosed()) {
        Log.d(DEBUG_TAG, "data.getCount() " + data.getCount());

        HashMap<Integer,Integer> groupMap = mAdapter.getGroupMap();
        try {
          int groupPos = groupMap.get(id);
          Log.d(DEBUG_TAG, "onLoadFinished() for groupPos " + groupPos);
          mAdapter.setChildrenCursor(groupPos, data);
        } catch (NullPointerException e) {
          Log.w("DEBUG","Adapter expired, try again on the next query: "
            + e.getMessage());
        }
      }
    } else {
      mAdapter.setGroupCursor(data);
    }

  }

  public void onLoaderReset(Loader<Cursor> loader) {
    // This is called when the last Cursor provided to onLoadFinished()
    // is about to be closed.
    int id = loader.getId();
    Log.d(DEBUG_TAG, "onLoaderReset() for loader_id " + id);
    if (id != -1) {
      // child cursor
      try {
        mAdapter.setChildrenCursor(id, null);
      } catch (NullPointerException e) {
        Log.w("TAG", "Adapter expired, try again on the next query: "
          + e.getMessage());
      }
    } else {
      mAdapter.setGroupCursor(null);
    }
  }

  /**
  * Populate the contact list
  */
  private void populateContactList() {
    // Set up our adapter
    mAdapter = new GroupsAdapter(getActivity(),this,
      android.R.layout.simple_expandable_list_item_1,
      android.R.layout.simple_expandable_list_item_1,
      new String[] { ContactsContract.Groups.TITLE }, // Name for group layouts
      new int[] { android.R.id.text1 },
      new String[] { ContactsContract.Contacts.DISPLAY_NAME }, // Name for child layouts
      new int[] { android.R.id.text1 });

    setListAdapter(mAdapter);
  }
}

And here is my adapter which subclasses SimpleCursorTreeAdapter

public class GroupsAdapter extends SimpleCursorTreeAdapter {

  private final String DEBUG_TAG = getClass().getSimpleName().toString();

  private ContactManager mActivity;
  private GroupsListFragment mFragment;

  protected final HashMap<Integer, Integer> mGroupMap;

  // Note that the constructor does not take a Cursor. This is done to avoid
  // querying the database on the main thread.
  public GroupsAdapter(Context context, GroupsListFragment glf,
    int groupLayout, int childLayout, String[] groupFrom,
    int[] groupTo, String[] childrenFrom, int[] childrenTo) {

    super(context, null, groupLayout, groupFrom, groupTo, childLayout,
      childrenFrom, childrenTo);
    mActivity = (ContactManager) context;
    mFragment = glf;
    mGroupMap = new HashMap<Integer, Integer>();
  }

  @Override
  protected Cursor getChildrenCursor(Cursor groupCursor) {
    // Given the group, we return a cursor for all the children within that group
    int groupPos = groupCursor.getPosition();
    int groupId = groupCursor.getInt(groupCursor
      .getColumnIndex(ContactsContract.Groups._ID));
    Log.d(DEBUG_TAG, "getChildrenCursor() for groupPos " + groupPos);
    Log.d(DEBUG_TAG, "getChildrenCursor() for groupId " + groupId);

    mGroupMap.put(groupId, groupPos);

    Loader loader = mActivity.getLoaderManager().getLoader(groupId); 
    if ( loader != null && !loader.isReset() ) { 
      mActivity.getLoaderManager().restartLoader(groupId, null, mFragment); 
    } else { 
      mActivity.getLoaderManager().initLoader(groupId, null, mFragment); 
    } 

    return null;    
  }

  //Accessor method
  public HashMap<Integer, Integer> getGroupMap() {
    return mGroupMap;
  }

}
toobsco42
  • 6,131
  • 16
  • 73
  • 85
  • 2
    Note, Android has a SparseIntArray (see http://developer.android.com/reference/android/util/SparseIntArray.html) which is better to use than HashMap and serves the same purpose. – Dandre Allison Aug 07 '12 at 18:43
  • Thanks for the recommendation. I will give that a try. – toobsco42 Aug 07 '12 at 18:58
  • 1
    According to this analysis: http://mobile.dzone.com/articles/tweaking-your-android they achieve similar results for number of elements I have in the hashmap which is less than 1,000. – toobsco42 Aug 07 '12 at 19:03
  • I see, thanks for sharing the analysis, but I take away from that test that you can't do worse with SparseIntArray. So I guess it's up to the dev to decide, you can check out the source to see how SparseIntArray works. Apparently it's backed by a pair of arrays and binary search. – Dandre Allison Aug 07 '12 at 19:26
  • 1
    According to two google engineers (Romain Guy & Dianne Hackborn), https://groups.google.com/forum/?fromgroups#!starred/android-developers/G-ceop4xp48 they recommend using SparseArrays over HashMaps. So I will take their word for it. – toobsco42 Aug 07 '12 at 21:58
  • 1
    Nice. I had read something like that before, the key is "boxing/unboxing of primitive types", because Integer is an Object wrapping for ints. SparseArrays only deal with the primitive types so there is no boxing/unboxing which I think leads to less GCs. – Dandre Allison Aug 07 '12 at 22:49
  • In onLoadFinished(), I had to change the line mAdapter.setGroupCursor(data); to mAdapter.changeCursor(data); or else the getChildrenCursor() method would not be called if you would load another group cursor in. This was such and idiotic bug.... – Bogdan Zurac Feb 14 '13 at 22:37
  • Do you really need to map groupPosition and groupId? groupPosition can be used as loader id's and the groupId can be set in a Bundle and passed to CursorLoader instead of passing in null. – jobinbasani Apr 11 '14 at 14:22
  • 1
    should the reset method be like mAdapter.setChildrenCursor(position, null); instead of mAdapter.setChildrenCursor(id, null); – Dimitar Vukman Dec 05 '16 at 13:42
2

In my case, I use the first argument of initLoader (or restartLoader) to give the group position for callback and use Bundle to get children data in getChildrenCursor.

Like following;

public class ExpandableListAdapter extends SimpleCursorTreeAdapter implements LoaderManager.LoaderCallbacks<Cursor> {
    private Context mContext;
    private LoaderManager mManager;

    public ExpandableListAdapter(
            Context context, ExpandableListAdapterListener listener, LoaderManager manager, Cursor groupCursor,
            int groupLayout, String[] groupFrom, int[] groupTo,
            int childLayout, String[] childFrom, int[] childTo) {
        super(context, groupCursor, groupLayout, groupFrom, groupTo, childLayout, childFrom, childTo);
        mContext  = context;
        mManager  = manager;
    }
    @Override
    protected Cursor getChildrenCursor(Cursor groupCursor) {
        final long idGroup = groupCursor.getLong(groupCursor.getColumnIndex("_id"));
        Bundle bundle = new Bundle();
        bundle.putLong("idGroup", idGroup);
        int groupPos = groupCursor.getPosition();
        if (mManager.getLoader(groupPos) != null && !mManager.getLoader(groupPos).isReset()) {
            mManager.restartLoader(groupPos, bundle, this);
        }
        else {
            mManager.initLoader(groupPos, bundle, this);
        }
        return null;
    }
    @Override
    public Loader<Cursor> onCreateLoader(int groupPos, Bundle bundle) {
        long idGroup = bundle.getLong("idGroup");
        return new CursorLoader(
                mContext,
                Provider.URI,
                new String[]{Table.ID, Table.ID_GROUP, Table.TITLE, Table.CONTEXT},
                Table.ID_GROUP + " = ?",
                new String[]{String.valueOf(idGroup)},
                Table.CREATED + " DESC"
        );
    }
    @Override
    public void onLoadFinished(Loader<Cursor> loader, Cursor cursor) {
        setChildrenCursor(loader.getId(), cursor);
    }
    @Override
    public void onLoaderReset(Loader<Cursor> loader) {
    }
}
Mitsuaki Ishimoto
  • 2,944
  • 2
  • 22
  • 32
1

I have bad experience with using ExpandableListView. Its behavior in different Android versions are different. If you are not already too deep into it, you may like to consider redesigning your interface.

Anyway, to your questions, I suggest you review these 3 points.

First, in your call to init the children cursor loader

mActivity.getLoaderManager().initLoader(groupId, null, mFragment); 

The groupId you passed in is the value of ContactsContract.Groups._ID. Then, you use this id in the setChildrenCursor's first parameter. This is probably wrong. Instead of passing the groupId into the initLoader, try passing in the group cursor position. For example:

int iGroupPos = groupCursor.getPosition();
if ( loader != null && !loader.isReset())
    mActivity.getLoaderManager().restartLoader(iGroupPos, null, mFragment); 
else
    mActivity.getLoaderManager().initLoader(iGroupPos, null, mFragment); 

Second, you can see that in the code I suggested above, you probably should call restartLoader only when loader is not null and loader isReset is false.

Third, you need to return a value for the getChildrenCursor call, which I believe should probably be null.

Yam
  • 133
  • 2
  • 7
  • This definitely makes sense and appears to be closer to what exactly I want. But I am still having issues which i mentioned under UPDATE in the question above. – toobsco42 Jun 05 '12 at 16:43