From fd50a2c730490292a49d96aaa31ebc14b3a9e5a6 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 11 Aug 2016 23:02:59 +1000 Subject: [PATCH 1/4] Remove unused code. The code only existed so that it could be used in a test. Subsequently, a further test was written to test this code (used by the first test). Since none of the code is actually used in the app, it has been removed. --- .../org/fdroid/fdroid/data/ApkProvider.java | 24 -------- .../fdroid/fdroid/data/ApkProviderTest.java | 61 ------------------- .../java/org/fdroid/fdroid/mock/MockApp.java | 16 ----- 3 files changed, 101 deletions(-) delete mode 100644 app/src/test/java/org/fdroid/fdroid/mock/MockApp.java diff --git a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java index d3ebecc6f..176bb03cc 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java @@ -62,30 +62,6 @@ public class ApkProvider extends FDroidProvider { return resolver.delete(uri, null, null); } - public static void deleteApksByApp(Context context, App app) { - ContentResolver resolver = context.getContentResolver(); - final Uri uri = getAppUri(app.packageName); - resolver.delete(uri, null, null); - } - - public static void deleteApks(final Context context, final List apks) { - if (apks.size() > ApkProvider.MAX_APKS_TO_QUERY) { - int middle = apks.size() / 2; - List apks1 = apks.subList(0, middle); - List apks2 = apks.subList(middle, apks.size()); - deleteApks(context, apks1); - deleteApks(context, apks2); - } else { - deleteApksSafely(context, apks); - } - } - - private static void deleteApksSafely(final Context context, final List apks) { - ContentResolver resolver = context.getContentResolver(); - final Uri uri = getContentUri(apks); - resolver.delete(uri, null, null); - } - public static Apk find(Context context, String packageName, int versionCode) { return find(context, packageName, versionCode, Cols.ALL); } diff --git a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java index b0aac5a04..65b738045 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java @@ -10,7 +10,6 @@ import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.data.Schema.ApkTable.Cols; import org.fdroid.fdroid.data.Schema.RepoTable; import org.fdroid.fdroid.mock.MockApk; -import org.fdroid.fdroid.mock.MockApp; import org.fdroid.fdroid.mock.MockRepo; import org.junit.Test; import org.junit.runner.RunWith; @@ -23,7 +22,6 @@ import java.util.Date; import java.util.List; import static org.fdroid.fdroid.Assert.assertCantDelete; -import static org.fdroid.fdroid.Assert.assertContainsOnly; import static org.fdroid.fdroid.Assert.assertResultCount; import static org.fdroid.fdroid.Assert.insertApp; import static org.junit.Assert.assertEquals; @@ -64,65 +62,6 @@ public class ApkProviderTest extends FDroidProviderTest { assertResultCount(10, exampleApks); assertBelongsToApp(exampleApks, "com.example"); exampleApks.close(); - - ApkProvider.Helper.deleteApksByApp(context, new MockApp("com.example")); - - Cursor all = queryAllApks(); - assertResultCount(10, all); - assertBelongsToApp(all, "org.fdroid.fdroid"); - all.close(); - } - - @Test - public void testDeleteArbitraryApks() { - Apk one = insertApkForRepo("com.example.one", 1, 10); - Apk two = insertApkForRepo("com.example.two", 1, 10); - Apk three = insertApkForRepo("com.example.three", 1, 10); - Apk four = insertApkForRepo("com.example.four", 1, 10); - Apk five = insertApkForRepo("com.example.five", 1, 10); - - assertTotalApkCount(5); - - assertEquals("com.example.one", one.packageName); - assertEquals("com.example.two", two.packageName); - assertEquals("com.example.five", five.packageName); - - String[] expectedIds = { - "com.example.one", - "com.example.two", - "com.example.three", - "com.example.four", - "com.example.five", - }; - - List all = ApkProvider.Helper.findByRepo(context, new MockRepo(10), Cols.ALL); - List actualIds = new ArrayList<>(); - for (Apk apk : all) { - actualIds.add(apk.packageName); - } - - assertContainsOnly(actualIds, expectedIds); - - List toDelete = new ArrayList<>(3); - toDelete.add(two); - toDelete.add(three); - toDelete.add(four); - ApkProvider.Helper.deleteApks(context, toDelete); - - assertTotalApkCount(2); - - List allRemaining = ApkProvider.Helper.findByRepo(context, new MockRepo(10), Cols.ALL); - List actualRemainingIds = new ArrayList<>(); - for (Apk apk : allRemaining) { - actualRemainingIds.add(apk.packageName); - } - - String[] expectedRemainingIds = { - "com.example.one", - "com.example.five", - }; - - assertContainsOnly(actualRemainingIds, expectedRemainingIds); } @Test diff --git a/app/src/test/java/org/fdroid/fdroid/mock/MockApp.java b/app/src/test/java/org/fdroid/fdroid/mock/MockApp.java deleted file mode 100644 index 39e649981..000000000 --- a/app/src/test/java/org/fdroid/fdroid/mock/MockApp.java +++ /dev/null @@ -1,16 +0,0 @@ -package org.fdroid.fdroid.mock; - -import org.fdroid.fdroid.data.App; - -public class MockApp extends App { - - public MockApp(String id) { - this(id, "App " + id); - } - - public MockApp(String id, String name) { - this.packageName = id; - this.name = name; - } - -} From 005d1098183958e32278444f6d86f9055c1b1a10 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 11 Aug 2016 23:10:11 +1000 Subject: [PATCH 2/4] Clean up switch statements with only a single option. --- .../org/fdroid/fdroid/data/AppProvider.java | 18 +++------- .../fdroid/fdroid/data/TempApkProvider.java | 34 +++++++------------ .../fdroid/fdroid/data/TempAppProvider.java | 12 +++---- 3 files changed, 20 insertions(+), 44 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 6b7707192..db5874eb9 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -785,22 +785,12 @@ public class AppProvider extends FDroidProvider { @Override public int delete(Uri uri, String where, String[] whereArgs) { - - QuerySelection query = new QuerySelection(where, whereArgs); - switch (MATCHER.match(uri)) { - - case NO_APKS: - query = query.add(queryNoApks()); - break; - - default: - throw new UnsupportedOperationException("Delete not supported for " + uri + "."); - + if (MATCHER.match(uri) != NO_APKS) { + throw new UnsupportedOperationException("Delete not supported for " + uri + "."); } - int count = db().delete(getTableName(), query.getSelection(), query.getArgs()); - getContext().getContentResolver().notifyChange(uri, null); - return count; + AppQuerySelection selection = new AppQuerySelection(where, whereArgs).add(queryNoApks()); + return db().delete(getTableName(), selection.getSelection(), selection.getArgs()); } @Override diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java index 8d35fdf1a..db9d03b65 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java @@ -5,7 +5,6 @@ import android.content.Context; import android.content.UriMatcher; import android.database.sqlite.SQLiteDatabase; import android.net.Uri; -import android.util.Log; import org.fdroid.fdroid.data.Schema.ApkTable; @@ -16,8 +15,6 @@ import java.util.List; */ public class TempApkProvider extends ApkProvider { - private static final String TAG = "TempApkProvider"; - private static final String PROVIDER_NAME = "TempApkProvider"; static final String TABLE_TEMP_APK = "temp_" + ApkTable.NAME; @@ -89,18 +86,16 @@ public class TempApkProvider extends ApkProvider { @Override public Uri insert(Uri uri, ContentValues values) { - switch (MATCHER.match(uri)) { - case CODE_INIT: - initTable(); - return null; - default: - return super.insert(uri, values); + if (MATCHER.match(uri) == CODE_INIT) { + initTable(); + return null; } + + return super.insert(uri, values); } @Override public int update(Uri uri, ContentValues values, String where, String[] whereArgs) { - if (MATCHER.match(uri) != CODE_SINGLE) { throw new UnsupportedOperationException("Cannot update anything other than a single apk."); } @@ -110,20 +105,15 @@ public class TempApkProvider extends ApkProvider { @Override public int delete(Uri uri, String where, String[] whereArgs) { - - QuerySelection query = new QuerySelection(where, whereArgs); - - switch (MATCHER.match(uri)) { - case CODE_REPO_APK: - List pathSegments = uri.getPathSegments(); - query = query.add(queryRepo(Long.parseLong(pathSegments.get(1)), false)).add(queryApks(pathSegments.get(2), false)); - break; - - default: - Log.e(TAG, "Invalid URI for apk content provider: " + uri); - throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); + if (MATCHER.match(uri) != CODE_REPO_APK) { + throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); } + List pathSegments = uri.getPathSegments(); + QuerySelection query = new QuerySelection(where, whereArgs) + .add(queryRepo(Long.parseLong(pathSegments.get(1)), false)) + .add(queryApks(pathSegments.get(2), false)); + int rowsAffected = db().delete(getTableName(), query.getSelection(), query.getArgs()); if (!isApplyingBatch()) { getContext().getContentResolver().notifyChange(uri, null); diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java index d9d1f455b..3f73997af 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -122,16 +122,12 @@ public class TempAppProvider extends AppProvider { @Override public int update(Uri uri, ContentValues values, String where, String[] whereArgs) { - QuerySelection query = new QuerySelection(where, whereArgs); - switch (MATCHER.match(uri)) { - case CODE_SINGLE: - query = query.add(querySingle(uri.getLastPathSegment())); - break; - - 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()); if (!isApplyingBatch()) { getContext().getContentResolver().notifyChange(uri, null); From 6c1b277cabc37724f1c0321d920566389256d0ac Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 11 Aug 2016 23:10:54 +1000 Subject: [PATCH 3/4] Close cursors which previously were left dangling. --- .../main/java/org/fdroid/fdroid/data/DBHelper.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 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 8310a9a66..b352fee55 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -786,13 +786,19 @@ class DBHelper extends SQLiteOpenHelper { } private static boolean columnExists(SQLiteDatabase db, String table, String column) { - return db.rawQuery("select * from " + table + " limit 0,1", null) - .getColumnIndex(column) != -1; + Cursor cursor = db.rawQuery("select * from " + table + " limit 0,1", null); + boolean exists = cursor.getColumnIndex(column) != -1; + cursor.close(); + return exists; } 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; + Cursor cursor = db.query("sqlite_master", new String[] {"name"}, + "type = 'table' AND name = ?", new String[] {table}, null, null, null); + + boolean exists = cursor.getCount() > 0; + cursor.close(); + return exists; } } From ebb6d43cbb782a1e82f57b05c980f6a2a65d4b1d Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 11 Aug 2016 23:11:50 +1000 Subject: [PATCH 4/4] Remove dead code AS picked up that the statement is always false, so the body of the if is never executed. This is indeed the case, because the constructor assigns the object which is being checked for null. --- .../fdroid/privileged/views/InstallConfirmActivity.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java b/app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java index c7f443a7d..ed9137b53 100644 --- a/app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java @@ -56,8 +56,6 @@ import org.fdroid.fdroid.data.Schema; */ public class InstallConfirmActivity extends FragmentActivity implements OnCancelListener, OnClickListener { - private static final int RESULT_CANNOT_PARSE = RESULT_FIRST_USER + 1; - private Intent intent; private AppDiff appDiff; @@ -196,10 +194,6 @@ public class InstallConfirmActivity extends FragmentActivity implements OnCancel app = AppProvider.Helper.findByPackageName(getContentResolver(), apk.packageName); appDiff = new AppDiff(getPackageManager(), apk); - if (appDiff.pkgInfo == null) { - setResult(RESULT_CANNOT_PARSE, intent); - finish(); - } setContentView(R.layout.install_start);