Better handling of file attachments names, ui polish, code cleanup

astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java

@@ -1123,7 +1123,8 @@ public final class ActFmSyncService {
                         currentFiles.remove(id);
                     } else {
                         // Create new file attachment
-                        Metadata newAttachment = FileMetadata.createNewFileMetadata(model.getId(), null, file.getString("content_type"));
+                        Metadata newAttachment = FileMetadata.createNewFileMetadata(model.getId(), null,
+                                file.getString("name"), file.getString("content_type"));
                         String url = file.getString("url");
                         if (url.contains("?"))
                             url = url.substring(0, url.lastIndexOf('?'));
@@ -1358,17 +1359,6 @@ public final class ActFmSyncService {
             }
         }
 
-        public static void taskAttachmentFromJson(JSONObject json, Metadata model) throws JSONException {
-            model.clearValue(FileMetadata.REMOTE_ID);
-
-            long remoteId = json.getLong("id");
-            if (remoteId > 0)
-                model.setValue(FileMetadata.REMOTE_ID, remoteId);
-
-            model.setValue(FileMetadata.URL, json.getString("url"));
-            model.setValue(FileMetadata.FILE_TYPE, json.getString("content_type"));
-        }
-
         /** Filter out FROM */
         private static String filterRepeat(String repeat) {
             return repeat.replaceAll("BYDAY=;","").replaceAll(";?FROM=[^;]*", "");

astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java

@@ -258,37 +258,34 @@ public class ActFmSyncV2Provider extends SyncV2Provider {
 
     private <T extends AbstractModel> void pushQueued(final SyncResultCallback callback, final AtomicInteger finisher,
             TodorooCursor<T> cursor, boolean awaitTermination, final PushQueuedArgs<T> pusher) {
-        try {
+        callback.incrementMax(cursor.getCount() * 20);
-            callback.incrementMax(cursor.getCount() * 20);
+        finisher.addAndGet(cursor.getCount());
-            finisher.addAndGet(cursor.getCount());
+
-
+        ExecutorService executor = Executors.newFixedThreadPool(NUM_THREADS);
-            ExecutorService executor = Executors.newFixedThreadPool(NUM_THREADS);
+        for(int i = 0; i < cursor.getCount(); i++) {
-            for(int i = 0; i < cursor.getCount(); i++) {
+            cursor.moveToNext();
-                cursor.moveToNext();
+            final T model = pusher.getRemoteModelInstance(cursor);
-                final T model = pusher.getRemoteModelInstance(cursor);
+
-
+            executor.submit(new Runnable() {
-                executor.submit(new Runnable() {
+                public void run() {
-                    public void run() {
+                    try {
-                        try {
+                        pusher.pushRemoteModel(model);
-                            pusher.pushRemoteModel(model);
+                    } finally {
-                        } finally {
+                        callback.incrementProgress(20);
-                            callback.incrementProgress(20);
+                        if(finisher.decrementAndGet() == 0) {
-                            if(finisher.decrementAndGet() == 0) {
+                            finishSync(callback);
-                                finishSync(callback);
-                            }
                         }
                     }
-                });
-            }
-            executor.shutdown();
-            if (awaitTermination)
-                try {
-                    executor.awaitTermination(Long.MAX_VALUE, TimeUnit.MILLISECONDS);
-                } catch (InterruptedException e) {
-                    throw new RuntimeException(e);
                 }
-        } finally {
+            });
-            cursor.close();
+        }
+        executor.shutdown();
+        if (awaitTermination) {
+            try {
+                executor.awaitTermination(Long.MAX_VALUE, TimeUnit.MILLISECONDS);
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+            }
         }
     }
 
@@ -300,14 +297,22 @@ public class ActFmSyncV2Provider extends SyncV2Provider {
                                 Task.REMOTE_ID.isNull()),
                                 Criterion.and(Task.REMOTE_ID.isNotNull(),
                                         Task.MODIFICATION_DATE.gt(Task.LAST_SYNC)))));
+        try {
+            pushQueued(callback, finisher, taskCursor, true, taskPusher);
+        } finally {
+            taskCursor.close();
+        }
 
         TodorooCursor<Metadata> filesCursor = metadataService.query(Query.select(Metadata.PROPERTIES)
                 .where(Criterion.and(
                         MetadataCriteria.withKey(FileMetadata.METADATA_KEY),
                         FileMetadata.REMOTE_ID.eq(0))));
+        try {
+            pushQueued(callback, finisher, filesCursor, false, filesPusher);
+        } finally {
+            filesCursor.close();
+        }
 
-        pushQueued(callback, finisher, taskCursor, true, taskPusher);
-        pushQueued(callback, finisher, filesCursor, false, filesPusher);
     }
 
     private void pushQueuedTags(final SyncResultCallback callback,
@@ -317,8 +322,11 @@ public class ActFmSyncV2Provider extends SyncV2Provider {
                         TagData.REMOTE_ID.eq(0),
                         Criterion.and(TagData.REMOTE_ID.gt(0),
                                 TagData.MODIFICATION_DATE.gt(lastTagSyncTime)))));
-
+        try {
-        pushQueued(callback, finisher, tagDataCursor, true, tagPusher);
+            pushQueued(callback, finisher, tagDataCursor, true, tagPusher);
+        } finally {
+            tagDataCursor.close();
+        }
 
     }
 
@@ -463,15 +471,22 @@ public class ActFmSyncV2Provider extends SyncV2Provider {
                                 Task.REMOTE_ID.isNull()),
                         Criterion.and(Task.REMOTE_ID.isNotNull(),
                                 Task.MODIFICATION_DATE.gt(Task.LAST_SYNC)))));
+        try {
+            pushQueued(callback, finisher, taskCursor, true, taskPusher);
+        } finally {
+            taskCursor.close();
+        }
 
         TodorooCursor<Metadata> filesCursor = metadataService.query(Query.select(Metadata.PROPERTIES)
                 .where(Criterion.and(
                         MetadataCriteria.withKey(FileMetadata.METADATA_KEY),
                         FileMetadata.REMOTE_ID.eq(0),
                         Metadata.TASK.in(ids))));
-
+        try {
-        pushQueued(callback, finisher, taskCursor, true, taskPusher);
+            pushQueued(callback, finisher, filesCursor, false, filesPusher);
-        pushQueued(callback, finisher, filesCursor, false, filesPusher);
+        } finally {
+            filesCursor.close();
+        }
     }
 
 }

astrid/plugin-src/com/todoroo/astrid/files/AACRecordingActivity.java

@@ -1,7 +1,7 @@
 package com.todoroo.astrid.files;
 
-import java.io.File;
 import java.io.IOException;
+import java.util.concurrent.atomic.AtomicReference;
 
 import android.app.Activity;
 import android.app.ProgressDialog;
@@ -16,19 +16,17 @@ import com.timsu.astrid.R;
 import com.todoroo.aacenc.AACRecorder;
 import com.todoroo.aacenc.AACRecorder.AACRecorderCallbacks;
 import com.todoroo.aacenc.AACToM4A;
-import com.todoroo.andlib.utility.DateUtilities;
 import com.todoroo.andlib.utility.DialogUtilities;
 import com.todoroo.astrid.service.ThemeService;
 
 public class AACRecordingActivity extends Activity implements AACRecorderCallbacks {
 
     public static final String EXTRA_TEMP_FILE = "tempFile"; //$NON-NLS-1$
-    public static final String EXTRA_TASK_ID = "taskId"; //$NON-NLS-1$
     public static final String RESULT_OUTFILE = "outfile"; //$NON-NLS-1$
+    public static final String RESULT_FILENAME = "filename";  //$NON-NLS-1$
 
     private AACRecorder recorder;
     private String tempFile;
-    private long taskId;
 
     private ProgressDialog pd;
 
@@ -41,7 +39,6 @@ public class AACRecordingActivity extends Activity implements AACRecorderCallbac
         setupUi();
 
         tempFile = getIntent().getStringExtra(EXTRA_TEMP_FILE);
-        taskId = getIntent().getLongExtra(EXTRA_TASK_ID, 0L);
 
         recorder = new AACRecorder();
         recorder.setListener(this);
@@ -80,23 +77,18 @@ public class AACRecordingActivity extends Activity implements AACRecorderCallbac
         pd.show();
     }
 
-    @SuppressWarnings("nls")
     @Override
     public void encodingFinished() {
         try {
-            StringBuilder filePathBuilder = new StringBuilder();
+
-            filePathBuilder.append(getExternalFilesDir(FileMetadata.FILES_DIRECTORY).toString())
+            AtomicReference<String> nameRef = new AtomicReference<String>();
-                    .append(File.separator)
+            String outFile = FileUtilities.getNewAudioAttachmentPath(this, nameRef);
-                    .append(taskId)
+
-                    .append("_")
-                    .append(DateUtilities.now())
-                    .append("_audio.m4a");
-
-            String outFile = filePathBuilder.toString();
             new AACToM4A().convert(this, tempFile, outFile);
 
             Intent result = new Intent();
             result.putExtra(RESULT_OUTFILE, outFile);
+            result.putExtra(RESULT_FILENAME, nameRef.get());
             setResult(RESULT_OK, result);
             finish();
         } catch (IOException e) {

astrid/plugin-src/com/todoroo/astrid/files/FileMetadata.java

@@ -35,11 +35,15 @@ public class FileMetadata {
     public static final StringProperty URL = new StringProperty(Metadata.TABLE,
             Metadata.VALUE5.name);
 
+    public static final StringProperty NAME = new StringProperty(Metadata.TABLE,
+            Metadata.VALUE6.name);
 
-    public static Metadata createNewFileMetadata(long taskId, String filePath, String fileType) {
+
+    public static Metadata createNewFileMetadata(long taskId, String filePath, String fileName, String fileType) {
         Metadata metadata = new Metadata();
         metadata.setValue(Metadata.KEY, METADATA_KEY);
         metadata.setValue(Metadata.TASK, taskId);
+        metadata.setValue(NAME, fileName);
         metadata.setValue(FILE_PATH, filePath);
         metadata.setValue(FILE_TYPE, fileType);
         metadata.setValue(REMOTE_ID, 0L);

astrid/plugin-src/com/todoroo/astrid/files/FileUtilities.java

@@ -0,0 +1,74 @@
+package com.todoroo.astrid.files;
+
+import java.io.File;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.concurrent.atomic.AtomicReference;
+
+import android.content.Context;
+
+import com.timsu.astrid.R;
+import com.todoroo.andlib.utility.DateUtilities;
+
+public class FileUtilities {
+
+    /**
+     * @return Date string for use with file attachment names
+     */
+    public static String getDateStringForFilename(Context context, Date date) {
+        return DateUtilities.getDateStringHideYear(context, date) + ", " + getTimeStringForFilename(context, date); //$NON-NLS-1$
+    }
+
+    @SuppressWarnings("nls")
+    private static String getTimeStringForFilename(Context context, Date date) {
+        String value;
+        if (DateUtilities.is24HourFormat(context)) {
+            value = "HH.mm";
+        }
+        else {
+            value = "hh.mma";
+        }
+        return new SimpleDateFormat(value).format(date);
+    }
+
+    public static String getNewImageAttachmentPath(Context context, AtomicReference<String> nameReference) {
+        return getNewAttachmentPath(context, R.string.file_prefix_image, ".png", nameReference); //$NON-NLS-1$
+    }
+
+    public static String getNewAudioAttachmentPath(Context context, AtomicReference<String> nameReference) {
+        return getNewAttachmentPath(context, R.string.file_prefix_voice, ".m4a", nameReference); //$NON-NLS-1$
+    }
+
+    private static String getNewAttachmentPath(Context context, int prefixId, String extension, AtomicReference<String> nameReference) {
+        StringBuilder fileNameBuilder = new StringBuilder();
+        fileNameBuilder.append(context.getString(prefixId))
+                .append(" ") //$NON-NLS-1$
+                .append(getDateStringForFilename(context, new Date()));
+
+        String dir = context.getExternalFilesDir(FileMetadata.FILES_DIRECTORY).toString();
+
+        String name = getNonCollidingFileName(dir, fileNameBuilder.toString(), extension);
+
+        StringBuilder filePathBuilder = new StringBuilder();
+        filePathBuilder.append(dir)
+                .append(File.separator)
+                .append(name);
+        if (nameReference != null)
+            nameReference.set(name);
+
+        return filePathBuilder.toString();
+    }
+
+    private static String getNonCollidingFileName(String dir, String baseName, String extension) {
+        int tries = 1;
+        File f = new File(dir + File.separator + baseName + extension);
+        String tempName = baseName;
+        while (f.exists()) {
+            tempName = baseName + "-" + tries; //$NON-NLS-1$
+            f = new File(dir + File.separator + tempName + extension);
+            tries++;
+        }
+        return tempName + extension;
+    }
+
+}

astrid/plugin-src/com/todoroo/astrid/files/FilesControlSet.java

@@ -6,7 +6,6 @@ import java.io.InputStream;
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.Date;
 
 import android.app.Activity;
 import android.app.AlertDialog;
@@ -31,7 +30,6 @@ import com.todoroo.andlib.service.Autowired;
 import com.todoroo.andlib.service.DependencyInjectionService;
 import com.todoroo.andlib.sql.Query;
 import com.todoroo.andlib.utility.AndroidUtilities;
-import com.todoroo.andlib.utility.DateUtilities;
 import com.todoroo.andlib.utility.DialogUtilities;
 import com.todoroo.astrid.dao.MetadataDao.MetadataCriteria;
 import com.todoroo.astrid.data.Metadata;
@@ -302,30 +300,12 @@ public class FilesControlSet extends PopupControlSet {
         parent.addView(row, lp);
     }
 
-    @SuppressWarnings("nls")
     private String getNameString(Metadata metadata) {
-        String path = metadata.containsNonNullValue(FileMetadata.FILE_PATH) ? metadata.getValue(FileMetadata.FILE_PATH) : null;
+        String name = metadata.getValue(FileMetadata.NAME);
-        String name;
+        int extension = name.lastIndexOf('.');
-        if (TextUtils.isEmpty(path)) {
+        if (extension < 0)
-            name = metadata.getValue(FileMetadata.URL);
+            return name;
-            if (name.endsWith("/"))
+        return name.substring(0, extension);
-                name = name.substring(0, name.length() - 1);
-            if (name.contains("/"))
-                name = name.substring(name.lastIndexOf('/') + 1);
-        } else {
-            File f = new File(path);
-            name = f.getName();
-        }
-
-        if (name.matches("\\d+_\\d+_\\w+.\\w+")) { //$NON-NLS-1$
-            Date date = new Date(metadata.getValue(FileMetadata.ATTACH_DATE));
-            return DateUtilities.getDateStringWithTime(activity, date);
-        } else {
-            int extension = name.lastIndexOf('.');
-            if (extension < 0)
-                return name;
-            return name.substring(0, extension);
-        }
     }
 
     @SuppressWarnings("nls")

astrid/res/layout/file_display_row.xml

@@ -16,12 +16,14 @@
         android:textAppearance="@style/TextAppearance.EditRowDisplay"/>
     <TextView
         android:id="@+id/file_type"
+        android:minWidth="37dip"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:background="@drawable/file_type_background"
         android:layout_marginLeft="5dip"
         android:paddingLeft="4dip"
         android:paddingRight="4dip"
+        android:gravity="center"
         android:textColor="?attr/asTextColorInverse"/>
     <ImageView
         android:id="@+id/remove_file"

astrid/res/layout/file_row.xml

@@ -12,18 +12,21 @@
         android:layout_marginTop="5dip"
         android:layout_marginBottom="5dip"
         android:layout_marginLeft="10dip"
+        android:paddingRight="5dip"
         android:layout_weight="1"
         android:textAppearance="@style/TextAppearance.EditRowDisplay"
         android:background="@drawable/file_name_background"
-        android:gravity="center"/>
+        android:gravity="right|center_vertical"/>
     <TextView
         android:id="@+id/file_type"
+        android:minWidth="37dip"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:background="@drawable/file_type_background"
         android:layout_marginLeft="5dip"
         android:paddingLeft="4dip"
         android:paddingRight="4dip"
+        android:gravity="center"
         android:textColor="?attr/asTextColorInverse"/>
     <ImageView
         android:id="@+id/remove_file"

astrid/res/values/strings-premium.xml

@@ -17,6 +17,8 @@
     <string name="audio_encoding">Encoding...</string>
     <string name="audio_err_encoding">Error encoding audio</string>
     
+    <string name="file_prefix_image">Image</string>
+    <string name="file_prefix_voice">Voice</string>
     <string name="file_browser_up">Up</string>
     <string name="file_browser_title">Choose a file</string>
     <string name="file_browser_err_permissions">Permissions error! Please make sure you have not blocked Astrid from accessing the SD card.</string>

astrid/src/com/todoroo/astrid/activity/TaskEditFragment.java

@@ -26,6 +26,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
+import java.util.concurrent.atomic.AtomicReference;
 
 import android.app.Activity;
 import android.app.AlertDialog;
@@ -81,6 +82,7 @@ import com.todoroo.astrid.data.Task;
 import com.todoroo.astrid.files.AACRecordingActivity;
 import com.todoroo.astrid.files.FileExplore;
 import com.todoroo.astrid.files.FileMetadata;
+import com.todoroo.astrid.files.FileUtilities;
 import com.todoroo.astrid.files.FilesControlSet;
 import com.todoroo.astrid.gcal.GCalControlSet;
 import com.todoroo.astrid.helper.TaskEditControlSet;
@@ -1035,7 +1037,6 @@ ViewPager.OnPageChangeListener, EditNoteActivity.UpdatesChangedListener {
     private void startRecordingAudio() {
         Intent recordAudio = new Intent(getActivity(), AACRecordingActivity.class);
         recordAudio.putExtra(AACRecordingActivity.EXTRA_TEMP_FILE, getActivity().getFilesDir() + File.separator + "audio.aac"); //$NON-NLS-1$
-        recordAudio.putExtra(AACRecordingActivity.EXTRA_TASK_ID, model.getId());
         startActivityForResult(recordAudio, REQUEST_CODE_RECORD);
     }
 
@@ -1070,34 +1071,29 @@ ViewPager.OnPageChangeListener, EditNoteActivity.UpdatesChangedListener {
                 type = guessedType;
         }
 
-        createNewFileAttachment(path, type);
+        createNewFileAttachment(path, name, type);
     }
 
     @SuppressWarnings("nls")
     private void attachImage(Bitmap bitmap) {
-        StringBuilder filePathBuilder = new StringBuilder();
+
-        filePathBuilder.append(getActivity().getExternalFilesDir(FileMetadata.FILES_DIRECTORY).toString())
+        AtomicReference<String> nameRef = new AtomicReference<String>();
-                .append(File.separator)
+        String path = FileUtilities.getNewImageAttachmentPath(getActivity(), nameRef);
-                .append(model.getId())
+
-                .append("_")
-                .append(DateUtilities.now())
-                .append("_img.png");
-
-        String path = filePathBuilder.toString();
         try {
             FileOutputStream fos = new FileOutputStream(path);
             bitmap.compress(Bitmap.CompressFormat.PNG, 100, fos);
             fos.flush();
             fos.close();
 
-            createNewFileAttachment(path, FileMetadata.FILE_TYPE_IMAGE + "png");
+            createNewFileAttachment(path, nameRef.get(), FileMetadata.FILE_TYPE_IMAGE + "png");
         } catch (Exception e) {
             Toast.makeText(getActivity(), R.string.file_err_copy, Toast.LENGTH_LONG);
         }
     }
 
-    private void createNewFileAttachment(String path, String fileType) {
+    private void createNewFileAttachment(String path, String fileName, String fileType) {
-        Metadata fileMetadata = FileMetadata.createNewFileMetadata(model.getId(), path, fileType);
+        Metadata fileMetadata = FileMetadata.createNewFileMetadata(model.getId(), path, fileName, fileType);
         metadataService.save(fileMetadata);
         filesControlSet.refreshMetadata();
     }
@@ -1213,8 +1209,9 @@ ViewPager.OnPageChangeListener, EditNoteActivity.UpdatesChangedListener {
             // (due to the activity-change)
             notesControlSet.writeToModel(model);
         } else if (requestCode == REQUEST_CODE_RECORD && resultCode == Activity.RESULT_OK) {
-            String recordedAudio = data.getStringExtra(AACRecordingActivity.RESULT_OUTFILE);
+            String recordedAudioPath = data.getStringExtra(AACRecordingActivity.RESULT_OUTFILE);
-            createNewFileAttachment(recordedAudio, FileMetadata.FILE_TYPE_AUDIO + "m4a"); //$NON-NLS-1$
+            String recordedAudioName = data.getStringExtra(AACRecordingActivity.RESULT_FILENAME);
+            createNewFileAttachment(recordedAudioPath, recordedAudioName, FileMetadata.FILE_TYPE_AUDIO + "m4a"); //$NON-NLS-1$
         } else if (requestCode == REQUEST_CODE_ATTACH_FILE && resultCode == Activity.RESULT_OK) {
             attachFile(data.getStringExtra(FileExplore.EXTRA_FILE_SELECTED));
         }

astrid/src/com/todoroo/astrid/ui/QuickAddBar.java

@@ -1,6 +1,7 @@
 package com.todoroo.astrid.ui;
 
 import java.util.HashSet;
+import java.util.concurrent.atomic.AtomicReference;
 
 import android.app.Activity;
 import android.content.ContentValues;
@@ -47,6 +48,7 @@ import com.todoroo.astrid.data.SyncFlags;
 import com.todoroo.astrid.data.TagData;
 import com.todoroo.astrid.data.Task;
 import com.todoroo.astrid.files.FileMetadata;
+import com.todoroo.astrid.files.FileUtilities;
 import com.todoroo.astrid.gcal.GCalControlSet;
 import com.todoroo.astrid.gcal.GCalHelper;
 import com.todoroo.astrid.repeats.RepeatControlSet;
@@ -332,19 +334,14 @@ public class QuickAddBar extends LinearLayout implements RecognizerApiListener {
             }
 
             if (currentVoiceFile != null) {
-                StringBuilder filePathBuilder = new StringBuilder();
+
-                filePathBuilder.append(activity.getExternalFilesDir("audio").toString())
+                AtomicReference<String> nameRef = new AtomicReference<String>();
-                        .append("/")
+                String path = FileUtilities.getNewAudioAttachmentPath(activity, nameRef);
-                        .append(task.getId())
+
-                        .append("_")
+                voiceRecognizer.convert(path);
-                        .append(DateUtilities.now())
-                        .append("_audio.m4a");
-                String filePath = filePathBuilder.toString();
-                System.err.println("Saving to " + filePath);
-                voiceRecognizer.convert(filePath);
                 currentVoiceFile = null;
 
-                Metadata fileMetadata = FileMetadata.createNewFileMetadata(task.getId(), filePath, FileMetadata.FILE_TYPE_AUDIO + "m4a");
+                Metadata fileMetadata = FileMetadata.createNewFileMetadata(task.getId(), path, nameRef.get(), FileMetadata.FILE_TYPE_AUDIO + "m4a");
                 metadataService.save(fileMetadata);
             }