diff --git a/CHANGELOG.md b/CHANGELOG.md index 43f24a9960..93ab1d701b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # App Center SDK for Android Change Log +## Version 2.4.1 + +### App Center Distribute + +* **[Fix]** Fix a crash and improve logging when downloading an update fails on Android 5+. + +___ + ## Version 2.4.0 ### App Center Crashes diff --git a/sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/CrashesAndroidTest.java b/sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/CrashesAndroidTest.java index 8a7f855844..9e946658f0 100644 --- a/sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/CrashesAndroidTest.java +++ b/sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/CrashesAndroidTest.java @@ -106,10 +106,16 @@ public static void setUpClass() { public void setUp() { Thread.setDefaultUncaughtExceptionHandler(sDefaultCrashHandler); SharedPreferencesManager.clear(); - for (File logFile : ErrorLogHelper.getErrorStorageDirectory().listFiles()) { + File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(); + assertNotNull(files); + for (File logFile : files) { if (logFile.isDirectory()) { - for (File dumpDir : logFile.listFiles()) { - for (File dumpFile : dumpDir.listFiles()) { + File[] childFiles = logFile.listFiles(); + assertNotNull(childFiles); + for (File dumpDir : childFiles) { + File[] dumpFiles = dumpDir.listFiles(); + assertNotNull(dumpFiles); + for (File dumpFile : dumpFiles) { assertTrue(dumpFile.delete()); } } @@ -141,7 +147,7 @@ private void startFresh(CrashesListener listener) { AppCenter.setLogLevel(android.util.Log.VERBOSE); AppCenter.configure(sApplication, "a"); - /* Clean logs. */ + /* Clean state. */ AppCenter.setEnabled(false); AppCenter.setEnabled(true).get(); @@ -343,7 +349,9 @@ public void run() { thread.start(); thread.join(); verify(uncaughtExceptionHandler).uncaughtException(thread, exception); - assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(2, files.length); verifyZeroInteractions(crashesListener); /* Second process: enqueue log but network is down... */ @@ -374,7 +382,9 @@ public void run() { /* Waiting user confirmation so no log sent yet. */ verify(mChannel, never()).enqueue(isA(ManagedErrorLog.class), anyString(), anyInt()); - assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(2, files.length); verify(crashesListener).shouldProcess(any(ErrorReport.class)); verify(crashesListener).shouldAwaitUserConfirmation(); verifyNoMoreInteractions(crashesListener); @@ -386,7 +396,9 @@ public void run() { verify(mChannel).enqueue(log.capture(), anyString(), eq(CRITICAL)); assertNotNull(log.getValue()); assertEquals(mUserId, log.getValue().getUserId()); - assertEquals(1, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(1, files.length); verify(crashesListener).getErrorAttachments(any(ErrorReport.class)); verifyNoMoreInteractions(crashesListener); @@ -420,7 +432,9 @@ public void run() { }); semaphore.acquire(); - assertEquals(0, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(0, files.length); verify(mChannel, never()).enqueue(isA(ManagedErrorLog.class), anyString(), anyInt()); verify(crashesListener).onBeforeSending(any(ErrorReport.class)); verify(crashesListener).onSendingSucceeded(any(ErrorReport.class)); @@ -459,7 +473,9 @@ public void run() { /* Waiting user confirmation so no log sent yet. */ verify(mChannel, never()).enqueue(isA(ManagedErrorLog.class), anyString(), anyInt()); - assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(2, files.length); verify(crashesListener).shouldProcess(any(ErrorReport.class)); verify(crashesListener).shouldAwaitUserConfirmation(); verifyNoMoreInteractions(crashesListener); @@ -473,7 +489,9 @@ public void run() { assertEquals(mUserId, managedErrorLog.getValue().getUserId()); assertNotNull(managedErrorLog.getValue().getException()); assertNull(managedErrorLog.getValue().getException().getMinidumpFilePath()); - assertEquals(1, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(1, files.length); verify(crashesListener).getErrorAttachments(any(ErrorReport.class)); verifyNoMoreInteractions(crashesListener); @@ -514,12 +532,16 @@ public void run() { thread.start(); thread.join(); verify(uncaughtExceptionHandler).uncaughtException(thread, exception); - assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(2, files.length); /* Disable, test waiting for disable to finish. */ Crashes.setEnabled(false).get(); assertFalse(Crashes.isEnabled().get()); - assertEquals(0, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(0, files.length); } @Test @@ -545,6 +567,8 @@ public void run() { verify(uncaughtExceptionHandler).uncaughtException(thread, exception); /* Check there are only 2 files: the throwable and the json one. */ - assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length); + File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter); + assertNotNull(files); + assertEquals(2, files.length); } } diff --git a/sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/WrapperSdkExceptionManagerAndroidTest.java b/sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/WrapperSdkExceptionManagerAndroidTest.java index 0ec9cc8024..ed716cf4be 100644 --- a/sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/WrapperSdkExceptionManagerAndroidTest.java +++ b/sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/WrapperSdkExceptionManagerAndroidTest.java @@ -23,7 +23,6 @@ import org.junit.Test; import java.io.File; -import java.lang.reflect.Method; import java.util.UUID; import static com.microsoft.appcenter.test.TestUtils.TAG; @@ -58,7 +57,7 @@ public void setUp() { } } - private void startFresh() throws java.lang.Exception { + private void startFresh() { /* Configure new instance. */ AppCenterPrivateHelper.unsetInstance(); @@ -66,25 +65,12 @@ private void startFresh() throws java.lang.Exception { AppCenter.setLogLevel(android.util.Log.VERBOSE); AppCenter.configure(sApplication, "a"); - /* Replace channel. */ - Method method = AppCenter.class.getDeclaredMethod("getInstance"); - method.setAccessible(true); - AppCenter appCenter = (AppCenter) method.invoke(null); - method = AppCenter.class.getDeclaredMethod("setChannel", Channel.class); - method.setAccessible(true); - method.invoke(appCenter, mock(Channel.class)); - - /* - * Since this is a real Android test, it might actually try to send crash logs - * and will delete files on sending completion. Avoid that by requesting user confirmation. - */ - Crashes.setListener(new AbstractCrashesListener() { - - @Override - public boolean shouldAwaitUserConfirmation() { - return false; - } - }); + /* Clean state. */ + AppCenter.setEnabled(false); + AppCenter.setEnabled(true).get(); + + /* Replace channel to avoid trying to send logs. */ + AppCenter.getInstance().setChannel(mock(Channel.class)); /* Start crashes. */ AppCenter.start(Crashes.class); @@ -94,7 +80,7 @@ public boolean shouldAwaitUserConfirmation() { } @Test - public void saveWrapperException() throws java.lang.Exception { + public void saveWrapperException() { class ErrorData { private String data; diff --git a/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/ReleaseDownloadListener.java b/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/ReleaseDownloadListener.java index 15e07c8a3a..f907420761 100644 --- a/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/ReleaseDownloadListener.java +++ b/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/ReleaseDownloadListener.java @@ -10,6 +10,7 @@ import android.content.Intent; import android.net.Uri; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.support.annotation.UiThread; import android.support.annotation.WorkerThread; import android.widget.Toast; @@ -112,8 +113,9 @@ public boolean onComplete(@NonNull Uri localUri) { @WorkerThread @Override - public void onError(@NonNull String errorMessage) { - AppCenterLog.error(LOG_TAG, errorMessage); + public void onError(@Nullable String errorMessage) { + AppCenterLog.error(LOG_TAG, String.format(Locale.ENGLISH, "Failed to download %s (%d) update: %s", + mReleaseDetails.getShortVersion(), mReleaseDetails.getVersion(), errorMessage)); HandlerUtils.runOnUiThread(new Runnable() { @Override diff --git a/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/ReleaseDownloader.java b/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/ReleaseDownloader.java index 1c200df523..43defce2ad 100644 --- a/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/ReleaseDownloader.java +++ b/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/ReleaseDownloader.java @@ -8,6 +8,7 @@ import android.net.Uri; import android.support.annotation.AnyThread; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.support.annotation.WorkerThread; import com.microsoft.appcenter.distribute.ReleaseDetails; @@ -83,6 +84,6 @@ interface Listener { * * @param errorMessage The message of the exception. */ - void onError(String errorMessage); + void onError(@Nullable String errorMessage); } } diff --git a/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/http/HttpConnectionReleaseDownloader.java b/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/http/HttpConnectionReleaseDownloader.java index 2fbef15276..bf3ad299a3 100644 --- a/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/http/HttpConnectionReleaseDownloader.java +++ b/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/http/HttpConnectionReleaseDownloader.java @@ -234,7 +234,7 @@ synchronized void onDownloadComplete(File targetFile) { } @WorkerThread - synchronized void onDownloadError(String errorMessage) { + synchronized void onDownloadError(@Nullable String errorMessage) { if (isCancelled()) { return; } diff --git a/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTask.java b/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTask.java index d0c8788df2..51892e97dd 100644 --- a/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTask.java +++ b/sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTask.java @@ -38,18 +38,19 @@ protected Void doInBackground(Void... params) { try { Cursor cursor = downloadManager.query(new DownloadManager.Query().setFilterById(downloadId)); if (cursor == null) { - throw new NoSuchElementException(); + throw new NoSuchElementException("Cannot find download with id=" + downloadId); } try { if (!cursor.moveToFirst()) { - throw new NoSuchElementException(); + throw new NoSuchElementException("Cannot find download with id=" + downloadId); } if (isCancelled()) { return null; } int status = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS)); if (status == DownloadManager.STATUS_FAILED) { - throw new IllegalStateException(); + int reason = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_REASON)); + throw new IllegalStateException("The download has failed with reason code: " + reason); } if (status != DownloadManager.STATUS_SUCCESSFUL) { mDownloader.onDownloadProgress(cursor); diff --git a/sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/ReleaseDownloadListenerTest.java b/sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/ReleaseDownloadListenerTest.java index edef49c605..61d41e5ffa 100644 --- a/sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/ReleaseDownloadListenerTest.java +++ b/sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/ReleaseDownloadListenerTest.java @@ -232,7 +232,7 @@ public Void answer(InvocationOnMock invocation) { } @Test - public void onStartTest() throws Exception { + public void onStart() throws Exception { ReleaseDetails releaseDetails = mockReleaseDetails(true); ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, releaseDetails); long mockTime = 1000 * 1000 * 1000; @@ -245,7 +245,7 @@ public void onStartTest() throws Exception { } @Test - public void dontShowDialogOnZeroProgressTest() throws Exception { + public void doNotShowDialogOnZeroProgress() throws Exception { ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(true)); /* Setup progressDialog. */ @@ -265,7 +265,7 @@ public void dontShowDialogOnZeroProgressTest() throws Exception { } @Test - public void dontShowNullDialogOnProgressTest() throws Exception { + public void doNotShowNullDialogOnProgress() throws Exception { /* If release is mandatory, the progressDialog is not set and equals to null. */ ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(false)); @@ -285,7 +285,7 @@ public void dontShowNullDialogOnProgressTest() throws Exception { } @Test - public void dontHideNullDialogTest() throws Exception { + public void doNotHideNullDialog() throws Exception { ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(false)); /* We don't setup progressDialog here by calling showDownloadProgress(). */ @@ -297,7 +297,7 @@ public void dontHideNullDialogTest() throws Exception { } @Test - public void hideDialogTest() throws Exception { + public void hideDialog() throws Exception { ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(true)); /* Setup progressDialog. */ @@ -310,7 +310,7 @@ public void hideDialogTest() throws Exception { } @Test - public void showDialogOnProgressTest() throws Exception { + public void showDialogOnProgress() throws Exception { ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(true)); when(mProgressDialog.isIndeterminate()).thenReturn(true); @@ -329,7 +329,7 @@ public void showDialogOnProgressTest() throws Exception { } @Test - public void showIndeterminateDialogOnProgressTest() throws Exception { + public void showIndeterminateDialogOnProgress() throws Exception { ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(true)); when(mProgressDialog.isIndeterminate()).thenReturn(false); @@ -350,17 +350,27 @@ public void showIndeterminateDialogOnProgressTest() throws Exception { } @Test - public void onErrorTest() throws Exception { + public void onError() throws Exception { ReleaseDetails mockReleaseDetails = mockReleaseDetails(true); ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails); - releaseDownloadListener.onError(""); + releaseDownloadListener.onError("test error"); /* Verify that completeWorkflow() is called on error. */ verify(mDistribute).completeWorkflow(mockReleaseDetails); } @Test - public void onCompleteTest() throws Exception { + public void onErrorNullMessage() throws Exception { + ReleaseDetails mockReleaseDetails = mockReleaseDetails(true); + ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails); + releaseDownloadListener.onError(null); + + /* Verify that completeWorkflow() is called on error. */ + verify(mDistribute).completeWorkflow(mockReleaseDetails); + } + + @Test + public void onComplete() throws Exception { ReleaseDetails mockReleaseDetails = mockReleaseDetails(true); /* Do not notify the download. */ @@ -374,7 +384,7 @@ public void onCompleteTest() throws Exception { } @Test - public void onCompleteNotifyTest() throws Exception { + public void onCompleteNotify() throws Exception { boolean mandatoryUpdate = false; ReleaseDetails mockReleaseDetails = mockReleaseDetails(mandatoryUpdate); @@ -389,7 +399,7 @@ public void onCompleteNotifyTest() throws Exception { } @Test - public void onCompleteActivityNotResolvedTest() throws Exception { + public void onCompleteActivityNotResolved() throws Exception { boolean mandatoryUpdate = false; /* Mock resolving to null activity. */ diff --git a/sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTaskTest.java b/sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTaskTest.java index 4a203fd6a9..782ad9e38d 100644 --- a/sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTaskTest.java +++ b/sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTaskTest.java @@ -11,10 +11,13 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import static com.microsoft.appcenter.distribute.DistributeConstants.INVALID_DOWNLOAD_IDENTIFIER; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.ignoreStubs; @@ -46,6 +49,7 @@ public void setUp() { /* Mock Cursor. */ when(mCursor.moveToFirst()).thenReturn(true); when(mCursor.getColumnIndexOrThrow(eq(DownloadManager.COLUMN_STATUS))).thenReturn(DownloadManager.COLUMN_STATUS.hashCode()); + when(mCursor.getColumnIndexOrThrow(eq(DownloadManager.COLUMN_REASON))).thenReturn(DownloadManager.COLUMN_REASON.hashCode()); /* Mock DownloadManager. */ when(mDownloadManager.enqueue(any(DownloadManager.Request.class))).thenReturn(DOWNLOAD_ID); @@ -108,12 +112,17 @@ public void doNothingAfterCancellation() { @Test public void errorOnFailedStatus() { when(mCursor.getInt(eq(DownloadManager.COLUMN_STATUS.hashCode()))).thenReturn(DownloadManager.STATUS_FAILED); + when(mCursor.getInt(eq(DownloadManager.COLUMN_REASON.hashCode()))).thenReturn(42); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(RuntimeException.class); /* Perform background task. */ mUpdateTask.doInBackground(); /* Verify. */ - verify(mDownloader).onDownloadError(any(RuntimeException.class)); + verify(mDownloader).onDownloadError(argumentCaptor.capture()); + String capturedExceptionMessage = argumentCaptor.getValue().getMessage(); + assertNotNull(capturedExceptionMessage); + assertTrue(capturedExceptionMessage.endsWith("42")); verify(mCursor).close(); } diff --git a/sdk/appcenter/src/main/java/com/microsoft/appcenter/utils/AppCenterLog.java b/sdk/appcenter/src/main/java/com/microsoft/appcenter/utils/AppCenterLog.java index 0965331e18..bf44eaa7ac 100644 --- a/sdk/appcenter/src/main/java/com/microsoft/appcenter/utils/AppCenterLog.java +++ b/sdk/appcenter/src/main/java/com/microsoft/appcenter/utils/AppCenterLog.java @@ -6,6 +6,7 @@ package com.microsoft.appcenter.utils; import android.support.annotation.IntRange; +import android.support.annotation.NonNull; import android.util.Log; import static android.util.Log.VERBOSE; @@ -62,7 +63,7 @@ public static void setLogLevel(@IntRange(from = VERBOSE, to = NONE) int logLevel * @param tag the log tag for your message * @param message the log message */ - public static void verbose(String tag, String message) { + public static void verbose(@NonNull String tag, @NonNull String message) { if (sLogLevel <= Log.VERBOSE) { Log.v(tag, message); } @@ -76,7 +77,7 @@ public static void verbose(String tag, String message) { * @param throwable the throwable you want to log */ @SuppressWarnings("SameParameterValue") - public static void verbose(String tag, String message, Throwable throwable) { + public static void verbose(@NonNull String tag, @NonNull String message, Throwable throwable) { if (sLogLevel <= Log.VERBOSE) { Log.v(tag, message, throwable); } @@ -88,7 +89,7 @@ public static void verbose(String tag, String message, Throwable throwable) { * @param tag the log tag for your message * @param message the log message */ - public static void debug(String tag, String message) { + public static void debug(@NonNull String tag, @NonNull String message) { if (sLogLevel <= Log.DEBUG) { Log.d(tag, message); } @@ -101,7 +102,7 @@ public static void debug(String tag, String message) { * @param message the log message * @param throwable the throwable you want to log */ - public static void debug(String tag, String message, Throwable throwable) { + public static void debug(@NonNull String tag, @NonNull String message, Throwable throwable) { if (sLogLevel <= Log.DEBUG) { Log.d(tag, message, throwable); } @@ -113,7 +114,7 @@ public static void debug(String tag, String message, Throwable throwable) { * @param tag the log tag for your message * @param message the log message */ - public static void info(String tag, String message) { + public static void info(@NonNull String tag, @NonNull String message) { if (sLogLevel <= Log.INFO) { Log.i(tag, message); } @@ -127,7 +128,7 @@ public static void info(String tag, String message) { * @param throwable the throwable you want to log */ @SuppressWarnings("SameParameterValue") - public static void info(String tag, String message, Throwable throwable) { + public static void info(@NonNull String tag, @NonNull String message, Throwable throwable) { if (sLogLevel <= Log.INFO) { Log.i(tag, message, throwable); } @@ -139,7 +140,7 @@ public static void info(String tag, String message, Throwable throwable) { * @param tag the TAG * @param message the log message */ - public static void warn(String tag, String message) { + public static void warn(@NonNull String tag, @NonNull String message) { if (sLogLevel <= Log.WARN) { Log.w(tag, message); } @@ -152,7 +153,7 @@ public static void warn(String tag, String message) { * @param message the log message * @param throwable the throwable you want to log */ - public static void warn(String tag, String message, Throwable throwable) { + public static void warn(@NonNull String tag, @NonNull String message, Throwable throwable) { if (sLogLevel <= Log.WARN) { Log.w(tag, message, throwable); } @@ -164,7 +165,7 @@ public static void warn(String tag, String message, Throwable throwable) { * @param tag the log tag for your message * @param message the log message */ - public static void error(String tag, String message) { + public static void error(@NonNull String tag, @NonNull String message) { if (sLogLevel <= Log.ERROR) { Log.e(tag, message); } @@ -177,7 +178,7 @@ public static void error(String tag, String message) { * @param message the log message * @param throwable the throwable you want to log */ - public static void error(String tag, String message, Throwable throwable) { + public static void error(@NonNull String tag, @NonNull String message, Throwable throwable) { if (sLogLevel <= Log.ERROR) { Log.e(tag, message, throwable); } @@ -190,7 +191,7 @@ public static void error(String tag, String message, Throwable throwable) { * @param message the log message */ @SuppressWarnings("WeakerAccess") - public static void logAssert(String tag, String message) { + public static void logAssert(@NonNull String tag, @NonNull String message) { if (sLogLevel <= Log.ASSERT) { Log.println(Log.ASSERT, tag, message); } @@ -204,7 +205,7 @@ public static void logAssert(String tag, String message) { * @param throwable the throwable you want to log */ @SuppressWarnings({"WeakerAccess", "SameParameterValue"}) - public static void logAssert(String tag, String message, Throwable throwable) { + public static void logAssert(@NonNull String tag, @NonNull String message, Throwable throwable) { if (sLogLevel <= Log.ASSERT) { Log.println(Log.ASSERT, tag, message + "\n" + Log.getStackTraceString(throwable)); } diff --git a/sdk/appcenter/src/test/java/com/microsoft/appcenter/http/DefaultHttpClientTest.java b/sdk/appcenter/src/test/java/com/microsoft/appcenter/http/DefaultHttpClientTest.java index 2511ddee51..db8e75f817 100644 --- a/sdk/appcenter/src/test/java/com/microsoft/appcenter/http/DefaultHttpClientTest.java +++ b/sdk/appcenter/src/test/java/com/microsoft/appcenter/http/DefaultHttpClientTest.java @@ -45,6 +45,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.zip.GZIPOutputStream; @@ -122,6 +123,7 @@ public Object answer(InvocationOnMock invocation) { @Override public DefaultHttpClientCallTask answer(InvocationOnMock invocation) { + call.onPreExecute(); Object result = call.doInBackground(); if (call.isCancelled()) { call.onCancelled(result); @@ -673,16 +675,42 @@ public void cancelCurrentCallsOnClose() throws Exception { /* Mock AsyncTask. */ String urlString = "https://mock/get"; - DefaultHttpClientCallTask mockCall = mock(DefaultHttpClientCallTask.class); - whenNew(DefaultHttpClientCallTask.class).withAnyArguments().thenReturn(mockCall); + final AtomicReference callTask = new AtomicReference<>(); + whenNew(DefaultHttpClientCallTask.class).withAnyArguments().thenAnswer(new Answer() { + + @Override + public Object answer(InvocationOnMock invocation) { + @SuppressWarnings("unchecked") final DefaultHttpClientCallTask call = spy(new DefaultHttpClientCallTask( + invocation.getArguments()[0].toString(), + invocation.getArguments()[1].toString(), + (Map) invocation.getArguments()[2], + (HttpClient.CallTemplate) invocation.getArguments()[3], + (ServiceCallback) invocation.getArguments()[4], + (DefaultHttpClientCallTask.Tracker) invocation.getArguments()[5], + (boolean) invocation.getArguments()[6])); + callTask.set(call); + when(call.executeOnExecutor(any(Executor.class))).then(new Answer() { + + @Override + public DefaultHttpClientCallTask answer(InvocationOnMock invocation) { + call.onPreExecute(); + + /* Simulate we will cancel before doInBackground. */ + return call; + } + }); + return call; + } + }); + + /* Simulate HTTP call. */ DefaultHttpClient httpClient = new DefaultHttpClient(); ServiceCallback serviceCallback = mock(ServiceCallback.class); httpClient.callAsync(urlString, "", new HashMap(), mock(HttpClient.CallTemplate.class), serviceCallback); - httpClient.onStart(mockCall); /* Close and verify. */ httpClient.close(); - verify(mockCall).cancel(true); + verify(callTask.get()).cancel(true); assertEquals(0, httpClient.getTasks().size()); } diff --git a/sdk/appcenter/src/test/java/com/microsoft/appcenter/ingestion/AppCenterIngestionTest.java b/sdk/appcenter/src/test/java/com/microsoft/appcenter/ingestion/AppCenterIngestionTest.java index 0b94e88a3e..0a26d8ece5 100644 --- a/sdk/appcenter/src/test/java/com/microsoft/appcenter/ingestion/AppCenterIngestionTest.java +++ b/sdk/appcenter/src/test/java/com/microsoft/appcenter/ingestion/AppCenterIngestionTest.java @@ -74,8 +74,7 @@ public void setUp() throws Exception { doReturn(mHttpClient).when(HttpUtils.class, "createHttpClient", any(Context.class)); } - @Test - public void sendAsync() throws Exception { + private void sendAuthToken(String authToken) throws Exception { /* Build some payload. */ LogContainer container = new LogContainer(); @@ -102,7 +101,6 @@ public ServiceCall answer(InvocationOnMock invocation) { AppCenterIngestion ingestion = new AppCenterIngestion(mock(Context.class), serializer); ingestion.setLogUrl("http://mock"); String appSecret = UUID.randomUUID().toString(); - String authToken = UUID.randomUUID().toString(); UUID installId = UUID.randomUUID(); ServiceCallback serviceCallback = mock(ServiceCallback.class); assertEquals(call, ingestion.sendAsync(authToken, appSecret, installId, container, serviceCallback)); @@ -110,13 +108,14 @@ public ServiceCall answer(InvocationOnMock invocation) { /* Verify call to http client. */ HashMap expectedHeaders = new HashMap<>(); expectedHeaders.put(Constants.APP_SECRET, appSecret); - expectedHeaders.put(Constants.AUTHORIZATION_HEADER, String.format(Constants.AUTH_TOKEN_FORMAT, authToken)); + if (authToken != null) { + expectedHeaders.put(Constants.AUTHORIZATION_HEADER, String.format(Constants.AUTH_TOKEN_FORMAT, authToken)); + } expectedHeaders.put(AppCenterIngestion.INSTALL_ID, installId.toString()); verify(mHttpClient).callAsync(eq("http://mock" + AppCenterIngestion.API_PATH), eq(METHOD_POST), eq(expectedHeaders), notNull(HttpClient.CallTemplate.class), eq(serviceCallback)); assertNotNull(callTemplate.get()); assertEquals("mockPayload", callTemplate.get().buildRequestBody()); - assertEquals(authToken, authToken); - + /* Verify close. */ ingestion.close(); verify(mHttpClient).close(); @@ -126,6 +125,16 @@ public ServiceCall answer(InvocationOnMock invocation) { verify(mHttpClient).reopen(); } + @Test + public void sendAsyncWithoutAuthToken() throws Exception { + sendAuthToken(null); + } + + @Test + public void sendAsyncWithAuthToken() throws Exception { + sendAuthToken(UUID.randomUUID().toString()); + } + @Test public void failedSerialization() throws Exception { diff --git a/versions.gradle b/versions.gradle index ba7604a897..a3025a4b56 100644 --- a/versions.gradle +++ b/versions.gradle @@ -6,8 +6,8 @@ // Version constants ext { - versionCode = 49 - versionName = '2.4.0' + versionCode = 50 + versionName = '2.4.1' minSdkVersion = 16 targetSdkVersion = 29 compileSdkVersion = 29