Simplify vCard implementation.
- make ImportProcessor/ExportProcessor implement both Runnable and
Future (as FutureRunnable, introduced in 1.6). Getter capability
for Future interface is disabled.
- use just one hash map instead of managing three different ones.
Now that both processors are also Future<Object>, it is much
easier to do so.
Change-Id: Ia9deba6f4063eaae48d42a295d2ce84aca0a424f
diff --git a/src/com/android/contacts/vcard/ExportProcessor.java b/src/com/android/contacts/vcard/ExportProcessor.java
index 5e0a19d..c54df49 100644
--- a/src/com/android/contacts/vcard/ExportProcessor.java
+++ b/src/com/android/contacts/vcard/ExportProcessor.java
@@ -39,18 +39,17 @@
* Class for processing one export request from a user. Dropped after exporting requested Uri(s).
* {@link VCardService} will create another object when there is another export request.
*/
-public class ExportProcessor implements Runnable {
+public class ExportProcessor extends ProcessorBase {
private static final String LOG_TAG = "VCardExport";
private final VCardService mService;
+ private final ContentResolver mResolver;
+ private final NotificationManager mNotificationManager;
+ private final ExportRequest mExportRequest;
+ private final int mJobId;
- private ContentResolver mResolver;
- private NotificationManager mNotificationManager;
-
- private volatile boolean mCanceled;
-
- private ExportRequest mExportRequest;
- private int mJobId;
+ private volatile boolean mCancelled;
+ private volatile boolean mDone;
public ExportProcessor(VCardService service, ExportRequest exportRequest, int jobId) {
mService = service;
@@ -62,6 +61,11 @@
}
@Override
+ public final int getType() {
+ return PROCESSOR_TYPE_EXPORT;
+ }
+
+ @Override
public void run() {
// ExecutorService ignores RuntimeException, so we need to show it here.
try {
@@ -69,6 +73,10 @@
} catch (RuntimeException e) {
Log.e(LOG_TAG, "RuntimeException thrown during export", e);
throw e;
+ } finally {
+ synchronized (this) {
+ mDone = true;
+ }
}
}
@@ -78,6 +86,10 @@
VCardComposer composer = null;
boolean successful = false;
try {
+ if (mCancelled) {
+ Log.i(LOG_TAG, "Export request is cancelled before handling the request");
+ return;
+ }
final Uri uri = request.destUri;
final OutputStream outputStream;
try {
@@ -135,7 +147,8 @@
int current = 1; // 1-origin
while (!composer.isAfterLast()) {
- if (mCanceled) {
+ if (mCancelled) {
+ Log.i(LOG_TAG, "Export request is cancelled during composing vCard");
return;
}
if (!composer.createOneEntry()) {
@@ -223,8 +236,23 @@
mNotificationManager.notify(VCardService.EXPORT_NOTIFICATION_ID, notification);
}
- public void cancel() {
+ @Override
+ public synchronized boolean cancel(boolean mayInterruptIfRunning) {
Log.i(LOG_TAG, "received cancel request");
- mCanceled = true;
+ if (mDone || mCancelled) {
+ return false;
+ }
+ mCancelled = true;
+ return true;
+ }
+
+ @Override
+ public synchronized boolean isCancelled() {
+ return mCancelled;
+ }
+
+ @Override
+ public synchronized boolean isDone() {
+ return mDone;
}
}
diff --git a/src/com/android/contacts/vcard/ImportProcessor.java b/src/com/android/contacts/vcard/ImportProcessor.java
index acb6792..049d0a9 100644
--- a/src/com/android/contacts/vcard/ImportProcessor.java
+++ b/src/com/android/contacts/vcard/ImportProcessor.java
@@ -48,22 +48,24 @@
* Class for processing one import request from a user. Dropped after importing requested Uri(s).
* {@link VCardService} will create another object when there is another import request.
*/
-public class ImportProcessor implements Runnable {
+public class ImportProcessor extends ProcessorBase {
private static final String LOG_TAG = "VCardImport";
private final VCardService mService;
private final ContentResolver mResolver;
private final NotificationManager mNotificationManager;
+ private final ImportRequest mImportRequest;
+ private final int mJobId;
- private final List<Uri> mFailedUris = new ArrayList<Uri>();
private final ImportProgressNotifier mNotifier = new ImportProgressNotifier();
+ // TODO: remove and show appropriate message instead.
+ private final List<Uri> mFailedUris = new ArrayList<Uri>();
+
private VCardParser mVCardParser;
- private ImportRequest mImportRequest;
- private int mJobId;
-
- private boolean mCanceled;
+ private volatile boolean mCancelled;
+ private volatile boolean mDone;
public ImportProcessor(final VCardService service, final ImportRequest request,
int jobId) {
@@ -76,6 +78,10 @@
mJobId = jobId;
}
+ @Override
+ public final int getType() {
+ return PROCESSOR_TYPE_IMPORT;
+ }
@Override
public void run() {
@@ -85,13 +91,17 @@
} catch (RuntimeException e) {
Log.e(LOG_TAG, "RuntimeException thrown during import", e);
throw e;
+ } finally {
+ synchronized (this) {
+ mDone = true;
+ }
}
}
private void runInternal() {
Log.i(LOG_TAG, String.format("vCard import (id: %d) has started.", mJobId));
final ImportRequest request = mImportRequest;
- if (mCanceled) {
+ if (mCancelled) {
Log.i(LOG_TAG, "Canceled before actually handling parameter (" + request.uri + ")");
return;
}
@@ -131,7 +141,13 @@
mService.handleFinishImportNotification(mJobId, successful);
if (successful) {
- Log.i(LOG_TAG, "Successfully finished importing one vCard file: " + uri);
+ // TODO: successful becomes true even when cancelled. Should return more appropriate
+ // value
+ if (isCancelled()) {
+ Log.i(LOG_TAG, "vCard import has been canceled (uri: " + uri + ")");
+ } else {
+ Log.i(LOG_TAG, "Successfully finished importing one vCard file: " + uri);
+ }
List<Uri> uris = committer.getCreatedUris();
if (uris != null && uris.size() > 0) {
doFinishNotification(uris.get(0));
@@ -166,7 +182,7 @@
final String title;
notification.flags |= Notification.FLAG_AUTO_CANCEL;
- if (isCanceled()) {
+ if (isCancelled()) {
notification.icon = android.R.drawable.stat_notify_error;
title = mService.getString(R.string.importing_vcard_canceled_title);
} else {
@@ -190,8 +206,7 @@
mNotificationManager.notify(VCardService.IMPORT_NOTIFICATION_ID, notification);
}
- // Make package private for testing use.
- /* package */ boolean readOneVCard(Uri uri, int vcardType, String charset,
+ private boolean readOneVCard(Uri uri, int vcardType, String charset,
final VCardInterpreter interpreter,
final int[] possibleVCardVersions) {
Log.i(LOG_TAG, "start importing one vCard (Uri: " + uri + ")");
@@ -216,7 +231,7 @@
mVCardParser = (vcardVersion == ImportVCardActivity.VCARD_VERSION_V30 ?
new VCardParser_V30(vcardType) :
new VCardParser_V21(vcardType));
- if (mCanceled) {
+ if (mCancelled) {
Log.i(LOG_TAG, "ImportProcessor already recieves cancel request, so " +
"send cancel request to vCard parser too.");
mVCardParser.cancel();
@@ -260,17 +275,29 @@
return successful;
}
- public boolean isCanceled() {
- return mCanceled;
- }
-
- public void cancel() {
+ @Override
+ public synchronized boolean cancel(boolean mayInterruptIfRunning) {
Log.i(LOG_TAG, "ImportProcessor received cancel request");
- mCanceled = true;
+ if (mDone || mCancelled) {
+ return false;
+ }
+ mCancelled = true;
synchronized (this) {
if (mVCardParser != null) {
mVCardParser.cancel();
}
}
+ return true;
+ }
+
+ @Override
+ public synchronized boolean isCancelled() {
+ return mCancelled;
+ }
+
+
+ @Override
+ public synchronized boolean isDone() {
+ return mDone;
}
}
diff --git a/src/com/android/contacts/vcard/ProcessorBase.java b/src/com/android/contacts/vcard/ProcessorBase.java
new file mode 100644
index 0000000..17ac6d3
--- /dev/null
+++ b/src/com/android/contacts/vcard/ProcessorBase.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2010 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.contacts.vcard;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.RunnableFuture;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A base processor class. One instance processes vCard one import/export request (imports a given
+ * vCard or exports a vCard). Expected to be used with {@link ExecutorService}.
+ *
+ * This instance starts itself with {@link #run()} method, and can be cancelled with
+ * {@link #cancel(boolean)}. Users can check the processor's status using {@link #isCancelled()}
+ * and {@link #isDone()} asynchronously.
+ *
+ * {@link #get()} and {@link #get(long, TimeUnit)}, which are form {@link Future}, aren't
+ * supported and {@link UnsupportedOperationException} will be just thrown when they are called.
+ */
+public abstract class ProcessorBase implements RunnableFuture<Object> {
+
+ public static final int PROCESSOR_TYPE_IMPORT = 1;
+ public static final int PROCESSOR_TYPE_EXPORT = 2;
+
+ /**
+ * @return the type of the processor. Must be {@link #PROCESSOR_TYPE_IMPORT} or
+ * {@link #PROCESSOR_TYPE_EXPORT}.
+ */
+ public abstract int getType();
+
+ public abstract void run();
+
+ /**
+ * Cancels this operation.
+ *
+ * @param mayInterruptIfRunning ignored. When this method is called, the instance
+ * stops processing and finish itself even if the thread is running.
+ *
+ * @see Future#cancel(boolean)
+ */
+ public abstract boolean cancel(boolean mayInterruptIfRunning);
+ public abstract boolean isCancelled();
+ public abstract boolean isDone();
+
+ /**
+ * Just throws {@link UnsupportedOperationException}.
+ */
+ @Override
+ public final Object get() {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * Just throws {@link UnsupportedOperationException}.
+ */
+ @Override
+ public final Object get(long timeout, TimeUnit unit) {
+ throw new UnsupportedOperationException();
+ }
+}
\ No newline at end of file
diff --git a/src/com/android/contacts/vcard/VCardService.java b/src/com/android/contacts/vcard/VCardService.java
index cad2549..367634c 100644
--- a/src/com/android/contacts/vcard/VCardService.java
+++ b/src/com/android/contacts/vcard/VCardService.java
@@ -26,13 +26,10 @@
import android.util.Log;
import android.widget.Toast;
-import java.io.File;
-import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
import java.util.concurrent.RejectedExecutionException;
/**
@@ -56,7 +53,7 @@
/* package */ static final String CACHE_FILE_PREFIX = "import_tmp_";
- public class RequestHandler extends Handler {
+ private final Messenger mMessenger = new Messenger(new Handler() {
@Override
public void handleMessage(Message msg) {
switch (msg.what) {
@@ -79,32 +76,18 @@
}
}
}
- }
+ });
- private final Handler mHandler = new RequestHandler();
- private final Messenger mMessenger = new Messenger(mHandler);
// Should be single thread, as we don't want to simultaneously handle import and export
// requests.
- private ExecutorService mExecutorService = Executors.newSingleThreadExecutor();
+ private final ExecutorService mExecutorService = Executors.newSingleThreadExecutor();
- // Three types of map for remembering on-going jobs. Key is jobId. JobIds for import and export
- // are never overlapped each other.
- //
- // Note:
- // We don't use Future#cancel() but cancel() method in ImportProcessor/ExportProcessor,
- // so Future#isCanceled() should never be refered. Future objects in mFutureMap is used
- // just for checking each job is already finished or not (See Future#isDone()).
- //
- // Reason:
- // Future#cancel() doesn't halt the running thread. The thread keeps running even after Service
- // is stopped, while cancel() in ImporterProcessor/ExporterProcesser is expected to
- // stop the work and exit the thread appropriately.
private int mCurrentJobId;
- private final Map<Integer, ImportProcessor> mRunningJobMapForImport =
- new HashMap<Integer, ImportProcessor>();
- private final Map<Integer, ExportProcessor> mRunningJobMapForExport =
- new HashMap<Integer, ExportProcessor>();
- private final Map<Integer, Future<?>> mFutureMap = new HashMap<Integer, Future<?>>();
+
+ // Stores all unfinished import/export jobs which will be executed by mExecutorService.
+ // Key is jobId.
+ private final Map<Integer, ProcessorBase> mRunningJobMap =
+ new HashMap<Integer, ProcessorBase>();
@Override
public int onStartCommand(Intent intent, int flags, int id) {
@@ -118,89 +101,74 @@
@Override
public void onDestroy() {
- Log.i(LOG_TAG, "VCardService is finishing.");
- cancelRequestsAndshutdown();
+ Log.i(LOG_TAG, "VCardService is being destroyed.");
+ cancelAllRequestsAndShutdown();
clearCache();
super.onDestroy();
}
private synchronized void handleImportRequest(ImportRequest request) {
- Log.i(LOG_TAG, String.format("Received vCard import request. id: %d", mCurrentJobId));
- final ImportProcessor importProcessor =
- new ImportProcessor(this, request, mCurrentJobId);
- final Future<?> future;
- try {
- future = mExecutorService.submit(importProcessor);
- } catch (RejectedExecutionException e) {
- Log.w(LOG_TAG, "vCard import request is rejected.", e);
- // TODO: a little unkind to show Toast in this case, which is shown just a moment.
- // Ideally we should show some persistent something users can notice more easily.
- Toast.makeText(this, getString(R.string.vcard_import_request_rejected_message),
- Toast.LENGTH_LONG).show();
- return;
- }
- mRunningJobMapForImport.put(mCurrentJobId, importProcessor);
- mFutureMap.put(mCurrentJobId, future);
- mCurrentJobId++;
- // TODO: Ideally we should detect the current status of import/export and show "started"
- // when we can import right now and show "will start" when we cannot.
- Toast.makeText(this, getString(R.string.vcard_import_will_start_message),
- Toast.LENGTH_LONG).show();
+ tryExecute(new ImportProcessor(this, request, mCurrentJobId),
+ R.string.vcard_import_will_start_message,
+ R.string.vcard_import_request_rejected_message);
}
private synchronized void handleExportRequest(ExportRequest request) {
- Log.i(LOG_TAG, String.format("Received vCard export request. id: %d", mCurrentJobId));
- final ExportProcessor exportProcessor =
- new ExportProcessor(this, request, mCurrentJobId);
- final Future<?> future;
+ tryExecute(new ExportProcessor(this, request, mCurrentJobId),
+ R.string.vcard_export_will_start_message,
+ R.string.vcard_export_request_rejected_message);
+ }
+
+ /**
+ * Tries to call {@link ExecutorService#execute(Runnable)} toward a given processor and
+ * shows appropriate Toast using given resource ids.
+ * Updates relevant instances when successful.
+ */
+ private synchronized void tryExecute(ProcessorBase processor,
+ int successfulMessageId, int rejectedMessageId) {
try {
- future = mExecutorService.submit(exportProcessor);
+ mExecutorService.execute(processor);
+ mRunningJobMap.put(mCurrentJobId, processor);
+ mCurrentJobId++;
+ // TODO: Ideally we should detect the current status of import/export and show
+ // "started" when we can import right now and show "will start" when we cannot.
+ Toast.makeText(this, getString(successfulMessageId), Toast.LENGTH_LONG).show();
} catch (RejectedExecutionException e) {
- Log.w(LOG_TAG, "vCard export request is rejected.", e);
- Toast.makeText(this, getString(R.string.vcard_export_request_rejected_message),
- Toast.LENGTH_LONG).show();
- return;
+ Log.w(LOG_TAG, "Failed to excetute a job.", e);
+ // TODO: a little unkind to show Toast in this case, which is shown just a moment.
+ // Ideally we should show some persistent something users can notice more easily.
+ Toast.makeText(this, getString(rejectedMessageId), Toast.LENGTH_LONG).show();
}
- mRunningJobMapForExport.put(mCurrentJobId, exportProcessor);
- mFutureMap.put(mCurrentJobId, future);
- mCurrentJobId++;
- // See the comment in handleImportRequest()
- Toast.makeText(this, getString(R.string.vcard_export_will_start_message),
- Toast.LENGTH_LONG).show();
}
private void handleCancelAllImportRequest() {
Log.i(LOG_TAG, "Received cancel import request.");
- cancelAllImportRequest();
+ cancelAllImportRequests();
}
- private synchronized void cancelAllImportRequest() {
- for (final Map.Entry<Integer, ImportProcessor> entry :
- mRunningJobMapForImport.entrySet()) {
- final int jobId = entry.getKey();
- final ImportProcessor importProcessor = entry.getValue();
- importProcessor.cancel();
- if (mFutureMap.remove(jobId) == null) {
- Log.w(LOG_TAG, "Tried to remove unknown Future object with id: " + jobId);
+ private synchronized void cancelAllImportRequests() {
+ for (final Map.Entry<Integer, ProcessorBase> entry : mRunningJobMap.entrySet()) {
+ final ProcessorBase processor = entry.getValue();
+ if (processor.getType() == ProcessorBase.PROCESSOR_TYPE_IMPORT) {
+ final int jobId = entry.getKey();
+ processor.cancel(true);
+ mRunningJobMap.remove(jobId);
+ Log.i(LOG_TAG, String.format("Canceling job %d", jobId));
}
- Log.i(LOG_TAG, String.format("Canceling job %d", jobId));
}
- mRunningJobMapForImport.clear();
stopServiceWhenNoJob();
}
- private synchronized void cancelAllExportRequest() {
- for (final Map.Entry<Integer, ExportProcessor> entry :
- mRunningJobMapForExport.entrySet()) {
- final int jobId = entry.getKey();
- final ExportProcessor exportProcessor = entry.getValue();
- exportProcessor.cancel();
- if (mFutureMap.remove(jobId) == null) {
- Log.w(LOG_TAG, "Tried to remove unknown Future object with id: " + jobId);
+ private synchronized void cancelAllExportRequests() {
+ for (final Map.Entry<Integer, ProcessorBase> entry : mRunningJobMap.entrySet()) {
+ final ProcessorBase processor = entry.getValue();
+ if (processor.getType() == ProcessorBase.PROCESSOR_TYPE_EXPORT) {
+ final int jobId = entry.getKey();
+ processor.cancel(true);
+ mRunningJobMap.remove(jobId);
+ Log.i(LOG_TAG, String.format("Canceling job %d", jobId));
}
- Log.i(LOG_TAG, String.format("Canceling job %d", jobId));
}
- mRunningJobMapForExport.clear();
stopServiceWhenNoJob();
}
@@ -209,18 +177,12 @@
* A new job cannot be submitted any more after this call.
*/
private synchronized void stopServiceWhenNoJob() {
- // Log.v(LOG_TAG, "shutdownWhenNoJob() called");
- if (mFutureMap.size() > 0) {
- Log.v(LOG_TAG, "We have future tasks not removed from cache. Check their status.");
- for (final Map.Entry<Integer, Future<?>> entry : mFutureMap.entrySet()) {
+ if (mRunningJobMap.size() > 0) {
+ for (final Map.Entry<Integer, ProcessorBase> entry : mRunningJobMap.entrySet()) {
final int jobId = entry.getKey();
- final Future<?> future = entry.getValue();
- if (future.isDone()) {
- // This shouldn't happen. The other methods shold handle this case correctly.
- Log.w(LOG_TAG,
- String.format("Cache has already finished job (id: %d). Remove it",
- jobId));
- mFutureMap.remove(jobId);
+ final ProcessorBase processor = entry.getValue();
+ if (processor.isDone()) {
+ mRunningJobMap.remove(jobId);
} else {
Log.i(LOG_TAG, String.format("Found unfinished job (id: %d)", jobId));
return;
@@ -235,49 +197,35 @@
/* package */ synchronized void handleFinishImportNotification(
int jobId, boolean successful) {
- Log.i(LOG_TAG, String.format("Received vCard import finish notification (id: %d). "
+ Log.v(LOG_TAG, String.format("Received vCard import finish notification (id: %d). "
+ "Result: %b", jobId, (successful ? "success" : "failure")));
- if (mRunningJobMapForImport.remove(jobId) == null) {
+ if (mRunningJobMap.remove(jobId) == null) {
Log.w(LOG_TAG, String.format("Tried to remove unknown job (id: %d)", jobId));
}
- if (mFutureMap.remove(jobId) == null) {
- Log.w(LOG_TAG, "Tried to remove unknown Future object with id: " + jobId);
- }
stopServiceWhenNoJob();
}
/* package */ synchronized void handleFinishExportNotification(
int jobId, boolean successful) {
- Log.i(LOG_TAG, String.format("Received vCard export finish notification (id: %d). "
+ Log.v(LOG_TAG, String.format("Received vCard export finish notification (id: %d). "
+ "Result: %b", jobId, (successful ? "success" : "failure")));
- if (mRunningJobMapForExport.remove(jobId) == null) {
+ if (mRunningJobMap.remove(jobId) == null) {
Log.w(LOG_TAG, String.format("Tried to remove unknown job (id: %d)", jobId));
}
- if (mFutureMap.remove(jobId) == null) {
- Log.w(LOG_TAG, "Tried to remove unknown Future object with id: " + jobId);
- }
stopServiceWhenNoJob();
}
/**
- * Cancels all the import/export requests and call {@link ExecutorService#shutdown()}, which
+ * Cancels all the import/export requests and calls {@link ExecutorService#shutdown()}, which
* means this Service becomes no longer ready for import/export requests.
*
* Mainly called from onDestroy().
*/
- private synchronized void cancelRequestsAndshutdown() {
- if (mRunningJobMapForImport.size() > 0) {
- Log.i(LOG_TAG,
- String.format("Cancel existing all import requests (remains: ",
- mRunningJobMapForImport.size()));
- cancelAllImportRequest();
+ private synchronized void cancelAllRequestsAndShutdown() {
+ for (final Map.Entry<Integer, ProcessorBase> entry : mRunningJobMap.entrySet()) {
+ entry.getValue().cancel(true);
}
- if (mRunningJobMapForExport.size() > 0) {
- Log.i(LOG_TAG,
- String.format("Cancel existing all import requests (remains: ",
- mRunningJobMapForExport.size()));
- cancelAllExportRequest();
- }
+ mRunningJobMap.clear();
mExecutorService.shutdown();
}
@@ -285,15 +233,9 @@
* Removes import caches stored locally.
*/
private void clearCache() {
- Log.i(LOG_TAG, "start removing cache files if exist.");
- final String[] fileLists = fileList();
- for (String fileName : fileLists) {
+ for (final String fileName : fileList()) {
if (fileName.startsWith(CACHE_FILE_PREFIX)) {
// We don't want to keep all the caches so we remove cache files old enough.
- // TODO: Ideally we should ask VCardService whether the file is being used or
- // going to be used.
- final Date now = new Date();
- final File file = getFileStreamPath(fileName);
Log.i(LOG_TAG, "Remove a temporary file: " + fileName);
deleteFile(fileName);
}