diff --git a/android/jni/com/mapswithme/maps/DownloaderAdapter.cpp b/android/jni/com/mapswithme/maps/DownloaderAdapter.cpp index d4a5cb436d6..f9046a27ed6 100644 --- a/android/jni/com/mapswithme/maps/DownloaderAdapter.cpp +++ b/android/jni/com/mapswithme/maps/DownloaderAdapter.cpp @@ -35,14 +35,14 @@ BackgroundDownloaderAdapter::BackgroundDownloaderAdapter() BackgroundDownloaderAdapter::~BackgroundDownloaderAdapter() { - CHECK_THREAD_CHECKER(m_threadChecker, ("")); + CHECK_THREAD_CHECKER(m_threadChecker, ()); g_completionHandlers.clear(); } void BackgroundDownloaderAdapter::Remove(CountryId const & countryId) { - CHECK_THREAD_CHECKER(m_threadChecker, ("")); + CHECK_THREAD_CHECKER(m_threadChecker, ()); MapFilesDownloader::Remove(countryId); @@ -69,7 +69,7 @@ void BackgroundDownloaderAdapter::Remove(CountryId const & countryId) void BackgroundDownloaderAdapter::Clear() { - CHECK_THREAD_CHECKER(m_threadChecker, ("")); + CHECK_THREAD_CHECKER(m_threadChecker, ()); MapFilesDownloader::Clear(); @@ -92,7 +92,7 @@ void BackgroundDownloaderAdapter::Clear() QueueInterface const & BackgroundDownloaderAdapter::GetQueue() const { - CHECK_THREAD_CHECKER(m_threadChecker, ("")); + CHECK_THREAD_CHECKER(m_threadChecker, ()); if (m_queue.IsEmpty()) return MapFilesDownloader::GetQueue(); @@ -102,7 +102,7 @@ QueueInterface const & BackgroundDownloaderAdapter::GetQueue() const void BackgroundDownloaderAdapter::Download(QueuedCountry & queuedCountry) { - CHECK_THREAD_CHECKER(m_threadChecker, ("")); + CHECK_THREAD_CHECKER(m_threadChecker, ()); if (!IsDownloadingAllowed()) { @@ -134,18 +134,21 @@ void BackgroundDownloaderAdapter::DownloadFromLastUrl(CountryId const & countryI std::string const & downloadPath, std::vector && urls) { - CHECK_THREAD_CHECKER(m_threadChecker, ("")); + CHECK_THREAD_CHECKER(m_threadChecker, ()); + ASSERT(!urls.empty(), ()); auto env = jni::GetEnv(); jni::TScopedLocalRef url(env, jni::ToJavaString(env, urls.back())); auto id = static_cast(env->CallLongMethod(*m_downloadManager, m_downloadManagerEnqueue, url.get())); + jni::HandleJavaException(env); + m_queue.SetTaskInfoForCountryId(countryId, id); urls.pop_back(); auto onFinish = [this, countryId, downloadPath, urls = std::move(urls)](bool status) mutable { - CHECK_THREAD_CHECKER(m_threadChecker, ("")); + CHECK_THREAD_CHECKER(m_threadChecker, ()); if (!m_queue.Contains(countryId)) return; @@ -159,7 +162,9 @@ void BackgroundDownloaderAdapter::DownloadFromLastUrl(CountryId const & countryI { auto const country = m_queue.GetCountryById(countryId); m_queue.Remove(countryId); - country.OnDownloadFinished(downloader::DownloadStatus::Completed); + country.OnDownloadFinished(status + ? downloader::DownloadStatus::Completed + : downloader::DownloadStatus::Failed); if (m_queue.IsEmpty()) { for (auto const subscriber : m_subscribers) @@ -170,7 +175,7 @@ void BackgroundDownloaderAdapter::DownloadFromLastUrl(CountryId const & countryI auto onProgress = [this, countryId](int64_t bytesDownloaded, int64_t bytesTotal) { - CHECK_THREAD_CHECKER(m_threadChecker, ("")); + CHECK_THREAD_CHECKER(m_threadChecker, ()); if (!m_queue.Contains(countryId)) return; @@ -186,6 +191,8 @@ void BackgroundDownloaderAdapter::RemoveByRequestId(int64_t id) { auto env = jni::GetEnv(); env->CallVoidMethod(*m_downloadManager, m_downloadManagerRemove, static_cast(id)); + + jni::HandleJavaException(env); } std::unique_ptr GetDownloader() diff --git a/android/src/com/mapswithme/maps/background/AbstractLogBroadcastReceiver.java b/android/src/com/mapswithme/maps/background/AbstractLogBroadcastReceiver.java index 84fa4842b40..06ac0b7c2a5 100644 --- a/android/src/com/mapswithme/maps/background/AbstractLogBroadcastReceiver.java +++ b/android/src/com/mapswithme/maps/background/AbstractLogBroadcastReceiver.java @@ -23,7 +23,7 @@ public final void onReceive(Context context, Intent intent) { if (intent == null) { - LOGGER.w(getTag(), "An intent with null intent detected"); + LOGGER.w(getTag(), "A null intent detected"); return; } diff --git a/android/src/com/mapswithme/maps/background/SystemDownloadCompletedService.java b/android/src/com/mapswithme/maps/background/SystemDownloadCompletedService.java index de61c5d2b5a..2d09daf323e 100644 --- a/android/src/com/mapswithme/maps/background/SystemDownloadCompletedService.java +++ b/android/src/com/mapswithme/maps/background/SystemDownloadCompletedService.java @@ -1,6 +1,7 @@ package com.mapswithme.maps.background; import android.app.DownloadManager; +import android.content.Context; import android.content.Intent; import android.database.Cursor; @@ -13,6 +14,11 @@ public class SystemDownloadCompletedService extends JobIntentService { + private interface DownloadProcessor + { + boolean process(@NonNull Context context, long id, @NonNull Cursor cursor); + } + @Override public void onCreate() { @@ -44,10 +50,16 @@ private void processIntent(@NonNull DownloadManager manager, @NonNull Intent int if (!cursor.moveToFirst()) return; - if (MapDownloadCompletedProcessor.isSupported(cursor)) - MapDownloadCompletedProcessor.process(getApplicationContext(), id, cursor); - else - BookmarksDownloadCompletedProcessor.process(getApplicationContext(), cursor); + final DownloadProcessor[] processors = { + MapDownloadCompletedProcessor::process, + BookmarksDownloadCompletedProcessor::process + }; + + for (DownloadProcessor processor : processors) + { + if (processor.process(getApplicationContext(), id, cursor)) + break; + } } finally { diff --git a/android/src/com/mapswithme/maps/bookmarks/BookmarksDownloadCompletedProcessor.java b/android/src/com/mapswithme/maps/bookmarks/BookmarksDownloadCompletedProcessor.java index 5ce7dabfb49..b35fa25a825 100644 --- a/android/src/com/mapswithme/maps/bookmarks/BookmarksDownloadCompletedProcessor.java +++ b/android/src/com/mapswithme/maps/bookmarks/BookmarksDownloadCompletedProcessor.java @@ -10,6 +10,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.WorkerThread; import androidx.localbroadcastmanager.content.LocalBroadcastManager; import com.mapswithme.maps.MwmApplication; import com.mapswithme.maps.bookmarks.data.Error; @@ -26,39 +27,44 @@ public class BookmarksDownloadCompletedProcessor { + private static final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.DOWNLOADER); + private static final String TAG = BookmarksDownloadCompletedProcessor.class.getSimpleName(); + public final static String ACTION_DOWNLOAD_COMPLETED = "action_download_completed"; public final static String EXTRA_DOWNLOAD_STATUS = "extra_download_status"; - public static void process(@NonNull Context context, @NonNull Cursor cursor) + @WorkerThread + public static boolean process(@NonNull Context context, long id, @NonNull Cursor cursor) { - final OperationStatus status = processInternal(context, cursor); - Logger logger = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.BILLING); - String tag = BookmarksDownloadCompletedProcessor.class.getSimpleName(); - logger.i(tag, "Download status: " + status); + try + { + final OperationStatus status = processInternal(context, cursor); + LOGGER.i(TAG, "Download status: " + status); + + UiThread.run(new BookmarkDownloadCompletedTask(context, status)); + } + catch (Exception e) + { + LOGGER.e(TAG, "Failed to process bookmark download for request id " + id + ". Exception ", e); + return false; + } - UiThread.run(new BookmarkDownloadCompletedTask(context, status)); + return true; } @NonNull private static OperationStatus processInternal(@NonNull Context context, @NonNull Cursor cursor) { - try + if (isDownloadFailed(cursor)) { - if (isDownloadFailed(cursor)) - { - Error error = new Error(getHttpStatus(cursor), getErrorMessage(cursor)); - return new OperationStatus(null, error); - } + Error error = new Error(getHttpStatus(cursor), getErrorMessage(cursor)); + return new OperationStatus(null, error); + } - logToPushWoosh((Application) context, cursor); + logToPushWoosh((Application) context, cursor); - Result result = new Result(getFilePath(cursor), getArchiveId(cursor)); - return new OperationStatus(result, null); - } - catch (Exception e) - { - return new OperationStatus(null, new Error(e.getMessage())); - } + Result result = new Result(getFilePath(cursor), getArchiveId(cursor)); + return new OperationStatus(result, null); } private static boolean isDownloadFailed(@NonNull Cursor cursor) diff --git a/android/src/com/mapswithme/maps/downloader/MapDownloadCompletedProcessor.java b/android/src/com/mapswithme/maps/downloader/MapDownloadCompletedProcessor.java index b19f6a5bd90..bd346cc855a 100644 --- a/android/src/com/mapswithme/maps/downloader/MapDownloadCompletedProcessor.java +++ b/android/src/com/mapswithme/maps/downloader/MapDownloadCompletedProcessor.java @@ -10,6 +10,7 @@ import androidx.annotation.MainThread; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.WorkerThread; import com.mapswithme.maps.MwmApplication; import com.mapswithme.util.concurrency.UiThread; import com.mapswithme.util.log.Logger; @@ -25,29 +26,31 @@ public class MapDownloadCompletedProcessor private static final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.DOWNLOADER); private static final String TAG = MapDownloadCompletedProcessor.class.getSimpleName(); - public static boolean isSupported(@NonNull Cursor cursor) - { - String targetUri = cursor.getString(cursor.getColumnIndex(DownloadManager.COLUMN_URI)); - String targetUriPath = Uri.parse(targetUri).getPath(); - - return !TextUtils.isEmpty(targetUriPath) && MapManager.nativeIsUrlSupported(targetUriPath); - } - - public static void process(@NonNull Context context, long id, @NonNull Cursor cursor) + @WorkerThread + public static boolean process(@NonNull Context context, long id, @NonNull Cursor cursor) { try { String targetUri = cursor.getString(cursor.getColumnIndex(DownloadManager.COLUMN_URI)); + String targetUriPath = Uri.parse(targetUri).getPath(); + + if (TextUtils.isEmpty(targetUriPath) || !MapManager.nativeIsUrlSupported(targetUriPath)) + return false; + int status = cursor.getInt(cursor.getColumnIndex(DownloadManager.COLUMN_STATUS)); String downloadedFileUri = cursor.getString(cursor.getColumnIndex(DownloadManager.COLUMN_LOCAL_URI)); boolean result = processColumns(context, status, targetUri, downloadedFileUri); UiThread.run(new MapDownloadCompletedTask(context, result, id)); LOGGER.d(TAG, "Processing for map downloading for request id " + id + " is finished."); + + return true; } catch (Exception e) { LOGGER.e(TAG, "Failed to process map download for request id " + id + ". Exception ", e); + + return false; } } @@ -108,9 +111,6 @@ private MapDownloadCompletedTask(@NonNull Context applicationContext, boolean st public void run() { MwmApplication application = MwmApplication.from(mAppContext); - if (!application.arePlatformAndCoreInitialized()) - return; - MapDownloadManager manager = MapDownloadManager.from(application); manager.onDownloadFinished(mStatus, mId); } diff --git a/android/src/com/mapswithme/maps/downloader/MapDownloadManager.java b/android/src/com/mapswithme/maps/downloader/MapDownloadManager.java index 74980de5ec1..0539f3b4083 100644 --- a/android/src/com/mapswithme/maps/downloader/MapDownloadManager.java +++ b/android/src/com/mapswithme/maps/downloader/MapDownloadManager.java @@ -11,6 +11,8 @@ import androidx.annotation.Nullable; import com.mapswithme.maps.MwmApplication; import com.mapswithme.util.Utils; +import com.mapswithme.util.log.Logger; +import com.mapswithme.util.log.LoggerFactory; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; @@ -19,9 +21,12 @@ public class MapDownloadManager { + private static final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.DOWNLOADER); + private static final String TAG = MapDownloadManager.class.getSimpleName(); + @NonNull private DownloadManager mDownloadManager; - @NonNull + @Nullable private Map mRestoredRequests; @NonNull private MapDownloadProgressTracker mProgressTracker; @@ -35,15 +40,16 @@ public MapDownloadManager(@NonNull Context context) throw new NullPointerException("Download manager is null, failed to create MapDownloadManager"); mDownloadManager = downloadManager; - mRestoredRequests = loadEnqueued(); mProgressTracker = new MapDownloadProgressTracker(context); } @NonNull private Map loadEnqueued() { + Map result = new HashMap<>(); DownloadManager.Query query = new DownloadManager.Query(); - query.setFilterByStatus(DownloadManager.STATUS_PENDING | DownloadManager.STATUS_RUNNING); + query.setFilterByStatus(DownloadManager.STATUS_PENDING | DownloadManager.STATUS_RUNNING | + DownloadManager.STATUS_PAUSED | DownloadManager.STATUS_SUCCESSFUL); Cursor cursor = null; try { @@ -51,11 +57,10 @@ private Map loadEnqueued() cursor.moveToFirst(); int count = cursor.getCount(); - Map result = new HashMap<>(); for (int i = 0; i < count; ++i) { - long id = cursor.getInt(cursor.getColumnIndex(DownloadManager.COLUMN_ID)); + long id = cursor.getLong(cursor.getColumnIndex(DownloadManager.COLUMN_ID)); String url = cursor.getString(cursor.getColumnIndex(DownloadManager.COLUMN_URI)); String urlPath = getUrlPath(url); @@ -64,13 +69,17 @@ private Map loadEnqueued() cursor.moveToNext(); } - - return result; + } + catch (Exception e) + { + LOGGER.e(TAG, "Failed to load enqueued requests. Exception ", e); } finally { Utils.closeSafely(cursor); } + + return result; } @Nullable @@ -101,6 +110,9 @@ public long enqueue(@NonNull String url) if (uriPath == null) throw new AssertionError("The path must be not null"); + if (mRestoredRequests == null) + mRestoredRequests = loadEnqueued(); + Long id = mRestoredRequests.get(uriPath); long requestId = 0; diff --git a/android/src/com/mapswithme/maps/downloader/MapDownloadProgressTracker.java b/android/src/com/mapswithme/maps/downloader/MapDownloadProgressTracker.java index 6e719b26da9..e78e24638c1 100644 --- a/android/src/com/mapswithme/maps/downloader/MapDownloadProgressTracker.java +++ b/android/src/com/mapswithme/maps/downloader/MapDownloadProgressTracker.java @@ -21,9 +21,9 @@ public class MapDownloadProgressTracker private static final long PROGRESS_TRACKING_INTERVAL_MILLISECONDS = 1000; @NonNull - DownloadManager mDownloadManager; + private final DownloadManager mDownloadManager; @NonNull - private Set mTrackingIds = new HashSet<>(); + private final Set mTrackingIds = new HashSet<>(); private boolean mTrackingEnabled = false; private final Runnable mTrackingMethod = this::trackProgress; diff --git a/android/src/com/mapswithme/maps/downloader/MapManager.java b/android/src/com/mapswithme/maps/downloader/MapManager.java index 23a748bb32c..953c99f3fd6 100644 --- a/android/src/com/mapswithme/maps/downloader/MapManager.java +++ b/android/src/com/mapswithme/maps/downloader/MapManager.java @@ -436,7 +436,8 @@ public void run() public static native @Nullable String nativeGetSelectedCountry(); public static native boolean nativeIsUrlSupported(@NonNull String url); - public static native @NonNull String nativeGetFilePathByUrl(@NonNull String url); + @NonNull + public static native String nativeGetFilePathByUrl(@NonNull String url); public static native void nativeOnDownloadFinished(boolean status, long id); public static native void nativeOnDownloadProgress(long id, long bytesDownloaded, long bytesTotal); diff --git a/storage/CMakeLists.txt b/storage/CMakeLists.txt index bb367c18828..9450f32a4d6 100644 --- a/storage/CMakeLists.txt +++ b/storage/CMakeLists.txt @@ -51,7 +51,7 @@ set( storage_helpers.hpp ) -if (${PLATFORM_ANDROID}) +if(${PLATFORM_ANDROID}) append( SRC background_downloading/downloader_queue.hpp