diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java b/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java index 9ed2abf9c9b8e..f10af4378c219 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java +++ b/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java @@ -45,6 +45,8 @@ import android.os.ParcelFileDescriptor; import android.os.RemoteException; import android.provider.DocumentsContract; import android.provider.DocumentsContract.Document; +import android.system.ErrnoException; +import android.system.Os; import android.text.format.DateUtils; import android.util.Log; import android.webkit.MimeTypeMap; @@ -451,7 +453,7 @@ class CopyJob extends Job { ParcelFileDescriptor srcFile = null; ParcelFileDescriptor dstFile = null; InputStream in = null; - OutputStream out = null; + ParcelFileDescriptor.AutoCloseOutputStream out = null; boolean success = false; try { @@ -502,6 +504,8 @@ class CopyJob extends Job { makeCopyProgress(len); } + // Need to invoke IoUtils.close explicitly to avoid from ignoring errors at flush. + IoUtils.close(dstFile.getFileDescriptor()); srcFile.checkError(); } catch (IOException e) { throw new ResourceException( diff --git a/packages/MtpDocumentsProvider/jni/com_android_mtp_AppFuse.cpp b/packages/MtpDocumentsProvider/jni/com_android_mtp_AppFuse.cpp index 1d4ed1dc5c5b7..eb96015379968 100644 --- a/packages/MtpDocumentsProvider/jni/com_android_mtp_AppFuse.cpp +++ b/packages/MtpDocumentsProvider/jni/com_android_mtp_AppFuse.cpp @@ -52,6 +52,8 @@ static jclass app_fuse_class; static jmethodID app_fuse_get_file_size; static jmethodID app_fuse_read_object_bytes; static jmethodID app_fuse_write_object_bytes; +static jmethodID app_fuse_flush_file_handle; +static jmethodID app_fuse_close_file_handle; static jfieldID app_fuse_buffer; // NOTE: @@ -307,7 +309,8 @@ private: const uint32_t size = in->size; const void* const buffer = reinterpret_cast(in) + sizeof(fuse_write_in); uint32_t written_size; - const int result = write_object_bytes(it->second, offset, size, buffer, &written_size); + const int result = write_object_bytes( + in->fh, it->second, offset, size, buffer, &written_size); if (result < 0) { return result; } @@ -320,13 +323,13 @@ private: const fuse_release_in* in, FuseResponse* /* out */) { handles_.erase(in->fh); - return 0; + return env_->CallIntMethod(self_, app_fuse_close_file_handle, file_handle_to_jlong(in->fh)); } int handle_fuse_flush(const fuse_in_header& /* header */, - const void* /* in */, + const fuse_flush_in* in, FuseResponse* /* out */) { - return 0; + return env_->CallIntMethod(self_, app_fuse_flush_file_handle, file_handle_to_jlong(in->fh)); } template @@ -382,8 +385,10 @@ private: return read_size; } - int write_object_bytes(int inode, uint64_t offset, uint32_t size, const void* buffer, - uint32_t* written_size) { + int write_object_bytes(uint64_t handle, int inode, uint64_t offset, uint32_t size, + const void* buffer, uint32_t* written_size) { + static_assert(sizeof(uint64_t) <= sizeof(jlong), + "jlong must be able to express any uint64_t values"); ScopedLocalRef array( env_, static_cast(env_->GetObjectField(self_, app_fuse_buffer))); @@ -394,15 +399,28 @@ private: } memcpy(bytes.get(), buffer, size); } - *written_size = env_->CallIntMethod( - self_, app_fuse_write_object_bytes, inode, offset, size, array.get()); - if (env_->ExceptionCheck()) { - env_->ExceptionClear(); - return -EIO; + const int result = env_->CallIntMethod( + self_, + app_fuse_write_object_bytes, + file_handle_to_jlong(handle), + inode, + offset, + size, + array.get()); + if (result < 0) { + return result; } + *written_size = result; return 0; } + static jlong file_handle_to_jlong(uint64_t handle) { + static_assert( + sizeof(uint64_t) <= sizeof(jlong), + "jlong must be able to express any uint64_t values"); + return static_cast(handle); + } + static void fuse_reply(int fd, int unique, int reply_code, void* reply_data, size_t reply_size) { // Don't send any data for error case. @@ -511,15 +529,21 @@ jint JNI_OnLoad(JavaVM* vm, void* /* reserved */) { return -1; } - app_fuse_buffer = env->GetFieldID(app_fuse_class, "mBuffer", "[B"); - if (app_fuse_buffer == nullptr) { - ALOGE("Can't find mBuffer"); + app_fuse_write_object_bytes = env->GetMethodID(app_fuse_class, "writeObjectBytes", "(JIJI[B)I"); + if (app_fuse_write_object_bytes == nullptr) { + ALOGE("Can't find writeObjectBytes"); return -1; } - app_fuse_write_object_bytes = env->GetMethodID(app_fuse_class, "writeObjectBytes", "(IJI[B)I"); - if (app_fuse_write_object_bytes == nullptr) { - ALOGE("Can't find getWriteObjectBytes"); + app_fuse_flush_file_handle = env->GetMethodID(app_fuse_class, "flushFileHandle", "(J)I"); + if (app_fuse_flush_file_handle == nullptr) { + ALOGE("Can't find flushFileHandle"); + return -1; + } + + app_fuse_close_file_handle = env->GetMethodID(app_fuse_class, "closeFileHandle", "(J)I"); + if (app_fuse_close_file_handle == nullptr) { + ALOGE("Can't find closeFileHandle"); return -1; } diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/AppFuse.java b/packages/MtpDocumentsProvider/src/com/android/mtp/AppFuse.java index 777dc606f6c30..88858a8b53223 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/AppFuse.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/AppFuse.java @@ -20,6 +20,7 @@ import android.annotation.WorkerThread; import android.os.ParcelFileDescriptor; import android.os.Process; import android.os.storage.StorageManager; +import android.system.ErrnoException; import android.system.OsConstants; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; @@ -34,6 +35,8 @@ public class AppFuse { System.loadLibrary("appfuse_jni"); } + private static final boolean DEBUG = false; + /** * Max read amount specified at the FUSE kernel implementation. * The value is copied from sdcard.c. @@ -94,7 +97,8 @@ public class AppFuse { public ParcelFileDescriptor openFile(int i, int mode) throws FileNotFoundException { Preconditions.checkArgument( mode == ParcelFileDescriptor.MODE_READ_ONLY || - mode == ParcelFileDescriptor.MODE_WRITE_ONLY); + mode == (ParcelFileDescriptor.MODE_WRITE_ONLY | + ParcelFileDescriptor.MODE_TRUNCATE)); return ParcelFileDescriptor.open(new File( getMountPoint(), Integer.toString(i)), @@ -127,6 +131,7 @@ public class AppFuse { /** * Handles writing bytes for the give inode. + * @param fileHandle * @param inode * @param offset Offset for file bytes. * @param size Size for file bytes. @@ -134,7 +139,23 @@ public class AppFuse { * @return Number of read bytes. Must not be negative. * @throws IOException */ - int writeObjectBytes(int inode, long offset, int size, byte[] bytes) throws IOException; + int writeObjectBytes(long fileHandle, int inode, long offset, int size, byte[] bytes) + throws IOException, ErrnoException; + + /** + * Flushes bytes for file handle. + * @param fileHandle + * @throws IOException + * @throws ErrnoException + */ + void flushFileHandle(long fileHandle) throws IOException, ErrnoException; + + /** + * Closes file handle. + * @param fileHandle + * @throws IOException + */ + void closeFileHandle(long fileHandle) throws IOException, ErrnoException; } @UsedByNative("com_android_mtp_AppFuse.cpp") @@ -142,10 +163,8 @@ public class AppFuse { private long getFileSize(int inode) { try { return mCallback.getFileSize(inode); - } catch (FileNotFoundException e) { - return -OsConstants.ENOENT; - } catch (UnsupportedOperationException e) { - return -OsConstants.ENOTSUP; + } catch (Exception error) { + return -getErrnoFromException(error); } } @@ -159,20 +178,62 @@ public class AppFuse { // It's OK to share the same mBuffer among requests because the requests are processed // by AppFuseMessageThread sequentially. return mCallback.readObjectBytes(inode, offset, size, mBuffer); - } catch (IOException e) { - return -OsConstants.EIO; - } catch (UnsupportedOperationException e) { - return -OsConstants.ENOTSUP; + } catch (Exception error) { + return -getErrnoFromException(error); } } @UsedByNative("com_android_mtp_AppFuse.cpp") @WorkerThread - private /* unsgined */ int writeObjectBytes(int inode, + private /* unsgined */ int writeObjectBytes(long fileHandler, + int inode, /* unsigned */ long offset, /* unsigned */ int size, - byte[] bytes) throws IOException { - return mCallback.writeObjectBytes(inode, offset, size, bytes); + byte[] bytes) { + try { + return mCallback.writeObjectBytes(fileHandler, inode, offset, size, bytes); + } catch (Exception error) { + return -getErrnoFromException(error); + } + } + + @UsedByNative("com_android_mtp_AppFuse.cpp") + @WorkerThread + private int flushFileHandle(long fileHandle) { + try { + mCallback.flushFileHandle(fileHandle); + return 0; + } catch (Exception error) { + return -getErrnoFromException(error); + } + } + + @UsedByNative("com_android_mtp_AppFuse.cpp") + @WorkerThread + private int closeFileHandle(long fileHandle) { + try { + mCallback.closeFileHandle(fileHandle); + return 0; + } catch (Exception error) { + return -getErrnoFromException(error); + } + } + + private static int getErrnoFromException(Exception error) { + if (DEBUG) { + Log.e(MtpDocumentsProvider.TAG, "AppFuse callbacks", error); + } + if (error instanceof FileNotFoundException) { + return OsConstants.ENOENT; + } else if (error instanceof IOException) { + return OsConstants.EIO; + } else if (error instanceof UnsupportedOperationException) { + return OsConstants.ENOTSUP; + } else if (error instanceof IllegalArgumentException) { + return OsConstants.EINVAL; + } else { + return OsConstants.EIO; + } } private native boolean native_start_app_fuse_loop(int fd); diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java index 9f64046ce64af..50781bf2c9ca1 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java @@ -17,6 +17,7 @@ package com.android.mtp; import android.content.ContentResolver; +import android.content.Context; import android.content.UriPermission; import android.content.res.AssetFileDescriptor; import android.content.res.Resources; @@ -38,11 +39,16 @@ import android.provider.DocumentsContract.Root; import android.provider.DocumentsContract; import android.provider.DocumentsProvider; import android.provider.Settings; +import android.system.ErrnoException; +import android.system.Os; +import android.system.OsConstants; import android.util.Log; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; +import java.io.File; +import java.io.FileDescriptor; import java.io.FileNotFoundException; import java.io.IOException; import java.util.HashMap; @@ -82,6 +88,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { private MtpDatabase mDatabase; private AppFuse mAppFuse; private ServiceIntentSender mIntentSender; + private Context mContext; /** * Provides singleton instance to MtpDocumentsService. @@ -93,6 +100,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { @Override public boolean onCreate() { sSingleton = this; + mContext = getContext(); mResources = getContext().getResources(); mMtpManager = new MtpManager(getContext()); mResolver = getContext().getContentResolver(); @@ -137,12 +145,14 @@ public class MtpDocumentsProvider extends DocumentsProvider { @VisibleForTesting boolean onCreateForTesting( + Context context, Resources resources, MtpManager mtpManager, ContentResolver resolver, MtpDatabase database, StorageManager storageManager, ServiceIntentSender intentSender) { + mContext = context; mResources = resources; mMtpManager = mtpManager; mResolver = resolver; @@ -232,43 +242,43 @@ public class MtpDocumentsProvider extends DocumentsProvider { try { openDevice(identifier.mDeviceId); final MtpDeviceRecord device = getDeviceToolkit(identifier.mDeviceId).mDeviceRecord; - switch (mode) { - case "r": - long fileSize; - try { - fileSize = getFileSize(documentId); - } catch (UnsupportedOperationException exception) { - fileSize = -1; - } - // MTP getPartialObject operation does not support files that are larger than - // 4GB. Fallback to non-seekable file descriptor. - if (MtpDeviceRecord.isPartialReadSupported( - device.operationsSupported, fileSize)) { - return mAppFuse.openFile( - Integer.parseInt(documentId), ParcelFileDescriptor.MODE_READ_ONLY); - } else { - return getPipeManager(identifier).readDocument(mMtpManager, identifier); - } - case "w": - // TODO: Clear the parent document loader task (if exists) and call notify - // when writing is completed. - if (MtpDeviceRecord.isWritingSupported(device.operationsSupported)) { - return getPipeManager(identifier).writeDocument( - getContext(), mMtpManager, identifier, device.operationsSupported); - } else { - throw new UnsupportedOperationException( - "The device does not support writing operation."); - } - case "rw": - // TODO: Add support for "rw" mode. + // Turn off MODE_CREATE because openDocument does not allow to create new files. + final int modeFlag = + ParcelFileDescriptor.parseMode(mode) & ~ParcelFileDescriptor.MODE_CREATE; + if ((modeFlag & ParcelFileDescriptor.MODE_READ_ONLY) != 0) { + long fileSize; + try { + fileSize = getFileSize(documentId); + } catch (UnsupportedOperationException exception) { + fileSize = -1; + } + if (MtpDeviceRecord.isPartialReadSupported( + device.operationsSupported, fileSize)) { + return mAppFuse.openFile(Integer.parseInt(documentId), modeFlag); + } else { + // If getPartialObject{|64} are not supported for the device, returns + // non-seekable pipe FD instead. + return getPipeManager(identifier).readDocument(mMtpManager, identifier); + } + } else if ((modeFlag & ParcelFileDescriptor.MODE_WRITE_ONLY) != 0) { + // TODO: Clear the parent document loader task (if exists) and call notify + // when writing is completed. + if (MtpDeviceRecord.isWritingSupported(device.operationsSupported)) { + return mAppFuse.openFile(Integer.parseInt(documentId), modeFlag); + } else { throw new UnsupportedOperationException( - "The provider does not support 'rw' mode."); - default: - throw new IllegalArgumentException("Unknown mode for openDocument: " + mode); + "The device does not support writing operation."); + } + } else { + // TODO: Add support for "rw" mode. + throw new UnsupportedOperationException("The provider does not support 'rw' mode."); } + } catch (FileNotFoundException | RuntimeException error) { + Log.e(MtpDocumentsProvider.TAG, "openDocument", error); + throw error; } catch (IOException error) { Log.e(MtpDocumentsProvider.TAG, "openDocument", error); - throw new FileNotFoundException(error.getMessage()); + throw new IllegalStateException(error); } } @@ -595,6 +605,13 @@ public class MtpDocumentsProvider extends DocumentsProvider { } private class AppFuseCallback implements AppFuse.Callback { + private final Map mWriters = new HashMap<>(); + + @Override + public long getFileSize(int inode) throws FileNotFoundException { + return MtpDocumentsProvider.this.getFileSize(String.valueOf(inode)); + } + @Override public long readObjectBytes( int inode, long offset, long size, byte[] buffer) throws IOException { @@ -617,15 +634,43 @@ public class MtpDocumentsProvider extends DocumentsProvider { } @Override - public long getFileSize(int inode) throws FileNotFoundException { - return MtpDocumentsProvider.this.getFileSize(String.valueOf(inode)); + public int writeObjectBytes( + long fileHandle, int inode, long offset, int size, byte[] bytes) + throws IOException, ErrnoException { + final MtpFileWriter writer; + if (mWriters.containsKey(fileHandle)) { + writer = mWriters.get(fileHandle); + } else { + writer = new MtpFileWriter(mContext, String.valueOf(inode)); + mWriters.put(fileHandle, writer); + } + return writer.write(offset, size, bytes); } @Override - public int writeObjectBytes(int inode, long offset, int size, byte[] bytes) - throws IOException { - // TODO: Implement it. - throw new IOException(); + public void flushFileHandle(long fileHandle) throws IOException, ErrnoException { + final MtpFileWriter writer = mWriters.get(fileHandle); + if (writer == null) { + // File handle for reading. + return; + } + final MtpDeviceRecord device = getDeviceToolkit( + mDatabase.createIdentifier(writer.getDocumentId()).mDeviceId).mDeviceRecord; + writer.flush(mMtpManager, mDatabase, device.operationsSupported); + } + + @Override + public void closeFileHandle(long fileHandle) throws IOException, ErrnoException { + final MtpFileWriter writer = mWriters.get(fileHandle); + if (writer == null) { + // File handle for reading. + return; + } + try { + writer.close(); + } finally { + mWriters.remove(fileHandle); + } } } } diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpFileWriter.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpFileWriter.java new file mode 100644 index 0000000000000..3e1bedce49ea8 --- /dev/null +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpFileWriter.java @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.mtp; + +import android.content.Context; +import android.mtp.MtpObjectInfo; +import android.os.ParcelFileDescriptor; +import android.system.ErrnoException; +import android.system.Os; +import android.system.OsConstants; + +import com.android.internal.util.Preconditions; + +import java.io.File; +import java.io.IOException; + +class MtpFileWriter implements AutoCloseable { + final ParcelFileDescriptor mCacheFd; + final String mDocumentId; + boolean mDirty; + + MtpFileWriter(Context context, String documentId) throws IOException { + mDocumentId = documentId; + mDirty = false; + final File tempFile = File.createTempFile("mtp", "tmp", context.getCacheDir()); + mCacheFd = ParcelFileDescriptor.open( + tempFile, + ParcelFileDescriptor.MODE_READ_WRITE | + ParcelFileDescriptor.MODE_TRUNCATE | + ParcelFileDescriptor.MODE_CREATE); + tempFile.delete(); + } + + String getDocumentId() { + return mDocumentId; + } + + int write(long offset, int size, byte[] bytes) throws IOException, ErrnoException { + Preconditions.checkArgumentNonnegative(offset, "offset"); + Preconditions.checkArgumentNonnegative(size, "size"); + Preconditions.checkArgument(size <= bytes.length); + if (size == 0) { + return 0; + } + mDirty = true; + Os.lseek(mCacheFd.getFileDescriptor(), offset, OsConstants.SEEK_SET); + return Os.write(mCacheFd.getFileDescriptor(), bytes, 0, size); + } + + void flush(MtpManager manager, MtpDatabase database, int[] operationsSupported) + throws IOException, ErrnoException { + // Skip unnecessary flush. + if (!mDirty) { + return; + } + + // Get the placeholder object info. + final Identifier identifier = database.createIdentifier(mDocumentId); + final MtpObjectInfo placeholderObjectInfo = + manager.getObjectInfo(identifier.mDeviceId, identifier.mObjectHandle); + + // Delete the target object info if it already exists (as a placeholder). + manager.deleteDocument(identifier.mDeviceId, identifier.mObjectHandle); + + // Create the target object info with a correct file size and upload the file. + final long size = Os.lseek(mCacheFd.getFileDescriptor(), 0, OsConstants.SEEK_END); + final MtpObjectInfo targetObjectInfo = new MtpObjectInfo.Builder(placeholderObjectInfo) + .setCompressedSize(size) + .build(); + + Os.lseek(mCacheFd.getFileDescriptor(), 0, OsConstants.SEEK_SET); + final int newObjectHandle = manager.createDocument( + identifier.mDeviceId, targetObjectInfo, mCacheFd); + + final MtpObjectInfo newObjectInfo = manager.getObjectInfo( + identifier.mDeviceId, newObjectHandle); + final Identifier parentIdentifier = + database.getParentIdentifier(identifier.mDocumentId); + database.updateObject( + identifier.mDocumentId, + identifier.mDeviceId, + parentIdentifier.mDocumentId, + operationsSupported, + newObjectInfo, + size); + + mDirty = false; + } + + @Override + public void close() throws IOException { + mCacheFd.close(); + } +} diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/PipeManager.java b/packages/MtpDocumentsProvider/src/com/android/mtp/PipeManager.java index 1520f3b83c2e7..795bbc1903363 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/PipeManager.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/PipeManager.java @@ -16,13 +16,9 @@ package com.android.mtp; -import android.content.Context; -import android.mtp.MtpObjectInfo; import android.os.ParcelFileDescriptor; import android.util.Log; -import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -52,15 +48,6 @@ class PipeManager { return task.getReadingFileDescriptor(); } - ParcelFileDescriptor writeDocument(Context context, MtpManager model, Identifier identifier, - int[] operationsSupported) - throws IOException { - final Task task = new WriteDocumentTask( - context, model, identifier, operationsSupported, mDatabase); - mExecutor.execute(task); - return task.getWritingFileDescriptor(); - } - ParcelFileDescriptor readThumbnail(MtpManager model, Identifier identifier) throws IOException { final Task task = new GetThumbnailTask(model, identifier); mExecutor.execute(task); @@ -81,10 +68,6 @@ class PipeManager { ParcelFileDescriptor getReadingFileDescriptor() { return mDescriptors[0]; } - - ParcelFileDescriptor getWritingFileDescriptor() { - return mDescriptors[1]; - } } private static class ImportFileTask extends Task { @@ -108,85 +91,6 @@ class PipeManager { } } - private static class WriteDocumentTask extends Task { - private final Context mContext; - private final MtpDatabase mDatabase; - private final int[] mOperationsSupported; - - WriteDocumentTask(Context context, - MtpManager model, - Identifier identifier, - int[] supportedOperations, - MtpDatabase database) - throws IOException { - super(model, identifier); - mContext = context; - mDatabase = database; - mOperationsSupported = supportedOperations; - } - - @Override - public void run() { - File tempFile = null; - try { - // Obtain a temporary file and copy the data to it. - tempFile = File.createTempFile("mtp", "tmp", mContext.getCacheDir()); - try ( - final FileOutputStream tempOutputStream = - new ParcelFileDescriptor.AutoCloseOutputStream( - ParcelFileDescriptor.open( - tempFile, ParcelFileDescriptor.MODE_WRITE_ONLY)); - final ParcelFileDescriptor.AutoCloseInputStream inputStream = - new ParcelFileDescriptor.AutoCloseInputStream(mDescriptors[0]) - ) { - final byte[] buffer = new byte[32 * 1024]; - int bytes; - while ((bytes = inputStream.read(buffer)) != -1) { - mDescriptors[0].checkError(); - tempOutputStream.write(buffer, 0, bytes); - } - tempOutputStream.flush(); - } - - // Get the placeholder object info. - final MtpObjectInfo placeholderObjectInfo = - mManager.getObjectInfo(mIdentifier.mDeviceId, mIdentifier.mObjectHandle); - - // Delete the target object info if it already exists (as a placeholder). - mManager.deleteDocument(mIdentifier.mDeviceId, mIdentifier.mObjectHandle); - - // Create the target object info with a correct file size and upload the file. - final MtpObjectInfo targetObjectInfo = - new MtpObjectInfo.Builder(placeholderObjectInfo) - .setCompressedSize(tempFile.length()) - .build(); - final ParcelFileDescriptor tempInputDescriptor = ParcelFileDescriptor.open( - tempFile, ParcelFileDescriptor.MODE_READ_ONLY); - final int newObjectHandle = mManager.createDocument( - mIdentifier.mDeviceId, targetObjectInfo, tempInputDescriptor); - - final MtpObjectInfo newObjectInfo = mManager.getObjectInfo( - mIdentifier.mDeviceId, newObjectHandle); - final Identifier parentIdentifier = - mDatabase.getParentIdentifier(mIdentifier.mDocumentId); - mDatabase.updateObject( - mIdentifier.mDocumentId, - mIdentifier.mDeviceId, - parentIdentifier.mDocumentId, - mOperationsSupported, - newObjectInfo, - tempFile.length()); - } catch (IOException error) { - Log.w(MtpDocumentsProvider.TAG, - "Failed to send a file because of: " + error.getMessage()); - } finally { - if (tempFile != null) { - tempFile.delete(); - } - } - } - } - private static class GetThumbnailTask extends Task { GetThumbnailTask(MtpManager model, Identifier identifier) throws IOException { super(model, identifier); diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/AppFuseTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/AppFuseTest.java index 3b925068c50c8..e421de7113de7 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/AppFuseTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/AppFuseTest.java @@ -23,6 +23,8 @@ import android.system.Os; import android.test.AndroidTestCase; import android.test.suitebuilder.annotation.MediumTest; +import libcore.io.IoUtils; + import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -143,7 +145,8 @@ public class AppFuseTest extends AndroidTestCase { } @Override - public int writeObjectBytes(int inode, long offset, int size, byte[] bytes) { + public int writeObjectBytes( + long fileHandle, int inode, long offset, int size, byte[] bytes) { for (int i = 0; i < size; i++) { resultBytes[(int)(offset + i)] = bytes[i]; } @@ -152,7 +155,7 @@ public class AppFuseTest extends AndroidTestCase { }); appFuse.mount(storageManager); final ParcelFileDescriptor fd = appFuse.openFile( - INODE, ParcelFileDescriptor.MODE_WRITE_ONLY); + INODE, ParcelFileDescriptor.MODE_WRITE_ONLY | ParcelFileDescriptor.MODE_TRUNCATE); try (final ParcelFileDescriptor.AutoCloseOutputStream stream = new ParcelFileDescriptor.AutoCloseOutputStream(fd)) { stream.write('a'); @@ -182,7 +185,7 @@ public class AppFuseTest extends AndroidTestCase { }); appFuse.mount(storageManager); final ParcelFileDescriptor fd = appFuse.openFile( - INODE, ParcelFileDescriptor.MODE_WRITE_ONLY); + INODE, ParcelFileDescriptor.MODE_WRITE_ONLY | ParcelFileDescriptor.MODE_TRUNCATE); try (final ParcelFileDescriptor.AutoCloseOutputStream stream = new ParcelFileDescriptor.AutoCloseOutputStream(fd)) { stream.write('a'); @@ -192,6 +195,46 @@ public class AppFuseTest extends AndroidTestCase { appFuse.close(); } + public void testWriteFile_flushError() throws IOException { + final StorageManager storageManager = getContext().getSystemService(StorageManager.class); + final int INODE = 10; + final AppFuse appFuse = new AppFuse( + "test", + new TestCallback() { + @Override + public long getFileSize(int inode) throws FileNotFoundException { + if (inode != INODE) { + throw new FileNotFoundException(); + } + return 5; + } + + @Override + public int writeObjectBytes( + long fileHandle, int inode, long offset, int size, byte[] bytes) { + return size; + } + + @Override + public void flushFileHandle(long fileHandle) throws IOException { + throw new IOException(); + } + }); + appFuse.mount(storageManager); + final ParcelFileDescriptor fd = appFuse.openFile( + INODE, ParcelFileDescriptor.MODE_WRITE_ONLY | ParcelFileDescriptor.MODE_TRUNCATE); + try (final ParcelFileDescriptor.AutoCloseOutputStream stream = + new ParcelFileDescriptor.AutoCloseOutputStream(fd)) { + stream.write('a'); + try { + IoUtils.close(fd.getFileDescriptor()); + fail(); + } catch (IOException e) { + } + } + appFuse.close(); + } + private static class TestCallback implements AppFuse.Callback { @Override public long getFileSize(int inode) throws FileNotFoundException { @@ -205,9 +248,15 @@ public class AppFuseTest extends AndroidTestCase { } @Override - public int writeObjectBytes(int inode, long offset, int size, byte[] bytes) + public int writeObjectBytes(long fileHandle, int inode, long offset, int size, byte[] bytes) throws IOException { throw new IOException(); } + + @Override + public void flushFileHandle(long fileHandle) throws IOException {} + + @Override + public void closeFileHandle(long fileHandle) {} } } diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java index 0de761cb57c43..9ed15c82f3364 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java @@ -21,6 +21,7 @@ import android.mtp.MtpConstants; import android.mtp.MtpObjectInfo; import android.net.Uri; import android.os.ParcelFileDescriptor; +import android.os.ParcelFileDescriptor.AutoCloseOutputStream; import android.os.storage.StorageManager; import android.provider.DocumentsContract.Document; import android.provider.DocumentsContract.Root; @@ -533,6 +534,30 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } } + public void testOpenDocument_writing() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); + setupRoots(0, new MtpRoot[] { + new MtpRoot(0, 0, "Storage", 0, 0, "") + }); + final String documentId = mProvider.createDocument("2", "text/plain", "test.txt"); + { + final ParcelFileDescriptor fd = mProvider.openDocument(documentId, "w", null); + try (ParcelFileDescriptor.AutoCloseOutputStream stream = + new ParcelFileDescriptor.AutoCloseOutputStream(fd)) { + stream.write("Hello".getBytes()); + } + } + { + final ParcelFileDescriptor fd = mProvider.openDocument(documentId, "r", null); + try (ParcelFileDescriptor.AutoCloseInputStream stream = + new ParcelFileDescriptor.AutoCloseInputStream(fd)) { + final byte[] bytes = new byte[5]; + stream.read(bytes); + assertTrue(Arrays.equals("Hello".getBytes(), bytes)); + } + } + } + public void testBusyDevice() throws Exception { mMtpManager = new TestMtpManager(getContext()) { @Override @@ -740,6 +765,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { mProvider = new MtpDocumentsProvider(); final StorageManager storageManager = getContext().getSystemService(StorageManager.class); assertTrue(mProvider.onCreateForTesting( + getContext(), mResources, mMtpManager, mResolver, diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java index 86117971de923..53dc3dbcf1ea2 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java @@ -16,10 +16,7 @@ package com.android.mtp; -import android.database.Cursor; -import android.mtp.MtpObjectInfo; import android.os.ParcelFileDescriptor; -import android.provider.DocumentsContract.Document; import android.test.AndroidTestCase; import android.test.suitebuilder.annotation.MediumTest; @@ -66,64 +63,6 @@ public class PipeManagerTest extends AndroidTestCase { assertDescriptorError(descriptor); } - public void testWriteDocument_basic() throws Exception { - TestUtil.addTestDevice(mDatabase); - TestUtil.addTestStorage(mDatabase, "1"); - - final MtpObjectInfo info = - new MtpObjectInfo.Builder().setObjectHandle(1).setName("note.txt").build(); - mDatabase.getMapper().startAddingDocuments("2"); - mDatabase.getMapper().putChildDocuments( - 0, "2", TestUtil.OPERATIONS_SUPPORTED, - new MtpObjectInfo[] { info }, - new long[] { 0L }); - mDatabase.getMapper().stopAddingDocuments("2"); - // Create a placeholder file which should be replaced by a real file later. - mtpManager.setObjectInfo(0, info); - - // Upload testing bytes. - final ParcelFileDescriptor descriptor = mPipeManager.writeDocument( - getContext(), - mtpManager, - new Identifier(0, 0, 1, "2", MtpDatabaseConstants.DOCUMENT_TYPE_OBJECT), - TestUtil.OPERATIONS_SUPPORTED); - final ParcelFileDescriptor.AutoCloseOutputStream outputStream = - new ParcelFileDescriptor.AutoCloseOutputStream(descriptor); - outputStream.write(HELLO_BYTES, 0, HELLO_BYTES.length); - outputStream.close(); - mExecutor.shutdown(); - assertTrue(mExecutor.awaitTermination(1000, TimeUnit.MILLISECONDS)); - - // Check if the placeholder file is removed. - try { - mtpManager.getObjectInfo(0, 1); - fail(); // The placeholder file has not been deleted. - } catch (IOException e) { - // Expected error, as the file is gone. - } - - // Confirm that the target file is created. - final MtpObjectInfo targetDocument = mtpManager.getObjectInfo( - 0, TestMtpManager.CREATED_DOCUMENT_HANDLE); - assertTrue(targetDocument != null); - - // Confirm the object handle is updated. - try (final Cursor cursor = mDatabase.queryDocument( - "2", new String[] { MtpDatabaseConstants.COLUMN_OBJECT_HANDLE })) { - assertEquals(1, cursor.getCount()); - cursor.moveToNext(); - assertEquals(TestMtpManager.CREATED_DOCUMENT_HANDLE, cursor.getInt(0)); - } - - // Verify uploaded bytes. - final byte[] uploadedBytes = mtpManager.getImportFileBytes( - 0, TestMtpManager.CREATED_DOCUMENT_HANDLE); - assertEquals(HELLO_BYTES.length, uploadedBytes.length); - for (int i = 0; i < HELLO_BYTES.length; i++) { - assertEquals(HELLO_BYTES[i], uploadedBytes[i]); - } - } - public void testReadThumbnail_basic() throws Exception { mtpManager.setThumbnail(0, 1, HELLO_BYTES); final ParcelFileDescriptor descriptor = mPipeManager.readThumbnail(