Merge pull request #1240 from dhis2/androsdk-1130

fix: [ANDROSDK-1130] Fix database configuration migration

core/src/androidTest/java/org/hisp/dhis/android/core/configuration/internal/ConfigurationSecureStoreIntegrationShould.java → core/src/androidTest/java/org/hisp/dhis/android/core/configuration/internal/ConfigurationStoreIntegrationShould.java

@@ -28,62 +28,21 @@
 
 package org.hisp.dhis.android.core.configuration.internal;
 
-import android.content.Context;
-
-import androidx.test.InstrumentationRegistry;
-
-import org.hisp.dhis.android.core.arch.storage.internal.AndroidSecureStore;
-import org.hisp.dhis.android.core.arch.storage.internal.ObjectKeyValueStore;
 import org.hisp.dhis.android.core.data.configuration.ConfigurationSamples;
+import org.hisp.dhis.android.core.data.database.ObjectStoreAbstractIntegrationShould;
+import org.hisp.dhis.android.core.utils.integration.mock.TestDatabaseAdapterFactory;
 import org.hisp.dhis.android.core.utils.runner.D2JunitRunner;
-import org.junit.Before;
-import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import java.io.IOException;
-
-import okhttp3.HttpUrl;
-
-import static com.google.common.truth.Truth.assertThat;
-
 @RunWith(D2JunitRunner.class)
-public class ConfigurationSecureStoreIntegrationShould {
-
-    private final Configuration configuration;
-    private final ObjectKeyValueStore<Configuration> configurationSecureStore;
-
-    public ConfigurationSecureStoreIntegrationShould() {
-        Context context = InstrumentationRegistry.getTargetContext().getApplicationContext();
-        this.configurationSecureStore = new ConfigurationSecureStoreImpl(new AndroidSecureStore(context));
-        this.configuration = buildObject();
-    }
-
-    @Before
-    public void setUp() throws IOException {
-        configurationSecureStore.remove();
-    }
-
-    @Test
-    public void configure_and_get() {
-        configurationSecureStore.set(configuration);
-        Configuration objectFromDb = configurationSecureStore.get();
-        assertThat(objectFromDb.serverUrl()).isEqualTo(HttpUrl.parse("http://testserver.org/api/"));
-    }
-
-    @Test
-    public void configure_and_remove() {
-        configurationSecureStore.set(configuration);
-        configurationSecureStore.remove();
-        assertThat(configurationSecureStore.get()).isNull();
-    }
+public class ConfigurationStoreIntegrationShould extends ObjectStoreAbstractIntegrationShould<Configuration> {
 
-    @Test
-    public void overwrite_and_not_fail_in_a_consecutive_configuration() {
-        configurationSecureStore.set(configuration);
-        configurationSecureStore.set(configuration);
-        assertThat(configurationSecureStore.get()).isNotNull();
+    public ConfigurationStoreIntegrationShould() {
+        super(ConfigurationStore.create(TestDatabaseAdapterFactory.get()),
+                ConfigurationTableInfo.TABLE_INFO, TestDatabaseAdapterFactory.get());
     }
 
+    @Override
     protected Configuration buildObject() {
         return ConfigurationSamples.getConfiguration();
     }

core/src/androidTest/java/org/hisp/dhis/android/core/configuration/internal/DatabaseConfigurationMigrationIntegrationShould.java

@@ -34,11 +34,11 @@
 
 import org.hisp.dhis.android.core.arch.db.access.DatabaseAdapter;
 import org.hisp.dhis.android.core.arch.db.access.internal.DatabaseAdapterFactory;
+import org.hisp.dhis.android.core.arch.db.stores.internal.ObjectStore;
 import org.hisp.dhis.android.core.arch.storage.internal.InMemorySecureStore;
 import org.hisp.dhis.android.core.arch.storage.internal.InMemoryUnsecureStore;
 import org.hisp.dhis.android.core.arch.storage.internal.InsecureStore;
 import org.hisp.dhis.android.core.arch.storage.internal.ObjectKeyValueStore;
-import org.hisp.dhis.android.core.arch.storage.internal.SecureStore;
 import org.hisp.dhis.android.core.common.ObjectWithUid;
 import org.hisp.dhis.android.core.user.UserCredentials;
 import org.hisp.dhis.android.core.user.internal.UserCredentialsStore;
@@ -51,8 +51,6 @@
 import java.io.IOException;
 import java.util.Arrays;
 
-import okhttp3.HttpUrl;
-
 import static com.google.common.truth.Truth.assertThat;
 import static org.hisp.dhis.android.core.configuration.internal.DatabaseConfigurationMigration.OLD_DBNAME;
 
@@ -79,24 +77,20 @@ public class DatabaseConfigurationMigrationIntegrationShould {
             .user(ObjectWithUid.create("user"))
             .build();
 
-    private ObjectKeyValueStore<Configuration> oldConfigurationStore;
     private ObjectKeyValueStore<DatabasesConfiguration> newConfigurationStore;
 
     @Before
     public void setUp() throws IOException {
-        SecureStore secureStore = new InMemorySecureStore();
         InsecureStore insecureStore = new InMemoryUnsecureStore();
-        oldConfigurationStore = new ConfigurationSecureStoreImpl(secureStore);
         newConfigurationStore = DatabaseConfigurationInsecureStore.get(insecureStore);
-        migration = new DatabaseConfigurationMigration(context, oldConfigurationStore, newConfigurationStore,
+        migration = new DatabaseConfigurationMigration(context, newConfigurationStore,
                 transformer, nameGenerator, renamer, databaseAdapterFactory);
     }
 
     @Test
     public void delete_empty_database() {
         DatabaseAdapter databaseAdapter = databaseAdapterFactory.newParentDatabaseAdapter();
         databaseAdapterFactory.createOrOpenDatabase(databaseAdapter, OLD_DBNAME, false);
-        oldConfigurationStore.set(Configuration.forServerUrl(HttpUrl.parse(URL_STR)));
 
         assertThat(Arrays.asList(context.databaseList()).contains(OLD_DBNAME)).isTrue();
         migration.apply();
@@ -107,8 +101,7 @@ public void delete_empty_database() {
     public void rename_database_with_credentials() {
         DatabaseAdapter databaseAdapter = databaseAdapterFactory.newParentDatabaseAdapter();
         databaseAdapterFactory.createOrOpenDatabase(databaseAdapter, OLD_DBNAME, false);
-        oldConfigurationStore.set(Configuration.forServerUrl(HttpUrl.parse(URL_STR)));
-        setCredentials(databaseAdapter);
+        setCredentialsAndServerUrl(databaseAdapter);
 
         assertThat(Arrays.asList(context.databaseList()).contains(OLD_DBNAME)).isTrue();
         migration.apply();
@@ -138,20 +131,16 @@ public void return_existing_new_configuration_if_old_configuration_null() {
     public void return_empty_new_configuration_if_existing_empty_database() {
         DatabaseAdapter databaseAdapter = databaseAdapterFactory.newParentDatabaseAdapter();
         databaseAdapterFactory.createOrOpenDatabase(databaseAdapter, OLD_DBNAME, false);
-        oldConfigurationStore.set(Configuration.forServerUrl(HttpUrl.parse(URL_STR)));
         assertThat(migration.apply()).isNull();
     }
 
-    @Test
-    public void delete_old_configuration_after_applying_migration() {
-        oldConfigurationStore.set(Configuration.forServerUrl(HttpUrl.parse(URL_STR)));
-        assertThat(migration.apply()).isNull();
-        assertThat(oldConfigurationStore.get()).isNull();
-    }
+    public void setCredentialsAndServerUrl(DatabaseAdapter databaseAdapter) {
+        databaseAdapter.setForeignKeyConstraintsEnabled(false);
 
-    public void setCredentials(DatabaseAdapter databaseAdapter) {
         UserCredentialsStore credentialsStore = UserCredentialsStoreImpl.create(databaseAdapter);
-        databaseAdapter.setForeignKeyConstraintsEnabled(false);
         credentialsStore.insert(credentials);
+
+        ObjectStore<Configuration> configurationStore = ConfigurationStore.create(databaseAdapter);
+        configurationStore.insert(Configuration.forServerUrl(URL_STR));
     }
 }

core/src/main/assets/migrations/66.sql

@@ -1 +0,0 @@
-DROP TABLE IF EXISTS Configuration;

core/src/main/assets/snapshots/71.sql

@@ -1,3 +1,4 @@
+CREATE TABLE Configuration (_id INTEGER PRIMARY KEY AUTOINCREMENT, serverUrl TEXT NOT NULL UNIQUE);
 CREATE TABLE User (_id INTEGER PRIMARY KEY AUTOINCREMENT, uid TEXT NOT NULL UNIQUE, code TEXT, name TEXT, displayName TEXT, created TEXT, lastUpdated TEXT, birthday TEXT, education TEXT, gender TEXT, jobTitle TEXT, surname TEXT, firstName TEXT, introduction TEXT, employer TEXT, interests TEXT, languages TEXT, email TEXT, phoneNumber TEXT, nationality TEXT);
 CREATE TABLE UserCredentials (_id INTEGER PRIMARY KEY AUTOINCREMENT, uid TEXT NOT NULL UNIQUE, code TEXT, name TEXT, displayName TEXT, created TEXT, lastUpdated TEXT, username TEXT, user TEXT NOT NULL UNIQUE, FOREIGN KEY (user) REFERENCES User (uid) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED);
 CREATE TABLE OrganisationUnit (_id INTEGER PRIMARY KEY AUTOINCREMENT, uid TEXT NOT NULL UNIQUE, code TEXT, name TEXT, displayName TEXT, created TEXT, lastUpdated TEXT, shortName TEXT, displayShortName TEXT, description TEXT, displayDescription TEXT, path TEXT, openingDate TEXT, closedDate TEXT, level INTEGER, parent TEXT, displayNamePath TEXT, geometryType TEXT, geometryCoordinates TEXT);

core/src/main/java/org/hisp/dhis/android/core/D2Manager.java

@@ -113,7 +113,7 @@ public static Single<D2> instantiateD2(@NonNull D2Configuration d2Config) {
             }
 
             ObjectKeyValueStore<Credentials> credentialsSecureStore = new CredentialsSecureStoreImpl(secureStore);
-            MultiUserDatabaseManagerForD2Manager.create(databaseAdapter, d2Config.context(), secureStore, insecureStore,
+            MultiUserDatabaseManagerForD2Manager.create(databaseAdapter, d2Config.context(), insecureStore,
                     databaseAdapterFactory)
                     .loadIfLogged(credentialsSecureStore.get());
 

core/src/main/java/org/hisp/dhis/android/core/configuration/internal/Configuration.java

@@ -28,17 +28,29 @@
 
 package org.hisp.dhis.android.core.configuration.internal;
 
+import android.database.Cursor;
+
 import androidx.annotation.NonNull;
 
 import com.google.auto.value.AutoValue;
 
-import okhttp3.HttpUrl;
+import org.hisp.dhis.android.core.common.BaseObject;
+import org.hisp.dhis.android.core.common.CoreObject;
 
+/**
+ * Old configuration class. Needs to be kept for migration from SDK version previous to 1.1.0
+ * Use {@link DatabasesConfiguration} instead.
+ */
 @AutoValue
-public abstract class Configuration {
+@Deprecated
+public abstract class Configuration implements CoreObject {
 
     @NonNull
-    public abstract HttpUrl serverUrl();
+    public abstract String serverUrl();
+
+    public static Configuration create(Cursor cursor) {
+        return $AutoValue_Configuration.createFromCursor(cursor);
+    }
 
     public abstract Builder toBuilder();
 
@@ -47,13 +59,15 @@ public static Builder builder() {
     }
 
     @AutoValue.Builder
-    public abstract static class Builder {
-        public abstract Builder serverUrl(HttpUrl serverUrl);
+    public abstract static class Builder extends BaseObject.Builder<Builder> {
+        public abstract Builder id(Long id);
+
+        public abstract Builder serverUrl(String serverUrl);
 
         public abstract Configuration build();
     }
 
-    public static Configuration forServerUrl(HttpUrl url) {
+    static Configuration forServerUrl(String url) {
         return Configuration.builder().serverUrl(url).build();
     }
 }

core/src/main/java/org/hisp/dhis/android/core/configuration/internal/ConfigurationPackageDIModule.java

@@ -28,9 +28,6 @@
 
 package org.hisp.dhis.android.core.configuration.internal;
 
-import android.content.Context;
-
-import org.hisp.dhis.android.core.arch.db.access.internal.DatabaseAdapterFactory;
 import org.hisp.dhis.android.core.arch.storage.internal.InsecureStore;
 import org.hisp.dhis.android.core.arch.storage.internal.ObjectKeyValueStore;
 import org.hisp.dhis.android.core.arch.storage.internal.SecureStore;
@@ -62,14 +59,6 @@ DatabaseEncryptionPasswordManager passwordManager(SecureStore secureStore) {
         return DatabaseEncryptionPasswordManager.create(secureStore);
     }
 
-    @Provides
-    @Reusable
-    DatabaseConfigurationMigration configurationMigration(Context context, SecureStore secureStore,
-                                                          InsecureStore insecureStore,
-                                                          DatabaseAdapterFactory adapterFactory) {
-        return DatabaseConfigurationMigration.create(context, secureStore, insecureStore, adapterFactory);
-    }
-
     @Provides
     @Reusable
     ConstantModule module(ConstantModuleImpl impl) {

core/src/main/java/org/hisp/dhis/android/core/configuration/internal/ConfigurationStore.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2004-2019, University of Oslo
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ * Neither the name of the HISP project nor the names of its contributors may
+ * be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.hisp.dhis.android.core.configuration.internal;
+
+import org.hisp.dhis.android.core.arch.db.access.DatabaseAdapter;
+import org.hisp.dhis.android.core.arch.db.stores.binders.internal.StatementBinder;
+import org.hisp.dhis.android.core.arch.db.stores.internal.ObjectStore;
+import org.hisp.dhis.android.core.arch.db.stores.internal.StoreFactory;
+
+final class ConfigurationStore {
+
+    private static final StatementBinder<Configuration> BINDER = (o, w) ->
+            w.bind(1, o.serverUrl());
+
+    private ConfigurationStore() {
+    }
+
+    static ObjectStore<Configuration> create(DatabaseAdapter databaseAdapter) {
+        return StoreFactory.objectStore(databaseAdapter, ConfigurationTableInfo.TABLE_INFO, BINDER,
+                Configuration::create);
+    }
+}

core/src/main/java/org/hisp/dhis/android/core/configuration/internal/ConfigurationSecureStoreImpl.java → core/src/main/java/org/hisp/dhis/android/core/configuration/internal/ConfigurationTableInfo.java

@@ -28,42 +28,43 @@
 
 package org.hisp.dhis.android.core.configuration.internal;
 
-import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
+import org.hisp.dhis.android.core.arch.db.tableinfos.TableInfo;
+import org.hisp.dhis.android.core.arch.helpers.CollectionsHelper;
+import org.hisp.dhis.android.core.common.CoreColumns;
 
-import org.hisp.dhis.android.core.arch.storage.internal.ObjectKeyValueStore;
-import org.hisp.dhis.android.core.arch.storage.internal.SecureStore;
+final class ConfigurationTableInfo {
 
-import okhttp3.HttpUrl;
-
-public final class ConfigurationSecureStoreImpl implements ObjectKeyValueStore<Configuration> {
-
-    private static final String SERVER_URL = "server_url";
+    private ConfigurationTableInfo() {
+    }
 
-    private final SecureStore secureStore;
+    public static final TableInfo TABLE_INFO = new TableInfo() {
 
-    public ConfigurationSecureStoreImpl(@NonNull SecureStore secureStore) {
-        this.secureStore = secureStore;
-    }
+        @Override
+        public String name() {
+            return "Configuration";
+        }
 
-    @Override
-    public void set(@NonNull Configuration configuration) {
-        if (configuration == null) {
-            throw new IllegalArgumentException("configuration == null");
+        @Override
+        public CoreColumns columns() {
+            return new Columns();
         }
+    };
 
-        secureStore.setData(SERVER_URL, configuration.serverUrl().toString());
-    }
+    public static class Columns extends CoreColumns {
+        static final String SERVER_URL = "serverUrl";
 
-    @Nullable
-    @Override
-    public Configuration get() {
-        String serverUrl = secureStore.getData(SERVER_URL);
-        return serverUrl == null ? null : Configuration.forServerUrl(HttpUrl.parse(serverUrl));
-    }
+        @Override
+        public String[] all() {
+            return CollectionsHelper.appendInNewArray(super.all(),
+                    SERVER_URL
+            );
+        }
 
-    @Override
-    public void remove() {
-        secureStore.removeData(SERVER_URL);
+        @Override
+        public String[] whereUpdate() {
+            return CollectionsHelper.appendInNewArray(super.whereUpdate(),
+                    SERVER_URL
+            );
+        }
     }
 }