Fix IDs (#1016)
* Allow any String IDs as long as they're sortable * Allow any String IDs as long as they're sortable
This commit is contained in:
parent
cfb43c34ac
commit
3ce10d2a7f
@ -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(
|
||||
|
@ -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 :sinceId IS NOT NULL THEN s.serverId > :sinceId ELSE 1 END)
|
||||
ORDER BY s.serverId DESC
|
||||
AND (CASE WHEN :maxId IS NOT NULL THEN
|
||||
(LENGTH(s.serverId) < LENGTH(:maxId) OR LENGTH(s.serverId) == LENGTH(:maxId) AND s.serverId < :maxId)
|
||||
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>>
|
||||
|
||||
|
@ -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 (isBiggerThan(a, lastNoti)) {
|
||||
lastNoti = a;
|
||||
if (isLessThan(lastNotificationId, noti.getId())) {
|
||||
lastNotificationId = noti.getId();
|
||||
}
|
||||
}
|
||||
|
||||
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;
|
||||
|
@ -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())
|
||||
.subtract(BigInteger.ONE)
|
||||
.toString();
|
||||
final String placeholderId = StringUtils.dec(last.asRight().getId());
|
||||
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)
|
||||
.asRight().getId(), 1);
|
||||
String placeholderId = StringUtils.inc(
|
||||
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();
|
||||
}
|
||||
}
|
||||
|
@ -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 sinceIdDec = sinceId?.let { this.incId(it, -1) }
|
||||
val maxIdInc = maxId?.let(String::inc)
|
||||
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>>() {}
|
||||
}
|
||||
|
@ -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,
|
||||
|
@ -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());
|
||||
BigInteger newestId = BigInteger.ZERO;
|
||||
String newId = account.getLastNotificationId();
|
||||
String newestId = "";
|
||||
boolean isFirstOfBatch = true;
|
||||
|
||||
for (Notification notification : notificationList) {
|
||||
BigInteger currentId = new BigInteger(notification.getId());
|
||||
if (isBiggerThan(currentId, newestId)) {
|
||||
String currentId = notification.getId();
|
||||
if (isLessThan(newestId, currentId)) {
|
||||
newestId = currentId;
|
||||
}
|
||||
|
||||
if (isBiggerThan(currentId, newId)) {
|
||||
if (isLessThan(newId, currentId)) {
|
||||
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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -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);
|
||||
}
|
||||
|
||||
}
|
@ -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
|
||||
}
|
||||
}
|
@ -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
|
||||
}
|
||||
|
34
app/src/test/java/com/keylesspalace/tusky/StringUtilsTest.kt
Normal file
34
app/src/test/java/com/keylesspalace/tusky/StringUtilsTest.kt
Normal file
@ -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()) }
|
||||
}
|
||||
}
|
@ -1,2 +0,0 @@
|
||||
package com.keylesspalace.tusky
|
||||
|
Loading…
Reference in New Issue
Block a user