From 8736bcaf81f4cccec66ebac078d3baaff4a860d0 Mon Sep 17 00:00:00 2001 From: linyuh Date: Wed, 20 Jun 2018 17:11:19 -0700 Subject: Enforce AnnotatedCallLog column constraints in its content provider. Bug: 110185399 Test: AnnotatedCallLogConstraintsTest, AnnotatedCallLogContentProviderTest PiperOrigin-RevId: 201444134 Change-Id: I105ec7a201265ee5e7708ffb9a4b66b4cef01174 --- .../database/AnnotatedCallLogConstraints.java | 117 +++++++++++++++++++++ .../database/AnnotatedCallLogContentProvider.java | 7 +- .../database/AnnotatedCallLogDatabaseHelper.java | 45 ++++++-- .../datasources/voicemail/VoicemailDataSource.java | 6 ++ 4 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 java/com/android/dialer/calllog/database/AnnotatedCallLogConstraints.java diff --git a/java/com/android/dialer/calllog/database/AnnotatedCallLogConstraints.java b/java/com/android/dialer/calllog/database/AnnotatedCallLogConstraints.java new file mode 100644 index 000000000..3dc85147b --- /dev/null +++ b/java/com/android/dialer/calllog/database/AnnotatedCallLogConstraints.java @@ -0,0 +1,117 @@ +/* + * Copyright (C) 2018 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.dialer.calllog.database; + +import android.content.ContentValues; +import android.provider.CallLog.Calls; +import android.support.annotation.IntDef; +import com.android.dialer.calllog.database.contract.AnnotatedCallLogContract.AnnotatedCallLog; +import com.android.dialer.common.Assert; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.util.function.Predicate; + +/** Constraints for columns in the {@link AnnotatedCallLog}. */ +final class AnnotatedCallLogConstraints { + + /** Type of operation the {@link ContentValues} to be checked is used for. */ + @Retention(RetentionPolicy.SOURCE) + @IntDef({Operation.INSERT, Operation.UPDATE}) + @interface Operation { + int INSERT = 1; + int UPDATE = 2; + } + + private AnnotatedCallLogConstraints() {} + + /** + * Checks if the given {@link ContentValues} meets the constraints defined in this class. An + * {@link IllegalArgumentException} will be thrown if it doesn't. + */ + public static void check(ContentValues contentValues, @Operation int operationType) { + checkBooleanColumn(AnnotatedCallLog.IS_READ, contentValues, operationType); + checkBooleanColumn(AnnotatedCallLog.NEW, contentValues, operationType); + checkBooleanColumn(AnnotatedCallLog.IS_VOICEMAIL_CALL, contentValues, operationType); + checkCallTypeColumn(contentValues, operationType); + } + + /** + * Checks a boolean column. + * + *

Constraints: the value must be either 0 or 1 (SQLite database has no boolean type, so the + * value has to be an integer). + */ + private static void checkBooleanColumn( + String columnName, ContentValues contentValues, @Operation int operationType) { + checkColumn( + columnName, + contentValues, + operationType, + contentValuesToCheck -> { + Integer value = contentValuesToCheck.getAsInteger(columnName); + return value != null && (value == 0 || value == 1); + }); + } + + /** + * Checks column {@link AnnotatedCallLog#CALL_TYPE}. + * + *

Constraints: the value must be one of {@link android.provider.CallLog.Calls#TYPE}. + */ + private static void checkCallTypeColumn( + ContentValues contentValues, @Operation int operationType) { + checkColumn( + AnnotatedCallLog.CALL_TYPE, + contentValues, + operationType, + contentValuesToCheck -> { + Integer callType = contentValuesToCheck.getAsInteger(AnnotatedCallLog.CALL_TYPE); + return callType != null + && (callType == Calls.INCOMING_TYPE + || callType == Calls.OUTGOING_TYPE + || callType == Calls.MISSED_TYPE + || callType == Calls.VOICEMAIL_TYPE + || callType == Calls.REJECTED_TYPE + || callType == Calls.BLOCKED_TYPE + || callType == Calls.ANSWERED_EXTERNALLY_TYPE); + }); + } + + private static void checkColumn( + String columnName, + ContentValues contentValues, + @Operation int operationType, + Predicate predicate) { + switch (operationType) { + case Operation.UPDATE: + if (!contentValues.containsKey(columnName)) { + return; + } + // fall through + case Operation.INSERT: + Assert.checkArgument( + predicate.test(contentValues), + "Column %s contains invalid value: %s", + columnName, + contentValues.get(columnName)); + return; + default: + throw Assert.createUnsupportedOperationFailException( + String.format("Unsupported operation: %s", operationType)); + } + } +} diff --git a/java/com/android/dialer/calllog/database/AnnotatedCallLogContentProvider.java b/java/com/android/dialer/calllog/database/AnnotatedCallLogContentProvider.java index 3ca76ee23..36fa5ff26 100644 --- a/java/com/android/dialer/calllog/database/AnnotatedCallLogContentProvider.java +++ b/java/com/android/dialer/calllog/database/AnnotatedCallLogContentProvider.java @@ -31,6 +31,7 @@ import android.net.Uri; import android.os.Build; import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import com.android.dialer.calllog.database.AnnotatedCallLogConstraints.Operation; import com.android.dialer.calllog.database.contract.AnnotatedCallLogContract; import com.android.dialer.calllog.database.contract.AnnotatedCallLogContract.AnnotatedCallLog; import com.android.dialer.common.Assert; @@ -41,8 +42,6 @@ import java.util.Arrays; /** {@link ContentProvider} for the annotated call log. */ public class AnnotatedCallLogContentProvider extends ContentProvider { - - private static final int ANNOTATED_CALL_LOG_TABLE_CODE = 1; private static final int ANNOTATED_CALL_LOG_TABLE_ID_CODE = 2; private static final int ANNOTATED_CALL_LOG_TABLE_DISTINCT_NUMBER_CODE = 3; @@ -150,6 +149,8 @@ public class AnnotatedCallLogContentProvider extends ContentProvider { // Javadoc states values is not nullable, even though it is annotated as such (a bug)! Assert.checkArgument(values != null); + AnnotatedCallLogConstraints.check(values, Operation.INSERT); + SQLiteDatabase database = databaseHelper.getWritableDatabase(); int match = uriMatcher.match(uri); switch (match) { @@ -230,6 +231,8 @@ public class AnnotatedCallLogContentProvider extends ContentProvider { // Javadoc states values is not nullable, even though it is annotated as such (a bug)! Assert.checkArgument(values != null); + AnnotatedCallLogConstraints.check(values, Operation.UPDATE); + SQLiteDatabase database = databaseHelper.getWritableDatabase(); int match = uriMatcher.match(uri); switch (match) { diff --git a/java/com/android/dialer/calllog/database/AnnotatedCallLogDatabaseHelper.java b/java/com/android/dialer/calllog/database/AnnotatedCallLogDatabaseHelper.java index 8fc80a62d..0693c7980 100644 --- a/java/com/android/dialer/calllog/database/AnnotatedCallLogDatabaseHelper.java +++ b/java/com/android/dialer/calllog/database/AnnotatedCallLogDatabaseHelper.java @@ -35,7 +35,7 @@ import javax.inject.Singleton; @Singleton public class AnnotatedCallLogDatabaseHelper extends SQLiteOpenHelper { - @VisibleForTesting static final int VERSION = 3; + @VisibleForTesting static final int VERSION = 4; private static final String FILENAME = "annotated_call_log.db"; @@ -55,6 +55,18 @@ public class AnnotatedCallLogDatabaseHelper extends SQLiteOpenHelper { this.backgroundExecutor = backgroundExecutor; } + /** + * Important note: + * + *

Do NOT modify/delete columns (e.g., adding constraints, changing column type, etc). + * + *

As SQLite's "ALTER TABLE" statement doesn't support such operations, doing so requires + * complex, expensive, and error-prone operations to upgrade the database (see + * https://www.sqlite.org/lang_altertable.html "Making Other Kinds Of Table Schema Changes"). + * + *

All column constraints are enforced when data are inserted/updated via + * AnnotatedCallLogContentProvider. See AnnotatedCallLogConstraints for details. + */ private static final String CREATE_TABLE_SQL = "create table if not exists " + AnnotatedCallLog.TABLE @@ -66,17 +78,17 @@ public class AnnotatedCallLogDatabaseHelper extends SQLiteOpenHelper { + (AnnotatedCallLog.NUMBER_PRESENTATION + " integer, ") + (AnnotatedCallLog.DURATION + " integer, ") + (AnnotatedCallLog.DATA_USAGE + " integer, ") - + (AnnotatedCallLog.IS_READ + " integer not null, ") - + (AnnotatedCallLog.NEW + " integer not null, ") + + (AnnotatedCallLog.IS_READ + " integer, ") + + (AnnotatedCallLog.NEW + " integer, ") + (AnnotatedCallLog.GEOCODED_LOCATION + " text, ") + (AnnotatedCallLog.PHONE_ACCOUNT_COMPONENT_NAME + " text, ") + (AnnotatedCallLog.PHONE_ACCOUNT_ID + " text, ") + (AnnotatedCallLog.FEATURES + " integer, ") + (AnnotatedCallLog.TRANSCRIPTION + " integer, ") + (AnnotatedCallLog.VOICEMAIL_URI + " text, ") - + (AnnotatedCallLog.CALL_TYPE + " integer not null, ") + + (AnnotatedCallLog.CALL_TYPE + " integer, ") + (AnnotatedCallLog.NUMBER_ATTRIBUTES + " blob, ") - + (AnnotatedCallLog.IS_VOICEMAIL_CALL + " integer default 0, ") + + (AnnotatedCallLog.IS_VOICEMAIL_CALL + " integer, ") + (AnnotatedCallLog.VOICEMAIL_CALL_TAG + " text, ") + (AnnotatedCallLog.TRANSCRIPTION_STATE + " integer, ") + (AnnotatedCallLog.CALL_MAPPING_ID + " text") @@ -151,8 +163,11 @@ public class AnnotatedCallLogDatabaseHelper extends SQLiteOpenHelper { if (oldVersion < 2) { upgradeToV2(db); } - if (oldVersion < 3) { - upgradeToV3(db); + + // Version 3 upgrade was buggy and didn't make any schema changes. + // So we go directly to version 4. + if (oldVersion < 4) { + upgradeToV4(db); } } @@ -172,7 +187,21 @@ public class AnnotatedCallLogDatabaseHelper extends SQLiteOpenHelper { + AnnotatedCallLog.TIMESTAMP); } - private static void upgradeToV3(SQLiteDatabase db) { + private static void upgradeToV4(SQLiteDatabase db) { + // Starting from v4, we will enforce column constraints in the AnnotatedCallLogContentProvider + // instead of on the database level. + // The constraints are as follows (see AnnotatedCallLogConstraints for details). + // IS_READ: not null, must be 0 or 1; + // NEW: not null, must be 0 or 1; + // IS_VOICEMAIL_CALL: not null, must be 0 or 1; and + // CALL_TYPE: not null, must be one of android.provider.CallLog.Calls#TYPE. + // + // There is no need to update the old schema as the constraints above are more strict than + // those in the old schema. + // + // Version 3 schema defaulted column IS_VOICEMAIL_CALL to 0 but we didn't update the schema in + // onUpgrade. As a result, null values can still be inserted if the user has an older version of + // the database. For version 4, we need to set all null values to 0. db.execSQL( "update " + AnnotatedCallLog.TABLE diff --git a/java/com/android/dialer/calllog/datasources/voicemail/VoicemailDataSource.java b/java/com/android/dialer/calllog/datasources/voicemail/VoicemailDataSource.java index cbda9ac81..8d9cae224 100644 --- a/java/com/android/dialer/calllog/datasources/voicemail/VoicemailDataSource.java +++ b/java/com/android/dialer/calllog/datasources/voicemail/VoicemailDataSource.java @@ -63,8 +63,12 @@ public class VoicemailDataSource implements CallLogDataSource { @SuppressWarnings("missingPermission") public ListenableFuture fill(CallLogMutations mutations) { if (!PermissionsUtil.hasReadPhoneStatePermissions(appContext)) { + for (Entry insert : mutations.getInserts().entrySet()) { + insert.getValue().put(AnnotatedCallLog.IS_VOICEMAIL_CALL, 0); + } return Futures.immediateFuture(null); } + return backgroundExecutor.submit( () -> { TelecomManager telecomManager = appContext.getSystemService(TelecomManager.class); @@ -90,6 +94,8 @@ public class VoicemailDataSource implements CallLogDataSource { appContext, phoneAccountHandle); values.put( AnnotatedCallLog.VOICEMAIL_CALL_TAG, telephonyManager.getVoiceMailAlphaTag()); + } else { + values.put(AnnotatedCallLog.IS_VOICEMAIL_CALL, 0); } } return null; -- cgit v1.2.3