fix index update progress using simplified ProgressListener

The Event class is no longer needed once there is specific ProgressListener
instances for each type of progress update.  The sourceUrl serves as the
unique ID, like with DownloaderService and InstallManagerService.

fixes #633 https://gitlab.com/fdroid/fdroidclient/issues/633

app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java

@@ -30,7 +30,7 @@ import java.util.UUID;
 
 @SuppressWarnings("PMD")  // TODO port this to JUnit 4 semantics
 public class MultiRepoUpdaterTest extends InstrumentationTestCase {
-    private static final String TAG = "RepoUpdaterTest";
+    private static final String TAG = "MultiRepoUpdaterTest";
 
     private static final String REPO_MAIN = "Test F-Droid repo";
     private static final String REPO_ARCHIVE = "Test F-Droid repo (Archive)";
@@ -406,7 +406,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase {
     private RepoUpdater createUpdater(String name, Context context) {
         Repo repo = new Repo();
         repo.signingCertificate = PUB_KEY;
-        repo.address = UUID.randomUUID().toString();
+        repo.address = "https://fake.url/" + UUID.randomUUID().toString() + "/fdroid/repo";
         repo.name = name;
 
         ContentValues values = new ContentValues(2);

app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java

@@ -31,6 +31,7 @@ public class RepoUpdaterTest {
         context = instrumentation.getContext();
         testFilesDir = TestUtils.getWriteableDir(instrumentation);
         Repo repo = new Repo();
+        repo.address = "https://fake.url/fdroid/repo";
         repo.signingCertificate = this.simpleIndexSigningCert;
         repoUpdater = new RepoUpdater(context, repo);
     }

app/src/main/java/org/fdroid/fdroid/ProgressBufferedInputStream.java

@@ -1,17 +1,14 @@
 package org.fdroid.fdroid;
 
-import android.os.Bundle;
-
-import org.fdroid.fdroid.data.Repo;
-
 import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URL;
 
 public class ProgressBufferedInputStream extends BufferedInputStream {
 
     private final ProgressListener progressListener;
-    private final Bundle data;
+    private final URL sourceUrl;
     private final int totalBytes;
 
     private int currentBytes;
@@ -20,12 +17,10 @@ public class ProgressBufferedInputStream extends BufferedInputStream {
      * Reports progress to the specified {@link ProgressListener}, with the
      * progress based on the {@code totalBytes}.
      */
-    public ProgressBufferedInputStream(InputStream in, ProgressListener progressListener, Repo repo, int totalBytes)
+    public ProgressBufferedInputStream(InputStream in, ProgressListener progressListener, URL sourceUrl, int totalBytes) {
-            throws IOException {
         super(in);
         this.progressListener = progressListener;
-        this.data = new Bundle(1);
+        this.sourceUrl = sourceUrl;
-        this.data.putString(RepoUpdater.PROGRESS_DATA_REPO_ADDRESS, repo.address);
         this.totalBytes = totalBytes;
     }
 
@@ -37,10 +32,7 @@ public class ProgressBufferedInputStream extends BufferedInputStream {
              * the digits changing because it looks pretty, < 9000 since the reads won't
              * line up exactly */
             if (currentBytes % 333333 < 9000) {
-                progressListener.onProgress(
+                progressListener.onProgress(sourceUrl, currentBytes, totalBytes);
-                        new ProgressListener.Event(
-                                RepoUpdater.PROGRESS_TYPE_PROCESS_XML,
-                                currentBytes, totalBytes, data));
             }
         }
         return super.read(buffer, byteOffset, byteCount);

app/src/main/java/org/fdroid/fdroid/ProgressListener.java

@@ -1,76 +1,15 @@
 package org.fdroid.fdroid;
 
-import android.os.Bundle;
+import java.net.URL;
-import android.os.Parcel;
-import android.os.Parcelable;
 
+/**
+ * This is meant only to send download progress for any URL (e.g. index
+ * updates, APKs, etc). This also keeps this class pure Java so that classes
+ * that use {@code ProgressListener} can be tested on the JVM, without requiring
+ * an Android device or emulator.
+ */
 public interface ProgressListener {
 
-    void onProgress(Event event);
+    void onProgress(URL sourceUrl, int bytesRead, int totalBytes);
-
-    // I went a bit overboard with the overloaded constructors, but they all
-    // seemed potentially useful and unambiguous, so I just put them in there
-    // while I'm here.
-    class Event implements Parcelable {
-
-        public static final int NO_VALUE = Integer.MIN_VALUE;
-
-        public final String type;
-        public final Bundle data;
-
-        // These two are not final, so that you can create a template Event,
-        // pass it into a function which performs something over time, and
-        // that function can initialize "total" and progressively
-        // update "progress"
-        public int progress;
-        public final int total;
-
-        public Event(String type) {
-            this(type, NO_VALUE, NO_VALUE, null);
-        }
-
-        public Event(String type, int progress, int total, Bundle data) {
-            this.type = type;
-            this.progress = progress;
-            this.total = total;
-            this.data = (data == null) ? new Bundle() : data;
-        }
-
-        @Override
-        public int describeContents() {
-            return 0;
-        }
-
-        @Override
-        public void writeToParcel(Parcel dest, int flags) {
-            dest.writeString(type);
-            dest.writeInt(progress);
-            dest.writeInt(total);
-            dest.writeBundle(data);
-        }
-
-        public static final Parcelable.Creator<Event> CREATOR = new Parcelable.Creator<Event>() {
-            @Override
-            public Event createFromParcel(Parcel in) {
-                return new Event(in.readString(), in.readInt(), in.readInt(), in.readBundle());
-            }
-
-            @Override
-            public Event[] newArray(int size) {
-                return new Event[size];
-            }
-        };
-
-        /**
-         * Can help to provide context to the listener about what process is causing the event.
-         * For example, the repo updater uses one listener to listen to multiple downloaders.
-         * When it receives an event, it doesn't know which repo download is causing the event,
-         * so we pass that through to the downloader when we set the progress listener. This way,
-         * we can ask the event for the name of the repo.
-         */
-        public Bundle getData() {
-            return data;
-        }
-    }
 
 }

app/src/main/java/org/fdroid/fdroid/RepoUpdater.java

@@ -21,6 +21,8 @@ import org.xml.sax.XMLReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.security.CodeSigner;
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
@@ -48,10 +50,6 @@ public class RepoUpdater {
 
     private static final String TAG = "RepoUpdater";
 
-    public static final String PROGRESS_TYPE_PROCESS_XML = "processingXml";
-    public static final String PROGRESS_COMMITTING = "committing";
-    public static final String PROGRESS_DATA_REPO_ADDRESS = "repoAddress";
-
     public final String indexUrl;
 
     @NonNull
@@ -60,7 +58,8 @@ public class RepoUpdater {
     private final Repo repo;
     private boolean hasChanged;
     @Nullable
-    private ProgressListener progressListener;
+    private ProgressListener committingProgressListener;
+    private ProgressListener processXmlProgressListener;
     private String cacheTag;
     private X509Certificate signingCertFromJar;
 
@@ -84,8 +83,12 @@ public class RepoUpdater {
         this.indexUrl = url;
     }
 
-    public void setProgressListener(@Nullable ProgressListener progressListener) {
+    public void setProcessXmlProgressListener(ProgressListener progressListener) {
-        this.progressListener = progressListener;
+        this.processXmlProgressListener = progressListener;
+    }
+
+    public void setCommittingProgressListener(ProgressListener progressListener) {
+        this.committingProgressListener = progressListener;
     }
 
     public boolean hasChanged() {
@@ -177,7 +180,7 @@ public class RepoUpdater {
             JarFile jarFile = new JarFile(downloadedFile, true);
             JarEntry indexEntry = (JarEntry) jarFile.getEntry("index.xml");
             indexInputStream = new ProgressBufferedInputStream(jarFile.getInputStream(indexEntry),
-                progressListener, repo, (int) indexEntry.getSize());
+                processXmlProgressListener, new URL(repo.address), (int) indexEntry.getSize());
 
             // Process the index...
             final SAXParser parser = SAXParserFactory.newInstance().newSAXParser();
@@ -207,8 +210,13 @@ public class RepoUpdater {
 
     private void commitToDb() throws UpdateException {
         Log.i(TAG, "Repo signature verified, saving app metadata to database.");
-        if (progressListener != null) {
+        if (committingProgressListener != null) {
-            progressListener.onProgress(new ProgressListener.Event(PROGRESS_COMMITTING));
+            try {
+                //TODO this should be an event, not a progress listener
+                committingProgressListener.onProgress(new URL(indexUrl), 0, -1);
+            } catch (MalformedURLException e) {
+                e.printStackTrace();
+            }
         }
         persister.commit(repoDetailsToSave);
     }

app/src/main/java/org/fdroid/fdroid/UpdateService.java

@@ -52,10 +52,11 @@ import org.fdroid.fdroid.installer.InstallManagerService;
 import org.fdroid.fdroid.net.Downloader;
 import org.fdroid.fdroid.net.DownloaderService;
 
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
 
-public class UpdateService extends IntentService implements ProgressListener {
+public class UpdateService extends IntentService {
 
     private static final String TAG = "UpdateService";
 
@@ -376,7 +377,8 @@ public class UpdateService extends IntentService implements ProgressListener {
                 RepoUpdater updater = new RepoUpdater(getBaseContext(), repo);
                 localBroadcastManager.registerReceiver(downloadProgressReceiver,
                         DownloaderService.getIntentFilter(updater.indexUrl, Downloader.ACTION_PROGRESS));
-                updater.setProgressListener(this);
+                updater.setProcessXmlProgressListener(processXmlProgressListener);
+                updater.setCommittingProgressListener(committingProgressListener);
                 try {
                     updater.update();
                     if (updater.hasChanged()) {
@@ -526,25 +528,25 @@ public class UpdateService extends IntentService implements ProgressListener {
         notificationManager.notify(NOTIFY_ID_UPDATES_AVAILABLE, builder.build());
     }
 
-    /**
+    private final ProgressListener processXmlProgressListener = new ProgressListener() {
-     * Received progress event from the RepoXMLHandler. It could be progress
+        @Override
-     * downloading from the repo, or perhaps processing the info from the repo.
+        public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
-     */
+            String downloadedSize = Utils.getFriendlySize(bytesRead);
-    @Override
+            String totalSize = Utils.getFriendlySize(totalBytes);
-    public void onProgress(ProgressListener.Event event) {
+            int percent = -1;
-        String message = "";
+            if (totalBytes > 0) {
-        String repoAddress = event.getData().getString(RepoUpdater.PROGRESS_DATA_REPO_ADDRESS);
+                percent = (int) ((double) bytesRead / totalBytes * 100);
-        String downloadedSize = Utils.getFriendlySize(event.progress);
+            }
-        String totalSize = Utils.getFriendlySize(event.total);
+            String message = getString(R.string.status_processing_xml_percent, sourceUrl, downloadedSize, totalSize, percent);
-        int percent = event.total > 0 ? (int) ((double) event.progress / event.total * 100) : -1;
+            sendStatus(getApplicationContext(), STATUS_INFO, message, percent);
-        switch (event.type) {
-            case RepoUpdater.PROGRESS_TYPE_PROCESS_XML:
-                message = getString(R.string.status_processing_xml_percent, repoAddress, downloadedSize, totalSize, percent);
-                break;
-            case RepoUpdater.PROGRESS_COMMITTING:
-                message = getString(R.string.status_inserting_apps);
-                break;
         }
-        sendStatus(this, STATUS_INFO, message, percent);
+    };
-    }
+
+    private final ProgressListener committingProgressListener = new ProgressListener() {
+        @Override
+        public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
+            String message = getString(R.string.status_inserting_apps);
+            sendStatus(getApplicationContext(), STATUS_INFO, message);
+        }
+    };
 }

app/src/main/java/org/fdroid/fdroid/Utils.java

@@ -115,27 +115,13 @@ public final class Utils {
         return "/icons-120/";
     }
 
-    public static void copy(InputStream input, OutputStream output)
+    public static void copy(InputStream input, OutputStream output) throws IOException {
-            throws IOException {
-        copy(input, output, null, null);
-    }
-
-    public static void copy(InputStream input, OutputStream output,
-                    ProgressListener progressListener,
-                    ProgressListener.Event templateProgressEvent)
-    throws IOException {
         byte[] buffer = new byte[BUFFER_SIZE];
-        int bytesRead = 0;
         while (true) {
             int count = input.read(buffer);
             if (count == -1) {
                 break;
             }
-            if (progressListener != null) {
-                bytesRead += count;
-                templateProgressEvent.progress = bytesRead;
-                progressListener.onProgress(templateProgressEvent);
-            }
             output.write(buffer, 0, count);
         }
         output.flush();