From 6c462713aaecaa46c2186edddc3aa189f05b7ca1 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 5 Oct 2016 23:30:11 +1100 Subject: [PATCH 1/5] Renamed generic sounding methods to be more specific. Originally, I hoped that the arguments a method took would help enough to differentiate the intent of that method. This was the case for methods such as `getContentUri()` and `find()`. However they are a little confusing to work with, so this change renames a bunch of methods to be more specific. In addition, it makes some renames from app -> package which will help with the upcoming change to add a `package` table to the database. --- .../java/org/fdroid/fdroid/AppDetails.java | 8 +- .../java/org/fdroid/fdroid/RepoUpdater.java | 4 +- .../java/org/fdroid/fdroid/UpdateService.java | 2 +- .../org/fdroid/fdroid/data/ApkProvider.java | 74 +++++++++---------- .../org/fdroid/fdroid/data/AppProvider.java | 12 ++- .../org/fdroid/fdroid/data/RepoProvider.java | 5 +- .../fdroid/fdroid/data/TempAppProvider.java | 2 +- .../fdroid/fdroid/installer/Installer.java | 2 +- .../views/InstallConfirmActivity.java | 2 +- .../fdroid/views/swap/SwapAppsView.java | 8 +- .../views/swap/SwapWorkflowActivity.java | 2 +- .../fdroid/fdroid/data/ApkProviderTest.java | 10 +-- .../fdroid/fdroid/data/ProviderUriTests.java | 4 +- 13 files changed, 66 insertions(+), 69 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index fd7030dff..39bafd4b6 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -856,7 +856,7 @@ public class AppDetails extends AppCompatActivity { case INSTALL: // Note that this handles updating as well as installing. if (app.suggestedVersionCode > 0) { - final Apk apkToInstall = ApkProvider.Helper.find(this, app.packageName, app.suggestedVersionCode); + final Apk apkToInstall = ApkProvider.Helper.findApkFromAnyRepo(this, app.packageName, app.suggestedVersionCode); install(apkToInstall); } return true; @@ -1006,7 +1006,7 @@ public class AppDetails extends AppCompatActivity { case REQUEST_PERMISSION_DIALOG: if (resultCode == Activity.RESULT_OK) { Uri uri = data.getData(); - Apk apk = ApkProvider.Helper.find(this, uri, Schema.ApkTable.Cols.ALL); + Apk apk = ApkProvider.Helper.findByUri(this, uri, Schema.ApkTable.Cols.ALL); startInstall(apk); } break; @@ -1624,7 +1624,7 @@ public class AppDetails extends AppCompatActivity { App app = appDetails.getApp(); AppDetails activity = (AppDetails) getActivity(); if (updateWanted && app.suggestedVersionCode > 0) { - Apk apkToInstall = ApkProvider.Helper.find(activity, app.packageName, app.suggestedVersionCode); + Apk apkToInstall = ApkProvider.Helper.findApkFromAnyRepo(activity, app.packageName, app.suggestedVersionCode); activity.install(apkToInstall); return; } @@ -1640,7 +1640,7 @@ public class AppDetails extends AppCompatActivity { // If not installed, install btMain.setEnabled(false); btMain.setText(R.string.system_install_installing); - final Apk apkToInstall = ApkProvider.Helper.find(activity, app.packageName, app.suggestedVersionCode); + final Apk apkToInstall = ApkProvider.Helper.findApkFromAnyRepo(activity, app.packageName, app.suggestedVersionCode); activity.install(apkToInstall); } } diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index c4acc9a76..d7c129479 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java @@ -473,7 +473,7 @@ public class RepoUpdater { if (packageInfo != null && versionCode == packageInfo.versionCode) { Utils.debugLog(TAG, repoPushRequest + " already installed, ignoring"); } else { - Apk apk = ApkProvider.Helper.find(context, packageName, versionCode); + Apk apk = ApkProvider.Helper.findApkFromAnyRepo(context, packageName, versionCode); InstallManagerService.queue(context, app, apk); } } else if (RepoPushRequest.UNINSTALL.equals(repoPushRequest.request)) { @@ -483,7 +483,7 @@ public class RepoUpdater { } if (repoPushRequest.versionCode == null || repoPushRequest.versionCode == packageInfo.versionCode) { - Apk apk = ApkProvider.Helper.find(context, repoPushRequest.packageName, + Apk apk = ApkProvider.Helper.findApkFromAnyRepo(context, repoPushRequest.packageName, packageInfo.versionCode); InstallerService.uninstall(context, apk); } else { diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index 37dc52f4a..143f67d19 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -468,7 +468,7 @@ public class UpdateService extends IntentService { cursor.moveToFirst(); for (int i = 0; i < cursor.getCount(); i++) { App app = new App(cursor); - Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVersionCode); + Apk apk = ApkProvider.Helper.findApkFromAnyRepo(this, app.packageName, app.suggestedVersionCode); InstallManagerService.queue(this, app, apk); cursor.moveToNext(); } 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 176bb03cc..3c635c35f 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java @@ -36,7 +36,7 @@ public class ApkProvider extends FDroidProvider { public static void update(Context context, Apk apk) { ContentResolver resolver = context.getContentResolver(); - Uri uri = getContentUri(apk.packageName, apk.versionCode); + Uri uri = getApkFromAnyRepoUri(apk.packageName, apk.versionCode); resolver.update(uri, apk.toContentValues(), null, null); } @@ -62,8 +62,8 @@ public class ApkProvider extends FDroidProvider { return resolver.delete(uri, null, null); } - public static Apk find(Context context, String packageName, int versionCode) { - return find(context, packageName, versionCode, Cols.ALL); + public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode) { + return findApkFromAnyRepo(context, packageName, versionCode, Cols.ALL); } /** @@ -77,12 +77,12 @@ public class ApkProvider extends FDroidProvider { return cursorToList(cursor); } - public static Apk find(Context context, String packageName, int versionCode, String[] projection) { - final Uri uri = getContentUri(packageName, versionCode); - return find(context, uri, projection); + public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode, String[] projection) { + final Uri uri = getApkFromAnyRepoUri(packageName, versionCode); + return findByUri(context, uri, projection); } - public static Apk find(Context context, Uri uri, String[] projection) { + public static Apk findByUri(Context context, Uri uri, String[] projection) { ContentResolver resolver = context.getContentResolver(); Cursor cursor = resolver.query(uri, projection, null, null, null); Apk apk = null; @@ -165,8 +165,8 @@ public class ApkProvider extends FDroidProvider { } } - private static final int CODE_APP = CODE_SINGLE + 1; - private static final int CODE_REPO = CODE_APP + 1; + private static final int CODE_PACKAGE = CODE_SINGLE + 1; + private static final int CODE_REPO = CODE_PACKAGE + 1; private static final int CODE_APKS = CODE_REPO + 1; private static final int CODE_REPO_APPS = CODE_APKS + 1; protected static final int CODE_REPO_APK = CODE_REPO_APPS + 1; @@ -184,17 +184,17 @@ public class ApkProvider extends FDroidProvider { private static final UriMatcher MATCHER = new UriMatcher(-1); private static final Map REPO_FIELDS = new HashMap<>(); - private static final Map APP_FIELDS = new HashMap<>(); + private static final Map PACKAGE_FIELDS = new HashMap<>(); static { REPO_FIELDS.put(Cols.Repo.VERSION, RepoTable.Cols.VERSION); REPO_FIELDS.put(Cols.Repo.ADDRESS, RepoTable.Cols.ADDRESS); - APP_FIELDS.put(Cols.App.PACKAGE_NAME, AppMetadataTable.Cols.PACKAGE_NAME); + PACKAGE_FIELDS.put(Cols.App.PACKAGE_NAME, AppMetadataTable.Cols.PACKAGE_NAME); MATCHER.addURI(getAuthority(), PATH_REPO + "/#", CODE_REPO); MATCHER.addURI(getAuthority(), PATH_APK + "/#/*", CODE_SINGLE); MATCHER.addURI(getAuthority(), PATH_APKS + "/*", CODE_APKS); - MATCHER.addURI(getAuthority(), PATH_APP + "/*", CODE_APP); + MATCHER.addURI(getAuthority(), PATH_APP + "/*", CODE_PACKAGE); MATCHER.addURI(getAuthority(), PATH_REPO_APPS + "/#/*", CODE_REPO_APPS); MATCHER.addURI(getAuthority(), PATH_REPO_APK + "/#/*", CODE_REPO_APK); MATCHER.addURI(getAuthority(), PATH_APK_ROW_ID + "/#", CODE_APK_ROW_ID); @@ -232,11 +232,11 @@ public class ApkProvider extends FDroidProvider { .build(); } - public static Uri getContentUri(Apk apk) { - return getContentUri(apk.packageName, apk.versionCode); + public static Uri getApkFromAnyRepoUri(Apk apk) { + return getApkFromAnyRepoUri(apk.packageName, apk.versionCode); } - public static Uri getContentUri(String packageName, int versionCode) { + public static Uri getApkFromAnyRepoUri(String packageName, int versionCode) { return getContentUri() .buildUpon() .appendPath(PATH_APK) @@ -315,8 +315,8 @@ public class ApkProvider extends FDroidProvider { @Override protected String getRequiredTables() { - final String apk = getTableName(); - final String app = getAppTableName(); + final String apk = getTableName(); + final String app = getAppTableName(); return apk + " AS apk " + " LEFT JOIN " + app + " AS app ON (app." + AppMetadataTable.Cols.ROW_ID + " = apk." + Cols.APP_ID + ")"; @@ -324,8 +324,8 @@ public class ApkProvider extends FDroidProvider { @Override public void addField(String field) { - if (APP_FIELDS.containsKey(field)) { - addAppField(APP_FIELDS.get(field), field); + if (PACKAGE_FIELDS.containsKey(field)) { + addPackageField(PACKAGE_FIELDS.get(field), field); } else if (REPO_FIELDS.containsKey(field)) { addRepoField(REPO_FIELDS.get(field), field); } else if (field.equals(Cols._ID)) { @@ -339,7 +339,7 @@ public class ApkProvider extends FDroidProvider { } } - private void addAppField(String field, String alias) { + private void addPackageField(String field, String alias) { appendField(field, "app", alias); } @@ -353,22 +353,22 @@ public class ApkProvider extends FDroidProvider { } - private QuerySelection queryApp(String packageName) { - return queryApp(packageName, true); + private QuerySelection queryPackage(String packageName) { + return queryPackage(packageName, true); } - private QuerySelection queryApp(String packageName, boolean includeTableAlias) { + private QuerySelection queryPackage(String packageName, boolean includeTableAlias) { String alias = includeTableAlias ? "apk." : ""; final String selection = alias + Cols.APP_ID + " = (" + getAppIdFromPackageNameQuery() + ")"; final String[] args = {packageName}; return new QuerySelection(selection, args); } - private QuerySelection querySingle(Uri uri) { - return querySingle(uri, true); + private QuerySelection querySingleFromAnyRepo(Uri uri) { + return querySingleFromAnyRepo(uri, true); } - private QuerySelection querySingle(Uri uri, boolean includeAlias) { + private QuerySelection querySingleFromAnyRepo(Uri uri, boolean includeAlias) { String alias = includeAlias ? "apk." : ""; final String selection = alias + Cols.VERSION_CODE + " = ? and " + alias + Cols.APP_ID + " = (" + getAppIdFromPackageNameQuery() + ")"; final String[] args = { @@ -403,7 +403,7 @@ public class ApkProvider extends FDroidProvider { } private QuerySelection queryRepoApps(long repoId, String packageNames) { - return queryRepo(repoId).add(AppProvider.queryApps(packageNames, "app." + AppMetadataTable.Cols.PACKAGE_NAME)); + return queryRepo(repoId).add(AppProvider.queryPackageNames(packageNames, "app." + AppMetadataTable.Cols.PACKAGE_NAME)); } protected QuerySelection queryApks(String apkKeys) { @@ -456,15 +456,15 @@ public class ApkProvider extends FDroidProvider { break; case CODE_SINGLE: - query = query.add(querySingle(uri)); + query = query.add(querySingleFromAnyRepo(uri)); break; case CODE_APK_ROW_ID: query = query.add(querySingle(Long.parseLong(uri.getLastPathSegment()))); break; - case CODE_APP: - query = query.add(queryApp(uri.getLastPathSegment())); + case CODE_PACKAGE: + query = query.add(queryPackage(uri.getLastPathSegment())); break; case CODE_APKS: @@ -505,7 +505,7 @@ public class ApkProvider extends FDroidProvider { } } - for (Map.Entry appField : APP_FIELDS.entrySet()) { + for (Map.Entry appField : PACKAGE_FIELDS.entrySet()) { final String field = appField.getKey(); if (values.containsKey(field)) { values.remove(field); @@ -535,8 +535,8 @@ public class ApkProvider extends FDroidProvider { query = query.add(queryRepo(Long.parseLong(uri.getLastPathSegment()), false)); break; - case CODE_APP: - query = query.add(queryApp(uri.getLastPathSegment(), false)); + case CODE_PACKAGE: + query = query.add(queryPackage(uri.getLastPathSegment(), false)); break; case CODE_APKS: @@ -549,12 +549,6 @@ public class ApkProvider extends FDroidProvider { query = query.add(queryRepo(Long.parseLong(pathSegments.get(1)))).add(queryApks(pathSegments.get(2))); break; - case CODE_LIST: - throw new UnsupportedOperationException("Can't delete all apks."); - - case CODE_SINGLE: - throw new UnsupportedOperationException("Can't delete individual apks."); - default: Log.e(TAG, "Invalid URI for apk content provider: " + uri); throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); @@ -579,7 +573,7 @@ public class ApkProvider extends FDroidProvider { removeFieldsFromOtherTables(values); QuerySelection query = new QuerySelection(where, whereArgs); - query = query.add(querySingle(uri, false)); + query = query.add(querySingleFromAnyRepo(uri, false)); int numRows = db().update(getTableName(), values, query.getSelection(), query.getArgs()); if (!isApplyingBatch()) { 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 db5874eb9..256884f35 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -127,7 +127,10 @@ public class AppProvider extends FDroidProvider { public static App findByPackageName(ContentResolver resolver, String packageName, String[] projection) { final Uri uri = getContentUri(packageName); - Cursor cursor = resolver.query(uri, projection, null, null, null); + return cursorToApp(resolver.query(uri, projection, null, null, null)); + } + + private static App cursorToApp(Cursor cursor) { App app = null; if (cursor != null) { if (cursor.getCount() > 0) { @@ -675,7 +678,7 @@ public class AppProvider extends FDroidProvider { return new AppQuerySelection(selection); } - static AppQuerySelection queryApps(String packageNames, String packageNameField) { + static AppQuerySelection queryPackageNames(String packageNames, String packageNameField) { String[] args = packageNames.split(","); String selection = packageNameField + " IN (" + generateQuestionMarksForInClause(args.length) + ")"; return new AppQuerySelection(selection, args); @@ -727,8 +730,9 @@ public class AppProvider extends FDroidProvider { break; case SEARCH_REPO: - selection = selection.add(querySearch(uri.getPathSegments().get(2))); - selection = selection.add(queryRepo(Long.parseLong(uri.getPathSegments().get(1)))); + selection = selection + .add(querySearch(uri.getPathSegments().get(2))) + .add(queryRepo(Long.parseLong(uri.getPathSegments().get(1)))); break; case NO_APKS: diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java index caa4f8c39..dbe7dbb96 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java @@ -149,8 +149,7 @@ public class RepoProvider extends FDroidProvider { return repo; } - public static void update(Context context, Repo repo, - ContentValues values) { + public static void update(Context context, Repo repo, ContentValues values) { ContentResolver resolver = context.getContentResolver(); // Change the name to the new address. Next time we update the repo @@ -316,7 +315,7 @@ public class RepoProvider extends FDroidProvider { break; case CODE_ALL_EXCEPT_SWAP: - selection = Cols.IS_SWAP + " = 0 OR " + Cols.IS_SWAP + " IS NULL "; + selection = "COALESCE(" + Cols.IS_SWAP + ", 0) = 0 "; break; default: 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 18520e286..6a37e0db9 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -69,7 +69,7 @@ public class TempAppProvider extends AppProvider { } private AppQuerySelection queryApps(String packageNames) { - return queryApps(packageNames, getTableName() + "." + AppMetadataTable.Cols.PACKAGE_NAME); + return queryPackageNames(packageNames, getTableName() + "." + AppMetadataTable.Cols.PACKAGE_NAME); } public static class Helper { diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java index ce1216a0b..0b72bf157 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -97,7 +97,7 @@ public abstract class Installer { // no permission screen needed! return null; } - Uri uri = ApkProvider.getContentUri(apk); + Uri uri = ApkProvider.getApkFromAnyRepoUri(apk); Intent intent = new Intent(context, InstallConfirmActivity.class); intent.setData(uri); 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 ed9137b53..a14996357 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 @@ -190,7 +190,7 @@ public class InstallConfirmActivity extends FragmentActivity implements OnCancel intent = getIntent(); Uri uri = intent.getData(); - Apk apk = ApkProvider.Helper.find(this, uri, Schema.ApkTable.Cols.ALL); + Apk apk = ApkProvider.Helper.findByUri(this, uri, Schema.ApkTable.Cols.ALL); app = AppProvider.Helper.findByPackageName(getContentResolver(), apk.packageName); appDiff = new AppDiff(getPackageManager(), apk); diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java index 00de67ab5..0b570506c 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java @@ -48,7 +48,7 @@ import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.Repo; -import org.fdroid.fdroid.data.Schema; +import org.fdroid.fdroid.data.Schema.AppMetadataTable; import org.fdroid.fdroid.localrepo.SwapService; import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.DownloaderService; @@ -104,7 +104,7 @@ public class SwapAppsView extends ListView implements */ adapter = new AppListAdapter(getContext(), getContext().getContentResolver().query( - AppProvider.getRepoUri(repo), Schema.AppMetadataTable.Cols.ALL, null, null, null)); + AppProvider.getRepoUri(repo), AppMetadataTable.Cols.ALL, null, null, null)); setAdapter(adapter); @@ -194,7 +194,7 @@ public class SwapAppsView extends ListView implements ? AppProvider.getRepoUri(repo) : AppProvider.getSearchUri(repo, mCurrentFilterString); - return new CursorLoader(getActivity(), uri, Schema.AppMetadataTable.Cols.ALL, null, null, Schema.AppMetadataTable.Cols.NAME); + return new CursorLoader(getActivity(), uri, AppMetadataTable.Cols.ALL, null, null, AppMetadataTable.Cols.NAME); } @Override @@ -304,7 +304,7 @@ public class SwapAppsView extends ListView implements this.app = app; Context context = getContext(); - Apk apk = ApkProvider.Helper.find(context, app.packageName, app.suggestedVersionCode); + Apk apk = ApkProvider.Helper.findApkFromAnyRepo(context, app.packageName, app.suggestedVersionCode); String urlString = apk.getUrl(); // TODO unregister receivers? or will they just die with this instance diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java index fc760bda7..c047b6685 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java @@ -766,7 +766,7 @@ public class SwapWorkflowActivity extends AppCompatActivity { } public void install(@NonNull final App app) { - final Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVersionCode); + final Apk apk = ApkProvider.Helper.findApkFromAnyRepo(this, app.packageName, app.suggestedVersionCode); Uri downloadUri = Uri.parse(apk.getUrl()); localBroadcastManager.registerReceiver(installReceiver, Installer.getInstallIntentFilter(downloadUri)); 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 65b738045..95a153071 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java @@ -69,8 +69,8 @@ public class ApkProviderTest extends FDroidProviderTest { Apk apk = new MockApk("org.fdroid.fdroid", 10); assertCantDelete(contentResolver, ApkProvider.getContentUri()); - assertCantDelete(contentResolver, ApkProvider.getContentUri("org.fdroid.fdroid", 10)); - assertCantDelete(contentResolver, ApkProvider.getContentUri(apk)); + assertCantDelete(contentResolver, ApkProvider.getApkFromAnyRepoUri("org.fdroid.fdroid", 10)); + assertCantDelete(contentResolver, ApkProvider.getApkFromAnyRepoUri(apk)); assertCantDelete(contentResolver, Uri.withAppendedPath(ApkProvider.getContentUri(), "some-random-path")); } @@ -411,7 +411,7 @@ public class ApkProviderTest extends FDroidProviderTest { Assert.insertApk(context, "com.other.thing." + i, i); } - Apk apk = ApkProvider.Helper.find(context, "com.example", 11); + Apk apk = ApkProvider.Helper.findApkFromAnyRepo(context, "com.example", 11); assertNotNull(apk); @@ -428,7 +428,7 @@ public class ApkProviderTest extends FDroidProviderTest { Cols.HASH, }; - Apk apkLessFields = ApkProvider.Helper.find(context, "com.example", 11, projection); + Apk apkLessFields = ApkProvider.Helper.findApkFromAnyRepo(context, "com.example", 11, projection); assertNotNull(apkLessFields); @@ -440,7 +440,7 @@ public class ApkProviderTest extends FDroidProviderTest { assertNull(apkLessFields.versionName); assertEquals(0, apkLessFields.versionCode); - Apk notFound = ApkProvider.Helper.find(context, "com.doesnt.exist", 1000); + Apk notFound = ApkProvider.Helper.findApkFromAnyRepo(context, "com.doesnt.exist", 1000); assertNull(notFound); } 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 a9a55bd8a..2c5f067e0 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java @@ -135,9 +135,9 @@ public class ProviderUriTests { assertValidUri(resolver, ApkProvider.getContentUri(), "content://org.fdroid.fdroid.data.ApkProvider", projection); assertValidUri(resolver, ApkProvider.getAppUri("org.fdroid.fdroid"), "content://org.fdroid.fdroid.data.ApkProvider/app/org.fdroid.fdroid", projection); - assertValidUri(resolver, ApkProvider.getContentUri(new MockApk("org.fdroid.fdroid", 100)), "content://org.fdroid.fdroid.data.ApkProvider/apk/100/org.fdroid.fdroid", projection); + assertValidUri(resolver, ApkProvider.getApkFromAnyRepoUri(new MockApk("org.fdroid.fdroid", 100)), "content://org.fdroid.fdroid.data.ApkProvider/apk/100/org.fdroid.fdroid", projection); assertValidUri(resolver, ApkProvider.getContentUri(apks), projection); - assertValidUri(resolver, ApkProvider.getContentUri("org.fdroid.fdroid", 100), "content://org.fdroid.fdroid.data.ApkProvider/apk/100/org.fdroid.fdroid", projection); + assertValidUri(resolver, ApkProvider.getApkFromAnyRepoUri("org.fdroid.fdroid", 100), "content://org.fdroid.fdroid.data.ApkProvider/apk/100/org.fdroid.fdroid", projection); assertValidUri(resolver, ApkProvider.getRepoUri(1000), "content://org.fdroid.fdroid.data.ApkProvider/repo/1000", projection); } From 97cf69341ab89627737a5ec28c9db4dab2ca5274 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 5 Oct 2016 23:40:16 +1100 Subject: [PATCH 2/5] When inserting a new repo, assign the priority appropriately. Even though this is not used yet, it will be a requirement in the near future for the `RepoProvider` to be the one who decides what the priority of new repositories is. This will prevent clients of this provider from specifying wrong priorities that result in gaps For example, if we accidentally ended up with priorities of 1, 2, 4, and then 5, this would cause problems if the user tried to drag the second repo to the position of the 4th repo. It is easier to do these priority shuffles if we can assume that the priorities are contiguous. --- .../org/fdroid/fdroid/data/RepoProvider.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java index dbe7dbb96..f08fc1589 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java @@ -331,6 +331,10 @@ public class RepoProvider extends FDroidProvider { @Override public Uri insert(Uri uri, ContentValues values) { + // Don't let people specify arbitrary priorities. Instead, we are responsible + // for making sure that newly created repositories by default have the highest priority. + values.put(Cols.PRIORITY, getMaxPriority() + 1); + if (!values.containsKey(Cols.ADDRESS)) { throw new UnsupportedOperationException("Cannot add repo without an address."); } @@ -342,10 +346,6 @@ public class RepoProvider extends FDroidProvider { values.put(Cols.IN_USE, 1); } - if (!values.containsKey(Cols.PRIORITY)) { - values.put(Cols.PRIORITY, 10); - } - if (!values.containsKey(Cols.MAX_AGE)) { values.put(Cols.MAX_AGE, 0); } @@ -365,6 +365,14 @@ public class RepoProvider extends FDroidProvider { return getContentUri(id); } + private int getMaxPriority() { + Cursor cursor = db().query(RepoTable.NAME, new String[] {"MAX(" + Cols.PRIORITY + ")"}, "COALESCE(" + Cols.IS_SWAP + ", 0) = 0", null, null, null, null); + cursor.moveToFirst(); + int max = cursor.getInt(0); + cursor.close(); + return max; + } + @Override public int delete(Uri uri, String where, String[] whereArgs) { From 45f9379fee3207190cdc28568f96ec5bfabb0e4e Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 28 Sep 2016 22:57:40 +1000 Subject: [PATCH 3/5] Added helper function for debugging SQL queries during development It is often helpful during debugging to be able to dump the contents of an SQL result `Cursor` to the debug watch list. This is difficult to do under normal circumstances. This adds a utility method really only designed to be used during interactive debugging, which will do its best to build a `Map` for each row in the `Cursor`. This can then be used to test queries while the debugger is paused. --- .../main/java/org/fdroid/fdroid/Utils.java | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index c39b0ab1a..eca09c6bd 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -20,10 +20,12 @@ package org.fdroid.fdroid; import android.content.Context; import android.content.pm.PackageManager; +import android.database.Cursor; import android.graphics.Bitmap; import android.net.Uri; import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import android.support.annotation.RequiresApi; import android.text.Editable; import android.text.Html; import android.text.TextUtils; @@ -49,6 +51,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.math.BigInteger; +import java.nio.charset.Charset; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.cert.Certificate; @@ -56,9 +59,13 @@ import java.security.cert.CertificateEncodingException; import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; +import java.util.ArrayList; import java.util.Date; import java.util.Formatter; +import java.util.HashMap; +import java.util.List; import java.util.Locale; +import java.util.Map; public final class Utils { @@ -541,4 +548,52 @@ public final class Utils { return versionName; } + /** + * Useful for debugging during development, so that arbitrary queries can be made, and their + * results inspected in the debugger. + */ + @SuppressWarnings("unused") + @RequiresApi(api = 11) + public static List> dumpCursor(Cursor cursor) { + List> data = new ArrayList<>(); + + if (cursor == null) { + return data; + } + + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + Map row = new HashMap<>(cursor.getColumnCount()); + for (String col : cursor.getColumnNames()) { + int i = cursor.getColumnIndex(col); + switch (cursor.getType(i)) { + case Cursor.FIELD_TYPE_NULL: + row.put(col, null); + break; + + case Cursor.FIELD_TYPE_INTEGER: + row.put(col, Integer.toString(cursor.getInt(i))); + break; + + case Cursor.FIELD_TYPE_FLOAT: + row.put(col, Double.toString(cursor.getFloat(i))); + break; + + case Cursor.FIELD_TYPE_STRING: + row.put(col, cursor.getString(i)); + break; + + case Cursor.FIELD_TYPE_BLOB: + row.put(col, new String(cursor.getBlob(i), Charset.defaultCharset())); + break; + } + } + data.add(row); + cursor.moveToNext(); + } + + cursor.close(); + return data; + } + } From 486e8e699ff9e5578c68c9afa13c7f46aca55df5 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 5 Oct 2016 23:51:45 +1100 Subject: [PATCH 4/5] Cleanup DBHelper in prep for package table in the future. --- .../java/org/fdroid/fdroid/data/DBHelper.java | 24 ++++++++++++------- .../fdroid/fdroid/data/TempAppProvider.java | 2 +- 2 files changed, 16 insertions(+), 10 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 bcf757446..7af5b95e7 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -95,7 +95,7 @@ class DBHelper extends SQLiteOpenHelper { + "PRIMARY KEY (" + ApkTable.Cols.APP_ID + ", " + ApkTable.Cols.VERSION_CODE + ", " + ApkTable.Cols.REPO_ID + ")" + ");"; - static final String CREATE_TABLE_APP = "CREATE TABLE " + AppMetadataTable.NAME + static final String CREATE_TABLE_APP_METADATA = "CREATE TABLE " + AppMetadataTable.NAME + " ( " + AppMetadataTable.Cols.PACKAGE_NAME + " text not null, " + AppMetadataTable.Cols.NAME + " text not null, " @@ -248,7 +248,7 @@ class DBHelper extends SQLiteOpenHelper { @Override public void onCreate(SQLiteDatabase db) { - db.execSQL(CREATE_TABLE_APP); + db.execSQL(CREATE_TABLE_APP_METADATA); db.execSQL(CREATE_TABLE_APK); db.execSQL(CREATE_TABLE_INSTALLED_APP); db.execSQL(CREATE_TABLE_REPO); @@ -731,12 +731,18 @@ class DBHelper extends SQLiteOpenHelper { .putBoolean("triedEmptyUpdate", false) .apply(); - db.execSQL("DROP TABLE " + AppMetadataTable.NAME); - db.execSQL("DROP TABLE " + ApkTable.NAME); - db.execSQL(CREATE_TABLE_APP); - db.execSQL(CREATE_TABLE_APK); - clearRepoEtags(db); - ensureIndexes(db); + db.beginTransaction(); + try { + db.execSQL("DROP TABLE " + AppMetadataTable.NAME); + db.execSQL("DROP TABLE " + ApkTable.NAME); + db.execSQL(CREATE_TABLE_APP_METADATA); + db.execSQL(CREATE_TABLE_APK); + clearRepoEtags(db); + ensureIndexes(db); + db.setTransactionSuccessful(); + } finally { + db.endTransaction(); + } } private void resetTransientPre42(SQLiteDatabase db, int oldVersion) { @@ -753,7 +759,7 @@ class DBHelper extends SQLiteOpenHelper { db.execSQL("drop table " + AppMetadataTable.NAME); db.execSQL("drop table " + ApkTable.NAME); clearRepoEtags(db); - db.execSQL(CREATE_TABLE_APP); + db.execSQL(CREATE_TABLE_APP_METADATA); db.execSQL(CREATE_TABLE_APK); ensureIndexes(db); } 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 6a37e0db9..955e82cde 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -161,7 +161,7 @@ public class TempAppProvider extends AppProvider { final SQLiteDatabase db = db(); ensureTempTableDetached(db); db.execSQL("ATTACH DATABASE ':memory:' AS " + DB); - db.execSQL(DBHelper.CREATE_TABLE_APP.replaceFirst(AppMetadataTable.NAME, DB + "." + getTableName())); + db.execSQL(DBHelper.CREATE_TABLE_APP_METADATA.replaceFirst(AppMetadataTable.NAME, DB + "." + getTableName())); db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, AppMetadataTable.NAME, DB + "." + getTableName())); db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_id ON " + getTableName() + " (" + AppMetadataTable.Cols.PACKAGE_NAME + ");"); db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_upstreamVercode ON " + getTableName() + " (" + AppMetadataTable.Cols.UPSTREAM_VERSION_CODE + ");"); From 88c536efb409e7a9a7289c05492608651216c8df Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 5 Oct 2016 23:52:06 +1100 Subject: [PATCH 5/5] Fix incorrect version check in db helper. --- app/src/main/java/org/fdroid/fdroid/data/DBHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7af5b95e7..36d7e93e8 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -819,7 +819,7 @@ class DBHelper extends SQLiteOpenHelper { } private void supportRepoPushRequests(SQLiteDatabase db, int oldVersion) { - if (oldVersion >= 61) { + if (oldVersion >= 62) { return; } Utils.debugLog(TAG, "Adding " + RepoTable.Cols.PUSH_REQUESTS