From d8879dd42572410883ff78c0ac28a41467e5de53 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 26 Jan 2018 17:42:59 +0100 Subject: [PATCH] make DBHelper follow the Java Singleton pattern It was already behaving like a singleton, but the code was spread around in other classes. DBHelper does not use a private constructor though since the tests prevent it. --- .../java/org/fdroid/fdroid/Preferences.java | 3 +- .../java/org/fdroid/fdroid/data/DBHelper.java | 26 +++++++++++++++-- .../fdroid/fdroid/data/FDroidProvider.java | 28 +++---------------- .../org/fdroid/fdroid/AntiFeaturesTest.java | 11 ++------ .../fdroid/data/AppPrefsProviderTest.java | 3 +- .../fdroid/fdroid/data/AppProviderTest.java | 8 +----- .../fdroid/data/FDroidProviderTest.java | 3 +- .../fdroid/data/InstalledAppProviderTest.java | 8 +----- .../fdroid/data/PreferredSignatureTest.java | 9 +----- .../fdroid/fdroid/data/ProviderUriTests.java | 2 +- .../fdroid/fdroid/data/RepoProviderTest.java | 1 - .../fdroid/data/SuggestedVersionTest.java | 8 +----- .../AcceptableMultiRepoUpdaterTest.java | 1 - .../fdroid/updater/IndexV1UpdaterTest.java | 8 +----- .../fdroid/updater/Issue763MultiRepo.java | 1 - .../fdroid/updater/MultiRepoUpdaterTest.java | 8 +----- .../updater/ProperMultiRepoUpdaterTest.java | 1 - .../fdroid/views/AppDetailsAdapterTest.java | 7 ++--- 18 files changed, 44 insertions(+), 92 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/Preferences.java b/app/src/main/java/org/fdroid/fdroid/Preferences.java index 7b59b3c95..e7bd2dfa3 100644 --- a/app/src/main/java/org/fdroid/fdroid/Preferences.java +++ b/app/src/main/java/org/fdroid/fdroid/Preferences.java @@ -453,8 +453,9 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh * While it is likely that most places asking for preferences have access to a {@link Context}, * it is a minor convenience to be able to ask for preferences without. */ - public static void clearSingletonForTesting() { + public static void setupForTests(Context context) { instance = null; + setup(context); } /** diff --git a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java index 22555916b..6a07920b4 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -47,13 +47,20 @@ import org.fdroid.fdroid.data.Schema.RepoTable; import java.util.ArrayList; import java.util.List; +/** + * This is basically a singleton used to represent the database at the core + * of all of the {@link android.content.ContentProvider}s used at the core + * of this app. {@link DBHelper} is not {@code private} so that it can be easily + * used in test subclasses. + */ @SuppressWarnings("LineLength") -class DBHelper extends SQLiteOpenHelper { +public class DBHelper extends SQLiteOpenHelper { private static final String TAG = "DBHelper"; public static final int REPO_XML_ARG_COUNT = 8; + private static DBHelper instance; private static final String DATABASE_NAME = "fdroid"; private static final String CREATE_TABLE_PACKAGE = "CREATE TABLE " + PackageTable.NAME @@ -214,7 +221,22 @@ class DBHelper extends SQLiteOpenHelper { DBHelper(Context context) { super(context, DATABASE_NAME, null, DB_VERSION); - this.context = context; + this.context = context.getApplicationContext(); + } + + /** + * Only used for testing. Not quite sure how to mock a singleton variable like this. + */ + public static void clearDbHelperSingleton() { + instance = null; + } + + static synchronized DBHelper getInstance(Context context) { + if (instance == null) { + Utils.debugLog(TAG, "First time accessing database, creating new helper"); + instance = new DBHelper(context); + } + return instance; } @Override diff --git a/app/src/main/java/org/fdroid/fdroid/data/FDroidProvider.java b/app/src/main/java/org/fdroid/fdroid/data/FDroidProvider.java index da1bde5d1..4e561c407 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/FDroidProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/FDroidProvider.java @@ -5,16 +5,13 @@ import android.content.ContentProvider; import android.content.ContentProviderOperation; import android.content.ContentProviderResult; import android.content.ContentValues; -import android.content.Context; import android.content.OperationApplicationException; import android.content.UriMatcher; import android.database.sqlite.SQLiteDatabase; import android.net.Uri; import android.os.Build; import android.support.annotation.NonNull; - import org.fdroid.fdroid.BuildConfig; -import org.fdroid.fdroid.Utils; import java.util.ArrayList; import java.util.HashSet; @@ -23,15 +20,13 @@ import java.util.Set; public abstract class FDroidProvider extends ContentProvider { - private static final String TAG = "FDroidProvider"; + public static final String TAG = "FDroidProvider"; static final String AUTHORITY = BuildConfig.APPLICATION_ID + ".data"; - static final int CODE_LIST = 1; + static final int CODE_LIST = 1; static final int CODE_SINGLE = 2; - private static DBHelper dbHelper; - private boolean isApplyingBatch; protected abstract String getTableName(); @@ -73,28 +68,13 @@ public abstract class FDroidProvider extends ContentProvider { return result; } - /** - * Only used for testing. Not quite sure how to mock a singleton variable like this. - */ - public static void clearDbHelperSingleton() { - dbHelper = null; - } - - private static synchronized DBHelper getOrCreateDb(Context context) { - if (dbHelper == null) { - Utils.debugLog(TAG, "First time accessing database, creating new helper"); - dbHelper = new DBHelper(context); - } - return dbHelper; - } - @Override public boolean onCreate() { return true; } protected final synchronized SQLiteDatabase db() { - return getOrCreateDb(getContext().getApplicationContext()).getWritableDatabase(); + return DBHelper.getInstance(getContext()).getWritableDatabase(); } @Override @@ -154,7 +134,7 @@ public abstract class FDroidProvider extends ContentProvider { if (!isValid) { throw new IllegalArgumentException( - "Cannot save field '" + key + "' to provider " + getProviderName()); + "Cannot save field '" + key + "' to provider " + getProviderName()); } } } diff --git a/app/src/test/java/org/fdroid/fdroid/AntiFeaturesTest.java b/app/src/test/java/org/fdroid/fdroid/AntiFeaturesTest.java index 2adb32ef7..16b2f712e 100644 --- a/app/src/test/java/org/fdroid/fdroid/AntiFeaturesTest.java +++ b/app/src/test/java/org/fdroid/fdroid/AntiFeaturesTest.java @@ -2,7 +2,6 @@ package org.fdroid.fdroid; import android.app.Application; import android.content.ContentValues; - import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; @@ -12,7 +11,6 @@ import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.FDroidProviderTest; import org.fdroid.fdroid.data.InstalledAppTestUtils; import org.fdroid.fdroid.data.Schema; -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -21,9 +19,9 @@ import org.robolectric.annotation.Config; import java.util.List; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; @Config(constants = BuildConfig.class, application = Application.class, sdk = 24) @RunWith(RobolectricTestRunner.class) @@ -35,7 +33,7 @@ public class AntiFeaturesTest extends FDroidProviderTest { @Before public void setup() { - Preferences.setup(context); + Preferences.setupForTests(context); ContentValues vulnValues = new ContentValues(1); vulnValues.put(Schema.ApkTable.Cols.AntiFeatures.ANTI_FEATURES, "KnownVuln,ContainsGreenButtons"); @@ -58,11 +56,6 @@ public class AntiFeaturesTest extends FDroidProviderTest { AppProvider.Helper.recalculatePreferredMetadata(context); } - @After - public void tearDown() { - Preferences.clearSingletonForTesting(); - } - private static String generateHash(String packageName, int versionCode) { return packageName + "-" + versionCode; } diff --git a/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java index 857424d3a..d0044c8ae 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java @@ -1,7 +1,6 @@ package org.fdroid.fdroid.data; import android.app.Application; - import org.fdroid.fdroid.Assert; import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.TestUtils; @@ -11,10 +10,10 @@ import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; @Config(constants = BuildConfig.class, application = Application.class, sdk = 24) @RunWith(RobolectricTestRunner.class) diff --git a/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java index fa230fdb9..6cc8803c7 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java @@ -10,7 +10,6 @@ import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.TestUtils; import org.fdroid.fdroid.data.Schema.AppMetadataTable.Cols; -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -40,12 +39,7 @@ public class AppProviderTest extends FDroidProviderTest { @Before public void setup() { TestUtils.registerContentProvider(AppProvider.getAuthority(), AppProvider.class); - Preferences.setup(context); - } - - @After - public void tearDown() { - Preferences.clearSingletonForTesting(); + Preferences.setupForTests(context); } /** diff --git a/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java index d03b610a5..416f564c5 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java @@ -1,7 +1,6 @@ package org.fdroid.fdroid.data; import android.content.ContextWrapper; - import org.fdroid.fdroid.TestUtils; import org.junit.After; import org.junit.Before; @@ -24,7 +23,7 @@ public abstract class FDroidProviderTest { @After public final void tearDownBase() { CategoryProvider.Helper.clearCategoryIdCache(); - FDroidProvider.clearDbHelperSingleton(); + DBHelper.clearDbHelperSingleton(); } } diff --git a/app/src/test/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java index 405967a8a..5bbedaa8d 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java @@ -8,7 +8,6 @@ import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.TestUtils; import org.fdroid.fdroid.data.Schema.InstalledAppTable.Cols; -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -31,12 +30,7 @@ public class InstalledAppProviderTest extends FDroidProviderTest { @Before public void setup() { TestUtils.registerContentProvider(InstalledAppProvider.getAuthority(), InstalledAppProvider.class); - Preferences.setup(context); - } - - @After - public void tearDown() { - Preferences.clearSingletonForTesting(); + Preferences.setupForTests(context); } @Test diff --git a/app/src/test/java/org/fdroid/fdroid/data/PreferredSignatureTest.java b/app/src/test/java/org/fdroid/fdroid/data/PreferredSignatureTest.java index d86510ef1..2d2ddf8ef 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/PreferredSignatureTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/PreferredSignatureTest.java @@ -2,11 +2,9 @@ package org.fdroid.fdroid.data; import android.app.Application; import android.content.Context; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.TestUtils; -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -24,7 +22,7 @@ public class PreferredSignatureTest extends FDroidProviderTest { @Before public void setup() { TestUtils.registerContentProvider(AppProvider.getAuthority(), AppProvider.class); - Preferences.setup(context); + Preferences.setupForTests(context); // This is what the FDroidApp does when this preference is changed. Need to also do this under testing. Preferences.get().registerUnstableUpdatesChangeListener(new Preferences.ChangeListener() { @@ -35,11 +33,6 @@ public class PreferredSignatureTest extends FDroidProviderTest { }); } - @After - public void tearDown() { - Preferences.clearSingletonForTesting(); - } - private Repo createFDroidRepo() { return RepoProviderTest.insertRepo(context, "https://f-droid.org/fdroid/repo", "", "", ""); } diff --git a/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java b/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java index 44e29ff30..5f738bcc0 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java @@ -39,7 +39,7 @@ public class ProviderUriTests { @After public void teardown() { - FDroidProvider.clearDbHelperSingleton(); + DBHelper.clearDbHelperSingleton(); } @Test diff --git a/app/src/test/java/org/fdroid/fdroid/data/RepoProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/RepoProviderTest.java index cbcd865d9..c7e75d6f9 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/RepoProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/RepoProviderTest.java @@ -26,7 +26,6 @@ import android.content.ContentValues; import android.content.Context; import android.net.Uri; import android.support.annotation.Nullable; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; diff --git a/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java index 4157de23b..269684c4d 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java @@ -5,7 +5,6 @@ import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.TestUtils; import org.fdroid.fdroid.data.Schema.AppMetadataTable.Cols; -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -24,7 +23,7 @@ public class SuggestedVersionTest extends FDroidProviderTest { @Before public void setup() { TestUtils.registerContentProvider(AppProvider.getAuthority(), AppProvider.class); - Preferences.setup(context); + Preferences.setupForTests(context); // This is what the FDroidApp does when this preference is changed. Need to also do this under testing. Preferences.get().registerUnstableUpdatesChangeListener(new Preferences.ChangeListener() { @@ -35,11 +34,6 @@ public class SuggestedVersionTest extends FDroidProviderTest { }); } - @After - public void tearDown() { - Preferences.clearSingletonForTesting(); - } - @Test public void singleRepoSingleSig() { App singleApp = TestUtils.insertApp( diff --git a/app/src/test/java/org/fdroid/fdroid/updater/AcceptableMultiRepoUpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/AcceptableMultiRepoUpdaterTest.java index 0f7517c46..fe90ccc26 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/AcceptableMultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/AcceptableMultiRepoUpdaterTest.java @@ -4,7 +4,6 @@ package org.fdroid.fdroid.updater; import android.content.ContentValues; import android.support.annotation.NonNull; import android.util.Log; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.RepoUpdater.UpdateException; import org.fdroid.fdroid.data.Repo; diff --git a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java index 2da5c4260..58f1d7111 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java @@ -26,7 +26,6 @@ import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.data.RepoPushRequest; import org.fdroid.fdroid.mock.RepoDetails; -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -66,12 +65,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest { @Before public void setup() { - Preferences.setup(context); - } - - @After - public void tearDown() { - Preferences.clearSingletonForTesting(); + Preferences.setupForTests(context); } @Test diff --git a/app/src/test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java b/app/src/test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java index 4db34ac39..6f5608f4b 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java @@ -1,7 +1,6 @@ package org.fdroid.fdroid.updater; import android.content.ContentValues; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.RepoUpdater; import org.fdroid.fdroid.data.Apk; diff --git a/app/src/test/java/org/fdroid/fdroid/updater/MultiRepoUpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/MultiRepoUpdaterTest.java index be5d81fe9..f7782553d 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/MultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/MultiRepoUpdaterTest.java @@ -17,7 +17,6 @@ import org.fdroid.fdroid.data.FDroidProviderTest; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.data.Schema; -import org.junit.After; import org.junit.Before; import java.io.File; @@ -86,12 +85,7 @@ public abstract class MultiRepoUpdaterTest extends FDroidProviderTest { RepoProvider.Helper.remove(context, 3); RepoProvider.Helper.remove(context, 4); - Preferences.setup(context); - } - - @After - public final void tearDownMultiRepo() { - Preferences.clearSingletonForTesting(); + Preferences.setupForTests(context); } protected void assertApp(String packageName, int[] versionCodes) { diff --git a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java index 2936110fa..aad16e4f8 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java @@ -4,7 +4,6 @@ package org.fdroid.fdroid.updater; import android.content.ContentValues; import android.support.annotation.StringDef; import android.util.Log; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.RepoUpdater; import org.fdroid.fdroid.TestUtils; diff --git a/app/src/test/java/org/fdroid/fdroid/views/AppDetailsAdapterTest.java b/app/src/test/java/org/fdroid/fdroid/views/AppDetailsAdapterTest.java index 73bbe5bf2..3532f1192 100644 --- a/app/src/test/java/org/fdroid/fdroid/views/AppDetailsAdapterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/views/AppDetailsAdapterTest.java @@ -14,7 +14,7 @@ import org.fdroid.fdroid.R; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProviderTest; -import org.fdroid.fdroid.data.FDroidProvider; +import org.fdroid.fdroid.data.DBHelper; import org.fdroid.fdroid.data.FDroidProviderTest; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProviderTest; @@ -36,7 +36,7 @@ public class AppDetailsAdapterTest extends FDroidProviderTest { @Before public void setup() { ImageLoader.getInstance().init(ImageLoaderConfiguration.createDefault(context)); - Preferences.setup(context); + Preferences.setupForTests(context); Repo repo = RepoProviderTest.insertRepo(context, "http://www.example.com/fdroid/repo", "", "", "Test Repo"); app = AppProviderTest.insertApp(contentResolver, context, "com.example.app", "Test App", @@ -46,8 +46,7 @@ public class AppDetailsAdapterTest extends FDroidProviderTest { @After public void teardown() { ImageLoader.getInstance().destroy(); - FDroidProvider.clearDbHelperSingleton(); - Preferences.clearSingletonForTesting(); + DBHelper.clearDbHelperSingleton(); } @Test