Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4ee24a94 authored by zachh's avatar zachh Committed by Copybara-Service
Browse files

Fixed bug in handling of empty numbers in new call log. 

Empty numbers were not being inserted into PhoneLookupHistory because the URI "content://.../PhoneLookupHistory/" is treated the same as "content://.../PhoneLookupHistory" (w/o the trailing slash). This caused the update (i.e. replace) operation to incorrectly update all rows in the table when it should have updated a single row.

The fix for this was to switch to a query parameter, so the empty number URI now looks like "content://.../PhoneLookupHistory?number="

Also improved some logging while debugging this problem.

Bug: 71866050
Test: unit and manual
PiperOrigin-RevId: 181659081
Change-Id: Idec4fb77e74920cd5485620b0a997db03aa8ff9b
parent 5dd30438

java/com/android/dialer/calllog/database/MutationApplier.java

+3 −3
Original line number Diff line number Diff line
@@ -68,7 +68,7 @@ public class MutationApplier { 


    if (!mutations.getInserts().isEmpty()) {

      LogUtil.i(

          "CallLogMutations.applyToDatabase", "inserting %d rows", mutations.getInserts().size());

          "MutationApplier.applyToDatabase", "inserting %d rows", mutations.getInserts().size());

      for (Entry<Long, ContentValues> entry : mutations.getInserts().entrySet()) {

        long id = entry.getKey();

        ContentValues contentValues = entry.getValue();
@@ -82,7 +82,7 @@ public class MutationApplier { 


    if (!mutations.getUpdates().isEmpty()) {

      LogUtil.i(

          "CallLogMutations.applyToDatabase", "updating %d rows", mutations.getUpdates().size());

          "MutationApplier.applyToDatabase", "updating %d rows", mutations.getUpdates().size());

      for (Entry<Long, ContentValues> entry : mutations.getUpdates().entrySet()) {

        long id = entry.getKey();

        ContentValues contentValues = entry.getValue();
@@ -96,7 +96,7 @@ public class MutationApplier { 


    if (!mutations.getDeletes().isEmpty()) {

      LogUtil.i(

          "CallLogMutations.applyToDatabase", "deleting %d rows", mutations.getDeletes().size());

          "MutationApplier.applyToDatabase", "deleting %d rows", mutations.getDeletes().size());

      String[] questionMarks = new String[mutations.getDeletes().size()];

      Arrays.fill(questionMarks, "?");

java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java

+6 −7
Original line number Diff line number Diff line
@@ -21,7 +21,6 @@ import android.content.ContentValues; 
import android.content.Context;

import android.content.OperationApplicationException;

import android.database.Cursor;

import android.net.Uri;

import android.os.RemoteException;

import android.support.annotation.MainThread;

import android.support.annotation.WorkerThread;
@@ -268,12 +267,16 @@ public final class PhoneLookupDataSource 
      contentValues.put(PhoneLookupHistory.PHONE_LOOKUP_INFO, phoneLookupInfo.toByteArray());

      contentValues.put(PhoneLookupHistory.LAST_MODIFIED, currentTimestamp);

      operations.add(

          ContentProviderOperation.newUpdate(numberUri(normalizedNumber))

          ContentProviderOperation.newUpdate(

                  PhoneLookupHistory.contentUriForNumber(normalizedNumber))

              .withValues(contentValues)

              .build());

    }

    for (String normalizedNumber : phoneLookupHistoryRowsToDelete) {

      operations.add(ContentProviderOperation.newDelete(numberUri(normalizedNumber)).build());

      operations.add(

          ContentProviderOperation.newDelete(

                  PhoneLookupHistory.contentUriForNumber(normalizedNumber))

              .build());

    }

    appContext.getContentResolver().applyBatch(PhoneLookupHistoryContract.AUTHORITY, operations);

    return null;
@@ -596,8 +599,4 @@ public final class PhoneLookupDataSource 
    contentValues.put(

        AnnotatedCallLog.CP2_INFO_INCOMPLETE, phoneLookupInfo.getCp2LocalInfo().getIsIncomplete());

  }



  private static Uri numberUri(String number) {

    return PhoneLookupHistory.CONTENT_URI.buildUpon().appendEncodedPath(number).build();

  }

}

java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java

+13 −1
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ import com.google.common.collect.Maps; 
import com.google.common.util.concurrent.Futures;

import com.google.common.util.concurrent.ListenableFuture;

import com.google.common.util.concurrent.ListeningExecutorService;

import com.google.common.util.concurrent.MoreExecutors;

import java.util.ArrayList;

import java.util.List;

import java.util.Map;
@@ -88,7 +89,18 @@ public final class CompositePhoneLookup implements PhoneLookup<PhoneLookupInfo> 
  public ListenableFuture<Boolean> isDirty(ImmutableSet<DialerPhoneNumber> phoneNumbers) {

    List<ListenableFuture<Boolean>> futures = new ArrayList<>();

    for (PhoneLookup<?> phoneLookup : phoneLookups) {

      futures.add(phoneLookup.isDirty(phoneNumbers));

      futures.add(

          Futures.transform(

              phoneLookup.isDirty(phoneNumbers),

              isDirty -> {

                LogUtil.v(

                    "CompositePhoneLookup.isDirty",

                    "isDirty for %s: %b",

                    phoneLookup.getClass().getSimpleName(),

                    isDirty);

                return isDirty;

              },

              MoreExecutors.directExecutor()));

    }

    // Executes all child lookups (possibly in parallel), completing when the first composite lookup

    // which returns "true" completes, and cancels the others.

java/com/android/dialer/phonelookup/database/PhoneLookupHistoryContentProvider.java

+56 −45
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import android.database.DatabaseUtils; 
import android.database.sqlite.SQLiteDatabase;

import android.database.sqlite.SQLiteQueryBuilder;

import android.net.Uri;

import android.support.annotation.IntDef;

import android.support.annotation.NonNull;

import android.support.annotation.Nullable;

import android.text.TextUtils;
@@ -34,7 +35,10 @@ import com.android.dialer.common.Assert; 
import com.android.dialer.common.LogUtil;

import com.android.dialer.phonelookup.database.contract.PhoneLookupHistoryContract;

import com.android.dialer.phonelookup.database.contract.PhoneLookupHistoryContract.PhoneLookupHistory;

import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.util.ArrayList;

import java.util.List;



/**

 * {@link ContentProvider} for the PhoneLookupHistory.
@@ -54,22 +58,17 @@ import java.util.ArrayList; 
 */

public class PhoneLookupHistoryContentProvider extends ContentProvider {



  /**

   * Can't use {@link UriMatcher} because it doesn't support empty values, and numbers can be empty.

   */

  @Retention(RetentionPolicy.SOURCE)

  @IntDef({UriType.PHONE_LOOKUP_HISTORY_TABLE_CODE, UriType.PHONE_LOOKUP_HISTORY_TABLE_ID_CODE})

  private @interface UriType {

    // For operations against: content://com.android.dialer.phonelookuphistory/PhoneLookupHistory

  private static final int PHONE_LOOKUP_HISTORY_TABLE_CODE = 1;

  // For operations against: content://com.android.dialer.phonelookuphistory/PhoneLookupHistory/+123

  private static final int PHONE_LOOKUP_HISTORY_TABLE_ID_CODE = 2;



  private static final UriMatcher uriMatcher = new UriMatcher(UriMatcher.NO_MATCH);



  static {

    uriMatcher.addURI(

        PhoneLookupHistoryContract.AUTHORITY,

        PhoneLookupHistory.TABLE,

        PHONE_LOOKUP_HISTORY_TABLE_CODE);

    uriMatcher.addURI(

        PhoneLookupHistoryContract.AUTHORITY,

        PhoneLookupHistory.TABLE + "/*", // The last path component should be a normalized number

        PHONE_LOOKUP_HISTORY_TABLE_ID_CODE);

    int PHONE_LOOKUP_HISTORY_TABLE_CODE = 1;

    // For operations against:

    // content://com.android.dialer.phonelookuphistory/PhoneLookupHistory?number=123

    int PHONE_LOOKUP_HISTORY_TABLE_ID_CODE = 2;

  }



  private PhoneLookupHistoryDatabaseHelper databaseHelper;
@@ -98,15 +97,16 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
    SQLiteDatabase db = databaseHelper.getReadableDatabase();

    SQLiteQueryBuilder queryBuilder = new SQLiteQueryBuilder();

    queryBuilder.setTables(PhoneLookupHistory.TABLE);

    int match = uriMatcher.match(uri);

    switch (match) {

      case PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

    @UriType int uriType = uriType(uri);

    switch (uriType) {

      case UriType.PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

        queryBuilder.appendWhere(

            PhoneLookupHistory.NORMALIZED_NUMBER

                + "="

                + DatabaseUtils.sqlEscapeString(uri.getLastPathSegment()));

                + DatabaseUtils.sqlEscapeString(

                    Uri.decode(uri.getQueryParameter(PhoneLookupHistory.NUMBER_QUERY_PARAM))));

        // fall through

      case PHONE_LOOKUP_HISTORY_TABLE_CODE:

      case UriType.PHONE_LOOKUP_HISTORY_TABLE_CODE:

        Cursor cursor =

            queryBuilder.query(db, projection, selection, selectionArgs, null, null, sortOrder);

        if (cursor == null) {
@@ -134,15 +134,16 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
    Assert.checkArgument(values != null);



    SQLiteDatabase database = databaseHelper.getWritableDatabase();

    int match = uriMatcher.match(uri);

    switch (match) {

      case PHONE_LOOKUP_HISTORY_TABLE_CODE:

    @UriType int uriType = uriType(uri);

    switch (uriType) {

      case UriType.PHONE_LOOKUP_HISTORY_TABLE_CODE:

        Assert.checkArgument(

            !TextUtils.isEmpty(values.getAsString(PhoneLookupHistory.NORMALIZED_NUMBER)),

            values.getAsString(PhoneLookupHistory.NORMALIZED_NUMBER) != null,

            "You must specify a normalized number when inserting");

        break;

      case PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

        String normalizedNumberFromUri = uri.getLastPathSegment();

      case UriType.PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

        String normalizedNumberFromUri =

            Uri.decode(uri.getQueryParameter(PhoneLookupHistory.NUMBER_QUERY_PARAM));

        String normalizedNumberFromValues =

            values.getAsString(PhoneLookupHistory.NORMALIZED_NUMBER);

        Assert.checkArgument(
@@ -168,10 +169,8 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
      return null;

    }

    Uri insertedUri =

        PhoneLookupHistory.CONTENT_URI

            .buildUpon()

            .appendEncodedPath(values.getAsString(PhoneLookupHistory.NORMALIZED_NUMBER))

            .build();

        PhoneLookupHistory.contentUriForNumber(

            values.getAsString(PhoneLookupHistory.NORMALIZED_NUMBER));

    if (!isApplyingBatch()) {

      notifyChange(insertedUri);

    }
@@ -182,15 +181,15 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
  public int delete(

      @NonNull Uri uri, @Nullable String selection, @Nullable String[] selectionArgs) {

    SQLiteDatabase database = databaseHelper.getWritableDatabase();

    final int match = uriMatcher.match(uri);

    switch (match) {

      case PHONE_LOOKUP_HISTORY_TABLE_CODE:

    @UriType int uriType = uriType(uri);

    switch (uriType) {

      case UriType.PHONE_LOOKUP_HISTORY_TABLE_CODE:

        break;

      case PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

      case UriType.PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

        Assert.checkArgument(selection == null, "Do not specify selection when deleting by number");

        Assert.checkArgument(

            selectionArgs == null, "Do not specify selection args when deleting by number");

        String number = uri.getLastPathSegment();

        String number = Uri.decode(uri.getQueryParameter(PhoneLookupHistory.NUMBER_QUERY_PARAM));

        Assert.checkArgument(!TextUtils.isEmpty(number), "error parsing number from uri: %s", uri);

        selection = PhoneLookupHistory.NORMALIZED_NUMBER + "= ?";

        selectionArgs = new String[] {number};
@@ -228,9 +227,9 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
    Assert.checkArgument(values != null);



    SQLiteDatabase database = databaseHelper.getWritableDatabase();

    int match = uriMatcher.match(uri);

    switch (match) {

      case PHONE_LOOKUP_HISTORY_TABLE_CODE:

    @UriType int uriType = uriType(uri);

    switch (uriType) {

      case UriType.PHONE_LOOKUP_HISTORY_TABLE_CODE:

        int rows = database.update(PhoneLookupHistory.TABLE, values, selection, selectionArgs);

        if (rows == 0) {

          LogUtil.w("PhoneLookupHistoryContentProvider.update", "no rows updated");
@@ -240,7 +239,7 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
          notifyChange(uri);

        }

        return rows;

      case PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

      case UriType.PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

        Assert.checkArgument(

            !values.containsKey(PhoneLookupHistory.NORMALIZED_NUMBER),

            "Do not specify number in values when updating by number");
@@ -248,7 +247,8 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
        Assert.checkArgument(

            selectionArgs == null, "Do not specify selection args when updating by ID");



        String normalizedNumber = uri.getLastPathSegment();

        String normalizedNumber =

            Uri.decode(uri.getQueryParameter(PhoneLookupHistory.NUMBER_QUERY_PARAM));

        values.put(PhoneLookupHistory.NORMALIZED_NUMBER, normalizedNumber);

        long result = database.replace(PhoneLookupHistory.TABLE, null, values);

        Assert.checkArgument(result != -1, "replacing PhoneLookupHistory row failed");
@@ -282,10 +282,10 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
      database.beginTransaction();

      for (int i = 0; i < operations.size(); i++) {

        ContentProviderOperation operation = operations.get(i);

        int match = uriMatcher.match(operation.getUri());

        switch (match) {

          case PHONE_LOOKUP_HISTORY_TABLE_CODE:

          case PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

        @UriType int uriType = uriType(operation.getUri());

        switch (uriType) {

          case UriType.PHONE_LOOKUP_HISTORY_TABLE_CODE:

          case UriType.PHONE_LOOKUP_HISTORY_TABLE_ID_CODE:

            ContentProviderResult result = operation.apply(this, results, i);

            if (operation.isInsert()) {

              if (result.uri == null) {
@@ -312,4 +312,15 @@ public class PhoneLookupHistoryContentProvider extends ContentProvider { 
  private void notifyChange(Uri uri) {

    getContext().getContentResolver().notifyChange(uri, null);

  }



  @UriType

  private int uriType(Uri uri) {

    Assert.checkArgument(uri.getAuthority().equals(PhoneLookupHistoryContract.AUTHORITY));

    List<String> pathSegments = uri.getPathSegments();

    Assert.checkArgument(pathSegments.size() == 1);

    Assert.checkArgument(pathSegments.get(0).equals(PhoneLookupHistory.TABLE));

    return uri.getQueryParameter(PhoneLookupHistory.NUMBER_QUERY_PARAM) == null

        ? UriType.PHONE_LOOKUP_HISTORY_TABLE_CODE

        : UriType.PHONE_LOOKUP_HISTORY_TABLE_ID_CODE;

  }

}

java/com/android/dialer/phonelookup/database/contract/PhoneLookupHistoryContract.java

+10 −0
Original line number Diff line number Diff line
@@ -30,10 +30,20 @@ public class PhoneLookupHistoryContract { 


    public static final String TABLE = "PhoneLookupHistory";



    public static final String NUMBER_QUERY_PARAM = "number";



    /** The content URI for this table. */

    public static final Uri CONTENT_URI =

        Uri.withAppendedPath(PhoneLookupHistoryContract.CONTENT_URI, TABLE);



    /** Returns a URI for a specific normalized number */

    public static Uri contentUriForNumber(String normalizedNumber) {

      return CONTENT_URI

          .buildUpon()

          .appendQueryParameter(NUMBER_QUERY_PARAM, Uri.encode(normalizedNumber))

          .build();

    }



    /** The MIME type of a {@link android.content.ContentProvider#getType(Uri)} single entry. */

    public static final String CONTENT_ITEM_TYPE = "vnd.android.cursor.item/phone_lookup_history";