android: Return copies of cached blobs instead of originals.

This should fix
http://code.google.com/p/camlistore/issues/detail?id=3,
"Android client permits mutating the immutable cache".

I'm also removing the call to
URLConnection.guessContentTypeFromStream() in favor of
just using URLConnection.guessContentTypeFromName().
I don't think that the former was ever successful, and it
was hitting the disk from the UI thread.
This commit is contained in:
Daniel Erat 2011-03-27 14:19:48 -07:00
parent 58c678e94e
commit 64040340c3
3 changed files with 101 additions and 28 deletions

View File

@ -60,6 +60,13 @@ public class BrowseActivity extends ListActivity {
// TODO: Remove this; it's pretty ugly.
private HashMap<String, Entry> mEntriesByContentBlobRef = new HashMap<String, Entry>();
// Map from the request code of an activity that we started to the file that we passed to it.
// We keep track of this so that we can delete the file when the activity is done.
private HashMap<Integer, File> mOutstandingActivities = new HashMap<Integer, File>();
// Next request code to allocate for |mOutstandingActivities|.
private int mNextActivityRequestCode = 1;
private enum EntryType {
UNKNOWN("unknown"),
FILE("file"),
@ -189,6 +196,24 @@ public class BrowseActivity extends ListActivity {
unbindService(mConnection);
}
@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
final File file = mOutstandingActivities.get(requestCode);
if (file == null) {
Log.e(TAG, "got unknown activity result with request code " + requestCode);
return;
}
mOutstandingActivities.remove(requestCode);
Util.runAsync(new Runnable() {
@Override
public void run() {
Log.d(TAG, "deleting " + file.getPath());
file.delete();
}
});
}
@Override
protected void onListItemClick(ListView listView, View view, int position, long id) {
Entry entry = mEntries.get(position);
@ -371,18 +396,9 @@ public class BrowseActivity extends ListActivity {
return;
}
// Try to guess the MIME type from the data itself first.
// Try to guess the mime type from the filename's extension.
String mimeType = null;
try {
FileInputStream inputStream = new FileInputStream(file);
mimeType = URLConnection.guessContentTypeFromStream(inputStream);
inputStream.close();
} catch (IOException e) {
Log.e(TAG, "got IO error while trying to guess mime type for " + file.getPath(), e);
}
// If that didn't work, try to guess it from the filename.
if (mimeType == null && entry.getFilename() != null)
if (entry.getFilename() != null)
mimeType = URLConnection.guessContentTypeFromName(entry.getFilename());
if (mimeType == null)
mimeType = DEFAULT_MIME_TYPE;
@ -390,8 +406,11 @@ public class BrowseActivity extends ListActivity {
Intent intent = new Intent();
intent.setAction(intent.ACTION_VIEW);
intent.setDataAndType(Uri.fromFile(file), mimeType);
final int requestCode = mNextActivityRequestCode++;
if (mOutstandingActivities.put(requestCode, file) != null)
throw new RuntimeException("request code " + requestCode + " is already in use");
try {
startActivity(intent);
startActivityForResult(intent, requestCode);
} catch (android.content.ActivityNotFoundException e) {
Toast.makeText(BrowseActivity.this, "No activity found to display " + mimeType + ".", Toast.LENGTH_SHORT).show();
}

View File

@ -20,6 +20,7 @@ import android.app.Service;
import android.content.Intent;
import android.content.SharedPreferences;
import android.os.Binder;
import android.os.ConditionVariable;
import android.os.Handler;
import android.os.IBinder;
import android.util.Log;
@ -47,27 +48,38 @@ public class DownloadService extends Service {
private static final int BUFFER_SIZE = 4096;
private static final String USERNAME = "TODO-DUMMY-USER";
private static final String SEARCH_BLOBREF = "search";
// Name of directory on external storage where cached blobs are stored.
private static final String BLOB_SUBDIR = "blobs";
// Name of cache directory on external storage where we write copies of cached blobs
// to hand out to listeners. Cleared at startup.
private static final String BLOB_COPIES_SUBDIR = "blob_copies";
private final IBinder mBinder = new LocalBinder();
private final Handler mHandler = new Handler();
// Effectively-final objects initialized in onCreate().
private SharedPreferences mSharedPrefs;
private DownloadCache mCache;
// Directory on external storage where we make copies of blob files to hand to clients.
private File mBlobCopiesDir;
// Protects members containing the state of current downloads.
private final ReentrantLock mDownloadLock = new ReentrantLock();
private final ReentrantLock mLock = new ReentrantLock();
// Blobs currently being downloaded. Protected by |mDownloadLock|.
// Blobs currently being downloaded. Protected by |mLock|.
private final HashSet<String> mInProgressBlobRefs = new HashSet<String>();
// Maps from blobrefs to callbacks for their contents. Protected by |mDownloadLock|.
// Maps from blobrefs to callbacks for their contents. Protected by |mLock|.
private final HashMap<String, ArrayList<ByteArrayListener>> mByteArrayListenersByBlobRef =
new HashMap<String, ArrayList<ByteArrayListener>>();
private final HashMap<String, ArrayList<FileListener>> mFileListenersByBlobRef =
new HashMap<String, ArrayList<FileListener>>();
// Closed until |mBlobCopiesDir| has been created and cleared.
private final ConditionVariable mBlobCopiesDirIsReady = new ConditionVariable(false);
// Callback for receiving a blob's contents as an in-memory array of bytes.
interface ByteArrayListener {
void onBlobDownloadSuccess(String blobRef, byte[] bytes);
@ -76,6 +88,8 @@ public class DownloadService extends Service {
// Callback for receiving a blob's contents as a File.
interface FileListener {
// The listener is responsible for deleting |file| once it's done using it.
// Abandoned files are cleared when the service starts.
void onBlobDownloadSuccess(String blobRef, File file);
void onBlobDownloadFailure(String data);
}
@ -93,6 +107,20 @@ public class DownloadService extends Service {
mSharedPrefs = getSharedPreferences(Preferences.NAME, 0);
mCache = new DownloadCache(new File(getExternalFilesDir(null), BLOB_SUBDIR).getPath(),
new Preferences(mSharedPrefs));
// Create |mBlobCopiesDir| and delete any stale files left over in it from a previous run.
mBlobCopiesDir = new File(getExternalCacheDir(), BLOB_COPIES_SUBDIR);
Util.runAsync(new Runnable() {
@Override
public void run() {
mBlobCopiesDir.mkdirs();
for (File file : mBlobCopiesDir.listFiles()) {
Log.d(TAG, "deleting stale temp blob file " + file.getPath());
file.delete();
}
mBlobCopiesDirIsReady.open();
}
});
}
@Override
@ -131,7 +159,7 @@ public class DownloadService extends Service {
// Get a list of byte array listeners waiting for |blobRef|.
// If |insert| is true, the returned list can be used for adding new listeners.
private ArrayList<ByteArrayListener> getByteArrayListenersForBlobRef(String blobRef, boolean insert) {
Util.assertLockIsHeld(mDownloadLock);
Util.assertLockIsHeld(mLock);
ArrayList<ByteArrayListener> listeners = mByteArrayListenersByBlobRef.get(blobRef);
if (listeners == null) {
listeners = new ArrayList<ByteArrayListener>();
@ -144,7 +172,7 @@ public class DownloadService extends Service {
// Get a list of file listeners waiting for |blobRef|.
// If |insert| is true, the returned list can be used for adding new listeners.
private ArrayList<FileListener> getFileListenersForBlobRef(String blobRef, boolean insert) {
Util.assertLockIsHeld(mDownloadLock);
Util.assertLockIsHeld(mLock);
ArrayList<FileListener> listeners = mFileListenersByBlobRef.get(blobRef);
if (listeners == null) {
listeners = new ArrayList<FileListener>();
@ -176,7 +204,7 @@ public class DownloadService extends Service {
@Override
public void run() {
mDownloadLock.lock();
mLock.lock();
try {
if (mByteArrayListener != null) {
getByteArrayListenersForBlobRef(mBlobRef, true).add(mByteArrayListener);
@ -193,7 +221,7 @@ public class DownloadService extends Service {
return;
mInProgressBlobRefs.add(mBlobRef);
} finally {
mDownloadLock.unlock();
mLock.unlock();
}
mBlobFile = mCache.getFileForBlob(mBlobRef);
@ -249,9 +277,9 @@ public class DownloadService extends Service {
return;
}
mDownloadLock.lock();
mLock.lock();
final boolean shouldDownloadToByteArray = !getByteArrayListenersForBlobRef(mBlobRef, false).isEmpty();
mDownloadLock.unlock();
mLock.unlock();
if (canBlobBeCached(mBlobRef)) {
tempFile = mCache.getTempFileForDownload(mBlobRef, mSizeHintBytes);
@ -301,10 +329,10 @@ public class DownloadService extends Service {
// handle the file listeners and any byte array listeners that were added in the meantime; this is just
// an optimization so we don't block on disk if we only have byte array listeners.
if (shouldDownloadToByteArray) {
mDownloadLock.lock();
mLock.lock();
ArrayList<ByteArrayListener> byteArrayListeners = getByteArrayListenersForBlobRef(mBlobRef, false);
mByteArrayListenersByBlobRef.remove(mBlobRef);
mDownloadLock.unlock();
mLock.unlock();
mBlobBytes = ((ByteArrayOutputStream) outputStream).toByteArray();
sendBlobToByteArrayListeners(byteArrayListeners, mBlobBytes);
@ -362,16 +390,16 @@ public class DownloadService extends Service {
// Removes |mBlobRef| from |mInProgressBlobRefs| and removes listeners from
// |mByteArrayListenersByBlobRef| and |mFileListenersByBlobRef|.
private void notifyListeners() {
mDownloadLock.lock();
mLock.lock();
ArrayList<ByteArrayListener> byteArrayListeners = getByteArrayListenersForBlobRef(mBlobRef, false);
mByteArrayListenersByBlobRef.remove(mBlobRef);
ArrayList<FileListener> fileListeners = getFileListenersForBlobRef(mBlobRef, false);
mFileListenersByBlobRef.remove(mBlobRef);
mInProgressBlobRefs.remove(mBlobRef);
mDownloadLock.unlock();
mLock.unlock();
// Make sure that we're not holding the lock; we're about to hit the disk.
Util.assertLockIsNotHeld(mDownloadLock);
Util.assertLockIsNotHeld(mLock);
if (!byteArrayListeners.isEmpty()) {
// If we don't have the data in memory already but it's on disk, read it.
@ -387,11 +415,25 @@ public class DownloadService extends Service {
}
for (final FileListener listener : fileListeners) {
File listenerFile = null;
if (mBlobFile != null) {
mBlobCopiesDirIsReady.block();
// Make a copy of the file for each listener to ensure that they won't modify the cached blob.
try {
listenerFile = File.createTempFile(mBlobRef + "-", "", mBlobCopiesDir);
Log.d(TAG, "copying " + mBlobFile.getPath() + " to " + listenerFile.getPath());
Util.copyFile(mBlobFile, listenerFile);
} catch (IOException e) {
Log.e(TAG, "got IO error while making copy of blob " + mBlobRef, e);
}
}
final File finalListenerFile = listenerFile;
mHandler.post(new Runnable() {
@Override
public void run() {
if (mBlobFile != null) {
listener.onBlobDownloadSuccess(mBlobRef, mBlobFile);
if (finalListenerFile != null) {
listener.onBlobDownloadSuccess(mBlobRef, finalListenerFile);
} else {
listener.onBlobDownloadFailure(mBlobRef);
}

View File

@ -18,8 +18,10 @@ package org.camlistore;
import java.io.BufferedInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileDescriptor;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.security.MessageDigest;
@ -52,6 +54,16 @@ public class Util {
return outputStream.toByteArray();
}
public static void copyFile(File fromFile, File toFile) throws IOException {
FileInputStream inputStream = new FileInputStream(fromFile);
FileOutputStream outputStream = new FileOutputStream(toFile);
byte[] buffer = new byte[4096];
for (int numRead; (numRead = inputStream.read(buffer)) != -1;)
outputStream.write(buffer, 0, numRead);
inputStream.close();
outputStream.close();
}
public static void runAsync(final Runnable r) {
new AsyncTask<Void, Void, Void>() {
@Override