Fixed local path NULL when making favourite a file not down ; fixed change of local path for uploaded file when redownloading

src/com/owncloud/android/datamodel/FileDataStorageManager.java

@@ -27,7 +27,7 @@ import java.util.Vector;
 
 import com.owncloud.android.db.ProviderMeta;
 import com.owncloud.android.db.ProviderMeta.ProviderTableMeta;
-import com.owncloud.android.files.services.FileDownloader;
+import com.owncloud.android.utils.FileStorageUtils;
 
 import android.accounts.Account;
 import android.content.ContentProviderClient;
@@ -390,7 +390,7 @@ public class FileDataStorageManager implements DataStorageManager {
                         .getColumnIndex(ProviderTableMeta.FILE_STORAGE_PATH)));
                 if (file.getStoragePath() == null) {
                     // try to find existing file and bind it with current account
-                    File f = new File(FileDownloader.getSavePath(mAccount.name) + file.getRemotePath());
+                    File f = new File(FileStorageUtils.getDefaultSavePathFor(mAccount.name, file));
                     if (f.exists())
                         file.setStoragePath(f.getAbsolutePath());
                 }

src/com/owncloud/android/files/OwnCloudFileObserver.java

@@ -47,7 +47,7 @@ public class OwnCloudFileObserver extends FileObserver {
     DataStorageManager mStorage;
     Account mOCAccount;
     OCFile mFile;
-    static Context mContext;
+    static Context mContext;    // ISSUE 4: why is this static?
     List<FileObserverStatusListener> mListeners;
     
     public OwnCloudFileObserver(String path) {

src/com/owncloud/android/files/services/FileDownloader.java

@@ -42,9 +42,7 @@ import android.app.PendingIntent;
 import android.app.Service;
 import android.content.ContentValues;
 import android.content.Intent;
-import android.net.Uri;
 import android.os.Binder;
-import android.os.Environment;
 import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.IBinder;
@@ -93,18 +91,6 @@ public class FileDownloader extends Service implements OnDatatransferProgressLis
     private String buildRemoteName(Account account, OCFile file) {
         return account.name + file.getRemotePath();
     }
-    
-    public static final String getSavePath(String accountName) {
-        File sdCard = Environment.getExternalStorageDirectory();
-        return sdCard.getAbsolutePath() + "/owncloud/" + Uri.encode(accountName, "@");   
-            // URL encoding is an 'easy fix' to overcome that NTFS and FAT32 don't allow ":" in file names, that can be in the accountName since 0.1.190B
-    }
-    
-    public static final String getTemporalPath(String accountName) {
-        File sdCard = Environment.getExternalStorageDirectory();
-        return sdCard.getAbsolutePath() + "/owncloud/tmp/" + Uri.encode(accountName, "@");
-            // URL encoding is an 'easy fix' to overcome that NTFS and FAT32 don't allow ":" in file names, that can be in the accountName since 0.1.190B
-    }
 
     
     /**

src/com/owncloud/android/files/services/FileObserverService.java

@@ -50,7 +50,7 @@ public class FileObserverService extends Service implements FileObserverStatusLi
     public final static int CMD_DEL_OBSERVED_FILE = 3;
     public final static int CMD_ADD_DOWNLOADING_FILE = 4;
 
-    private static String TAG = "FileObserverService";
+    private static String TAG = FileObserverService.class.getSimpleName();
     private static List<OwnCloudFileObserver> mObservers;
     private static List<DownloadCompletedReceiver> mDownloadReceivers;
     private static Object mReceiverListLock = new Object();
@@ -181,7 +181,7 @@ public class FileObserverService extends Service implements FileObserverStatusLi
         if (path == null) return;
         if (mObservers == null) {
             initializeObservedList();
-            return;
+            return; // ISSUE 2: why return? ; the file still has to be removed of the mObservers !
         }
         for (int i = 0; i < mObservers.size(); ++i) {
             OwnCloudFileObserver observer = mObservers.get(i);
@@ -229,6 +229,7 @@ public class FileObserverService extends Service implements FileObserverStatusLi
         switch (status) {
             case CONFLICT:
             {
+                // ISSUE 5: if the user is not running the app (this is a service!), this can be very intrusive; a notification should be preferred
                 Intent i = new Intent(getApplicationContext(), ConflictsResolveActivity.class);
                 i.setFlags(i.getFlags() | Intent.FLAG_ACTIVITY_NEW_TASK);
                 i.putExtra("remotepath", remotePath);
@@ -257,7 +258,7 @@ public class FileObserverService extends Service implements FileObserverStatusLi
         
         @Override
         public void onReceive(Context context, Intent intent) {
-            if (mPath.equals(intent.getStringExtra(FileDownloader.EXTRA_FILE_PATH))) {
+            if (mPath.equals(intent.getStringExtra(FileDownloader.EXTRA_FILE_PATH))) {  // ISSUE 3: this condition will be false if the download failed; in that case, the download won't ever be retried
                 context.unregisterReceiver(this);
                 removeReceiverFromList(this);
                 mObserver.startWatching();

src/com/owncloud/android/operations/DownloadFileOperation.java

@@ -32,9 +32,9 @@ import org.apache.commons.httpclient.methods.GetMethod;
 import org.apache.http.HttpStatus;
 
 import com.owncloud.android.datamodel.OCFile;
-import com.owncloud.android.files.services.FileDownloader;
 import com.owncloud.android.operations.RemoteOperation;
 import com.owncloud.android.operations.RemoteOperationResult;
+import com.owncloud.android.utils.FileStorageUtils;
 
 import eu.alefzero.webdav.OnDatatransferProgressListener;
 import eu.alefzero.webdav.WebdavClient;
@@ -78,11 +78,15 @@ public class DownloadFileOperation extends RemoteOperation {
     }
 
     public String getSavePath() {
-        return FileDownloader.getSavePath(mAccount.name) + mFile.getRemotePath();
+        String path = mFile.getStoragePath();   // re-downloads should be done over the original file 
+        if (path != null && path.length() > 0) {
+            return path;
+        }
+        return FileStorageUtils.getDefaultSavePathFor(mAccount.name, mFile);
     }
     
     public String getTmpPath() {
-        return FileDownloader.getTemporalPath(mAccount.name) + mFile.getRemotePath();
+        return FileStorageUtils.getTemporalPath(mAccount.name) + mFile.getRemotePath();
     }
     
     public String getRemotePath() {

src/com/owncloud/android/operations/RenameFileOperation.java

@@ -28,8 +28,8 @@ import android.util.Log;
 
 import com.owncloud.android.datamodel.DataStorageManager;
 import com.owncloud.android.datamodel.OCFile;
-import com.owncloud.android.files.services.FileDownloader;
 import com.owncloud.android.operations.RemoteOperationResult.ResultCode;
+import com.owncloud.android.utils.FileStorageUtils;
 
 import eu.alefzero.webdav.WebdavClient;
 import eu.alefzero.webdav.WebdavUtils;
@@ -163,7 +163,7 @@ public class RenameFileOperation extends RemoteOperation {
             return false;
         }
         // create a test file
-        String tmpFolder = FileDownloader.getTemporalPath("");
+        String tmpFolder = FileStorageUtils.getTemporalPath("");
         File testFile = new File(tmpFolder + mNewName);
         try {
             testFile.createNewFile();   // return value is ignored; it could be 'false' because the file already existed, that doesn't invalidate the name

src/com/owncloud/android/operations/SynchronizeFolderOperation.java

@@ -34,6 +34,7 @@ import com.owncloud.android.datamodel.DataStorageManager;
 import com.owncloud.android.datamodel.OCFile;
 import com.owncloud.android.files.services.FileDownloader;
 import com.owncloud.android.files.services.FileObserverService;
+import com.owncloud.android.utils.FileStorageUtils;
 
 import eu.alefzero.webdav.WebdavClient;
 import eu.alefzero.webdav.WebdavEntry;
@@ -147,7 +148,7 @@ public class SynchronizeFolderOperation extends RemoteOperation {
                 // removal of obsolete files
                 mChildren = mStorageManager.getDirectoryContent(mStorageManager.getFileById(mParentId));
                 OCFile file;
-                String currentSavePath = FileDownloader.getSavePath(mAccount.name);
+                String currentSavePath = FileStorageUtils.getSavePath(mAccount.name);
                 for (int i=0; i < mChildren.size(); ) {
                     file = mChildren.get(i);
                     if (file.getLastSyncDate() != mCurrentSyncTime) {

src/com/owncloud/android/ui/fragment/FileDetailFragment.java

@@ -87,6 +87,7 @@ import com.owncloud.android.ui.activity.FileDisplayActivity;
 import com.owncloud.android.ui.activity.TransferServiceGetter;
 import com.owncloud.android.ui.dialog.EditNameDialog;
 import com.owncloud.android.ui.dialog.EditNameDialog.EditNameDialogListener;
+import com.owncloud.android.utils.FileStorageUtils;
 import com.owncloud.android.utils.OwnCloudVersion;
 
 import com.owncloud.android.R;
@@ -289,6 +290,7 @@ public class FileDetailFragment extends SherlockFragment implements
                     }
                     
                 } else {
+                    // ISSUE 6: this button should be promoted to 'synchronize' if the file is DOWN, not just redownload
                     Intent i = new Intent(getActivity(), FileDownloader.class);
                     i.putExtra(FileDownloader.EXTRA_ACCOUNT, mAccount);
                     i.putExtra(FileDownloader.EXTRA_FILE, mFile);
@@ -321,7 +323,11 @@ public class FileDetailFragment extends SherlockFragment implements
                            (cb.isChecked()?
                                    FileObserverService.CMD_ADD_OBSERVED_FILE:
                                    FileObserverService.CMD_DEL_OBSERVED_FILE));
-                intent.putExtra(FileObserverService.KEY_CMD_ARG, mFile.getStoragePath());
+                String localPath = mFile.getStoragePath();
+                if (localPath == null || localPath.length() <= 0) {
+                    localPath = FileStorageUtils.getDefaultSavePathFor(mAccount.name, mFile);
+                }
+                intent.putExtra(FileObserverService.KEY_CMD_ARG, localPath);
                 Log.e(TAG, "starting observer service");
                 getActivity().startService(intent);
                 

src/com/owncloud/android/utils/FileStorageUtils.java

@@ -0,0 +1,47 @@
+/* ownCloud Android client application
+ *   Copyright (C) 2012  Bartek Przybylski
+ *
+ *   This program is free software: you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation, either version 3 of the License, or
+ *   (at your option) any later version.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package com.owncloud.android.utils;
+
+import java.io.File;
+
+import android.net.Uri;
+import android.os.Environment;
+import com.owncloud.android.datamodel.OCFile;
+
+
+public class FileStorageUtils {
+    
+    public static final String getSavePath(String accountName) {
+        File sdCard = Environment.getExternalStorageDirectory();
+        return sdCard.getAbsolutePath() + "/owncloud/" + Uri.encode(accountName, "@");   
+            // URL encoding is an 'easy fix' to overcome that NTFS and FAT32 don't allow ":" in file names, that can be in the accountName since 0.1.190B
+    }
+    
+    public static final String getDefaultSavePathFor(String accountName, OCFile file) {
+        return getSavePath(accountName) + file.getRemotePath();
+    }
+    
+    public static final String getTemporalPath(String accountName) {
+        File sdCard = Environment.getExternalStorageDirectory();
+        return sdCard.getAbsolutePath() + "/owncloud/tmp/" + Uri.encode(accountName, "@");
+            // URL encoding is an 'easy fix' to overcome that NTFS and FAT32 don't allow ":" in file names, that can be in the accountName since 0.1.190B
+    }
+
+    
+}