diff --git a/src/org/fdroid/fdroid/data/ApkProvider.java b/src/org/fdroid/fdroid/data/ApkProvider.java index b957b2093..6c9afbdeb 100644 --- a/src/org/fdroid/fdroid/data/ApkProvider.java +++ b/src/org/fdroid/fdroid/data/ApkProvider.java @@ -64,7 +64,7 @@ public class ApkProvider extends FDroidProvider { return cursorToList(cursor); } - private static List cursorToList(Cursor cursor) { + public static List cursorToList(Cursor cursor) { List apks = new ArrayList(); if (cursor != null) { cursor.moveToFirst(); @@ -77,20 +77,16 @@ public class ApkProvider extends FDroidProvider { return apks; } - public static void deleteApksByRepo(Context context, Repo repo) { + public static int deleteApksByRepo(Context context, Repo repo) { ContentResolver resolver = context.getContentResolver(); - Uri uri = getContentUri(); - String[] args = { Long.toString(repo.getId()) }; - String selection = DataColumns.REPO_ID + " = ?"; - int count = resolver.delete(uri, selection, args); + Uri uri = getRepoUri(repo.getId()); + return resolver.delete(uri, null, null); } public static void deleteApksByApp(Context context, App app) { ContentResolver resolver = context.getContentResolver(); - Uri uri = getContentUri(); - String[] args = { app.id }; - String selection = DataColumns.APK_ID + " = ?"; - resolver.delete(uri, selection, args); + Uri uri = getAppUri(app.id); + resolver.delete(uri, null, null); } public static Apk find(Context context, String id, int versionCode) { @@ -340,7 +336,7 @@ public class ApkProvider extends FDroidProvider { } private QuerySelection queryApp(String appId) { - String selection = " id = ? "; + String selection = DataColumns.APK_ID + " = ? "; String[] args = new String[] { appId }; return new QuerySelection(selection, args); } @@ -357,7 +353,7 @@ public class ApkProvider extends FDroidProvider { } private QuerySelection queryRepo(long repoId) { - String selection = " repo = ? "; + String selection = DataColumns.REPO_ID + " = ? "; String[] args = new String[]{ Long.toString(repoId) }; return new QuerySelection(selection, args); } @@ -442,6 +438,7 @@ public class ApkProvider extends FDroidProvider { @Override public Uri insert(Uri uri, ContentValues values) { removeRepoFields(values); + validateFields(DataColumns.ALL, values); long id = write().insertOrThrow(getTableName(), null, values); if (!isApplyingBatch()) { getContext().getContentResolver().notifyChange(uri, null); @@ -458,18 +455,30 @@ public class ApkProvider extends FDroidProvider { QuerySelection query = new QuerySelection(where, whereArgs); switch (matcher.match(uri)) { - case CODE_LIST: - // Don't support deleting of multiple apks yet. - return 0; case CODE_REPO: query = query.add(queryRepo(Long.parseLong(uri.getLastPathSegment()))); break; - case CODE_SINGLE: - query = query.add(querySingle(uri)); + case CODE_APP: + query = query.add(queryApp(uri.getLastPathSegment())); break; + case CODE_LIST: + throw new UnsupportedOperationException( + "Can't delete all apks. " + + "Can only delete those belonging to an app, or a repo."); + + case CODE_APKS: + throw new UnsupportedOperationException( + "Can't delete arbitrary apks. " + + "Can only delete those belonging to an app, or a repo."); + + case CODE_SINGLE: + throw new UnsupportedOperationException( + "Can't delete individual apks. " + + "Can only delete those belonging to an app, or a repo."); + default: Log.e("FDroid", "Invalid URI for apk content provider: " + uri); throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); @@ -486,6 +495,8 @@ public class ApkProvider extends FDroidProvider { QuerySelection query = new QuerySelection(where, whereArgs); + validateFields(DataColumns.ALL, values); + switch (matcher.match(uri)) { case CODE_LIST: return 0; diff --git a/src/org/fdroid/fdroid/data/FDroidProvider.java b/src/org/fdroid/fdroid/data/FDroidProvider.java index 3e0c4b316..e056eef3e 100644 --- a/src/org/fdroid/fdroid/data/FDroidProvider.java +++ b/src/org/fdroid/fdroid/data/FDroidProvider.java @@ -1,5 +1,6 @@ package org.fdroid.fdroid.data; +import android.annotation.TargetApi; import android.content.*; import android.database.sqlite.SQLiteDatabase; import android.net.Uri; @@ -95,5 +96,24 @@ public abstract class FDroidProvider extends ContentProvider { } return sb.toString(); } + + @TargetApi(11) + protected void validateFields(String[] validFields, ContentValues values) + throws IllegalArgumentException { + for (String key : values.keySet()) { + boolean isValid = false; + for (String validKey : validFields) { + if (validKey.equals(key)) { + isValid = true; + break; + } + } + + if (!isValid) { + throw new IllegalArgumentException( + "Cannot save field '" + key + "' to provider " + getProviderName()); + } + } + } } diff --git a/src/org/fdroid/fdroid/data/Repo.java b/src/org/fdroid/fdroid/data/Repo.java index d5bc3bc56..1b0bdd055 100644 --- a/src/org/fdroid/fdroid/data/Repo.java +++ b/src/org/fdroid/fdroid/data/Repo.java @@ -14,7 +14,7 @@ public class Repo extends ValueObject { public static final int VERSION_DENSITY_SPECIFIC_ICONS = 11; - private long id; + protected long id; public String address; public String name; diff --git a/test/src/org/fdroid/fdroid/ApkProviderTest.java b/test/src/org/fdroid/fdroid/ApkProviderTest.java index 795172d27..0785a87db 100644 --- a/test/src/org/fdroid/fdroid/ApkProviderTest.java +++ b/test/src/org/fdroid/fdroid/ApkProviderTest.java @@ -2,10 +2,14 @@ package org.fdroid.fdroid; import android.content.ContentValues; import android.database.Cursor; +import android.net.Uri; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.AppProvider; +import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.mock.MockApk; +import org.fdroid.fdroid.mock.MockApp; +import org.fdroid.fdroid.mock.MockRepo; import java.util.ArrayList; import java.util.List; @@ -20,13 +24,14 @@ public class ApkProviderTest extends FDroidProviderTest { return new String[] { ApkProvider.DataColumns.APK_ID, ApkProvider.DataColumns.VERSION_CODE, - ApkProvider.DataColumns.NAME + ApkProvider.DataColumns.NAME, + ApkProvider.DataColumns.REPO_ID }; } public void testUris() { assertInvalidUri(ApkProvider.getAuthority()); - assertInvalidUri(AppProvider.getContentUri()); + assertInvalidUri(RepoProvider.getContentUri()); List apks = new ArrayList(3); for (int i = 0; i < 10; i ++) { @@ -61,7 +66,86 @@ public class ApkProviderTest extends FDroidProviderTest { } catch (Exception e) { fail(); } + } + public void testAppApks() { + for (int i = 1; i <= 10; i ++) { + insertApk("org.fdroid.fdroid", i); + insertApk("com.example", i); + } + + assertTotalApkCount(20); + + Cursor fdroidApks = getMockContentResolver().query( + ApkProvider.getAppUri("org.fdroid.fdroid"), + getMinimalProjection(), + null, null, null); + assertResultCount(10, fdroidApks); + assertBelongsToApp(fdroidApks, "org.fdroid.fdroid"); + + Cursor exampleApks = getMockContentResolver().query( + ApkProvider.getAppUri("com.example"), + getMinimalProjection(), + null, null, null); + assertResultCount(10, exampleApks); + assertBelongsToApp(exampleApks, "com.example"); + + ApkProvider.Helper.deleteApksByApp(getMockContext(), new MockApp("com.example")); + + Cursor all = queryAllApks(); + assertResultCount(10, all); + assertBelongsToApp(all, "org.fdroid.fdroid"); + } + + public void testInvalidDeleteUris() { + assertCantDelete(ApkProvider.getContentUri()); + assertCantDelete(ApkProvider.getContentUri(new ArrayList())); + assertCantDelete(ApkProvider.getContentUri("org.fdroid.fdroid", 10)); + assertCantDelete(ApkProvider.getContentUri(new MockApk("org.fdroid.fdroid", 10))); + + try { + getMockContentResolver().delete(RepoProvider.getContentUri(), null, null); + fail(); + } catch (IllegalArgumentException e) { + // Don't fail, it is what we were looking for... + } catch (Exception e) { + fail(); + } + } + + public void testRepoApks() { + + final long REPO_KEEP = 1; + final long REPO_DELETE = 2; + + // Insert apks into two repos, one of which we will later purge the + // the apks from. + for (int i = 1; i <= 5; i ++) { + insertApkForRepo("org.fdroid.fdroid", i, REPO_KEEP); + insertApkForRepo("com.example." + i, 1, REPO_DELETE); + } + for (int i = 6; i <= 10; i ++) { + insertApkForRepo("org.fdroid.fdroid", i, REPO_DELETE); + insertApkForRepo("com.example." + i, 1, REPO_KEEP); + } + + assertTotalApkCount(20); + + Cursor cursor = getMockContentResolver().query( + ApkProvider.getRepoUri(REPO_DELETE), getMinimalProjection(), null, null, null); + assertResultCount(10, cursor); + assertBelongsToRepo(cursor, REPO_DELETE); + + int count = ApkProvider.Helper.deleteApksByRepo(getMockContext(), new MockRepo(REPO_DELETE)); + assertEquals(10, count); + + assertTotalApkCount(10); + cursor = getMockContentResolver().query( + ApkProvider.getRepoUri(REPO_DELETE), getMinimalProjection(), null, null, null); + assertResultCount(0, cursor); + + // The only remaining apks should be those from REPO_KEEP. + assertBelongsToRepo(queryAllApks(), REPO_KEEP); } public void testQuery() { @@ -69,12 +153,6 @@ public class ApkProviderTest extends FDroidProviderTest { assertNotNull(cursor); } - private void insertApks(int count) { - for (int i = 0; i < count; i ++) { - insertApk("com.example.test." + i, i); - } - } - public void testInsert() { // Start with an empty database... @@ -82,8 +160,11 @@ public class ApkProviderTest extends FDroidProviderTest { assertNotNull(cursor); assertEquals(0, cursor.getCount()); + Apk apk = new MockApk("org.fdroid.fdroid", 13); + // Insert a new record... - insertApk("org.fdroid.fdroid", 13); + Uri newUri = insertApk(apk.id, apk.vercode); + assertEquals(ApkProvider.getContentUri(apk).toString(), newUri.toString()); cursor = queryAllApks(); assertNotNull(cursor); assertEquals(1, cursor.getCount()); @@ -103,29 +184,79 @@ public class ApkProviderTest extends FDroidProviderTest { // value object (because the queryAllApks() helper asks for VERSION_CODE and // APK_ID. cursor.moveToFirst(); - Apk apk = new Apk(cursor); - assertEquals("org.fdroid.fdroid", apk.id); - assertEquals(13, apk.vercode); + Apk toCheck = new Apk(cursor); + assertEquals("org.fdroid.fdroid", toCheck.id); + assertEquals(13, toCheck.vercode); + } + + public void testInsertWithExtraFields() { + + assertResultCount(0, queryAllApks()); + + String[] repoFields = new String[] { + RepoProvider.DataColumns.DESCRIPTION, + RepoProvider.DataColumns.ADDRESS, + RepoProvider.DataColumns.FINGERPRINT, + RepoProvider.DataColumns.NAME, + RepoProvider.DataColumns.PUBLIC_KEY + }; + + for (String field : repoFields) { + ContentValues invalidRepo = new ContentValues(); + invalidRepo.put(field, "Test data"); + try { + insertApk("org.fdroid.fdroid", 10, invalidRepo); + fail(); + } catch (IllegalArgumentException e) { + } catch (Exception e) { + fail(); + } + assertResultCount(0, queryAllApks()); + } + + // ApkProvider.DataColumns.REPO + } public void testIgnore() { - for (int i = 0; i < 10; i ++) { + /*for (int i = 0; i < 10; i ++) { insertApk("org.fdroid.fdroid", i); - } + }*/ + } + private void assertBelongsToApp(Cursor apks, String appId) { + for (Apk apk : ApkProvider.Helper.cursorToList(apks)) { + assertEquals(appId, apk.id); + } + } + + private void assertTotalApkCount(int expected) { + assertResultCount(expected, queryAllApks()); + } + + private void assertBelongsToRepo(Cursor apkCursor, long repoId) { + for (Apk apk : ApkProvider.Helper.cursorToList(apkCursor)) { + assertEquals(repoId, apk.repo); + } + } + + private void insertApkForRepo(String id, int versionCode, long repoId) { + ContentValues additionalValues = new ContentValues(); + additionalValues.put(ApkProvider.DataColumns.REPO_ID, repoId); + insertApk(id, versionCode, additionalValues); } private Cursor queryAllApks() { return getMockContentResolver().query(ApkProvider.getContentUri(), getMinimalProjection(), null, null, null); } - private void insertApk(String id, int versionCode) { - insertApk(id, versionCode, new ContentValues()); + private Uri insertApk(String id, int versionCode) { + return insertApk(id, versionCode, new ContentValues()); } - private void insertApk(String id, int versionCode, + private Uri insertApk(String id, int versionCode, ContentValues additionalValues) { - TestUtils.insertApk(getMockContentResolver(), id, versionCode, additionalValues); + return TestUtils.insertApk(getMockContentResolver(), id, versionCode, additionalValues); } } diff --git a/test/src/org/fdroid/fdroid/FDroidProviderTest.java b/test/src/org/fdroid/fdroid/FDroidProviderTest.java index fe85ea6d2..1fe2e3c48 100644 --- a/test/src/org/fdroid/fdroid/FDroidProviderTest.java +++ b/test/src/org/fdroid/fdroid/FDroidProviderTest.java @@ -50,6 +50,16 @@ public abstract class FDroidProviderTest extends Provi return swappableContext; } + protected void assertCantDelete(Uri uri) { + try { + getMockContentResolver().delete(uri, null, null); + fail(); + } catch (UnsupportedOperationException e) { + } catch (Exception e) { + fail(); + } + } + protected void assertInvalidUri(String uri) { assertInvalidUri(Uri.parse(uri)); } @@ -83,7 +93,11 @@ public abstract class FDroidProviderTest extends Provi protected void assertResultCount(int expectedCount, Uri uri) { Cursor cursor = getMockContentResolver().query(uri, getMinimalProjection(), null, null, null); - assertNotNull(cursor); - assertEquals(expectedCount, cursor.getCount()); + assertResultCount(expectedCount, cursor); + } + + protected void assertResultCount(int expectedCount, Cursor result) { + assertNotNull(result); + assertEquals(expectedCount, result.getCount()); } } diff --git a/test/src/org/fdroid/fdroid/TestUtils.java b/test/src/org/fdroid/fdroid/TestUtils.java index 19299e10a..420c32c45 100644 --- a/test/src/org/fdroid/fdroid/TestUtils.java +++ b/test/src/org/fdroid/fdroid/TestUtils.java @@ -82,7 +82,7 @@ public class TestUtils { resolver.insert(uri, values); } - public static void insertApk(ContentResolver resolver, String id, int versionCode, ContentValues additionalValues) { + public static Uri insertApk(ContentResolver resolver, String id, int versionCode, ContentValues additionalValues) { ContentValues values = new ContentValues(); @@ -101,6 +101,6 @@ public class TestUtils { Uri uri = ApkProvider.getContentUri(); - resolver.insert(uri, values); + return resolver.insert(uri, values); } } diff --git a/test/src/org/fdroid/fdroid/mock/MockRepo.java b/test/src/org/fdroid/fdroid/mock/MockRepo.java new file mode 100644 index 000000000..3b3fce976 --- /dev/null +++ b/test/src/org/fdroid/fdroid/mock/MockRepo.java @@ -0,0 +1,11 @@ +package org.fdroid.fdroid.mock; + +import org.fdroid.fdroid.data.Repo; + +public class MockRepo extends Repo { + + public MockRepo(long repoId) { + id = repoId; + } + +}