From 5de80a5c9a25f8f952c3c1309ae6e5b77f0d7974 Mon Sep 17 00:00:00 2001 From: zachh Date: Tue, 23 Jan 2018 15:56:00 -0800 Subject: Don't garbage collect voicemails in AnnotatedCallLog. We currently limit the size of AnnotatedCallLog to 999 via a trigger, but it doesn't exclude voicemails. Since we don't want to ever garbage collect the user's voicemails, exclude them from the trigger. This means that we can no longer assume a maximum size for the table (the user culd have more than 999 voicemails) so I've updated the places in the code where we did that before. Finally, I changed AnnotatedCallLog's CALL_TYPE column to be non-null. This is so that we can have more confidence that the trigger will work as intended. Null values cannot be compared in SQLite, so an expression like "where call_type != 4" won't actually match a null call type. Rather than implicitly fail to clean up such rows, we just crash completely when encountering such rows (even though I don't expect that to happen). Bug: 70989634 Test: unit PiperOrigin-RevId: 183006714 Change-Id: I9f4394a4812afe4b65c1e8427c355d825356557c --- .../database/AnnotatedCallLogDatabaseHelper.java | 23 ++++++-- .../dialer/calllog/database/MutationApplier.java | 33 ++++++----- .../systemcalllog/SystemCallLogDataSource.java | 68 +++++++++++++--------- 3 files changed, 78 insertions(+), 46 deletions(-) (limited to 'java/com/android/dialer/calllog') diff --git a/java/com/android/dialer/calllog/database/AnnotatedCallLogDatabaseHelper.java b/java/com/android/dialer/calllog/database/AnnotatedCallLogDatabaseHelper.java index da93ff8a3..eed77ebed 100644 --- a/java/com/android/dialer/calllog/database/AnnotatedCallLogDatabaseHelper.java +++ b/java/com/android/dialer/calllog/database/AnnotatedCallLogDatabaseHelper.java @@ -19,6 +19,7 @@ package com.android.dialer.calllog.database; import android.content.Context; import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteOpenHelper; +import android.provider.CallLog.Calls; import com.android.dialer.calllog.database.contract.AnnotatedCallLogContract.AnnotatedCallLog; import com.android.dialer.common.LogUtil; import java.util.Locale; @@ -52,17 +53,23 @@ class AnnotatedCallLogDatabaseHelper extends SQLiteOpenHelper { + (AnnotatedCallLog.FEATURES + " integer, ") + (AnnotatedCallLog.TRANSCRIPTION + " integer, ") + (AnnotatedCallLog.VOICEMAIL_URI + " text, ") - + (AnnotatedCallLog.CALL_TYPE + " integer, ") + + (AnnotatedCallLog.CALL_TYPE + " integer not null, ") + (AnnotatedCallLog.NUMBER_ATTRIBUTES + " blob ") + ");"; - /** Deletes all but the first maxRows rows (by timestamp) to keep the table a manageable size. */ - // TODO(zachh): Prevent voicemails from being garbage collected. + /** + * Deletes all but the first maxRows rows (by timestamp, excluding voicemails) to keep the table a + * manageable size. + */ private static final String CREATE_TRIGGER_SQL = "create trigger delete_old_rows after insert on " + AnnotatedCallLog.TABLE + " when (select count(*) from " + AnnotatedCallLog.TABLE + + " where " + + AnnotatedCallLog.CALL_TYPE + + " != " + + Calls.VOICEMAIL_TYPE + ") > %d" + " begin delete from " + AnnotatedCallLog.TABLE @@ -72,10 +79,18 @@ class AnnotatedCallLogDatabaseHelper extends SQLiteOpenHelper { + AnnotatedCallLog._ID + " from " + AnnotatedCallLog.TABLE + + " where " + + AnnotatedCallLog.CALL_TYPE + + " != " + + Calls.VOICEMAIL_TYPE + " order by timestamp limit (select count(*)-%d" + " from " + AnnotatedCallLog.TABLE - + " )); end;"; + + " where " + + AnnotatedCallLog.CALL_TYPE + + " != " + + Calls.VOICEMAIL_TYPE + + ")); end;"; private static final String CREATE_INDEX_ON_CALL_TYPE_SQL = "create index call_type_index on " diff --git a/java/com/android/dialer/calllog/database/MutationApplier.java b/java/com/android/dialer/calllog/database/MutationApplier.java index 2fb52558e..c9edd3ee1 100644 --- a/java/com/android/dialer/calllog/database/MutationApplier.java +++ b/java/com/android/dialer/calllog/database/MutationApplier.java @@ -29,11 +29,13 @@ import com.android.dialer.calllog.datasources.CallLogMutations; import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; import com.android.dialer.common.concurrent.Annotations.BackgroundExecutor; +import com.google.common.collect.Iterables; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Map.Entry; import javax.inject.Inject; @@ -97,22 +99,27 @@ public class MutationApplier { if (!mutations.getDeletes().isEmpty()) { LogUtil.i( "MutationApplier.applyToDatabase", "deleting %d rows", mutations.getDeletes().size()); - String[] questionMarks = new String[mutations.getDeletes().size()]; - Arrays.fill(questionMarks, "?"); - String whereClause = - (AnnotatedCallLog._ID + " in (") + TextUtils.join(",", questionMarks) + ")"; + // Batch the deletes into chunks of 999, the maximum size for SQLite selection args. + Iterable> batches = Iterables.partition(mutations.getDeletes(), 999); + for (List idsInBatch : batches) { + String[] questionMarks = new String[idsInBatch.size()]; + Arrays.fill(questionMarks, "?"); - String[] whereArgs = new String[mutations.getDeletes().size()]; - int i = 0; - for (long id : mutations.getDeletes()) { - whereArgs[i++] = String.valueOf(id); - } + String whereClause = + (AnnotatedCallLog._ID + " in (") + TextUtils.join(",", questionMarks) + ")"; + + String[] whereArgs = new String[idsInBatch.size()]; + int i = 0; + for (long id : idsInBatch) { + whereArgs[i++] = String.valueOf(id); + } - operations.add( - ContentProviderOperation.newDelete(AnnotatedCallLog.CONTENT_URI) - .withSelection(whereClause, whereArgs) - .build()); + operations.add( + ContentProviderOperation.newDelete(AnnotatedCallLog.CONTENT_URI) + .withSelection(whereClause, whereArgs) + .build()); + } } appContext.getContentResolver().applyBatch(AnnotatedCallLogContract.AUTHORITY, operations); diff --git a/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java b/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java index ee169e1d2..14cde46dd 100644 --- a/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java +++ b/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java @@ -53,6 +53,7 @@ import com.android.dialer.phonenumberproto.DialerPhoneNumberUtil; import com.android.dialer.storage.StorageComponent; import com.android.dialer.theme.R; import com.android.dialer.util.PermissionsUtil; +import com.google.common.collect.Iterables; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.i18n.phonenumbers.PhoneNumberUtil; @@ -293,7 +294,11 @@ public class SystemCallLogDataSource implements CallLogDataSource { long id = cursor.getLong(idColumn); long date = cursor.getLong(dateColumn); String numberAsStr = cursor.getString(numberColumn); - long type = cursor.getInt(typeColumn); + long type; + if (cursor.isNull(typeColumn) || (type = cursor.getInt(typeColumn)) == 0) { + // CallLog.Calls#TYPE lists the allowed values, which are non-null and non-zero. + throw new IllegalStateException("call type is missing"); + } String countryIso = cursor.getString(countryIsoColumn); int duration = cursor.getInt(durationsColumn); int dataUsage = cursor.getInt(dataUsageColumn); @@ -434,38 +439,43 @@ public class SystemCallLogDataSource implements CallLogDataSource { Context appContext, Set matchingIds) { ArraySet ids = new ArraySet<>(); - String[] questionMarks = new String[matchingIds.size()]; - Arrays.fill(questionMarks, "?"); - String whereClause = (Calls._ID + " in (") + TextUtils.join(",", questionMarks) + ")"; - String[] whereArgs = new String[matchingIds.size()]; - int i = 0; - for (long id : matchingIds) { - whereArgs[i++] = String.valueOf(id); - } - - try (Cursor cursor = - appContext - .getContentResolver() - .query( - Calls.CONTENT_URI_WITH_VOICEMAIL, - new String[] {Calls._ID}, - whereClause, - whereArgs, - null)) { - - if (cursor == null) { - LogUtil.e("SystemCallLogDataSource.getIdsFromSystemCallLog", "null cursor"); - return ids; + // Batch the select statements into chunks of 999, the maximum size for SQLite selection args. + Iterable> batches = Iterables.partition(matchingIds, 999); + for (List idsInBatch : batches) { + String[] questionMarks = new String[idsInBatch.size()]; + Arrays.fill(questionMarks, "?"); + + String whereClause = (Calls._ID + " in (") + TextUtils.join(",", questionMarks) + ")"; + String[] whereArgs = new String[idsInBatch.size()]; + int i = 0; + for (long id : idsInBatch) { + whereArgs[i++] = String.valueOf(id); } - if (cursor.moveToFirst()) { - int idColumn = cursor.getColumnIndexOrThrow(Calls._ID); - do { - ids.add(cursor.getLong(idColumn)); - } while (cursor.moveToNext()); + try (Cursor cursor = + appContext + .getContentResolver() + .query( + Calls.CONTENT_URI_WITH_VOICEMAIL, + new String[] {Calls._ID}, + whereClause, + whereArgs, + null)) { + + if (cursor == null) { + LogUtil.e("SystemCallLogDataSource.getIdsFromSystemCallLog", "null cursor"); + return ids; + } + + if (cursor.moveToFirst()) { + int idColumn = cursor.getColumnIndexOrThrow(Calls._ID); + do { + ids.add(cursor.getLong(idColumn)); + } while (cursor.moveToNext()); + } } - return ids; } + return ids; } private static class CallLogObserver extends ContentObserver { -- cgit v1.2.3