From 76070f09ff48cacfd19f2c49d19f7188f2c24b2d Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 28 Dec 2022 18:11:03 -0300 Subject: [PATCH] [app] Fix weight (priority) of additional repos Our old DbHelper code would assign all additional repos a weight of 0 which the new database implementation doesn't like. Apps from those repos would simply not appear as if the repo is disabled. This fix simplifies the weight assignment of initial repos by incrementing the weight for each repo, so the weight assignment works the same way as for manually added repos: later repos get a higher weight. Incidentally, this also unifies the format of `additional_repos.xml` and `default_repos.xml`. --- .../java/org/fdroid/fdroid/data/DBHelper.java | 43 +++++++++++-------- app/src/main/res/values/default_repos.xml | 8 ---- .../org/fdroid/fdroid/data/DBHelperTest.java | 12 +++--- 3 files changed, 31 insertions(+), 32 deletions(-) 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 57cd94741..806d221d0 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -44,6 +44,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedList; import java.util.List; @@ -58,7 +59,7 @@ import java.util.List; public class DBHelper { private static final String TAG = "DBHelper"; - static final int REPO_XML_ITEM_COUNT = 8; + static final int REPO_XML_ITEM_COUNT = 7; public static FDroidDatabase getDb(Context context) { return FDroidDatabaseHolder.getDb(context, "fdroid_db", db -> prePopulateDb(context, db)); @@ -68,15 +69,16 @@ public class DBHelper { @VisibleForTesting static void prePopulateDb(Context context, FDroidDatabase db) { List initialRepos = DBHelper.loadInitialRepos(context); + int weight = 1; for (int i = 0; i < initialRepos.size(); i += REPO_XML_ITEM_COUNT) { InitialRepository repo = new InitialRepository( initialRepos.get(i), // name initialRepos.get(i + 1), // address initialRepos.get(i + 2), // description - initialRepos.get(i + 7), // certificate + initialRepos.get(i + 6), // certificate Integer.parseInt(initialRepos.get(i + 3)), // version initialRepos.get(i + 4).equals("1"), // enabled - Integer.parseInt(initialRepos.get(i + 5)) // weight + weight++ // weight ); db.getRepositoryDao().insert(repo); } @@ -125,22 +127,32 @@ public class DBHelper { */ static List loadInitialRepos(Context context) throws IllegalArgumentException { String packageName = context.getPackageName(); - List initialRepos = DBHelper.loadAdditionalRepos(packageName); - List defaultRepos = Arrays.asList(context.getResources().getStringArray(R.array.default_repos)); - initialRepos.addAll(defaultRepos); - if (initialRepos.size() % REPO_XML_ITEM_COUNT != 0) { - throw new IllegalArgumentException("default_repos.xml has wrong item count: " + - initialRepos.size() + " % REPO_XML_ARG_COUNT(" + REPO_XML_ITEM_COUNT + ") != 0"); + // get additional repos from OS (lowest priority/weight) + List additionalRepos = DBHelper.loadAdditionalRepos(packageName); + if (additionalRepos.size() % REPO_XML_ITEM_COUNT != 0) { + throw new IllegalArgumentException("additional_repos.xml has wrong item count: " + + additionalRepos.size() + " % REPO_XML_ARG_COUNT(" + REPO_XML_ITEM_COUNT + ") != 0"); } + // get our own default repos (higher priority/weight) + List defaultRepos = Arrays.asList(context.getResources().getStringArray(R.array.default_repos)); + if (defaultRepos.size() % REPO_XML_ITEM_COUNT != 0) { + throw new IllegalArgumentException("default_repos.xml has wrong item count: " + + defaultRepos.size() + " % REPO_XML_ARG_COUNT(" + REPO_XML_ITEM_COUNT + ") != 0"); + } + + List repos = new ArrayList<>(additionalRepos.size() + defaultRepos.size()); + repos.addAll(additionalRepos); + repos.addAll(defaultRepos); + final int descriptionIndex = 2; - for (int i = descriptionIndex; i < initialRepos.size(); i += REPO_XML_ITEM_COUNT) { - String description = initialRepos.get(i); - initialRepos.set(i, description.replaceAll("\\s+", " ")); + for (int i = descriptionIndex; i < repos.size(); i += REPO_XML_ITEM_COUNT) { + String description = repos.get(i); + repos.set(i, description.replaceAll("\\s+", " ")); } - return initialRepos; + return repos; } /** @@ -208,11 +220,6 @@ public class DBHelper { } xmlInputStream.close(); - final int priorityIndex = 5; - for (int i = priorityIndex; i < repoItems.size(); i += REPO_XML_ITEM_COUNT) { - repoItems.add(i, "0"); - } - if (repoItems.size() % REPO_XML_ITEM_COUNT == 0) { return repoItems; } diff --git a/app/src/main/res/values/default_repos.xml b/app/src/main/res/values/default_repos.xml index 58239bb9e..3d64dccae 100644 --- a/app/src/main/res/values/default_repos.xml +++ b/app/src/main/res/values/default_repos.xml @@ -15,8 +15,6 @@ 13 0 - - 1 ignore @@ -35,8 +33,6 @@ 13 1 - - 2 ignore @@ -56,8 +52,6 @@ 13 0 - - 3 ignore @@ -78,8 +72,6 @@ 13 0 - - 4 ignore diff --git a/app/src/test/java/org/fdroid/fdroid/data/DBHelperTest.java b/app/src/test/java/org/fdroid/fdroid/data/DBHelperTest.java index 8dd1bd818..7fd454aca 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/DBHelperTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/DBHelperTest.java @@ -28,6 +28,7 @@ import androidx.test.core.app.ApplicationProvider; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -201,8 +202,8 @@ public class DBHelperTest { List repos = getReposFromXml(validXml); assertEquals(2 * DBHelper.REPO_XML_ITEM_COUNT, repos.size()); - assertEquals("Repo Name", repos.get(8)); - assertEquals("https://www.oem0.com/yeah/repo", repos.get(9)); + assertEquals("Repo Name", repos.get(7)); + assertEquals("https://www.oem0.com/yeah/repo", repos.get(8)); } @Test @@ -228,7 +229,6 @@ public class DBHelperTest { "I'm the first oem repo.", "22", "1", - "0", // priority is inserted by DBHelper.parseAdditionalReposXml() "ignore", "fffff2313aaaaabcccc111"); List oem1 = Arrays.asList( @@ -237,7 +237,6 @@ public class DBHelperTest { "Who is the first repo?", "22", "0", - "0", // priority is inserted by DBHelper.parseAdditionalReposXml() "ignore", "ddddddd2313aaaaabcccc111"); List shouldBeRepos = new LinkedList<>(); @@ -259,6 +258,7 @@ public class DBHelperTest { if (oemEtcDir.canWrite() || new File("/").canWrite()) { oemEtcPackageDir.mkdirs(); } + assumeTrue(oemEtcPackageDir.isDirectory()); if (TextUtils.isEmpty(System.getenv("CI")) && !oemEtcPackageDir.isDirectory()) { Log.e(TAG, "Cannot create " + oemEtcDir + ", skipping test!"); return; @@ -308,9 +308,9 @@ public class DBHelperTest { // Construct the repos that we should have loaded List oem0 = Arrays.asList("oem0Name", "https://www.oem0.com/yeah/repo", "I'm the first oem repo.", - "22", "1", "0", "ignore", "fffff2313aaaaabcccc111"); + "22", "1", "ignore", "fffff2313aaaaabcccc111"); List oem1 = Arrays.asList("oem1MyNameIs", "https://www.mynameis.com/rapper/repo", "Who is the first repo?", - "22", "0", "0", "ignore", "ddddddd2313aaaaabcccc111"); + "22", "0", "ignore", "ddddddd2313aaaaabcccc111"); String[] defaultRepos = context.getResources().getStringArray(R.array.default_repos);