From ce937498839ef0df90f69e567894794d25fe2ffa Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Tue, 1 May 2018 07:56:38 -0500 Subject: [PATCH 01/13] Prepare initial release of 6.6.1 --- CHANGELOG.md | 4 ++++ app/build.gradle | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86e67dbbb..2c5bca509 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ Release Notes ============= +Version 6.6.1 +------------- + * Bug fixes + Version 6.6 ----------- * More UI polish diff --git a/app/build.gradle b/app/build.gradle index f0b7cd9f9..ada0e02f3 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -11,8 +11,8 @@ buildscript { def versionMajor = 6 def versionMinor = 6 -def versionPatch = 0 -def versionBuild = 4 +def versionPatch = 1 +def versionBuild = 0 apply plugin: 'com.android.application' apply plugin: 'kotlin-android' From 4875515ba8ca2294f85c214db0c6d84ec2204e8e Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Tue, 1 May 2018 08:40:15 -0500 Subject: [PATCH 02/13] Handle null username or password in the AuthInterceptor --- app/src/main/java/com/boardgamegeek/io/AuthInterceptor.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/boardgamegeek/io/AuthInterceptor.kt b/app/src/main/java/com/boardgamegeek/io/AuthInterceptor.kt index a070d8b35..e68ea086e 100644 --- a/app/src/main/java/com/boardgamegeek/io/AuthInterceptor.kt +++ b/app/src/main/java/com/boardgamegeek/io/AuthInterceptor.kt @@ -22,7 +22,8 @@ class AuthInterceptor(private val context: Context?) : Interceptor { if (account != null) { try { val password = accountManager.blockingGetAuthToken(account, Authenticator.AUTH_TOKEN_TYPE, true) - if (account.name.isNotBlank() && password.isNotBlank()) { + if (account.name != null && account.name.isNotBlank() && + password != null && password.isNotBlank()) { val username = HttpUtils.encode(account.name) val cookieValue = "bggusername=$username; bggpassword=$password" val request = originalRequest.newBuilder().addHeader("Cookie", cookieValue).build() From a62cabc98c295fd13752cfdec046f85817b9665b Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Tue, 1 May 2018 19:46:22 -0500 Subject: [PATCH 03/13] Correct the notification message shown when a 500 error is received --- app/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ab114fe21..5f43c78ec 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -640,7 +640,7 @@ Sync has been remotely disabled Request to fetch %s is invalid Invalid %s received - There\'s a problem with BoardGameGeek. Be patient while they fix it. (HTTP code: %d) + There\'s a problem with BoardGameGeek. Be patient while they fix it. (HTTP code: %s) Rate limit exceeded. BoardGameGeek is still processing this request. Problem communicating with BoardGameGeek (HTTP code: %s) From 7481676403946da0a42b8f87f360de03b6b0e369 Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Tue, 1 May 2018 20:45:46 -0500 Subject: [PATCH 04/13] Apply a namespace to the notification tags/groups and create the big text style differently in the hope that automatic grouping on 7.0+ will work --- .../boardgamegeek/service/SyncUploadTask.kt | 3 +-- .../boardgamegeek/util/NotificationUtils.java | 21 ++++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/boardgamegeek/service/SyncUploadTask.kt b/app/src/main/java/com/boardgamegeek/service/SyncUploadTask.kt index c9bf3816c..9cca11397 100644 --- a/app/src/main/java/com/boardgamegeek/service/SyncUploadTask.kt +++ b/app/src/main/java/com/boardgamegeek/service/SyncUploadTask.kt @@ -66,8 +66,7 @@ abstract class SyncUploadTask(context: Context, service: BggService, syncResult: .setLargeIcon(largeIcon) .setOnlyAlertOnce(true) .setGroup(notificationMessageTag) - val detail = NotificationCompat.BigTextStyle(builder) - detail.bigText(message) + .setStyle(NotificationCompat.BigTextStyle().bigText(message)) val action = createMessageAction() if (action != null) { builder.addAction(action) diff --git a/app/src/main/java/com/boardgamegeek/util/NotificationUtils.java b/app/src/main/java/com/boardgamegeek/util/NotificationUtils.java index 8260ce784..5452243ee 100644 --- a/app/src/main/java/com/boardgamegeek/util/NotificationUtils.java +++ b/app/src/main/java/com/boardgamegeek/util/NotificationUtils.java @@ -22,16 +22,17 @@ import com.boardgamegeek.util.LargeIconLoader.Callback; public class NotificationUtils { - public static final String TAG_PLAY_STATS = "PLAY_STATS"; - public static final String TAG_PERSIST_ERROR = "PERSIST_ERROR"; - public static final String TAG_PLAY_TIMER = "PLAY_TIMER"; - public static final String TAG_PROVIDER_ERROR = "PROVIDER_ERROR"; - public static final String TAG_SYNC_PROGRESS = "SYNC_PROGRESS"; - public static final String TAG_SYNC_ERROR = "SYNC_ERROR"; - public static final String TAG_UPLOAD_PLAY = "UPLOAD_PLAY"; - public static final String TAG_UPLOAD_PLAY_ERROR = "UPLOAD_PLAY_ERROR"; - public static final String TAG_UPLOAD_COLLECTION = "UPLOAD_COLLECTION"; - public static final String TAG_UPLOAD_COLLECTION_ERROR = "UPLOAD_COLLECTION_ERROR"; + private static final String TAG_PREFIX = "com.boardgamegeek."; + public static final String TAG_PLAY_STATS = TAG_PREFIX + "PLAY_STATS"; + public static final String TAG_PERSIST_ERROR = TAG_PREFIX + "PERSIST_ERROR"; + public static final String TAG_PLAY_TIMER = TAG_PREFIX + "PLAY_TIMER"; + public static final String TAG_PROVIDER_ERROR = TAG_PREFIX + "PROVIDER_ERROR"; + public static final String TAG_SYNC_PROGRESS = TAG_PREFIX + "SYNC_PROGRESS"; + public static final String TAG_SYNC_ERROR = TAG_PREFIX + "SYNC_ERROR"; + public static final String TAG_UPLOAD_PLAY = TAG_PREFIX + "UPLOAD_PLAY"; + public static final String TAG_UPLOAD_PLAY_ERROR = TAG_PREFIX + "UPLOAD_PLAY_ERROR"; + public static final String TAG_UPLOAD_COLLECTION = TAG_PREFIX + "UPLOAD_COLLECTION"; + public static final String TAG_UPLOAD_COLLECTION_ERROR = TAG_PREFIX + "UPLOAD_COLLECTION_ERROR"; public static final String CHANNEL_ID_SYNC_PROGRESS = "sync"; public static final String CHANNEL_ID_ERROR = "sync_error"; From b9fea4bb728f965d16f5e25b683d975cc05a3174 Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Wed, 2 May 2018 21:27:15 -0500 Subject: [PATCH 05/13] In all cases, set the Crashlytics user identifier as a hash of the username, not the actual username --- app/src/main/java/com/boardgamegeek/auth/AccountUtils.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/boardgamegeek/auth/AccountUtils.java b/app/src/main/java/com/boardgamegeek/auth/AccountUtils.java index 19c8432d2..38ffaa788 100644 --- a/app/src/main/java/com/boardgamegeek/auth/AccountUtils.java +++ b/app/src/main/java/com/boardgamegeek/auth/AccountUtils.java @@ -4,6 +4,7 @@ import android.content.SharedPreferences; import android.preference.PreferenceManager; import android.support.annotation.Nullable; +import android.text.TextUtils; import com.crashlytics.android.Crashlytics; @@ -24,7 +25,8 @@ public static void clearFields(final Context context) { public static void setUsername(final Context context, final String username) { setString(context, username, KEY_USERNAME); - Crashlytics.setUserIdentifier(username); + if (!TextUtils.isEmpty(username)) + Crashlytics.setUserIdentifier(String.valueOf(username.hashCode())); } public static void setFullName(final Context context, String fullName) { From c54b46e7cd7d92f48c010484bdbd714e58c42467 Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Wed, 2 May 2018 21:34:27 -0500 Subject: [PATCH 06/13] When trying to construct a PlayMdoel and the cursor is invalid, return an object, don't throw an exception. --- .../com/boardgamegeek/ui/model/PlayModel.kt | 70 +++++++++++-------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/com/boardgamegeek/ui/model/PlayModel.kt b/app/src/main/java/com/boardgamegeek/ui/model/PlayModel.kt index 2ad01986b..fffda5d1a 100644 --- a/app/src/main/java/com/boardgamegeek/ui/model/PlayModel.kt +++ b/app/src/main/java/com/boardgamegeek/ui/model/PlayModel.kt @@ -2,29 +2,34 @@ package com.boardgamegeek.ui.model import android.content.Context import android.database.Cursor +import android.database.CursorIndexOutOfBoundsException +import com.boardgamegeek.provider.BggContract import com.boardgamegeek.provider.BggContract.Games import com.boardgamegeek.provider.BggContract.Plays import com.boardgamegeek.util.CursorUtils +import timber.log.Timber class PlayModel( val playId: Int, val gameId: Int, - val name: String?, - val date: String?, - val location: String?, - val quantity: Int, - val length: Int, - val playerCount: Int, - val comments: String?, - val thumbnailUrl: String?, - val imageUrl: String?, - val heroImageUrl: String?, - val deleteTimestamp: Long, - val updateTimestamp: Long, - val dirtyTimestamp: Long + val name: String = "", + val date: String = "", + val location: String = "", + val quantity: Int = 1, + val length: Int = 0, + val playerCount: Int = 0, + val comments: String = "", + val thumbnailUrl: String = "", + val imageUrl: String = "", + val heroImageUrl: String = "", + val deleteTimestamp: Long = 0, + val updateTimestamp: Long = 0, + val dirtyTimestamp: Long = 0 ) { + private constructor() : this(BggContract.INVALID_ID, BggContract.INVALID_ID) + companion object { @JvmStatic val projection = arrayOf( @@ -64,23 +69,28 @@ class PlayModel( @JvmStatic fun fromCursor(cursor: Cursor, context: Context): PlayModel { - return PlayModel( - cursor.getInt(PLAY_ID), - cursor.getInt(GAME_ID), - cursor.getString(GAME_NAME), - CursorUtils.getFormattedDateAbbreviated(cursor, context, DATE), - cursor.getString(LOCATION), - cursor.getInt(QUANTITY), - cursor.getInt(LENGTH), - cursor.getInt(PLAYER_COUNT), - CursorUtils.getString(cursor, COMMENTS).trim(), - cursor.getString(THUMBNAIL_URL) ?: "", - cursor.getString(IMAGE_URL) ?: "", - cursor.getString(HERO_IMAGE_URL) ?: "", - cursor.getLong(DELETE_TIMESTAMP), - cursor.getLong(UPDATE_TIMESTAMP), - cursor.getLong(DIRTY_TIMESTAMP) - ) + try { + return PlayModel( + cursor.getInt(PLAY_ID), + cursor.getInt(GAME_ID), + cursor.getString(GAME_NAME) ?: "", + CursorUtils.getFormattedDateAbbreviated(cursor, context, DATE), + cursor.getString(LOCATION) ?: "", + cursor.getInt(QUANTITY), + cursor.getInt(LENGTH), + cursor.getInt(PLAYER_COUNT), + (cursor.getString(COMMENTS) ?: "").trim(), + cursor.getString(THUMBNAIL_URL) ?: "", + cursor.getString(IMAGE_URL) ?: "", + cursor.getString(HERO_IMAGE_URL) ?: "", + cursor.getLong(DELETE_TIMESTAMP), + cursor.getLong(UPDATE_TIMESTAMP), + cursor.getLong(DIRTY_TIMESTAMP) + ) + } catch (e: CursorIndexOutOfBoundsException) { + Timber.w(e) + return PlayModel() + } } } } From 6e4dadb490cc8cb400fc665f9196ba122adcfb65 Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Wed, 2 May 2018 21:43:35 -0500 Subject: [PATCH 07/13] Sync notification indicate when they are sleeping --- CHANGELOG.md | 1 + .../service/SyncBuddiesDetail.kt | 6 +- .../service/SyncCollectionComplete.kt | 7 +- .../service/SyncCollectionUpload.kt | 8 +- .../boardgamegeek/service/SyncPlaysUpload.kt | 73 ++++++++++--------- .../com/boardgamegeek/service/SyncTask.kt | 29 ++++---- .../main/res/values/strings_sync_service.xml | 17 ++++- 7 files changed, 79 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c5bca509..9d0cf07f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Release Notes Version 6.6.1 ------------- + * Improved sync notifications * Bug fixes Version 6.6 diff --git a/app/src/main/java/com/boardgamegeek/service/SyncBuddiesDetail.kt b/app/src/main/java/com/boardgamegeek/service/SyncBuddiesDetail.kt index 89a0a0883..640081283 100644 --- a/app/src/main/java/com/boardgamegeek/service/SyncBuddiesDetail.kt +++ b/app/src/main/java/com/boardgamegeek/service/SyncBuddiesDetail.kt @@ -12,7 +12,7 @@ import timber.log.Timber import java.io.IOException abstract class SyncBuddiesDetail(context: Context, service: BggService, syncResult: SyncResult) : SyncTask(context, service, syncResult) { - private var notificationMessage: String? = null + private var notificationMessage: String = "" /** * Returns a log message to use for debugging purposes. @@ -71,13 +71,13 @@ abstract class SyncBuddiesDetail(context: Context, service: BggService, syncResu val call = service.user(name) val response = call.execute() if (!response.isSuccessful) { - showError(notificationMessage!!, response.code()) + showError(notificationMessage, response.code()) syncResult.stats.numIoExceptions++ cancel() } user = response.body() } catch (e: IOException) { - showError(notificationMessage!!, e) + showError(notificationMessage, e) syncResult.stats.numIoExceptions++ cancel() } diff --git a/app/src/main/java/com/boardgamegeek/service/SyncCollectionComplete.kt b/app/src/main/java/com/boardgamegeek/service/SyncCollectionComplete.kt index 3ea7962e2..c8aacfe92 100644 --- a/app/src/main/java/com/boardgamegeek/service/SyncCollectionComplete.kt +++ b/app/src/main/java/com/boardgamegeek/service/SyncCollectionComplete.kt @@ -18,6 +18,7 @@ import hugo.weaving.DebugLog import timber.log.Timber import java.io.IOException import java.util.* +import java.util.concurrent.TimeUnit import kotlin.collections.set /** @@ -79,15 +80,13 @@ class SyncCollectionComplete(context: Context, service: BggService, syncResult: Timber.i("...cancelled") return } - updateProgressNotification() - if (wasSleepInterrupted(5000)) return + if (wasSleepInterrupted(5, TimeUnit.SECONDS)) return } val excludedStatuses = (0 until i).map { statuses[it] } syncByStatus("", statuses[i], *excludedStatuses.toTypedArray()) - updateProgressNotification() - if (wasSleepInterrupted(5000)) return + if (wasSleepInterrupted(5, TimeUnit.SECONDS)) return syncByStatus(BggService.THING_SUBTYPE_BOARDGAME_ACCESSORY, statuses[i], *excludedStatuses.toTypedArray()) } diff --git a/app/src/main/java/com/boardgamegeek/service/SyncCollectionUpload.kt b/app/src/main/java/com/boardgamegeek/service/SyncCollectionUpload.kt index 100e73825..986daf3c4 100644 --- a/app/src/main/java/com/boardgamegeek/service/SyncCollectionUpload.kt +++ b/app/src/main/java/com/boardgamegeek/service/SyncCollectionUpload.kt @@ -25,6 +25,7 @@ import okhttp3.OkHttpClient import org.jetbrains.anko.intentFor import timber.log.Timber import java.util.* +import java.util.concurrent.TimeUnit class SyncCollectionUpload(context: Context, service: BggService, syncResult: SyncResult) : SyncUploadTask(context, service, syncResult) { private val okHttpClient: OkHttpClient = HttpUtils.getHttpClientWithAuth(context) @@ -78,27 +79,24 @@ class SyncCollectionUpload(context: Context, service: BggService, syncResult: Sy processDeletedCollectionItem(c) } } - NotificationUtils.cancel(context, NotificationUtils.TAG_SYNC_PROGRESS) val newItemsCursor = fetchNewCollectionItems() newItemsCursor?.use { c -> while (c.moveToNext()) { if (isCancelled) break - if (wasSleepInterrupted(1000)) break + if (wasSleepInterrupted(1, TimeUnit.SECONDS)) break processNewCollectionItem(c) } } - NotificationUtils.cancel(context, NotificationUtils.TAG_SYNC_PROGRESS) val dirtyItemsCursor = fetchDirtyCollectionItems() dirtyItemsCursor?.use { c -> while (c.moveToNext()) { if (isCancelled) break - if (wasSleepInterrupted(1000)) break + if (wasSleepInterrupted(1, TimeUnit.SECONDS)) break processDirtyCollectionItem(c) } } - NotificationUtils.cancel(context, NotificationUtils.TAG_SYNC_PROGRESS) } private fun fetchDeletedCollectionItems(): Cursor? { diff --git a/app/src/main/java/com/boardgamegeek/service/SyncPlaysUpload.kt b/app/src/main/java/com/boardgamegeek/service/SyncPlaysUpload.kt index b455d1f29..abf4f49d9 100644 --- a/app/src/main/java/com/boardgamegeek/service/SyncPlaysUpload.kt +++ b/app/src/main/java/com/boardgamegeek/service/SyncPlaysUpload.kt @@ -30,6 +30,7 @@ import hugo.weaving.DebugLog import okhttp3.FormBody import okhttp3.Request.Builder import org.jetbrains.anko.intentFor +import java.util.concurrent.TimeUnit class SyncPlaysUpload(context: Context, service: BggService, syncResult: SyncResult) : SyncUploadTask(context, service, syncResult) { private val httpClient = HttpUtils.getHttpClientWithAuth(context) @@ -92,12 +93,15 @@ class SyncPlaysUpload(context: Context, service: BggService, syncResult: SyncRes null, Plays.UPDATE_TIMESTAMP) cursor?.use { c -> - val playCount = c.count - updateProgressNotificationAsPlural(R.plurals.sync_notification_progress_update, playCount, playCount) + var currentNumberOfPlays = 0 + val totalNumberOfPlays = c.count + updateProgressNotificationAsPlural(R.plurals.sync_notification_plays_update, totalNumberOfPlays, totalNumberOfPlays) while (c.moveToNext()) { if (isCancelled) break - if (wasSleepInterrupted(1000)) break + if (wasSleepInterrupted(1, TimeUnit.SECONDS, false)) break + + updateProgressNotificationAsPlural(R.plurals.sync_notification_plays_update_increment, totalNumberOfPlays, ++currentNumberOfPlays, totalNumberOfPlays) val internalId = CursorUtils.getLong(c, Plays._ID, BggContract.INVALID_ID.toLong()) val play = PlayBuilder.fromCursor(c) @@ -132,7 +136,7 @@ class SyncPlaysUpload(context: Context, service: BggService, syncResult: SyncRes play.updateTimestamp = 0 play.deleteTimestamp = 0 currentPlay = PlayForNotification(internalId, play.gameId, play.gameName) - + notifyUser(play, message) persister.save(play, internalId, false) @@ -142,30 +146,6 @@ class SyncPlaysUpload(context: Context, service: BggService, syncResult: SyncRes } } - private fun notifyUser(play: Play, message: CharSequence) { - if (play.gameId != BggContract.INVALID_ID) { - val gameCursor = context.contentResolver.query( - Games.buildGameUri(play.gameId), - arrayOf(Games.IMAGE_URL, Games.THUMBNAIL_URL, Games.HERO_IMAGE_URL), - null, - null, - null) - gameCursor?.use { c -> - if (c.moveToFirst()) { - currentPlay.imageUrl = c.getString(0) ?: "" - currentPlay.thumbnailUrl = c.getString(1) ?: "" - currentPlay.heroImageUrl = c.getString(2) ?: "" - } - } - } - notifyUser(play.gameName, - message, - NotificationUtils.getIntegerId(currentPlay.internalId), - currentPlay.imageUrl, - currentPlay.thumbnailUrl, - currentPlay.heroImageUrl) - } - @DebugLog private fun deletePendingPlays() { val cursor = context.contentResolver.query(Plays.CONTENT_SIMPLE_URI, @@ -174,16 +154,20 @@ class SyncPlaysUpload(context: Context, service: BggService, syncResult: SyncRes null, Plays.DELETE_TIMESTAMP) cursor?.use { c -> - val playCount = c.count - updateProgressNotificationAsPlural(R.plurals.sync_notification_progress_delete, playCount, playCount) + var currentNumberOfPlays = 0 + val totalNumberOfPlays = c.count + updateProgressNotificationAsPlural(R.plurals.sync_notification_plays_delete, totalNumberOfPlays, totalNumberOfPlays) while (c.moveToNext()) { if (isCancelled) break + if (wasSleepInterrupted(1, TimeUnit.SECONDS, false)) break + + updateProgressNotificationAsPlural(R.plurals.sync_notification_plays_delete_increment, totalNumberOfPlays, ++currentNumberOfPlays, totalNumberOfPlays) + val play = PlayBuilder.fromCursor(c) val internalId = CursorUtils.getLong(c, Plays._ID, BggContract.INVALID_ID.toLong()) currentPlay = PlayForNotification(internalId, play.gameId, play.gameName) if (play.playId > 0) { - if (wasSleepInterrupted(1000)) break val response = postPlayDelete(play.playId) if (response.isSuccessful) { syncResult.stats.numDeletes++ @@ -288,12 +272,10 @@ class SyncPlaysUpload(context: Context, service: BggService, syncResult: SyncRes /** * Deletes the specified play from the content provider */ - @DebugLog private fun deletePlay(internalId: Long) { persister.delete(internalId) } - @DebugLog private fun getPlayCountDescription(count: Int, quantity: Int): String { return when (quantity) { 1 -> StringUtils.getOrdinal(count) @@ -302,7 +284,6 @@ class SyncPlaysUpload(context: Context, service: BggService, syncResult: SyncRes } } - @DebugLog private fun notifyUserOfDelete(@StringRes messageId: Int, play: Play) { NotificationUtils.cancel(context, notificationMessageTag, @@ -311,6 +292,30 @@ class SyncPlaysUpload(context: Context, service: BggService, syncResult: SyncRes notifyUser(play, PresentationUtils.getText(context, messageId, play.gameName)) } + private fun notifyUser(play: Play, message: CharSequence) { + if (play.gameId != BggContract.INVALID_ID) { + val gameCursor = context.contentResolver.query( + Games.buildGameUri(play.gameId), + arrayOf(Games.IMAGE_URL, Games.THUMBNAIL_URL, Games.HERO_IMAGE_URL), + null, + null, + null) + gameCursor?.use { c -> + if (c.moveToFirst()) { + currentPlay.imageUrl = c.getString(0) ?: "" + currentPlay.thumbnailUrl = c.getString(1) ?: "" + currentPlay.heroImageUrl = c.getString(2) ?: "" + } + } + } + notifyUser(play.gameName, + message, + NotificationUtils.getIntegerId(currentPlay.internalId), + currentPlay.imageUrl, + currentPlay.thumbnailUrl, + currentPlay.heroImageUrl) + } + @DebugLog override fun createMessageAction(): Action? { if (currentPlay.internalId != BggContract.INVALID_ID.toLong()) { diff --git a/app/src/main/java/com/boardgamegeek/service/SyncTask.kt b/app/src/main/java/com/boardgamegeek/service/SyncTask.kt index cacccccff..5e7be0e0c 100644 --- a/app/src/main/java/com/boardgamegeek/service/SyncTask.kt +++ b/app/src/main/java/com/boardgamegeek/service/SyncTask.kt @@ -8,7 +8,6 @@ import android.support.annotation.PluralsRes import android.support.v4.app.NotificationCompat import android.support.v4.app.NotificationCompat.BigTextStyle import android.text.TextUtils - import com.boardgamegeek.R import com.boardgamegeek.io.BggService import com.boardgamegeek.util.NotificationUtils @@ -16,8 +15,8 @@ import com.boardgamegeek.util.PreferencesUtils import com.boardgamegeek.util.PresentationUtils import com.boardgamegeek.util.fabric.CrashKeys import com.crashlytics.android.Crashlytics - import timber.log.Timber +import java.util.concurrent.TimeUnit abstract class SyncTask(protected val context: Context, protected val service: BggService, protected val syncResult: SyncResult) { /** @@ -108,18 +107,15 @@ abstract class SyncTask(protected val context: Context, protected val service: B * existing error notification. */ private fun showError(detailMessage: String, errorMessage: String) { - Timber.w("$detailMessage\n$errorMessage") + Timber.w("$detailMessage\n$errorMessage".trim()) if (!PreferencesUtils.getSyncShowErrors(context)) return - val contentMessage = if (notificationSummaryMessageId == NO_NOTIFICATION) - detailMessage - else - context.getString(notificationSummaryMessageId) - val bigText = if (notificationSummaryMessageId == NO_NOTIFICATION) - errorMessage - else - detailMessage + "\n" + errorMessage + val contentMessage = if (notificationSummaryMessageId == NO_NOTIFICATION) detailMessage + else context.getString(notificationSummaryMessageId) + + val bigText = if (notificationSummaryMessageId == NO_NOTIFICATION) errorMessage + else (detailMessage + "\n" + errorMessage).trim() val builder = NotificationUtils .createNotificationBuilder(context, R.string.sync_notification_title_error, NotificationUtils.CHANNEL_ID_ERROR) @@ -137,12 +133,17 @@ abstract class SyncTask(protected val context: Context, protected val service: B * Sleep for the specified number of milliseconds. Returns true if thread was interrupted. This typically means the * task should stop processing. */ - protected fun wasSleepInterrupted(millis: Long): Boolean { + protected fun wasSleepInterrupted(duration: Long, timeUnit: TimeUnit = TimeUnit.MILLISECONDS, showNotification: Boolean = true): Boolean { try { - Timber.d("Sleeping for %,d millis", millis) - Thread.sleep(millis) + Timber.d("Sleeping for %,d millis", timeUnit.toMillis(duration)) + if (showNotification) { + val durationSeconds = timeUnit.toSeconds(duration).toInt() + updateProgressNotification(context.resources.getQuantityString(R.plurals.sync_notification_collection_sleeping, durationSeconds, durationSeconds)) + } + timeUnit.sleep(duration) } catch (e: InterruptedException) { Timber.w(e, "Sleeping interrupted during sync.") + NotificationUtils.cancel(context, NotificationUtils.TAG_SYNC_PROGRESS) return true } diff --git a/app/src/main/res/values/strings_sync_service.xml b/app/src/main/res/values/strings_sync_service.xml index c9923d4e9..f1fa8eaa9 100644 --- a/app/src/main/res/values/strings_sync_service.xml +++ b/app/src/main/res/values/strings_sync_service.xml @@ -51,14 +51,22 @@ Uploading %,d collection item. Uploading %,d collection items. - + Updating %,d play Updating %,d plays - + + Updating %,d of %,d play + Updating %,d of %,d plays + + Deleting %,d play Deleting %,d plays + + Deleting %,d of %,d play + Deleting %, of %,d plays + Syncing %1$,d game: %2$s Syncing %1$,d games: %2$s @@ -67,4 +75,9 @@ Deleting %,d game from your collection Deleting %,d games from your collection + + Taking a break for a second + Taking a break for %,d seconds + + \ No newline at end of file From 95e19461ab9c66ce91a5a36c22b61a48f08e8535 Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Thu, 3 May 2018 22:24:36 -0500 Subject: [PATCH 08/13] Restore missing checkboxes for collection statuses; reorder statuses so "own" is first --- CHANGELOG.md | 1 + .../layout/fragment_game_collection_item.xml | 32 ++++++++----------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d0cf07f1..25bbb3465 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Release Notes Version 6.6.1 ------------- + * Restore missing checkboxes for collection statuses * Improved sync notifications * Bug fixes diff --git a/app/src/main/res/layout/fragment_game_collection_item.xml b/app/src/main/res/layout/fragment_game_collection_item.xml index 8738f4a39..6594884b1 100644 --- a/app/src/main/res/layout/fragment_game_collection_item.xml +++ b/app/src/main/res/layout/fragment_game_collection_item.xml @@ -60,12 +60,12 @@ android:visibility="gone"> + android:tag="own" + android:text="@string/collection_status_own"/> + android:tag="previously_owned" + android:text="@string/collection_status_prev_owned"/> + android:tag="want_to_buy" + android:text="@string/collection_status_want_to_buy"/> + android:tag="want_to_play" + android:text="@string/collection_status_want_to_play"/> + android:text="@string/collection_status_want_in_trade"/> + android:text="@string/collection_status_for_trade"/> Date: Thu, 3 May 2018 22:25:07 -0500 Subject: [PATCH 09/13] Consider exceptions thrown while loading top games as warnings, not errors --- app/src/main/java/com/boardgamegeek/ui/TopGamesFragment.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/boardgamegeek/ui/TopGamesFragment.java b/app/src/main/java/com/boardgamegeek/ui/TopGamesFragment.java index 02f4f2404..30389deff 100644 --- a/app/src/main/java/com/boardgamegeek/ui/TopGamesFragment.java +++ b/app/src/main/java/com/boardgamegeek/ui/TopGamesFragment.java @@ -100,7 +100,7 @@ public void onSuccess(List topGames) { @Override public void onError(Throwable error) { - Timber.e(error, "Error loading top games"); + Timber.w(error, "Error loading top games"); if (!isAdded()) return; if (emptyView != null) { emptyView.setText(getString(R.string.empty_http_error, error.getLocalizedMessage())); From 784aa00734263b632396400059478e75e49d9f6b Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Thu, 3 May 2018 23:02:37 -0500 Subject: [PATCH 10/13] Clean up the buddy sync logic, doing away with batched saves --- .../java/com/boardgamegeek/model/Buddy.java | 21 +--- .../java/com/boardgamegeek/model/User.java | 11 +-- .../model/persister/BuddyPersister.java | 95 ++++++------------- .../service/SyncBuddiesDetail.kt | 16 ++-- .../boardgamegeek/service/SyncBuddiesList.kt | 19 ++-- 5 files changed, 50 insertions(+), 112 deletions(-) diff --git a/app/src/main/java/com/boardgamegeek/model/Buddy.java b/app/src/main/java/com/boardgamegeek/model/Buddy.java index d9ad67772..ccda650ba 100644 --- a/app/src/main/java/com/boardgamegeek/model/Buddy.java +++ b/app/src/main/java/com/boardgamegeek/model/Buddy.java @@ -1,27 +1,10 @@ package com.boardgamegeek.model; -import com.boardgamegeek.provider.BggContract; -import com.boardgamegeek.util.StringUtils; - import org.simpleframework.xml.Attribute; import org.simpleframework.xml.Root; @Root(name = "buddy") public class Buddy { - @Attribute - private String id; - - @Attribute - public String name; - - public int getId() { - return StringUtils.parseInt(id, BggContract.INVALID_ID); - } - - public static Buddy fromUser(User user) { - Buddy buddy = new Buddy(); - buddy.id = String.valueOf(user.getId()); - buddy.name = user.name; - return buddy; - } + @Attribute public String id; + @Attribute public String name; } diff --git a/app/src/main/java/com/boardgamegeek/model/User.java b/app/src/main/java/com/boardgamegeek/model/User.java index 959050bd2..e74de5231 100644 --- a/app/src/main/java/com/boardgamegeek/model/User.java +++ b/app/src/main/java/com/boardgamegeek/model/User.java @@ -7,8 +7,6 @@ import org.simpleframework.xml.Element; import org.simpleframework.xml.Path; -import java.util.ArrayList; - public class User { @Attribute private String id; @@ -73,16 +71,9 @@ public class User { private String tradeRating; @Element(required = false) - private Buddies buddies; + public Buddies buddies; public int getId() { return StringUtils.parseInt(id, BggContract.INVALID_ID); } - - public ArrayList getBuddies() { - if (buddies == null || buddies.buddies == null) { - return new ArrayList<>(); - } - return new ArrayList<>(buddies.buddies); - } } diff --git a/app/src/main/java/com/boardgamegeek/model/persister/BuddyPersister.java b/app/src/main/java/com/boardgamegeek/model/persister/BuddyPersister.java index 2108dfa78..6f6c2b0bf 100644 --- a/app/src/main/java/com/boardgamegeek/model/persister/BuddyPersister.java +++ b/app/src/main/java/com/boardgamegeek/model/persister/BuddyPersister.java @@ -1,14 +1,11 @@ package com.boardgamegeek.model.persister; -import android.content.ContentProviderOperation; -import android.content.ContentProviderResult; import android.content.ContentResolver; import android.content.ContentValues; import android.content.Context; import android.net.Uri; import android.text.TextUtils; -import com.boardgamegeek.model.Buddy; import com.boardgamegeek.model.User; import com.boardgamegeek.provider.BggContract; import com.boardgamegeek.provider.BggContract.Avatars; @@ -16,9 +13,6 @@ import com.boardgamegeek.util.FileUtils; import com.boardgamegeek.util.ResolverUtils; -import java.util.ArrayList; -import java.util.List; - import timber.log.Timber; public class BuddyPersister { @@ -39,14 +33,10 @@ public void resetTimestamp() { } public int saveUser(User buddy) { - ContentResolver resolver = context.getContentResolver(); - ArrayList batch = new ArrayList<>(); - StringBuilder debugMessage = new StringBuilder(); if (buddy != null && !TextUtils.isEmpty(buddy.name)) { - Uri uri = Buddies.buildBuddyUri(buddy.name); ContentValues values = new ContentValues(); values.put(Buddies.UPDATED, updateTime); - int oldSyncHashCode = ResolverUtils.queryInt(resolver, uri, Buddies.SYNC_HASH_CODE); + int oldSyncHashCode = ResolverUtils.queryInt(context.getContentResolver(), Buddies.buildBuddyUri(buddy.name), Buddies.SYNC_HASH_CODE); int newSyncHashCode = generateSyncHashCode(buddy); if (oldSyncHashCode != newSyncHashCode) { values.put(Buddies.BUDDY_ID, buddy.getId()); @@ -56,74 +46,48 @@ public int saveUser(User buddy) { values.put(Buddies.AVATAR_URL, buddy.avatarUrl); values.put(Buddies.SYNC_HASH_CODE, newSyncHashCode); } - debugMessage.append("Saving ").append(uri).append("; "); - addToBatch(resolver, values, batch, uri, debugMessage); + return upsert(values); } - ContentProviderResult[] result = ResolverUtils.applyBatch(context, batch, debugMessage.toString()); - return result == null ? 0 : result.length; + return 0; } - public int saveBuddy(Buddy buddy) { - ContentResolver resolver = context.getContentResolver(); - if (buddy.getId() != BggContract.INVALID_ID && !TextUtils.isEmpty(buddy.name)) { - Uri uri = Buddies.buildBuddyUri(buddy.name); - ContentValues values = toValues(buddy); - if (!ResolverUtils.rowExists(resolver, uri)) { - Uri insertedUri = resolver.insert(Buddies.CONTENT_URI, values); - Timber.d("Inserted buddy at %s", insertedUri); - return 1; - } else { - int count = resolver.update(uri, values, null, null); - Timber.d("Updated %,d buddy at %s", count, uri); - return count; - } + public int saveBuddy(int userId, String username, boolean isBuddy) { + if (userId != BggContract.INVALID_ID && !TextUtils.isEmpty(username)) { + ContentValues values = new ContentValues(); + values.put(Buddies.BUDDY_ID, userId); + values.put(Buddies.BUDDY_NAME, username); + values.put(Buddies.BUDDY_FLAG, isBuddy ? 1 : 0); + values.put(Buddies.UPDATED_LIST, updateTime); + + return upsert(values); + } else { + Timber.i("Un-savable buddy %s (%d)", username, userId); } return 0; } - public int saveBuddies(List buddies) { + private int upsert(ContentValues values) { ContentResolver resolver = context.getContentResolver(); - ArrayList batch = new ArrayList<>(); - StringBuilder debugMessage = new StringBuilder(); - if (buddies != null) { - for (Buddy buddy : buddies) { - if (buddy.getId() != BggContract.INVALID_ID && !TextUtils.isEmpty(buddy.name)) { - ContentValues values = toValues(buddy); - addToBatch(resolver, values, batch, Buddies.buildBuddyUri(buddy.name), debugMessage); - } - } - } - ContentProviderResult[] result = ResolverUtils.applyBatch(context, batch, debugMessage.toString()); - return result == null ? 0 : result.length; - } - - private void addToBatch(ContentResolver resolver, ContentValues values, ArrayList batch, Uri uri, StringBuilder debugMessage) { - if (!ResolverUtils.rowExists(resolver, uri)) { - debugMessage.append("Inserting ").append(uri).append("; "); - values.put(Buddies.UPDATED_LIST, updateTime); - batch.add(ContentProviderOperation.newInsert(Buddies.CONTENT_URI).withValues(values).build()); - } else { - debugMessage.append("Updating ").append(uri).append("; "); - maybeDeleteAvatar(values, uri, resolver); + String username = values.getAsString(Buddies.BUDDY_NAME); + Uri uri = Buddies.buildBuddyUri(username); + if (ResolverUtils.rowExists(resolver, uri)) { values.remove(Buddies.BUDDY_NAME); - batch.add(ContentProviderOperation.newUpdate(uri).withValues(values).build()); + int count = resolver.update(uri, values, null, null); + Timber.d("Updated %,d buddy rows at %s", count, uri); + maybeDeleteAvatar(values, uri); + return count; + } else { + Uri insertedUri = resolver.insert(Buddies.CONTENT_URI, values); + Timber.d("Inserted buddy at %s", insertedUri); + return 1; } } - private ContentValues toValues(Buddy buddy) { - ContentValues values = new ContentValues(); - values.put(Buddies.BUDDY_ID, buddy.getId()); - values.put(Buddies.BUDDY_NAME, buddy.name); - values.put(Buddies.UPDATED_LIST, updateTime); - values.put(Buddies.BUDDY_FLAG, 1); - return values; - } - private static int generateSyncHashCode(User buddy) { return (buddy.firstName + "\n" + buddy.lastName + "\n" + buddy.avatarUrl + "\n").hashCode(); } - private static void maybeDeleteAvatar(ContentValues values, Uri uri, ContentResolver resolver) { + private void maybeDeleteAvatar(ContentValues values, Uri uri) { if (!values.containsKey(Buddies.AVATAR_URL)) { // nothing to do - no avatar return; @@ -134,7 +98,7 @@ private static void maybeDeleteAvatar(ContentValues values, Uri uri, ContentReso newAvatarUrl = ""; } - String oldAvatarUrl = ResolverUtils.queryString(resolver, uri, Buddies.AVATAR_URL); + String oldAvatarUrl = ResolverUtils.queryString(context.getContentResolver(), uri, Buddies.AVATAR_URL); if (newAvatarUrl.equals(oldAvatarUrl)) { // nothing to do - avatar hasn't changed return; @@ -142,8 +106,7 @@ private static void maybeDeleteAvatar(ContentValues values, Uri uri, ContentReso String avatarFileName = FileUtils.getFileNameFromUrl(oldAvatarUrl); if (!TextUtils.isEmpty(avatarFileName)) { - // TODO: use batch - resolver.delete(Avatars.buildUri(avatarFileName), null, null); + context.getContentResolver().delete(Avatars.buildUri(avatarFileName), null, null); } } } diff --git a/app/src/main/java/com/boardgamegeek/service/SyncBuddiesDetail.kt b/app/src/main/java/com/boardgamegeek/service/SyncBuddiesDetail.kt index 640081283..b9e06d0d9 100644 --- a/app/src/main/java/com/boardgamegeek/service/SyncBuddiesDetail.kt +++ b/app/src/main/java/com/boardgamegeek/service/SyncBuddiesDetail.kt @@ -54,7 +54,7 @@ abstract class SyncBuddiesDetail(context: Context, service: BggService, syncResu syncResult.stats.numUpdates++ count++ - if (wasSleepInterrupted(fetchPauseMillis)) break + if (wasSleepInterrupted(fetchPauseMillis, showNotification = false)) break } } else { Timber.i("...no buddies to update") @@ -66,22 +66,20 @@ abstract class SyncBuddiesDetail(context: Context, service: BggService, syncResu } private fun requestUser(name: String): User? { - var user: User? = null try { val call = service.user(name) val response = call.execute() - if (!response.isSuccessful) { - showError(notificationMessage, response.code()) - syncResult.stats.numIoExceptions++ - cancel() - } - user = response.body() + if (response.isSuccessful) return response.body() + showError(notificationMessage, response.code()) + syncResult.stats.numIoExceptions++ + cancel() + return response.body() } catch (e: IOException) { showError(notificationMessage, e) syncResult.stats.numIoExceptions++ cancel() } - return user + return null } } diff --git a/app/src/main/java/com/boardgamegeek/service/SyncBuddiesList.kt b/app/src/main/java/com/boardgamegeek/service/SyncBuddiesList.kt index 0509c7644..ba9870ad8 100644 --- a/app/src/main/java/com/boardgamegeek/service/SyncBuddiesList.kt +++ b/app/src/main/java/com/boardgamegeek/service/SyncBuddiesList.kt @@ -12,11 +12,9 @@ import com.boardgamegeek.model.Buddy import com.boardgamegeek.model.User import com.boardgamegeek.model.persister.BuddyPersister import com.boardgamegeek.pref.SyncPrefs +import com.boardgamegeek.provider.BggContract import com.boardgamegeek.provider.BggContract.Buddies -import com.boardgamegeek.util.DateTimeUtils -import com.boardgamegeek.util.PreferencesUtils -import com.boardgamegeek.util.PresentationUtils -import com.boardgamegeek.util.RemoteConfig +import com.boardgamegeek.util.* import timber.log.Timber import java.io.IOException @@ -67,7 +65,7 @@ class SyncBuddiesList(context: Context, service: BggService, syncResult: SyncRes } private fun updateNotification(@StringRes detailResId: Int) { - currentDetailResId = R.string.sync_notification_buddies_list_downloading + currentDetailResId = detailResId updateProgressNotification(context.getString(detailResId)) } @@ -99,9 +97,14 @@ class SyncBuddiesList(context: Context, service: BggService, syncResult: SyncRes } private fun persistUser(user: User) { - var count = 0 - count += persister.saveBuddy(Buddy.fromUser(user)) - count += persister.saveBuddies(user.buddies) + var count = persister.saveBuddy(user.id, user.name, false) + + val buddies = user.buddies?.buddies ?: listOf() + for (buddy in buddies) { + val userId = StringUtils.parseInt(buddy.id, BggContract.INVALID_ID) + count += persister.saveBuddy(userId, buddy.name, true) + } + syncResult.stats.numEntries += count.toLong() Timber.i("Synced %,d buddies", count) } From 8cde29711acc6ea699fb405926da599df5e9d29f Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Fri, 4 May 2018 21:23:01 -0500 Subject: [PATCH 11/13] Set version build to 1 for RC --- app/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle b/app/build.gradle index ada0e02f3..1739d385c 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -12,7 +12,7 @@ buildscript { def versionMajor = 6 def versionMinor = 6 def versionPatch = 1 -def versionBuild = 0 +def versionBuild = 1 apply plugin: 'com.android.application' apply plugin: 'kotlin-android' From 9f1727d16dd23b4ed249327d976cf0a9bfa4003a Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Sat, 5 May 2018 20:30:31 -0500 Subject: [PATCH 12/13] Change import/export finished event error messages to nullable --- .../main/java/com/boardgamegeek/events/ExportFinishedEvent.kt | 2 +- .../main/java/com/boardgamegeek/events/ImportFinishedEvent.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/boardgamegeek/events/ExportFinishedEvent.kt b/app/src/main/java/com/boardgamegeek/events/ExportFinishedEvent.kt index f65635084..6d6561209 100644 --- a/app/src/main/java/com/boardgamegeek/events/ExportFinishedEvent.kt +++ b/app/src/main/java/com/boardgamegeek/events/ExportFinishedEvent.kt @@ -1,3 +1,3 @@ package com.boardgamegeek.events -class ExportFinishedEvent(val type: String, val errorMessage: String) +class ExportFinishedEvent(val type: String, val errorMessage: String?) diff --git a/app/src/main/java/com/boardgamegeek/events/ImportFinishedEvent.kt b/app/src/main/java/com/boardgamegeek/events/ImportFinishedEvent.kt index 0b0e02f73..a1c65ce77 100644 --- a/app/src/main/java/com/boardgamegeek/events/ImportFinishedEvent.kt +++ b/app/src/main/java/com/boardgamegeek/events/ImportFinishedEvent.kt @@ -1,3 +1,3 @@ package com.boardgamegeek.events -class ImportFinishedEvent(val type: String, val errorMessage: String) +class ImportFinishedEvent(val type: String, val errorMessage: String?) From 3a931aedb6c55ca475517d2c9268c08ecb95fa59 Mon Sep 17 00:00:00 2001 From: Chris Comeaux Date: Sat, 5 May 2018 20:31:41 -0500 Subject: [PATCH 13/13] Set version build to 2 for final release --- app/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle b/app/build.gradle index 1739d385c..f73c60eb8 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -12,7 +12,7 @@ buildscript { def versionMajor = 6 def versionMinor = 6 def versionPatch = 1 -def versionBuild = 1 +def versionBuild = 2 apply plugin: 'com.android.application' apply plugin: 'kotlin-android'