From: David A. Velasco Date: Thu, 15 Nov 2012 18:29:30 +0000 (+0100) Subject: Fixed bug: when a file is checked as 'keep in sync' and the immediate synchronization... X-Git-Tag: oc-android-1.4.3~104 X-Git-Url: http://git.linex4red.de/pub/Android/ownCloud.git/commitdiff_plain/97dd59063ba1bf16fa2c176563614a9ce310b158 Fixed bug: when a file is checked as 'keep in sync' and the immediate synchronization results in an upload, when this finish the 'kepp in sync' check was being forgotten; not more --- diff --git a/src/com/owncloud/android/files/OwnCloudFileObserver.java b/src/com/owncloud/android/files/OwnCloudFileObserver.java index c2a066ee..f72e2ce0 100644 --- a/src/com/owncloud/android/files/OwnCloudFileObserver.java +++ b/src/com/owncloud/android/files/OwnCloudFileObserver.java @@ -101,7 +101,7 @@ public class OwnCloudFileObserver extends FileObserver { return; } WebdavClient wc = OwnCloudClientUtils.createOwnCloudClient(mOCAccount, mContext); - SynchronizeFileOperation sfo = new SynchronizeFileOperation(getRemotePath(), mStorage, mOCAccount, true, false, mContext); + SynchronizeFileOperation sfo = new SynchronizeFileOperation(mFile, mStorage, mOCAccount, true, false, mContext); RemoteOperationResult result = sfo.execute(wc); for (FileObserverStatusListener l : mListeners) { l.onObservedFileStatusUpdate(mPath, getRemotePath(), mOCAccount, result); diff --git a/src/com/owncloud/android/files/services/FileObserverService.java b/src/com/owncloud/android/files/services/FileObserverService.java index 678c46d2..cfd5fec8 100644 --- a/src/com/owncloud/android/files/services/FileObserverService.java +++ b/src/com/owncloud/android/files/services/FileObserverService.java @@ -156,10 +156,6 @@ public class FileObserverService extends Service implements FileObserverStatusLi * Registers the local copy of a remote file to be observed for local changes, * an automatically updated in the ownCloud server. * - * If there is no local copy of the remote file, a request to download it is send - * to the FileDownloader service. The observation is delayed until the download - * is finished. - * * @param file Object representing a remote file which local copy must be observed. * @param account OwnCloud account containing file. */ @@ -216,11 +212,6 @@ public class FileObserverService extends Service implements FileObserverStatusLi DownloadCompletedReceiver receiver = new DownloadCompletedReceiver(localPath, observer); registerReceiver(receiver, new IntentFilter(FileDownloader.DOWNLOAD_FINISH_MESSAGE)); - Intent i = new Intent(this, FileDownloader.class); - i.putExtra(FileDownloader.EXTRA_ACCOUNT, account); - i.putExtra(FileDownloader.EXTRA_FILE, file); - startService(i); - } else { observer.startWatching(); Log.d(TAG, "Started watching " + localPath); diff --git a/src/com/owncloud/android/files/services/FileUploader.java b/src/com/owncloud/android/files/services/FileUploader.java index 20c81adf..754e1610 100644 --- a/src/com/owncloud/android/files/services/FileUploader.java +++ b/src/com/owncloud/android/files/services/FileUploader.java @@ -76,8 +76,9 @@ public class FileUploader extends Service implements OnDatatransferProgressListe public static final String EXTRA_FILE_PATH = "FILE_PATH"; public static final String ACCOUNT_NAME = "ACCOUNT_NAME"; - public static final String KEY_LOCAL_FILE = "LOCAL_FILE"; - public static final String KEY_REMOTE_FILE = "REMOTE_FILE"; + public static final String KEY_FILE = "FILE"; + public static final String KEY_LOCAL_FILE = "LOCAL_FILE"; // TODO remove this as a possible input argument ; use KEY_FILE everywhere + public static final String KEY_REMOTE_FILE = "REMOTE_FILE"; // TODO remove this as a possible input argument ; use KEY_FILE everywhere public static final String KEY_MIME_TYPE = "MIME_TYPE"; public static final String KEY_ACCOUNT = "ACCOUNT"; @@ -158,7 +159,7 @@ public class FileUploader extends Service implements OnDatatransferProgressListe */ @Override public int onStartCommand(Intent intent, int flags, int startId) { - if (!intent.hasExtra(KEY_ACCOUNT) || !intent.hasExtra(KEY_UPLOAD_TYPE)) { + if (!intent.hasExtra(KEY_ACCOUNT) || !intent.hasExtra(KEY_UPLOAD_TYPE) || !(intent.hasExtra(KEY_LOCAL_FILE) || intent.hasExtra(KEY_FILE))) { Log.e(TAG, "Not enough information provided in intent"); return Service.START_NOT_STICKY; } @@ -169,33 +170,57 @@ public class FileUploader extends Service implements OnDatatransferProgressListe } Account account = intent.getParcelableExtra(KEY_ACCOUNT); - String[] localPaths, remotePaths, mimeTypes; + String[] localPaths = null, remotePaths = null, mimeTypes = null; + OCFile[] files = null; if (uploadType == UPLOAD_SINGLE_FILE) { - localPaths = new String[] { intent.getStringExtra(KEY_LOCAL_FILE) }; - remotePaths = new String[] { intent - .getStringExtra(KEY_REMOTE_FILE) }; - mimeTypes = new String[] { intent.getStringExtra(KEY_MIME_TYPE) }; + + if (intent.hasExtra(KEY_FILE)) { + files = new OCFile[] {intent.getParcelableExtra(KEY_FILE) }; + + } else { + localPaths = new String[] { intent.getStringExtra(KEY_LOCAL_FILE) }; + remotePaths = new String[] { intent.getStringExtra(KEY_REMOTE_FILE) }; + mimeTypes = new String[] { intent.getStringExtra(KEY_MIME_TYPE) }; + } } else { // mUploadType == UPLOAD_MULTIPLE_FILES - localPaths = intent.getStringArrayExtra(KEY_LOCAL_FILE); - remotePaths = intent.getStringArrayExtra(KEY_REMOTE_FILE); - mimeTypes = intent.getStringArrayExtra(KEY_MIME_TYPE); + + if (intent.hasExtra(KEY_FILE)) { + files = (OCFile[]) intent.getParcelableArrayExtra(KEY_FILE); // TODO will this casting work fine? + + } else { + localPaths = intent.getStringArrayExtra(KEY_LOCAL_FILE); + remotePaths = intent.getStringArrayExtra(KEY_REMOTE_FILE); + mimeTypes = intent.getStringArrayExtra(KEY_MIME_TYPE); + } } - if (localPaths == null) { - Log.e(TAG, "Incorrect array for local paths provided in upload intent"); - return Service.START_NOT_STICKY; - } - if (remotePaths == null) { - Log.e(TAG, "Incorrect array for remote paths provided in upload intent"); + FileDataStorageManager storageManager = new FileDataStorageManager(account, getContentResolver()); + + if (intent.hasExtra(KEY_FILE) && files == null) { + Log.e(TAG, "Incorrect array for OCFiles provided in upload intent"); return Service.START_NOT_STICKY; - } - if (localPaths.length != remotePaths.length) { - Log.e(TAG, "Different number of remote paths and local paths!"); - return Service.START_NOT_STICKY; + } else if (!intent.hasExtra(KEY_FILE)) { + if (localPaths == null) { + Log.e(TAG, "Incorrect array for local paths provided in upload intent"); + return Service.START_NOT_STICKY; + } + if (remotePaths == null) { + Log.e(TAG, "Incorrect array for remote paths provided in upload intent"); + return Service.START_NOT_STICKY; + } + if (localPaths.length != remotePaths.length) { + Log.e(TAG, "Different number of remote paths and local paths!"); + return Service.START_NOT_STICKY; + } + + files = new OCFile[localPaths.length]; + for (int i=0; i < localPaths.length; i++) { + files[i] = obtainNewOCFileToUpload(remotePaths[i], localPaths[i], ((mimeTypes!=null)?mimeTypes[i]:(String)null), storageManager); + } } - + boolean isInstant = intent.getBooleanExtra(KEY_INSTANT_UPLOAD, false); boolean forceOverwrite = intent.getBooleanExtra(KEY_FORCE_OVERWRITE, false); @@ -204,27 +229,17 @@ public class FileUploader extends Service implements OnDatatransferProgressListe AbstractList requestedUploads = new Vector(); String uploadKey = null; UploadFileOperation newUpload = null; - OCFile file = null; - FileDataStorageManager storageManager = new FileDataStorageManager(account, getContentResolver()); boolean fixed = false; if (isInstant) { fixed = checkAndFixInstantUploadDirectory(storageManager); } try { - for (int i=0; i < localPaths.length; i++) { - uploadKey = buildRemoteName(account, remotePaths[i]); - file = storageManager.getFileByLocalPath(remotePaths[i]); - if (file != null) { - Log.d(TAG, "Upload of file already in server: " + remotePaths[i]); - // TODO - review handling of input OCFiles in FileDownloader and FileUploader ; some times retrieving them from database can be necessary, some times not; we should make something consistent - } else { - Log.d(TAG, "Upload of new file: " + remotePaths[i]); - file = obtainNewOCFileToUpload(remotePaths[i], localPaths[i], ((mimeTypes!=null)?mimeTypes[i]:(String)null), isInstant, storageManager); - } + for (int i=0; i < files.length; i++) { + uploadKey = buildRemoteName(account, files[i].getRemotePath()); if (chunked) { - newUpload = new ChunkedUploadFileOperation(account, file, isInstant, forceOverwrite); + newUpload = new ChunkedUploadFileOperation(account, files[i], isInstant, forceOverwrite); } else { - newUpload = new UploadFileOperation(account, file, isInstant, forceOverwrite); + newUpload = new UploadFileOperation(account, files[i], isInstant, forceOverwrite); } if (fixed && i==0) { newUpload.setRemoteFolderToBeCreated(); @@ -486,7 +501,7 @@ public class FileUploader extends Service implements OnDatatransferProgressListe } - private OCFile obtainNewOCFileToUpload(String remotePath, String localPath, String mimeType, boolean isInstant, FileDataStorageManager storageManager) { + private OCFile obtainNewOCFileToUpload(String remotePath, String localPath, String mimeType, FileDataStorageManager storageManager) { OCFile newFile = new OCFile(remotePath); newFile.setStoragePath(localPath); newFile.setLastSyncDateForProperties(0); diff --git a/src/com/owncloud/android/operations/SynchronizeFileOperation.java b/src/com/owncloud/android/operations/SynchronizeFileOperation.java index 71c6d437..9923c17e 100644 --- a/src/com/owncloud/android/operations/SynchronizeFileOperation.java +++ b/src/com/owncloud/android/operations/SynchronizeFileOperation.java @@ -40,7 +40,8 @@ import eu.alefzero.webdav.WebdavUtils; public class SynchronizeFileOperation extends RemoteOperation { private String TAG = SynchronizeFileOperation.class.getSimpleName(); - private String mRemotePath; + //private String mRemotePath; + private OCFile mLocalFile; private DataStorageManager mStorageManager; private Account mAccount; private boolean mSyncFileContents; @@ -50,14 +51,15 @@ public class SynchronizeFileOperation extends RemoteOperation { private boolean mTransferWasRequested = false; public SynchronizeFileOperation( - String remotePath, + OCFile localFile, DataStorageManager dataStorageManager, Account account, boolean syncFileContents, boolean localChangeAlreadyKnown, Context context) { - mRemotePath = remotePath; + //mRemotePath = remotePath; + mLocalFile = localFile; mStorageManager = dataStorageManager; mAccount = account; mSyncFileContents = syncFileContents; @@ -73,17 +75,15 @@ public class SynchronizeFileOperation extends RemoteOperation { RemoteOperationResult result = null; mTransferWasRequested = false; try { - OCFile localFile = mStorageManager.getFileByPath(mRemotePath); - - if (!localFile.isDown()) { + if (!mLocalFile.isDown()) { /// easy decision - requestForDownload(localFile); + requestForDownload(mLocalFile); result = new RemoteOperationResult(ResultCode.OK); } else { - /// local copy in the device -> need to think a bit more before do nothing + /// local copy in the device -> need to think a bit more before do anything - propfind = new PropFindMethod(client.getBaseUri() + WebdavUtils.encodePath(mRemotePath)); + propfind = new PropFindMethod(client.getBaseUri() + WebdavUtils.encodePath(mLocalFile.getRemotePath())); int status = client.executeMethod(propfind); boolean isMultiStatus = status == HttpStatus.SC_MULTI_STATUS; if (isMultiStatus) { @@ -95,12 +95,13 @@ public class SynchronizeFileOperation extends RemoteOperation { /// check changes in server and local file boolean serverChanged = false; if (serverFile.getEtag() != null) { - serverChanged = (!serverFile.getEtag().equals(localFile.getEtag())); + serverChanged = (!serverFile.getEtag().equals(mLocalFile.getEtag())); // TODO could this be dangerous when the user upgrades the server from non-tagged to tagged? } else { // server without etags - serverChanged = (serverFile.getModificationTimestamp() > localFile.getModificationTimestamp()); + serverChanged = (serverFile.getModificationTimestamp() > mLocalFile.getModificationTimestamp()); } - boolean localChanged = (mLocalChangeAlreadyKnown || localFile.getLocalModificationTimestamp() > localFile.getLastSyncDateForData()); + boolean localChanged = (mLocalChangeAlreadyKnown || mLocalFile.getLocalModificationTimestamp() > mLocalFile.getLastSyncDateForData()); + // TODO this will be always true after the app is upgraded to database version 3; will result in unnecessary uploads /// decide action to perform depending upon changes if (localChanged && serverChanged) { @@ -109,7 +110,7 @@ public class SynchronizeFileOperation extends RemoteOperation { } else if (localChanged) { if (mSyncFileContents) { - requestForUpload(localFile); + requestForUpload(mLocalFile); // the local update of file properties will be done by the FileUploader service when the upload finishes } else { // NOTHING TO DO HERE: updating the properties of the file in the server without uploading the contents would be stupid; @@ -120,12 +121,12 @@ public class SynchronizeFileOperation extends RemoteOperation { } else if (serverChanged) { if (mSyncFileContents) { - requestForDownload(serverFile); + requestForDownload(mLocalFile); // local, not server; we won't to keep the value of keepInSync! // the update of local data will be done later by the FileUploader service when the upload finishes } else { // TODO CHECK: is this really useful in some point in the code? - serverFile.setKeepInSync(localFile.keepInSync()); - serverFile.setParentId(localFile.getParentId()); + serverFile.setKeepInSync(mLocalFile.keepInSync()); + serverFile.setParentId(mLocalFile.getParentId()); mStorageManager.saveFile(serverFile); } @@ -143,11 +144,11 @@ public class SynchronizeFileOperation extends RemoteOperation { } - Log.i(TAG, "Synchronizing " + mAccount.name + ", file " + mRemotePath + ": " + result.getLogMessage()); + Log.i(TAG, "Synchronizing " + mAccount.name + ", file " + mLocalFile.getRemotePath() + ": " + result.getLogMessage()); } catch (Exception e) { result = new RemoteOperationResult(e); - Log.e(TAG, "Synchronizing " + mAccount.name + ", file " + mRemotePath + ": " + result.getLogMessage(), result.getException()); + Log.e(TAG, "Synchronizing " + mAccount.name + ", file " + mLocalFile.getRemotePath() + ": " + result.getLogMessage(), result.getException()); } finally { if (propfind != null) @@ -160,13 +161,14 @@ public class SynchronizeFileOperation extends RemoteOperation { /** * Requests for an upload to the FileUploader service * - * @param localFile OCFile object representing the file to upload + * @param file OCFile object representing the file to upload */ - private void requestForUpload(OCFile localFile) { + private void requestForUpload(OCFile file) { Intent i = new Intent(mContext, FileUploader.class); i.putExtra(FileUploader.KEY_ACCOUNT, mAccount); - i.putExtra(FileUploader.KEY_REMOTE_FILE, mRemotePath); - i.putExtra(FileUploader.KEY_LOCAL_FILE, localFile.getStoragePath()); + i.putExtra(FileUploader.KEY_FILE, file); + /*i.putExtra(FileUploader.KEY_REMOTE_FILE, mRemotePath); // doing this we would lose the value of keepInSync in the road, and maybe it's not updated in the database when the FileUploader service gets it! + i.putExtra(FileUploader.KEY_LOCAL_FILE, localFile.getStoragePath());*/ i.putExtra(FileUploader.KEY_UPLOAD_TYPE, FileUploader.UPLOAD_SINGLE_FILE); i.putExtra(FileUploader.KEY_FORCE_OVERWRITE, true); mContext.startService(i); diff --git a/src/com/owncloud/android/ui/fragment/FileDetailFragment.java b/src/com/owncloud/android/ui/fragment/FileDetailFragment.java index 01fc1866..53e0630d 100644 --- a/src/com/owncloud/android/ui/fragment/FileDetailFragment.java +++ b/src/com/owncloud/android/ui/fragment/FileDetailFragment.java @@ -292,7 +292,7 @@ public class FileDetailFragment extends SherlockFragment implements } } else { - mLastRemoteOperation = new SynchronizeFileOperation(mFile.getRemotePath(), fdsm, mAccount, true, false, getActivity()); + mLastRemoteOperation = new SynchronizeFileOperation(mFile, fdsm, mAccount, true, false, getActivity()); WebdavClient wc = OwnCloudClientUtils.createOwnCloudClient(mAccount, getSherlockActivity().getApplicationContext()); mLastRemoteOperation.execute(wc, this, mHandler); @@ -309,19 +309,6 @@ public class FileDetailFragment extends SherlockFragment implements mFile.setKeepInSync(cb.isChecked()); fdsm.saveFile(mFile); - /* NOT HERE - * now that FileObserverService is involved, the easiest way to coordinate it with the download service - * in every possible case is let the FileObserverService decide if the download should be started at - * this moment or not - * - * see changes at FileObserverService#addObservedFile - - if (mFile.keepInSync()) { - onClick(getView().findViewById(R.id.fdDownloadBtn)); - } else { - mContainerActivity.onFileStateChanged(); // put inside 'else' to not call it twice (here, and in the virtual click on fdDownloadBtn) - }*/ - /// register the OCFile instance in the observer service to monitor local updates; /// if necessary, the file is download Intent intent = new Intent(getActivity().getApplicationContext(), @@ -335,7 +322,9 @@ public class FileDetailFragment extends SherlockFragment implements Log.e(TAG, "starting observer service"); getActivity().startService(intent); - mContainerActivity.onFileStateChanged(); + if (mFile.keepInSync()) { + onClick(getView().findViewById(R.id.fdDownloadBtn)); // force an immediate synchronization + } break; } case R.id.fdRenameBtn: {