Improved update handling:

Database is locked for much less time - only briefly before starting an
update sequence, and again after all downloads and parsing are complete.
Also, when the same app is defined in multiple repos, the apks are
merged rather than just having one take priority.

src/org/fdroid/fdroid/RepoXMLHandler.java

@@ -54,9 +54,8 @@ import android.util.Log;
 
 public class RepoXMLHandler extends DefaultHandler {
 
-    String mserver;
+    String server;
-
+    private Vector<DB.App> apps;
-    private DB db;
 
     private DB.App curapp = null;
     private DB.Apk curapk = null;
@@ -68,9 +67,9 @@ public class RepoXMLHandler extends DefaultHandler {
     // The date format used in the repo XML file.
     private SimpleDateFormat mXMLDateFormat = new SimpleDateFormat("yyyy-MM-dd");
 
-    public RepoXMLHandler(String srv, DB db) {
+    public RepoXMLHandler(String srv, Vector<DB.App> apps) {
-        mserver = srv;
+        this.server = srv;
-        this.db = db;
+        this.apps = apps;
         pubkey = null;
     }
 
@@ -99,12 +98,25 @@ public class RepoXMLHandler extends DefaultHandler {
         }
 
         if (curel.equals("application") && curapp != null) {
-            // Log.d("FDroid", "Repo: Updating application " + curapp.id);
-            db.updateApplication(curapp);
             getIcon(curapp);
+
+            // If we already have this application (must be from scanning a
+            // different repo) then just merge in the apks.
+            // TODO: Scanning the whole app list like this every time is
+            // going to be stupid if the list gets very big!
+            boolean merged = false;
+            for (DB.App app : apps) {
+                if (app.id == curapp.id) {
+                    app.apks.addAll(curapp.apks);
+                    break;
+                }
+            }
+            if (!merged)
+                apps.add(curapp);
+
             curapp = null;
+
         } else if (curel.equals("package") && curapk != null && curapp != null) {
-            // Log.d("FDroid", "Repo: Package added (" + curapk.version + ")");
             curapp.apks.add(curapk);
             curapk = null;
         } else if (curapk != null && str != null) {
@@ -160,7 +172,6 @@ public class RepoXMLHandler extends DefaultHandler {
             }
         } else if (curapp != null && str != null) {
             if (curel.equals("id")) {
-                // Log.d("FDroid", "App id is " + str);
                 curapp.id = str;
             } else if (curel.equals("name")) {
                 curapp.name = str;
@@ -223,13 +234,11 @@ public class RepoXMLHandler extends DefaultHandler {
             if (pk != null)
                 pubkey = pk;
         } else if (localName == "application" && curapp == null) {
-            // Log.d("FDroid", "Repo: Found application at " + mserver);
             curapp = new DB.App();
         } else if (localName == "package" && curapp != null && curapk == null) {
-            // Log.d("FDroid", "Repo: Found package for " + curapp.id);
             curapk = new DB.Apk();
             curapk.id = curapp.id;
-            curapk.server = mserver;
+            curapk.server = server;
             hashType = null;
         } else if (localName == "hash" && curapk != null) {
             hashType = attributes.getValue("", "type");
@@ -245,7 +254,7 @@ public class RepoXMLHandler extends DefaultHandler {
             if (f.exists())
                 return;
 
-            URL u = new URL(mserver + "/icons/" + app.icon);
+            URL u = new URL(server + "/icons/" + app.icon);
             HttpURLConnection uc = (HttpURLConnection) u.openConnection();
             if (uc.getResponseCode() == 200) {
                 BufferedInputStream getit = new BufferedInputStream(
@@ -289,126 +298,115 @@ public class RepoXMLHandler extends DefaultHandler {
 
     }
 
-    public static boolean doUpdates(Context ctx, DB db) {
+    // Do an update from the given repo. All applications found, and their
-        long startTime = System.currentTimeMillis();
+    // APKs, are added to 'apps'. (If 'apps' already contains an app, its
-        db.beginUpdate();
+    // APKs are merged into the existing one)
-        Vector<DB.Repo> repos = db.getRepos();
+    public static boolean doUpdate(Context ctx, DB.Repo repo,
-        for (DB.Repo repo : repos) {
+            Vector<DB.App> apps) {
-            if (repo.inuse) {
+        try {
 
+            if (repo.pubkey != null) {
+
+                // This is a signed repo - we download the jar file,
+                // check the signature, and extract the index...
+                Log.d("FDroid", "Getting signed index from " + repo.address);
+                String address = repo.address + "/index.jar";
+                PackageManager pm = ctx.getPackageManager();
                 try {
-
+                    PackageInfo pi = pm.getPackageInfo(ctx.getPackageName(), 0);
-                    if (repo.pubkey != null) {
+                    address += "?" + pi.versionName;
-
-                        // This is a signed repo - we download the jar file,
-                        // check the signature, and extract the index...
-                        Log.d("FDroid", "Getting signed index from "
-                                + repo.address);
-                        String address = repo.address + "/index.jar";
-                        PackageManager pm = ctx.getPackageManager();
-                        try {
-                            PackageInfo pi = pm.getPackageInfo(
-                                    ctx.getPackageName(), 0);
-                            address += "?" + pi.versionName;
-                        } catch (Exception e) {
-                        }
-                        getRemoteFile(ctx, address, "tempindex.jar");
-                        String jarpath = ctx.getFilesDir() + "/tempindex.jar";
-                        JarFile jar;
-                        JarEntry je;
-                        try {
-                            jar = new JarFile(jarpath, true);
-                            je = (JarEntry) jar.getEntry("index.xml");
-                            File efile = new File(ctx.getFilesDir(),
-                                    "/tempindex.xml");
-                            InputStream in = new BufferedInputStream(
-                                    jar.getInputStream(je), 8192);
-                            OutputStream out = new BufferedOutputStream(
-                                    new FileOutputStream(efile), 8192);
-                            byte[] buffer = new byte[8192];
-                            while (true) {
-                                int nBytes = in.read(buffer);
-                                if (nBytes <= 0)
-                                    break;
-                                out.write(buffer, 0, nBytes);
-                            }
-                            out.flush();
-                            out.close();
-                            in.close();
-                        } catch (SecurityException e) {
-                            Log.e("FDroid", "Invalid hash for index file");
-                            return false;
-                        }
-                        Certificate[] certs = je.getCertificates();
-                        jar.close();
-                        if (certs == null) {
-                            Log.d("FDroid", "No signature found in index");
-                            return false;
-                        }
-                        Log.d("FDroid", "Index has " + certs.length
-                                + " signature"
-                                + (certs.length > 1 ? "s." : "."));
-
-                        boolean match = false;
-                        for (Certificate cert : certs) {
-                            String certdata = Hasher.hex(cert.getEncoded());
-                            if (repo.pubkey.equals(certdata)) {
-                                match = true;
-                                break;
-                            }
-                        }
-                        if (!match) {
-                            Log.d("FDroid", "Index signature mismatch");
-                            return false;
-                        }
-
-                    } else {
-
-                        // It's an old-fashioned unsigned repo...
-                        Log.d("FDroid", "Getting unsigned index from "
-                                + repo.address);
-                        getRemoteFile(ctx, repo.address + "/index.xml",
-                                "tempindex.xml");
-                    }
-
-                    // Process the index...
-                    SAXParserFactory spf = SAXParserFactory.newInstance();
-                    SAXParser sp = spf.newSAXParser();
-                    XMLReader xr = sp.getXMLReader();
-                    RepoXMLHandler handler = new RepoXMLHandler(repo.address,
-                            db);
-                    xr.setContentHandler(handler);
-
-                    InputStreamReader isr = new FileReader(new File(
-                            ctx.getFilesDir() + "/tempindex.xml"));
-                    InputSource is = new InputSource(isr);
-                    xr.parse(is);
-
-                    if (handler.pubkey != null && repo.pubkey == null) {
-                        // We read an unsigned index, but that indicates that
-                        // a signed version is now available...
-                        Log.d("FDroid",
-                                "Public key found - switching to signed repo for future updates");
-                        repo.pubkey = handler.pubkey;
-                        db.updateRepoByAddress(repo);
-                    }
-
                 } catch (Exception e) {
-                    Log.e("FDroid", "Exception updating from " + repo.address
+                }
-                            + ":\n" + Log.getStackTraceString(e));
+                getRemoteFile(ctx, address, "tempindex.jar");
-                    db.cancelUpdate();
+                String jarpath = ctx.getFilesDir() + "/tempindex.jar";
+                JarFile jar;
+                JarEntry je;
+                try {
+                    jar = new JarFile(jarpath, true);
+                    je = (JarEntry) jar.getEntry("index.xml");
+                    File efile = new File(ctx.getFilesDir(), "/tempindex.xml");
+                    InputStream in = new BufferedInputStream(
+                            jar.getInputStream(je), 8192);
+                    OutputStream out = new BufferedOutputStream(
+                            new FileOutputStream(efile), 8192);
+                    byte[] buffer = new byte[8192];
+                    while (true) {
+                        int nBytes = in.read(buffer);
+                        if (nBytes <= 0)
+                            break;
+                        out.write(buffer, 0, nBytes);
+                    }
+                    out.flush();
+                    out.close();
+                    in.close();
+                } catch (SecurityException e) {
+                    Log.e("FDroid", "Invalid hash for index file");
+                    return false;
+                }
+                Certificate[] certs = je.getCertificates();
+                jar.close();
+                if (certs == null) {
+                    Log.d("FDroid", "No signature found in index");
+                    return false;
+                }
+                Log.d("FDroid", "Index has " + certs.length + " signature"
+                        + (certs.length > 1 ? "s." : "."));
+
+                boolean match = false;
+                for (Certificate cert : certs) {
+                    String certdata = Hasher.hex(cert.getEncoded());
+                    if (repo.pubkey.equals(certdata)) {
+                        match = true;
+                        break;
+                    }
+                }
+                if (!match) {
+                    Log.d("FDroid", "Index signature mismatch");
                     return false;
-                } finally {
-                    ctx.deleteFile("tempindex.xml");
-                    ctx.deleteFile("tempindex.jar");
                 }
 
+            } else {
+
+                // It's an old-fashioned unsigned repo...
+                Log.d("FDroid", "Getting unsigned index from " + repo.address);
+                getRemoteFile(ctx, repo.address + "/index.xml", "tempindex.xml");
             }
+
+            // Process the index...
+            SAXParserFactory spf = SAXParserFactory.newInstance();
+            SAXParser sp = spf.newSAXParser();
+            XMLReader xr = sp.getXMLReader();
+            RepoXMLHandler handler = new RepoXMLHandler(repo.address, apps);
+            xr.setContentHandler(handler);
+
+            InputStreamReader isr = new FileReader(new File(ctx.getFilesDir()
+                    + "/tempindex.xml"));
+            InputSource is = new InputSource(isr);
+            xr.parse(is);
+
+            if (handler.pubkey != null && repo.pubkey == null) {
+                // We read an unsigned index, but that indicates that
+                // a signed version is now available...
+                Log.d("FDroid",
+                        "Public key found - switching to signed repo for future updates");
+                repo.pubkey = handler.pubkey;
+                DB db = DB.getDB();
+                try {
+                    db.updateRepoByAddress(repo);
+                } finally {
+                    DB.releaseDB();
+                }
+            }
+
+        } catch (Exception e) {
+            Log.e("FDroid", "Exception updating from " + repo.address + ":\n"
+                    + Log.getStackTraceString(e));
+            return false;
+        } finally {
+            ctx.deleteFile("tempindex.xml");
+            ctx.deleteFile("tempindex.jar");
         }
-        db.endUpdate();
+
-        Log.d("FDroid", "Update completed in "
-                + ((System.currentTimeMillis() - startTime) / 1000)
-                + " seconds.");
         return true;
     }
 

src/org/fdroid/fdroid/UpdateService.java

@@ -18,6 +18,8 @@
 
 package org.fdroid.fdroid;
 
+import java.util.Vector;
+
 import android.app.AlarmManager;
 import android.app.IntentService;
 import android.app.Notification;
@@ -38,7 +40,6 @@ public class UpdateService extends IntentService {
         super("UpdateService");
     }
 
-
     // Schedule (or cancel schedule for) this service, according to the
     // current preferences. Should be called a) at boot, or b) if the preference
     // is changed.
@@ -63,87 +64,130 @@ public class UpdateService extends IntentService {
         }
     }
 
-
     protected void onHandleIntent(Intent intent) {
 
         // We might be doing a scheduled run, or we might have been launched by
-        // the app in response to a user's request. If we get this receiver, it's
+        // the app in response to a user's request. If we get this receiver,
+        // it's
         // the latter...
         ResultReceiver receiver = intent.getParcelableExtra("receiver");
-        
-        SharedPreferences prefs = PreferenceManager
-                .getDefaultSharedPreferences(getBaseContext());
 
-        // See if it's time to actually do anything yet...
+        long startTime = System.currentTimeMillis();
-        if(receiver == null) {
-            long lastUpdate = prefs.getLong("lastUpdateCheck", 0);
-            String sint = prefs.getString("updateInterval", "0");
-            int interval = Integer.parseInt(sint);
-            if (interval == 0)
-                return;
-            if (lastUpdate + (interval * 60 * 60) > System
-                    .currentTimeMillis())
-                return;
-        }
 
-        // Do the update...
-        DB db = null;
         try {
-            db = DB.getDB();
+
+            SharedPreferences prefs = PreferenceManager
+                    .getDefaultSharedPreferences(getBaseContext());
+
+            // See if it's time to actually do anything yet...
+            if (receiver == null) {
+                long lastUpdate = prefs.getLong("lastUpdateCheck", 0);
+                String sint = prefs.getString("updateInterval", "0");
+                int interval = Integer.parseInt(sint);
+                if (interval == 0)
+                    return;
+                if (lastUpdate + (interval * 60 * 60) > System
+                        .currentTimeMillis())
+                    return;
+            }
+
             boolean notify = prefs.getBoolean("updateNotify", false);
 
-            // Get the number of updates available before we
+            // Grab some preliminary information, then we can release the
-            // start, so we can notify if there are new ones.
+            // database
-            // (But avoid doing it if the user doesn't want
+            // while we do all the downloading, etc...
-            // notifications, since it may be time consuming)
+            DB db = DB.getDB();
             int prevUpdates = 0;
-            if (notify)
+            int newUpdates = 0;
-                prevUpdates = db.getNumUpdates();
+            Vector<DB.Repo> repos;
+            try {
 
-            boolean success = RepoXMLHandler.doUpdates(
+                // Get the number of updates available before we
-                    getBaseContext(), db);
+                // start, so we can notify if there are new ones.
+                // (But avoid doing it if the user doesn't want
+                // notifications, since it may be time consuming)
+                if (notify)
+                    prevUpdates = db.getNumUpdates();
+
+                repos = db.getRepos();
+
+            } finally {
+                DB.releaseDB();
+            }
+
+            // Process each repo...
+            Vector<DB.App> apps = new Vector<DB.App>();
+            boolean success = true;
+            for (DB.Repo repo : repos) {
+                if (repo.inuse) {
+                    if (!RepoXMLHandler.doUpdate(getBaseContext(), repo, apps)) {
+                        Log.d("FDroid", "Update failed for repo "
+                                + repo.address);
+                        success = false;
+                    }
+                }
+            }
+
+            if (success) {
+                db = DB.getDB();
+                try {
+                    db.beginUpdate();
+                    for (DB.App app : apps) {
+                        db.updateApplication(app);
+                    }
+                    db.endUpdate();
+                    if (notify)
+                        newUpdates = db.getNumUpdates();
+                } catch (Exception ex) {
+                    db.cancelUpdate();
+                    Log.e("FDroid", "Exception during update processing:\n"
+                            + Log.getStackTraceString(ex));
+                    success = false;
+                } finally {
+                    DB.releaseDB();
+                }
+            }
 
             if (success && notify) {
-                int newUpdates = db.getNumUpdates();
+                Log.d("FDroid", "Updates before:" + prevUpdates + ", after: "
-                Log.d("FDroid", "Updates before:" + prevUpdates + ", after: " +newUpdates);                        
+                        + newUpdates);
                 if (newUpdates > prevUpdates) {
                     NotificationManager n = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE);
                     Notification notification = new Notification(
-                            R.drawable.icon,
+                            R.drawable.icon, "F-Droid Updates Available",
-                            "FDroid Updates Available", System
+                            System.currentTimeMillis());
-                                    .currentTimeMillis());
                     Context context = getApplicationContext();
-                    CharSequence contentTitle = "FDroid";
+                    CharSequence contentTitle = "F-Droid";
                     CharSequence contentText = "Updates are available.";
-                    Intent notificationIntent = new Intent(
+                    Intent notificationIntent = new Intent(UpdateService.this,
-                            UpdateService.this, FDroid.class);
+                            FDroid.class);
-                    PendingIntent contentIntent = PendingIntent
+                    PendingIntent contentIntent = PendingIntent.getActivity(
-                            .getActivity(UpdateService.this, 0,
+                            UpdateService.this, 0, notificationIntent, 0);
-                                    notificationIntent, 0);
+                    notification.setLatestEventInfo(context, contentTitle,
-                    notification.setLatestEventInfo(context,
+                            contentText, contentIntent);
-                            contentTitle, contentText, contentIntent);
                     notification.flags |= Notification.FLAG_AUTO_CANCEL;
                     n.notify(1, notification);
                 }
             }
-            
+
-            if(receiver != null) {
+            if (receiver != null) {
                 Bundle resultData = new Bundle();
                 receiver.send(0, resultData);
             }
 
         } catch (Exception e) {
-            Log.e("FDroid", "Exception during handleCommand():\n"
+            Log.e("FDroid",
-                    + Log.getStackTraceString(e));
+                    "Exception during update processing:\n"
-            if(receiver != null) {
+                            + Log.getStackTraceString(e));
+            if (receiver != null) {
                 Bundle resultData = new Bundle();
                 receiver.send(1, resultData);
             }
         } finally {
-            if (db != null)
+            Log.d("FDroid", "Update took "
-                DB.releaseDB();
+                    + ((System.currentTimeMillis() - startTime) / 1000)
+                    + " seconds.");
         }
 
     }
-
 }