From e67127f39dfab7d843ce66bd0856f4a661358349 Mon Sep 17 00:00:00 2001 From: Ivan Kupalov Date: Tue, 5 Feb 2019 20:06:00 +0100 Subject: [PATCH] Fix incorrectly incrementing IDs before sending to server. (#1026) * Fix incorrectly incrementing IDs before sending to server. * Add TimelineRepositoryTest, fix adding placeholder, fix String#dec() * Add more TimelineRepository tests, fix bugs * Add tests for adding statuses from DB. --- app/build.gradle | 1 + .../com/keylesspalace/tusky/db/TimelineDao.kt | 6 +- .../com/keylesspalace/tusky/di/AppModule.kt | 8 + .../tusky/di/RepositoryModule.kt | 7 +- .../tusky/fragment/TimelineFragment.java | 42 ++- .../tusky/repository/TimelineRepository.kt | 340 ++++++++---------- .../keylesspalace/tusky/util/HtmlConverter.kt | 22 ++ .../keylesspalace/tusky/util/StringUtils.kt | 17 +- .../java/android/text/FakeSpannableString.kt | 50 +++ .../keylesspalace/tusky/StringUtilsTest.kt | 16 +- .../tusky/fragment/TimelineRepositoryTest.kt | 330 +++++++++++++++++ 11 files changed, 631 insertions(+), 208 deletions(-) create mode 100644 app/src/main/java/com/keylesspalace/tusky/util/HtmlConverter.kt create mode 100644 app/src/test/java/android/text/FakeSpannableString.kt create mode 100644 app/src/test/java/com/keylesspalace/tusky/fragment/TimelineRepositoryTest.kt diff --git a/app/build.gradle b/app/build.gradle index 66f917b1..845e92ad 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -123,6 +123,7 @@ dependencies { kapt "com.google.dagger:dagger-android-processor:$daggerVersion" testImplementation 'org.robolectric:robolectric:4.1' testImplementation 'org.mockito:mockito-inline:2.23.4' + testImplementation "com.nhaarman.mockitokotlin2:mockito-kotlin:2.1.0" androidTestImplementation('androidx.test.espresso:espresso-core:3.1.0', { exclude group: 'com.android.support', module: 'support-annotations' }) diff --git a/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt b/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt index 945e5074..c2d638ea 100644 --- a/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt +++ b/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt @@ -59,7 +59,11 @@ LIMIT :limit""") } @Query("""DELETE FROM TimelineStatusEntity WHERE authorServerId = null -AND timelineUserId = :acccount AND serverId > :sinceId AND serverId < :maxId""") +AND timelineUserId = :acccount AND +(LENGTH(serverId) < LENGTH(:maxId) OR LENGTH(serverId) == LENGTH(:maxId) AND serverId < :maxId) +AND +(LENGTH(serverId) > LENGTH(:sinceId) OR LENGTH(serverId) == LENGTH(:sinceId) AND serverId > :sinceId) +""") abstract fun removeAllPlaceholdersBetween(acccount: Long, maxId: String, sinceId: String) @Query("""UPDATE TimelineStatusEntity SET favourited = :favourited diff --git a/app/src/main/java/com/keylesspalace/tusky/di/AppModule.kt b/app/src/main/java/com/keylesspalace/tusky/di/AppModule.kt index fcd12ee9..1cbe2190 100644 --- a/app/src/main/java/com/keylesspalace/tusky/di/AppModule.kt +++ b/app/src/main/java/com/keylesspalace/tusky/di/AppModule.kt @@ -29,6 +29,8 @@ import com.keylesspalace.tusky.db.AppDatabase import com.keylesspalace.tusky.network.MastodonApi import com.keylesspalace.tusky.network.TimelineCases import com.keylesspalace.tusky.network.TimelineCasesImpl +import com.keylesspalace.tusky.util.HtmlConverter +import com.keylesspalace.tusky.util.HtmlConverterImpl import dagger.Module import dagger.Provides import javax.inject.Singleton @@ -77,4 +79,10 @@ class AppModule { fun providesDatabase(app: TuskyApplication): AppDatabase { return app.serviceLocator.get(AppDatabase::class.java) } + + @Provides + @Singleton + fun providesHtmlConverter(): HtmlConverter { + return HtmlConverterImpl() + } } \ No newline at end of file diff --git a/app/src/main/java/com/keylesspalace/tusky/di/RepositoryModule.kt b/app/src/main/java/com/keylesspalace/tusky/di/RepositoryModule.kt index 6db47709..1b5206a7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/di/RepositoryModule.kt +++ b/app/src/main/java/com/keylesspalace/tusky/di/RepositoryModule.kt @@ -6,6 +6,7 @@ import com.keylesspalace.tusky.db.AppDatabase import com.keylesspalace.tusky.network.MastodonApi import com.keylesspalace.tusky.repository.TimelineRepository import com.keylesspalace.tusky.repository.TimelineRepositoryImpl +import com.keylesspalace.tusky.util.HtmlConverter import dagger.Module import dagger.Provides @@ -13,7 +14,9 @@ import dagger.Provides class RepositoryModule { @Provides fun providesTimelineRepository(db: AppDatabase, mastodonApi: MastodonApi, - accountManager: AccountManager, gson: Gson): TimelineRepository { - return TimelineRepositoryImpl(db.timelineDao(), mastodonApi, accountManager, gson) + accountManager: AccountManager, gson: Gson, + htmlConverter: HtmlConverter): TimelineRepository { + return TimelineRepositoryImpl(db.timelineDao(), mastodonApi, accountManager, gson, + htmlConverter) } } \ No newline at end of file diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/TimelineFragment.java b/app/src/main/java/com/keylesspalace/tusky/fragment/TimelineFragment.java index 4f5ad686..7b1da9c9 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/TimelineFragment.java +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/TimelineFragment.java @@ -244,14 +244,14 @@ public class TimelineFragment extends SFragment implements if (this.kind == Kind.HOME) { this.tryCache(); } else { - sendFetchTimelineRequest(null, null, FetchEnd.BOTTOM, -1); + sendFetchTimelineRequest(null, null, null, FetchEnd.BOTTOM, -1); } } private void tryCache() { // Request timeline from disk to make it quick, then replace it with timeline from // the server to update it - this.timelineRepo.getStatuses(null, null, LOAD_AT_ONCE, + this.timelineRepo.getStatuses(null, null, null, LOAD_AT_ONCE, TimelineRequestMode.DISK) .observeOn(AndroidSchedulers.mainThread()) .as(autoDisposable(from(this, Lifecycle.Event.ON_DESTROY))) @@ -278,7 +278,7 @@ public class TimelineFragment extends SFragment implements } else { topId = CollectionsKt.first(statuses, Either::isRight).asRight().getId(); } - this.timelineRepo.getStatuses(topId, null, LOAD_AT_ONCE, + this.timelineRepo.getStatuses(topId, null, null, LOAD_AT_ONCE, TimelineRequestMode.NETWORK) .observeOn(AndroidSchedulers.mainThread()) .as(autoDisposable(from(this, Lifecycle.Event.ON_DESTROY))) @@ -520,12 +520,22 @@ public class TimelineFragment extends SFragment implements } private void loadAbove() { - Either firstOrNull = - CollectionsKt.firstOrNull(this.statuses, Either::isRight); + String firstOrNull = null; + String secondOrNull = null; + for (int i = 0; i < this.statuses.size(); i++) { + Either status = this.statuses.get(i); + if (status.isRight()) { + firstOrNull = status.asRight().getId(); + if (i + 1 < statuses.size() && statuses.get(i + 1).isRight()) { + secondOrNull = statuses.get(i + 1).asRight().getId(); + } + break; + } + } if (firstOrNull != null) { - this.sendFetchTimelineRequest(null, firstOrNull.asRight().getId(), FetchEnd.TOP, -1); + this.sendFetchTimelineRequest(null, firstOrNull, secondOrNull, FetchEnd.TOP, -1); } else { - this.sendFetchTimelineRequest(null, null, FetchEnd.BOTTOM, -1); + this.sendFetchTimelineRequest(null, null, null, FetchEnd.BOTTOM, -1); } } @@ -631,11 +641,16 @@ public class TimelineFragment extends SFragment implements if (statuses.size() >= position && position > 0) { Status fromStatus = statuses.get(position - 1).asRightOrNull(); Status toStatus = statuses.get(position + 1).asRightOrNull(); + String maxMinusOne = + statuses.size() > position + 1 && statuses.get(position + 2).isRight() + ? statuses.get(position + 1).asRight().getId() + : null; if (fromStatus == null || toStatus == null) { Log.e(TAG, "Failed to load more at " + position + ", wrong placeholder position"); return; } - sendFetchTimelineRequest(fromStatus.getId(), toStatus.getId(), FetchEnd.MIDDLE, position); + sendFetchTimelineRequest(fromStatus.getId(), toStatus.getId(), maxMinusOne, + FetchEnd.MIDDLE, position); Placeholder placeholder = statuses.get(position).asLeft(); StatusViewData newViewData = new StatusViewData.Placeholder(placeholder.getId(), true); @@ -810,14 +825,14 @@ public class TimelineFragment extends SFragment implements break; } } - sendFetchTimelineRequest(bottomId, null, FetchEnd.BOTTOM, -1); + sendFetchTimelineRequest(bottomId, null, null, FetchEnd.BOTTOM, -1); } private void fullyRefresh() { statuses.clear(); updateAdapter(); bottomLoading = true; - sendFetchTimelineRequest(null, null, FetchEnd.BOTTOM, -1); + sendFetchTimelineRequest(null, null, null, FetchEnd.BOTTOM, -1); } private boolean jumpToTopAllowed() { @@ -861,7 +876,8 @@ public class TimelineFragment extends SFragment implements } } - private void sendFetchTimelineRequest(@Nullable String fromId, @Nullable String uptoId, + private void sendFetchTimelineRequest(@Nullable String maxId, @Nullable String sinceId, + @Nullable String sinceIdMinusOne, final FetchEnd fetchEnd, final int pos) { if (kind == Kind.HOME) { TimelineRequestMode mode; @@ -871,7 +887,7 @@ public class TimelineFragment extends SFragment implements } else { mode = TimelineRequestMode.NETWORK; } - timelineRepo.getStatuses(fromId, uptoId, LOAD_AT_ONCE, mode) + timelineRepo.getStatuses(maxId, sinceId, sinceIdMinusOne, LOAD_AT_ONCE, mode) .observeOn(AndroidSchedulers.mainThread()) .as(autoDisposable(from(this, Lifecycle.Event.ON_DESTROY))) .subscribe( @@ -895,7 +911,7 @@ public class TimelineFragment extends SFragment implements } }; - Call> listCall = getFetchCallByTimelineType(kind, hashtagOrId, fromId, uptoId); + Call> listCall = getFetchCallByTimelineType(kind, hashtagOrId, maxId, sinceId); callList.add(listCall); listCall.enqueue(callback); } diff --git a/app/src/main/java/com/keylesspalace/tusky/repository/TimelineRepository.kt b/app/src/main/java/com/keylesspalace/tusky/repository/TimelineRepository.kt index eca8aba7..26f1b6e4 100644 --- a/app/src/main/java/com/keylesspalace/tusky/repository/TimelineRepository.kt +++ b/app/src/main/java/com/keylesspalace/tusky/repository/TimelineRepository.kt @@ -11,10 +11,7 @@ import com.keylesspalace.tusky.entity.Status import com.keylesspalace.tusky.network.MastodonApi import com.keylesspalace.tusky.repository.TimelineRequestMode.DISK import com.keylesspalace.tusky.repository.TimelineRequestMode.NETWORK -import com.keylesspalace.tusky.util.Either -import com.keylesspalace.tusky.util.HtmlUtils -import com.keylesspalace.tusky.util.dec -import com.keylesspalace.tusky.util.inc +import com.keylesspalace.tusky.util.* import io.reactivex.Single import io.reactivex.schedulers.Schedulers import java.io.IOException @@ -30,7 +27,7 @@ enum class TimelineRequestMode { } interface TimelineRepository { - fun getStatuses(maxId: String?, sinceId: String?, limit: Int, + fun getStatuses(maxId: String?, sinceId: String?, sincedIdMinusOne: String?, limit: Int, requestMode: TimelineRequestMode): Single> companion object { @@ -42,15 +39,17 @@ class TimelineRepositoryImpl( private val timelineDao: TimelineDao, private val mastodonApi: MastodonApi, private val accountManager: AccountManager, - private val gson: Gson + private val gson: Gson, + private val htmlConverter: HtmlConverter ) : TimelineRepository { init { this.cleanup() } - override fun getStatuses(maxId: String?, sinceId: String?, limit: Int, - requestMode: TimelineRequestMode): Single> { + override fun getStatuses(maxId: String?, sinceId: String?, sincedIdMinusOne: String?, + limit: Int, requestMode: TimelineRequestMode + ): Single> { val acc = accountManager.activeAccount ?: throw IllegalStateException() val accountId = acc.id val instance = acc.domain @@ -58,21 +57,19 @@ class TimelineRepositoryImpl( return if (requestMode == DISK) { this.getStatusesFromDb(accountId, maxId, sinceId, limit) } else { - getStatusesFromNetwork(maxId, sinceId, limit, instance, accountId, requestMode) + getStatusesFromNetwork(maxId, sinceId, sincedIdMinusOne, limit, instance, accountId, + requestMode) } } - private fun getStatusesFromNetwork(maxId: String?, sinceId: String?, limit: Int, - instance: String, accountId: Long, - requestMode: TimelineRequestMode + private fun getStatusesFromNetwork(maxId: String?, sinceId: String?, + sinceIdMinusOne: String?, limit: Int, instance: String, + accountId: Long, requestMode: TimelineRequestMode ): Single> { - val maxIdInc = maxId?.let(String::inc) - val sinceIdDec = sinceId?.let(String::dec) - return mastodonApi.homeTimelineSingle(maxIdInc, sinceIdDec, limit + 2) - .doAfterSuccess { statuses -> + return mastodonApi.homeTimelineSingle(maxId, sinceIdMinusOne, limit + 1) + .map { statuses -> this.saveStatusesToDb(instance, accountId, statuses, maxId, sinceId) } - .map { statuses -> this.removePlaceholdersAndMap(statuses, maxId, sinceId) } .flatMap { statuses -> this.addFromDbIfNeeded(accountId, statuses, maxId, sinceId, limit, requestMode) } @@ -85,22 +82,6 @@ class TimelineRepositoryImpl( } } - private fun removePlaceholdersAndMap(statuses: List, maxId: String?, - sinceId: String? - ): List> { - val statusesCopy = statuses.toMutableList() - - // Remove first and last statuses if they were used used just for overlap - if (maxId != null && statusesCopy.firstOrNull()?.id == maxId) { - statusesCopy.removeAt(0) - } - if (sinceId != null && statusesCopy.lastOrNull()?.id == sinceId) { - statusesCopy.removeAt(statusesCopy.size - 1) - } - - return statusesCopy.map { s -> Either.Right(s) } - } - private fun addFromDbIfNeeded(accountId: Long, statuses: List>, maxId: String?, sinceId: String?, limit: Int, requestMode: TimelineRequestMode @@ -109,8 +90,7 @@ class TimelineRepositoryImpl( val newMaxID = if (statuses.isEmpty()) { maxId } else { - // It's statuses from network. They're always Right - statuses.last().asRight().id + statuses.last { it.isRight() }.asRight().id } this.getStatusesFromDb(accountId, newMaxID, sinceId, limit) .map { fromDb -> @@ -137,71 +117,64 @@ class TimelineRepositoryImpl( } private fun saveStatusesToDb(instance: String, accountId: Long, statuses: List, - maxId: String?, sinceId: String?) { + maxId: String?, sinceId: String? + ): List> { + var placeholderToInsert: Placeholder? = null + + // Look for overlap + val resultStatuses = if (statuses.isNotEmpty() && sinceId != null) { + val indexOfSince = statuses.indexOfLast { it.id == sinceId } + if (indexOfSince == -1) { + // We didn't find the status which must be there. Add a placeholder + placeholderToInsert = Placeholder(sinceId.inc()) + statuses.mapTo(mutableListOf(), Status::lift) + .apply { + add(Either.Left(placeholderToInsert)) + } + } else { + // There was an overlap. Remove all overlapped statuses. No need for a placeholder. + statuses.mapTo(mutableListOf(), Status::lift) + .apply { + subList(indexOfSince, size).clear() + } + } + } else { + // Just a normal case. + statuses.map(Status::lift) + } + Single.fromCallable { - val (prepend, append) = calculatePlaceholders(maxId, sinceId, statuses) - - if (prepend != null) { - timelineDao.insertStatusIfNotThere(prepend.toEntity(accountId)) - } - - if (append != null) { - timelineDao.insertStatusIfNotThere(append.toEntity(accountId)) - } - for (status in statuses) { timelineDao.insertInTransaction( - status.toEntity(accountId, instance), - status.account.toEntity(instance, accountId), - status.reblog?.account?.toEntity(instance, accountId) + status.toEntity(accountId, instance, htmlConverter, gson), + status.account.toEntity(instance, accountId, gson), + status.reblog?.account?.toEntity(instance, accountId, gson) ) } + placeholderToInsert?.let { + timelineDao.insertStatusIfNotThere(placeholderToInsert.toEntity(accountId)) + } + + // If we're loading in the bottom insert placeholder after every load + // (for requests on next launches) but not return it. + if (sinceId == null && statuses.isNotEmpty()) { + timelineDao.insertStatusIfNotThere( + Placeholder(statuses.last().id.dec()).toEntity(accountId)) + } + // There may be placeholders which we thought could be from our TL but they are not if (statuses.size > 2) { timelineDao.removeAllPlaceholdersBetween(accountId, statuses.first().id, statuses.last().id) - } else if (maxId != null && sinceId != null) { + } else if (placeholderToInsert == null && maxId != null && sinceId != null) { timelineDao.removeAllPlaceholdersBetween(accountId, maxId, sinceId) } } .subscribeOn(Schedulers.io()) .subscribe() - } - - private fun calculatePlaceholders(maxId: String?, sinceId: String?, - statuses: List - ): Pair { - if (statuses.isEmpty()) return null to null - - val firstId = statuses.first().id - val prepend = if (maxId != null) { - if (maxId > firstId) { - val decMax = maxId.dec() - if (decMax != firstId) { - Placeholder(decMax) - } else null - } else null - } else { - // Placeholders never overwrite real values so it's safe - Placeholder(firstId.inc()) - } - - val lastId = statuses.last().id - val append = if (sinceId != null) { - if (sinceId < lastId) { - val incSince = sinceId.inc() - if (incSince != lastId) { - Placeholder(incSince) - } else null - } else null - } else { - // Placeholders never overwrite real values so it's safe - Placeholder(lastId.dec()) - } - - return prepend to append + return resultStatuses } private fun cleanup() { @@ -215,42 +188,6 @@ class TimelineRepositoryImpl( .subscribe() } - private fun Account.toEntity(instance: String, accountId: Long): TimelineAccountEntity { - return TimelineAccountEntity( - serverId = id, - timelineUserId = accountId, - instance = instance, - localUsername = localUsername, - username = username, - displayName = displayName, - url = url, - avatar = avatar, - emojis = gson.toJson(emojis) - ) - } - - private fun TimelineAccountEntity.toAccount(): Account { - return Account( - id = serverId, - localUsername = localUsername, - username = username, - displayName = displayName, - note = SpannedString(""), - url = url, - avatar = avatar, - header = "", - locked = false, - followingCount = 0, - followersCount = 0, - statusesCount = 0, - source = null, - bot = false, - emojis = gson.fromJson(this.emojis, emojisListTypeToken.type), - fields = null, - moved = null - ) - } - private fun TimelineStatusWithAccount.toStatus(): TimelineStatus { if (this.status.authorServerId == null) { return Either.Left(Placeholder(this.status.serverId)) @@ -268,11 +205,11 @@ class TimelineRepositoryImpl( Status( id = id, url = status.url, - account = account.toAccount(), + account = account.toAccount(gson), inReplyToId = status.inReplyToId, inReplyToAccountId = status.inReplyToAccountId, reblog = null, - content = HtmlUtils.fromHtml(status.content), + content = status.content?.let(htmlConverter::fromHtml) ?: SpannedString(""), createdAt = Date(status.createdAt), emojis = emojis, reblogsCount = status.reblogsCount, @@ -293,7 +230,7 @@ class TimelineRepositoryImpl( Status( id = status.serverId, url = null, // no url for reblogs - account = this.reblogAccount!!.toAccount(), + account = this.reblogAccount!!.toAccount(gson), inReplyToId = null, inReplyToAccountId = null, reblog = reblog, @@ -316,11 +253,11 @@ class TimelineRepositoryImpl( Status( id = status.serverId, url = status.url, - account = account.toAccount(), + account = account.toAccount(gson), inReplyToId = status.inReplyToId, inReplyToAccountId = status.inReplyToAccountId, reblog = null, - content = HtmlUtils.fromHtml(status.content), + content = status.content?.let(htmlConverter::fromHtml) ?: SpannedString(""), createdAt = Date(status.createdAt), emojis = emojis, reblogsCount = status.reblogsCount, @@ -338,64 +275,103 @@ class TimelineRepositoryImpl( } return Either.Right(status) } +} - private fun Status.toEntity(timelineUserId: Long, instance: String): TimelineStatusEntity { - val actionable = actionableStatus - return TimelineStatusEntity( - serverId = this.id, - url = actionable.url!!, - instance = instance, - timelineUserId = timelineUserId, - authorServerId = actionable.account.id, - inReplyToId = actionable.inReplyToId, - inReplyToAccountId = actionable.inReplyToAccountId, - content = HtmlUtils.toHtml(actionable.content), - createdAt = actionable.createdAt.time, - emojis = actionable.emojis.let(gson::toJson), - reblogsCount = actionable.reblogsCount, - favouritesCount = actionable.favouritesCount, - reblogged = actionable.reblogged, - favourited = actionable.favourited, - sensitive = actionable.sensitive, - spoilerText = actionable.spoilerText, - visibility = actionable.visibility, - attachments = actionable.attachments.let(gson::toJson), - mentions = actionable.mentions.let(gson::toJson), - application = actionable.let(gson::toJson), - reblogServerId = reblog?.id, - reblogAccountId = reblog?.let { this.account.id } - ) - } +private val emojisListTypeToken = object : TypeToken>() {} - private fun Placeholder.toEntity(timelineUserId: Long): TimelineStatusEntity { - return TimelineStatusEntity( - serverId = this.id, - url = null, - instance = null, - timelineUserId = timelineUserId, - authorServerId = null, - inReplyToId = null, - inReplyToAccountId = null, - content = null, - createdAt = 0L, - emojis = null, - reblogsCount = 0, - favouritesCount = 0, - reblogged = false, - favourited = false, - sensitive = false, - spoilerText = null, - visibility = null, - attachments = null, - mentions = null, - application = null, - reblogServerId = null, - reblogAccountId = null +fun Account.toEntity(instance: String, accountId: Long, gson: Gson): TimelineAccountEntity { + return TimelineAccountEntity( + serverId = id, + timelineUserId = accountId, + instance = instance, + localUsername = localUsername, + username = username, + displayName = displayName, + url = url, + avatar = avatar, + emojis = gson.toJson(emojis) + ) +} - ) - } +fun TimelineAccountEntity.toAccount(gson: Gson): Account { + return Account( + id = serverId, + localUsername = localUsername, + username = username, + displayName = displayName, + note = SpannedString(""), + url = url, + avatar = avatar, + header = "", + locked = false, + followingCount = 0, + followersCount = 0, + statusesCount = 0, + source = null, + bot = false, + emojis = gson.fromJson(this.emojis, emojisListTypeToken.type), + fields = null, + moved = null + ) +} - companion object { - private val emojisListTypeToken = object : TypeToken>() {} - } -} \ No newline at end of file + +fun Placeholder.toEntity(timelineUserId: Long): TimelineStatusEntity { + return TimelineStatusEntity( + serverId = this.id, + url = null, + instance = null, + timelineUserId = timelineUserId, + authorServerId = null, + inReplyToId = null, + inReplyToAccountId = null, + content = null, + createdAt = 0L, + emojis = null, + reblogsCount = 0, + favouritesCount = 0, + reblogged = false, + favourited = false, + sensitive = false, + spoilerText = null, + visibility = null, + attachments = null, + mentions = null, + application = null, + reblogServerId = null, + reblogAccountId = null + + ) +} + +fun Status.toEntity(timelineUserId: Long, instance: String, + htmlConverter: HtmlConverter, + gson: Gson): TimelineStatusEntity { + val actionable = actionableStatus + return TimelineStatusEntity( + serverId = this.id, + url = actionable.url!!, + instance = instance, + timelineUserId = timelineUserId, + authorServerId = actionable.account.id, + inReplyToId = actionable.inReplyToId, + inReplyToAccountId = actionable.inReplyToAccountId, + content = htmlConverter.toHtml(actionable.content), + createdAt = actionable.createdAt.time, + emojis = actionable.emojis.let(gson::toJson), + reblogsCount = actionable.reblogsCount, + favouritesCount = actionable.favouritesCount, + reblogged = actionable.reblogged, + favourited = actionable.favourited, + sensitive = actionable.sensitive, + spoilerText = actionable.spoilerText, + visibility = actionable.visibility, + attachments = actionable.attachments.let(gson::toJson), + mentions = actionable.mentions.let(gson::toJson), + application = actionable.let(gson::toJson), + reblogServerId = reblog?.id, + reblogAccountId = reblog?.let { this.account.id } + ) +} + +fun Status.lift(): Either = Either.Right(this) diff --git a/app/src/main/java/com/keylesspalace/tusky/util/HtmlConverter.kt b/app/src/main/java/com/keylesspalace/tusky/util/HtmlConverter.kt new file mode 100644 index 00000000..72efc28c --- /dev/null +++ b/app/src/main/java/com/keylesspalace/tusky/util/HtmlConverter.kt @@ -0,0 +1,22 @@ +package com.keylesspalace.tusky.util + +import android.text.Spanned + +/** + * Abstracting away Android-specific things. + */ +interface HtmlConverter { + fun fromHtml(html: String): Spanned + + fun toHtml(text: Spanned): String +} + +internal class HtmlConverterImpl : HtmlConverter { + override fun fromHtml(html: String): Spanned { + return HtmlUtils.fromHtml(html) + } + + override fun toHtml(text: Spanned): String { + return HtmlUtils.toHtml(text) + } +} \ No newline at end of file diff --git a/app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt b/app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt index 0fddb45a..32adaf89 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt @@ -25,7 +25,7 @@ fun randomAlphanumericString(count: Int): String { fun String.inc(): String { // We assume that we will stay in the safe range for now val builder = this.toCharArray() - builder.last().inc() + builder[lastIndex] = builder[lastIndex].inc() return String(builder) } @@ -34,24 +34,25 @@ fun String.inc(): String { * "Decrement" string so that during sorting it's smaller than [this]. */ fun String.dec(): String { + if (this.isEmpty()) return this + val builder = this.toCharArray() var i = builder.lastIndex while (i > 0) { if (builder[i] > '0') { builder[i] = builder[i].dec() - break + return String(builder) } else { builder[i] = 'z' } i-- } - // All characters were '0' - if (i == 0 && this.isNotEmpty()) { - // Remove one character - return String(builder.copyOfRange(1, builder.size)) + return if (builder[0] > '1') { + builder[0] = builder[0].dec() + String(builder) + } else { + String(builder.copyOfRange(1, builder.size)) } - - return String(builder) } /** diff --git a/app/src/test/java/android/text/FakeSpannableString.kt b/app/src/test/java/android/text/FakeSpannableString.kt new file mode 100644 index 00000000..c4e4e4cc --- /dev/null +++ b/app/src/test/java/android/text/FakeSpannableString.kt @@ -0,0 +1,50 @@ +package android.text + +// Used for stubbing Android implementation without slow & buggy Robolectric things +@Suppress("unused") +class SpannableString(private val text: CharSequence) : Spannable { + + override fun setSpan(what: Any?, start: Int, end: Int, flags: Int) { + throw NotImplementedError() + } + + override fun getSpans(start: Int, end: Int, type: Class?): Array { + throw NotImplementedError() + } + + override fun removeSpan(what: Any?) { + throw NotImplementedError() + } + + override fun toString(): String { + return "FakeSpannableString[text=$text]" + } + + override val length: Int + get() = text.length + + + override fun nextSpanTransition(start: Int, limit: Int, type: Class<*>?): Int { + throw NotImplementedError() + } + + override fun getSpanEnd(tag: Any?): Int { + throw NotImplementedError() + } + + override fun getSpanFlags(tag: Any?): Int { + throw NotImplementedError() + } + + override fun get(index: Int): Char { + throw NotImplementedError() + } + + override fun subSequence(startIndex: Int, endIndex: Int): CharSequence { + throw NotImplementedError() + } + + override fun getSpanStart(tag: Any?): Int { + throw NotImplementedError() + } +} \ No newline at end of file diff --git a/app/src/test/java/com/keylesspalace/tusky/StringUtilsTest.kt b/app/src/test/java/com/keylesspalace/tusky/StringUtilsTest.kt index 8d13bc43..7b23297e 100644 --- a/app/src/test/java/com/keylesspalace/tusky/StringUtilsTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/StringUtilsTest.kt @@ -1,6 +1,7 @@ package com.keylesspalace.tusky import com.keylesspalace.tusky.util.dec +import com.keylesspalace.tusky.util.inc import com.keylesspalace.tusky.util.isLessThan import org.junit.Assert.* import org.junit.Test @@ -11,7 +12,8 @@ class StringUtilsTest { val lessList = listOf( "abc" to "bcd", "ab" to "abc", - "cb" to "abc" + "cb" to "abc", + "1" to "2" ) lessList.forEach { (l, r) -> assertTrue("$l < $r", l.isLessThan(r)) } val notLessList = lessList.map { (l, r) -> r to l } + listOf( @@ -20,6 +22,15 @@ class StringUtilsTest { notLessList.forEach { (l, r) -> assertFalse("not $l < $r", l.isLessThan(r)) } } + @Test + fun inc() { + listOf( + "122" to "123", + "12A" to "12B", + "1" to "2" + ).forEach { (l, r) -> assertEquals("$l + 1 = $r", r, l.inc()) } + } + @Test fun dec() { listOf( @@ -28,7 +39,8 @@ class StringUtilsTest { "120" to "11z", "100" to "zz", "0" to "", - "" to "" + "" to "", + "2" to "1" ).forEach { (l, r) -> assertEquals("$l - 1 = $r", r, l.dec()) } } } \ No newline at end of file diff --git a/app/src/test/java/com/keylesspalace/tusky/fragment/TimelineRepositoryTest.kt b/app/src/test/java/com/keylesspalace/tusky/fragment/TimelineRepositoryTest.kt new file mode 100644 index 00000000..8625f147 --- /dev/null +++ b/app/src/test/java/com/keylesspalace/tusky/fragment/TimelineRepositoryTest.kt @@ -0,0 +1,330 @@ +package com.keylesspalace.tusky.fragment + +import android.text.Spanned +import com.google.gson.Gson +import com.keylesspalace.tusky.SpanUtilsTest +import com.keylesspalace.tusky.db.AccountEntity +import com.keylesspalace.tusky.db.AccountManager +import com.keylesspalace.tusky.db.TimelineDao +import com.keylesspalace.tusky.db.TimelineStatusWithAccount +import com.keylesspalace.tusky.entity.Account +import com.keylesspalace.tusky.entity.Status +import com.keylesspalace.tusky.network.MastodonApi +import com.keylesspalace.tusky.repository.* +import com.keylesspalace.tusky.util.Either +import com.keylesspalace.tusky.util.HtmlConverter +import com.nhaarman.mockitokotlin2.isNull +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions +import com.nhaarman.mockitokotlin2.whenever +import io.reactivex.Single +import io.reactivex.plugins.RxJavaPlugins +import io.reactivex.schedulers.Schedulers +import io.reactivex.schedulers.TestScheduler +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import java.util.* +import java.util.concurrent.TimeUnit + +class TimelineRepositoryTest { + @Mock + lateinit var timelineDao: TimelineDao + + @Mock + lateinit var mastodonApi: MastodonApi + + @Mock + lateinit var accountManager: AccountManager + + lateinit var gson: Gson + + lateinit var subject: TimelineRepository + + lateinit var testScheduler: TestScheduler + + + val limit = 30 + val account = AccountEntity( + id = 2, + accessToken = "token", + domain = "domain.com", + isActive = true + ) + val htmlConverter = object : HtmlConverter { + override fun fromHtml(html: String): Spanned { + return SpanUtilsTest.FakeSpannable(html) + } + + override fun toHtml(text: Spanned): String { + return text.toString() + } + } + + @Before + fun setup() { + MockitoAnnotations.initMocks(this) + whenever(accountManager.activeAccount).thenReturn(account) + + gson = Gson() + testScheduler = TestScheduler() + RxJavaPlugins.setIoSchedulerHandler { testScheduler } + subject = TimelineRepositoryImpl(timelineDao, mastodonApi, accountManager, gson, + htmlConverter) + } + + @Test + fun testNetworkUnbounded() { + val statuses = listOf( + makeStatus("3"), + makeStatus("2") + ) + whenever(mastodonApi.homeTimelineSingle(isNull(), isNull(), anyInt())) + .thenReturn(Single.just(statuses)) + val result = subject.getStatuses(null, null, null, limit, TimelineRequestMode.NETWORK) + .blockingGet() + + assertEquals(statuses.map(Status::lift), result) + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + verify(timelineDao).insertStatusIfNotThere(Placeholder("1").toEntity(account.id)) + for (status in statuses) { + verify(timelineDao).insertInTransaction( + status.toEntity(account.id, account.domain, htmlConverter, gson), + status.account.toEntity(account.domain, account.id, gson), + null + ) + } + verifyNoMoreInteractions(timelineDao) + } + + @Test + fun testNetworkLoadingTopNoGap() { + val response = listOf( + makeStatus("4"), + makeStatus("3"), + makeStatus("2") + ) + val sinceId = "2" + val sinceIdMinusOne = "1" + whenever(mastodonApi.homeTimelineSingle(null, sinceIdMinusOne, limit + 1)) + .thenReturn(Single.just(response)) + val result = subject.getStatuses(null, sinceId, sinceIdMinusOne, limit, + TimelineRequestMode.NETWORK) + .blockingGet() + + assertEquals( + response.subList(0, 2).map(Status::lift), + result + ) + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + // We assume for now that overlapped one is inserted but it's not that important + for (status in response) { + verify(timelineDao).insertInTransaction( + status.toEntity(account.id, account.domain, htmlConverter, gson), + status.account.toEntity(account.domain, account.id, gson), + null + ) + } + verify(timelineDao).removeAllPlaceholdersBetween(account.id, response.first().id, + response.last().id) + verifyNoMoreInteractions(timelineDao) + } + + @Test + fun testNetworkLoadingTopWithGap() { + val response = listOf( + makeStatus("5"), + makeStatus("4") + ) + val sinceId = "2" + val sinceIdMinusOne = "1" + whenever(mastodonApi.homeTimelineSingle(null, sinceIdMinusOne, limit + 1)) + .thenReturn(Single.just(response)) + val result = subject.getStatuses(null, sinceId, sinceIdMinusOne, limit, + TimelineRequestMode.NETWORK) + .blockingGet() + + val placeholder = Placeholder("3") + assertEquals(response.map(Status::lift) + Either.Left(placeholder), result) + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + for (status in response) { + verify(timelineDao).insertInTransaction( + status.toEntity(account.id, account.domain, htmlConverter, gson), + status.account.toEntity(account.domain, account.id, gson), + null + ) + } + verify(timelineDao).insertStatusIfNotThere(placeholder.toEntity(account.id)) + verifyNoMoreInteractions(timelineDao) + } + + @Test + fun testNetworkLoadingMiddleNoGap() { + // Example timelne: + // 5 + // 4 + // [gap] + // 2 + // 1 + + val response = listOf( + makeStatus("5"), + makeStatus("4"), + makeStatus("3"), + makeStatus("2") + ) + val sinceId = "2" + val sinceIdMinusOne = "1" + val maxId = "3" + whenever(mastodonApi.homeTimelineSingle(maxId, sinceIdMinusOne, limit + 1)) + .thenReturn(Single.just(response)) + val result = subject.getStatuses(maxId, sinceId, sinceIdMinusOne, limit, + TimelineRequestMode.NETWORK) + .blockingGet() + + assertEquals( + response.subList(0, response.lastIndex).map(Status::lift), + result + ) + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + // We assume for now that overlapped one is inserted but it's not that important + for (status in response) { + verify(timelineDao).insertInTransaction( + status.toEntity(account.id, account.domain, htmlConverter, gson), + status.account.toEntity(account.domain, account.id, gson), + null + ) + } + verify(timelineDao).removeAllPlaceholdersBetween(account.id, response.first().id, + response.last().id) + verifyNoMoreInteractions(timelineDao) + } + + @Test + fun testNetworkLoadingMiddleWithGap() { + // Example timelne: + // 6 + // 5 + // [gap] + // 2 + // 1 + + val response = listOf( + makeStatus("6"), + makeStatus("5"), + makeStatus("4") + ) + val sinceId = "2" + val sinceIdMinusOne = "1" + val maxId = "4" + whenever(mastodonApi.homeTimelineSingle(maxId, sinceIdMinusOne, limit + 1)) + .thenReturn(Single.just(response)) + val result = subject.getStatuses(maxId, sinceId, sinceIdMinusOne, limit, + TimelineRequestMode.NETWORK) + .blockingGet() + + val placeholder = Placeholder("3") + assertEquals( + response.map(Status::lift) + Either.Left(placeholder), + result + ) + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + // We assume for now that overlapped one is inserted but it's not that important + for (status in response) { + verify(timelineDao).insertInTransaction( + status.toEntity(account.id, account.domain, htmlConverter, gson), + status.account.toEntity(account.domain, account.id, gson), + null + ) + } + verify(timelineDao).removeAllPlaceholdersBetween(account.id, response.first().id, + response.last().id) + verify(timelineDao).insertStatusIfNotThere(placeholder.toEntity(account.id)) + verifyNoMoreInteractions(timelineDao) + } + + @Test + fun addingFromDb() { + RxJavaPlugins.setIoSchedulerHandler { Schedulers.single() } + val status = makeStatus("2") + val dbStatus = makeStatus("1") + val dbResult = TimelineStatusWithAccount() + dbResult.status = dbStatus.toEntity(account.id, account.domain, htmlConverter, gson) + dbResult.account = status.account.toEntity(account.domain, account.id, gson) + + whenever(mastodonApi.homeTimelineSingle(any(), any(), any())) + .thenReturn(Single.just(listOf(status))) + whenever(timelineDao.getStatusesForAccount(account.id, status.id, null, 30)) + .thenReturn(Single.just(listOf(dbResult))) + val result = subject.getStatuses(null, null, null, limit, TimelineRequestMode.ANY) + .blockingGet() + assertEquals(listOf(status, dbStatus).map(Status::lift), result) + } + + @Test + fun addingFromDbExhausted() { + RxJavaPlugins.setIoSchedulerHandler { Schedulers.single() } + val status = makeStatus("4") + val dbResult = TimelineStatusWithAccount() + dbResult.status = Placeholder("2").toEntity(account.id) + val dbResult2 = TimelineStatusWithAccount() + dbResult2.status = Placeholder("1").toEntity(account.id) + + whenever(mastodonApi.homeTimelineSingle(any(), any(), any())) + .thenReturn(Single.just(listOf(status))) + whenever(timelineDao.getStatusesForAccount(account.id, status.id, null, 30)) + .thenReturn(Single.just(listOf(dbResult, dbResult2))) + val result = subject.getStatuses(null, null, null, limit, TimelineRequestMode.ANY) + .blockingGet() + assertEquals(listOf(status).map(Status::lift), result) + } + + private fun makeStatus(id: String, account: Account = makeAccount(id)): Status { + return Status( + id = id, + account = account, + content = SpanUtilsTest.FakeSpannable("hello$id"), + createdAt = Date(), + emojis = listOf(), + reblogsCount = 3, + favouritesCount = 5, + sensitive = false, + visibility = Status.Visibility.PUBLIC, + spoilerText = "", + reblogged = true, + favourited = false, + attachments = listOf(), + mentions = arrayOf(), + application = null, + inReplyToAccountId = null, + inReplyToId = null, + pinned = false, + reblog = null, + url = "http://example.com/statuses/$id" + ) + } + + private fun makeAccount(id: String): Account { + return Account( + id = id, + localUsername = "test$id", + username = "test$id@example.com", + displayName = "Example Account $id", + note = SpanUtilsTest.FakeSpannable("Note! $id"), + url = "https://example.com/@test$id", + avatar = "avatar$id", + header = "Header$id", + followersCount = 300, + followingCount = 400, + statusesCount = 1000, + bot = false, + emojis = listOf(), + fields = null, + source = null + ) + } +} \ No newline at end of file