Fixed some bugs in the update of OCFile#mLastSyncDateForProperties at account synchronization time ; and some refactoring

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

@@ -124,10 +124,6 @@ public class FileDataStorageManager implements DataStorageManager {
 
         if (fileExists(file.getRemotePath())) {
             OCFile oldFile = getFileByPath(file.getRemotePath());
-            if (file.getStoragePath() == null && oldFile.isDown())
-                file.setStoragePath(oldFile.getStoragePath());
-            if (!file.isDirectory());
-                cv.put(ProviderTableMeta.FILE_STORAGE_PATH, file.getStoragePath());
             file.setFileId(oldFile.getFileId());
 
             overriden = true;
@@ -203,12 +199,8 @@ public class FileDataStorageManager implements DataStorageManager {
             cv.put(ProviderTableMeta.FILE_KEEP_IN_SYNC, file.keepInSync() ? 1 : 0);
 
             if (fileExists(file.getRemotePath())) {
-                OCFile tmpfile = getFileByPath(file.getRemotePath());
-                file.setStoragePath(tmpfile.getStoragePath());
-                if (!file.isDirectory());
-                    cv.put(ProviderTableMeta.FILE_STORAGE_PATH, file.getStoragePath());
-                file.setFileId(tmpfile.getFileId());
-
+                OCFile oldFile = getFileByPath(file.getRemotePath());
+                file.setFileId(oldFile.getFileId());
                 operations.add(ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI).
                         withValues(cv).
                         withSelection(  ProviderTableMeta._ID + "=?", 
@@ -391,10 +383,12 @@ public class FileDataStorageManager implements DataStorageManager {
                 file.setStoragePath(c.getString(c
                         .getColumnIndex(ProviderTableMeta.FILE_STORAGE_PATH)));
                 if (file.getStoragePath() == null) {
-                    // try to find existing file and bind it with current account
+                    // try to find existing file and bind it with current account; - with the current update of SynchronizeFolderOperation, this won't be necessary anymore after a full synchronization of the account
                     File f = new File(FileStorageUtils.getDefaultSavePathFor(mAccount.name, file));
-                    if (f.exists())
+                    if (f.exists()) {
                         file.setStoragePath(f.getAbsolutePath());
+                        file.setLastSyncDateForData(f.lastModified());
+                    }
                 }
             }
             file.setFileLength(c.getLong(c

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

@@ -19,19 +19,21 @@
 package com.owncloud.android.files;
 
 import java.io.File;
-import java.util.LinkedList;
-import java.util.List;
 
-import com.owncloud.android.datamodel.DataStorageManager;
+import com.owncloud.android.datamodel.FileDataStorageManager;
 import com.owncloud.android.datamodel.OCFile;
 import com.owncloud.android.network.OwnCloudClientUtils;
 import com.owncloud.android.operations.RemoteOperationResult;
 import com.owncloud.android.operations.SynchronizeFileOperation;
+import com.owncloud.android.operations.RemoteOperationResult.ResultCode;
+import com.owncloud.android.ui.activity.ConflictsResolveActivity;
+import com.owncloud.android.utils.FileStorageUtils;
 
 import eu.alefzero.webdav.WebdavClient;
 
 import android.accounts.Account;
 import android.content.Context;
+import android.content.Intent;
 import android.os.FileObserver;
 import android.util.Log;
 
@@ -40,51 +42,31 @@ public class OwnCloudFileObserver extends FileObserver {
     public static int CHANGES_ONLY = CLOSE_WRITE;
     
     private static String TAG = OwnCloudFileObserver.class.getSimpleName();
+    
     private String mPath;
     private int mMask;
-    private DataStorageManager mStorage;
     private Account mOCAccount;
     private OCFile mFile;
     private Context mContext;
-    private List<FileObserverStatusListener> mListeners;
-    
-    public OwnCloudFileObserver(String path) {
-        this(path, ALL_EVENTS);
-    }
+
     
-    public OwnCloudFileObserver(String path, int mask) {
+    public OwnCloudFileObserver(String path, OCFile file, Account account, Context context, int mask) {
         super(path, mask);
+        if (path == null)
+            throw new IllegalArgumentException("NULL path argument received"); 
+        if (file == null)
+            throw new IllegalArgumentException("NULL file argument received"); 
+        if (account == null)
+            throw new IllegalArgumentException("NULL account argument received"); 
+        if (context == null)
+            throw new IllegalArgumentException("NULL context argument received");
+        if (!path.equals(file.getStoragePath()) && !path.equals(FileStorageUtils.getDefaultSavePathFor(account.name, file)))
+            throw new IllegalArgumentException("File argument is not linked to the local file set in path argument");
         mPath = path;
-        mMask = mask;
-        mListeners = new LinkedList<FileObserverStatusListener>();
-    }
-    
-    public void setAccount(Account account) {
-        mOCAccount = account;
-    }
-    
-    public void setStorageManager(DataStorageManager manager) {
-        mStorage = manager;
-    }
-    
-    public void setOCFile(OCFile file) {
         mFile = file;
-    }
-    
-    public void setContext(Context context) {
-        mContext = context;
-    }
-
-    public String getPath() {
-        return mPath;
-    }
-    
-    public String getRemotePath() {
-        return mFile.getRemotePath();
-    }
-    
-    public void addObserverStatusListener(FileObserverStatusListener listener) {
-        mListeners.add(listener);
+        mOCAccount = account;
+        mContext = context; 
+        mMask = mask;
     }
     
     @Override
@@ -94,34 +76,29 @@ public class OwnCloudFileObserver extends FileObserver {
             Log.wtf(TAG, "Incorrect event " + event + " sent for file " + mPath + ((path != null) ? File.separator + path : "") +
                          " with registered for " + mMask + " and original path " +
                          mPath);
-            /* Unexpected event that will be ignored; no reason to propagate it 
-            for (FileObserverStatusListener l : mListeners)
-                l.OnObservedFileStatusUpdate(mPath, getRemotePath(), mOCAccount, Status.INCORRECT_MASK);
-            */
             return;
         }
         WebdavClient wc = OwnCloudClientUtils.createOwnCloudClient(mOCAccount, mContext);
-        SynchronizeFileOperation sfo = new SynchronizeFileOperation(mFile, null, mStorage, mOCAccount, true, false, mContext);
+        SynchronizeFileOperation sfo = new SynchronizeFileOperation(mFile, 
+                                                                    null, 
+                                                                    new FileDataStorageManager(mOCAccount, mContext.getContentResolver()), 
+                                                                    mOCAccount, 
+                                                                    true, 
+                                                                    true, 
+                                                                    mContext);
         RemoteOperationResult result = sfo.execute(wc);
-        for (FileObserverStatusListener l : mListeners) {
-            l.onObservedFileStatusUpdate(mPath, getRemotePath(), mOCAccount, result);
+        if (result.getCode() == ResultCode.SYNC_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(mContext, ConflictsResolveActivity.class);
+            i.setFlags(i.getFlags() | Intent.FLAG_ACTIVITY_NEW_TASK);
+            i.putExtra("remotepath", mFile.getRemotePath());
+            i.putExtra("localpath", mPath);
+            i.putExtra("account", mOCAccount);
+            mContext.startActivity(i);
         }
-        
-    }
-    
-    public interface FileObserverStatusListener {
-        public void onObservedFileStatusUpdate(String localPath,
-                                               String remotePath,
-                                               Account account,
-                                               RemoteOperationResult result);
-    }
-
-    public OCFile getOCFile() {
-        return mFile;
-    }
-
-    public Account getAccount() {
-        return mOCAccount;
+        // TODO save other errors in some point where the user can inspect them later;
+        //      or maybe just toast them;
+        //      or nothing, very strange fails
     }
     
 }

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

@@ -26,11 +26,7 @@ import com.owncloud.android.datamodel.FileDataStorageManager;
 import com.owncloud.android.datamodel.OCFile;
 import com.owncloud.android.db.ProviderMeta.ProviderTableMeta;
 import com.owncloud.android.files.OwnCloudFileObserver;
-import com.owncloud.android.files.OwnCloudFileObserver.FileObserverStatusListener;
-import com.owncloud.android.operations.RemoteOperationResult;
-import com.owncloud.android.operations.RemoteOperationResult.ResultCode;
 import com.owncloud.android.operations.SynchronizeFileOperation;
-import com.owncloud.android.ui.activity.ConflictsResolveActivity;
 import com.owncloud.android.utils.FileStorageUtils;
 
 import android.accounts.Account;
@@ -45,7 +41,7 @@ import android.os.Binder;
 import android.os.IBinder;
 import android.util.Log;
 
-public class FileObserverService extends Service implements FileObserverStatusListener {
+public class FileObserverService extends Service {
 
     public final static int CMD_INIT_OBSERVED_LIST = 1;
     public final static int CMD_ADD_OBSERVED_FILE = 2;
@@ -77,7 +73,7 @@ public class FileObserverService extends Service implements FileObserverStatusLi
         registerReceiver(mDownloadReceiver, filter);
         
         mObserversMap = new HashMap<String, OwnCloudFileObserver>();
-        initializeObservedList();
+        //initializeObservedList();
     }
     
     
@@ -86,6 +82,7 @@ public class FileObserverService extends Service implements FileObserverStatusLi
         super.onDestroy();
         unregisterReceiver(mDownloadReceiver);
         mObserversMap = null;   // TODO study carefully the life cycle of Services to grant the best possible observance
+        Log.d(TAG, "Bye, bye");
     }
     
     
@@ -159,12 +156,11 @@ public class FileObserverService extends Service implements FileObserverStatusLi
 
             String path = c.getString(c.getColumnIndex(ProviderTableMeta.FILE_STORAGE_PATH));
             OwnCloudFileObserver observer =
-                    new OwnCloudFileObserver(path, OwnCloudFileObserver.CHANGES_ONLY);
-            observer.setContext(getApplicationContext());
-            observer.setAccount(account);
-            observer.setStorageManager(storage);
-            observer.setOCFile(storage.getFileByPath(c.getString(c.getColumnIndex(ProviderTableMeta.FILE_PATH))));
-            observer.addObserverStatusListener(this);
+                    new OwnCloudFileObserver(   path, 
+                                                storage.getFileByPath(c.getString(c.getColumnIndex(ProviderTableMeta.FILE_PATH))),
+                                                account, 
+                                                getApplicationContext(), 
+                                                OwnCloudFileObserver.CHANGES_ONLY);
             mObserversMap.put(path, observer);
             if (new File(path).exists()) {
                 observer.startWatching();
@@ -175,6 +171,7 @@ public class FileObserverService extends Service implements FileObserverStatusLi
         c.close();
     }
     
+    
     /**
      * Registers the local copy of a remote file to be observed for local changes,
      * an automatically updated in the ownCloud server.
@@ -200,17 +197,11 @@ public class FileObserverService extends Service implements FileObserverStatusLi
         OwnCloudFileObserver observer = mObserversMap.get(localPath);
         if (observer == null) {
             /// the local file was never registered to observe before
-            observer = new OwnCloudFileObserver(localPath, OwnCloudFileObserver.CHANGES_ONLY);
-            //Account account = AccountUtils.getCurrentOwnCloudAccount(getApplicationContext());
-            observer.setAccount(account);
-            FileDataStorageManager storage =
-                    new FileDataStorageManager(account, getContentResolver());  // I don't trust in this resolver's life span...
-            observer.setStorageManager(storage);
-            //observer.setOCFile(storage.getFileByLocalPath(path));   // ISSUE 10 - the fix in FileDetailsFragment to avoid path == null was not enough; it the file was never down before, this sets a NULL OCFile in the observer
-            observer.setOCFile(file);
-            observer.addObserverStatusListener(this);
-            observer.setContext(getApplicationContext());
-
+            observer = new OwnCloudFileObserver(   localPath, 
+                                                    file,
+                                                    account, 
+                                                    getApplicationContext(), 
+                                                    OwnCloudFileObserver.CHANGES_ONLY);
             mObserversMap.put(localPath, observer);
             Log.d(TAG, "Observer added for path " + localPath);
         
@@ -254,26 +245,6 @@ public class FileObserverService extends Service implements FileObserverStatusLi
         
     }
 
-    
-    @Override
-    public void onObservedFileStatusUpdate(String localPath, String remotePath, Account account, RemoteOperationResult result) {
-        if (!result.isSuccess()) {
-            if (result.getCode() == ResultCode.SYNC_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);
-                i.putExtra("localpath", localPath);
-                i.putExtra("account", account);
-                startActivity(i);
-                
-            } else {
-                // TODO send notification to the notification bar?
-            }
-        } // else, nothing else to do; now it's duty of FileUploader service 
-    }
-    
-    
 
     /**
      *  Private receiver listening to events broadcast by the FileDownloader service.

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

@@ -419,12 +419,14 @@ public class FileUploader extends Service implements OnDatatransferProgressListe
     private void saveUploadedFile() {
         OCFile file = mCurrentUpload.getFile();
         
+        /// new PROPFIND to keep data consistent with server in theory, should return the same we already have
         PropFindMethod propfind = null;
         RemoteOperationResult result = null;
+        long syncDate = System.currentTimeMillis();
         try {
           propfind = new PropFindMethod(mUploadClient.getBaseUri() + WebdavUtils.encodePath(mCurrentUpload.getRemotePath()));
           int status = mUploadClient.executeMethod(propfind);
-          boolean isMultiStatus = status == HttpStatus.SC_MULTI_STATUS;
+          boolean isMultiStatus = (status == HttpStatus.SC_MULTI_STATUS);
           if (isMultiStatus) {
               MultiStatus resp = propfind.getResponseBodyAsMultiStatus();
               WebdavEntry we = new WebdavEntry(resp.getResponses()[0],
@@ -432,10 +434,10 @@ public class FileUploader extends Service implements OnDatatransferProgressListe
               OCFile newFile = fillOCFile(we);
               newFile.setStoragePath(file.getStoragePath());
               newFile.setKeepInSync(file.keepInSync());
+              newFile.setLastSyncDateForProperties(syncDate);
               file = newFile;
               
           } else {
-              // this would be a problem
               mUploadClient.exhaustResponse(propfind.getResponseBodyAsStream());
           }
           
@@ -451,27 +453,22 @@ public class FileUploader extends Service implements OnDatatransferProgressListe
                 propfind.releaseConnection();
         }
 
-        long syncDate = System.currentTimeMillis();
-        if (result.isSuccess()) {
-            file.setLastSyncDateForProperties(syncDate);
-            
-        } else {
-            // file was successfully uploaded, but the new time stamp and Etag in the server could not be read; 
-            // just keeping old values :(
-            if (!mCurrentUpload.getRemotePath().equals(file.getRemotePath())) {
-                // true when the file was automatically renamed to avoid an overwrite 
-                OCFile newFile = new OCFile(mCurrentUpload.getRemotePath());
-                newFile.setCreationTimestamp(file.getCreationTimestamp());
-                newFile.setFileLength(file.getFileLength());
-                newFile.setMimetype(file.getMimetype());
-                newFile.setModificationTimestamp(file.getModificationTimestamp());
-                newFile.setLastSyncDateForProperties(file.getLastSyncDateForProperties());
-                newFile.setKeepInSync(file.keepInSync());
-                // newFile.setEtag(file.getEtag()) // TODO and this is still worse 
-                file = newFile;
-            }
+        file.setLastSyncDateForData(syncDate);  // this is right, no matter if the PROPFIND was successful or not
+        
+        if (!result.isSuccess() && !mCurrentUpload.getRemotePath().equals(file.getRemotePath())) {
+            // true when the file was automatically renamed to avoid an overwrite ; yes, this is a bit obscure...
+            OCFile newFile = new OCFile(mCurrentUpload.getRemotePath());
+            newFile.setCreationTimestamp(file.getCreationTimestamp());
+            newFile.setFileLength(file.getFileLength());
+            newFile.setMimetype(file.getMimetype());
+            newFile.setModificationTimestamp(file.getModificationTimestamp());
+            newFile.setLastSyncDateForProperties(file.getLastSyncDateForProperties());
+            newFile.setStoragePath(file.getStoragePath());
+            newFile.setKeepInSync(file.keepInSync());
+            // newFile.setEtag(file.getEtag())
+            file = newFile;
         }
-        file.setLastSyncDateForData(syncDate);
+        
         mStorageManager.saveFile(file);
     }
 

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

@@ -95,6 +95,7 @@ public class SynchronizeFileOperation extends RemoteOperation {
                         WebdavEntry we = new WebdavEntry(resp.getResponses()[0],
                                                client.getBaseUri().getPath());
                         mServerFile = fillOCFile(we);
+                        mServerFile.setLastSyncDateForProperties(System.currentTimeMillis());
                         
                     } else {
                         client.exhaustResponse(propfind.getResponseBodyAsStream());
@@ -137,6 +138,8 @@ public class SynchronizeFileOperation extends RemoteOperation {
                         } else {
                             // TODO CHECK: is this really useful in some point in the code?
                             mServerFile.setKeepInSync(mLocalFile.keepInSync());
+                            mServerFile.setLastSyncDateForData(mLocalFile.getLastSyncDateForData());
+                            mServerFile.setStoragePath(mLocalFile.getStoragePath());
                             mServerFile.setParentId(mLocalFile.getParentId());
                             mStorageManager.saveFile(mServerFile);
                             
@@ -210,8 +213,6 @@ public class SynchronizeFileOperation extends RemoteOperation {
         file.setFileLength(we.contentLength());
         file.setMimetype(we.contentType());
         file.setModificationTimestamp(we.modifiedTimesamp());
-        file.setLastSyncDateForProperties(System.currentTimeMillis());
-        file.setLastSyncDateForData(0);
         return file;
     }
 

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

@@ -18,6 +18,7 @@
 
 package com.owncloud.android.operations;
 
+import java.io.File;
 import java.util.List;
 import java.util.Vector;
 
@@ -138,27 +139,40 @@ public class SynchronizeFolderOperation extends RemoteOperation {
                 List<OCFile> updatedFiles = new Vector<OCFile>(resp.getResponses().length - 1);
                 List<SynchronizeFileOperation> filesToSyncContents = new Vector<SynchronizeFileOperation>();
                 for (int i = 1; i < resp.getResponses().length; ++i) {
+                    /// new OCFile instance with the data from the server
                     WebdavEntry we = new WebdavEntry(resp.getResponses()[i], client.getBaseUri().getPath());
                     OCFile file = fillOCFile(we);
+                    
+                    /// set data about local state, keeping unchanged former data if existing
+                    file.setLastSyncDateForProperties(mCurrentSyncTime);
                     OCFile oldFile = mStorageManager.getFileByPath(file.getRemotePath());
                     if (oldFile != null) {
                         file.setKeepInSync(oldFile.keepInSync());
                         file.setLastSyncDateForData(oldFile.getLastSyncDateForData());
-                        if (file.keepInSync()) {
-                            //disableObservance(file);        // first disable observer so we won't get file upload right after download
-                            //                                // now, the FileDownloader service sends a broadcast before start a download; the FileObserverService is listening for it
-                            //requestFileSynchronization(file, oldFile, client);
-                            SynchronizeFileOperation operation = new SynchronizeFileOperation(  oldFile,        
-                                                                                                file, 
-                                                                                                mStorageManager,
-                                                                                                mAccount,       
-                                                                                                true, 
-                                                                                                false,          
-                                                                                                mContext
-                                                                                                );
-                            filesToSyncContents.add(operation);
+                        file.setStoragePath(oldFile.getStoragePath());
+                    }
+
+                    /// scan default location if local copy of file is not linked in OCFile instance
+                    if (file.getStoragePath() == null && !file.isDirectory()) {
+                        File f = new File(FileStorageUtils.getDefaultSavePathFor(mAccount.name, file));
+                        if (f.exists()) {
+                            file.setStoragePath(f.getAbsolutePath());
+                            file.setLastSyncDateForData(f.lastModified());
                         }
                     }
+                    
+                    /// prepare content synchronization for kept-in-sync files
+                    if (file.keepInSync()) {
+                        SynchronizeFileOperation operation = new SynchronizeFileOperation(  oldFile,        
+                                                                                            file, 
+                                                                                            mStorageManager,
+                                                                                            mAccount,       
+                                                                                            true, 
+                                                                                            false,          
+                                                                                            mContext
+                                                                                            );
+                        filesToSyncContents.add(operation);
+                    }
                 
                     updatedFiles.add(file);
                 }
@@ -250,7 +264,6 @@ public class SynchronizeFolderOperation extends RemoteOperation {
         file.setFileLength(we.contentLength());
         file.setMimetype(we.contentType());
         file.setModificationTimestamp(we.modifiedTimesamp());
-        file.setLastSyncDateForProperties(mCurrentSyncTime);
         file.setParentId(mParentId);
         return file;
     }