Fix IDs (#1016)

* Allow any String IDs as long as they're sortable

* Allow any String IDs as long as they're sortable

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

@@ -1,8 +1,8 @@
 package com.keylesspalace.tusky
 
 import androidx.room.Room
+import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
-import androidx.test.runner.AndroidJUnit4
 import com.keylesspalace.tusky.db.*
 import com.keylesspalace.tusky.entity.Status
 import com.keylesspalace.tusky.repository.TimelineRepository
@@ -32,7 +32,7 @@ class TimelineDAOTest {
 
     @Test
     fun insertGetStatus() {
-        val setOne = makeStatus()
+        val setOne = makeStatus(statusId = 3)
         val setTwo = makeStatus(statusId = 20, reblog = true)
         val ignoredOne = makeStatus(statusId = 1)
         val ignoredTwo = makeStatus(accountId = 2)
@@ -79,7 +79,7 @@ class TimelineDAOTest {
         val now = System.currentTimeMillis()
         val oldDate = now - TimelineRepository.CLEANUP_INTERVAL - 20_000
         val oldByThisAccount = makeStatus(
-                statusId = 30,
+                statusId = 5,
                 createdAt = oldDate
         )
         val oldByAnotherAccount = makeStatus(
@@ -94,7 +94,7 @@ class TimelineDAOTest {
                 createdAt = oldDate
         )
         val recentByThisAccount = makeStatus(
-                statusId = 50,
+                statusId = 30,
                 createdAt = System.currentTimeMillis()
         )
         val recentByAnotherAccount = makeStatus(

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

@@ -39,9 +39,13 @@ FROM TimelineStatusEntity s
 LEFT JOIN TimelineAccountEntity a ON (s.timelineUserId = a.timelineUserId AND s.authorServerId = a.serverId)
 LEFT JOIN TimelineAccountEntity rb ON (s.timelineUserId = rb.timelineUserId AND s.reblogAccountId = rb.serverId)
 WHERE s.timelineUserId = :account
-AND (CASE WHEN :maxId IS NOT NULL THEN s.serverId < :maxId ELSE 1 END)
+AND (CASE WHEN :maxId IS NOT NULL THEN
-AND (CASE WHEN :sinceId IS NOT NULL THEN s.serverId > :sinceId ELSE 1 END)
+(LENGTH(s.serverId) < LENGTH(:maxId) OR LENGTH(s.serverId) == LENGTH(:maxId) AND s.serverId < :maxId)
-ORDER BY s.serverId DESC
+ELSE 1 END)
+AND (CASE WHEN :sinceId IS NOT NULL THEN
+(LENGTH(s.serverId) > LENGTH(:sinceId) OR LENGTH(s.serverId) == LENGTH(:sinceId) AND s.serverId > :sinceId)
+ELSE 1 END)
+ORDER BY LENGTH(s.serverId) DESC, s.serverId DESC
 LIMIT :limit""")
     abstract fun getStatusesForAccount(account: Long, maxId: String?, sinceId: String?, limit: Int): Single<List<TimelineStatusWithAccount>>
 

app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java

@@ -83,6 +83,7 @@ import retrofit2.Call;
 import retrofit2.Callback;
 import retrofit2.Response;
 
+import static com.keylesspalace.tusky.util.StringUtils.isLessThan;
 import static com.uber.autodispose.AutoDispose.autoDisposable;
 import static com.uber.autodispose.android.lifecycle.AndroidLifecycleScopeProvider.from;
 
@@ -756,16 +757,14 @@ public class NotificationsFragment extends SFragment implements
 
         AccountEntity account = accountManager.getActiveAccount();
         if (account != null) {
-            BigInteger lastNoti = new BigInteger(account.getLastNotificationId());
+            String lastNotificationId = account.getLastNotificationId();
 
             for (Notification noti : notifications) {
-                BigInteger a = new BigInteger(noti.getId());
+                if (isLessThan(lastNotificationId, noti.getId())) {
-                if (isBiggerThan(a, lastNoti)) {
+                    lastNotificationId = noti.getId();
-                    lastNoti = a;
                 }
             }
 
-            String lastNotificationId = lastNoti.toString();
             if (!account.getLastNotificationId().equals(lastNotificationId)) {
                 Log.d(TAG, "saving newest noti id: " + lastNotificationId);
                 account.setLastNotificationId(lastNotificationId);
@@ -774,10 +773,6 @@ public class NotificationsFragment extends SFragment implements
         }
     }
 
-    private boolean isBiggerThan(BigInteger newId, BigInteger lastShownNotificationId) {
-        return lastShownNotificationId.compareTo(newId) < 0;
-    }
-
     private void update(@Nullable List<Notification> newNotifications, @Nullable String fromId) {
         if (ListUtils.isEmpty(newNotifications)) {
             return;

app/src/main/java/com/keylesspalace/tusky/fragment/TimelineFragment.java

@@ -53,6 +53,7 @@ import com.keylesspalace.tusky.util.CollectionUtil;
 import com.keylesspalace.tusky.util.Either;
 import com.keylesspalace.tusky.util.ListUtils;
 import com.keylesspalace.tusky.util.PairedList;
+import com.keylesspalace.tusky.util.StringUtils;
 import com.keylesspalace.tusky.util.ThemeUtils;
 import com.keylesspalace.tusky.util.ViewDataUtils;
 import com.keylesspalace.tusky.view.BackgroundMessageView;
@@ -788,9 +789,7 @@ public class TimelineFragment extends SFragment implements
         Either<Placeholder, Status> last = statuses.get(statuses.size() - 1);
         Placeholder placeholder;
         if (last.isRight()) {
-            final String placeholderId = new BigInteger(last.asRight().getId())
+            final String placeholderId = StringUtils.dec(last.asRight().getId());
-                    .subtract(BigInteger.ONE)
-                    .toString();
             placeholder = new Placeholder(placeholderId);
             statuses.add(new Either.Left<>(placeholder));
         } else {
@@ -963,7 +962,7 @@ public class TimelineFragment extends SFragment implements
                 StatusViewData newViewData;
                 if (placeholder == null) {
                     Status above = statuses.get(position - 1).asRight();
-                    String newId = this.idPlus(above.getId(), -1);
+                    String newId = StringUtils.dec(above.getId());
                     placeholder = new Placeholder(newId);
                 }
                 newViewData = new StatusViewData.Placeholder(placeholder.getId(), false);
@@ -1033,8 +1032,8 @@ public class TimelineFragment extends SFragment implements
             int newIndex = newStatuses.indexOf(statuses.get(0));
             if (newIndex == -1) {
                 if (index == -1 && fullFetch) {
-                    String placeholderId = idPlus(CollectionsKt.last(newStatuses, Either::isRight)
+                    String placeholderId = StringUtils.inc(
-                            .asRight().getId(), 1);
+                            CollectionsKt.last(newStatuses, Either::isRight).asRight().getId());
                     newStatuses.add(new Either.Left<>(new Placeholder(placeholderId)));
                 }
                 statuses.addAll(0, newStatuses);
@@ -1246,8 +1245,4 @@ public class TimelineFragment extends SFragment implements
             return oldItem.deepEquals(newItem);
         }
     };
-
-    private String idPlus(String id, int delta) {
-        return new BigInteger(id).add(BigInteger.valueOf(delta)).toString();
-    }
 }

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

@@ -13,10 +13,11 @@ 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 io.reactivex.Single
 import io.reactivex.schedulers.Schedulers
 import java.io.IOException
-import java.math.BigInteger
 import java.util.*
 import java.util.concurrent.TimeUnit
 
@@ -65,8 +66,8 @@ class TimelineRepositoryImpl(
                                        instance: String, accountId: Long,
                                        requestMode: TimelineRequestMode
     ): Single<out List<TimelineStatus>> {
-        val maxIdInc = maxId?.let { this.incId(it, 1) }
+        val maxIdInc = maxId?.let(String::inc)
-        val sinceIdDec = sinceId?.let { this.incId(it, -1) }
+        val sinceIdDec = sinceId?.let(String::dec)
         return mastodonApi.homeTimelineSingle(maxIdInc, sinceIdDec, limit + 2)
                 .doAfterSuccess { statuses ->
                     this.saveStatusesToDb(instance, accountId, statuses, maxId, sinceId)
@@ -177,27 +178,27 @@ class TimelineRepositoryImpl(
         val firstId = statuses.first().id
         val prepend = if (maxId != null) {
             if (maxId > firstId) {
-                val decMax = this.incId(maxId, -1)
+                val decMax = maxId.dec()
                 if (decMax != firstId) {
                     Placeholder(decMax)
                 } else null
             } else null
         } else {
             // Placeholders never overwrite real values so it's safe
-            Placeholder(incId(firstId, 1))
+            Placeholder(firstId.inc())
         }
 
         val lastId = statuses.last().id
         val append = if (sinceId != null) {
             if (sinceId < lastId) {
-                val incSince = this.incId(sinceId, 1)
+                val incSince = sinceId.inc()
                 if (incSince != lastId) {
                     Placeholder(incSince)
                 } else null
             } else null
         } else {
             // Placeholders never overwrite real values so it's safe
-            Placeholder(incId(lastId, -1))
+            Placeholder(lastId.dec())
         }
 
         return prepend to append
@@ -394,10 +395,6 @@ class TimelineRepositoryImpl(
         )
     }
 
-    private fun incId(id: String, value: Long): String {
-        return BigInteger(id).add(BigInteger.valueOf(value)).toString()
-    }
-
     companion object {
         private val emojisListTypeToken = object : TypeToken<List<Emoji>>() {}
     }

app/src/main/java/com/keylesspalace/tusky/service/SendTootService.kt

@@ -25,7 +25,7 @@ import com.keylesspalace.tusky.di.Injectable
 import com.keylesspalace.tusky.entity.Status
 import com.keylesspalace.tusky.network.MastodonApi
 import com.keylesspalace.tusky.util.SaveTootHelper
-import com.keylesspalace.tusky.util.StringUtils
+import com.keylesspalace.tusky.util.randomAlphanumericString
 import dagger.android.AndroidInjection
 import kotlinx.android.parcel.Parcelize
 import retrofit2.Call
@@ -285,7 +285,7 @@ class SendTootService : Service(), Injectable {
         ): Intent {
             val intent = Intent(context, SendTootService::class.java)
 
-            val idempotencyKey = StringUtils.randomAlphanumericString(16)
+            val idempotencyKey = randomAlphanumericString(16)
 
             val tootToSend = TootToSend(text,
                     warningText,

app/src/main/java/com/keylesspalace/tusky/util/NotificationPullJobCreator.java

@@ -16,8 +16,6 @@
 package com.keylesspalace.tusky.util;
 
 import android.content.Context;
-import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
 import android.util.Log;
 
 import com.evernote.android.job.Job;
@@ -28,15 +26,18 @@ import com.keylesspalace.tusky.entity.Notification;
 import com.keylesspalace.tusky.network.MastodonApi;
 
 import java.io.IOException;
-import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
 import javax.inject.Inject;
 
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import retrofit2.Response;
 
+import static com.keylesspalace.tusky.util.StringUtils.isLessThan;
+
 /**
  * Created by charlag on 31/10/17.
  */
@@ -111,28 +112,24 @@ public final class NotificationPullJobCreator implements JobCreator {
 
         private void onNotificationsReceived(AccountEntity account, List<Notification> notificationList) {
             Collections.reverse(notificationList);
-            BigInteger newId = new BigInteger(account.getLastNotificationId());
+            String newId = account.getLastNotificationId();
-            BigInteger newestId = BigInteger.ZERO;
+            String newestId = "";
             boolean isFirstOfBatch = true;
 
             for (Notification notification : notificationList) {
-                BigInteger currentId = new BigInteger(notification.getId());
+                String currentId = notification.getId();
-                if (isBiggerThan(currentId, newestId)) {
+                if (isLessThan(newestId, currentId)) {
                     newestId = currentId;
                 }
-
+                if (isLessThan(newId, currentId)) {
-                if (isBiggerThan(currentId, newId)) {
                     NotificationHelper.make(context, notification, account, isFirstOfBatch);
                     isFirstOfBatch = false;
                 }
             }
 
-            account.setLastNotificationId(newestId.toString());
+            account.setLastNotificationId(newestId);
             accountManager.saveAccount(account);
         }
 
-        private boolean isBiggerThan(BigInteger newId, BigInteger lastShownNotificationId) {
-            return lastShownNotificationId.compareTo(newId) < 0;
-        }
     }
 }

app/src/main/java/com/keylesspalace/tusky/util/StringUtils.java

@@ -1,17 +0,0 @@
-package com.keylesspalace.tusky.util;
-
-import java.util.Random;
-
-public class StringUtils {
-
-    public static String randomAlphanumericString(int count) {
-        char[] chars = new char[count];
-        Random random = new Random();
-        final String POSSIBLE_CHARS = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
-        for (int i = 0; i < count; i++) {
-            chars[i] = POSSIBLE_CHARS.charAt(random.nextInt(POSSIBLE_CHARS.length()));
-        }
-        return new String(chars);
-    }
-
-}

app/src/main/java/com/keylesspalace/tusky/util/StringUtils.kt

@@ -0,0 +1,72 @@
+@file:JvmName("StringUtils")
+
+package com.keylesspalace.tusky.util
+
+import java.util.Random
+
+
+private const val POSSIBLE_CHARS = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+
+fun randomAlphanumericString(count: Int): String {
+    val chars = CharArray(count)
+    val random = Random()
+    for (i in 0 until count) {
+        chars[i] = POSSIBLE_CHARS[random.nextInt(POSSIBLE_CHARS.length)]
+    }
+    return String(chars)
+}
+
+// We sort statuses by ID. Something we need to invent some ID for placeholder.
+// Not sure if inc()/dec() should be made `operator` or not
+
+/**
+ * "Increment" string so that during sorting it's bigger than [this].
+ */
+fun String.inc(): String {
+    // We assume that we will stay in the safe range for now
+    val builder = this.toCharArray()
+    builder.last().inc()
+    return String(builder)
+}
+
+
+/**
+ * "Decrement" string so that during sorting it's smaller than [this].
+ */
+fun String.dec(): String {
+    val builder = this.toCharArray()
+    var i = builder.lastIndex
+    while (i > 0) {
+        if (builder[i] > '0') {
+            builder[i] = builder[i].dec()
+            break
+        } 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 String(builder)
+}
+
+/**
+ * A < B (strictly) by length and then by content.
+ * Examples:
+ * "abc" < "bcd"
+ * "ab"  < "abc"
+ * "cb"  < "abc"
+ * not: "ab" < "ab"
+ * not: "abc" > "cb"
+ */
+fun String.isLessThan(other: String): Boolean {
+    return when {
+        this.length < other.length -> true
+        this.length > other.length -> false
+        else -> this < other
+    }
+}

app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt

@@ -166,14 +166,14 @@ class EditProfileViewModel  @Inject constructor(
 
         val avatar = if (avatarData.value is Success && avatarData.value?.data != null) {
             val avatarBody = RequestBody.create(MediaType.parse("image/png"), getCacheFileForName(context, AVATAR_FILE_NAME))
-            MultipartBody.Part.createFormData("avatar", StringUtils.randomAlphanumericString(12), avatarBody)
+            MultipartBody.Part.createFormData("avatar", randomAlphanumericString(12), avatarBody)
         } else {
             null
         }
 
         val header = if (headerData.value is Success && headerData.value?.data != null) {
             val headerBody = RequestBody.create(MediaType.parse("image/png"), getCacheFileForName(context, HEADER_FILE_NAME))
-            MultipartBody.Part.createFormData("header", StringUtils.randomAlphanumericString(12), headerBody)
+            MultipartBody.Part.createFormData("header", randomAlphanumericString(12), headerBody)
         } else {
             null
         }

app/src/test/java/com/keylesspalace/tusky/StringUtilsTest.kt

@@ -0,0 +1,34 @@
+package com.keylesspalace.tusky
+
+import com.keylesspalace.tusky.util.dec
+import com.keylesspalace.tusky.util.isLessThan
+import org.junit.Assert.*
+import org.junit.Test
+
+class StringUtilsTest {
+    @Test
+    fun isLessThan() {
+        val lessList = listOf(
+                "abc" to "bcd",
+                "ab" to "abc",
+                "cb" to "abc"
+        )
+        lessList.forEach { (l, r) -> assertTrue("$l < $r", l.isLessThan(r)) }
+        val notLessList = lessList.map { (l, r) -> r to l } + listOf(
+                "abc" to "abc"
+        )
+        notLessList.forEach { (l, r) -> assertFalse("not $l < $r", l.isLessThan(r)) }
+    }
+
+    @Test
+    fun dec() {
+        listOf(
+                "123" to "122",
+                "12B" to "12A",
+                "120" to "11z",
+                "100" to "zz",
+                "0" to "",
+                "" to ""
+        ).forEach { (l, r) -> assertEquals("$l - 1 = $r", r, l.dec()) }
+    }
+}

app/src/test/java/com/keylesspalace/tusky/TimelineFragmentTest.kt

@@ -1,2 +0,0 @@
-package com.keylesspalace.tusky
-