Changed backup to delete the oldest backups, retaining 7 files. Added a backup unit test

astrid/src-legacy/com/timsu/astrid/activities/TaskListSubActivity.java

@@ -1345,7 +1345,7 @@ public class TaskListSubActivity extends SubActivity {
     private void exportTasks() {
         TasksXmlExporter exporter = new TasksXmlExporter(false);
         exporter.setContext(getParent());
-        exporter.exportTasks();
+        exporter.exportTasks(TasksXmlExporter.getExportDirectory());
     }
 
     /*

astrid/src-legacy/com/timsu/astrid/utilities/BackupService.java

@@ -12,6 +12,7 @@ import android.app.Service;
 import android.content.Context;
 import android.content.Intent;
 import android.os.IBinder;
+import android.util.Log;
 
 import com.timsu.astrid.R;
 
@@ -29,11 +30,10 @@ public class BackupService extends Service {
      * how often to back up
      */
     private static final long BACKUP_INTERVAL = AlarmManager.INTERVAL_DAY;
-    private static final String BACKUP_ACTION = "backup";
+    public static final String BACKUP_ACTION = "backup";
-    private static final String BACKUP_FILE_NAME_REGEX = "auto\\.\\d{6}\\-\\d{4}\\.xml";
+    public static final String BACKUP_FILE_NAME_REGEX = "auto\\.[-\\d]+\\.xml";
     private static final int DAYS_TO_KEEP_BACKUP = 7;
 
-
     @Override
     public IBinder onBind(Intent intent) {
         return null;
@@ -46,6 +46,14 @@ public class BackupService extends Service {
         }
     }
 
+    /**
+     * Test hook for backup
+     * @param ctx
+     */
+    public void testBackup(Context ctx) {
+        startBackup(ctx);
+    }
+
     private void startBackup(Context ctx) {
         if (ctx == null || ctx.getResources() == null) {
             return;
@@ -55,10 +63,14 @@ public class BackupService extends Service {
                 return;
             }
 
-            deleteOldBackups();
+            try {
+                deleteOldBackups();
+            } catch (Exception e) {
+                Log.e("error-deleting", "Error deleting old backups: " + e);
+            }
             TasksXmlExporter exporter = new TasksXmlExporter(true);
             exporter.setContext(ctx);
-            exporter.exportTasks();
+            exporter.exportTasks(backupDirectorySetting.getBackupDirectory());
             Preferences.setBackupSummary(ctx,
                     ctx.getString(R.string.prefs_backup_desc_success,
                             DateUtilities.getFormattedDate(ctx.getResources(), new Date())));
@@ -110,7 +122,7 @@ public class BackupService extends Service {
                 return false;
             }
         };
-        File astridDir = TasksXmlExporter.getExportDirectory();
+        File astridDir = backupDirectorySetting.getBackupDirectory();
 
         // grab all backup files, sort by modified date, delete old ones
         File[] files = astridDir.listFiles(backupFileFilter);
@@ -124,4 +136,24 @@ public class BackupService extends Service {
             files[i].delete();
         }
     }
+
+    /**
+     * Interface for setting where backups go
+     * @author Tim Su <tim@todoroo.com>
+     *
+     */
+    public interface BackupDirectorySetting {
+        public File getBackupDirectory();
+    }
+
+    private BackupDirectorySetting backupDirectorySetting = new BackupDirectorySetting() {
+        public File getBackupDirectory() {
+            return TasksXmlExporter.getExportDirectory();
+        }
+    };
+
+    public void setBackupDirectorySetting(
+            BackupDirectorySetting backupDirectorySetting) {
+        this.backupDirectorySetting = backupDirectorySetting;
+    }
 }

astrid/src-legacy/com/timsu/astrid/utilities/Preferences.java

@@ -294,6 +294,13 @@ public class Preferences {
         return getPrefs(context).getBoolean(r.getString(R.string.p_backup), true);
     }
 
+    public static void setBackupEnabled(Context context, boolean setting) {
+        Resources r = context.getResources();
+        Editor editor = getPrefs(context).edit();
+        editor.putBoolean(r.getString(R.string.p_backup), setting);
+        editor.commit();
+    }
+
     /**
      * @return error when doing backup, empty string if successful, or null
      *         if no backup has been attempted

astrid/src-legacy/com/timsu/astrid/utilities/TasksXmlExporter.java

@@ -1,5 +1,16 @@
 package com.timsu.astrid.utilities;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+
+import org.xmlpull.v1.XmlSerializer;
+
 import android.content.Context;
 import android.database.Cursor;
 import android.os.Environment;
@@ -7,6 +18,7 @@ import android.os.Looper;
 import android.util.Log;
 import android.util.Xml;
 import android.widget.Toast;
+
 import com.timsu.astrid.R;
 import com.timsu.astrid.data.alerts.AlertController;
 import com.timsu.astrid.data.sync.SyncDataController;
@@ -17,12 +29,6 @@ import com.timsu.astrid.data.tag.TagModelForView;
 import com.timsu.astrid.data.task.TaskController;
 import com.timsu.astrid.data.task.TaskIdentifier;
 import com.timsu.astrid.data.task.TaskModelForXml;
-import org.xmlpull.v1.XmlSerializer;
-
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.util.*;
 
 public class TasksXmlExporter {
 
@@ -175,12 +181,12 @@ public class TasksXmlExporter {
         syncDataController.open();
     }
 
-    public void exportTasks() {
+    public void exportTasks(File directory) {
         if (isService && !Preferences.isBackupEnabled(ctx)) {
             // Automatic backups are disabled.
             return;
         }
-        if (setupFile()) {
+        if (setupFile(directory)) {
             Thread thread = new Thread(doBackgroundExport);
             thread.start();
         }
@@ -196,8 +202,8 @@ public class TasksXmlExporter {
         return null;
     }
 
-    private boolean setupFile() {
+    private boolean setupFile(File directory) {
-        File astridDir = getExportDirectory();
+        File astridDir = directory;
         if (astridDir != null) {
             // Check for /sdcard/astrid directory. If it doesn't exist, make it.
             if (astridDir.exists() || astridDir.mkdir()) {

tests/src/com/todoroo/astrid/service/BackupServiceTests.java

@@ -0,0 +1,150 @@
+package com.todoroo.astrid.service;
+
+import java.io.File;
+import java.io.IOException;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+
+import com.timsu.astrid.utilities.BackupService;
+import com.timsu.astrid.utilities.Preferences;
+import com.timsu.astrid.utilities.BackupService.BackupDirectorySetting;
+import com.todoroo.andlib.test.TodorooTestCase;
+
+public class BackupServiceTests extends TodorooTestCase {
+
+    File temporaryDirectory = null;
+
+    BackupDirectorySetting setting = new BackupDirectorySetting() {
+        public File getBackupDirectory() {
+            return temporaryDirectory;
+        }
+    };
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        temporaryDirectory = File.createTempFile("backup",
+                Long.toString(System.nanoTime()));
+        if(!(temporaryDirectory.delete()))
+            throw new IOException("Could not delete temp file: " + temporaryDirectory.getAbsolutePath());
+        if(!(temporaryDirectory.mkdir()))
+            throw new IOException("Could not create temp directory: " + temporaryDirectory.getAbsolutePath());
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        super.tearDown();
+
+        if(temporaryDirectory != null) {
+            for(File file : temporaryDirectory.listFiles())
+                file.delete();
+            temporaryDirectory.delete();
+        }
+    }
+
+    /** Test backup works */
+    public void testBackup() {
+        assertEquals(0, temporaryDirectory.list().length);
+
+        boolean backupSetting = Preferences.isBackupEnabled(getContext());
+        try {
+            Preferences.setBackupEnabled(getContext(), true);
+            Preferences.setBackupSummary(getContext(), "");
+
+            // create a backup
+            BackupService service = new BackupService();
+            service.setBackupDirectorySetting(setting);
+            service.testBackup(getContext());
+
+            // assert file created
+            File[] files = temporaryDirectory.listFiles();
+            assertEquals(1, files.length);
+            assertTrue(files[0].getName().matches(BackupService.BACKUP_FILE_NAME_REGEX));
+
+            // assert summary updated
+            assertTrue(Preferences.getBackupSummary(getContext()).length() > 0);
+            assertFalse(Preferences.getBackupSummary(getContext()).toLowerCase().contains("error"));
+        } finally {
+            Preferences.setBackupEnabled(getContext(), backupSetting);
+        }
+    }
+
+    /** Test no backup */
+    public void testNoBackup() {
+        assertEquals(0, temporaryDirectory.list().length);
+
+        boolean backupSetting = Preferences.isBackupEnabled(getContext());
+        try {
+            Preferences.setBackupEnabled(getContext(), false);
+            Preferences.setBackupSummary(getContext(), "");
+
+            // create a backup
+            BackupService service = new BackupService();
+            service.setBackupDirectorySetting(new BackupDirectorySetting() {
+                public File getBackupDirectory() {
+                    fail("Why was this method called?");
+                    return null;
+                }
+            });
+            service.testBackup(getContext());
+
+            // assert no file created
+            File[] files = temporaryDirectory.listFiles();
+            assertEquals(0, files.length);
+
+            // assert summary not updated
+            assertEquals(0, Preferences.getBackupSummary(getContext()).length());
+        } finally {
+            Preferences.setBackupEnabled(getContext(), backupSetting);
+        }
+    }
+
+    public void testDeletion() throws IOException {
+        // create a bunch of backups
+        assertEquals(0, temporaryDirectory.list().length);
+
+        boolean backupSetting = Preferences.isBackupEnabled(getContext());
+        try {
+            Preferences.setBackupEnabled(getContext(), true);
+            Preferences.setBackupSummary(getContext(), "");
+
+            // create some user files
+            File myFile = new File(temporaryDirectory, "beans");
+            myFile.createNewFile();
+
+            // create some backup files
+            for(int i = 0; i < 10; i++) {
+                DateFormat df = new SimpleDateFormat("MMdd-HHmm");
+                String name = String.format("auto.%02d%s.xml", i, df.format(new Date()));
+                File tempFile = new File(temporaryDirectory, name);
+                tempFile.createNewFile();
+            }
+
+            // make one really old
+            File[] files = temporaryDirectory.listFiles();
+            files[4].setLastModified(System.currentTimeMillis() - 20000);
+
+            // assert files created
+            assertEquals(11, files.length);
+
+            // backup
+            BackupService service = new BackupService();
+            service.setBackupDirectorySetting(setting);
+            service.testBackup(getContext());
+
+            // assert the oldest file was deleted
+            assertTrue(temporaryDirectory.listFiles().length < 11);
+            assertFalse(files[4].exists());
+
+            // assert user file still exists
+            service.testBackup(getContext());
+            assertTrue(myFile.exists());
+
+        } finally {
+            Preferences.setBackupEnabled(getContext(), backupSetting);
+        }
+    }
+
+}