From f4e7fa80384ac72d0228ca5de6e949a9162cefbf Mon Sep 17 00:00:00 2001 From: Daichi Hirono Date: Mon, 28 Mar 2016 16:07:45 +0900 Subject: [PATCH] Use AppFuse to write document. Previously MtpDocumentsProvider used pipes to transfer bytes from an application to the provider when writing a document. The problem was application could not ensure that the last chunk of bytes was successfully written to MTP device, since pipes had been already closed when the provider transferred bytes to MTP device. Though the provider encountered an error, the provider could not report the error to an application. The CL switches the method to transfer bytes from pipes to AppFuse. Now application can flush() bytes on the file descriptor, and flush will not complete until the provider completes writing bytes to MTP device. Fixed: 23093747 Change-Id: I4e28f8cbf19d6c97e591943349a7535241d768f7 --- .../android/documentsui/services/CopyJob.java | 6 +- .../jni/com_android_mtp_AppFuse.cpp | 58 ++++++--- .../src/com/android/mtp/AppFuse.java | 87 +++++++++++-- .../com/android/mtp/MtpDocumentsProvider.java | 123 ++++++++++++------ .../src/com/android/mtp/MtpFileWriter.java | 108 +++++++++++++++ .../src/com/android/mtp/PipeManager.java | 96 -------------- .../src/com/android/mtp/AppFuseTest.java | 57 +++++++- .../android/mtp/MtpDocumentsProviderTest.java | 26 ++++ .../src/com/android/mtp/PipeManagerTest.java | 61 --------- 9 files changed, 391 insertions(+), 231 deletions(-) create mode 100644 packages/MtpDocumentsProvider/src/com/android/mtp/MtpFileWriter.java 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(