Only copy the apps/apks for the current repo to temp tables.

When preparing a temp database to write to, don't copy all apps/apks.
Instead, only copy those _not_ belonging to the repo we are updating.
In an ideal world, we'd not even need to copy them, but we need
their IDs to be in the temp database so that we don't accidentally
use the same auto-generated ID as the main database.

This also means that we can drop the check for "does this app exist,
and hence should we UPDATE it instead of INSERTing it?" and always
just insert it.

Then, when copying the temp table back to disk, first delete all
apps/apks _belonging to the repo being updated_. Then, copy back the
apks/apps we found in the repo. This again improves performance because
we no longer need to bopy back and forth data which we know
wont change (as evidenced by the fact it belongs to a differen trepo).

I don't think this was possible earlier before we did the work to
support repo priorities properly. That is because we had a single app
which was serviced by several repositories. Now, we have multiple
entries in the `fdroid_app` table, for each repo which supports
that app.
This commit is contained in:
Peter Serwylo 2017-07-14 15:24:22 +10:00
parent 8c3441939f
commit 5bde27daa8
5 changed files with 64 additions and 31 deletions

View File

@ -161,11 +161,11 @@ public class RepoPersister {
private ArrayList<ContentProviderOperation> insertOrUpdateApps(List<App> apps) { private ArrayList<ContentProviderOperation> insertOrUpdateApps(List<App> apps) {
ArrayList<ContentProviderOperation> operations = new ArrayList<>(apps.size()); ArrayList<ContentProviderOperation> operations = new ArrayList<>(apps.size());
for (App app : apps) { for (App app : apps) {
if (isAppInDatabase(app)) { /*if (isAppInDatabase(app)) {
operations.add(updateExistingApp(app)); operations.add(updateExistingApp(app));
} else { } else {
operations.add(insertNewApp(app)); */operations.add(insertNewApp(app));
} /*}*/
} }
return operations; return operations;
} }
@ -175,16 +175,16 @@ public class RepoPersister {
* will queue up an update or an insert {@link ContentProviderOperation} for each package. * will queue up an update or an insert {@link ContentProviderOperation} for each package.
*/ */
private ArrayList<ContentProviderOperation> insertOrUpdateApks(List<Apk> packages) { private ArrayList<ContentProviderOperation> insertOrUpdateApks(List<Apk> packages) {
String[] projection = new String[]{ /*String[] projection = new String[]{
Schema.ApkTable.Cols.Package.PACKAGE_NAME, Schema.ApkTable.Cols.Package.PACKAGE_NAME,
Schema.ApkTable.Cols.VERSION_CODE, Schema.ApkTable.Cols.VERSION_CODE,
Schema.ApkTable.Cols.REPO_ID, Schema.ApkTable.Cols.REPO_ID,
Schema.ApkTable.Cols.APP_ID, Schema.ApkTable.Cols.APP_ID,
}; };
List<Apk> existingApks = ApkProvider.Helper.knownApks(context, packages, projection); List<Apk> existingApks = ApkProvider.Helper.knownApks(context, packages, projection);*/
ArrayList<ContentProviderOperation> operations = new ArrayList<>(packages.size()); ArrayList<ContentProviderOperation> operations = new ArrayList<>(packages.size());
for (Apk apk : packages) { for (Apk apk : packages) {
boolean exists = false; /*boolean exists = false;
for (Apk existing : existingApks) { for (Apk existing : existingApks) {
if (existing.repoId == apk.repoId && existing.packageName.equals(apk.packageName) && existing.versionCode == apk.versionCode) { if (existing.repoId == apk.repoId && existing.packageName.equals(apk.packageName) && existing.versionCode == apk.versionCode) {
exists = true; exists = true;
@ -195,8 +195,8 @@ public class RepoPersister {
if (exists) { if (exists) {
operations.add(updateExistingApk(apk)); operations.add(updateExistingApk(apk));
} else { } else {
operations.add(insertNewApk(apk)); */operations.add(insertNewApk(apk));
} /*}*/
} }
return operations; return operations;

View File

@ -85,6 +85,8 @@ public interface Schema {
String NAME = "fdroid_categoryAppMetadataJoin"; String NAME = "fdroid_categoryAppMetadataJoin";
interface Cols { interface Cols {
String ROW_ID = "rowid";
/** /**
* Foreign key to {@link AppMetadataTable}. * Foreign key to {@link AppMetadataTable}.
* @see AppMetadataTable * @see AppMetadataTable
@ -100,7 +102,7 @@ public interface Schema {
/** /**
* @see AppMetadataTable.Cols#ALL_COLS * @see AppMetadataTable.Cols#ALL_COLS
*/ */
String[] ALL_COLS = {APP_METADATA_ID, CATEGORY_ID}; String[] ALL_COLS = {ROW_ID, APP_METADATA_ID, CATEGORY_ID};
} }
} }

View File

@ -7,6 +7,7 @@ import android.database.sqlite.SQLiteDatabase;
import android.net.Uri; import android.net.Uri;
import org.fdroid.fdroid.data.Schema.ApkTable; import org.fdroid.fdroid.data.Schema.ApkTable;
import org.fdroid.fdroid.data.Schema.ApkTable.Cols;
import java.util.List; import java.util.List;
@ -130,10 +131,14 @@ public class TempApkProvider extends ApkProvider {
private void initTable(long repoIdBeingUpdated) { private void initTable(long repoIdBeingUpdated) {
final SQLiteDatabase db = db(); final SQLiteDatabase db = db();
final String memoryDbName = TempAppProvider.DB; final String memoryDbName = TempAppProvider.DB;
db.execSQL(DBHelper.CREATE_TABLE_APK.replaceFirst(Schema.ApkTable.NAME, memoryDbName + "." + getTableName())); db.execSQL(DBHelper.CREATE_TABLE_APK.replaceFirst(ApkTable.NAME, memoryDbName + "." + getTableName()));
db.execSQL(TempAppProvider.copyData(Schema.ApkTable.Cols.ALL_COLS, Schema.ApkTable.NAME, memoryDbName + "." + getTableName()));
db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_appId on " + getTableName() + " (" + ApkTable.Cols.APP_ID + ");"); String where = ApkTable.NAME + "." + Cols.REPO_ID + " != ?";
db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + ApkTable.Cols.IS_COMPATIBLE + ");"); String[] whereArgs = new String[]{Long.toString(repoIdBeingUpdated)};
db.execSQL(TempAppProvider.copyData(Cols.ALL_COLS, ApkTable.NAME, memoryDbName + "." + getTableName(), where), whereArgs);
db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_appId on " + getTableName() + " (" + Cols.APP_ID + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + Cols.IS_COMPATIBLE + ");");
} }
} }

View File

@ -228,24 +228,40 @@ public class TempAppProvider extends AppProvider {
private void initTable(long repoIdBeingUpdated) { private void initTable(long repoIdBeingUpdated) {
final SQLiteDatabase db = db(); final SQLiteDatabase db = db();
String mainApp = AppMetadataTable.NAME;
String tempApp = DB + "." + getTableName();
String mainCat = CatJoinTable.NAME;
String tempCat = DB + "." + getCatJoinTableName();
ensureTempTableDetached(db); ensureTempTableDetached(db);
db.execSQL("ATTACH DATABASE ':memory:' AS " + 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_APP_METADATA.replaceFirst(AppMetadataTable.NAME, tempApp));
db.execSQL(DBHelper.CREATE_TABLE_CAT_JOIN.replaceFirst(CatJoinTable.NAME, DB + "." + getCatJoinTableName())); db.execSQL(DBHelper.CREATE_TABLE_CAT_JOIN.replaceFirst(CatJoinTable.NAME, tempCat));
db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, AppMetadataTable.NAME, DB + "." + getTableName()));
db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, CatJoinTable.NAME, DB + "." + getCatJoinTableName())); String appWhere = mainApp + "." + Cols.REPO_ID + " != ?";
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_id ON " + getTableName() + " (" + AppMetadataTable.Cols.PACKAGE_ID + ");"); String[] repoArgs = new String[]{Long.toString(repoIdBeingUpdated)};
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_upstreamVercode ON " + getTableName() + " (" + AppMetadataTable.Cols.UPSTREAM_VERSION_CODE + ");"); db.execSQL(copyData(Cols.ALL_COLS, mainApp, tempApp, appWhere), repoArgs);
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_compatible ON " + getTableName() + " (" + AppMetadataTable.Cols.IS_COMPATIBLE + ");");
// TODO: String catWhere = mainCat + "." + CatJoinTable.Cols..Cols.REPO_ID + " != ?";
db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, mainCat, tempCat, null));
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_id ON " + getTableName() + " (" + Cols.PACKAGE_ID + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_upstreamVercode ON " + getTableName() + " (" + Cols.UPSTREAM_VERSION_CODE + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_compatible ON " + getTableName() + " (" + Cols.IS_COMPATIBLE + ");");
} }
/** /**
* Constructs an INSERT INTO ... SELECT statement as a means from getting data from one table * Constructs an INSERT INTO ... SELECT statement as a means from getting data from one table
* into another. The list of columns to copy are explicitly specified using colsToCopy. * into another. The list of columns to copy are explicitly specified using colsToCopy.
*/ */
static String copyData(String[] colsToCopy, String fromTable, String toTable) { static String copyData(String[] colsToCopy, String fromTable, String toTable, String where) {
String cols = TextUtils.join(", ", colsToCopy); String cols = TextUtils.join(", ", colsToCopy);
return "INSERT INTO " + toTable + " (" + cols + ") SELECT " + cols + " FROM " + fromTable; String sql = "INSERT INTO " + toTable + " (" + cols + ") SELECT " + cols + " FROM " + fromTable;
if (!TextUtils.isEmpty(where)) {
sql += " WHERE " + where;
}
return sql;
} }
private void commitTable(long repoIdToCommit) { private void commitTable(long repoIdToCommit) {
@ -257,14 +273,16 @@ public class TempAppProvider extends AppProvider {
final String tempApk = DB + "." + TempApkProvider.TABLE_TEMP_APK; final String tempApk = DB + "." + TempApkProvider.TABLE_TEMP_APK;
final String tempCatJoin = DB + "." + TABLE_TEMP_CAT_JOIN; final String tempCatJoin = DB + "." + TABLE_TEMP_CAT_JOIN;
db.execSQL("DELETE FROM " + AppMetadataTable.NAME + " WHERE 1"); final String[] repoArgs = new String[]{Long.toString(repoIdToCommit)};
db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, tempApp, AppMetadataTable.NAME));
db.execSQL("DELETE FROM " + ApkTable.NAME + " WHERE 1"); db.execSQL("DELETE FROM " + AppMetadataTable.NAME + " WHERE " + Cols.REPO_ID + " = ?", repoArgs);
db.execSQL(copyData(ApkTable.Cols.ALL_COLS, tempApk, ApkTable.NAME)); db.execSQL(copyData(Cols.ALL_COLS, tempApp, AppMetadataTable.NAME, Cols.REPO_ID + " = ?"), repoArgs);
db.execSQL("DELETE FROM " + CatJoinTable.NAME + " WHERE 1"); db.execSQL("DELETE FROM " + ApkTable.NAME + " WHERE " + ApkTable.Cols.REPO_ID + " = ?", repoArgs);
db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, tempCatJoin, CatJoinTable.NAME)); db.execSQL(copyData(ApkTable.Cols.ALL_COLS, tempApk, ApkTable.NAME, ApkTable.Cols.REPO_ID + " = ?"), repoArgs);
db.execSQL("DELETE FROM " + CatJoinTable.NAME + " WHERE " + getCatRepoWhere(CatJoinTable.NAME), repoArgs);
db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, tempCatJoin, CatJoinTable.NAME, getCatRepoWhere(tempCatJoin)), repoArgs);
db.setTransactionSuccessful(); db.setTransactionSuccessful();
@ -276,4 +294,14 @@ public class TempAppProvider extends AppProvider {
db.execSQL("DETACH DATABASE " + DB); // Can't be done in a transaction. db.execSQL("DETACH DATABASE " + DB); // Can't be done in a transaction.
} }
} }
private String getCatRepoWhere(String categoryTable) {
String catRepoSubquery =
"SELECT DISTINCT innerCatJoin." + CatJoinTable.Cols.ROW_ID + " " +
"FROM " + categoryTable + " AS innerCatJoin " +
"JOIN " + getTableName() + " AS app ON (app." + Cols.ROW_ID + " = innerCatJoin." + CatJoinTable.Cols.APP_METADATA_ID + ") " +
"WHERE app." + Cols.REPO_ID + " = ?";
return CatJoinTable.Cols.ROW_ID + " IN (" + catRepoSubquery + ")";
}
} }

View File

@ -45,8 +45,6 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest {
@StringDef({"Conflicting", "Normal"}) @StringDef({"Conflicting", "Normal"})
public @interface RepoIdentifier { } public @interface RepoIdentifier { }
/*
*This test fails due to issue #568 (https://gitlab.com/fdroid/fdroidclient/issues/568).
@Test @Test
public void appsRemovedFromRepo() throws RepoUpdater.UpdateException { public void appsRemovedFromRepo() throws RepoUpdater.UpdateException {
assertEquals(0, AppProvider.Helper.all(context.getContentResolver()).size()); assertEquals(0, AppProvider.Helper.all(context.getContentResolver()).size());
@ -67,7 +65,7 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest {
assertEquals(6, ApkProvider.Helper.findByRepo(context, repo, Schema.ApkTable.Cols.ALL).size()); assertEquals(6, ApkProvider.Helper.findByRepo(context, repo, Schema.ApkTable.Cols.ALL).size());
assertEquals(4, ApkProvider.Helper.findByPackageName(context, "org.adaway").size()); assertEquals(4, ApkProvider.Helper.findByPackageName(context, "org.adaway").size());
assertEquals(2, ApkProvider.Helper.findByPackageName(context, "org.dgtale.icsimport").size()); assertEquals(2, ApkProvider.Helper.findByPackageName(context, "org.dgtale.icsimport").size());
}*/ }
@Test @Test
public void mainRepo() throws RepoUpdater.UpdateException { public void mainRepo() throws RepoUpdater.UpdateException {