Update the call log contact info cache.

Now that we use the call log cache again, we should also update it when
we find more up-to-date information when looking up in the contacts
database.

Bug: 5101753
Change-Id: I408c1d5c8ec3752d5c15e761274eb06fb9242de6
diff --git a/src/com/android/contacts/calllog/CallLogAdapter.java b/src/com/android/contacts/calllog/CallLogAdapter.java
index 7e934b6..28e2e90 100644
--- a/src/com/android/contacts/calllog/CallLogAdapter.java
+++ b/src/com/android/contacts/calllog/CallLogAdapter.java
@@ -24,6 +24,7 @@
 import com.android.contacts.util.ExpirableCache;
 import com.google.common.annotations.VisibleForTesting;
 
+import android.content.ContentValues;
 import android.content.Context;
 import android.content.res.Resources;
 import android.database.Cursor;
@@ -37,6 +38,7 @@
 import android.provider.ContactsContract.PhoneLookup;
 import android.telephony.PhoneNumberUtils;
 import android.text.TextUtils;
+import android.util.Pair;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -47,7 +49,7 @@
 /**
  * Adapter class to fill in data for the Call Log.
  */
-public final class CallLogAdapter extends GroupingListAdapter
+public class CallLogAdapter extends GroupingListAdapter
         implements Runnable, ViewTreeObserver.OnPreDrawListener, CallLogGroupBuilder.GroupCreator {
     /** Interface used to initiate a refresh of the content. */
     public interface CallFetcher {
@@ -75,10 +77,13 @@
     /**
      * List of requests to update contact details.
      * <p>
+     * Each request is made of a phone number to look up, and the contact info currently stored in
+     * the call log for this number.
+     * <p>
      * The requests are added when displaying the contacts and are processed by a background
      * thread.
      */
-    private final LinkedList<String> mRequests;
+    private final LinkedList<Pair<String, ContactInfo>> mRequests;
 
     private volatile boolean mDone;
     private boolean mLoading = true;
@@ -155,7 +160,7 @@
         mCallFetcher = callFetcher;
 
         mContactInfoCache = ExpirableCache.create(CONTACT_INFO_CACHE_SIZE);
-        mRequests = new LinkedList<String>();
+        mRequests = new LinkedList<Pair<String,ContactInfo>>();
         mPreDrawListener = null;
 
         Resources resources = mContext.getResources();
@@ -230,10 +235,21 @@
         mPreDrawListener = null;
     }
 
-    private void enqueueRequest(String number, boolean immediate) {
+    /**
+     * Enqueues a request to look up the contact details for the given phone number.
+     * <p>
+     * It also provides the current contact info stored in the call log for this number.
+     * <p>
+     * If the {@code immediate} parameter is true, it will start immediately the thread that looks
+     * up the contact information (if it has not been already started). Otherwise, it will be
+     * started with a delay. See {@link #START_PROCESSING_REQUESTS_DELAY_MILLIS}.
+     */
+    @VisibleForTesting
+    void enqueueRequest(String number, ContactInfo callLogInfo, boolean immediate) {
+        Pair<String, ContactInfo> request = new Pair<String, ContactInfo>(number, callLogInfo);
         synchronized (mRequests) {
-            if (!mRequests.contains(number)) {
-                mRequests.add(number);
+            if (!mRequests.contains(request)) {
+                mRequests.add(request);
                 mRequests.notifyAll();
             }
         }
@@ -378,12 +394,15 @@
     /**
      * Queries the appropriate content provider for the contact associated with the number.
      * <p>
+     * Upon completion it also updates the cache in the call log, if it is different from
+     * {@code callLogInfo}.
+     * <p>
      * The number might be either a SIP address or a phone number.
      * <p>
      * It returns true if it updated the content of the cache and we should therefore tell the
      * view to update its content.
      */
-    private boolean queryContactInfo(String number) {
+    private boolean queryContactInfo(String number, ContactInfo callLogInfo) {
         final ContactInfo info;
 
         // Determine the contact info.
@@ -411,6 +430,9 @@
         // Store the data in the cache so that the UI thread can use to display it. Store it
         // even if it has not changed so that it is marked as not expired.
         mContactInfoCache.put(number, info);
+        // Update the call log even if the cache it is up-to-date: it is possible that the cache
+        // contains the value from a different call log entry.
+        updateCallLogContactInfoCache(number, info, callLogInfo);
         return updated;
     }
 
@@ -423,9 +445,12 @@
         boolean needNotify = false;
         while (!mDone) {
             String number = null;
+            ContactInfo callLogInfo = null;
             synchronized (mRequests) {
                 if (!mRequests.isEmpty()) {
-                    number = mRequests.removeFirst();
+                    Pair<String, ContactInfo> request = mRequests.removeFirst();
+                    number = request.first;
+                    callLogInfo  = request.second;
                 } else {
                     if (needNotify) {
                         needNotify = false;
@@ -439,7 +464,7 @@
                     }
                 }
             }
-            if (!mDone && number != null && queryContactInfo(number)) {
+            if (!mDone && number != null && queryContactInfo(number, callLogInfo)) {
                 needNotify = true;
             }
         }
@@ -571,14 +596,20 @@
             info = ContactInfo.EMPTY;
             mContactInfoCache.put(number, info);
             // Request the contact details immediately since they are currently missing.
-            enqueueRequest(number, true);
+            enqueueRequest(number, cachedContactInfo, true);
             // Format the phone number in the call log as best as we can.
             formattedNumber = formatPhoneNumber(number, null, countryIso);
         } else {
             if (cachedInfo.isExpired()) {
                 // The contact info is no longer up to date, we should request it. However, we
                 // do not need to request them immediately.
-                enqueueRequest(number, false);
+                enqueueRequest(number, cachedContactInfo, false);
+            } else  if (!callLogInfoMatches(cachedContactInfo, info)) {
+                // The call log information does not match the one we have, look it up again.
+                // We could simply update the call log directly, but that needs to be done in a
+                // background thread, so it is easier to simply request a new lookup, which will, as
+                // a side-effect, update the call log.
+                enqueueRequest(number, cachedContactInfo, false);
             }
 
             if (info != ContactInfo.EMPTY) {
@@ -629,6 +660,52 @@
         }
     }
 
+    /** Checks whether the contact info from the call log matches the one from the contacts db. */
+    private boolean callLogInfoMatches(ContactInfo callLogInfo, ContactInfo info) {
+        // The call log only contains a subset of the fields in the contacts db.
+        // Only check those.
+        return TextUtils.equals(callLogInfo.name, info.name)
+                && callLogInfo.type == info.type
+                && TextUtils.equals(callLogInfo.label, info.label);
+    }
+
+    /** Stores the updated contact info in the call log if it is different from the current one. */
+    private void updateCallLogContactInfoCache(String number, ContactInfo updatedInfo,
+            ContactInfo callLogInfo) {
+        final ContentValues values = new ContentValues();
+        boolean needsUpdate = false;
+
+        if (callLogInfo != null) {
+            if (!TextUtils.equals(updatedInfo.name, callLogInfo.name)) {
+                values.put(Calls.CACHED_NAME, updatedInfo.name);
+                needsUpdate = true;
+            }
+
+            if (updatedInfo.type != callLogInfo.type) {
+                values.put(Calls.CACHED_NUMBER_TYPE, updatedInfo.type);
+                needsUpdate = true;
+            }
+
+            if (!TextUtils.equals(updatedInfo.label, callLogInfo.label)) {
+                values.put(Calls.CACHED_NUMBER_LABEL, updatedInfo.label);
+                needsUpdate = true;
+            }
+        } else {
+            needsUpdate = true;
+        }
+
+        if (!needsUpdate) {
+            return;
+        }
+
+        StringBuilder where = new StringBuilder();
+        where.append(Calls.NUMBER);
+        where.append(" = ?");
+
+        mContext.getContentResolver().update(Calls.CONTENT_URI_WITH_VOICEMAIL, values,
+                where.toString(), new String[]{ number });
+    }
+
     /** Returns the contact information as stored in the call log. */
     private ContactInfo getContactInfoFromCallLog(Cursor c) {
         ContactInfo info = new ContactInfo();
diff --git a/tests/src/com/android/contacts/calllog/CallLogAdapterTest.java b/tests/src/com/android/contacts/calllog/CallLogAdapterTest.java
new file mode 100644
index 0000000..28db896
--- /dev/null
+++ b/tests/src/com/android/contacts/calllog/CallLogAdapterTest.java
@@ -0,0 +1,206 @@
+/*
+ * Copyright (C) 2011 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.contacts.calllog;
+
+import com.google.common.collect.Lists;
+
+import android.content.Context;
+import android.database.MatrixCursor;
+import android.test.AndroidTestCase;
+import android.view.View;
+
+import java.util.List;
+
+/**
+ * Unit tests for {@link CallLogAdapter}.
+ */
+public class CallLogAdapterTest extends AndroidTestCase {
+    private static final String TEST_NUMBER = "12345678";
+    private static final String TEST_NAME = "name";
+    private static final String TEST_NUMBER_LABEL = "label";
+    private static final int TEST_NUMBER_TYPE = 1;
+    private static final String TEST_COUNTRY_ISO = "US";
+    private static final String TEST_VOICEMAIL_NUMBER = "111";
+
+    /** The object under test. */
+    private TestCallLogAdapter mAdapter;
+
+    private MatrixCursor mCursor;
+    private View mView;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        // Use a call fetcher that does not do anything.
+        CallLogAdapter.CallFetcher fakeCallFetcher = new CallLogAdapter.CallFetcher() {
+            @Override
+            public void startCallsQuery() {}
+        };
+
+        mAdapter = new TestCallLogAdapter(getContext(), fakeCallFetcher, TEST_COUNTRY_ISO,
+                TEST_VOICEMAIL_NUMBER);
+        // The cursor used in the tests to store the entries to display.
+        mCursor = new MatrixCursor(CallLogQuery.EXTENDED_PROJECTION);
+        mCursor.moveToFirst();
+        // The views into which to store the data.
+        mView = new View(getContext());
+        mView.setTag(CallLogListItemViews.createForTest(getContext()));
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        mAdapter = null;
+        mCursor = null;
+        mView = null;
+        super.tearDown();
+    }
+
+    public void testBindView_NoCallLogCacheNorMemoryCache_EnqueueRequest() {
+        mCursor.addRow(createCallLogEntry());
+
+        // Bind the views of a single row.
+        mAdapter.bindStandAloneView(mView, getContext(), mCursor);
+
+        // There is one request for contact details.
+        assertEquals(1, mAdapter.requests.size());
+
+        TestCallLogAdapter.Request request = mAdapter.requests.get(0);
+        // It is for the number we need to show.
+        assertEquals(TEST_NUMBER, request.number);
+        // Since there is nothing in the cache, it is an immediate request.
+        assertTrue("should be immediate", request.immediate);
+    }
+
+    public void testBindView_CallLogCacheButNoMemoryCache_EnqueueRequest() {
+        mCursor.addRow(createCallLogEntryWithCachedValues());
+
+        // Bind the views of a single row.
+        mAdapter.bindStandAloneView(mView, getContext(), mCursor);
+
+        // There is one request for contact details.
+        assertEquals(1, mAdapter.requests.size());
+
+        TestCallLogAdapter.Request request = mAdapter.requests.get(0);
+        // The values passed to the request, match the ones in the call log cache.
+        assertEquals(TEST_NAME, request.callLogInfo.name);
+        assertEquals(1, request.callLogInfo.type);
+        assertEquals(TEST_NUMBER_LABEL, request.callLogInfo.label);
+    }
+
+
+    public void testBindView_NoCallLogButMemoryCache_EnqueueRequest() {
+        mCursor.addRow(createCallLogEntry());
+        mAdapter.injectContactInfoForTest(TEST_NUMBER, createContactInfo());
+
+        // Bind the views of a single row.
+        mAdapter.bindStandAloneView(mView, getContext(), mCursor);
+
+        // There is one request for contact details.
+        assertEquals(1, mAdapter.requests.size());
+
+        TestCallLogAdapter.Request request = mAdapter.requests.get(0);
+        // Since there is something in the cache, it is not an immediate request.
+        assertFalse("should not be immediate", request.immediate);
+    }
+
+    public void testBindView_BothCallLogAndMemoryCache_NoEnqueueRequest() {
+        mCursor.addRow(createCallLogEntryWithCachedValues());
+        mAdapter.injectContactInfoForTest(TEST_NUMBER, createContactInfo());
+
+        // Bind the views of a single row.
+        mAdapter.bindStandAloneView(mView, getContext(), mCursor);
+
+        // Cache and call log are up-to-date: no need to request update.
+        assertEquals(0, mAdapter.requests.size());
+    }
+
+    public void testBindView_MismatchBetwenCallLogAndMemoryCache_EnqueueRequest() {
+        mCursor.addRow(createCallLogEntryWithCachedValues());
+
+        // Contact info contains a different name.
+        ContactInfo info = createContactInfo();
+        info.name = "new name";
+        mAdapter.injectContactInfoForTest(TEST_NUMBER, info);
+
+        // Bind the views of a single row.
+        mAdapter.bindStandAloneView(mView, getContext(), mCursor);
+
+        // There is one request for contact details.
+        assertEquals(1, mAdapter.requests.size());
+
+        TestCallLogAdapter.Request request = mAdapter.requests.get(0);
+        // Since there is something in the cache, it is not an immediate request.
+        assertFalse("should not be immediate", request.immediate);
+    }
+
+    /** Returns a contact info with default values. */
+    private ContactInfo createContactInfo() {
+        ContactInfo info = new ContactInfo();
+        info.number = TEST_NUMBER;
+        info.name = TEST_NAME;
+        info.type = TEST_NUMBER_TYPE;
+        info.label = TEST_NUMBER_LABEL;
+        return info;
+    }
+
+    /** Returns a call log entry without cached values. */
+    private Object[] createCallLogEntry() {
+        Object[] values = CallLogQueryTestUtils.createTestExtendedValues();
+        values[CallLogQuery.NUMBER] = TEST_NUMBER;
+        return values;
+    }
+
+    /** Returns a call log entry with a cached values. */
+    private Object[] createCallLogEntryWithCachedValues() {
+        Object[] values = createCallLogEntry();
+        values[CallLogQuery.CACHED_NAME] = TEST_NAME;
+        values[CallLogQuery.CACHED_NUMBER_TYPE] = TEST_NUMBER_TYPE;
+        values[CallLogQuery.CACHED_NUMBER_LABEL] = TEST_NUMBER_LABEL;
+        return values;
+    }
+
+    /**
+     * Subclass of {@link CallLogAdapter} used in tests to intercept certain calls.
+     */
+    // TODO: This would be better done by splitting the contact lookup into a collaborator class
+    // instead.
+    private static final class TestCallLogAdapter extends CallLogAdapter {
+        public static class Request {
+            public final String number;
+            public final ContactInfo callLogInfo;
+            public final boolean immediate;
+
+            public Request(String number, ContactInfo callLogInfo, boolean immediate) {
+                this.number = number;
+                this.callLogInfo = callLogInfo;
+                this.immediate = immediate;
+            }
+        }
+
+        public final List<Request> requests = Lists.newArrayList();
+
+        public TestCallLogAdapter(Context context, CallFetcher callFetcher,
+                String currentCountryIso, String voicemailNumber) {
+            super(context, callFetcher, currentCountryIso, voicemailNumber);
+        }
+
+        @Override
+        void enqueueRequest(String number, ContactInfo callLogInfo, boolean immediate) {
+            requests.add(new Request(number, callLogInfo, immediate));
+        }
+    }
+}
diff --git a/tests/src/com/android/contacts/calllog/CallLogQueryTestUtils.java b/tests/src/com/android/contacts/calllog/CallLogQueryTestUtils.java
index 0e1952a..c273612 100644
--- a/tests/src/com/android/contacts/calllog/CallLogQueryTestUtils.java
+++ b/tests/src/com/android/contacts/calllog/CallLogQueryTestUtils.java
@@ -17,6 +17,9 @@
 package com.android.contacts.calllog;
 
 import static junit.framework.Assert.assertEquals;
+
+import android.provider.CallLog.Calls;
+
 import junit.framework.Assert;
 
 /**
@@ -24,13 +27,18 @@
  */
 public class CallLogQueryTestUtils {
     public static Object[] createTestValues() {
-        Object[] values = new Object[]{ -1L, "", 0L, 0L, 0, "", "", "", null, 0, null };
+        Object[] values = new Object[]{
+                -1L, "", 0L, 0L, Calls.INCOMING_TYPE, "", "", "", null, 0, null,
+        };
         assertEquals(CallLogQuery._PROJECTION.length, values.length);
         return values;
     }
 
     public static Object[] createTestExtendedValues() {
-        Object[] values = new Object[]{ -1L, "", 0L, 0L, 0, "", "", "", null, 0, null, 0 };
+        Object[] values = new Object[]{
+                -1L, "", 0L, 0L, Calls.INCOMING_TYPE, "", "", "", null, 0, null,
+                CallLogQuery.SECTION_OLD_ITEM
+        };
         Assert.assertEquals(CallLogQuery.EXTENDED_PROJECTION.length, values.length);
         return values;
     }