Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

split IndexV1Updater's repo saving into working code

Browse Source 
RepoUpdater.prepareRepoDetailsForSaving() was broken here because the Repo
properties were being set before calling it, and then the Repo instance was
passed to it for comparison.  So the comparison was always saying the value
was unchanged.  In IndexV1Updater, the flow doesn't need those checks.

This also fixes the bug where added repos never had their name/description/
icon/etc show up in ManageRepos and RepoDetails.

@cde found this bug working on mirror support, thanks!

related to #35
closes #1016
This commit is contained in:
Hans-Christoph Steiner 2017-07-06 21:32:43 +02:00
parent 0180e754fe
commit 2a7fe78483
4 changed files with 90 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java
View File

@ -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.");

17
app/src/main/java/org/fdroid/fdroid/RepoUpdater.java
View File

@ -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(), ""));

7
app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java
View File

@ -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();

41
app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java
View File

@ -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<Repo> 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<App> 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)