From fa7f57a18addfd58e1cdb5d6c09793fdaeac3ff1 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 2 Jun 2016 14:44:23 +1000
Subject: [PATCH 1/6] Remove unused test code.
Since refactoring the installed app cache stuff, these methods are no longer
required for testing purposes. This is because the tests directly ask the
content provider to insert relevant apps, rather than testing the broadcast
receiving functionality.
---
.../fdroid/data/InstalledAppProviderTest.java | 70 -------------------
1 file changed, 70 deletions(-)
diff --git a/app/src/androidTest/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java b/app/src/androidTest/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java
index 7c96e66e2..7da03fcd0 100644
--- a/app/src/androidTest/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java
+++ b/app/src/androidTest/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java
@@ -11,23 +11,10 @@ import mock.MockInstallablePackageManager;
@SuppressWarnings("PMD") // TODO port this to JUnit 4 semantics
public class InstalledAppProviderTest extends FDroidProviderTest<InstalledAppProvider> {
- private MockInstallablePackageManager packageManager;
-
public InstalledAppProviderTest() {
super(InstalledAppProvider.class, InstalledAppProvider.getAuthority());
}
- @Override
- public void setUp() throws Exception {
- super.setUp();
- packageManager = new MockInstallablePackageManager();
- getSwappableContext().setPackageManager(packageManager);
- }
-
- protected MockInstallablePackageManager getPackageManager() {
- return packageManager;
- }
-
public void testUris() {
assertInvalidUri(InstalledAppProvider.getAuthority());
assertInvalidUri(RepoProvider.getContentUri());
@@ -130,45 +117,6 @@ public class InstalledAppProviderTest extends FDroidProviderTest<InstalledAppPro
}
- public void testInsertWithBroadcast() {
- install("com.example.broadcasted1", 10, "v1.0");
- install("com.example.broadcasted2", 105, "v1.05");
-
- assertResultCount(2, InstalledAppProvider.getContentUri());
- assertIsInstalledVersionInDb("com.example.broadcasted1", 10, "v1.0");
- assertIsInstalledVersionInDb("com.example.broadcasted2", 105, "v1.05");
- }
-
- public void testUpdateWithBroadcast() {
-
- install("com.example.toUpgrade", 1, "v0.1");
-
- assertResultCount(1, InstalledAppProvider.getContentUri());
- assertIsInstalledVersionInDb("com.example.toUpgrade", 1, "v0.1");
-
- install("com.example.toUpgrade", 2, "v0.2");
-
- assertResultCount(1, InstalledAppProvider.getContentUri());
- assertIsInstalledVersionInDb("com.example.toUpgrade", 2, "v0.2");
-
- }
-
- public void testDeleteWithBroadcast() {
-
- install("com.example.toKeep", 1, "v0.1");
- install("com.example.toDelete", 1, "v0.1");
-
- assertResultCount(2, InstalledAppProvider.getContentUri());
- assertIsInstalledVersionInDb("com.example.toKeep", 1, "v0.1");
- assertIsInstalledVersionInDb("com.example.toDelete", 1, "v0.1");
-
- remove("com.example.toDelete");
-
- assertResultCount(1, InstalledAppProvider.getContentUri());
- assertIsInstalledVersionInDb("com.example.toKeep", 1, "v0.1");
-
- }
-
@Override
protected String[] getMinimalProjection() {
return new String[]{
@@ -202,14 +150,6 @@ public class InstalledAppProviderTest extends FDroidProviderTest<InstalledAppPro
getMockContentResolver().insert(InstalledAppProvider.getContentUri(), values);
}
- private void remove(String packageName) {
- remove(getSwappableContext(), getPackageManager(), packageName);
- }
-
- private void install(String appId, int versionCode, String versionName) {
- install(getSwappableContext(), getPackageManager(), appId, versionCode, versionName);
- }
-
/**
* Will tell {@code pm} that we are installing {@code packageName}, and then update the
* "installed apps" table in the database.
@@ -224,14 +164,4 @@ public class InstalledAppProviderTest extends FDroidProviderTest<InstalledAppPro
InstalledAppProviderService.insertAppIntoDb(context, packageName, packageInfo);
}
- /**
- * @see #install(mock.MockContextSwappableComponents, mock.MockInstallablePackageManager, String, int, String)
- */
- public static void remove(MockContextSwappableComponents context, MockInstallablePackageManager pm, String packageName) {
-
- context.setPackageManager(pm);
- pm.remove(packageName);
- InstalledAppProviderService.deleteAppFromDb(context, packageName);
- }
-
}
From 371312ef650c652c3c522e5cb5ccd17a7a88ab97 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 2 Jun 2016 14:24:04 +1000
Subject: [PATCH 2/6] Replace rate limiting code with RX.
Now that we have RX as a dependency, it can be used as a nice concise way to
achieve certain tasks. Rate limiting is one thing it does well - via the
`debounce` mechanism:
http://reactivex.io/documentation/operators/debounce.html
The semantics of this code is the same as before, limiting content change notifications
to one per second.
---
.../data/InstalledAppProviderService.java | 61 +++++++++----------
1 file changed, 28 insertions(+), 33 deletions(-)
diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
index 3dc103bac..1a9ff48b8 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
@@ -14,10 +14,12 @@ import org.fdroid.fdroid.Utils;
import java.io.File;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
+import rx.functions.Action1;
+import rx.schedulers.Schedulers;
+import rx.subjects.PublishSubject;
+
/**
* Handles all updates to {@link InstalledAppProvider}, whether checking the contents
* versus what Android says is installed, or processing {@link Intent}s that come
@@ -39,13 +41,34 @@ public class InstalledAppProviderService extends IntentService {
private static final String EXTRA_PACKAGE_INFO = "org.fdroid.fdroid.data.extra.PACKAGE_INFO";
- private ScheduledExecutorService worker;
- private boolean notifyChangeNeedsSending;
+ /**
+ * This is for notifing the users of this {@link android.content.ContentProvider}
+ * that the contents has changed. Since {@link Intent}s can come in slow
+ * or fast, and this can trigger a lot of UI updates, the actual
+ * notifications are rate limited to one per second.
+ */
+ private PublishSubject<Void> notifyEvents;
public InstalledAppProviderService() {
super("InstalledAppProviderService");
}
+ @Override
+ public void onCreate() {
+ super.onCreate();
+ notifyEvents = PublishSubject.create();
+ notifyEvents.debounce(1, TimeUnit.SECONDS)
+ .subscribeOn(Schedulers.newThread())
+ .subscribe(new Action1<Void>() {
+ @Override
+ public void call(Void voidArg) {
+ Utils.debugLog(TAG, "Notifying content providers (so they can update the relevant views).");
+ getContentResolver().notifyChange(AppProvider.getContentUri(), null);
+ getContentResolver().notifyChange(ApkProvider.getContentUri(), null);
+ }
+ });
+ }
+
/**
* Inserts an app into {@link InstalledAppProvider} based on a {@code package:} {@link Uri}.
* This has no checks for whether it is inserting an exact duplicate, whatever is provided
@@ -134,7 +157,7 @@ public class InstalledAppProviderService extends IntentService {
} else if (ACTION_DELETE.equals(action)) {
deleteAppFromDb(this, packageName);
}
- notifyChange();
+ notifyEvents.onNext(null);
}
}
@@ -172,32 +195,4 @@ public class InstalledAppProviderService extends IntentService {
Uri uri = InstalledAppProvider.getAppUri(packageName);
context.getContentResolver().delete(uri, null, null);
}
-
- /**
- * This notifies the users of this {@link android.content.ContentProvider}
- * that the contents has changed. Since {@link Intent}s can come in slow
- * or fast, and this can trigger a lot of UI updates, the actual
- * notifications are rate limited to one per second.
- */
- private void notifyChange() {
- notifyChangeNeedsSending = true;
- Runnable task = new Runnable() {
- @Override
- public void run() {
- if (notifyChangeNeedsSending) {
- Utils.debugLog(TAG, "Notifying content providers (so they can update the relevant views).");
- getContentResolver().notifyChange(AppProvider.getContentUri(), null);
- getContentResolver().notifyChange(ApkProvider.getContentUri(), null);
- notifyChangeNeedsSending = false;
- } else {
- worker.shutdown();
- worker = null;
- }
- }
- };
- if (worker == null || worker.isShutdown()) {
- worker = Executors.newSingleThreadScheduledExecutor();
- worker.scheduleAtFixedRate(task, 0, 1, TimeUnit.SECONDS);
- }
- }
}
\ No newline at end of file
From a74e951cdf2e3f79f6dcb67b2488bc23ae25216c Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 2 Jun 2016 14:46:58 +1000
Subject: [PATCH 3/6] Simplify code by creating the object when required rather
than using singleton.
This should not be a particularly expensive opperation,. Also, at time of
writing it is only used in a background thread, and only used once in that
thread (i.e. not in a loop or anything like that).
---
.../fdroid/fdroid/localrepo/LocalRepoManager.java | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java b/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java
index 3cf9b79b3..1e18438f7 100644
--- a/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java
+++ b/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java
@@ -319,16 +319,6 @@ public final class LocalRepoManager {
* Helper class to aid in constructing index.xml file.
*/
public static final class IndexXmlBuilder {
-
- private static IndexXmlBuilder indexXmlBuilder;
-
- public static IndexXmlBuilder get() throws XmlPullParserException {
- if (indexXmlBuilder == null) {
- indexXmlBuilder = new IndexXmlBuilder();
- }
- return indexXmlBuilder;
- }
-
@NonNull
private final XmlSerializer serializer;
@@ -487,7 +477,7 @@ public final class LocalRepoManager {
JarOutputStream jo = new JarOutputStream(bo);
JarEntry je = new JarEntry("index.xml");
jo.putNextEntry(je);
- IndexXmlBuilder.get().build(context, apps, jo);
+ new IndexXmlBuilder().build(context, apps, jo);
jo.close();
bo.close();
From 7076bb767d69bc57fdc824df725592fa864334c9 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 2 Jun 2016 14:54:27 +1000
Subject: [PATCH 4/6] Clarify what needs to be passed into `parseApp` and make
it private.
The `parseApp` method was previously accepting an `Intent`, which could
have been anything. Given it was only used once, this now pushed the
creation of that `Intent` into the `parseApp` method, and also reduced the
visibility of the method as it is only used once at time of writing.
---
.../org/fdroid/fdroid/localrepo/CacheSwapAppsService.java | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/CacheSwapAppsService.java b/app/src/main/java/org/fdroid/fdroid/localrepo/CacheSwapAppsService.java
index 63a1fa1c0..31d3c880c 100644
--- a/app/src/main/java/org/fdroid/fdroid/localrepo/CacheSwapAppsService.java
+++ b/app/src/main/java/org/fdroid/fdroid/localrepo/CacheSwapAppsService.java
@@ -37,7 +37,9 @@ public class CacheSwapAppsService extends IntentService {
* Parse the locally installed APK for {@code packageName} and save its XML
* to the APK XML cache.
*/
- public static void parseApp(Context context, Intent intent) {
+ private static void parseApp(Context context, String packageName) {
+ Intent intent = new Intent();
+ intent.setData(Utils.getPackageUri(packageName));
intent.setClass(context, CacheSwapAppsService.class);
intent.setAction(ACTION_PARSE_APP);
context.startService(intent);
@@ -57,9 +59,7 @@ public class CacheSwapAppsService extends IntentService {
}
if (!indexJarFile.exists()
|| FileUtils.isFileNewer(new File(applicationInfo.sourceDir), indexJarFile)) {
- Intent intent = new Intent();
- intent.setData(Utils.getPackageUri(applicationInfo.packageName));
- parseApp(context, intent);
+ parseApp(context, applicationInfo.packageName);
}
}
}
From 2d90a484df1097c7d088b28d221429148f438530 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 2 Jun 2016 20:41:00 +1000
Subject: [PATCH 5/6] Move method only used by `InstalledAppProviderService`.
The method was only used here, so lets move the method here. May as well
make it private too until somebody else comes up with a use case for it.
---
.../fdroid/data/InstalledAppProvider.java | 18 ---------------
.../data/InstalledAppProviderService.java | 22 +++++++++++++++++--
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
index 166d838f0..17d9a86d5 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
@@ -6,17 +6,14 @@ import android.content.UriMatcher;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageInfo;
import android.content.pm.PackageManager;
-import android.content.pm.Signature;
import android.content.res.Resources;
import android.database.Cursor;
import android.net.Uri;
import android.util.Log;
-import org.fdroid.fdroid.Hasher;
import org.fdroid.fdroid.R;
import org.fdroid.fdroid.Utils;
-import java.security.NoSuchAlgorithmException;
import java.util.HashMap;
import java.util.Map;
@@ -118,21 +115,6 @@ public class InstalledAppProvider extends FDroidProvider {
return packageName; // all else fails, return packageName
}
- public static String getPackageSig(PackageInfo info) {
- if (info == null || info.signatures == null || info.signatures.length < 1) {
- return "";
- }
- Signature sig = info.signatures[0];
- String sigHash = "";
- try {
- Hasher hash = new Hasher("MD5", sig.toCharsString().getBytes());
- sigHash = hash.getHash();
- } catch (NoSuchAlgorithmException e) {
- // ignore
- }
- return sigHash;
- }
-
@Override
protected String getTableName() {
return DBHelper.TABLE_INSTALLED_APP;
diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
index 1a9ff48b8..f1959596f 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
@@ -6,12 +6,15 @@ import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageInfo;
import android.content.pm.PackageManager;
+import android.content.pm.Signature;
import android.net.Uri;
import android.os.Process;
+import org.fdroid.fdroid.Hasher;
import org.fdroid.fdroid.Utils;
import java.io.File;
+import java.security.NoSuchAlgorithmException;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@@ -179,8 +182,7 @@ public class InstalledAppProviderService extends IntentService {
contentValues.put(InstalledAppProvider.DataColumns.VERSION_NAME, packageInfo.versionName);
contentValues.put(InstalledAppProvider.DataColumns.APPLICATION_LABEL,
InstalledAppProvider.getApplicationLabel(context, packageInfo.packageName));
- contentValues.put(InstalledAppProvider.DataColumns.SIGNATURE,
- InstalledAppProvider.getPackageSig(packageInfo));
+ contentValues.put(InstalledAppProvider.DataColumns.SIGNATURE, getPackageSig(packageInfo));
contentValues.put(InstalledAppProvider.DataColumns.LAST_UPDATE_TIME, packageInfo.lastUpdateTime);
String hashType = "sha256";
@@ -195,4 +197,20 @@ public class InstalledAppProviderService extends IntentService {
Uri uri = InstalledAppProvider.getAppUri(packageName);
context.getContentResolver().delete(uri, null, null);
}
+
+ private static String getPackageSig(PackageInfo info) {
+ if (info == null || info.signatures == null || info.signatures.length < 1) {
+ return "";
+ }
+ Signature sig = info.signatures[0];
+ String sigHash = "";
+ try {
+ Hasher hash = new Hasher("MD5", sig.toCharsString().getBytes());
+ sigHash = hash.getHash();
+ } catch (NoSuchAlgorithmException e) {
+ // ignore
+ }
+ return sigHash;
+ }
+
}
\ No newline at end of file
From 7c8ea5c5af29d1994506bfacb085d8130a34509f Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 2 Jun 2016 20:44:41 +1000
Subject: [PATCH 6/6] Prevent `InstalledAppProvider` from notifying about
changes.
Historically the providers were responsible for notifying about inserts/deletes
for this table. However this is no longer the case with the new service responsible
for throttling the rate with which these notifications occur.
---
.../org/fdroid/fdroid/data/InstalledAppProvider.java | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
index 17d9a86d5..ffeb36f59 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
@@ -183,11 +183,7 @@ public class InstalledAppProvider extends FDroidProvider {
QuerySelection query = new QuerySelection(where, whereArgs);
query = query.add(queryApp(uri.getLastPathSegment()));
- int count = db().delete(getTableName(), query.getSelection(), query.getArgs());
- if (!isApplyingBatch()) {
- getContext().getContentResolver().notifyChange(uri, null);
- }
- return count;
+ return db().delete(getTableName(), query.getSelection(), query.getArgs());
}
@Override
@@ -199,9 +195,6 @@ public class InstalledAppProvider extends FDroidProvider {
verifyVersionNameNotNull(values);
db().replaceOrThrow(getTableName(), null, values);
- if (!isApplyingBatch()) {
- getContext().getContentResolver().notifyChange(uri, null);
- }
return getAppUri(values.getAsString(DataColumns.PACKAGE_NAME));
}