fix deleted status reappearing in the timeline (#1225)

* fix deleted status reappearing in the timeline

* fix crash

* fix tests

* fix instrumented tests

* add test for deleted status in timeline

app/build.gradle

@@ -133,6 +133,7 @@ dependencies {
         exclude group: 'com.android.support', module: 'support-annotations'
     })
     androidTestImplementation 'android.arch.persistence.room:testing:1.1.1'
+    androidTestImplementation 'androidx.test.ext:junit:1.1.0'
     testImplementation 'androidx.test.ext:junit:1.1.0'
     debugImplementation 'im.dino:dbinspector:3.4.1@aar'
     implementation 'io.reactivex.rxjava2:rxjava:2.2.8'

app/src/androidTest/java/com/keylesspalace/tusky/MigrationsTest.kt

@@ -2,8 +2,8 @@ package com.keylesspalace.tusky
 
 import androidx.room.testing.MigrationTestHelper
 import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
-import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
+import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.keylesspalace.tusky.db.AppDatabase
 import org.junit.Assert.assertEquals
 import org.junit.Rule

app/src/androidTest/java/com/keylesspalace/tusky/TimelineDAOTest.kt

@@ -123,6 +123,42 @@ class TimelineDAOTest {
         )
     }
 
+    @Test
+    fun overwriteDeletedStatus() {
+
+        val oldStatuses = listOf(
+                makeStatus(statusId = 3),
+                makeStatus(statusId = 2),
+                makeStatus(statusId = 1)
+        )
+
+        timelineDao.deleteRange(1, oldStatuses.last().first.serverId, oldStatuses.first().first.serverId)
+
+        for ((status, author, reblogAuthor) in oldStatuses) {
+            timelineDao.insertInTransaction(status, author, reblogAuthor)
+        }
+
+        // status 2 gets deleted, newly loaded status contain only 1 + 3
+        val newStatuses = listOf(
+                makeStatus(statusId = 3),
+                makeStatus(statusId = 1)
+        )
+
+        timelineDao.deleteRange(1, newStatuses.last().first.serverId, newStatuses.first().first.serverId)
+
+        for ((status, author, reblogAuthor) in newStatuses) {
+            timelineDao.insertInTransaction(status, author, reblogAuthor)
+        }
+
+        //make sure status 2 is no longer in db
+
+        assertEquals(
+                newStatuses,
+                timelineDao.getStatusesForAccount(1, null, null, 100).blockingGet()
+                        .map { it.toTriple() }
+        )
+    }
+
     private fun makeStatus(
             accountId: Long = 1,
             statusId: Long = 10,
@@ -177,7 +213,8 @@ class TimelineDAOTest {
                 mentions = "mentions$accountId",
                 application = "application$accountId",
                 reblogServerId = if (reblog) (statusId * 100).toString() else null,
-                reblogAccountId = reblogAuthor?.serverId
+                reblogAccountId = reblogAuthor?.serverId,
+                poll = null
         )
         return Triple(status, author, reblogAuthor)
     }
@@ -204,8 +241,8 @@ class TimelineDAOTest {
                 mentions = null,
                 application = null,
                 reblogServerId = null,
-                reblogAccountId = null
-
+                reblogAccountId = null,
+                poll = null
         )
     }
 

app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt

@@ -14,7 +14,6 @@ abstract class TimelineDao {
     @Insert(onConflict = REPLACE)
     abstract fun insertAccount(timelineAccountEntity: TimelineAccountEntity): Long
 
-
     @Insert(onConflict = REPLACE)
     abstract fun insertStatus(timelineAccountEntity: TimelineStatusEntity): Long
 
@@ -58,13 +57,20 @@ LIMIT :limit""")
         insertStatus(status)
     }
 
+    @Query("""DELETE FROM TimelineStatusEntity WHERE timelineUserId = :accountId AND
+        (LENGTH(serverId) < LENGTH(:maxId) OR LENGTH(serverId) == LENGTH(:maxId) AND serverId < :maxId)
+AND
+(LENGTH(serverId) > LENGTH(:minId) OR LENGTH(serverId) == LENGTH(:minId) AND serverId > :minId)
+    """)
+    abstract fun deleteRange(accountId: Long, minId: String, maxId: String)
+
     @Query("""DELETE FROM TimelineStatusEntity WHERE authorServerId = null
-AND timelineUserId = :acccount AND
+AND timelineUserId = :account 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)
+    abstract fun removeAllPlaceholdersBetween(account: Long, maxId: String, sinceId: String)
 
     @Query("""UPDATE TimelineStatusEntity SET favourited = :favourited
 WHERE timelineUserId = :accountId AND (serverId = :statusId OR reblogServerId - :statusId)""")

app/src/main/java/com/keylesspalace/tusky/repository/TimelineRepository.kt

@@ -144,6 +144,11 @@ class TimelineRepositoryImpl(
         }
 
         Single.fromCallable {
+
+            if(statuses.isNotEmpty()) {
+                timelineDao.deleteRange(accountId, statuses.last().id, statuses.first().id)
+            }
+
             for (status in statuses) {
                 timelineDao.insertInTransaction(
                         status.toEntity(accountId, htmlConverter, gson),

app/src/test/java/com/keylesspalace/tusky/fragment/TimelineRepositoryTest.kt

@@ -91,6 +91,9 @@ class TimelineRepositoryTest {
 
         assertEquals(statuses.map(Status::lift), result)
         testScheduler.advanceTimeBy(100, TimeUnit.SECONDS)
+
+        verify(timelineDao).deleteRange(account.id, statuses.last().id, statuses.first().id)
+
         verify(timelineDao).insertStatusIfNotThere(Placeholder("1").toEntity(account.id))
         for (status in statuses) {
             verify(timelineDao).insertInTransaction(
@@ -122,6 +125,7 @@ class TimelineRepositoryTest {
                 result
         )
         testScheduler.advanceTimeBy(100, TimeUnit.SECONDS)
+        verify(timelineDao).deleteRange(account.id, response.last().id, response.first().id)
         // We assume for now that overlapped one is inserted but it's not that important
         for (status in response) {
             verify(timelineDao).insertInTransaction(
@@ -152,6 +156,7 @@ class TimelineRepositoryTest {
         val placeholder = Placeholder("3")
         assertEquals(response.map(Status::lift) + Either.Left(placeholder), result)
         testScheduler.advanceTimeBy(100, TimeUnit.SECONDS)
+        verify(timelineDao).deleteRange(account.id, response.last().id, response.first().id)
         for (status in response) {
             verify(timelineDao).insertInTransaction(
                     status.toEntity(account.id, htmlConverter, gson),
@@ -192,6 +197,7 @@ class TimelineRepositoryTest {
                 result
         )
         testScheduler.advanceTimeBy(100, TimeUnit.SECONDS)
+        verify(timelineDao).deleteRange(account.id, response.last().id, response.first().id)
         // We assume for now that overlapped one is inserted but it's not that important
         for (status in response) {
             verify(timelineDao).insertInTransaction(
@@ -235,6 +241,9 @@ class TimelineRepositoryTest {
         )
         testScheduler.advanceTimeBy(100, TimeUnit.SECONDS)
         // We assume for now that overlapped one is inserted but it's not that important
+
+        verify(timelineDao).deleteRange(account.id, response.last().id, response.first().id)
+
         for (status in response) {
             verify(timelineDao).insertInTransaction(
                     status.toEntity(account.id, htmlConverter, gson),