From f20b6a9282e8e9812533f183330c20bad54315cb Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Fri, 28 Jun 2019 17:07:38 -0700 Subject: [PATCH 01/12] Added tests for upgrade --- .../persistence/SpyEventStoreModule.java | 48 ++++-- .../persistence/TestEventStoreModule.java | 48 ++++-- .../runtime/TransportContext.java | 6 + .../persistence/DatabaseBootstrapClient.java | 19 ++- .../persistence/DatabaseMigrationClient.java | 35 +++++ .../persistence/EventStoreModule.java | 76 ++++++++-- .../persistence/SQLiteEventStore.java | 3 +- .../scheduling/persistence/SchemaManager.java | 17 ++- .../scheduling/persistence/LegacySQL.java | 55 +++++++ .../persistence/SQLiteEventStoreTest.java | 15 +- .../persistence/SchemaManagerTest.java | 138 +++++++++++++++++- 11 files changed, 409 insertions(+), 51 deletions(-) create mode 100644 transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseMigrationClient.java create mode 100644 transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/LegacySQL.java diff --git a/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/SpyEventStoreModule.java b/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/SpyEventStoreModule.java index c6a2828bc3e..cbb5412a423 100644 --- a/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/SpyEventStoreModule.java +++ b/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/SpyEventStoreModule.java @@ -14,17 +14,22 @@ package com.google.android.datatransport.runtime.scheduling.persistence; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V1; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_CONTEXTS_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENTS_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENT_METADATA_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.MIGRATE_TO_V2; import static org.mockito.Mockito.spy; import com.google.android.datatransport.runtime.synchronization.SynchronizationGuard; import dagger.Binds; import dagger.Module; import dagger.Provides; +import java.util.Collections; import javax.inject.Named; import javax.inject.Singleton; @@ -47,30 +52,53 @@ static EventStore eventStore(SQLiteEventStore store) { @Provides @Named("CREATE_EVENTS_SQL") static String createEventsSql() { - return CREATE_EVENTS_SQL_V1; + return CREATE_EVENTS_SQL_V2; } @Provides @Named("CREATE_EVENT_METADATA_SQL") static String createEventMetadataSql() { - return CREATE_EVENT_METADATA_SQL_V1; + return CREATE_EVENT_METADATA_SQL_V2; } @Provides @Named("CREATE_CONTEXTS_SQL") static String createContextsSql() { - return CREATE_CONTEXTS_SQL_V1; + return CREATE_CONTEXTS_SQL_V2; } @Provides @Named("CREATE_EVENT_BACKEND_INDEX") static String getCreateEventBackendIndex() { - return CREATE_EVENT_BACKEND_INDEX_V1; + return CREATE_EVENT_BACKEND_INDEX_V2; } @Provides @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") static String createEventBackendPriorityIndex() { - return CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1; + return CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; + } + + @Provides + @Named("DROP_EVENTS_SQL") + static String dropEventsSQL() { + return DROP_EVENTS_SQL; + } + + @Provides + @Named("DROP_EVENT_METADATA_SQL") + static String dropEventMetadataSql() { + return DROP_EVENT_METADATA_SQL; + } + + @Provides + @Named("DROP_CONTEXTS_SQL") + static String dropContextsSql() { + return DROP_CONTEXTS_SQL; + } + + @Provides + static DatabaseMigrationClient createDatabaseMigrationClient() { + return new DatabaseMigrationClient(Collections.singletonList(MIGRATE_TO_V2)); } } diff --git a/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/TestEventStoreModule.java b/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/TestEventStoreModule.java index 77c1a819a25..f4ebf039cc1 100644 --- a/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/TestEventStoreModule.java +++ b/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/TestEventStoreModule.java @@ -14,16 +14,21 @@ package com.google.android.datatransport.runtime.scheduling.persistence; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V1; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_CONTEXTS_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENTS_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENT_METADATA_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.MIGRATE_TO_V2; import com.google.android.datatransport.runtime.synchronization.SynchronizationGuard; import dagger.Binds; import dagger.Module; import dagger.Provides; +import java.util.Collections; import javax.inject.Named; @Module @@ -51,30 +56,53 @@ static EventStoreConfig storeConfig() { @Provides @Named("CREATE_EVENTS_SQL") static String createEventsSql() { - return CREATE_EVENTS_SQL_V1; + return CREATE_EVENTS_SQL_V2; } @Provides @Named("CREATE_EVENT_METADATA_SQL") static String createEventMetadataSql() { - return CREATE_EVENT_METADATA_SQL_V1; + return CREATE_EVENT_METADATA_SQL_V2; } @Provides @Named("CREATE_CONTEXTS_SQL") static String createContextsSql() { - return CREATE_CONTEXTS_SQL_V1; + return CREATE_CONTEXTS_SQL_V2; } @Provides @Named("CREATE_EVENT_BACKEND_INDEX") static String getCreateEventBackendIndex() { - return CREATE_EVENT_BACKEND_INDEX_V1; + return CREATE_EVENT_BACKEND_INDEX_V2; } @Provides @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") static String createEventBackendPriorityIndex() { - return CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1; + return CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; + } + + @Provides + @Named("DROP_EVENTS_SQL") + static String dropEventsSQL() { + return DROP_EVENTS_SQL; + } + + @Provides + @Named("DROP_EVENT_METADATA_SQL") + static String dropEventMetadataSql() { + return DROP_EVENT_METADATA_SQL; + } + + @Provides + @Named("DROP_CONTEXTS_SQL") + static String dropContextsSql() { + return DROP_CONTEXTS_SQL; + } + + @Provides + static DatabaseMigrationClient createDatabaseMigrationClient() { + return new DatabaseMigrationClient(Collections.singletonList(MIGRATE_TO_V2)); } } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/TransportContext.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/TransportContext.java index b82d9899c67..46353f6edc4 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/TransportContext.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/TransportContext.java @@ -14,6 +14,7 @@ package com.google.android.datatransport.runtime; +import androidx.annotation.Nullable; import androidx.annotation.RestrictTo; import com.google.android.datatransport.Priority; import com.google.auto.value.AutoValue; @@ -26,6 +27,9 @@ public abstract class TransportContext { /** Backend events are sent to. */ public abstract String getBackendName(); + @Nullable + public abstract byte[] getExtras(); + /** * Priority of the event. * @@ -58,6 +62,8 @@ public abstract static class Builder { public abstract Builder setBackendName(String name); + public abstract Builder setExtras(@Nullable byte[] extras); + /** @hide */ @RestrictTo(RestrictTo.Scope.LIBRARY) public abstract Builder setPriority(Priority priority); diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseBootstrapClient.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseBootstrapClient.java index bbbf661e919..46a72921953 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseBootstrapClient.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseBootstrapClient.java @@ -25,18 +25,28 @@ class DatabaseBootstrapClient { private final String createEventBackendIndex; private final String createContextBackedPriorityIndex; + private final String dropEventsSql; + private final String dropEventMetadataSql; + private final String dropContextsSql; + @Inject DatabaseBootstrapClient( @Named("CREATE_EVENTS_SQL") String createEventsSql, @Named("CREATE_EVENT_METADATA_SQL") String createEventMetadataSql, @Named("CREATE_CONTEXTS_SQL") String createContextsSql, @Named("CREATE_EVENT_BACKEND_INDEX") String createEventBackendIndex, - @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") String createContextBackendPriorityIndex) { + @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") String createContextBackendPriorityIndex, + @Named("DROP_EVENTS_SQL") String dropEventsSql, + @Named("DROP_EVENT_METADATA_SQL") String dropEventMetadataSql, + @Named("DROP_CONTEXTS_SQL") String dropContextsSql) { this.createEventsSql = createEventsSql; this.createEventMetadataSql = createEventMetadataSql; this.createContextsSql = createContextsSql; this.createEventBackendIndex = createEventBackendIndex; this.createContextBackedPriorityIndex = createContextBackendPriorityIndex; + this.dropEventsSql = dropEventsSql; + this.dropEventMetadataSql = dropEventMetadataSql; + this.dropContextsSql = dropContextsSql; } void bootstrap(SQLiteDatabase db) { @@ -46,4 +56,11 @@ void bootstrap(SQLiteDatabase db) { db.execSQL(createEventBackendIndex); db.execSQL(createContextBackedPriorityIndex); } + + void teardown(SQLiteDatabase db) { + db.execSQL(dropEventsSql); + db.execSQL(dropEventMetadataSql); + db.execSQL(dropContextsSql); + // Indices are dropped automatically when the tables are dropped + } } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseMigrationClient.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseMigrationClient.java new file mode 100644 index 00000000000..88edeb8bfc9 --- /dev/null +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseMigrationClient.java @@ -0,0 +1,35 @@ +// Copyright 2018 Google LLC +// +// 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.google.android.datatransport.runtime.scheduling.persistence; + +import android.database.sqlite.SQLiteDatabase; +import java.util.List; + +class DatabaseMigrationClient { + private final List incrementalMigrations; + + DatabaseMigrationClient(List incrementalMigrations) { + this.incrementalMigrations = incrementalMigrations; + } + + void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { + for (Migration m : incrementalMigrations) { + m.upgrade(db, fromVersion, toVersion); + } + } + + public interface Migration { + void upgrade(SQLiteDatabase db, int fromVersion, int toVersion); + } +} diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java index 4dfef347862..7e731e7e196 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java @@ -18,11 +18,23 @@ import dagger.Binds; import dagger.Module; import dagger.Provides; +import java.util.Collections; import javax.inject.Named; @Module public abstract class EventStoreModule { - static final String CREATE_EVENTS_SQL_V1 = + // Schema migration guidelines + // 1. Copy the SQL statements to create the tables and change as necessary. Rename variables to + // fit the format {}_SQL_Vn + // 2. Move the legacy SQL statements into the LegacySQL.java test class to reference in tests. + // 3. Write the Migration for existing clients, assign to variable name migrateToVn and add it to + // the collection injected into DatabaseMigrationClient + // 4. Write tests that cover the following scenarios + // 4a. Migrating versions V1..Vn-1 to Vn + // 4b. Bootstrapping the table at Vn + + // Note: 4. Migrations handle only upgrades. Downgrades will drop and recreate all tables/indices. + static final String CREATE_EVENTS_SQL_V2 = "CREATE TABLE events " + "(_id INTEGER PRIMARY KEY," + " context_id INTEGER NOT NULL," @@ -34,7 +46,7 @@ public abstract class EventStoreModule { + " num_attempts INTEGER NOT NULL," + "FOREIGN KEY (context_id) REFERENCES transport_contexts(_id) ON DELETE CASCADE)"; - static final String CREATE_EVENT_METADATA_SQL_V1 = + static final String CREATE_EVENT_METADATA_SQL_V2 = "CREATE TABLE event_metadata " + "(_id INTEGER PRIMARY KEY," + " event_id INTEGER NOT NULL," @@ -42,18 +54,35 @@ public abstract class EventStoreModule { + " value TEXT NOT NULL," + "FOREIGN KEY (event_id) REFERENCES events(_id) ON DELETE CASCADE)"; - static final String CREATE_CONTEXTS_SQL_V1 = + static final String CREATE_CONTEXTS_SQL_V2 = "CREATE TABLE transport_contexts " + "(_id INTEGER PRIMARY KEY," + " backend_name TEXT NOT NULL," + " priority INTEGER NOT NULL," - + " next_request_ms INTEGER NOT NULL)"; + + " next_request_ms INTEGER NOT NULL," + + " extras BLOB)"; - static final String CREATE_EVENT_BACKEND_INDEX_V1 = + static final String CREATE_EVENT_BACKEND_INDEX_V2 = "CREATE INDEX events_backend_id on events(context_id)"; - static final String CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1 = - "CREATE UNIQUE INDEX contexts_backend_priority on transport_contexts(backend_name, priority)"; + static final String CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2 = + "CREATE UNIQUE INDEX contexts_backend_priority_extras on transport_contexts(backend_name, priority, extras)"; + + static final String DROP_EVENTS_SQL = "DROP TABLE events"; + + static final String DROP_EVENT_METADATA_SQL = "DROP TABLE event_metadata"; + + static final String DROP_CONTEXTS_SQL = "DROP TABLE transport_contexts"; + + static final DatabaseMigrationClient.Migration MIGRATE_TO_V2 = + (db, fromVersion, toVersion) -> { + if (fromVersion == 1 && toVersion == 2) { + db.execSQL("ALTER TABLE transport_contexts ADD COLUMN extras BLOB"); + db.execSQL( + "CREATE UNIQUE INDEX contexts_backend_priority_extras on transport_contexts(backend_name, priority, extras)"); + db.execSQL("DROP INDEX contexts_backend_priority"); + } + }; @Provides static EventStoreConfig storeConfig() { @@ -69,30 +98,53 @@ static EventStoreConfig storeConfig() { @Provides @Named("CREATE_EVENTS_SQL") static String createEventsSql() { - return CREATE_EVENTS_SQL_V1; + return CREATE_EVENTS_SQL_V2; } @Provides @Named("CREATE_EVENT_METADATA_SQL") static String createEventMetadataSql() { - return CREATE_EVENT_METADATA_SQL_V1; + return CREATE_EVENT_METADATA_SQL_V2; } @Provides @Named("CREATE_CONTEXTS_SQL") static String createContextsSql() { - return CREATE_CONTEXTS_SQL_V1; + return CREATE_CONTEXTS_SQL_V2; } @Provides @Named("CREATE_EVENT_BACKEND_INDEX") static String getCreateEventBackendIndex() { - return CREATE_EVENT_BACKEND_INDEX_V1; + return CREATE_EVENT_BACKEND_INDEX_V2; } @Provides @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") static String createEventBackendPriorityIndex() { - return CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1; + return CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; + } + + @Provides + @Named("DROP_EVENTS_SQL") + static String dropEventsSQL() { + return DROP_EVENTS_SQL; + } + + @Provides + @Named("DROP_EVENT_METADATA_SQL") + static String dropEventMetadataSql() { + return DROP_EVENT_METADATA_SQL; + } + + @Provides + @Named("DROP_CONTEXTS_SQL") + static String dropContextsSql() { + return DROP_CONTEXTS_SQL; + } + + @Provides + static DatabaseMigrationClient createDatabaseMigrationClient() { + return new DatabaseMigrationClient(Collections.singletonList(MIGRATE_TO_V2)); } } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java index 834ce55bf9e..f6efb8b4229 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java @@ -78,7 +78,6 @@ private SQLiteDatabase getDb() { @Override @Nullable public PersistedEvent persist(TransportContext transportContext, EventInternal event) { - long newRowId = inTransaction( db -> { @@ -127,6 +126,8 @@ private long ensureTransportContext(SQLiteDatabase db, TransportContext transpor record.put("backend_name", transportContext.getBackendName()); record.put("priority", transportContext.getPriority().ordinal()); record.put("next_request_ms", 0); + // record.put("extras", transportContext.getExtras()); + return db.insert("transport_contexts", null, record); } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index c6d4993a63a..ce381b9b283 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -24,13 +24,18 @@ final class SchemaManager extends SQLiteOpenHelper { // upgrades work as expected, e.g. `up+down+up` is equivalent to `up`. private static int SCHEMA_VERSION = 1; private static final String DB_NAME = "com.google.android.datatransport.events"; - private final DatabaseBootstrapClient databaseBootstrapClient; + private final DatabaseBootstrapClient bootstrapClient; + private final DatabaseMigrationClient migrationClient; private boolean configured = false; @Inject - SchemaManager(Context context, DatabaseBootstrapClient databaseBootstrapClient) { + SchemaManager( + Context context, + DatabaseBootstrapClient bootstrapClient, + DatabaseMigrationClient migrationClient) { super(context, DB_NAME, null, SCHEMA_VERSION); - this.databaseBootstrapClient = databaseBootstrapClient; + this.bootstrapClient = bootstrapClient; + this.migrationClient = migrationClient; } @Override @@ -55,17 +60,19 @@ private void ensureConfigured(SQLiteDatabase db) { @Override public void onCreate(SQLiteDatabase db) { ensureConfigured(db); - databaseBootstrapClient.bootstrap(db); + bootstrapClient.bootstrap(db); } @Override public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) { ensureConfigured(db); + migrationClient.upgrade(db, oldVersion, newVersion); } @Override public void onDowngrade(SQLiteDatabase db, int oldVersion, int newVersion) { - ensureConfigured(db); + bootstrapClient.teardown(db); + onCreate(db); } @Override diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/LegacySQL.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/LegacySQL.java new file mode 100644 index 00000000000..dbc760e97ed --- /dev/null +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/LegacySQL.java @@ -0,0 +1,55 @@ +// Copyright 2018 Google LLC +// +// 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.google.android.datatransport.runtime.scheduling.persistence; + +/** + * Contains SQL strings that were used to bootstrap databases at previous versions. + * + *

Preserving these helps us simulate database state at previous versions and test migrations. + */ +class LegacySQL { + static final String CREATE_EVENTS_SQL_V1 = + "CREATE TABLE events " + + "(_id INTEGER PRIMARY KEY," + + " context_id INTEGER NOT NULL," + + " transport_name TEXT NOT NULL," + + " timestamp_ms INTEGER NOT NULL," + + " uptime_ms INTEGER NOT NULL," + + " payload BLOB NOT NULL," + + " code INTEGER," + + " num_attempts INTEGER NOT NULL," + + "FOREIGN KEY (context_id) REFERENCES transport_contexts(_id) ON DELETE CASCADE)"; + + static final String CREATE_EVENT_METADATA_SQL_V1 = + "CREATE TABLE event_metadata " + + "(_id INTEGER PRIMARY KEY," + + " event_id INTEGER NOT NULL," + + " name TEXT NOT NULL," + + " value TEXT NOT NULL," + + "FOREIGN KEY (event_id) REFERENCES events(_id) ON DELETE CASCADE)"; + + static final String CREATE_CONTEXTS_SQL_V1 = + "CREATE TABLE transport_contexts " + + "(_id INTEGER PRIMARY KEY," + + " backend_name TEXT NOT NULL," + + " priority INTEGER NOT NULL," + + " next_request_ms INTEGER NOT NULL)"; + + static final String CREATE_EVENT_BACKEND_INDEX_V1 = + "CREATE INDEX events_backend_id on events(context_id)"; + + static final String CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1 = + "CREATE UNIQUE INDEX contexts_backend_priority on transport_contexts(backend_name, priority)"; +} diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java index 3d4a6a73fbc..8f2936692a3 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java @@ -14,6 +14,7 @@ package com.google.android.datatransport.runtime.scheduling.persistence; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.*; import static com.google.common.truth.Truth.assertThat; import com.google.android.datatransport.Priority; @@ -60,11 +61,15 @@ private static SQLiteEventStore newStoreWithConfig(Clock clock, EventStoreConfig new SchemaManager( RuntimeEnvironment.application, new DatabaseBootstrapClient( - EventStoreModule.CREATE_EVENTS_SQL_V1, - EventStoreModule.CREATE_EVENT_METADATA_SQL_V1, - EventStoreModule.CREATE_CONTEXTS_SQL_V1, - EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V1, - EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1))); + CREATE_EVENTS_SQL_V2, + CREATE_EVENT_METADATA_SQL_V2, + CREATE_CONTEXTS_SQL_V2, + CREATE_EVENT_BACKEND_INDEX_V2, + CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2, + DROP_EVENTS_SQL, + DROP_EVENT_METADATA_SQL, + DROP_CONTEXTS_SQL), + new DatabaseMigrationClient(Collections.singletonList(MIGRATE_TO_V2)))); } @Test diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java index cb61b9e7ff5..7a85e05f5b2 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java @@ -13,17 +13,30 @@ // limitations under the License. package com.google.android.datatransport.runtime.scheduling.persistence; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V1; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_CONTEXTS_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENTS_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENT_METADATA_SQL; +import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_CONTEXTS_SQL_V1; +import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1; +import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_EVENTS_SQL_V1; +import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_EVENT_BACKEND_INDEX_V1; +import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_EVENT_METADATA_SQL_V1; import static com.google.common.truth.Truth.assertThat; +import android.content.ContentValues; +import android.database.sqlite.SQLiteDatabase; +import com.google.android.datatransport.Priority; import com.google.android.datatransport.runtime.EventInternal; import com.google.android.datatransport.runtime.TransportContext; import com.google.android.datatransport.runtime.time.TestClock; import com.google.android.datatransport.runtime.time.UptimeClock; +import edu.emory.mathcs.backport.java.util.Collections; +import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -33,6 +46,14 @@ public class SchemaManagerTest { private static final TransportContext CONTEXT1 = TransportContext.builder().setBackendName("b1").build(); + + private static final TransportContext CONTEXT2 = + TransportContext.builder() + .setBackendName("b2") + .setExtras("e2".getBytes()) + .build() + .withPriority(Priority.VERY_LOW); + private static final EventInternal EVENT1 = EventInternal.builder() .setTransportName("42") @@ -42,6 +63,7 @@ public class SchemaManagerTest { .addMetadata("key1", "value1") .addMetadata("key2", "value2") .build(); + private static final EventInternal EVENT2 = EVENT1.toBuilder().setPayload("World".getBytes()).build(); @@ -57,12 +79,29 @@ public class SchemaManagerTest { CREATE_EVENT_METADATA_SQL_V1, CREATE_CONTEXTS_SQL_V1, CREATE_EVENT_BACKEND_INDEX_V1, - CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1); + CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1, + DROP_EVENTS_SQL, + DROP_EVENT_METADATA_SQL, + DROP_CONTEXTS_SQL); + + private final DatabaseBootstrapClient V2_BOOTSTRAP_CLIENT = + new DatabaseBootstrapClient( + CREATE_EVENTS_SQL_V2, + CREATE_EVENT_METADATA_SQL_V2, + CREATE_CONTEXTS_SQL_V2, + CREATE_EVENT_BACKEND_INDEX_V2, + CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2, + DROP_EVENTS_SQL, + DROP_EVENT_METADATA_SQL, + DROP_CONTEXTS_SQL); + + private final DatabaseMigrationClient V2_MIGRATION = + new DatabaseMigrationClient(Collections.singletonList(EventStoreModule.MIGRATE_TO_V2)); @Test public void persist_correctlyRoundTrips() { SchemaManager schemaManager = - new SchemaManager(RuntimeEnvironment.application, V1_BOOTSTRAP_CLIENT); + new SchemaManager(RuntimeEnvironment.application, V1_BOOTSTRAP_CLIENT, V2_MIGRATION); SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); PersistedEvent newEvent = store.persist(CONTEXT1, EVENT1); @@ -71,4 +110,89 @@ public void persist_correctlyRoundTrips() { assertThat(newEvent.getEvent()).isEqualTo(EVENT1); assertThat(events).containsExactly(newEvent); } + + @Test + public void upgrading_emptyDatabase_allowsPersistsAfterUpgrade() { + SchemaManager schemaManager = + new SchemaManager(RuntimeEnvironment.application, V1_BOOTSTRAP_CLIENT, V2_MIGRATION); + SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); + + schemaManager.onUpgrade(schemaManager.getWritableDatabase(), 1, 2); + PersistedEvent newEvent1 = store.persist(CONTEXT1, EVENT1); + + assertThat(store.loadBatch(CONTEXT1)).containsExactly(newEvent1); + } + + @Test + public void upgradeding_nonEmptyDB_preservesValues() { + SchemaManager schemaManager = + new SchemaManager(RuntimeEnvironment.application, V1_BOOTSTRAP_CLIENT, V2_MIGRATION); + SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); + // We simulate operations as done by an older SQLLiteEventStore at V1 + // We cannot simulate older operations with a newer client + PersistedEvent event1 = simulatedPersistOnV1Database(schemaManager, CONTEXT1, EVENT1); + + // Upgrade to V2 + schemaManager.onUpgrade(schemaManager.getWritableDatabase(), 1, 2); + + assertThat(store.loadBatch(CONTEXT1)).containsExactly(event1); + } + + @Test + public void downgradeFromAFutureVersion_withEmptyDB_allowsPersistanceAfterMigration() { + SchemaManager schemaManager = + new SchemaManager(RuntimeEnvironment.application, V2_BOOTSTRAP_CLIENT, V2_MIGRATION); + SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); + // We simulate operations as done by an older SQLLiteEventStore at V1 + // We cannot simulate older operations with a newer client + simulatedPersistOnV1Database(schemaManager, CONTEXT1, EVENT1); + + schemaManager.onDowngrade(schemaManager.getWritableDatabase(), 3, 2); + PersistedEvent event2 = store.persist(CONTEXT2, EVENT2); + + assertThat(store.loadBatch(CONTEXT2)).containsExactly(event2); + } + + @Test + public void downgradeFromAFutureVersion_withNonEmptyDB_isLossy() { + SchemaManager schemaManager = + new SchemaManager(RuntimeEnvironment.application, V2_BOOTSTRAP_CLIENT, V2_MIGRATION); + SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); + PersistedEvent event1 = store.persist(CONTEXT1, EVENT1); + + schemaManager.onDowngrade(schemaManager.getWritableDatabase(), 3, 2); + + assertThat(store.loadBatch(CONTEXT1)).doesNotContain(event1); + } + + private PersistedEvent simulatedPersistOnV1Database( + SchemaManager schemaManager, TransportContext transportContext, EventInternal eventInternal) { + SQLiteDatabase db = schemaManager.getWritableDatabase(); + + ContentValues record = new ContentValues(); + record.put("backend_name", transportContext.getBackendName()); + record.put("priority", transportContext.getPriority().ordinal()); + record.put("next_request_ms", 0); + long contextId = db.insert("transport_contexts", null, record); + + ContentValues values = new ContentValues(); + values.put("context_id", contextId); + values.put("transport_name", eventInternal.getTransportName()); + values.put("timestamp_ms", eventInternal.getEventMillis()); + values.put("uptime_ms", eventInternal.getUptimeMillis()); + values.put("payload", eventInternal.getPayload()); + values.put("code", eventInternal.getCode()); + values.put("num_attempts", 0); + long newEventId = db.insert("events", null, values); + + for (Map.Entry entry : eventInternal.getMetadata().entrySet()) { + ContentValues metadata = new ContentValues(); + metadata.put("event_id", newEventId); + metadata.put("name", entry.getKey()); + metadata.put("value", entry.getValue()); + db.insert("event_metadata", null, metadata); + } + + return PersistedEvent.create(newEventId, transportContext, eventInternal); + } } From 8f336bdf9c2c41b037b2e8a598d99d68553c0c76 Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Mon, 1 Jul 2019 17:58:47 -0700 Subject: [PATCH 02/12] Added tests for nullability --- .../persistence/EventStoreModule.java | 4 +- .../persistence/SQLiteEventStore.java | 27 +++++++--- .../persistence/SQLiteEventStoreTest.java | 54 ++++++++++++++++++- 3 files changed, 75 insertions(+), 10 deletions(-) diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java index 7e731e7e196..d0f5baa2d57 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java @@ -60,7 +60,7 @@ public abstract class EventStoreModule { + " backend_name TEXT NOT NULL," + " priority INTEGER NOT NULL," + " next_request_ms INTEGER NOT NULL," - + " extras BLOB)"; + + " extras TEXT)"; static final String CREATE_EVENT_BACKEND_INDEX_V2 = "CREATE INDEX events_backend_id on events(context_id)"; @@ -77,7 +77,7 @@ public abstract class EventStoreModule { static final DatabaseMigrationClient.Migration MIGRATE_TO_V2 = (db, fromVersion, toVersion) -> { if (fromVersion == 1 && toVersion == 2) { - db.execSQL("ALTER TABLE transport_contexts ADD COLUMN extras BLOB"); + db.execSQL("ALTER TABLE transport_contexts ADD COLUMN extras TEXT"); db.execSQL( "CREATE UNIQUE INDEX contexts_backend_priority_extras on transport_contexts(backend_name, priority, extras)"); db.execSQL("DROP INDEX contexts_backend_priority"); diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java index f6efb8b4229..8619cf6d20d 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java @@ -14,6 +14,9 @@ package com.google.android.datatransport.runtime.scheduling.persistence; +import static android.util.Base64.DEFAULT; +import static android.util.Base64.encodeToString; + import android.content.ContentValues; import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; @@ -30,6 +33,7 @@ import com.google.android.datatransport.runtime.time.Monotonic; import com.google.android.datatransport.runtime.time.WallTime; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -126,22 +130,33 @@ private long ensureTransportContext(SQLiteDatabase db, TransportContext transpor record.put("backend_name", transportContext.getBackendName()); record.put("priority", transportContext.getPriority().ordinal()); record.put("next_request_ms", 0); - // record.put("extras", transportContext.getExtras()); + if (transportContext.getExtras() != null) { + record.put("extras", encodeToString(transportContext.getExtras(), DEFAULT)); + } return db.insert("transport_contexts", null, record); } @Nullable private Long getTransportContextId(SQLiteDatabase db, TransportContext transportContext) { + final StringBuilder selection = new StringBuilder("backend_name = ? and priority = ?"); + ArrayList selectionArgs = + new ArrayList<>( + Arrays.asList( + transportContext.getBackendName(), + String.valueOf(transportContext.getPriority().ordinal()))); + + if (transportContext.getExtras() != null) { + selection.append("and extras = ?"); + selectionArgs.add(encodeToString(transportContext.getExtras(), DEFAULT)); + } + return tryWithCursor( db.query( "transport_contexts", new String[] {"_id"}, - "backend_name = ? and priority = ?", - new String[] { - transportContext.getBackendName(), - String.valueOf(transportContext.getPriority().ordinal()) - }, + selection.toString(), + selectionArgs.toArray(new String[0]), null, null, null), diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java index 8f2936692a3..77557c9e102 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java @@ -83,8 +83,14 @@ public void persist_correctlyRoundTrips() { @Test public void persist_withEventsOfDifferentPriority_shouldEndBeStoredUnderDifferentContexts() { - TransportContext ctx1 = TRANSPORT_CONTEXT; - TransportContext ctx2 = TRANSPORT_CONTEXT.withPriority(Priority.VERY_LOW); + TransportContext ctx1 = + TransportContext.builder().setBackendName("backend1").setExtras("e1".getBytes()).build(); + TransportContext ctx2 = + TransportContext.builder() + .setBackendName("backend1") + .setExtras("e1".getBytes()) + .setPriority(Priority.VERY_LOW) + .build(); EventInternal event1 = EVENT; EventInternal event2 = EVENT.toBuilder().setPayload("World".getBytes()).build(); @@ -96,6 +102,50 @@ public void persist_withEventsOfDifferentPriority_shouldEndBeStoredUnderDifferen assertThat(store.loadBatch(ctx2)).containsExactly(newEvent2); } + @Test + public void persist_withEventsOfDifferentExtras_shouldEndBeStoredUnderDifferentContexts() { + TransportContext ctx1 = + TransportContext.builder().setBackendName("backend1").setExtras("e1".getBytes()).build(); + TransportContext ctx2 = + TransportContext.builder().setBackendName("backend1").setExtras("e2".getBytes()).build(); + + EventInternal event1 = EVENT; + EventInternal event2 = EVENT.toBuilder().setPayload("World".getBytes()).build(); + + PersistedEvent newEvent1 = store.persist(ctx1, event1); + PersistedEvent newEvent2 = store.persist(ctx2, event2); + + assertThat(store.loadBatch(ctx1)).containsExactly(newEvent1); + assertThat(store.loadBatch(ctx2)).containsExactly(newEvent2); + } + + @Test + public void persist_withEventsOfSameExtras_shouldEndBeStoredUnderSameContexts() { + TransportContext ctx1 = + TransportContext.builder().setBackendName("backend1").setExtras("e1".getBytes()).build(); + TransportContext ctx2 = + TransportContext.builder().setBackendName("backend1").setExtras("e1".getBytes()).build(); + + PersistedEvent newEvent1 = store.persist(ctx1, EVENT); + PersistedEvent newEvent2 = store.persist(ctx2, EVENT); + + assertThat(store.loadBatch(ctx2)).containsExactly(newEvent1, newEvent2); + } + + @Test + public void persist_sameBackendswithDifferentExtras_shouldEndBeStoredUnderDifferentContexts() { + TransportContext ctx1 = + TransportContext.builder().setBackendName("backend1").setExtras(null).build(); + TransportContext ctx2 = + TransportContext.builder().setBackendName("backend1").setExtras("e1".getBytes()).build(); + + PersistedEvent newEvent1 = store.persist(ctx1, EVENT); + PersistedEvent newEvent2 = store.persist(ctx2, EVENT); + + assertThat(store.loadBatch(ctx1)).containsExactly(newEvent1); + assertThat(store.loadBatch(ctx2)).containsExactly(newEvent2); + } + @Test public void persist_withEventCode_correctlyRoundTrips() { EventInternal eventWithCode = EVENT.toBuilder().setCode(5).build(); From 71795669f68602cd431b5d5e31ac69baf65f08e5 Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Tue, 2 Jul 2019 12:12:16 -0700 Subject: [PATCH 03/12] Modelling creates as migrations --- .../persistence/SpyEventStoreModule.java | 63 +--------- .../persistence/TestEventStoreModule.java | 64 +--------- .../persistence/DatabaseBootstrapClient.java | 66 ---------- .../persistence/DatabaseMigrationClient.java | 35 ------ .../persistence/EventStoreModule.java | 116 +----------------- .../scheduling/persistence/SchemaManager.java | 110 +++++++++++++++-- .../scheduling/persistence/LegacySQL.java | 55 --------- .../persistence/SQLiteEventStoreTest.java | 15 +-- .../persistence/SchemaManagerTest.java | 77 ++++-------- 9 files changed, 136 insertions(+), 465 deletions(-) delete mode 100644 transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseBootstrapClient.java delete mode 100644 transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseMigrationClient.java delete mode 100644 transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/LegacySQL.java diff --git a/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/SpyEventStoreModule.java b/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/SpyEventStoreModule.java index cbb5412a423..9487774001c 100644 --- a/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/SpyEventStoreModule.java +++ b/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/SpyEventStoreModule.java @@ -14,22 +14,12 @@ package com.google.android.datatransport.runtime.scheduling.persistence; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_CONTEXTS_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENTS_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENT_METADATA_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.MIGRATE_TO_V2; import static org.mockito.Mockito.spy; import com.google.android.datatransport.runtime.synchronization.SynchronizationGuard; import dagger.Binds; import dagger.Module; import dagger.Provides; -import java.util.Collections; import javax.inject.Named; import javax.inject.Singleton; @@ -50,55 +40,8 @@ static EventStore eventStore(SQLiteEventStore store) { abstract SynchronizationGuard synchronizationGuard(SQLiteEventStore store); @Provides - @Named("CREATE_EVENTS_SQL") - static String createEventsSql() { - return CREATE_EVENTS_SQL_V2; - } - - @Provides - @Named("CREATE_EVENT_METADATA_SQL") - static String createEventMetadataSql() { - return CREATE_EVENT_METADATA_SQL_V2; - } - - @Provides - @Named("CREATE_CONTEXTS_SQL") - static String createContextsSql() { - return CREATE_CONTEXTS_SQL_V2; - } - - @Provides - @Named("CREATE_EVENT_BACKEND_INDEX") - static String getCreateEventBackendIndex() { - return CREATE_EVENT_BACKEND_INDEX_V2; - } - - @Provides - @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") - static String createEventBackendPriorityIndex() { - return CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; - } - - @Provides - @Named("DROP_EVENTS_SQL") - static String dropEventsSQL() { - return DROP_EVENTS_SQL; - } - - @Provides - @Named("DROP_EVENT_METADATA_SQL") - static String dropEventMetadataSql() { - return DROP_EVENT_METADATA_SQL; - } - - @Provides - @Named("DROP_CONTEXTS_SQL") - static String dropContextsSql() { - return DROP_CONTEXTS_SQL; - } - - @Provides - static DatabaseMigrationClient createDatabaseMigrationClient() { - return new DatabaseMigrationClient(Collections.singletonList(MIGRATE_TO_V2)); + @Named("SCHEMA_VERSION") + static int schemaVersion() { + return SchemaManager.SCHEMA_VERSION; } } diff --git a/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/TestEventStoreModule.java b/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/TestEventStoreModule.java index f4ebf039cc1..758be3234e0 100644 --- a/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/TestEventStoreModule.java +++ b/transport/transport-runtime/src/androidTest/java/com/google/android/datatransport/runtime/scheduling/persistence/TestEventStoreModule.java @@ -14,21 +14,12 @@ package com.google.android.datatransport.runtime.scheduling.persistence; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_CONTEXTS_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENTS_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENT_METADATA_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.MIGRATE_TO_V2; +import static com.google.android.datatransport.runtime.scheduling.persistence.SchemaManager.SCHEMA_VERSION; import com.google.android.datatransport.runtime.synchronization.SynchronizationGuard; import dagger.Binds; import dagger.Module; import dagger.Provides; -import java.util.Collections; import javax.inject.Named; @Module @@ -54,55 +45,8 @@ static EventStoreConfig storeConfig() { abstract SynchronizationGuard synchronizationGuard(SQLiteEventStore store); @Provides - @Named("CREATE_EVENTS_SQL") - static String createEventsSql() { - return CREATE_EVENTS_SQL_V2; - } - - @Provides - @Named("CREATE_EVENT_METADATA_SQL") - static String createEventMetadataSql() { - return CREATE_EVENT_METADATA_SQL_V2; - } - - @Provides - @Named("CREATE_CONTEXTS_SQL") - static String createContextsSql() { - return CREATE_CONTEXTS_SQL_V2; - } - - @Provides - @Named("CREATE_EVENT_BACKEND_INDEX") - static String getCreateEventBackendIndex() { - return CREATE_EVENT_BACKEND_INDEX_V2; - } - - @Provides - @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") - static String createEventBackendPriorityIndex() { - return CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; - } - - @Provides - @Named("DROP_EVENTS_SQL") - static String dropEventsSQL() { - return DROP_EVENTS_SQL; - } - - @Provides - @Named("DROP_EVENT_METADATA_SQL") - static String dropEventMetadataSql() { - return DROP_EVENT_METADATA_SQL; - } - - @Provides - @Named("DROP_CONTEXTS_SQL") - static String dropContextsSql() { - return DROP_CONTEXTS_SQL; - } - - @Provides - static DatabaseMigrationClient createDatabaseMigrationClient() { - return new DatabaseMigrationClient(Collections.singletonList(MIGRATE_TO_V2)); + @Named("SCHEMA_VERSION") + static int schemaVersion() { + return SCHEMA_VERSION; } } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseBootstrapClient.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseBootstrapClient.java deleted file mode 100644 index 46a72921953..00000000000 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseBootstrapClient.java +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2019 Google LLC -// -// 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.google.android.datatransport.runtime.scheduling.persistence; - -import android.database.sqlite.SQLiteDatabase; -import javax.inject.Inject; -import javax.inject.Named; - -class DatabaseBootstrapClient { - private final String createContextsSql; - private final String createEventsSql; - private final String createEventMetadataSql; - private final String createEventBackendIndex; - private final String createContextBackedPriorityIndex; - - private final String dropEventsSql; - private final String dropEventMetadataSql; - private final String dropContextsSql; - - @Inject - DatabaseBootstrapClient( - @Named("CREATE_EVENTS_SQL") String createEventsSql, - @Named("CREATE_EVENT_METADATA_SQL") String createEventMetadataSql, - @Named("CREATE_CONTEXTS_SQL") String createContextsSql, - @Named("CREATE_EVENT_BACKEND_INDEX") String createEventBackendIndex, - @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") String createContextBackendPriorityIndex, - @Named("DROP_EVENTS_SQL") String dropEventsSql, - @Named("DROP_EVENT_METADATA_SQL") String dropEventMetadataSql, - @Named("DROP_CONTEXTS_SQL") String dropContextsSql) { - this.createEventsSql = createEventsSql; - this.createEventMetadataSql = createEventMetadataSql; - this.createContextsSql = createContextsSql; - this.createEventBackendIndex = createEventBackendIndex; - this.createContextBackedPriorityIndex = createContextBackendPriorityIndex; - this.dropEventsSql = dropEventsSql; - this.dropEventMetadataSql = dropEventMetadataSql; - this.dropContextsSql = dropContextsSql; - } - - void bootstrap(SQLiteDatabase db) { - db.execSQL(createEventsSql); - db.execSQL(createEventMetadataSql); - db.execSQL(createContextsSql); - db.execSQL(createEventBackendIndex); - db.execSQL(createContextBackedPriorityIndex); - } - - void teardown(SQLiteDatabase db) { - db.execSQL(dropEventsSql); - db.execSQL(dropEventMetadataSql); - db.execSQL(dropContextsSql); - // Indices are dropped automatically when the tables are dropped - } -} diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseMigrationClient.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseMigrationClient.java deleted file mode 100644 index 88edeb8bfc9..00000000000 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/DatabaseMigrationClient.java +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2018 Google LLC -// -// 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.google.android.datatransport.runtime.scheduling.persistence; - -import android.database.sqlite.SQLiteDatabase; -import java.util.List; - -class DatabaseMigrationClient { - private final List incrementalMigrations; - - DatabaseMigrationClient(List incrementalMigrations) { - this.incrementalMigrations = incrementalMigrations; - } - - void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { - for (Migration m : incrementalMigrations) { - m.upgrade(db, fromVersion, toVersion); - } - } - - public interface Migration { - void upgrade(SQLiteDatabase db, int fromVersion, int toVersion); - } -} diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java index d0f5baa2d57..480247eba92 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/EventStoreModule.java @@ -14,75 +14,16 @@ package com.google.android.datatransport.runtime.scheduling.persistence; +import static com.google.android.datatransport.runtime.scheduling.persistence.SchemaManager.SCHEMA_VERSION; + import com.google.android.datatransport.runtime.synchronization.SynchronizationGuard; import dagger.Binds; import dagger.Module; import dagger.Provides; -import java.util.Collections; import javax.inject.Named; @Module public abstract class EventStoreModule { - // Schema migration guidelines - // 1. Copy the SQL statements to create the tables and change as necessary. Rename variables to - // fit the format {}_SQL_Vn - // 2. Move the legacy SQL statements into the LegacySQL.java test class to reference in tests. - // 3. Write the Migration for existing clients, assign to variable name migrateToVn and add it to - // the collection injected into DatabaseMigrationClient - // 4. Write tests that cover the following scenarios - // 4a. Migrating versions V1..Vn-1 to Vn - // 4b. Bootstrapping the table at Vn - - // Note: 4. Migrations handle only upgrades. Downgrades will drop and recreate all tables/indices. - static final String CREATE_EVENTS_SQL_V2 = - "CREATE TABLE events " - + "(_id INTEGER PRIMARY KEY," - + " context_id INTEGER NOT NULL," - + " transport_name TEXT NOT NULL," - + " timestamp_ms INTEGER NOT NULL," - + " uptime_ms INTEGER NOT NULL," - + " payload BLOB NOT NULL," - + " code INTEGER," - + " num_attempts INTEGER NOT NULL," - + "FOREIGN KEY (context_id) REFERENCES transport_contexts(_id) ON DELETE CASCADE)"; - - static final String CREATE_EVENT_METADATA_SQL_V2 = - "CREATE TABLE event_metadata " - + "(_id INTEGER PRIMARY KEY," - + " event_id INTEGER NOT NULL," - + " name TEXT NOT NULL," - + " value TEXT NOT NULL," - + "FOREIGN KEY (event_id) REFERENCES events(_id) ON DELETE CASCADE)"; - - static final String CREATE_CONTEXTS_SQL_V2 = - "CREATE TABLE transport_contexts " - + "(_id INTEGER PRIMARY KEY," - + " backend_name TEXT NOT NULL," - + " priority INTEGER NOT NULL," - + " next_request_ms INTEGER NOT NULL," - + " extras TEXT)"; - - static final String CREATE_EVENT_BACKEND_INDEX_V2 = - "CREATE INDEX events_backend_id on events(context_id)"; - - static final String CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2 = - "CREATE UNIQUE INDEX contexts_backend_priority_extras on transport_contexts(backend_name, priority, extras)"; - - static final String DROP_EVENTS_SQL = "DROP TABLE events"; - - static final String DROP_EVENT_METADATA_SQL = "DROP TABLE event_metadata"; - - static final String DROP_CONTEXTS_SQL = "DROP TABLE transport_contexts"; - - static final DatabaseMigrationClient.Migration MIGRATE_TO_V2 = - (db, fromVersion, toVersion) -> { - if (fromVersion == 1 && toVersion == 2) { - db.execSQL("ALTER TABLE transport_contexts ADD COLUMN extras TEXT"); - db.execSQL( - "CREATE UNIQUE INDEX contexts_backend_priority_extras on transport_contexts(backend_name, priority, extras)"); - db.execSQL("DROP INDEX contexts_backend_priority"); - } - }; @Provides static EventStoreConfig storeConfig() { @@ -96,55 +37,8 @@ static EventStoreConfig storeConfig() { abstract SynchronizationGuard synchronizationGuard(SQLiteEventStore store); @Provides - @Named("CREATE_EVENTS_SQL") - static String createEventsSql() { - return CREATE_EVENTS_SQL_V2; - } - - @Provides - @Named("CREATE_EVENT_METADATA_SQL") - static String createEventMetadataSql() { - return CREATE_EVENT_METADATA_SQL_V2; - } - - @Provides - @Named("CREATE_CONTEXTS_SQL") - static String createContextsSql() { - return CREATE_CONTEXTS_SQL_V2; - } - - @Provides - @Named("CREATE_EVENT_BACKEND_INDEX") - static String getCreateEventBackendIndex() { - return CREATE_EVENT_BACKEND_INDEX_V2; - } - - @Provides - @Named("CREATE_CONTEXT_BACKEND_PRIORITY_INDEX") - static String createEventBackendPriorityIndex() { - return CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; - } - - @Provides - @Named("DROP_EVENTS_SQL") - static String dropEventsSQL() { - return DROP_EVENTS_SQL; - } - - @Provides - @Named("DROP_EVENT_METADATA_SQL") - static String dropEventMetadataSql() { - return DROP_EVENT_METADATA_SQL; - } - - @Provides - @Named("DROP_CONTEXTS_SQL") - static String dropContextsSql() { - return DROP_CONTEXTS_SQL; - } - - @Provides - static DatabaseMigrationClient createDatabaseMigrationClient() { - return new DatabaseMigrationClient(Collections.singletonList(MIGRATE_TO_V2)); + @Named("SCHEMA_VERSION") + static int schemaVersion() { + return SCHEMA_VERSION; } } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index ce381b9b283..9e1f8dfa3d0 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -17,25 +17,95 @@ import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteOpenHelper; import android.os.Build; +import java.util.Arrays; +import java.util.List; import javax.inject.Inject; +import javax.inject.Named; final class SchemaManager extends SQLiteOpenHelper { // TODO: when we do schema upgrades in the future we need to make sure both downgrades and // upgrades work as expected, e.g. `up+down+up` is equivalent to `up`. - private static int SCHEMA_VERSION = 1; private static final String DB_NAME = "com.google.android.datatransport.events"; - private final DatabaseBootstrapClient bootstrapClient; - private final DatabaseMigrationClient migrationClient; + private final int schemaVersion; private boolean configured = false; + // Schema migration guidelines + // 1. Model migration at Vn as an operation performed on the database at Vn-1. + // 2. Append the migration to the ordered list of Migrations in the static initializer + // 3. Write tests that cover the following scenarios migrating to Vn from V0..Vn-1 + // Note: Migrations handle only upgrades. Downgrades will drop and recreate all tables/indices. + private static final String CREATE_EVENTS_SQL_V1 = + "CREATE TABLE events " + + "(_id INTEGER PRIMARY KEY," + + " context_id INTEGER NOT NULL," + + " transport_name TEXT NOT NULL," + + " timestamp_ms INTEGER NOT NULL," + + " uptime_ms INTEGER NOT NULL," + + " payload BLOB NOT NULL," + + " code INTEGER," + + " num_attempts INTEGER NOT NULL," + + "FOREIGN KEY (context_id) REFERENCES transport_contexts(_id) ON DELETE CASCADE)"; + + private static final String CREATE_EVENT_METADATA_SQL_V1 = + "CREATE TABLE event_metadata " + + "(_id INTEGER PRIMARY KEY," + + " event_id INTEGER NOT NULL," + + " name TEXT NOT NULL," + + " value TEXT NOT NULL," + + "FOREIGN KEY (event_id) REFERENCES events(_id) ON DELETE CASCADE)"; + + private static final String CREATE_CONTEXTS_SQL_V1 = + "CREATE TABLE transport_contexts " + + "(_id INTEGER PRIMARY KEY," + + " backend_name TEXT NOT NULL," + + " priority INTEGER NOT NULL," + + " next_request_ms INTEGER NOT NULL)"; + + private static final String CREATE_EVENT_BACKEND_INDEX_V1 = + "CREATE INDEX events_backend_id on events(context_id)"; + + private static final String CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1 = + "CREATE UNIQUE INDEX contexts_backend_priority on transport_contexts(backend_name, priority)"; + + private static final String DROP_EVENTS_SQL = "DROP TABLE events"; + + private static final String DROP_EVENT_METADATA_SQL = "DROP TABLE event_metadata"; + + private static final String DROP_CONTEXTS_SQL = "DROP TABLE transport_contexts"; + + static int SCHEMA_VERSION = 2; + + static final SchemaManager.Migration MIGRATE_TO_V1 = + (db, fromVersion, toVersion) -> { + if (fromVersion < 1 && toVersion >= 1) { + db.execSQL(CREATE_EVENTS_SQL_V1); + db.execSQL(CREATE_EVENT_METADATA_SQL_V1); + db.execSQL(CREATE_CONTEXTS_SQL_V1); + db.execSQL(CREATE_EVENT_BACKEND_INDEX_V1); + db.execSQL(CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1); + } + }; + + static final SchemaManager.Migration MIGRATE_TO_V2 = + (db, fromVersion, toVersion) -> { + if (fromVersion < 2 && toVersion >= 2) { + db.execSQL("ALTER TABLE transport_contexts ADD COLUMN extras TEXT"); + db.execSQL( + "CREATE UNIQUE INDEX contexts_backend_priority_extras on transport_contexts(backend_name, priority, extras)"); + db.execSQL("DROP INDEX contexts_backend_priority"); + } + }; + + static final List INCREMENTAL_MIGRATIONS; + + static { + INCREMENTAL_MIGRATIONS = Arrays.asList(MIGRATE_TO_V1, MIGRATE_TO_V2); + } + @Inject - SchemaManager( - Context context, - DatabaseBootstrapClient bootstrapClient, - DatabaseMigrationClient migrationClient) { - super(context, DB_NAME, null, SCHEMA_VERSION); - this.bootstrapClient = bootstrapClient; - this.migrationClient = migrationClient; + SchemaManager(Context context, @Named("SCHEMA_VERSION") int schemaVersion) { + super(context, DB_NAME, null, schemaVersion); + this.schemaVersion = schemaVersion; } @Override @@ -60,18 +130,22 @@ private void ensureConfigured(SQLiteDatabase db) { @Override public void onCreate(SQLiteDatabase db) { ensureConfigured(db); - bootstrapClient.bootstrap(db); + upgrade(db, 0, schemaVersion); } @Override public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) { ensureConfigured(db); - migrationClient.upgrade(db, oldVersion, newVersion); + upgrade(db, oldVersion, newVersion); } @Override public void onDowngrade(SQLiteDatabase db, int oldVersion, int newVersion) { - bootstrapClient.teardown(db); + db.execSQL(DROP_EVENTS_SQL); + db.execSQL(DROP_EVENT_METADATA_SQL); + db.execSQL(DROP_CONTEXTS_SQL); + // Indices are dropped automatically when the tables are dropped + onCreate(db); } @@ -79,4 +153,14 @@ public void onDowngrade(SQLiteDatabase db, int oldVersion, int newVersion) { public void onOpen(SQLiteDatabase db) { ensureConfigured(db); } + + private void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { + for (Migration m : INCREMENTAL_MIGRATIONS) { + m.upgrade(db, fromVersion, toVersion); + } + } + + public interface Migration { + void upgrade(SQLiteDatabase db, int fromVersion, int toVersion); + } } diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/LegacySQL.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/LegacySQL.java deleted file mode 100644 index dbc760e97ed..00000000000 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/LegacySQL.java +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright 2018 Google LLC -// -// 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.google.android.datatransport.runtime.scheduling.persistence; - -/** - * Contains SQL strings that were used to bootstrap databases at previous versions. - * - *

Preserving these helps us simulate database state at previous versions and test migrations. - */ -class LegacySQL { - static final String CREATE_EVENTS_SQL_V1 = - "CREATE TABLE events " - + "(_id INTEGER PRIMARY KEY," - + " context_id INTEGER NOT NULL," - + " transport_name TEXT NOT NULL," - + " timestamp_ms INTEGER NOT NULL," - + " uptime_ms INTEGER NOT NULL," - + " payload BLOB NOT NULL," - + " code INTEGER," - + " num_attempts INTEGER NOT NULL," - + "FOREIGN KEY (context_id) REFERENCES transport_contexts(_id) ON DELETE CASCADE)"; - - static final String CREATE_EVENT_METADATA_SQL_V1 = - "CREATE TABLE event_metadata " - + "(_id INTEGER PRIMARY KEY," - + " event_id INTEGER NOT NULL," - + " name TEXT NOT NULL," - + " value TEXT NOT NULL," - + "FOREIGN KEY (event_id) REFERENCES events(_id) ON DELETE CASCADE)"; - - static final String CREATE_CONTEXTS_SQL_V1 = - "CREATE TABLE transport_contexts " - + "(_id INTEGER PRIMARY KEY," - + " backend_name TEXT NOT NULL," - + " priority INTEGER NOT NULL," - + " next_request_ms INTEGER NOT NULL)"; - - static final String CREATE_EVENT_BACKEND_INDEX_V1 = - "CREATE INDEX events_backend_id on events(context_id)"; - - static final String CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1 = - "CREATE UNIQUE INDEX contexts_backend_priority on transport_contexts(backend_name, priority)"; -} diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java index 77557c9e102..49ebfd898bc 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java @@ -60,16 +60,11 @@ private static SQLiteEventStore newStoreWithConfig(Clock clock, EventStoreConfig config, new SchemaManager( RuntimeEnvironment.application, - new DatabaseBootstrapClient( - CREATE_EVENTS_SQL_V2, - CREATE_EVENT_METADATA_SQL_V2, - CREATE_CONTEXTS_SQL_V2, - CREATE_EVENT_BACKEND_INDEX_V2, - CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2, - DROP_EVENTS_SQL, - DROP_EVENT_METADATA_SQL, - DROP_CONTEXTS_SQL), - new DatabaseMigrationClient(Collections.singletonList(MIGRATE_TO_V2)))); + Arrays.asList(EventStoreModule.MIGRATE_TO_V1, EventStoreModule.MIGRATE_TO_V2), + DROP_EVENTS_SQL, + DROP_EVENT_METADATA_SQL, + DROP_CONTEXTS_SQL, + SCHEMA_VERSION)); } @Test diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java index 7a85e05f5b2..b41b1fa1e08 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java @@ -13,19 +13,7 @@ // limitations under the License. package com.google.android.datatransport.runtime.scheduling.persistence; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXTS_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENTS_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_BACKEND_INDEX_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.CREATE_EVENT_METADATA_SQL_V2; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_CONTEXTS_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENTS_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.DROP_EVENT_METADATA_SQL; -import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_CONTEXTS_SQL_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_EVENTS_SQL_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_EVENT_BACKEND_INDEX_V1; -import static com.google.android.datatransport.runtime.scheduling.persistence.LegacySQL.CREATE_EVENT_METADATA_SQL_V1; +import static com.google.android.datatransport.runtime.scheduling.persistence.SchemaManager.SCHEMA_VERSION; import static com.google.common.truth.Truth.assertThat; import android.content.ContentValues; @@ -35,7 +23,6 @@ import com.google.android.datatransport.runtime.TransportContext; import com.google.android.datatransport.runtime.time.TestClock; import com.google.android.datatransport.runtime.time.UptimeClock; -import edu.emory.mathcs.backport.java.util.Collections; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -73,35 +60,9 @@ public class SchemaManagerTest { private final TestClock clock = new TestClock(1); - private final DatabaseBootstrapClient V1_BOOTSTRAP_CLIENT = - new DatabaseBootstrapClient( - CREATE_EVENTS_SQL_V1, - CREATE_EVENT_METADATA_SQL_V1, - CREATE_CONTEXTS_SQL_V1, - CREATE_EVENT_BACKEND_INDEX_V1, - CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1, - DROP_EVENTS_SQL, - DROP_EVENT_METADATA_SQL, - DROP_CONTEXTS_SQL); - - private final DatabaseBootstrapClient V2_BOOTSTRAP_CLIENT = - new DatabaseBootstrapClient( - CREATE_EVENTS_SQL_V2, - CREATE_EVENT_METADATA_SQL_V2, - CREATE_CONTEXTS_SQL_V2, - CREATE_EVENT_BACKEND_INDEX_V2, - CREATE_CONTEXT_BACKEND_PRIORITY_EXTRAS_INDEX_V2, - DROP_EVENTS_SQL, - DROP_EVENT_METADATA_SQL, - DROP_CONTEXTS_SQL); - - private final DatabaseMigrationClient V2_MIGRATION = - new DatabaseMigrationClient(Collections.singletonList(EventStoreModule.MIGRATE_TO_V2)); - @Test public void persist_correctlyRoundTrips() { - SchemaManager schemaManager = - new SchemaManager(RuntimeEnvironment.application, V1_BOOTSTRAP_CLIENT, V2_MIGRATION); + SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, SCHEMA_VERSION); SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); PersistedEvent newEvent = store.persist(CONTEXT1, EVENT1); @@ -112,42 +73,47 @@ public void persist_correctlyRoundTrips() { } @Test - public void upgrading_emptyDatabase_allowsPersistsAfterUpgrade() { - SchemaManager schemaManager = - new SchemaManager(RuntimeEnvironment.application, V1_BOOTSTRAP_CLIENT, V2_MIGRATION); + public void upgradingV1ToV2_emptyDatabase_allowsPersistsAfterUpgrade() { + int oldVersion = 1; + int newVersion = 2; + SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, oldVersion); + SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); - schemaManager.onUpgrade(schemaManager.getWritableDatabase(), 1, 2); + schemaManager.onUpgrade(schemaManager.getWritableDatabase(), oldVersion, newVersion); PersistedEvent newEvent1 = store.persist(CONTEXT1, EVENT1); assertThat(store.loadBatch(CONTEXT1)).containsExactly(newEvent1); } @Test - public void upgradeding_nonEmptyDB_preservesValues() { - SchemaManager schemaManager = - new SchemaManager(RuntimeEnvironment.application, V1_BOOTSTRAP_CLIENT, V2_MIGRATION); + public void upgradingV1ToV2_nonEmptyDB_isLossless() { + int oldVersion = 1; + int newVersion = 2; + SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, oldVersion); SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); // We simulate operations as done by an older SQLLiteEventStore at V1 // We cannot simulate older operations with a newer client PersistedEvent event1 = simulatedPersistOnV1Database(schemaManager, CONTEXT1, EVENT1); // Upgrade to V2 - schemaManager.onUpgrade(schemaManager.getWritableDatabase(), 1, 2); + schemaManager.onUpgrade(schemaManager.getWritableDatabase(), oldVersion, newVersion); assertThat(store.loadBatch(CONTEXT1)).containsExactly(event1); } @Test public void downgradeFromAFutureVersion_withEmptyDB_allowsPersistanceAfterMigration() { - SchemaManager schemaManager = - new SchemaManager(RuntimeEnvironment.application, V2_BOOTSTRAP_CLIENT, V2_MIGRATION); + int newVersion = 2; + int futureVersion = 3; + SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, futureVersion); + SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); // We simulate operations as done by an older SQLLiteEventStore at V1 // We cannot simulate older operations with a newer client simulatedPersistOnV1Database(schemaManager, CONTEXT1, EVENT1); - schemaManager.onDowngrade(schemaManager.getWritableDatabase(), 3, 2); + schemaManager.onDowngrade(schemaManager.getWritableDatabase(), futureVersion, newVersion); PersistedEvent event2 = store.persist(CONTEXT2, EVENT2); assertThat(store.loadBatch(CONTEXT2)).containsExactly(event2); @@ -155,12 +121,13 @@ public void downgradeFromAFutureVersion_withEmptyDB_allowsPersistanceAfterMigrat @Test public void downgradeFromAFutureVersion_withNonEmptyDB_isLossy() { - SchemaManager schemaManager = - new SchemaManager(RuntimeEnvironment.application, V2_BOOTSTRAP_CLIENT, V2_MIGRATION); + int newVersion = 2; + int futureVersion = 3; + SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, futureVersion); SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); PersistedEvent event1 = store.persist(CONTEXT1, EVENT1); - schemaManager.onDowngrade(schemaManager.getWritableDatabase(), 3, 2); + schemaManager.onDowngrade(schemaManager.getWritableDatabase(), futureVersion, newVersion); assertThat(store.loadBatch(CONTEXT1)).doesNotContain(event1); } From 993ad551854eb69fe33ca90244afaffe60a62cf1 Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Wed, 3 Jul 2019 10:35:05 -0700 Subject: [PATCH 04/12] Review feedback --- .../persistence/SQLiteEventStore.java | 2 +- .../scheduling/persistence/SchemaManager.java | 46 +++++++++---------- .../persistence/SQLiteEventStoreTest.java | 5 +- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java index 8619cf6d20d..5ec00c1e2c8 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java @@ -147,7 +147,7 @@ private Long getTransportContextId(SQLiteDatabase db, TransportContext transport String.valueOf(transportContext.getPriority().ordinal()))); if (transportContext.getExtras() != null) { - selection.append("and extras = ?"); + selection.append(" and extras = ?"); selectionArgs.add(encodeToString(transportContext.getExtras(), DEFAULT)); } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index 9e1f8dfa3d0..239855a56a8 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -75,32 +75,24 @@ final class SchemaManager extends SQLiteOpenHelper { static int SCHEMA_VERSION = 2; - static final SchemaManager.Migration MIGRATE_TO_V1 = - (db, fromVersion, toVersion) -> { - if (fromVersion < 1 && toVersion >= 1) { - db.execSQL(CREATE_EVENTS_SQL_V1); - db.execSQL(CREATE_EVENT_METADATA_SQL_V1); - db.execSQL(CREATE_CONTEXTS_SQL_V1); - db.execSQL(CREATE_EVENT_BACKEND_INDEX_V1); - db.execSQL(CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1); - } + private static final SchemaManager.Migration MIGRATE_TO_V1 = + (db) -> { + db.execSQL(CREATE_EVENTS_SQL_V1); + db.execSQL(CREATE_EVENT_METADATA_SQL_V1); + db.execSQL(CREATE_CONTEXTS_SQL_V1); + db.execSQL(CREATE_EVENT_BACKEND_INDEX_V1); + db.execSQL(CREATE_CONTEXT_BACKEND_PRIORITY_INDEX_V1); }; - static final SchemaManager.Migration MIGRATE_TO_V2 = - (db, fromVersion, toVersion) -> { - if (fromVersion < 2 && toVersion >= 2) { - db.execSQL("ALTER TABLE transport_contexts ADD COLUMN extras TEXT"); - db.execSQL( - "CREATE UNIQUE INDEX contexts_backend_priority_extras on transport_contexts(backend_name, priority, extras)"); - db.execSQL("DROP INDEX contexts_backend_priority"); - } + private static final SchemaManager.Migration MIGRATE_TO_V2 = + (db) -> { + db.execSQL("ALTER TABLE transport_contexts ADD COLUMN extras BLOB"); + db.execSQL( + "CREATE UNIQUE INDEX contexts_backend_priority_extras on transport_contexts(backend_name, priority, extras)"); + db.execSQL("DROP INDEX contexts_backend_priority"); }; - static final List INCREMENTAL_MIGRATIONS; - - static { - INCREMENTAL_MIGRATIONS = Arrays.asList(MIGRATE_TO_V1, MIGRATE_TO_V2); - } + static final List INCREMENTAL_MIGRATIONS = Arrays.asList(MIGRATE_TO_V1, MIGRATE_TO_V2); @Inject SchemaManager(Context context, @Named("SCHEMA_VERSION") int schemaVersion) { @@ -155,12 +147,16 @@ public void onOpen(SQLiteDatabase db) { } private void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { - for (Migration m : INCREMENTAL_MIGRATIONS) { - m.upgrade(db, fromVersion, toVersion); + int highestVersion = INCREMENTAL_MIGRATIONS.size(); + + for (int version = 1; version <= highestVersion; version++) { + if (fromVersion < version && toVersion >= version) { + INCREMENTAL_MIGRATIONS.get(version - 1).upgrade(db); + } } } public interface Migration { - void upgrade(SQLiteDatabase db, int fromVersion, int toVersion); + void upgrade(SQLiteDatabase db); } } diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java index 49ebfd898bc..0c667ba7c75 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java @@ -15,6 +15,7 @@ package com.google.android.datatransport.runtime.scheduling.persistence; import static com.google.android.datatransport.runtime.scheduling.persistence.EventStoreModule.*; +import static com.google.android.datatransport.runtime.scheduling.persistence.SchemaManager.SCHEMA_VERSION; import static com.google.common.truth.Truth.assertThat; import com.google.android.datatransport.Priority; @@ -60,10 +61,6 @@ private static SQLiteEventStore newStoreWithConfig(Clock clock, EventStoreConfig config, new SchemaManager( RuntimeEnvironment.application, - Arrays.asList(EventStoreModule.MIGRATE_TO_V1, EventStoreModule.MIGRATE_TO_V2), - DROP_EVENTS_SQL, - DROP_EVENT_METADATA_SQL, - DROP_CONTEXTS_SQL, SCHEMA_VERSION)); } From 28cf4085076f807f3908587932fcb05c8d5098b5 Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Wed, 3 Jul 2019 13:17:08 -0700 Subject: [PATCH 05/12] Review feedbackwq --- .../runtime/scheduling/persistence/SQLiteEventStore.java | 9 ++++----- .../runtime/scheduling/persistence/SchemaManager.java | 8 ++------ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java index 5ec00c1e2c8..3988134090d 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java @@ -14,14 +14,13 @@ package com.google.android.datatransport.runtime.scheduling.persistence; -import static android.util.Base64.DEFAULT; -import static android.util.Base64.encodeToString; - import android.content.ContentValues; import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteDatabaseLockedException; import android.os.SystemClock; +import android.util.Base64; + import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import androidx.annotation.WorkerThread; @@ -131,7 +130,7 @@ private long ensureTransportContext(SQLiteDatabase db, TransportContext transpor record.put("priority", transportContext.getPriority().ordinal()); record.put("next_request_ms", 0); if (transportContext.getExtras() != null) { - record.put("extras", encodeToString(transportContext.getExtras(), DEFAULT)); + record.put("extras", Base64.encodeToString(transportContext.getExtras(), Base64.DEFAULT)); } return db.insert("transport_contexts", null, record); @@ -148,7 +147,7 @@ private Long getTransportContextId(SQLiteDatabase db, TransportContext transport if (transportContext.getExtras() != null) { selection.append(" and extras = ?"); - selectionArgs.add(encodeToString(transportContext.getExtras(), DEFAULT)); + selectionArgs.add(Base64.encodeToString(transportContext.getExtras(), Base64.DEFAULT)); } return tryWithCursor( diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index 239855a56a8..c0d2d897862 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -147,12 +147,8 @@ public void onOpen(SQLiteDatabase db) { } private void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { - int highestVersion = INCREMENTAL_MIGRATIONS.size(); - - for (int version = 1; version <= highestVersion; version++) { - if (fromVersion < version && toVersion >= version) { - INCREMENTAL_MIGRATIONS.get(version - 1).upgrade(db); - } + for (int version = fromVersion; version < toVersion; version++) { + INCREMENTAL_MIGRATIONS.get(version).upgrade(db); } } From 4c14bec85aea7d4850200abde4cf24cb700067d9 Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Wed, 3 Jul 2019 13:25:18 -0700 Subject: [PATCH 06/12] Throwing exception --- .../runtime/scheduling/persistence/SQLiteEventStore.java | 1 - .../runtime/scheduling/persistence/SchemaManager.java | 8 ++++++++ .../scheduling/persistence/SQLiteEventStoreTest.java | 4 +--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java index 3988134090d..a3f8b9d1642 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStore.java @@ -20,7 +20,6 @@ import android.database.sqlite.SQLiteDatabaseLockedException; import android.os.SystemClock; import android.util.Base64; - import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import androidx.annotation.WorkerThread; diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index c0d2d897862..379512bc692 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -147,6 +147,14 @@ public void onOpen(SQLiteDatabase db) { } private void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { + throw new RuntimeException( + "Migration from " + + fromVersion + + " to " + + toVersion + + " was requested, but cannot be performed. Only " + + INCREMENTAL_MIGRATIONS.size() + + " migrations are provided"); for (int version = fromVersion; version < toVersion; version++) { INCREMENTAL_MIGRATIONS.get(version).upgrade(db); } diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java index 0c667ba7c75..1c5454ff8b0 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SQLiteEventStoreTest.java @@ -59,9 +59,7 @@ private static SQLiteEventStore newStoreWithConfig(Clock clock, EventStoreConfig clock, new UptimeClock(), config, - new SchemaManager( - RuntimeEnvironment.application, - SCHEMA_VERSION)); + new SchemaManager(RuntimeEnvironment.application, SCHEMA_VERSION)); } @Test From 70ffb738d3c40147b771a645d4e105f3001d9f29 Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Wed, 3 Jul 2019 14:05:10 -0700 Subject: [PATCH 07/12] added test --- .../scheduling/persistence/SchemaManager.java | 18 ++++++++++-------- .../persistence/SchemaManagerTest.java | 9 +++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index 379512bc692..3ced614af73 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -147,14 +147,16 @@ public void onOpen(SQLiteDatabase db) { } private void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { - throw new RuntimeException( - "Migration from " - + fromVersion - + " to " - + toVersion - + " was requested, but cannot be performed. Only " - + INCREMENTAL_MIGRATIONS.size() - + " migrations are provided"); + if(toVersion < INCREMENTAL_MIGRATIONS.size()) { + throw new RuntimeException( + "Migration from " + + fromVersion + + " to " + + toVersion + + " was requested, but cannot be performed. Only " + + INCREMENTAL_MIGRATIONS.size() + + " migrations are provided"); + } for (int version = fromVersion; version < toVersion; version++) { INCREMENTAL_MIGRATIONS.get(version).upgrade(db); } diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java index b41b1fa1e08..27e59fa59f5 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java @@ -132,6 +132,15 @@ public void downgradeFromAFutureVersion_withNonEmptyDB_isLossy() { assertThat(store.loadBatch(CONTEXT1)).doesNotContain(event1); } + @Test(expected = RuntimeException.class) + public void upgaden_toANonExistentVersion_fails() { + int oldVersion = 1; + int nonExistentVersion = 1000; + SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, oldVersion); + + schemaManager.onUpgrade(schemaManager.getWritableDatabase(), oldVersion, nonExistentVersion); + } + private PersistedEvent simulatedPersistOnV1Database( SchemaManager schemaManager, TransportContext transportContext, EventInternal eventInternal) { SQLiteDatabase db = schemaManager.getWritableDatabase(); From 00599069222fd832db6e777a60e918b4bd474ef2 Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Wed, 3 Jul 2019 14:05:43 -0700 Subject: [PATCH 08/12] java formatting --- .../scheduling/persistence/SchemaManager.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index 3ced614af73..4ea423d80dc 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -147,15 +147,15 @@ public void onOpen(SQLiteDatabase db) { } private void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { - if(toVersion < INCREMENTAL_MIGRATIONS.size()) { + if (toVersion < INCREMENTAL_MIGRATIONS.size()) { throw new RuntimeException( - "Migration from " - + fromVersion - + " to " - + toVersion - + " was requested, but cannot be performed. Only " - + INCREMENTAL_MIGRATIONS.size() - + " migrations are provided"); + "Migration from " + + fromVersion + + " to " + + toVersion + + " was requested, but cannot be performed. Only " + + INCREMENTAL_MIGRATIONS.size() + + " migrations are provided"); } for (int version = fromVersion; version < toVersion; version++) { INCREMENTAL_MIGRATIONS.get(version).upgrade(db); From 23a36f139c709c908fe771b705f9ba880332f9f3 Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Wed, 3 Jul 2019 15:31:31 -0700 Subject: [PATCH 09/12] Fixed tests --- .../scheduling/persistence/SchemaManager.java | 2 +- .../persistence/SchemaManagerTest.java | 21 +++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index 4ea423d80dc..eded1a7a387 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -147,7 +147,7 @@ public void onOpen(SQLiteDatabase db) { } private void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { - if (toVersion < INCREMENTAL_MIGRATIONS.size()) { + if (toVersion > INCREMENTAL_MIGRATIONS.size()) { throw new RuntimeException( "Migration from " + fromVersion diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java index 27e59fa59f5..04c1c1906eb 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java @@ -103,37 +103,36 @@ public void upgradingV1ToV2_nonEmptyDB_isLossless() { } @Test - public void downgradeFromAFutureVersion_withEmptyDB_allowsPersistanceAfterMigration() { - int newVersion = 2; - int futureVersion = 3; - SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, futureVersion); + public void downgradeV2ToV1_withEmptyDB_allowsPersistanceAfterMigration() { + int fromVersion = 2; + SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, fromVersion); SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); // We simulate operations as done by an older SQLLiteEventStore at V1 // We cannot simulate older operations with a newer client simulatedPersistOnV1Database(schemaManager, CONTEXT1, EVENT1); - schemaManager.onDowngrade(schemaManager.getWritableDatabase(), futureVersion, newVersion); + schemaManager.onDowngrade(schemaManager.getWritableDatabase(), fromVersion, -1); PersistedEvent event2 = store.persist(CONTEXT2, EVENT2); assertThat(store.loadBatch(CONTEXT2)).containsExactly(event2); } @Test - public void downgradeFromAFutureVersion_withNonEmptyDB_isLossy() { - int newVersion = 2; - int futureVersion = 3; - SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, futureVersion); + public void downgradeV2ToV1_withNonEmptyDB_isLossy() { + int fromVersion = 2; + int toVersion = fromVersion - 1; + SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, fromVersion); SQLiteEventStore store = new SQLiteEventStore(clock, new UptimeClock(), CONFIG, schemaManager); PersistedEvent event1 = store.persist(CONTEXT1, EVENT1); - schemaManager.onDowngrade(schemaManager.getWritableDatabase(), futureVersion, newVersion); + schemaManager.onDowngrade(schemaManager.getWritableDatabase(), toVersion, fromVersion); assertThat(store.loadBatch(CONTEXT1)).doesNotContain(event1); } @Test(expected = RuntimeException.class) - public void upgaden_toANonExistentVersion_fails() { + public void upgrade_toANonExistentVersion_fails() { int oldVersion = 1; int nonExistentVersion = 1000; SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, oldVersion); From 393e32b82b85340660a2f045a57e8bccb45dbb2a Mon Sep 17 00:00:00 2001 From: Ashwin Raghav Date: Fri, 12 Jul 2019 10:33:53 -0700 Subject: [PATCH 10/12] Apis to wire in extras (#581) --- .../datatransport/runtime/Destination.java | 29 +++++++++++++++++++ .../runtime/TransportRuntime.java | 11 +++++++ .../runtime/backends/BackendRequest.java | 19 +++++++++++- .../jobscheduling/AlarmManagerScheduler.java | 6 ++++ ...larmManagerSchedulerBroadcastReceiver.java | 16 +++++++--- .../jobscheduling/JobInfoScheduler.java | 6 ++++ .../JobInfoSchedulerService.java | 15 +++++++--- .../scheduling/jobscheduling/Uploader.java | 11 +++++-- .../AlarmManagerSchedulerTest.java | 17 +++++++++++ .../jobscheduling/JobInfoSchedulerTest.java | 14 +++++++++ 10 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/Destination.java diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/Destination.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/Destination.java new file mode 100644 index 00000000000..c88dac05180 --- /dev/null +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/Destination.java @@ -0,0 +1,29 @@ +// Copyright 2018 Google LLC +// +// 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.google.android.datatransport.runtime; + +import androidx.annotation.Nullable; + +public interface Destination { + /** Name that can be used to discover the backend */ + String getName(); + + /** + * Any extras that must be passed to the backend while uploading. Uploads to the backend are + * grouped by (backend_name, priority, extras) + */ + @Nullable + byte[] getExtras(); +} diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/TransportRuntime.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/TransportRuntime.java index 081a16cd9d1..da2fe60e363 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/TransportRuntime.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/TransportRuntime.java @@ -104,11 +104,22 @@ static void withInstance(TransportRuntimeComponent component, Callable cal } /** Returns a {@link TransportFactory} for a given {@code backendName}. */ + @Deprecated public TransportFactory newFactory(String backendName) { return new TransportFactoryImpl( TransportContext.builder().setBackendName(backendName).build(), this); } + /** Returns a {@link TransportFactory} for a given {@code backendName}. */ + public TransportFactory newFactory(Destination destination) { + return new TransportFactoryImpl( + TransportContext.builder() + .setBackendName(destination.getName()) + .setExtras(destination.getExtras()) + .build(), + this); + } + @RestrictTo(RestrictTo.Scope.LIBRARY) public Uploader getUploader() { return uploader; diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/backends/BackendRequest.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/backends/BackendRequest.java index b51a70d2a8b..ca5b9953aa1 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/backends/BackendRequest.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/backends/BackendRequest.java @@ -14,6 +14,7 @@ package com.google.android.datatransport.runtime.backends; +import androidx.annotation.Nullable; import com.google.android.datatransport.runtime.EventInternal; import com.google.auto.value.AutoValue; @@ -23,8 +24,24 @@ public abstract class BackendRequest { /** Events to be sent to the backend. */ public abstract Iterable getEvents(); + @Nullable + public abstract byte[] getExtras(); + /** Creates a new instance of the request. */ public static BackendRequest create(Iterable events) { - return new AutoValue_BackendRequest(events); + return BackendRequest.builder().setEvents(events).build(); + } + + public static Builder builder() { + return new AutoValue_BackendRequest.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setEvents(Iterable events); + + public abstract Builder setExtras(@Nullable byte[] extras); + + public abstract BackendRequest build(); } } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerScheduler.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerScheduler.java index 3fbed4aadb7..9862ecb73bd 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerScheduler.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerScheduler.java @@ -19,6 +19,7 @@ import android.content.Context; import android.content.Intent; import android.net.Uri; +import android.util.Base64; import androidx.annotation.VisibleForTesting; import com.google.android.datatransport.runtime.TransportContext; import com.google.android.datatransport.runtime.scheduling.persistence.EventStore; @@ -31,6 +32,7 @@ public class AlarmManagerScheduler implements WorkScheduler { static final String ATTEMPT_NUMBER = "attemptNumber"; static final String BACKEND_NAME = "backendName"; static final String EVENT_PRIORITY = "priority"; + static final String EXTRAS = "extras"; private final Context context; @@ -78,6 +80,10 @@ public void schedule(TransportContext transportContext, int attemptNumber) { intentDataBuilder.appendQueryParameter(BACKEND_NAME, transportContext.getBackendName()); intentDataBuilder.appendQueryParameter( EVENT_PRIORITY, String.valueOf(transportContext.getPriority().ordinal())); + if (transportContext.getExtras() != null) { + intentDataBuilder.appendQueryParameter( + EXTRAS, Base64.encodeToString(transportContext.getExtras(), Base64.DEFAULT)); + } Intent intent = new Intent(context, AlarmManagerSchedulerBroadcastReceiver.class); intent.setData(intentDataBuilder.build()); intent.putExtra(ATTEMPT_NUMBER, attemptNumber); diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerSchedulerBroadcastReceiver.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerSchedulerBroadcastReceiver.java index 80844c6a9ae..98db1058b8e 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerSchedulerBroadcastReceiver.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerSchedulerBroadcastReceiver.java @@ -14,6 +14,8 @@ package com.google.android.datatransport.runtime.scheduling.jobscheduling; +import static android.util.Base64.*; + import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; @@ -26,15 +28,21 @@ public class AlarmManagerSchedulerBroadcastReceiver extends BroadcastReceiver { @Override public void onReceive(Context context, Intent intent) { String backendName = intent.getData().getQueryParameter(AlarmManagerScheduler.BACKEND_NAME); + String extras = intent.getData().getQueryParameter(AlarmManagerScheduler.EXTRAS); int priority = Integer.valueOf(intent.getData().getQueryParameter(AlarmManagerScheduler.EVENT_PRIORITY)); int attemptNumber = intent.getExtras().getInt(AlarmManagerScheduler.ATTEMPT_NUMBER); TransportRuntime.initialize(context); + + TransportContext.Builder transportContext = + TransportContext.builder().setBackendName(backendName).setPriority(priority); + + if (extras != null) { + transportContext.setExtras(decode(extras, DEFAULT)); + } + TransportRuntime.getInstance() .getUploader() - .upload( - TransportContext.builder().setBackendName(backendName).setPriority(priority).build(), - attemptNumber, - () -> {}); + .upload(transportContext.build(), attemptNumber, () -> {}); } } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoScheduler.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoScheduler.java index 9d45c97caea..0a70e52cacc 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoScheduler.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoScheduler.java @@ -14,6 +14,8 @@ package com.google.android.datatransport.runtime.scheduling.jobscheduling; +import static android.util.Base64.*; + import android.app.job.JobInfo; import android.app.job.JobScheduler; import android.content.ComponentName; @@ -36,6 +38,7 @@ public class JobInfoScheduler implements WorkScheduler { static final String ATTEMPT_NUMBER = "attemptNumber"; static final String BACKEND_NAME = "backendName"; static final String EVENT_PRIORITY = "priority"; + static final String EXTRAS = "extras"; private final Context context; @@ -97,6 +100,9 @@ public void schedule(TransportContext transportContext, int attemptNumber) { bundle.putInt(ATTEMPT_NUMBER, attemptNumber); bundle.putString(BACKEND_NAME, transportContext.getBackendName()); bundle.putInt(EVENT_PRIORITY, transportContext.getPriority().ordinal()); + if (transportContext.getExtras() != null) { + bundle.putString(EXTRAS, encodeToString(transportContext.getExtras(), DEFAULT)); + } builder.setExtras(bundle); jobScheduler.schedule(builder.build()); diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoSchedulerService.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoSchedulerService.java index 75f5f0ada28..c493c95c9b4 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoSchedulerService.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoSchedulerService.java @@ -17,6 +17,7 @@ import android.app.job.JobParameters; import android.app.job.JobService; import android.os.Build; +import android.util.Base64; import androidx.annotation.RequiresApi; import com.google.android.datatransport.runtime.TransportContext; import com.google.android.datatransport.runtime.TransportRuntime; @@ -28,15 +29,21 @@ public class JobInfoSchedulerService extends JobService { @Override public boolean onStartJob(JobParameters params) { String backendName = params.getExtras().getString(JobInfoScheduler.BACKEND_NAME); + String extras = params.getExtras().getString(JobInfoScheduler.EXTRAS); + int priority = params.getExtras().getInt(JobInfoScheduler.EVENT_PRIORITY); int attemptNumber = params.getExtras().getInt(JobInfoScheduler.ATTEMPT_NUMBER); TransportRuntime.initialize(getApplicationContext()); + TransportContext.Builder transportContext = + TransportContext.builder().setBackendName(backendName).setPriority(priority); + + if (extras != null) { + transportContext.setExtras(Base64.decode(extras, Base64.DEFAULT)); + } + TransportRuntime.getInstance() .getUploader() - .upload( - TransportContext.builder().setBackendName(backendName).setPriority(priority).build(), - attemptNumber, - () -> this.jobFinished(params, false)); + .upload(transportContext.build(), attemptNumber, () -> this.jobFinished(params, false)); return true; } diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/Uploader.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/Uploader.java index 00c73827095..b2b5806c78e 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/Uploader.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/Uploader.java @@ -98,7 +98,7 @@ void logAndUpdateState(TransportContext transportContext, int attemptNumber) { Iterable persistedEvents = guard.runCriticalSection(() -> eventStore.loadBatch(transportContext)); - // Donot make a call to the backend if the list is empty. + // Do not make a call to the backend if the list is empty. if (!persistedEvents.iterator().hasNext()) { return; } @@ -106,7 +106,14 @@ void logAndUpdateState(TransportContext transportContext, int attemptNumber) { for (PersistedEvent persistedEvent : persistedEvents) { eventInternals.add(persistedEvent.getEvent()); } - BackendResponse response = backend.send(BackendRequest.create(eventInternals)); + + BackendResponse response = + backend.send( + BackendRequest.builder() + .setEvents(eventInternals) + .setExtras(transportContext.getExtras()) + .build()); + guard.runCriticalSection( () -> { if (response.getStatus() == BackendResponse.Status.TRANSIENT_ERROR) { diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerSchedulerTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerSchedulerTest.java index fce96ca8ead..4df9d3127d6 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerSchedulerTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/AlarmManagerSchedulerTest.java @@ -28,6 +28,7 @@ import android.content.Context; import android.content.Intent; import android.net.Uri; +import android.util.Base64; import com.google.android.datatransport.Priority; import com.google.android.datatransport.runtime.TransportContext; import com.google.android.datatransport.runtime.scheduling.persistence.EventStore; @@ -65,6 +66,12 @@ private Intent getIntent(TransportContext transportContext) { AlarmManagerScheduler.EVENT_PRIORITY, String.valueOf(transportContext.getPriority().ordinal())); + if (transportContext.getExtras() != null) { + intentDataBuilder.appendQueryParameter( + AlarmManagerScheduler.EXTRAS, + Base64.encodeToString(transportContext.getExtras(), Base64.DEFAULT)); + } + Intent intent = new Intent(context, AlarmManagerSchedulerBroadcastReceiver.class); intent.setData(intentDataBuilder.build()); return intent; @@ -124,6 +131,16 @@ public void schedule_twoJobs() { verify(alarmManager, times(1)).set(eq(AlarmManager.ELAPSED_REALTIME), gt(1000000L), any()); } + @Test + public void schedule_whenExtrasEvailable_transmitsExtras() { + TransportContext transportContext = + TransportContext.builder().setBackendName("backend1").setExtras("e1".getBytes()).build(); + Intent intent = getIntent(transportContext); + assertThat(scheduler.isJobServiceOn(intent)).isFalse(); + scheduler.schedule(transportContext, 1); + assertThat(scheduler.isJobServiceOn(intent)).isTrue(); + } + @Test public void schedule_smallWaitTImeFirstAttempt_multiplePriorities() { Intent intent1 = getIntent(TRANSPORT_CONTEXT); diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoSchedulerTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoSchedulerTest.java index 3d045b2b862..17c7e087026 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoSchedulerTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/JobInfoSchedulerTest.java @@ -21,6 +21,7 @@ import android.app.job.JobScheduler; import android.content.Context; import android.os.PersistableBundle; +import android.util.Base64; import com.google.android.datatransport.Priority; import com.google.android.datatransport.runtime.TransportContext; import com.google.android.datatransport.runtime.scheduling.persistence.EventStore; @@ -135,6 +136,19 @@ public void schedule_twoJobs() { .isEqualTo(TRANSPORT_CONTEXT.getBackendName()); } + @Test + public void schedule_whenExtrasEvailable_transmitsExtras() { + String extras = "e1"; + TransportContext transportContext = + TransportContext.builder().setBackendName("backend1").setExtras(extras.getBytes()).build(); + store.recordNextCallTime(transportContext, 1000000); + scheduler.schedule(transportContext, 1); + JobInfo jobInfo = jobScheduler.getAllPendingJobs().get(0); + PersistableBundle bundle = jobInfo.getExtras(); + assertThat(bundle.get(JobInfoScheduler.EXTRAS)) + .isEqualTo(Base64.encodeToString(extras.getBytes(), Base64.DEFAULT)); + } + @Test public void schedule_smallWaitTImeFirstAttempt_multiplePriorities() { store.recordNextCallTime(TRANSPORT_CONTEXT, 5); From 7b7755318a87899b0a68994d3f84dff029f7d83d Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Fri, 12 Jul 2019 10:45:04 -0700 Subject: [PATCH 11/12] Review feedback --- .../runtime/scheduling/persistence/SchemaManager.java | 5 +++-- .../scheduling/persistence/SchemaManagerTest.java | 9 +++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java index eded1a7a387..2395b5703c2 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManager.java @@ -92,7 +92,8 @@ final class SchemaManager extends SQLiteOpenHelper { db.execSQL("DROP INDEX contexts_backend_priority"); }; - static final List INCREMENTAL_MIGRATIONS = Arrays.asList(MIGRATE_TO_V1, MIGRATE_TO_V2); + private static final List INCREMENTAL_MIGRATIONS = + Arrays.asList(MIGRATE_TO_V1, MIGRATE_TO_V2); @Inject SchemaManager(Context context, @Named("SCHEMA_VERSION") int schemaVersion) { @@ -148,7 +149,7 @@ public void onOpen(SQLiteDatabase db) { private void upgrade(SQLiteDatabase db, int fromVersion, int toVersion) { if (toVersion > INCREMENTAL_MIGRATIONS.size()) { - throw new RuntimeException( + throw new IllegalArgumentException( "Migration from " + fromVersion + " to " diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java index 04c1c1906eb..ca8becb01e0 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java @@ -24,6 +24,7 @@ import com.google.android.datatransport.runtime.time.TestClock; import com.google.android.datatransport.runtime.time.UptimeClock; import java.util.Map; +import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -131,13 +132,17 @@ public void downgradeV2ToV1_withNonEmptyDB_isLossy() { assertThat(store.loadBatch(CONTEXT1)).doesNotContain(event1); } - @Test(expected = RuntimeException.class) + @Test public void upgrade_toANonExistentVersion_fails() { int oldVersion = 1; int nonExistentVersion = 1000; SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, oldVersion); - schemaManager.onUpgrade(schemaManager.getWritableDatabase(), oldVersion, nonExistentVersion); + Assert.assertThrows( + RuntimeException.class, + () -> + schemaManager.onUpgrade( + schemaManager.getWritableDatabase(), oldVersion, nonExistentVersion)); } private PersistedEvent simulatedPersistOnV1Database( From b5deb66e6355422dc83d12682e9b15694955dd2f Mon Sep 17 00:00:00 2001 From: ashwinraghav Date: Fri, 12 Jul 2019 10:53:25 -0700 Subject: [PATCH 12/12] Review feedback --- .../runtime/scheduling/persistence/SchemaManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java index ca8becb01e0..f2a3fa6ff0e 100644 --- a/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java +++ b/transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/persistence/SchemaManagerTest.java @@ -139,7 +139,7 @@ public void upgrade_toANonExistentVersion_fails() { SchemaManager schemaManager = new SchemaManager(RuntimeEnvironment.application, oldVersion); Assert.assertThrows( - RuntimeException.class, + IllegalArgumentException.class, () -> schemaManager.onUpgrade( schemaManager.getWritableDatabase(), oldVersion, nonExistentVersion));