From d47967e03dc17bf620c7883546806b0ea9061239 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 25 Jul 2016 17:33:00 +1000 Subject: [PATCH 01/10] Added new table to store user preferences. Haven't yet migrated data from the old location in the app table yet. --- app/src/main/AndroidManifest.xml | 5 + .../java/org/fdroid/fdroid/data/AppPrefs.java | 24 +++ .../fdroid/fdroid/data/AppPrefsProvider.java | 173 ++++++++++++++++++ .../java/org/fdroid/fdroid/data/DBHelper.java | 9 + .../java/org/fdroid/fdroid/data/Schema.java | 13 ++ .../fdroid/data/AppPrefsProviderTest.java | 56 ++++++ 6 files changed, 280 insertions(+) create mode 100644 app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java create mode 100644 app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java create mode 100644 app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index aef660433..84e2e3c7b 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -95,6 +95,11 @@ android:name="org.fdroid.fdroid.data.InstalledAppProvider" android:exported="false"/> + + 0); + } + } finally { + cursor.close(); + } + } + } + + private class Query extends QueryBuilder { + + @Override + protected String getRequiredTables() { + return AppPrefsTable.NAME; + } + + @Override + public void addField(String field) { + appendField(field, getTableName()); + } + } + + private static final String PROVIDER_NAME = "AppPrefsProvider"; + + private static final UriMatcher MATCHER = new UriMatcher(-1); + + private static final String PATH_APP_ID = "appId"; + + static { + MATCHER.addURI(getAuthority(), PATH_APP_ID + "/#", CODE_SINGLE); + } + + private static Uri getContentUri() { + return Uri.parse("content://" + getAuthority()); + } + + public static Uri getAppUri(long appId) { + return getContentUri().buildUpon().appendPath(PATH_APP_ID).appendPath(Long.toString(appId)).build(); + } + + @Override + protected String getTableName() { + return AppPrefsTable.NAME; + } + + @Override + protected String getProviderName() { + return "AppPrefsProvider"; + } + + public static String getAuthority() { + return AUTHORITY + "." + PROVIDER_NAME; + } + + @Override + protected UriMatcher getMatcher() { + return MATCHER; + } + + protected QuerySelection querySingle(long appId) { + final String selection = getTableName() + "." + Cols.APP_ID + " = ?"; + final String[] args = {Long.toString(appId)}; + return new QuerySelection(selection, args); + } + + @Override + public Cursor query(Uri uri, String[] projection, String customSelection, String[] selectionArgs, String sortOrder) { + QuerySelection selection = new QuerySelection(customSelection, selectionArgs); + + switch (MATCHER.match(uri)) { + case CODE_SINGLE: + selection = selection.add(querySingle(Long.parseLong(uri.getLastPathSegment()))); + break; + + default: + Log.e(TAG, "Invalid URI for app content provider: " + uri); + throw new UnsupportedOperationException("Invalid URI for app content provider: " + uri); + } + + Query query = new Query(); + query.addSelection(selection); + query.addFields(projection); + query.addOrderBy(sortOrder); + + Cursor cursor = LoggingQuery.query(db(), query.toString(), query.getArgs()); + cursor.setNotificationUri(getContext().getContentResolver(), uri); + return cursor; + } + + @Override + public int delete(Uri uri, String where, String[] whereArgs) { + switch (MATCHER.match(uri)) { + default: + throw new UnsupportedOperationException("Delete not supported for " + uri + "."); + } + } + + @Override + public Uri insert(Uri uri, ContentValues values) { + db().insertOrThrow(getTableName(), null, values); + if (!isApplyingBatch()) { + getContext().getContentResolver().notifyChange(uri, null); + } + return getAppUri(values.getAsLong(Cols.APP_ID)); + } + + @Override + public int update(Uri uri, ContentValues values, String where, String[] whereArgs) { + switch (MATCHER.match(uri)) { + case CODE_SINGLE: + QuerySelection query = new QuerySelection(where, whereArgs) + .add(querySingle(Long.parseLong(uri.getLastPathSegment()))); + return db().update(getTableName(), values, query.getSelection(), query.getArgs()); + + default: + throw new UnsupportedOperationException("Update not supported for " + uri + "."); + + } + } +} 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 c0597b420..3aa8a338d 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -11,6 +11,7 @@ import android.util.Log; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Schema.ApkTable; +import org.fdroid.fdroid.data.Schema.AppPrefsTable; import org.fdroid.fdroid.data.Schema.AppTable; import org.fdroid.fdroid.data.Schema.InstalledAppTable; import org.fdroid.fdroid.data.Schema.RepoTable; @@ -101,6 +102,13 @@ class DBHelper extends SQLiteOpenHelper { + AppTable.Cols.ICON_URL_LARGE + " text, " + "primary key(" + AppTable.Cols.PACKAGE_NAME + "));"; + private static final String CREATE_TABLE_APP_PREFS = "CREATE TABLE " + AppPrefsTable.NAME + + " ( " + + AppPrefsTable.Cols.APP_ID + " INT REFERENCES " + AppTable.NAME + "(" + AppTable.Cols.ROW_ID + ") ON DELETE CASCADE, " + + AppPrefsTable.Cols.IGNORE_THIS_UPDATE+ " INT BOOLEAN NOT NULL, " + + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + " INT NOT NULL " + + " );"; + private static final String CREATE_TABLE_INSTALLED_APP = "CREATE TABLE " + InstalledAppTable.NAME + " ( " + InstalledAppTable.Cols.PACKAGE_NAME + " TEXT NOT NULL PRIMARY KEY, " @@ -219,6 +227,7 @@ class DBHelper extends SQLiteOpenHelper { createAppApk(db); db.execSQL(CREATE_TABLE_INSTALLED_APP); db.execSQL(CREATE_TABLE_REPO); + db.execSQL(CREATE_TABLE_APP_PREFS); insertRepo( db, diff --git a/app/src/main/java/org/fdroid/fdroid/data/Schema.java b/app/src/main/java/org/fdroid/fdroid/data/Schema.java index 7b39a045a..7735e554c 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -9,6 +9,19 @@ import android.provider.BaseColumns; */ public interface Schema { + interface AppPrefsTable { + + String NAME = "fdroid_appPrefs"; + + interface Cols extends BaseColumns { + String APP_ID = "appId"; + String IGNORE_ALL_UPDATES = "ignoreAllUpdates"; + String IGNORE_THIS_UPDATE = "ignoreThisUpdate"; + + String[] ALL = {APP_ID, IGNORE_ALL_UPDATES, IGNORE_THIS_UPDATE,}; + } + } + interface AppTable { String NAME = "fdroid_app"; diff --git a/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java new file mode 100644 index 000000000..81e18de0b --- /dev/null +++ b/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java @@ -0,0 +1,56 @@ +package org.fdroid.fdroid.data; + +import android.app.Application; + +import org.fdroid.fdroid.Assert; +import org.fdroid.fdroid.BuildConfig; +import org.fdroid.fdroid.TestUtils; +import org.fdroid.fdroid.data.Schema.AppPrefsTable.Cols; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricGradleTestRunner; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowContentResolver; + +import static junit.framework.Assert.assertTrue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; + +@Config(constants = BuildConfig.class, application = Application.class) +@RunWith(RobolectricGradleTestRunner.class) +public class AppPrefsProviderTest extends FDroidProviderTest { + + private static final String[] PROJ = Cols.ALL; + + @Before + public void setup() { + ShadowContentResolver.registerProvider(AppProvider.getAuthority(), new AppProvider()); + } + + @Test + public void newPreferences() { + App withPrefs = Assert.insertApp(context, "com.example.withPrefs", "With Prefs"); + App withoutPrefs = Assert.insertApp(context, "com.example.withoutPrefs", "Without Prefs"); + + assertNull(AppPrefsProvider.Helper.getPrefsOrNull(context, withPrefs)); + assertNull(AppPrefsProvider.Helper.getPrefsOrNull(context, withoutPrefs)); + + AppPrefs defaultPrefs = AppPrefsProvider.Helper.getPrefsOrDefault(context, withPrefs); + assertEquals(0, defaultPrefs.ignoreThisUpdate); + assertFalse(defaultPrefs.ignoreAllUpdates); + + AppPrefsProvider.Helper.update(context, withPrefs, new AppPrefs(12, false)); + AppPrefs newPrefs = AppPrefsProvider.Helper.getPrefsOrDefault(context, withPrefs); + assertEquals(12, newPrefs.ignoreThisUpdate); + assertFalse(newPrefs.ignoreAllUpdates); + + AppPrefsProvider.Helper.update(context, withPrefs, new AppPrefs(14, true)); + AppPrefs evenNewerPrefs = AppPrefsProvider.Helper.getPrefsOrDefault(context, withPrefs); + assertEquals(14, evenNewerPrefs.ignoreThisUpdate); + assertTrue(evenNewerPrefs.ignoreAllUpdates); + + assertNull(AppPrefsProvider.Helper.getPrefsOrNull(context, withoutPrefs)); + } +} From 3e3af3bbf379b894c763606f0760f7fade61812a Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 25 Jul 2016 18:11:31 +1000 Subject: [PATCH 02/10] Completely removed preferences from app table. --- .../java/org/fdroid/fdroid/AppDetails.java | 45 +++++++----------- .../main/java/org/fdroid/fdroid/data/App.java | 34 +++++--------- .../java/org/fdroid/fdroid/data/AppPrefs.java | 14 +++++- .../org/fdroid/fdroid/data/AppProvider.java | 46 ++++++++++++++++--- .../java/org/fdroid/fdroid/data/DBHelper.java | 2 - .../org/fdroid/fdroid/data/RepoPersister.java | 22 +-------- .../java/org/fdroid/fdroid/data/Schema.java | 5 +- .../fdroid/fdroid/views/AppListAdapter.java | 2 +- .../views/fragments/AppListFragment.java | 2 - .../test/java/org/fdroid/fdroid/Assert.java | 2 - .../fdroid/fdroid/data/AppProviderTest.java | 30 ++++++------ 11 files changed, 96 insertions(+), 108 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 5749dcbbf..7a683e756 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -81,6 +81,8 @@ import com.nostra13.universalimageloader.core.assist.ImageScaleType; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; +import org.fdroid.fdroid.data.AppPrefs; +import org.fdroid.fdroid.data.AppPrefsProvider; import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.InstalledAppProvider; import org.fdroid.fdroid.data.RepoProvider; @@ -317,8 +319,7 @@ public class AppDetails extends AppCompatActivity { private String activeDownloadUrlString; private LocalBroadcastManager localBroadcastManager; - private boolean startingIgnoreAll; - private int startingIgnoreThis; + private AppPrefs startingPrefs; private final Context context = this; @@ -472,10 +473,9 @@ public class AppDetails extends AppCompatActivity { .edit() .putString(getPackageNameFromIntent(getIntent()), activeDownloadUrlString) .apply(); - if (app != null && (app.ignoreAllUpdates != startingIgnoreAll - || app.ignoreThisUpdate != startingIgnoreThis)) { + if (app != null && !app.getPrefs(this).equals(startingPrefs)) { Utils.debugLog(TAG, "Updating 'ignore updates', as it has changed since we started the activity..."); - setIgnoreUpdates(app.packageName, app.ignoreAllUpdates, app.ignoreThisUpdate); + AppPrefsProvider.Helper.update(this, app, app.getPrefs(this)); } unregisterDownloaderReceiver(); } @@ -646,18 +646,6 @@ public class AppDetails extends AppCompatActivity { supportInvalidateOptionsMenu(); } - private void setIgnoreUpdates(String packageName, boolean ignoreAll, int ignoreVersionCode) { - - Uri uri = AppProvider.getContentUri(packageName); - - ContentValues values = new ContentValues(2); - values.put(Schema.AppTable.Cols.IGNORE_ALLUPDATES, ignoreAll ? 1 : 0); - values.put(Schema.AppTable.Cols.IGNORE_THISUPDATE, ignoreVersionCode); - - getContentResolver().update(uri, values, null, null); - - } - @Override public Object onRetainCustomNonConfigurationInstance() { return new ConfigurationChangeHelper(activeDownloadUrlString, app); @@ -711,8 +699,7 @@ public class AppDetails extends AppCompatActivity { app = newApp; - startingIgnoreAll = app.ignoreAllUpdates; - startingIgnoreThis = app.ignoreThisUpdate; + startingPrefs = app.getPrefs(this).createClone(); } private void refreshApkList() { @@ -733,7 +720,7 @@ public class AppDetails extends AppCompatActivity { return true; } - if (packageManager.getLaunchIntentForPackage(app.packageName) != null && app.canAndWantToUpdate()) { + if (packageManager.getLaunchIntentForPackage(app.packageName) != null && app.canAndWantToUpdate(this)) { MenuItemCompat.setShowAsAction(menu.add( Menu.NONE, LAUNCH, 1, R.string.menu_launch) .setIcon(R.drawable.ic_play_arrow_white), @@ -758,13 +745,13 @@ public class AppDetails extends AppCompatActivity { menu.add(Menu.NONE, IGNOREALL, 2, R.string.menu_ignore_all) .setIcon(R.drawable.ic_do_not_disturb_white) .setCheckable(true) - .setChecked(app.ignoreAllUpdates); + .setChecked(app.getPrefs(context).ignoreAllUpdates); if (app.hasUpdates()) { menu.add(Menu.NONE, IGNORETHIS, 2, R.string.menu_ignore_this) .setIcon(R.drawable.ic_do_not_disturb_white) .setCheckable(true) - .setChecked(app.ignoreThisUpdate >= app.suggestedVersionCode); + .setChecked(app.getPrefs(context).ignoreThisUpdate >= app.suggestedVersionCode); } // Ignore on devices without Bluetooth @@ -880,17 +867,17 @@ public class AppDetails extends AppCompatActivity { return true; case IGNOREALL: - app.ignoreAllUpdates ^= true; - item.setChecked(app.ignoreAllUpdates); + app.getPrefs(this).ignoreAllUpdates ^= true; + item.setChecked(app.getPrefs(this).ignoreAllUpdates); return true; case IGNORETHIS: - if (app.ignoreThisUpdate >= app.suggestedVersionCode) { - app.ignoreThisUpdate = 0; + if (app.getPrefs(this).ignoreThisUpdate >= app.suggestedVersionCode) { + app.getPrefs(this).ignoreThisUpdate = 0; } else { - app.ignoreThisUpdate = app.suggestedVersionCode; + app.getPrefs(this).ignoreThisUpdate = app.suggestedVersionCode; } - item.setChecked(app.ignoreThisUpdate > 0); + item.setChecked(app.getPrefs(this).ignoreThisUpdate > 0); return true; case SEND_VIA_BLUETOOTH: @@ -1600,7 +1587,7 @@ public class AppDetails extends AppCompatActivity { installed = true; statusView.setText(getString(R.string.details_installed, app.installedVersionName)); NfcHelper.setAndroidBeam(appDetails, app.packageName); - if (app.canAndWantToUpdate()) { + if (app.canAndWantToUpdate(appDetails)) { updateWanted = true; btMain.setText(R.string.menu_upgrade); } else { diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index 759fbcbde..35f94fd84 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -103,15 +103,7 @@ public class App extends ValueObject implements Comparable, Parcelable { */ public String[] requirements; - /** - * True if all updates for this app are to be ignored - */ - public boolean ignoreAllUpdates; - - /** - * True if the current update for this app is to be ignored - */ - public int ignoreThisUpdate; + private AppPrefs prefs; /** * To be displayed at 48dp (x1.0) @@ -233,12 +225,6 @@ public class App extends ValueObject implements Comparable, Parcelable { case Cols.REQUIREMENTS: requirements = Utils.parseCommaSeparatedString(cursor.getString(i)); break; - case Cols.IGNORE_ALLUPDATES: - ignoreAllUpdates = cursor.getInt(i) == 1; - break; - case Cols.IGNORE_THISUPDATE: - ignoreThisUpdate = cursor.getInt(i); - break; case Cols.ICON_URL: iconUrl = cursor.getString(i); break; @@ -471,8 +457,6 @@ public class App extends ValueObject implements Comparable, Parcelable { values.put(Cols.ANTI_FEATURES, Utils.serializeCommaSeparatedString(antiFeatures)); values.put(Cols.REQUIREMENTS, Utils.serializeCommaSeparatedString(requirements)); values.put(Cols.IS_COMPATIBLE, compatible ? 1 : 0); - values.put(Cols.IGNORE_ALLUPDATES, ignoreAllUpdates ? 1 : 0); - values.put(Cols.IGNORE_THISUPDATE, ignoreThisUpdate); return values; } @@ -492,13 +476,21 @@ public class App extends ValueObject implements Comparable, Parcelable { return updates; } + public AppPrefs getPrefs(Context context) { + if (prefs == null) { + prefs = AppPrefsProvider.Helper.getPrefsOrDefault(context, this); + } + return prefs; + } + /** * True if there are new versions (apks) available and the user wants * to be notified about them */ - public boolean canAndWantToUpdate() { + public boolean canAndWantToUpdate(Context context) { boolean canUpdate = hasUpdates(); - boolean wantsUpdate = !ignoreAllUpdates && ignoreThisUpdate < suggestedVersionCode; + AppPrefs prefs = getPrefs(context); + boolean wantsUpdate = !prefs.ignoreAllUpdates && prefs.ignoreThisUpdate < suggestedVersionCode; return canUpdate && wantsUpdate && !isFiltered(); } @@ -591,8 +583,6 @@ public class App extends ValueObject implements Comparable, Parcelable { dest.writeStringArray(this.categories); dest.writeStringArray(this.antiFeatures); dest.writeStringArray(this.requirements); - dest.writeByte(this.ignoreAllUpdates ? (byte) 1 : (byte) 0); - dest.writeInt(this.ignoreThisUpdate); dest.writeString(this.iconUrl); dest.writeString(this.iconUrlLarge); dest.writeString(this.installedVersionName); @@ -631,8 +621,6 @@ public class App extends ValueObject implements Comparable, Parcelable { this.categories = in.createStringArray(); this.antiFeatures = in.createStringArray(); this.requirements = in.createStringArray(); - this.ignoreAllUpdates = in.readByte() != 0; - this.ignoreThisUpdate = in.readInt(); this.iconUrl = in.readString(); this.iconUrlLarge = in.readString(); this.installedVersionName = in.readString(); diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java b/app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java index 89d310349..cd3571177 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java @@ -5,12 +5,12 @@ public class AppPrefs extends ValueObject { /** * True if all updates for this app are to be ignored */ - public final boolean ignoreAllUpdates; + public boolean ignoreAllUpdates; /** * True if the current update for this app is to be ignored */ - public final int ignoreThisUpdate; + public int ignoreThisUpdate; public AppPrefs(int ignoreThis, boolean ignoreAll) { ignoreThisUpdate = ignoreThis; @@ -21,4 +21,14 @@ public class AppPrefs extends ValueObject { return new AppPrefs(0, false); } + @Override + public boolean equals(Object o) { + return o != null && o instanceof AppPrefs && + ((AppPrefs)o).ignoreAllUpdates == ignoreAllUpdates && + ((AppPrefs)o).ignoreThisUpdate == ignoreThisUpdate; + } + + public AppPrefs createClone() { + return new AppPrefs(ignoreThisUpdate, ignoreAllUpdates); + } } diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java index 9653ba1c5..a0c312e2e 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -14,6 +14,7 @@ import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Schema.ApkTable; +import org.fdroid.fdroid.data.Schema.AppPrefsTable; import org.fdroid.fdroid.data.Schema.AppTable; import org.fdroid.fdroid.data.Schema.AppTable.Cols; import org.fdroid.fdroid.data.Schema.InstalledAppTable; @@ -186,6 +187,7 @@ public class AppProvider extends FDroidProvider { protected static class AppQuerySelection extends QuerySelection { private boolean naturalJoinToInstalled; + private boolean leftJoinPrefs; AppQuerySelection() { // The same as no selection, because "1" will always resolve to true when executing the SQL query. @@ -217,12 +219,25 @@ public class AppProvider extends FDroidProvider { return this; } + public boolean leftJoinToPrefs() { + return leftJoinPrefs; + } + + public AppQuerySelection requireLeftJoinPrefs() { + leftJoinPrefs = true; + return this; + } + public AppQuerySelection add(AppQuerySelection query) { QuerySelection both = super.add(query); AppQuerySelection bothWithJoin = new AppQuerySelection(both.getSelection(), both.getArgs()); if (this.naturalJoinToInstalled() || query.naturalJoinToInstalled()) { bothWithJoin.requireNaturalInstalledTable(); } + + if (this.leftJoinToPrefs() || query.leftJoinToPrefs()) { + bothWithJoin.requireLeftJoinPrefs(); + } return bothWithJoin; } @@ -232,6 +247,7 @@ public class AppProvider extends FDroidProvider { private boolean isSuggestedApkTableAdded; private boolean requiresInstalledTable; + private boolean requiresLeftJoinToPrefs; private boolean categoryFieldAdded; private boolean countFieldAppended; @@ -262,6 +278,9 @@ public class AppProvider extends FDroidProvider { if (selection.naturalJoinToInstalled()) { naturalJoinToInstalledTable(); } + if (selection.leftJoinToPrefs()) { + leftJoinToPrefs(); + } } // TODO: What if the selection requires a natural join, but we first get a left join @@ -276,6 +295,16 @@ public class AppProvider extends FDroidProvider { } } + public void leftJoinToPrefs() { + if (!requiresLeftJoinToPrefs) { + leftJoin( + AppPrefsTable.NAME, + "prefs", + "prefs." + AppPrefsTable.Cols.APP_ID + " = " + getTableName() + "." + Cols.ROW_ID); + requiresLeftJoinToPrefs = true; + } + } + public void leftJoinToInstalledTable() { if (!requiresInstalledTable) { leftJoin( @@ -527,11 +556,16 @@ public class AppProvider extends FDroidProvider { private AppQuerySelection queryCanUpdate() { final String app = getTableName(); - final String ignoreCurrent = app + "." + Cols.IGNORE_THISUPDATE + "!= " + app + "." + Cols.SUGGESTED_VERSION_CODE; - final String ignoreAll = app + "." + Cols.IGNORE_ALLUPDATES + " != 1"; + + // Need to use COALESCE because the prefs join may not resolve any rows, which means the + // ignore* fields will be NULL. In that case, we want to instead use a default value of 0. + final String ignoreCurrent = " COALESCE(prefs." + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ", 0) != " + app + "." + Cols.SUGGESTED_VERSION_CODE; + final String ignoreAll = "COALESCE(prefs." + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", 0) != 1"; + final String ignore = " (" + ignoreCurrent + " AND " + ignoreAll + ") "; final String where = ignore + " AND " + app + "." + Cols.SUGGESTED_VERSION_CODE + " > installed." + InstalledAppTable.Cols.VERSION_CODE; - return new AppQuerySelection(where).requireNaturalInstalledTable(); + + return new AppQuerySelection(where).requireNaturalInstalledTable().requireLeftJoinPrefs(); } private AppQuerySelection queryRepo(long repoId) { @@ -604,9 +638,9 @@ public class AppProvider extends FDroidProvider { private AppQuerySelection queryIgnored() { final String table = getTableName(); - final String selection = table + "." + Cols.IGNORE_ALLUPDATES + " = 1 OR " + - table + "." + Cols.IGNORE_THISUPDATE + " >= " + table + "." + Cols.SUGGESTED_VERSION_CODE; - return new AppQuerySelection(selection); + final String selection = "COALESCE(prefs." + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", 0) = 1 OR " + + "COALESCE(prefs." + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ", 0) >= " + table + "." + Cols.SUGGESTED_VERSION_CODE; + return new AppQuerySelection(selection).requireLeftJoinPrefs(); } private AppQuerySelection queryExcludeSwap() { 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 3aa8a338d..737ea3c66 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -96,8 +96,6 @@ class DBHelper extends SQLiteOpenHelper { + AppTable.Cols.ADDED + " string," + AppTable.Cols.LAST_UPDATED + " string," + AppTable.Cols.IS_COMPATIBLE + " int not null," - + AppTable.Cols.IGNORE_ALLUPDATES + " int not null," - + AppTable.Cols.IGNORE_THISUPDATE + " int not null," + AppTable.Cols.ICON_URL + " text, " + AppTable.Cols.ICON_URL_LARGE + " text, " + "primary key(" + AppTable.Cols.PACKAGE_NAME + "));"; diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java index 43e1dd177..fb7df077e 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java @@ -22,20 +22,6 @@ public class RepoPersister { private static final String TAG = "RepoPersister"; - /** - * When an app already exists in the db, and we are updating it on the off chance that some - * values changed in the index, some fields should not be updated. Rather, they should be - * ignored, because they were explicitly set by the user, and hence can't be automatically - * overridden by the index. - * - * NOTE: In the future, these attributes will be moved to a join table, so that the app table - * is essentially completely transient, and can be nuked at any time. - */ - private static final String[] APP_FIELDS_TO_IGNORE = { - Schema.AppTable.Cols.IGNORE_ALLUPDATES, - Schema.AppTable.Cols.IGNORE_THISUPDATE, - }; - /** * Crappy benchmark with a Nexus 4, Android 5.0 on a fairly crappy internet connection I get: * * 25 = 37 seconds @@ -219,13 +205,7 @@ public class RepoPersister { */ private ContentProviderOperation updateExistingApp(App app) { Uri uri = TempAppProvider.getAppUri(app); - ContentValues values = app.toContentValues(); - for (final String toIgnore : APP_FIELDS_TO_IGNORE) { - if (values.containsKey(toIgnore)) { - values.remove(toIgnore); - } - } - return ContentProviderOperation.newUpdate(uri).withValues(values).build(); + return ContentProviderOperation.newUpdate(uri).withValues(app.toContentValues()).build(); } /** diff --git a/app/src/main/java/org/fdroid/fdroid/data/Schema.java b/app/src/main/java/org/fdroid/fdroid/data/Schema.java index 7735e554c..c92567760 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -59,8 +59,6 @@ public interface Schema { String CATEGORIES = "categories"; String ANTI_FEATURES = "antiFeatures"; String REQUIREMENTS = "requirements"; - String IGNORE_ALLUPDATES = "ignoreAllUpdates"; - String IGNORE_THISUPDATE = "ignoreThisUpdate"; String ICON_URL = "iconUrl"; String ICON_URL_LARGE = "iconUrlLarge"; @@ -79,8 +77,7 @@ public interface Schema { LICENSE, AUTHOR, EMAIL, WEB_URL, TRACKER_URL, SOURCE_URL, CHANGELOG_URL, DONATE_URL, BITCOIN_ADDR, LITECOIN_ADDR, FLATTR_ID, UPSTREAM_VERSION_NAME, UPSTREAM_VERSION_CODE, ADDED, LAST_UPDATED, - CATEGORIES, ANTI_FEATURES, REQUIREMENTS, IGNORE_ALLUPDATES, - IGNORE_THISUPDATE, ICON_URL, ICON_URL_LARGE, + CATEGORIES, ANTI_FEATURES, REQUIREMENTS, ICON_URL, ICON_URL_LARGE, SUGGESTED_VERSION_CODE, SuggestedApk.VERSION_NAME, InstalledApp.VERSION_CODE, InstalledApp.VERSION_NAME, InstalledApp.SIGNATURE, diff --git a/app/src/main/java/org/fdroid/fdroid/views/AppListAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/AppListAdapter.java index a1f222dc0..17149c6a0 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/AppListAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/AppListAdapter.java @@ -123,7 +123,7 @@ public abstract class AppListAdapter extends CursorAdapter { final String installedVersionString = app.installedVersionName; - if (app.canAndWantToUpdate() && showStatusUpdate()) { + if (app.canAndWantToUpdate(mContext) && showStatusUpdate()) { return String.format(upgradeFromTo, installedVersionString, app.getSuggestedVersionName()); } diff --git a/app/src/main/java/org/fdroid/fdroid/views/fragments/AppListFragment.java b/app/src/main/java/org/fdroid/fdroid/views/fragments/AppListFragment.java index 1886af329..67e267288 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/fragments/AppListFragment.java +++ b/app/src/main/java/org/fdroid/fdroid/views/fragments/AppListFragment.java @@ -50,8 +50,6 @@ public abstract class AppListFragment extends ListFragment implements AppTable.Cols.InstalledApp.VERSION_NAME, AppTable.Cols.SuggestedApk.VERSION_NAME, AppTable.Cols.SUGGESTED_VERSION_CODE, - AppTable.Cols.IGNORE_ALLUPDATES, - AppTable.Cols.IGNORE_THISUPDATE, AppTable.Cols.REQUIREMENTS, // Needed for filtering apps that require root. }; diff --git a/app/src/test/java/org/fdroid/fdroid/Assert.java b/app/src/test/java/org/fdroid/fdroid/Assert.java index 40b59e71e..459a8f9ba 100644 --- a/app/src/test/java/org/fdroid/fdroid/Assert.java +++ b/app/src/test/java/org/fdroid/fdroid/Assert.java @@ -191,8 +191,6 @@ public class Assert { values.put(AppTable.Cols.DESCRIPTION, "test description"); values.put(AppTable.Cols.LICENSE, "GPL?"); values.put(AppTable.Cols.IS_COMPATIBLE, 1); - values.put(AppTable.Cols.IGNORE_ALLUPDATES, 0); - values.put(AppTable.Cols.IGNORE_THISUPDATE, 0); values.putAll(additionalValues); 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 170710365..c0c770f5c 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java @@ -89,9 +89,8 @@ public class AppProviderTest extends FDroidProviderTest { boolean ignoreAll, int ignoreVercode) { ContentValues values = new ContentValues(3); values.put(Cols.SUGGESTED_VERSION_CODE, suggestedVercode); - values.put(Cols.IGNORE_ALLUPDATES, ignoreAll); - values.put(Cols.IGNORE_THISUPDATE, ignoreVercode); - insertApp(packageName, "App: " + packageName, values); + App app = insertApp(packageName, "App: " + packageName, values); + AppPrefsProvider.Helper.update(context, app, new AppPrefs(ignoreVercode, ignoreAll)); InstalledAppTestUtils.install(context, packageName, installedVercode, "v" + installedVercode); } @@ -113,7 +112,7 @@ public class AppProviderTest extends FDroidProviderTest { // Can't "update", although can "install"... App notInstalled = AppProvider.Helper.findByPackageName(r, "not installed"); - assertFalse(notInstalled.canAndWantToUpdate()); + assertFalse(notInstalled.canAndWantToUpdate(context)); App installedOnlyOneVersionAvailable = AppProvider.Helper.findByPackageName(r, "installed, only one version available"); App installedAlreadyLatestNoIgnore = AppProvider.Helper.findByPackageName(r, "installed, already latest, no ignore"); @@ -121,21 +120,21 @@ public class AppProviderTest extends FDroidProviderTest { App installedAlreadyLatestIgnoreLatest = AppProvider.Helper.findByPackageName(r, "installed, already latest, ignore latest"); App installedAlreadyLatestIgnoreOld = AppProvider.Helper.findByPackageName(r, "installed, already latest, ignore old"); - assertFalse(installedOnlyOneVersionAvailable.canAndWantToUpdate()); - assertFalse(installedAlreadyLatestNoIgnore.canAndWantToUpdate()); - assertFalse(installedAlreadyLatestIgnoreAll.canAndWantToUpdate()); - assertFalse(installedAlreadyLatestIgnoreLatest.canAndWantToUpdate()); - assertFalse(installedAlreadyLatestIgnoreOld.canAndWantToUpdate()); + assertFalse(installedOnlyOneVersionAvailable.canAndWantToUpdate(context)); + assertFalse(installedAlreadyLatestNoIgnore.canAndWantToUpdate(context)); + assertFalse(installedAlreadyLatestIgnoreAll.canAndWantToUpdate(context)); + assertFalse(installedAlreadyLatestIgnoreLatest.canAndWantToUpdate(context)); + assertFalse(installedAlreadyLatestIgnoreOld.canAndWantToUpdate(context)); App installedOldNoIgnore = AppProvider.Helper.findByPackageName(r, "installed, old version, no ignore"); App installedOldIgnoreAll = AppProvider.Helper.findByPackageName(r, "installed, old version, ignore all"); App installedOldIgnoreLatest = AppProvider.Helper.findByPackageName(r, "installed, old version, ignore latest"); App installedOldIgnoreNewerNotLatest = AppProvider.Helper.findByPackageName(r, "installed, old version, ignore newer, but not latest"); - assertTrue(installedOldNoIgnore.canAndWantToUpdate()); - assertFalse(installedOldIgnoreAll.canAndWantToUpdate()); - assertFalse(installedOldIgnoreLatest.canAndWantToUpdate()); - assertTrue(installedOldIgnoreNewerNotLatest.canAndWantToUpdate()); + assertTrue(installedOldNoIgnore.canAndWantToUpdate(context)); + assertFalse(installedOldIgnoreAll.canAndWantToUpdate(context)); + assertFalse(installedOldIgnoreLatest.canAndWantToUpdate(context)); + assertTrue(installedOldIgnoreNewerNotLatest.canAndWantToUpdate(context)); Cursor canUpdateCursor = r.query(AppProvider.getCanUpdateUri(), Cols.ALL, null, null, null); assertNotNull(canUpdateCursor); @@ -348,7 +347,7 @@ public class AppProviderTest extends FDroidProviderTest { insertApp(id, name, values); } - public void insertApp(String id, String name, ContentValues additionalValues) { + public App insertApp(String id, String name, ContentValues additionalValues) { ContentValues values = new ContentValues(); values.put(Cols.PACKAGE_NAME, id); @@ -359,13 +358,12 @@ public class AppProviderTest extends FDroidProviderTest { values.put(Cols.DESCRIPTION, "test description"); values.put(Cols.LICENSE, "GPL?"); values.put(Cols.IS_COMPATIBLE, 1); - values.put(Cols.IGNORE_ALLUPDATES, 0); - values.put(Cols.IGNORE_THISUPDATE, 0); values.putAll(additionalValues); Uri uri = AppProvider.getContentUri(); contentResolver.insert(uri, values); + return AppProvider.Helper.findByPackageName(context.getContentResolver(), id); } } From 903048ffe4309c18150f9383d328def88209fde8 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 25 Jul 2016 18:25:50 +1000 Subject: [PATCH 03/10] Add covering indexes for main queries. With no indexes at all, a join between X and Y tables would require a full table scan of Y for each row in X. With an index on the relevant field in Y, it would require an index lookup on the join field in Y for each row in X, which contains a pointer to the row of interest in Y. This row is then looked up and the relevant value extracted. By using a covering index (one which includes all fields required to satisfy the query, with the first field being the one which is looked up in the join), then once the index has been searched, there is no need to then go to table Y because all the relevant data is already in the index. This offers a marginal performance improvement. --- .../java/org/fdroid/fdroid/data/DBHelper.java | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) 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 737ea3c66..00883bf1f 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -222,10 +222,12 @@ class DBHelper extends SQLiteOpenHelper { @Override public void onCreate(SQLiteDatabase db) { - createAppApk(db); + db.execSQL(CREATE_TABLE_APP); + db.execSQL(CREATE_TABLE_APK); db.execSQL(CREATE_TABLE_INSTALLED_APP); db.execSQL(CREATE_TABLE_REPO); db.execSQL(CREATE_TABLE_APP_PREFS); + ensureIndexes(db); insertRepo( db, @@ -683,10 +685,6 @@ class DBHelper extends SQLiteOpenHelper { db.execSQL("drop table " + AppTable.NAME); db.execSQL("drop table " + ApkTable.NAME); clearRepoEtags(db); - createAppApk(db); - } - - private static void createAppApk(SQLiteDatabase db) { db.execSQL(CREATE_TABLE_APP); db.execSQL(CREATE_TABLE_APK); ensureIndexes(db); @@ -702,6 +700,21 @@ class DBHelper extends SQLiteOpenHelper { db.execSQL("CREATE INDEX IF NOT EXISTS apk_vercode on " + ApkTable.NAME + " (" + ApkTable.Cols.VERSION_CODE + ");"); db.execSQL("CREATE INDEX IF NOT EXISTS apk_appId on " + ApkTable.NAME + " (" + ApkTable.Cols.APP_ID + ");"); db.execSQL("CREATE INDEX IF NOT EXISTS repoId ON " + ApkTable.NAME + " (" + ApkTable.Cols.REPO_ID + ");"); + + Utils.debugLog(TAG, "Ensuring indexes exist for " + AppPrefsTable.NAME); + db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_appId on " + AppPrefsTable.NAME + " (" + AppPrefsTable.Cols.APP_ID + ");"); + db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_appId_ignoreAll_ignoreThis on " + AppPrefsTable.NAME + " (" + + AppPrefsTable.Cols.APP_ID + ", " + + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", " + + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ");"); + + Utils.debugLog(TAG, "Ensuring indexes exist for " + InstalledAppTable.NAME); + db.execSQL("CREATE INDEX IF NOT EXISTS installedApp_appId_vercode on " + InstalledAppTable.NAME + " (" + + InstalledAppTable.Cols.PACKAGE_NAME + ", " + InstalledAppTable.Cols.VERSION_CODE + ");"); + + Utils.debugLog(TAG, "Ensuring indexes exist for " + RepoTable.NAME); + db.execSQL("CREATE INDEX IF NOT EXISTS repo_id_isSwap on " + RepoTable.NAME + " (" + + RepoTable.Cols._ID + ", " + RepoTable.Cols.IS_SWAP + ");"); } /** From 004c86bc42c250b475a8f7fd39e538c24cb7449d Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 25 Jul 2016 22:12:44 +1000 Subject: [PATCH 04/10] Notify content observers correctly --- .../java/org/fdroid/fdroid/data/AppPrefsProvider.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java index 5ce9fddf9..e4e0cf10c 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java @@ -151,9 +151,7 @@ public class AppPrefsProvider extends FDroidProvider { @Override public Uri insert(Uri uri, ContentValues values) { db().insertOrThrow(getTableName(), null, values); - if (!isApplyingBatch()) { - getContext().getContentResolver().notifyChange(uri, null); - } + getContext().getContentResolver().notifyChange(AppProvider.getCanUpdateUri(), null); return getAppUri(values.getAsLong(Cols.APP_ID)); } @@ -163,7 +161,9 @@ public class AppPrefsProvider extends FDroidProvider { case CODE_SINGLE: QuerySelection query = new QuerySelection(where, whereArgs) .add(querySingle(Long.parseLong(uri.getLastPathSegment()))); - return db().update(getTableName(), values, query.getSelection(), query.getArgs()); + int count = db().update(getTableName(), values, query.getSelection(), query.getArgs()); + getContext().getContentResolver().notifyChange(AppProvider.getCanUpdateUri(), null); + return count; default: throw new UnsupportedOperationException("Update not supported for " + uri + "."); From 5e263c0e0f704c0e5d51bb71d367e742d842d34c Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 25 Jul 2016 22:14:06 +1000 Subject: [PATCH 05/10] Use "COALESCE(x, 0)" instead of "x = 0 OR x IS NULL" This is a more concise syntax to say the same thing, and avoids an OR clause in the where - which is often the cause of slowness in many queries. Not sure if it was problematic in these cases, however this COALESCE syntax is still more consise. --- app/src/main/java/org/fdroid/fdroid/data/AppProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java index a0c312e2e..4432094e8 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -648,7 +648,7 @@ public class AppProvider extends FDroidProvider { // being no apks for the app in the result set. In that case, we can't tell if it is from // a swap repo or not. final String isSwap = RepoTable.NAME + "." + RepoTable.Cols.IS_SWAP; - final String selection = isSwap + " = 0 OR " + isSwap + " IS NULL"; + final String selection = "COALESCE(" + isSwap + ", 0) = 0"; return new AppQuerySelection(selection); } @@ -930,7 +930,7 @@ public class AppProvider extends FDroidProvider { " WHERE " + app + "." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + " AND " + " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + ApkTable.Cols.IS_COMPATIBLE + " = 1 ) ) " + - " WHERE " + Cols.UPSTREAM_VERSION_CODE + " = 0 OR " + Cols.UPSTREAM_VERSION_CODE + " IS NULL OR " + Cols.SUGGESTED_VERSION_CODE + " IS NULL "; + " WHERE COALESCE(" + Cols.UPSTREAM_VERSION_CODE + ", 0) = 0 OR " + Cols.SUGGESTED_VERSION_CODE + " IS NULL "; db().execSQL(updateSql); } From 4b5481b8f2522edc4019b9e4321f7c928f75346f Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 25 Jul 2016 22:28:51 +1000 Subject: [PATCH 06/10] Appese checkstyle + pmd --- .../main/java/org/fdroid/fdroid/AppDetails.java | 1 - .../java/org/fdroid/fdroid/data/AppPrefs.java | 9 +++++++-- .../java/org/fdroid/fdroid/data/DBHelper.java | 17 ++++++++++++++--- .../java/org/fdroid/fdroid/data/Schema.java | 2 +- .../fdroid/data/AppPrefsProviderTest.java | 6 +----- 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 7a683e756..f433ae184 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -26,7 +26,6 @@ import android.app.PendingIntent; import android.bluetooth.BluetoothAdapter; import android.content.ActivityNotFoundException; import android.content.BroadcastReceiver; -import android.content.ContentValues; import android.content.Context; import android.content.DialogInterface; import android.content.Intent; diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java b/app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java index cd3571177..1f76e1a6e 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppPrefs.java @@ -24,8 +24,13 @@ public class AppPrefs extends ValueObject { @Override public boolean equals(Object o) { return o != null && o instanceof AppPrefs && - ((AppPrefs)o).ignoreAllUpdates == ignoreAllUpdates && - ((AppPrefs)o).ignoreThisUpdate == ignoreThisUpdate; + ((AppPrefs) o).ignoreAllUpdates == ignoreAllUpdates && + ((AppPrefs) o).ignoreThisUpdate == ignoreThisUpdate; + } + + @Override + public int hashCode() { + return (ignoreThisUpdate + "-" + ignoreAllUpdates).hashCode(); } public AppPrefs createClone() { 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 00883bf1f..9c0e01b9d 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -103,7 +103,7 @@ class DBHelper extends SQLiteOpenHelper { private static final String CREATE_TABLE_APP_PREFS = "CREATE TABLE " + AppPrefsTable.NAME + " ( " + AppPrefsTable.Cols.APP_ID + " INT REFERENCES " + AppTable.NAME + "(" + AppTable.Cols.ROW_ID + ") ON DELETE CASCADE, " - + AppPrefsTable.Cols.IGNORE_THIS_UPDATE+ " INT BOOLEAN NOT NULL, " + + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + " INT BOOLEAN NOT NULL, " + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + " INT NOT NULL " + " );"; @@ -305,7 +305,7 @@ class DBHelper extends SQLiteOpenHelper { // The other tables are transient and can just be reset. Do this after // the repo table changes though, because it also clears the lastetag // fields which didn't always exist. - resetTransient(db, oldVersion); + resetTransientPre42(db, oldVersion); addNameAndDescriptionToRepo(db, oldVersion); addFingerprintToRepo(db, oldVersion); @@ -672,6 +672,17 @@ class DBHelper extends SQLiteOpenHelper { } private void resetTransient(SQLiteDatabase db, int oldVersion) { + context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit() + .putBoolean("triedEmptyUpdate", false).apply(); + db.execSQL("drop table " + AppTable.NAME); + db.execSQL("drop table " + ApkTable.NAME); + clearRepoEtags(db); + db.execSQL(CREATE_TABLE_APP); + db.execSQL(CREATE_TABLE_APK); + ensureIndexes(db); + } + + private void resetTransientPre42(SQLiteDatabase db, int oldVersion) { // Before version 42, only transient info was stored in here. As of some time // just before 42 (F-Droid 0.60ish) it now has "ignore this version" info which // was is specified by the user. We don't want to weely-neely nuke that data. @@ -681,7 +692,7 @@ class DBHelper extends SQLiteOpenHelper { return; } context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit() - .putBoolean("triedEmptyUpdate", false).commit(); + .putBoolean("triedEmptyUpdate", false).apply(); db.execSQL("drop table " + AppTable.NAME); db.execSQL("drop table " + ApkTable.NAME); clearRepoEtags(db); diff --git a/app/src/main/java/org/fdroid/fdroid/data/Schema.java b/app/src/main/java/org/fdroid/fdroid/data/Schema.java index c92567760..fd5a1ec48 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -18,7 +18,7 @@ public interface Schema { String IGNORE_ALL_UPDATES = "ignoreAllUpdates"; String IGNORE_THIS_UPDATE = "ignoreThisUpdate"; - String[] ALL = {APP_ID, IGNORE_ALL_UPDATES, IGNORE_THIS_UPDATE,}; + String[] ALL = {APP_ID, IGNORE_ALL_UPDATES, IGNORE_THIS_UPDATE}; } } 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 81e18de0b..fb42642a8 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java @@ -4,8 +4,6 @@ import android.app.Application; import org.fdroid.fdroid.Assert; import org.fdroid.fdroid.BuildConfig; -import org.fdroid.fdroid.TestUtils; -import org.fdroid.fdroid.data.Schema.AppPrefsTable.Cols; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -13,7 +11,7 @@ import org.robolectric.RobolectricGradleTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowContentResolver; -import static junit.framework.Assert.assertTrue; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -22,8 +20,6 @@ import static org.junit.Assert.assertNull; @RunWith(RobolectricGradleTestRunner.class) public class AppPrefsProviderTest extends FDroidProviderTest { - private static final String[] PROJ = Cols.ALL; - @Before public void setup() { ShadowContentResolver.registerProvider(AppProvider.getAuthority(), new AppProvider()); From 125acd6276f80aa07d29969486384188340a8367 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 25 Jul 2016 22:40:20 +1000 Subject: [PATCH 07/10] Migrate app preferences to different table. In the process, realised that using appId as a foreign key is worse than packageName, because appId can get removed and added again, but it will be different when the same app is inserted a second time. In order to maintain the association of which apps have preferences stored against them, they need to be stored against something with a bit more semantic meaning. Thus, join onto package name instead. --- .../fdroid/fdroid/data/AppPrefsProvider.java | 26 +++---- .../org/fdroid/fdroid/data/AppProvider.java | 2 +- .../java/org/fdroid/fdroid/data/DBHelper.java | 69 ++++++++++++++----- .../java/org/fdroid/fdroid/data/Schema.java | 8 ++- 4 files changed, 72 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java index e4e0cf10c..7ed4cf2f2 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java @@ -25,10 +25,10 @@ public class AppPrefsProvider extends FDroidProvider { values.put(Cols.IGNORE_THIS_UPDATE, prefs.ignoreThisUpdate); if (getPrefsOrNull(context, app) == null) { - values.put(Cols.APP_ID, app.getId()); + values.put(Cols.PACKAGE_NAME, app.packageName); context.getContentResolver().insert(getContentUri(), values); } else { - context.getContentResolver().update(getAppUri(app.getId()), values, null, null); + context.getContentResolver().update(getAppUri(app.packageName), values, null, null); } } @@ -40,7 +40,7 @@ public class AppPrefsProvider extends FDroidProvider { @Nullable public static AppPrefs getPrefsOrNull(Context context, App app) { - Cursor cursor = context.getContentResolver().query(getAppUri(app.getId()), Cols.ALL, null, null, null); + Cursor cursor = context.getContentResolver().query(getAppUri(app.packageName), Cols.ALL, null, null, null); if (cursor == null) { return null; } @@ -77,18 +77,18 @@ public class AppPrefsProvider extends FDroidProvider { private static final UriMatcher MATCHER = new UriMatcher(-1); - private static final String PATH_APP_ID = "appId"; + private static final String PATH_PACKAGE_NAME = "packageName"; static { - MATCHER.addURI(getAuthority(), PATH_APP_ID + "/#", CODE_SINGLE); + MATCHER.addURI(getAuthority(), PATH_PACKAGE_NAME + "/*", CODE_SINGLE); } private static Uri getContentUri() { return Uri.parse("content://" + getAuthority()); } - public static Uri getAppUri(long appId) { - return getContentUri().buildUpon().appendPath(PATH_APP_ID).appendPath(Long.toString(appId)).build(); + public static Uri getAppUri(String packageName) { + return getContentUri().buildUpon().appendPath(PATH_PACKAGE_NAME).appendPath(packageName).build(); } @Override @@ -110,9 +110,9 @@ public class AppPrefsProvider extends FDroidProvider { return MATCHER; } - protected QuerySelection querySingle(long appId) { - final String selection = getTableName() + "." + Cols.APP_ID + " = ?"; - final String[] args = {Long.toString(appId)}; + protected QuerySelection querySingle(String packageName) { + final String selection = getTableName() + "." + Cols.PACKAGE_NAME + " = ?"; + final String[] args = {packageName}; return new QuerySelection(selection, args); } @@ -122,7 +122,7 @@ public class AppPrefsProvider extends FDroidProvider { switch (MATCHER.match(uri)) { case CODE_SINGLE: - selection = selection.add(querySingle(Long.parseLong(uri.getLastPathSegment()))); + selection = selection.add(querySingle(uri.getLastPathSegment())); break; default: @@ -152,7 +152,7 @@ public class AppPrefsProvider extends FDroidProvider { public Uri insert(Uri uri, ContentValues values) { db().insertOrThrow(getTableName(), null, values); getContext().getContentResolver().notifyChange(AppProvider.getCanUpdateUri(), null); - return getAppUri(values.getAsLong(Cols.APP_ID)); + return getAppUri(values.getAsString(Cols.PACKAGE_NAME)); } @Override @@ -160,7 +160,7 @@ public class AppPrefsProvider extends FDroidProvider { switch (MATCHER.match(uri)) { case CODE_SINGLE: QuerySelection query = new QuerySelection(where, whereArgs) - .add(querySingle(Long.parseLong(uri.getLastPathSegment()))); + .add(querySingle(uri.getLastPathSegment())); int count = db().update(getTableName(), values, query.getSelection(), query.getArgs()); getContext().getContentResolver().notifyChange(AppProvider.getCanUpdateUri(), null); return count; diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java index 4432094e8..8ff64b039 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -300,7 +300,7 @@ public class AppProvider extends FDroidProvider { leftJoin( AppPrefsTable.NAME, "prefs", - "prefs." + AppPrefsTable.Cols.APP_ID + " = " + getTableName() + "." + Cols.ROW_ID); + "prefs." + AppPrefsTable.Cols.PACKAGE_NAME + " = " + getTableName() + "." + Cols.PACKAGE_NAME); requiresLeftJoinToPrefs = true; } } 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 9c0e01b9d..e97ffd63a 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -102,7 +102,7 @@ class DBHelper extends SQLiteOpenHelper { private static final String CREATE_TABLE_APP_PREFS = "CREATE TABLE " + AppPrefsTable.NAME + " ( " - + AppPrefsTable.Cols.APP_ID + " INT REFERENCES " + AppTable.NAME + "(" + AppTable.Cols.ROW_ID + ") ON DELETE CASCADE, " + + AppPrefsTable.Cols.PACKAGE_NAME + " TEXT, " + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + " INT BOOLEAN NOT NULL, " + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + " INT NOT NULL " + " );"; @@ -120,7 +120,7 @@ class DBHelper extends SQLiteOpenHelper { + " );"; private static final String DROP_TABLE_INSTALLED_APP = "DROP TABLE " + InstalledAppTable.NAME + ";"; - private static final int DB_VERSION = 59; + private static final int DB_VERSION = 60; private final Context context; @@ -326,6 +326,31 @@ class DBHelper extends SQLiteOpenHelper { addTargetSdkVersionToApk(db, oldVersion); migrateAppPrimaryKeyToRowId(db, oldVersion); removeApkPackageNameColumn(db, oldVersion); + addAppPrefsTable(db, oldVersion); + } + + private void addAppPrefsTable(SQLiteDatabase db, int oldVersion) { + if (oldVersion < 60) { + + Utils.debugLog(TAG, "Creating app preferences table"); + db.execSQL(CREATE_TABLE_APP_PREFS); + + Utils.debugLog(TAG, "Migrating app preferences to separate table"); + db.execSQL( + "INSERT INTO " + AppPrefsTable.NAME + " (" + + AppPrefsTable.Cols.PACKAGE_NAME + ", " + + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ", " + + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + + ") SELECT " + + AppTable.Cols.PACKAGE_NAME + ", " + + "ignoreThisUpdate, " + + "ignoreAllUpdates " + + "FROM " + AppTable.NAME + " " + + "WHERE ignoreThisUpdate > 0 OR ignoreAllUpdates > 0" + ); + + resetTransient(db); + } } /** @@ -666,19 +691,23 @@ class DBHelper extends SQLiteOpenHelper { * their repos (either manually or on a scheduled task), they will update regardless of whether * they have changed since last update or not. */ - private void clearRepoEtags(SQLiteDatabase db) { + private static void clearRepoEtags(SQLiteDatabase db) { Utils.debugLog(TAG, "Clearing repo etags, so next update will not be skipped with \"Repos up to date\"."); db.execSQL("update " + RepoTable.NAME + " set " + RepoTable.Cols.LAST_ETAG + " = NULL"); } - private void resetTransient(SQLiteDatabase db, int oldVersion) { - context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit() - .putBoolean("triedEmptyUpdate", false).apply(); - db.execSQL("drop table " + AppTable.NAME); - db.execSQL("drop table " + ApkTable.NAME); - clearRepoEtags(db); + private void resetTransient(SQLiteDatabase db) { + Utils.debugLog(TAG, "Removing app + apk tables so they can be recreated. Next time F-Droid updates it should trigger an index update."); + context.getSharedPreferences("FDroid", Context.MODE_PRIVATE) + .edit() + .putBoolean("triedEmptyUpdate", false) + .apply(); + + db.execSQL("DROP TABLE " + AppTable.NAME); + db.execSQL("DROP TABLE " + ApkTable.NAME); db.execSQL(CREATE_TABLE_APP); db.execSQL(CREATE_TABLE_APK); + clearRepoEtags(db); ensureIndexes(db); } @@ -712,12 +741,14 @@ class DBHelper extends SQLiteOpenHelper { db.execSQL("CREATE INDEX IF NOT EXISTS apk_appId on " + ApkTable.NAME + " (" + ApkTable.Cols.APP_ID + ");"); db.execSQL("CREATE INDEX IF NOT EXISTS repoId ON " + ApkTable.NAME + " (" + ApkTable.Cols.REPO_ID + ");"); - Utils.debugLog(TAG, "Ensuring indexes exist for " + AppPrefsTable.NAME); - db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_appId on " + AppPrefsTable.NAME + " (" + AppPrefsTable.Cols.APP_ID + ");"); - db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_appId_ignoreAll_ignoreThis on " + AppPrefsTable.NAME + " (" + - AppPrefsTable.Cols.APP_ID + ", " + - AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", " + - AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ");"); + if (tableExists(db, AppPrefsTable.NAME)) { + Utils.debugLog(TAG, "Ensuring indexes exist for " + AppPrefsTable.NAME); + db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_packageName on " + AppPrefsTable.NAME + " (" + AppPrefsTable.Cols.PACKAGE_NAME + ");"); + db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_packageName_ignoreAll_ignoreThis on " + AppPrefsTable.NAME + " (" + + AppPrefsTable.Cols.PACKAGE_NAME + ", " + + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", " + + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ");"); + } Utils.debugLog(TAG, "Ensuring indexes exist for " + InstalledAppTable.NAME); db.execSQL("CREATE INDEX IF NOT EXISTS installedApp_appId_vercode on " + InstalledAppTable.NAME + " (" + @@ -753,10 +784,14 @@ class DBHelper extends SQLiteOpenHelper { + ApkTable.Cols.TARGET_SDK_VERSION + " integer"); } - private static boolean columnExists(SQLiteDatabase db, - String table, String column) { + private static boolean columnExists(SQLiteDatabase db, String table, String column) { return db.rawQuery("select * from " + table + " limit 0,1", null) .getColumnIndex(column) != -1; } + private static boolean tableExists(SQLiteDatabase db, String table) { + return db.rawQuery("SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?", + new String[] {table}).getCount() > 0; + } + } diff --git a/app/src/main/java/org/fdroid/fdroid/data/Schema.java b/app/src/main/java/org/fdroid/fdroid/data/Schema.java index fd5a1ec48..dfef154c0 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -14,11 +14,15 @@ public interface Schema { String NAME = "fdroid_appPrefs"; interface Cols extends BaseColumns { - String APP_ID = "appId"; + // Join onto app table via packageName, not appId. The corresponding app row could + // be deleted and then re-added in the future with the same metadata but a different + // rowid. This should not cause us to forget the preferences specified by a user. + String PACKAGE_NAME = "packageName"; + String IGNORE_ALL_UPDATES = "ignoreAllUpdates"; String IGNORE_THIS_UPDATE = "ignoreThisUpdate"; - String[] ALL = {APP_ID, IGNORE_ALL_UPDATES, IGNORE_THIS_UPDATE}; + String[] ALL = {PACKAGE_NAME, IGNORE_ALL_UPDATES, IGNORE_THIS_UPDATE}; } } From 9637de5e4c31ef19ae7bbca8c19b3d34f692353a Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 28 Jul 2016 11:15:23 +1000 Subject: [PATCH 08/10] Make ignored app tests actually test code in use. The test was using a `findIgnored` method in `AppProvider`, which only existed for the purpose of testing. The test has been changed to instead check for apps which would end up in the "can update" list (which is really where the "ignored" apps are useful). --- .../org/fdroid/fdroid/data/AppProvider.java | 30 ++++--------------- .../fdroid/data/AppPrefsProviderTest.java | 3 +- .../fdroid/fdroid/data/AppProviderTest.java | 28 ++++++++++------- 3 files changed, 24 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java index 8ff64b039..4e90d4550 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -59,12 +59,6 @@ public class AppProvider extends FDroidProvider { return cursorToList(cursor); } - public static List findIgnored(Context context, String[] projection) { - final Uri uri = AppProvider.getIgnoredUri(); - Cursor cursor = context.getContentResolver().query(uri, projection, null, null, null); - return cursorToList(cursor); - } - static List cursorToList(Cursor cursor) { int knownAppCount = cursor != null ? cursor.getCount() : 0; List apps = new ArrayList<>(knownAppCount); @@ -154,6 +148,10 @@ public class AppProvider extends FDroidProvider { final Uri fromUpstream = calcAppDetailsFromIndexUri(); context.getContentResolver().update(fromUpstream, null, null, null); } + + public static List findCanUpdate(Context context, String[] projection) { + return cursorToList(context.getContentResolver().query(AppProvider.getCanUpdateUri(), projection, null, null, null)); + } } /** @@ -406,7 +404,6 @@ public class AppProvider extends FDroidProvider { private static final String PATH_RECENTLY_UPDATED = "recentlyUpdated"; private static final String PATH_NEWLY_ADDED = "newlyAdded"; private static final String PATH_CATEGORY = "category"; - private static final String PATH_IGNORED = "ignored"; private static final String PATH_CALC_APP_DETAILS_FROM_INDEX = "calcDetailsFromIndex"; private static final String PATH_REPO = "repo"; @@ -417,8 +414,7 @@ public class AppProvider extends FDroidProvider { private static final int RECENTLY_UPDATED = NO_APKS + 1; private static final int NEWLY_ADDED = RECENTLY_UPDATED + 1; private static final int CATEGORY = NEWLY_ADDED + 1; - private static final int IGNORED = CATEGORY + 1; - private static final int CALC_APP_DETAILS_FROM_INDEX = IGNORED + 1; + private static final int CALC_APP_DETAILS_FROM_INDEX = CATEGORY + 1; private static final int REPO = CALC_APP_DETAILS_FROM_INDEX + 1; private static final int SEARCH_REPO = REPO + 1; private static final int SEARCH_INSTALLED = SEARCH_REPO + 1; @@ -427,7 +423,6 @@ public class AppProvider extends FDroidProvider { static { MATCHER.addURI(getAuthority(), null, CODE_LIST); MATCHER.addURI(getAuthority(), PATH_CALC_APP_DETAILS_FROM_INDEX, CALC_APP_DETAILS_FROM_INDEX); - MATCHER.addURI(getAuthority(), PATH_IGNORED, IGNORED); MATCHER.addURI(getAuthority(), PATH_RECENTLY_UPDATED, RECENTLY_UPDATED); MATCHER.addURI(getAuthority(), PATH_NEWLY_ADDED, NEWLY_ADDED); MATCHER.addURI(getAuthority(), PATH_CATEGORY + "/*", CATEGORY); @@ -454,10 +449,6 @@ public class AppProvider extends FDroidProvider { return Uri.withAppendedPath(getContentUri(), PATH_NEWLY_ADDED); } - public static Uri getIgnoredUri() { - return Uri.withAppendedPath(getContentUri(), PATH_IGNORED); - } - private static Uri calcAppDetailsFromIndexUri() { return Uri.withAppendedPath(getContentUri(), PATH_CALC_APP_DETAILS_FROM_INDEX); } @@ -636,13 +627,6 @@ public class AppProvider extends FDroidProvider { return new AppQuerySelection(selection, args); } - private AppQuerySelection queryIgnored() { - final String table = getTableName(); - final String selection = "COALESCE(prefs." + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", 0) = 1 OR " + - "COALESCE(prefs." + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ", 0) >= " + table + "." + Cols.SUGGESTED_VERSION_CODE; - return new AppQuerySelection(selection).requireLeftJoinPrefs(); - } - private AppQuerySelection queryExcludeSwap() { // fdroid_repo will have null fields if the LEFT JOIN didn't resolve, e.g. due to there // being no apks for the app in the result set. In that case, we can't tell if it is from @@ -751,10 +735,6 @@ public class AppProvider extends FDroidProvider { selection = selection.add(queryNoApks()); break; - case IGNORED: - selection = selection.add(queryIgnored()); - break; - case CATEGORY: selection = selection.add(queryCategory(uri.getLastPathSegment())); includeSwap = false; 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 fb42642a8..128f52cb2 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java @@ -16,7 +16,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; -@Config(constants = BuildConfig.class, application = Application.class) +// TODO: Use sdk=24 when Robolectric supports this +@Config(constants = BuildConfig.class, application = Application.class, sdk = 23) @RunWith(RobolectricGradleTestRunner.class) public class AppPrefsProviderTest extends FDroidProviderTest { 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 c0c770f5c..f4cb5e01b 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java @@ -170,21 +170,27 @@ public class AppProviderTest extends FDroidProviderTest { assertResultCount(contentResolver, 10, AppProvider.getContentUri(), PROJ); String[] projection = {Cols.PACKAGE_NAME}; - List ignoredApps = AppProvider.Helper.findIgnored(context, projection); + List canUpdateApps = AppProvider.Helper.findCanUpdate(context, projection); - String[] expectedIgnored = { - "installed, already latest, ignore all", - "installed, already latest, ignore latest", - // NOT "installed, already latest, ignore old" - because it - // is should only ignore if "ignored version" is >= suggested + String[] expectedCanUpdate = { + "installed, old version, no ignore", + "installed, old version, ignore newer, but not latest", + + // These are ignored because they don't have updates available: + // "installed, only one version available", + // "installed, already latest, no ignore", + // "installed, already latest, ignore old", + // "not installed", + + // These four should be ignored due to the app preferences: + // "installed, already latest, ignore all", + // "installed, already latest, ignore latest", + // "installed, old version, ignore all", + // "installed, old version, ignore latest", - "installed, old version, ignore all", - "installed, old version, ignore latest", - // NOT "installed, old version, ignore newer, but not latest" - // for the same reason as above. }; - assertContainsOnlyIds(ignoredApps, expectedIgnored); + assertContainsOnlyIds(canUpdateApps, expectedCanUpdate); } private void assertContainsOnlyIds(List actualApps, String[] expectedIds) { From bb88be9403bf38dd16a1bd060cad4d4a121c9694 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 4 Aug 2016 10:50:22 +1000 Subject: [PATCH 09/10] Further tests for AppPrefs.equals() which is used by AppDetails. --- .../fdroid/fdroid/data/AppPrefsProviderTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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 128f52cb2..150e99f99 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppPrefsProviderTest.java @@ -26,6 +26,21 @@ public class AppPrefsProviderTest extends FDroidProviderTest { ShadowContentResolver.registerProvider(AppProvider.getAuthority(), new AppProvider()); } + @SuppressWarnings({"PMD.EqualsNull", "EqualsWithItself", "EqualsBetweenInconvertibleTypes", "ObjectEqualsNull"}) + @Test + public void prefEquality() { + AppPrefs original = new AppPrefs(101, true); + + assertTrue(original.equals(new AppPrefs(101, true))); + assertTrue(original.equals(original)); + + assertFalse(original.equals(null)); + assertFalse(original.equals("String")); + assertFalse(original.equals(new AppPrefs(102, true))); + assertFalse(original.equals(new AppPrefs(101, false))); + assertFalse(original.equals(new AppPrefs(100, false))); + } + @Test public void newPreferences() { App withPrefs = Assert.insertApp(context, "com.example.withPrefs", "With Prefs"); From 203bcda69570ba4819da802852c057dad9a119d2 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 4 Aug 2016 21:35:46 +1000 Subject: [PATCH 10/10] Cleanup in response to CR comments --- .../fdroid/fdroid/data/AppPrefsProvider.java | 51 +++++++------------ .../java/org/fdroid/fdroid/data/DBHelper.java | 41 +++++++-------- 2 files changed, 39 insertions(+), 53 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java index 7ed4cf2f2..f45f244ff 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java @@ -7,15 +7,12 @@ import android.database.Cursor; import android.net.Uri; import android.support.annotation.NonNull; import android.support.annotation.Nullable; -import android.util.Log; import org.fdroid.fdroid.data.Schema.AppPrefsTable; import org.fdroid.fdroid.data.Schema.AppPrefsTable.Cols; public class AppPrefsProvider extends FDroidProvider { - private static final String TAG = "AppPrefsProvider"; - public static final class Helper { private Helper() { } @@ -48,12 +45,12 @@ public class AppPrefsProvider extends FDroidProvider { try { if (cursor.getCount() == 0) { return null; - } else { - cursor.moveToFirst(); - return new AppPrefs( - cursor.getInt(cursor.getColumnIndexOrThrow(Cols.IGNORE_THIS_UPDATE)), - cursor.getInt(cursor.getColumnIndexOrThrow(Cols.IGNORE_ALL_UPDATES)) > 0); } + + cursor.moveToFirst(); + return new AppPrefs( + cursor.getInt(cursor.getColumnIndexOrThrow(Cols.IGNORE_THIS_UPDATE)), + cursor.getInt(cursor.getColumnIndexOrThrow(Cols.IGNORE_ALL_UPDATES)) > 0); } finally { cursor.close(); } @@ -118,18 +115,13 @@ public class AppPrefsProvider extends FDroidProvider { @Override public Cursor query(Uri uri, String[] projection, String customSelection, String[] selectionArgs, String sortOrder) { - QuerySelection selection = new QuerySelection(customSelection, selectionArgs); - - switch (MATCHER.match(uri)) { - case CODE_SINGLE: - selection = selection.add(querySingle(uri.getLastPathSegment())); - break; - - default: - Log.e(TAG, "Invalid URI for app content provider: " + uri); - throw new UnsupportedOperationException("Invalid URI for app content provider: " + uri); + if (MATCHER.match(uri) != CODE_SINGLE) { + throw new UnsupportedOperationException("Invalid URI for app content provider: " + uri); } + QuerySelection selection = new QuerySelection(customSelection, selectionArgs) + .add(querySingle(uri.getLastPathSegment())); + Query query = new Query(); query.addSelection(selection); query.addFields(projection); @@ -142,10 +134,7 @@ public class AppPrefsProvider extends FDroidProvider { @Override public int delete(Uri uri, String where, String[] whereArgs) { - switch (MATCHER.match(uri)) { - default: - throw new UnsupportedOperationException("Delete not supported for " + uri + "."); - } + throw new UnsupportedOperationException("Delete not supported for " + uri + "."); } @Override @@ -157,17 +146,13 @@ public class AppPrefsProvider extends FDroidProvider { @Override public int update(Uri uri, ContentValues values, String where, String[] whereArgs) { - switch (MATCHER.match(uri)) { - case CODE_SINGLE: - QuerySelection query = new QuerySelection(where, whereArgs) - .add(querySingle(uri.getLastPathSegment())); - int count = db().update(getTableName(), values, query.getSelection(), query.getArgs()); - getContext().getContentResolver().notifyChange(AppProvider.getCanUpdateUri(), null); - return count; - - default: - throw new UnsupportedOperationException("Update not supported for " + uri + "."); - + if (MATCHER.match(uri) != CODE_SINGLE) { + throw new UnsupportedOperationException("Update not supported for " + uri + "."); } + + QuerySelection query = new QuerySelection(where, whereArgs).add(querySingle(uri.getLastPathSegment())); + int count = db().update(getTableName(), values, query.getSelection(), query.getArgs()); + getContext().getContentResolver().notifyChange(AppProvider.getCanUpdateUri(), null); + return count; } } 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 e97ffd63a..2c0fcb24c 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -330,27 +330,28 @@ class DBHelper extends SQLiteOpenHelper { } private void addAppPrefsTable(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 60) { - - Utils.debugLog(TAG, "Creating app preferences table"); - db.execSQL(CREATE_TABLE_APP_PREFS); - - Utils.debugLog(TAG, "Migrating app preferences to separate table"); - db.execSQL( - "INSERT INTO " + AppPrefsTable.NAME + " (" - + AppPrefsTable.Cols.PACKAGE_NAME + ", " - + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ", " - + AppPrefsTable.Cols.IGNORE_ALL_UPDATES - + ") SELECT " - + AppTable.Cols.PACKAGE_NAME + ", " - + "ignoreThisUpdate, " - + "ignoreAllUpdates " - + "FROM " + AppTable.NAME + " " - + "WHERE ignoreThisUpdate > 0 OR ignoreAllUpdates > 0" - ); - - resetTransient(db); + if (oldVersion >= 60) { + return; } + + Utils.debugLog(TAG, "Creating app preferences table"); + db.execSQL(CREATE_TABLE_APP_PREFS); + + Utils.debugLog(TAG, "Migrating app preferences to separate table"); + db.execSQL( + "INSERT INTO " + AppPrefsTable.NAME + " (" + + AppPrefsTable.Cols.PACKAGE_NAME + ", " + + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ", " + + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + + ") SELECT " + + AppTable.Cols.PACKAGE_NAME + ", " + + "ignoreThisUpdate, " + + "ignoreAllUpdates " + + "FROM " + AppTable.NAME + " " + + "WHERE ignoreThisUpdate > 0 OR ignoreAllUpdates > 0" + ); + + resetTransient(db); } /**