Merge "Enforce AnnotatedCallLog column constraints in its content provider."

4 files changed, 165 insertions, 10 deletions

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.
+   *
+   * <p>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}.
+   *
+   * <p>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<ContentValues> 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:
+   *
+   * <p>Do NOT modify/delete columns (e.g., adding constraints, changing column type, etc).
+   *
+   * <p>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").
+   *
+   * <p>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<Void> fill(CallLogMutations mutations) {
     if (!PermissionsUtil.hasReadPhoneStatePermissions(appContext)) {
+      for (Entry<Long, ContentValues> 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;