refactor: Use Date type for scheduled post date / times (#1032)

Previous code accepted the `scheduledAt` value as a String, and kept it
as a String (including when serialising as part of a draft). Then it was
converted to an actual Date for display.

Refactor to keep it as a Date for as long as possible. Moshi decodes
Dates correctly over the network, and the database is configured to
serialise Dates as Longs.

This necessitates two migration steps to preserve any existing
`scheduledAt` values for drafts. The first step adds a new column to
store the date as a Long and copies over existing data. The second step
replaces the old column with the new column.
This commit is contained in:
Nik Clayton 2024-10-20 16:29:32 +02:00 committed by GitHub
parent e68ab54f95
commit 0fe84f1611
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 2511 additions and 62 deletions

View File

@ -121,6 +121,7 @@ import com.mikepenz.iconics.utils.sizeDp
import dagger.hilt.android.AndroidEntryPoint
import java.io.File
import java.io.IOException
import java.util.Date
import java.util.Locale
import javax.inject.Inject
import kotlin.math.max
@ -292,8 +293,8 @@ class ComposeActivity :
binding.composeEditField.setText(statusContent)
}
if (!composeOptions?.scheduledAt.isNullOrEmpty()) {
binding.composeScheduleView.setDateTime(composeOptions?.scheduledAt)
composeOptions?.scheduledAt?.let {
binding.composeScheduleView.setDateTime(it)
}
setupLanguageSpinner(getInitialLanguages(composeOptions?.language, activeAccount))
@ -314,7 +315,7 @@ class ComposeActivity :
viewModel.showContentWarningChanged(this)
}
it.getString(KEY_SCHEDULED_TIME)?.let { time ->
(it.getSerializable(KEY_SCHEDULED_TIME) as? Date)?.let { time ->
viewModel.updateScheduledAt(time)
}
}
@ -717,7 +718,7 @@ class ComposeActivity :
outState.putParcelable(KEY_PHOTO_UPLOAD_URI, photoUploadUri)
outState.putSerializable(KEY_VISIBILITY, viewModel.statusVisibility.value)
outState.putBoolean(KEY_CONTENT_WARNING_VISIBLE, viewModel.showContentWarning.value)
outState.putString(KEY_SCHEDULED_TIME, viewModel.scheduledAt.value)
outState.putSerializable(KEY_SCHEDULED_TIME, viewModel.scheduledAt.value)
super.onSaveInstanceState(outState)
}
@ -783,7 +784,7 @@ class ComposeActivity :
// Can't reschedule a published status
enableButton(binding.composeScheduleButton, clickable = false, colorActive = false)
} else {
val attr = if (binding.composeScheduleView.time == null) {
val attr = if (viewModel.scheduledAt.value == null) {
android.R.attr.colorControlNormal
} else {
android.R.attr.colorPrimary
@ -970,7 +971,7 @@ class ComposeActivity :
}
private fun verifyScheduledTime(): Boolean {
return binding.composeScheduleView.verifyScheduledTime(binding.composeScheduleView.getDateTime(viewModel.scheduledAt.value))
return binding.composeScheduleView.verifyScheduledTime(viewModel.scheduledAt.value)
}
private fun onSendClicked() = lifecycleScope.launch {
@ -1410,7 +1411,7 @@ class ComposeActivity :
}
}
override fun onTimeSet(time: String?) {
override fun onTimeSet(time: Date?) {
viewModel.updateScheduledAt(time)
if (verifyScheduledTime()) {
scheduleBehavior.state = BottomSheetBehavior.STATE_HIDDEN

View File

@ -58,6 +58,7 @@ import com.github.michaelbull.result.mapBoth
import com.github.michaelbull.result.mapError
import dagger.hilt.android.lifecycle.HiltViewModel
import io.github.z4kn4fein.semver.constraints.toConstraint
import java.util.Date
import javax.inject.Inject
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.channels.BufferOverflow
@ -138,7 +139,7 @@ class ComposeViewModel @Inject constructor(
val showContentWarning = _showContentWarning.asStateFlow()
private val _poll: MutableStateFlow<NewPoll?> = MutableStateFlow(null)
val poll = _poll.asStateFlow()
private val _scheduledAt: MutableStateFlow<String?> = MutableStateFlow(null)
private val _scheduledAt: MutableStateFlow<Date?> = MutableStateFlow(null)
val scheduledAt = _scheduledAt.asStateFlow()
private val _media: MutableStateFlow<List<QueuedMedia>> = MutableStateFlow(emptyList())
@ -637,7 +638,7 @@ class ComposeViewModel @Inject constructor(
setupComplete = true
}
fun updateScheduledAt(newScheduledAt: String?) {
fun updateScheduledAt(newScheduledAt: Date?) {
if (newScheduledAt != scheduledAt.value) {
scheduledTimeChanged = true
}

View File

@ -33,11 +33,9 @@ import com.google.android.material.datepicker.DateValidatorPointForward
import com.google.android.material.datepicker.MaterialDatePicker
import com.google.android.material.timepicker.MaterialTimePicker
import com.google.android.material.timepicker.TimeFormat
import java.text.ParseException
import java.text.SimpleDateFormat
import java.util.Calendar
import java.util.Date
import java.util.Locale
import java.util.TimeZone
class ComposeScheduleView
@ -47,7 +45,7 @@ class ComposeScheduleView
defStyleAttr: Int = 0,
) : ConstraintLayout(context, attrs, defStyleAttr) {
interface OnTimeSetListener {
fun onTimeSet(time: String?)
fun onTimeSet(time: Date?)
}
private var binding = ViewComposeScheduleBinding.inflate(
@ -57,12 +55,6 @@ class ComposeScheduleView
private var listener: OnTimeSetListener? = null
private var dateFormat = SimpleDateFormat.getDateInstance()
private var timeFormat = SimpleDateFormat.getTimeInstance()
private var iso8601 = SimpleDateFormat(
"yyyy-MM-dd'T'HH:mm:ss.SSS'Z'",
Locale.getDefault(),
).apply {
timeZone = TimeZone.getTimeZone("UTC")
}
/** The date/time the user has chosen to schedule the status, in UTC */
private var scheduleDateTimeUtc: Calendar? = null
@ -190,20 +182,9 @@ class ComposeScheduleView
picker.show((context as AppCompatActivity).supportFragmentManager, "time_picker")
}
fun getDateTime(scheduledAt: String?): Date? {
scheduledAt?.let {
try {
return iso8601.parse(it)
} catch (_: ParseException) {
}
}
return null
}
fun setDateTime(scheduledAt: String?) {
val date = getDateTime(scheduledAt) ?: return
fun setDateTime(scheduledAt: Date) {
initializeSuggestedTime()
scheduleDateTimeUtc!!.time = date
scheduleDateTimeUtc!!.time = scheduledAt
updateScheduleUi()
}
@ -238,12 +219,9 @@ class ComposeScheduleView
scheduleDateTimeUtc?.set(Calendar.HOUR_OF_DAY, hourOfDay)
scheduleDateTimeUtc?.set(Calendar.MINUTE, minute)
updateScheduleUi()
listener?.onTimeSet(time)
listener?.onTimeSet(scheduleDateTimeUtc?.time)
}
val time: String?
get() = scheduleDateTimeUtc?.time?.let { iso8601.format(it) }
private fun initializeSuggestedTime() {
if (scheduleDateTimeUtc == null) {
scheduleDateTimeUtc = calendar().apply {

View File

@ -63,7 +63,7 @@ class DraftHelper @Inject constructor(
poll: NewPoll?,
failedToSend: Boolean,
failedToSendAlert: Boolean,
scheduledAt: String?,
scheduledAt: Date?,
language: String?,
statusId: String?,
) = withContext(Dispatchers.IO) {

View File

@ -40,6 +40,7 @@ import at.connyduck.calladapter.networkresult.fold
import com.github.michaelbull.result.getOrElse
import dagger.hilt.android.AndroidEntryPoint
import java.io.IOException
import java.util.Date
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.TimeUnit
import javax.inject.Inject
@ -247,7 +248,8 @@ class SendStatusService : Service() {
)
}
sendResult.fold({ sentStatus ->
sendResult.fold(
{ sentStatus ->
statusesToSend.remove(statusId)
// If the status was loaded from a draft, delete the draft and associated media files.
if (statusToSend.draftId != 0) {
@ -256,7 +258,7 @@ class SendStatusService : Service() {
mediaUploader.cancelUploadScope(*statusToSend.media.map { it.localId }.toIntArray())
val scheduled = !statusToSend.scheduledAt.isNullOrEmpty()
val scheduled = statusToSend.scheduledAt != null
if (scheduled) {
eventHub.dispatch(StatusScheduledEvent)
@ -267,10 +269,12 @@ class SendStatusService : Service() {
}
notificationManager.cancel(statusId)
}, { throwable ->
},
{ throwable ->
Timber.w(throwable, "failed sending status")
failOrRetry(throwable, statusId)
})
},
)
stopSelfWhenDone()
}
}
@ -475,7 +479,7 @@ data class StatusToSend(
val visibility: String,
val sensitive: Boolean,
val media: List<MediaToSend>,
val scheduledAt: String?,
val scheduledAt: Date?,
val inReplyToId: String?,
val poll: NewPoll?,
val replyingStatusContent: String?,

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -17,11 +17,17 @@
package app.pachli.core.database
import android.annotation.SuppressLint
import android.content.ContentValues
import android.database.sqlite.SQLiteDatabase.CONFLICT_ABORT
import androidx.core.database.getStringOrNull
import androidx.room.AutoMigration
import androidx.room.Database
import androidx.room.DeleteColumn
import androidx.room.RenameColumn
import androidx.room.RoomDatabase
import androidx.room.migration.AutoMigrationSpec
import androidx.sqlite.db.SupportSQLiteDatabase
import app.pachli.core.database.dao.AccountDao
import app.pachli.core.database.dao.ConversationsDao
import app.pachli.core.database.dao.DraftDao
@ -40,6 +46,9 @@ import app.pachli.core.database.model.StatusViewDataEntity
import app.pachli.core.database.model.TimelineAccountEntity
import app.pachli.core.database.model.TimelineStatusEntity
import app.pachli.core.database.model.TranslatedStatusEntity
import java.text.SimpleDateFormat
import java.util.Locale
import java.util.TimeZone
@Suppress("ClassName")
@Database(
@ -55,13 +64,15 @@ import app.pachli.core.database.model.TranslatedStatusEntity
TranslatedStatusEntity::class,
LogEntryEntity::class,
],
version = 6,
version = 8,
autoMigrations = [
AutoMigration(from = 1, to = 2, spec = AppDatabase.MIGRATE_1_2::class),
AutoMigration(from = 2, to = 3),
AutoMigration(from = 3, to = 4),
AutoMigration(from = 4, to = 5),
AutoMigration(from = 5, to = 6),
AutoMigration(from = 6, to = 7, spec = AppDatabase.MIGRATE_6_7::class),
AutoMigration(from = 7, to = 8, spec = AppDatabase.MIGRATE_7_8::class),
],
)
abstract class AppDatabase : RoomDatabase() {
@ -78,4 +89,54 @@ abstract class AppDatabase : RoomDatabase() {
@DeleteColumn("TimelineStatusEntity", "contentCollapsed")
@DeleteColumn("TimelineStatusEntity", "contentShowing")
class MIGRATE_1_2 : AutoMigrationSpec
/**
* Part one of migrating [DraftEntity.scheduledAt] from String to Long.
*
* Copies existing data from `scheduledAt` into `scheduledAtLong`.
*/
class MIGRATE_6_7 : AutoMigrationSpec {
@SuppressLint("ConstantLocale")
private val iso8601 = SimpleDateFormat(
"yyyy-MM-dd'T'HH:mm:ss.SSS'Z'",
Locale.getDefault(),
).apply {
timeZone = TimeZone.getTimeZone("UTC")
}
override fun onPostMigrate(db: SupportSQLiteDatabase) {
db.beginTransaction()
val draftCursor = db.query("SELECT id, scheduledAt FROM DraftEntity")
with(draftCursor) {
while (moveToNext()) {
val scheduledAt = getStringOrNull(1) ?: continue
// Parse the string representation to a Date. Ignore errors, they
// shouldn't be possible.
val scheduledDate = runCatching { iso8601.parse(scheduledAt) }.getOrNull()
?: continue
// Dates are stored as Long, see Converters.dateToLong.
val values = ContentValues().apply {
put("scheduledAtLong", scheduledDate.time)
}
val draftId = getInt(0)
db.update("DraftEntity", CONFLICT_ABORT, values, "id = ?", arrayOf(draftId))
}
}
db.setTransactionSuccessful()
db.endTransaction()
}
}
/**
* Completes the migration started in [MIGRATE_6_7]. SQLite on Android can't
* drop/rename columns, so use Room's annotations to generate the code to do this.
*/
@DeleteColumn("DraftEntity", "scheduledAt")
@RenameColumn("DraftEntity", "scheduledAtLong", "scheduledAt")
class MIGRATE_7_8 : AutoMigrationSpec
}

View File

@ -29,6 +29,7 @@ import app.pachli.core.network.model.NewPoll
import app.pachli.core.network.model.Status
import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass
import java.util.Date
import kotlinx.parcelize.Parcelize
@Entity
@ -45,7 +46,7 @@ data class DraftEntity(
val poll: NewPoll?,
val failedToSend: Boolean,
val failedToSendNew: Boolean,
val scheduledAt: String?,
val scheduledAt: Date?,
val language: String?,
val statusId: String?,
)

View File

@ -38,6 +38,7 @@ import app.pachli.core.network.model.NewPoll
import app.pachli.core.network.model.Notification
import app.pachli.core.network.model.Status
import com.gaelmarhic.quadrant.QuadrantConstants
import java.util.Date
import kotlinx.parcelize.Parcelize
private const val EXTRA_PACHLI_ACCOUNT_ID = "app.pachli.EXTRA_PACHLI_ACCOUNT_ID"
@ -147,7 +148,7 @@ class ComposeActivityIntent(context: Context) : Intent() {
val replyingStatusContent: String? = null,
val mediaAttachments: List<Attachment>? = null,
val draftAttachments: List<DraftAttachment>? = null,
val scheduledAt: String? = null,
val scheduledAt: Date? = null,
val sensitive: Boolean? = null,
val poll: NewPoll? = null,
val modifiedInitialState: Boolean? = null,

View File

@ -19,6 +19,7 @@ package app.pachli.core.network.model
import android.os.Parcelable
import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass
import java.util.Date
import kotlinx.parcelize.Parcelize
@JsonClass(generateAdapter = true)
@ -30,7 +31,7 @@ data class NewStatus(
val sensitive: Boolean,
@Json(name = "media_ids") val mediaIds: List<String>?,
@Json(name = "media_attributes") val mediaAttributes: List<MediaAttribute>?,
@Json(name = "scheduled_at") val scheduledAt: String?,
@Json(name = "scheduled_at") val scheduledAt: Date?,
val poll: NewPoll?,
val language: String?,
)

View File

@ -18,11 +18,12 @@ package app.pachli.core.network.model
import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass
import java.util.Date
@JsonClass(generateAdapter = true)
data class ScheduledStatus(
val id: String,
@Json(name = "scheduled_at") val scheduledAt: String,
@Json(name = "scheduled_at") val scheduledAt: Date,
val params: StatusParams,
@Json(name = "media_attachments") val mediaAttachments: List<Attachment>,
)