From a7a7f77b42ef458762318954611641ce89d480f7 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 13 Oct 2016 10:17:54 +1100
Subject: [PATCH] Refactored categories field from column in metadata table to
join table.
When updating existing apps or inserting new apps, instead of splatting
a comma separated list into a single sqlite3 column, we now put it into
several rows in the `CatJoinTable`. This is done after deleting existing
categories, to make sure that if the app has been removed from a category,
then this is reflected during the update.
---
.../main/java/org/fdroid/fdroid/data/App.java | 7 +-
.../org/fdroid/fdroid/data/AppProvider.java | 65 +++++++++++++++----
.../java/org/fdroid/fdroid/data/DBHelper.java | 14 +++-
.../java/org/fdroid/fdroid/data/Schema.java | 16 +++--
.../fdroid/fdroid/data/TempAppProvider.java | 32 ++++++++-
.../fdroid/data/CategoryProviderTest.java | 2 +-
6 files changed, 111 insertions(+), 25 deletions(-)
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 9bce69640..c72d184b9 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/App.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/App.java
@@ -101,6 +101,8 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
/**
* List of categories (as defined in the metadata documentation) or null if there aren't any.
+ * This is only populated when parsing a repository. If you need to know about the categories
+ * an app is in any other part of F-Droid, use the {@link CategoryProvider}.
*/
public String[] categories;
@@ -230,9 +232,6 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
case Cols.LAST_UPDATED:
lastUpdated = Utils.parseDate(cursor.getString(i), null);
break;
- case Cols.CATEGORIES:
- categories = Utils.parseCommaSeparatedString(cursor.getString(i));
- break;
case Cols.ANTI_FEATURES:
antiFeatures = Utils.parseCommaSeparatedString(cursor.getString(i));
break;
@@ -504,7 +503,7 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
values.put(Cols.SUGGESTED_VERSION_CODE, suggestedVersionCode);
values.put(Cols.UPSTREAM_VERSION_NAME, upstreamVersionName);
values.put(Cols.UPSTREAM_VERSION_CODE, upstreamVersionCode);
- values.put(Cols.CATEGORIES, Utils.serializeCommaSeparatedString(categories));
+ values.put(Cols.Categories.CATEGORIES, Utils.serializeCommaSeparatedString(categories));
values.put(Cols.ANTI_FEATURES, Utils.serializeCommaSeparatedString(antiFeatures));
values.put(Cols.REQUIREMENTS, Utils.serializeCommaSeparatedString(requirements));
values.put(Cols.IS_COMPATIBLE, compatible ? 1 : 0);
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 e23b2830b..a04e845f0 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
@@ -109,7 +109,7 @@ public class AppProvider extends FDroidProvider {
public static List<String> categories(Context context) {
final ContentResolver resolver = context.getContentResolver();
final Uri uri = getContentUri();
- final String[] projection = {Cols.CATEGORIES};
+ final String[] projection = {Cols.Categories.CATEGORIES};
final Cursor cursor = resolver.query(uri, projection, null, null, null);
final Set<String> categorySet = new HashSet<>();
if (cursor != null) {
@@ -268,7 +268,6 @@ public class AppProvider extends FDroidProvider {
private boolean isSuggestedApkTableAdded;
private boolean requiresInstalledTable;
private boolean requiresLeftJoinToPrefs;
- private boolean categoryFieldAdded;
private boolean countFieldAppended;
@Override
@@ -288,14 +287,10 @@ public class AppProvider extends FDroidProvider {
" LEFT JOIN " + apk + " ON (" + apk + "." + ApkTable.Cols.APP_ID + " = " + app + "." + Cols.ROW_ID + ") ";
}
- @Override
- protected boolean isDistinct() {
- return fieldCount() == 1 && categoryFieldAdded;
- }
-
@Override
protected String groupBy() {
- // If the count field has been requested, then we want to group all rows together.
+ // If the count field has been requested, then we want to group all rows together. Otherwise
+ // we will only group all the rows belonging to a single app together.
return countFieldAppended ? null : getTableName() + "." + Cols.ROW_ID;
}
@@ -362,10 +357,10 @@ public class AppProvider extends FDroidProvider {
case Cols._COUNT:
appendCountField();
break;
+ case Cols.Categories.CATEGORIES:
+ appendCategoriesField();
+ break;
default:
- if (field.equals(Cols.CATEGORIES)) {
- categoryFieldAdded = true;
- }
appendField(field, getTableName());
break;
}
@@ -393,6 +388,10 @@ public class AppProvider extends FDroidProvider {
appendField(fieldName, "suggestedApk", alias);
}
+ private void appendCategoriesField() {
+ appendField("GROUP_CONCAT(" + CategoryTable.NAME + "." + CategoryTable.Cols.NAME + ")", null, Cols.Categories.CATEGORIES);
+ }
+
private void addInstalledAppVersionName() {
addInstalledAppField(
InstalledAppTable.Cols.VERSION_NAME,
@@ -586,6 +585,10 @@ public class AppProvider extends FDroidProvider {
return AppMetadataTable.NAME;
}
+ protected String getCatJoinTableName() {
+ return CatJoinTable.NAME;
+ }
+
protected String getApkTableName() {
return ApkTable.NAME;
}
@@ -904,13 +907,51 @@ public class AppProvider extends FDroidProvider {
values.remove(Cols.Package.PACKAGE_NAME);
values.put(Cols.PACKAGE_ID, packageId);
- db().insertOrThrow(getTableName(), null, values);
+ String[] categories = null;
+ boolean saveCategories = false;
+ if (values.containsKey(Cols.Categories.CATEGORIES)) {
+ // Hold onto these categories, so that after we have an ID to reference the newly inserted
+ // app metadata we can then specify its categories.
+ saveCategories = true;
+ categories = Utils.parseCommaSeparatedString(values.getAsString(Cols.Categories.CATEGORIES));
+ values.remove(Cols.Categories.CATEGORIES);
+ }
+
+ long appMetadataId = db().insertOrThrow(getTableName(), null, values);
if (!isApplyingBatch()) {
getContext().getContentResolver().notifyChange(uri, null);
}
+
+ if (saveCategories) {
+ ensureCategories(categories, appMetadataId);
+ }
+
return getSpecificAppUri(values.getAsString(PackageTable.Cols.PACKAGE_NAME), values.getAsLong(Cols.REPO_ID));
}
+ protected void ensureCategories(String[] categories, long appMetadataId) {
+ db().delete(getCatJoinTableName(), CatJoinTable.Cols.APP_METADATA_ID + " = ?", new String[] {Long.toString(appMetadataId)});
+ if (categories != null) {
+ Set<String> categoriesSet = new HashSet<>();
+ for (String categoryName : categories) {
+
+ // There is nothing stopping a server repeating a category name in the metadata of
+ // an app. In order to prevent unique constraint violations, only insert once into
+ // the join table.
+ if (categoriesSet.contains(categoryName)) {
+ continue;
+ }
+
+ categoriesSet.add(categoryName);
+ long categoryId = CategoryProvider.Helper.ensureExists(getContext(), categoryName);
+ ContentValues categoryValues = new ContentValues(2);
+ categoryValues.put(CatJoinTable.Cols.APP_METADATA_ID, appMetadataId);
+ categoryValues.put(CatJoinTable.Cols.CATEGORY_ID, categoryId);
+ db().insert(getCatJoinTableName(), null, categoryValues);
+ }
+ }
+ }
+
@Override
public int update(Uri uri, ContentValues values, String where, String[] whereArgs) {
if (MATCHER.match(uri) != CALC_APP_DETAILS_FROM_INDEX) {
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 72dcebf57..4803e78b3 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
@@ -131,7 +131,6 @@ class DBHelper extends SQLiteOpenHelper {
+ AppMetadataTable.Cols.LITECOIN_ADDR + " string,"
+ AppMetadataTable.Cols.FLATTR_ID + " string,"
+ AppMetadataTable.Cols.REQUIREMENTS + " string,"
- + AppMetadataTable.Cols.CATEGORIES + " string,"
+ AppMetadataTable.Cols.ADDED + " string,"
+ AppMetadataTable.Cols.LAST_UPDATED + " string,"
+ AppMetadataTable.Cols.IS_COMPATIBLE + " int not null,"
@@ -151,11 +150,20 @@ class DBHelper extends SQLiteOpenHelper {
+ Schema.CategoryTable.Cols.NAME + " TEXT NOT NULL "
+ " );";
- private static final String CREATE_TABLE_CAT_JOIN = "CREATE TABLE " + CatJoinTable.NAME
+ /**
+ * The order of the two columns in the primary key matters for this table. The index that is
+ * built for sqlite to quickly search the primary key will be sorted by app metadata id first,
+ * and category id second. This means that we don't need a separate individual index on the
+ * app metadata id, because it can instead look through the primary key index. This can be
+ * observed by flipping the order of the primary key columns, and noting the resulting sqlite
+ * logs along the lines of:
+ * E/SQLiteLog(14164): (284) automatic index on fdroid_categoryAppMetadataJoin(appMetadataId)
+ */
+ static final String CREATE_TABLE_CAT_JOIN = "CREATE TABLE " + CatJoinTable.NAME
+ " ( "
+ CatJoinTable.Cols.APP_METADATA_ID + " INT NOT NULL, "
+ CatJoinTable.Cols.CATEGORY_ID + " INT NOT NULL, "
- + "primary key(" + CatJoinTable.Cols.CATEGORY_ID + ", " + CatJoinTable.Cols.APP_METADATA_ID + ") "
+ + "primary key(" + CatJoinTable.Cols.APP_METADATA_ID + ", " + CatJoinTable.Cols.CATEGORY_ID + ") "
+ " );";
private static final String CREATE_TABLE_INSTALLED_APP = "CREATE TABLE " + InstalledAppTable.NAME
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 7deb04595..ea795f7d8 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java
@@ -96,6 +96,11 @@ public interface Schema {
* @see CategoryTable
*/
String CATEGORY_ID = "categoryId";
+
+ /**
+ * @see AppMetadataTable.Cols#ALL_COLS
+ */
+ String[] ALL_COLS = {APP_METADATA_ID, CATEGORY_ID};
}
}
@@ -134,7 +139,6 @@ public interface Schema {
String UPSTREAM_VERSION_CODE = "upstreamVercode";
String ADDED = "added";
String LAST_UPDATED = "lastUpdated";
- String CATEGORIES = "categories";
String ANTI_FEATURES = "antiFeatures";
String REQUIREMENTS = "requirements";
String ICON_URL = "iconUrl";
@@ -154,6 +158,10 @@ public interface Schema {
String PACKAGE_NAME = "package_packageName";
}
+ interface Categories {
+ String CATEGORIES = "categories_commaSeparatedCateogryNames";
+ }
+
/**
* Each of the physical columns in the sqlite table. Differs from {@link Cols#ALL} in
* that it doesn't include fields which are aliases of other fields (e.g. {@link Cols#_ID}
@@ -164,7 +172,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, ICON_URL, ICON_URL_LARGE,
+ ANTI_FEATURES, REQUIREMENTS, ICON_URL, ICON_URL_LARGE,
SUGGESTED_VERSION_CODE,
};
@@ -178,10 +186,10 @@ 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, ICON_URL, ICON_URL_LARGE,
+ ANTI_FEATURES, REQUIREMENTS, ICON_URL, ICON_URL_LARGE,
SUGGESTED_VERSION_CODE, SuggestedApk.VERSION_NAME,
InstalledApp.VERSION_CODE, InstalledApp.VERSION_NAME,
- InstalledApp.SIGNATURE, Package.PACKAGE_NAME,
+ InstalledApp.SIGNATURE, Package.PACKAGE_NAME, Categories.CATEGORIES,
};
}
}
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 6ebc1112f..2770d891e 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java
@@ -11,9 +11,11 @@ import android.text.TextUtils;
import java.util.List;
+import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.data.Schema.ApkTable;
import org.fdroid.fdroid.data.Schema.AppMetadataTable;
import org.fdroid.fdroid.data.Schema.AppMetadataTable.Cols;
+import org.fdroid.fdroid.data.Schema.CatJoinTable;
import org.fdroid.fdroid.data.Schema.PackageTable;
/**
@@ -29,6 +31,7 @@ public class TempAppProvider extends AppProvider {
private static final String PROVIDER_NAME = "TempAppProvider";
static final String TABLE_TEMP_APP = "temp_" + AppMetadataTable.NAME;
+ static final String TABLE_TEMP_CAT_JOIN = "temp_" + CatJoinTable.NAME;
private static final String PATH_INIT = "init";
private static final String PATH_COMMIT = "commit";
@@ -51,6 +54,11 @@ public class TempAppProvider extends AppProvider {
return TABLE_TEMP_APP;
}
+ @Override
+ protected String getCatJoinTableName() {
+ return TABLE_TEMP_CAT_JOIN;
+ }
+
public static String getAuthority() {
return AUTHORITY + "." + PROVIDER_NAME;
}
@@ -153,6 +161,12 @@ public class TempAppProvider extends AppProvider {
// Package names for apps cannot change...
values.remove(Cols.Package.PACKAGE_NAME);
+ if (values.containsKey(Cols.Categories.CATEGORIES)) {
+ String[] categories = Utils.parseCommaSeparatedString(values.getAsString(Cols.Categories.CATEGORIES));
+ ensureCategories(categories, packageName, repoId);
+ values.remove(Cols.Categories.CATEGORIES);
+ }
+
int count = db().update(getTableName(), values, query.getSelection(), query.getArgs());
if (!isApplyingBatch()) {
getContext().getContentResolver().notifyChange(getHighestPriorityMetadataUri(packageName), null);
@@ -160,6 +174,16 @@ public class TempAppProvider extends AppProvider {
return count;
}
+ private void ensureCategories(String[] categories, String packageName, long repoId) {
+ QuerySelection forId = querySingle(packageName, repoId);
+ Cursor cursor = db().query(getTableName(), new String[] {Cols.ROW_ID}, forId.getSelection(), forId.getArgs(), null, null, null);
+ cursor.moveToFirst();
+ long appMetadataId = cursor.getLong(0);
+ cursor.close();
+
+ ensureCategories(categories, appMetadataId);
+ }
+
@Override
public Cursor query(Uri uri, String[] projection, String customSelection, String[] selectionArgs, String sortOrder) {
AppQuerySelection selection = new AppQuerySelection(customSelection, selectionArgs);
@@ -188,7 +212,9 @@ public class TempAppProvider extends AppProvider {
ensureTempTableDetached(db);
db.execSQL("ATTACH DATABASE ':memory:' AS " + DB);
db.execSQL(DBHelper.CREATE_TABLE_APP_METADATA.replaceFirst(AppMetadataTable.NAME, DB + "." + getTableName()));
+ db.execSQL(DBHelper.CREATE_TABLE_CAT_JOIN.replaceFirst(CatJoinTable.NAME, DB + "." + getCatJoinTableName()));
db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, AppMetadataTable.NAME, DB + "." + getTableName()));
+ db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, CatJoinTable.NAME, DB + "." + getCatJoinTableName()));
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_id ON " + getTableName() + " (" + AppMetadataTable.Cols.PACKAGE_ID + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_upstreamVercode ON " + getTableName() + " (" + AppMetadataTable.Cols.UPSTREAM_VERSION_CODE + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_compatible ON " + getTableName() + " (" + AppMetadataTable.Cols.IS_COMPATIBLE + ");");
@@ -208,8 +234,9 @@ public class TempAppProvider extends AppProvider {
try {
db.beginTransaction();
- final String tempApp = DB + "." + TempAppProvider.TABLE_TEMP_APP;
+ final String tempApp = DB + "." + TABLE_TEMP_APP;
final String tempApk = DB + "." + TempApkProvider.TABLE_TEMP_APK;
+ final String tempCatJoin = DB + "." + TABLE_TEMP_CAT_JOIN;
db.execSQL("DELETE FROM " + AppMetadataTable.NAME + " WHERE 1");
db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, tempApp, AppMetadataTable.NAME));
@@ -217,6 +244,9 @@ public class TempAppProvider extends AppProvider {
db.execSQL("DELETE FROM " + ApkTable.NAME + " WHERE 1");
db.execSQL(copyData(ApkTable.Cols.ALL_COLS, tempApk, ApkTable.NAME));
+ db.execSQL("DELETE FROM " + CatJoinTable.NAME + " WHERE 1");
+ db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, tempCatJoin, CatJoinTable.NAME));
+
db.setTransactionSuccessful();
getContext().getContentResolver().notifyChange(AppProvider.getContentUri(), null);
diff --git a/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java
index f36147cb1..2fa3dbe20 100644
--- a/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java
+++ b/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java
@@ -154,7 +154,7 @@ public class CategoryProviderTest extends FDroidProviderTest {
private void insertAppWithCategory(String id, String name, String categories) {
ContentValues values = new ContentValues(1);
- values.put(Cols.CATEGORIES, categories);
+ values.put(Cols.Categories.CATEGORIES, categories);
AppProviderTest.insertApp(contentResolver, context, id, name, values);
}
}