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, 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, 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, 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 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 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 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); } }