diff --git a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java index a8de65bb9..ac5f7eb7a 100644 --- a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java +++ b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java @@ -6,7 +6,6 @@ import android.net.Uri; import android.support.annotation.NonNull; import android.text.TextUtils; import android.util.Log; - import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.PropertyAccessor; import com.fasterxml.jackson.core.JsonFactory; @@ -15,7 +14,6 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.InjectableValues; import com.fasterxml.jackson.databind.ObjectMapper; - import org.apache.commons.io.FileUtils; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.App; @@ -138,14 +136,21 @@ public class IndexV1Updater extends RepoUpdater { /** * Parses the index and feeds it to the database via {@link Repo}, {@link App}, - * and {@link Apk} instances. + * and {@link Apk} instances. This uses {@link RepoPersister} to add the apps + * and packages to the database in {@link RepoPersister#saveToDb(App, List)} + * to write the {@link Repo}, and commit the whole thing in + * {@link RepoPersister#commit(ContentValues)}. One confusing thing about this + * whole process is that {@link RepoPersister} needs to first create and entry + * in the database, then fetch the ID from the database to populate + * {@link Repo#id}. That has to happen first, then the rest of the {@code Repo} + * data must be added later. * * @param indexInputStream {@link InputStream} to {@code index-v1.json} - * @param cacheTag the {@code etag} value from HTTP headers + * @param etag the {@code etag} value from HTTP headers * @throws IOException * @throws UpdateException */ - public void processIndexV1(InputStream indexInputStream, JarEntry indexEntry, String cacheTag) + public void processIndexV1(InputStream indexInputStream, JarEntry indexEntry, String etag) throws IOException, UpdateException { Utils.Profiler profiler = new Utils.Profiler(TAG); profiler.log("Starting to process index-v1.json"); @@ -200,6 +205,7 @@ public class IndexV1Updater extends RepoUpdater { // timestamp is absolutely required repo.timestamp = timestamp; // below are optional, can be null + repo.lastetag = etag; repo.name = getStringRepoValue(repoMap, "name"); repo.icon = getStringRepoValue(repoMap, "icon"); repo.description = getStringRepoValue(repoMap, "description"); @@ -234,15 +240,32 @@ public class IndexV1Updater extends RepoUpdater { repoPersister.saveToDb(app, apks); } } - profiler.log("Saved to database, but only a temporary table. Now persisting to database..."); - notifyCommittingToDb(); - ContentValues values = prepareRepoDetailsForSaving(repo.name, - repo.description, repo.maxage, repo.version, repo.timestamp, repo.icon, - repo.mirrors, cacheTag); - repoPersister.commit(values); + ContentValues contentValues = new ContentValues(); + contentValues.put(Schema.RepoTable.Cols.LAST_UPDATED, Utils.formatTime(new Date(), "")); + contentValues.put(Schema.RepoTable.Cols.TIMESTAMP, repo.timestamp); + contentValues.put(Schema.RepoTable.Cols.LAST_ETAG, repo.lastetag); + if (repo.version != Repo.INT_UNSET_VALUE) { + contentValues.put(Schema.RepoTable.Cols.VERSION, repo.version); + } + if (repo.maxage != Repo.INT_UNSET_VALUE) { + contentValues.put(Schema.RepoTable.Cols.MAX_AGE, repo.maxage); + } + if (repo.description != null) { + contentValues.put(Schema.RepoTable.Cols.DESCRIPTION, repo.description); + } + if (repo.name != null) { + contentValues.put(Schema.RepoTable.Cols.NAME, repo.name); + } + if (repo.icon != null) { + contentValues.put(Schema.RepoTable.Cols.ICON, repo.icon); + } + if (repo.mirrors != null && repo.mirrors.length > 0) { + contentValues.put(Schema.RepoTable.Cols.MIRRORS, Utils.serializeCommaSeparatedString(repo.mirrors)); + } + repoPersister.commit(contentValues); profiler.log("Persited to database."); diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index b63424f70..f77548e33 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java @@ -30,7 +30,6 @@ import android.content.pm.PackageManager; import android.support.annotation.NonNull; import android.text.TextUtils; import android.util.Log; - import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; @@ -48,6 +47,9 @@ import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.XMLReader; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParser; +import javax.xml.parsers.SAXParserFactory; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -62,10 +64,6 @@ import java.util.List; import java.util.jar.JarEntry; import java.util.jar.JarFile; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; - // TODO move to org.fdroid.fdroid.updater // TODO reduce visibility of methods once in .updater package (.e.g tests need it public now) @@ -297,11 +295,12 @@ public class RepoUpdater { /** * Update tracking data for the repo represented by this instance (index version, etag, - * description, human-readable name, etc. + * description, human-readable name, etc. This is not reused in {@link IndexV1Updater} + * because its too tied up into the old parsing flow in this class. */ - ContentValues prepareRepoDetailsForSaving(String name, String description, int maxAge, - int version, long timestamp, String icon, - String[] mirrors, String cacheTag) { + private ContentValues prepareRepoDetailsForSaving(String name, String description, int maxAge, + int version, long timestamp, String icon, + String[] mirrors, String cacheTag) { ContentValues values = new ContentValues(); values.put(RepoTable.Cols.LAST_UPDATED, Utils.formatTime(new Date(), "")); diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java index 6bcc7311a..849bee1b1 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java @@ -151,6 +151,13 @@ public class RepoProvider extends FDroidProvider { return repo; } + /** + * Updates the repo metadata in the database. All data comes from the + * index file except {@link Repo#id}, which is generated by the database. + * That makes for an two cycle workflow, where first this must be called + * to fetch the {@code Repo.id} from the database, then it is called a + * second time to actually set the repo metadata. + */ public static void update(Context context, Repo repo, ContentValues values) { ContentResolver resolver = context.getContentResolver(); diff --git a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java index 10675b127..41df377df 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java @@ -17,9 +17,12 @@ import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.RepoUpdater; import org.fdroid.fdroid.TestUtils; import org.fdroid.fdroid.data.Apk; +import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; +import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.FDroidProviderTest; import org.fdroid.fdroid.data.Repo; +import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.data.RepoPushRequest; import org.fdroid.fdroid.mock.RepoDetails; import org.junit.After; @@ -48,6 +51,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @Config(constants = BuildConfig.class, sdk = 24) @@ -71,6 +75,12 @@ public class IndexV1UpdaterTest extends FDroidProviderTest { @Test public void testIndexV1Processing() throws IOException, RepoUpdater.UpdateException { + List repos = RepoProvider.Helper.all(context); + for (Repo repo : repos) { + RepoProvider.Helper.remove(context, repo.getId()); + } + assertEquals("No repos present", 0, RepoProvider.Helper.all(context).size()); + assertEquals("No apps present", 0, AppProvider.Helper.all(context.getContentResolver()).size()); Repo repo = MultiRepoUpdaterTest.createRepo("Testy", TESTY_JAR, context, TESTY_CERT); repo.timestamp = 1481222110; IndexV1Updater updater = new IndexV1Updater(context, repo); @@ -80,6 +90,37 @@ public class IndexV1UpdaterTest extends FDroidProviderTest { InputStream indexInputStream = jarFile.getInputStream(indexEntry); updater.processIndexV1(indexInputStream, indexEntry, "fakeEtag"); IOUtils.closeQuietly(indexInputStream); + List apps = AppProvider.Helper.all(context.getContentResolver()); + assertEquals("53 apps present", 53, apps.size()); + + String[] packages = { + "fake.app.one", + "org.adaway", + "This_does_not_exist", + }; + for (String id : packages) { + assertEquals("No apks for " + id, 0, ApkProvider.Helper.findByPackageName(context, id).size()); + } + + for (App app : apps) { + assertTrue("Some apks for " + app.packageName, + ApkProvider.Helper.findByPackageName(context, app.packageName).size() > 0); + } + + repos = RepoProvider.Helper.all(context); + assertEquals("One repo", 1, repos.size()); + Repo repoFromDb = repos.get(0); + assertEquals("repo.timestamp should be set", 1481222111, repoFromDb.timestamp); + assertEquals("repo.address should be the same", repo.address, repoFromDb.address); + assertEquals("repo.name should be set", "non-public test repo", repoFromDb.name); + assertEquals("repo.maxage should be set", 0, repoFromDb.maxage); + assertEquals("repo.version should be set", 18, repoFromDb.version); + assertEquals("repo.icon should be set", "fdroid-icon.png", repoFromDb.icon); + String description = "This is a repository of apps to be used with F-Droid. Applications in this repository are either official binaries built by the original application developers, or are binaries built from source by the admin of f-droid.org using the tools on https://gitlab.com/u/fdroid. "; // NOCHECKSTYLE LineLength + assertEquals("repo.description should be set", description, repoFromDb.description); + assertEquals("repo.mirrors should have items", 2, repo.mirrors.length); + assertEquals("repo.mirrors first URL", "http://frkcchxlcvnb4m5a.onion/fdroid/repo", repo.mirrors[0]); + assertEquals("repo.mirrors second URL", "http://testy.at.or.at/fdroid/repo", repo.mirrors[1]); } @Test(expected = RepoUpdater.SigningException.class)