summaryrefslogtreecommitdiff
path: root/java
diff options
context:
space:
mode:
authorzachh <zachh@google.com>2018-01-11 14:05:10 -0800
committerCopybara-Service <copybara-piper@google.com>2018-01-11 14:23:40 -0800
commit4ee24a9492c7c83952f59ecc54071c31aa68fa86 (patch)
tree88259dfd2edf1eefdbb367cd93bc7dfd6a7db548 /java
parent5dd30438fd3e4384b57cef3c7606ec20fad9b50d (diff)
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
Diffstat (limited to 'java')
-rw-r--r--java/com/android/dialer/calllog/database/MutationApplier.java6
-rw-r--r--java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java13
-rw-r--r--java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java14
-rw-r--r--java/com/android/dialer/phonelookup/database/PhoneLookupHistoryContentProvider.java101
-rw-r--r--java/com/android/dialer/phonelookup/database/contract/PhoneLookupHistoryContract.java10
-rw-r--r--java/com/android/incallui/PhoneLookupHistoryRecorder.java5
6 files changed, 89 insertions, 60 deletions
diff --git a/java/com/android/dialer/calllog/database/MutationApplier.java b/java/com/android/dialer/calllog/database/MutationApplier.java
index eee810eb8..2fb52558e 100644
--- a/java/com/android/dialer/calllog/database/MutationApplier.java
+++ b/java/com/android/dialer/calllog/database/MutationApplier.java
@@ -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, "?");
diff --git a/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java b/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java
index dca992789..30461a4be 100644
--- a/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java
+++ b/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java
@@ -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();
- }
}
diff --git a/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java b/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java
index 34f35311f..68695b7e9 100644
--- a/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java
+++ b/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java
@@ -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.
diff --git a/java/com/android/dialer/phonelookup/database/PhoneLookupHistoryContentProvider.java b/java/com/android/dialer/phonelookup/database/PhoneLookupHistoryContentProvider.java
index 5c0c00f81..e20e81369 100644
--- a/java/com/android/dialer/phonelookup/database/PhoneLookupHistoryContentProvider.java
+++ b/java/com/android/dialer/phonelookup/database/PhoneLookupHistoryContentProvider.java
@@ -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 {
- // 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);
+ /**
+ * 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
+ 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;
+ }
}
diff --git a/java/com/android/dialer/phonelookup/database/contract/PhoneLookupHistoryContract.java b/java/com/android/dialer/phonelookup/database/contract/PhoneLookupHistoryContract.java
index f8e108496..a195926ac 100644
--- a/java/com/android/dialer/phonelookup/database/contract/PhoneLookupHistoryContract.java
+++ b/java/com/android/dialer/phonelookup/database/contract/PhoneLookupHistoryContract.java
@@ -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";
diff --git a/java/com/android/incallui/PhoneLookupHistoryRecorder.java b/java/com/android/incallui/PhoneLookupHistoryRecorder.java
index 667c0d1cc..8517deb65 100644
--- a/java/com/android/incallui/PhoneLookupHistoryRecorder.java
+++ b/java/com/android/incallui/PhoneLookupHistoryRecorder.java
@@ -66,10 +66,7 @@ final class PhoneLookupHistoryRecorder {
appContext
.getContentResolver()
.update(
- PhoneLookupHistory.CONTENT_URI
- .buildUpon()
- .appendEncodedPath(normalizedNumber.get())
- .build(),
+ PhoneLookupHistory.contentUriForNumber(normalizedNumber.get()),
contentValues,
null,
null);