From ca1c6ecb0ca0dc777b901e5d08087f5913d8549d Mon Sep 17 00:00:00 2001 From: VladiMihaylenko Date: Wed, 4 Jul 2018 17:55:46 +0300 Subject: [PATCH] [android] small downloader's refactoring on Android. --- .../maps/DownloadResourcesLegacyActivity.cpp | 16 ++--- .../mapswithme/maps/downloader/ChunkTask.java | 62 ++++++++----------- 2 files changed, 33 insertions(+), 45 deletions(-) diff --git a/android/jni/com/mapswithme/maps/DownloadResourcesLegacyActivity.cpp b/android/jni/com/mapswithme/maps/DownloadResourcesLegacyActivity.cpp index c6e700fd00..4ee048f33c 100644 --- a/android/jni/com/mapswithme/maps/DownloadResourcesLegacyActivity.cpp +++ b/android/jni/com/mapswithme/maps/DownloadResourcesLegacyActivity.cpp @@ -55,7 +55,7 @@ static std::shared_ptr g_currentRequest; extern "C" { - using TCallback = HttpRequest::CallbackT; + using Callback = HttpRequest::Callback; static int HasSpaceForFiles(Platform & pl, std::string const & sdcardPath, size_t fileSize) { @@ -143,11 +143,11 @@ extern "C" static void DownloadFileFinished(std::shared_ptr obj, HttpRequest const & req) { - HttpRequest::StatusT const status = req.Status(); - ASSERT_NOT_EQUAL(status, HttpRequest::EInProgress, ()); + auto const status = req.GetStatus(); + ASSERT_NOT_EQUAL(status, HttpRequest::Status::InProgress, ()); int errorCode = ERR_DOWNLOAD_ERROR; - if (status == HttpRequest::ECompleted) + if (status == HttpRequest::Status::Completed) errorCode = ERR_DOWNLOAD_SUCCESS; g_currentRequest.reset(); @@ -175,10 +175,10 @@ extern "C" JNIEnv * env = jni::GetEnv(); static jmethodID methodID = jni::GetMethodID(env, *listener, "onProgress", "(I)V"); - env->CallVoidMethod(*listener, methodID, static_cast(g_totalDownloadedBytes + req.Progress().first)); + env->CallVoidMethod(*listener, methodID, static_cast(g_totalDownloadedBytes + req.GetProgress().first)); } - static void DownloadURLListFinished(HttpRequest const & req, TCallback const & onFinish, TCallback const & onProgress) + static void DownloadURLListFinished(HttpRequest const & req, Callback const & onFinish, Callback const & onProgress) { FileToDownload & curFile = g_filesToDownload.back(); @@ -206,8 +206,8 @@ extern "C" LOG(LDEBUG, ("downloading", curFile.m_fileName, "sized", curFile.m_fileSize, "bytes")); - TCallback onFinish(std::bind(&DownloadFileFinished, jni::make_global_ref(listener), _1)); - TCallback onProgress(std::bind(&DownloadFileProgress, jni::make_global_ref(listener), _1)); + Callback onFinish(std::bind(&DownloadFileFinished, jni::make_global_ref(listener), _1)); + Callback onProgress(std::bind(&DownloadFileProgress, jni::make_global_ref(listener), _1)); g_currentRequest.reset(HttpRequest::PostJson(GetPlatform().ResourcesMetaServerUrl(), curFile.m_fileName, std::bind(&DownloadURLListFinished, _1, onFinish, onProgress))); diff --git a/android/src/com/mapswithme/maps/downloader/ChunkTask.java b/android/src/com/mapswithme/maps/downloader/ChunkTask.java index 9c54d2b525..dd2561f182 100644 --- a/android/src/com/mapswithme/maps/downloader/ChunkTask.java +++ b/android/src/com/mapswithme/maps/downloader/ChunkTask.java @@ -21,7 +21,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.Executors; @SuppressWarnings("unused") -class ChunkTask extends AsyncTask +class ChunkTask extends AsyncTask { private static final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.DOWNLOADER); private static final String TAG = "ChunkTask"; @@ -36,13 +36,13 @@ class ChunkTask extends AsyncTask private byte[] mPostBody; private final String mUserAgent; - private static final int NOT_SET = -1; - private static final int IO_ERROR = -2; - private static final int INVALID_URL = -3; - private static final int WRITE_ERROR = -4; - private static final int FILE_SIZE_CHECK_FAILED = -5; + private static final int IO_EXCEPTION = -1; + private static final int WRITE_EXCEPTION = -2; + private static final int INCONSISTENT_FILE_SIZE = -3; + private static final int NON_HTTP_RESPONSE = -4; + private static final int INVALID_URL = -5; + private static final int CANCELLED = -6; - private int mHttpErrorCode = NOT_SET; private long mDownloadedBytes; private static final Executor sExecutors = Executors.newFixedThreadPool(4); @@ -68,7 +68,7 @@ class ChunkTask extends AsyncTask } @Override - protected void onPostExecute(Boolean success) + protected void onPostExecute(Integer httpOrErrorCode) { //Log.i(TAG, "Writing chunk " + getChunkID()); @@ -78,7 +78,7 @@ class ChunkTask extends AsyncTask // start activity when no connection is present. if (!isCancelled()) - nativeOnFinish(mHttpCallbackID, success ? 200 : mHttpErrorCode, mBeg, mEnd); + nativeOnFinish(mHttpCallbackID, httpOrErrorCode, mBeg, mEnd); } @Override @@ -93,7 +93,7 @@ class ChunkTask extends AsyncTask { // Cancel downloading and notify about error. cancel(false); - nativeOnFinish(mHttpCallbackID, WRITE_ERROR, mBeg, mEnd); + nativeOnFinish(mHttpCallbackID, WRITE_EXCEPTION, mBeg, mEnd); } } } @@ -123,7 +123,7 @@ class ChunkTask extends AsyncTask } @Override - protected Boolean doInBackground(Void... p) + protected Integer doInBackground(Void... p) { //Log.i(TAG, "Start downloading chunk " + getChunkID()); @@ -139,7 +139,7 @@ class ChunkTask extends AsyncTask urlConnection = (HttpURLConnection) url.openConnection(); if (isCancelled()) - return false; + return CANCELLED; urlConnection.setUseCaches(false); urlConnection.setConnectTimeout(TIMEOUT_IN_SECONDS * 1000); @@ -180,9 +180,12 @@ class ChunkTask extends AsyncTask } if (isCancelled()) - return false; + return CANCELLED; final int err = urlConnection.getResponseCode(); + if (err == HttpURLConnection.HTTP_NOT_FOUND) + return err; + // @TODO We can handle redirect (301, 302 and 307) here and display redirected page to user, // to avoid situation when downloading is always failed by "unknown" reason // When we didn't ask for chunks, code should be 200 @@ -191,11 +194,10 @@ class ChunkTask extends AsyncTask if ((isChunk && err != HttpURLConnection.HTTP_PARTIAL) || (!isChunk && err != HttpURLConnection.HTTP_OK)) { // we've set error code so client should be notified about the error - mHttpErrorCode = FILE_SIZE_CHECK_FAILED; LOGGER.w(TAG, "Error for " + urlConnection.getURL() + ": Server replied with code " + err + ", aborting download. " + Utils.mapPrettyPrint(requestParams)); - return false; + return INCONSISTENT_FILE_SIZE; } // Check for content size - are we downloading requested file or some router's garbage? @@ -209,11 +211,10 @@ class ChunkTask extends AsyncTask if (contentLength != mExpectedFileSize) { // we've set error code so client should be notified about the error - mHttpErrorCode = FILE_SIZE_CHECK_FAILED; LOGGER.w(TAG, "Error for " + urlConnection.getURL() + ": Invalid file size received (" + contentLength + ") while expecting " + mExpectedFileSize + ". Aborting download."); - return false; + return INCONSISTENT_FILE_SIZE; } // @TODO Else display received web page to user - router is redirecting us to some page } @@ -222,30 +223,24 @@ class ChunkTask extends AsyncTask } catch (final MalformedURLException ex) { LOGGER.e(TAG, "Invalid url: " + mUrl, ex); - - mHttpErrorCode = INVALID_URL; - return false; + return INVALID_URL; } catch (final IOException ex) { LOGGER.d(TAG, "IOException in doInBackground for URL: " + mUrl, ex); - - mHttpErrorCode = IO_ERROR; - return false; + return IO_EXCEPTION; } finally { if (urlConnection != null) urlConnection.disconnect(); - else - mHttpErrorCode = IO_ERROR; } } - private boolean downloadFromStream(InputStream stream) + private Integer downloadFromStream(InputStream stream) { // Because of timeouts in InputStream.read (for bad connection), // try to introduce dynamic buffer size to read in one query. final int arrSize[] = {64, 32, 1}; - int ret = -1; + int ret = IO_EXCEPTION; for (int size : arrSize) { @@ -259,18 +254,11 @@ class ChunkTask extends AsyncTask } } - if (ret < 0) - mHttpErrorCode = IO_ERROR; - Utils.closeSafely(stream); - - return (ret == 0); + return ret; } /** - * @return 0 - download successful; - * 1 - download canceled; - * -1 - some error occurred; * @throws IOException */ private int downloadFromStreamImpl(InputStream stream, int bufferSize) throws IOException @@ -281,7 +269,7 @@ class ChunkTask extends AsyncTask while ((readBytes = stream.read(tempBuf)) > 0) { if (isCancelled()) - return 1; + return CANCELLED; final byte[] chunk = new byte[readBytes]; System.arraycopy(tempBuf, 0, chunk, 0, readBytes); @@ -290,7 +278,7 @@ class ChunkTask extends AsyncTask } // -1 - means the end of the stream (success), else - some error occurred - return (readBytes == -1 ? 0 : -1); + return (readBytes == -1 ? HttpURLConnection.HTTP_OK : IO_EXCEPTION); } private static native boolean nativeOnWrite(long httpCallbackID, long beg, byte[] data, long size);