From: David A. Velasco Date: Mon, 19 Nov 2012 14:08:48 +0000 (+0100) Subject: Fixed some bugs in the update of OCFile#mLastSyncDateForProperties at account synchro... X-Git-Tag: oc-android-1.4.3~100 X-Git-Url: http://git.linex4red.de/pub/Android/ownCloud.git/commitdiff_plain/22a789e8d2cecffc6b445f4487ddaada2d6f5e4a?ds=inline Fixed some bugs in the update of OCFile#mLastSyncDateForProperties at account synchronization time ; and some refactoring --- diff --git a/src/com/owncloud/android/datamodel/FileDataStorageManager.java b/src/com/owncloud/android/datamodel/FileDataStorageManager.java index 4c92d09b..7d9a678c 100644 --- a/src/com/owncloud/android/datamodel/FileDataStorageManager.java +++ b/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 diff --git a/src/com/owncloud/android/files/OwnCloudFileObserver.java b/src/com/owncloud/android/files/OwnCloudFileObserver.java index 3cbdb1e5..1854f349 100644 --- a/src/com/owncloud/android/files/OwnCloudFileObserver.java +++ b/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 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(); - } - - 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 } } diff --git a/src/com/owncloud/android/files/services/FileObserverService.java b/src/com/owncloud/android/files/services/FileObserverService.java index 4caf8f42..1b665092 100644 --- a/src/com/owncloud/android/files/services/FileObserverService.java +++ b/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(); - 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. diff --git a/src/com/owncloud/android/files/services/FileUploader.java b/src/com/owncloud/android/files/services/FileUploader.java index 754e1610..81841a4d 100644 --- a/src/com/owncloud/android/files/services/FileUploader.java +++ b/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); } diff --git a/src/com/owncloud/android/operations/SynchronizeFileOperation.java b/src/com/owncloud/android/operations/SynchronizeFileOperation.java index 77724d09..31dd5dc8 100644 --- a/src/com/owncloud/android/operations/SynchronizeFileOperation.java +++ b/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; } diff --git a/src/com/owncloud/android/operations/SynchronizeFolderOperation.java b/src/com/owncloud/android/operations/SynchronizeFolderOperation.java index 12e7f3f0..bfff6800 100644 --- a/src/com/owncloud/android/operations/SynchronizeFolderOperation.java +++ b/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 updatedFiles = new Vector(resp.getResponses().length - 1); List filesToSyncContents = new Vector(); 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; }