Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'fix-202' into 'master'

Browse Source 
Fix issue #202 - crash due to sqlite parameter limit being hit.

*NOTE: Queuing here for merge after next stable.*

The queries which have the potential to cause crashes due to too many parameters in the `ApkProvider` are now encapsulated in `ApkProvider` and can only be accessed by safe helper methods, which alleviate the problem by breaking big requests down into many smaller requests.

This will probably have to be done for the `ApkProvider`, but leaving for now because the limit is twice as big.

See merge request !70
This commit is contained in:
Peter Serwylo 2015-04-28 21:35:46 +00:00
commit 1b114b6bae
3 changed files with 63 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
F-Droid/src/org/fdroid/fdroid/UpdateService.java
View File

@ -605,23 +605,6 @@ public class UpdateService extends IntentService implements ProgressListener {
return knownIds;
}
/**
* If you call this with too many apks, then it will likely hit limit of
* parameters allowed for sqlite3 query. Rather, you should use
* {@link org.fdroid.fdroid.UpdateService#getKnownApks(java.util.List)}
* instead, which will only call this with the right number of apks at
* a time.
* @see org.fdroid.fdroid.UpdateService#getKnownAppIds(java.util.List)
*/
private List<Apk> getKnownApksFromProvider(List<Apk> apks) {
final String[] fields = {
ApkProvider.DataColumns.APK_ID,
ApkProvider.DataColumns.VERSION,
ApkProvider.DataColumns.VERSION_CODE
};
return ApkProvider.Helper.knownApks(this, apks, fields);
}
private void updateOrInsertApps(List<App> appsToUpdate, int totalUpdateCount, int currentCount) {
List<ContentProviderOperation> operations = new ArrayList<>();
@ -663,17 +646,12 @@ public class UpdateService extends IntentService implements ProgressListener {
* Return list of apps from the "apks" argument which are already in the database.
*/
private List<Apk> getKnownApks(List<Apk> apks) {
List<Apk> knownApks = new ArrayList<>();
if (apks.size() > ApkProvider.MAX_APKS_TO_QUERY) {
int middle = apks.size() / 2;
List<Apk> apks1 = apks.subList(0, middle);
List<Apk> apks2 = apks.subList(middle, apks.size());
knownApks.addAll(getKnownApks(apks1));
knownApks.addAll(getKnownApks(apks2));
} else {
knownApks.addAll(getKnownApksFromProvider(apks));
}
return knownApks;
final String[] fields = {
ApkProvider.DataColumns.APK_ID,
ApkProvider.DataColumns.VERSION,
ApkProvider.DataColumns.VERSION_CODE
};
return ApkProvider.Helper.knownApks(this, apks, fields);
}
private void updateOrInsertApks(List<Apk> apksToUpdate, int totalApksAppsCount, int currentCount) {

43
F-Droid/src/org/fdroid/fdroid/data/ApkProvider.java
View File

@ -24,7 +24,7 @@ public class ApkProvider extends FDroidProvider {
* we may want to add additional constraints, so we give our self some
* room by saying only 450 apks can be queried at once.
*/
public static final int MAX_APKS_TO_QUERY = 450;
protected static final int MAX_APKS_TO_QUERY = 450;
public static final class Helper {
@ -64,7 +64,19 @@ public class ApkProvider extends FDroidProvider {
resolver.delete(uri, null, null);
}
public static void deleteApks(Context context, List<Apk> apks) {
public static void deleteApks(final Context context, final List<Apk> apks) {
if (apks.size() > ApkProvider.MAX_APKS_TO_QUERY) {
int middle = apks.size() / 2;
List<Apk> apks1 = apks.subList(0, middle);
List<Apk> 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<Apk> apks) {
ContentResolver resolver = context.getContentResolver();
final Uri uri = getContentUri(apks);
resolver.delete(uri, null, null);
@ -106,11 +118,26 @@ public class ApkProvider extends FDroidProvider {
* Returns apks in the database, which have the same id and version as
* one of the apks in the "apks" argument.
*/
public static List<Apk> knownApks(Context context,
List<Apk> apks, String[] fields) {
public static List<Apk> knownApks(Context context, List<Apk> apks, String[] fields) {
if (apks.size() == 0) {
return new ArrayList<>();
}
List<Apk> knownApks = new ArrayList<>();
if (apks.size() > ApkProvider.MAX_APKS_TO_QUERY) {
int middle = apks.size() / 2;
List<Apk> apks1 = apks.subList(0, middle);
List<Apk> apks2 = apks.subList(middle, apks.size());
knownApks.addAll(knownApks(context, apks1, fields));
knownApks.addAll(knownApks(context,apks2, fields));
} else {
knownApks.addAll(knownApksSafe(context,apks, fields));
}
return knownApks;
}
private static List<Apk> knownApksSafe(final Context context, final List<Apk> apks, final String[] fields) {
ContentResolver resolver = context.getContentResolver();
final Uri uri = getContentUri(apks);
Cursor cursor = resolver.query(uri, fields, null, null, null);
@ -238,7 +265,13 @@ public class ApkProvider extends FDroidProvider {
.build();
}
public static Uri getContentUri(List<Apk> apks) {
/**
* Intentionally left protected because it will break if apks is larger than
* {@link org.fdroid.fdroid.data.ApkProvider#MAX_APKS_TO_QUERY}. Instead of using
* this directly, think about using
* {@link org.fdroid.fdroid.data.ApkProvider.Helper#knownApks(android.content.Context, java.util.List, String[])}
*/
protected static Uri getContentUri(List<Apk> apks) {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < apks.size(); i++) {
if (i != 0) {

25
F-Droid/test/src/org/fdroid/fdroid/ApkProviderTest.java
View File

@ -16,6 +16,19 @@ import java.util.List;
public class ApkProviderTest extends BaseApkProviderTest {
/**
* I want to test the protected {@link org.fdroid.fdroid.data.ApkProvider#getContentUri(java.util.List)}
* method, but don't want to make it public. This exposes it.
*/
private static class PublicApkProvider extends ApkProvider {
public static final int MAX_APKS_TO_QUERY = ApkProvider.MAX_APKS_TO_QUERY;
public static Uri getContentUri(List<Apk> apks) {
return ApkProvider.getContentUri(apks);
}
}
public void testUris() {
assertInvalidUri(ApkProvider.getAuthority());
assertInvalidUri(RepoProvider.getContentUri());
@ -29,15 +42,15 @@ public class ApkProviderTest extends BaseApkProviderTest {
assertValidUri(ApkProvider.getAppUri("org.fdroid.fdroid"));
assertValidUri(ApkProvider.getContentUri(new MockApk("org.fdroid.fdroid", 100)));
assertValidUri(ApkProvider.getContentUri());
assertValidUri(ApkProvider.getContentUri(apks));
assertValidUri(PublicApkProvider.getContentUri(apks));
assertValidUri(ApkProvider.getContentUri("org.fdroid.fdroid", 100));
assertValidUri(ApkProvider.getRepoUri(1000));
List<Apk> manyApks = new ArrayList<Apk>(ApkProvider.MAX_APKS_TO_QUERY - 5);
for (int i = 0; i < ApkProvider.MAX_APKS_TO_QUERY - 1; i ++) {
List<Apk> manyApks = new ArrayList<Apk>(PublicApkProvider.MAX_APKS_TO_QUERY - 5);
for (int i = 0; i < PublicApkProvider.MAX_APKS_TO_QUERY - 1; i ++) {
manyApks.add(new MockApk("com.example." + i, i));
}
assertValidUri(ApkProvider.getContentUri(manyApks));
assertValidUri(PublicApkProvider.getContentUri(manyApks));
manyApks.add(new MockApk("org.fdroid.fdroid.1", 1));
manyApks.add(new MockApk("org.fdroid.fdroid.2", 2));
@ -46,7 +59,7 @@ public class ApkProviderTest extends BaseApkProviderTest {
// throw an UnsupportedOperationException. However it
// is still not okay (we run out of bindable parameters
// in the sqlite query.
assertValidUri(ApkProvider.getContentUri(manyApks));
assertValidUri(PublicApkProvider.getContentUri(manyApks));
fail();
} catch (IllegalArgumentException e) {
// This is the expected error behaviour.
@ -93,7 +106,7 @@ public class ApkProviderTest extends BaseApkProviderTest {
assertCantUpdate(ApkProvider.getContentUri());
assertCantUpdate(ApkProvider.getAppUri("org.fdroid.fdroid"));
assertCantUpdate(ApkProvider.getRepoUri(1));
assertCantUpdate(ApkProvider.getContentUri(apks));
assertCantUpdate(PublicApkProvider.getContentUri(apks));
assertCantUpdate(Uri.withAppendedPath(ApkProvider.getContentUri(), "some-random-path"));
// The only valid ones are: