diff --git a/service/src/main/kotlin/app/cash/backfila/dashboard/GetBackfillRunsAction.kt b/service/src/main/kotlin/app/cash/backfila/dashboard/GetBackfillRunsAction.kt index 44c27f064..a232b8406 100644 --- a/service/src/main/kotlin/app/cash/backfila/dashboard/GetBackfillRunsAction.kt +++ b/service/src/main/kotlin/app/cash/backfila/dashboard/GetBackfillRunsAction.kt @@ -60,10 +60,12 @@ class GetBackfillRunsAction @Inject constructor( @QueryParam pagination_token: String? = null, @QueryParam backfill_name: String? = null, @QueryParam created_by_user: String? = null, + @QueryParam show_deleted: Boolean = false, ): GetBackfillRunsResponse { val filterArgs = FilterArgs( backfillName = backfill_name, createdByUser = created_by_user, + showDeleted = show_deleted, ) return search(service, variant, pagination_token, filterArgs) } @@ -102,6 +104,7 @@ class GetBackfillRunsAction @Inject constructor( val (pausedBackfills, nextOffset) = queryFactory.newQuery() .serviceId(dbService.id) .stateNot(BackfillState.RUNNING) + .apply { if (!filterArgs.showDeleted) notSoftDeleted() } .filterByArgs(filterArgs) .newPager( idDescPaginator(), @@ -218,5 +221,6 @@ class GetBackfillRunsAction @Inject constructor( private data class FilterArgs( val backfillName: String? = null, val createdByUser: String? = null, + val showDeleted: Boolean = false, ) } diff --git a/service/src/main/kotlin/app/cash/backfila/dashboard/GetBackfillStatusAction.kt b/service/src/main/kotlin/app/cash/backfila/dashboard/GetBackfillStatusAction.kt index f9197e9d0..dd40ec3d2 100644 --- a/service/src/main/kotlin/app/cash/backfila/dashboard/GetBackfillStatusAction.kt +++ b/service/src/main/kotlin/app/cash/backfila/dashboard/GetBackfillStatusAction.kt @@ -69,6 +69,7 @@ data class GetBackfillStatusResponse( val backoff_schedule: String?, val partitions: List, val event_logs: List, + val deleted_at: Instant?, val next_offset: String?, ) @@ -124,6 +125,7 @@ class GetBackfillStatusAction @Inject constructor( event.extra_data, ) }, + run.deleted_at, nextOffset?.offset, ) } diff --git a/service/src/main/kotlin/app/cash/backfila/dashboard/SoftDeleteBackfillAction.kt b/service/src/main/kotlin/app/cash/backfila/dashboard/SoftDeleteBackfillAction.kt new file mode 100644 index 000000000..51cc0d3f8 --- /dev/null +++ b/service/src/main/kotlin/app/cash/backfila/dashboard/SoftDeleteBackfillAction.kt @@ -0,0 +1,55 @@ +package app.cash.backfila.dashboard + +import app.cash.backfila.service.persistence.BackfilaDb +import app.cash.backfila.service.persistence.BackfillState +import app.cash.backfila.service.persistence.DbBackfillRun +import app.cash.backfila.service.persistence.DbEventLog +import javax.inject.Inject +import misk.MiskCaller +import misk.exceptions.BadRequestException +import misk.hibernate.Id +import misk.hibernate.Transacter +import misk.hibernate.load +import misk.scope.ActionScoped +import misk.security.authz.Authenticated +import misk.web.PathParam +import misk.web.Post +import misk.web.RequestContentType +import misk.web.ResponseContentType +import misk.web.actions.WebAction +import misk.web.mediatype.MediaTypes + +class SoftDeleteBackfillAction @Inject constructor( + @BackfilaDb private val transacter: Transacter, + private val caller: @JvmSuppressWildcards ActionScoped, +) : WebAction { + @Post("/backfill/delete/{id}") + @RequestContentType(MediaTypes.APPLICATION_JSON) + @ResponseContentType(MediaTypes.APPLICATION_JSON) + @Authenticated(capabilities = ["users"]) + fun softDelete( + @PathParam id: Long, + ) { + transacter.transaction { session -> + val backfillRun = session.load(Id(id)) + + // Only allow soft delete for COMPLETE or CANCELLED backfills + if (backfillRun.state != BackfillState.COMPLETE && backfillRun.state != BackfillState.CANCELLED) { + throw BadRequestException("Can only delete completed or cancelled backfills") + } + + backfillRun.deleted_at = java.time.Instant.now() + + // Log the deletion event + session.save( + DbEventLog( + backfillRun.id, + partition_id = null, + user = caller.get()?.principal ?: "", + type = DbEventLog.Type.STATE_CHANGE, + message = "backfill soft deleted", + ), + ) + } + } +} diff --git a/service/src/main/kotlin/app/cash/backfila/service/persistence/BackfillRunQuery.kt b/service/src/main/kotlin/app/cash/backfila/service/persistence/BackfillRunQuery.kt index 8ddbe8f90..8730ce627 100644 --- a/service/src/main/kotlin/app/cash/backfila/service/persistence/BackfillRunQuery.kt +++ b/service/src/main/kotlin/app/cash/backfila/service/persistence/BackfillRunQuery.kt @@ -30,4 +30,7 @@ interface BackfillRunQuery : Query { @Order("updated_at", asc = false) fun orderByUpdatedAtDesc(): BackfillRunQuery + + @Constraint("deleted_at", Operator.IS_NULL) + fun notSoftDeleted(): BackfillRunQuery } diff --git a/service/src/main/kotlin/app/cash/backfila/service/persistence/DbBackfillRun.kt b/service/src/main/kotlin/app/cash/backfila/service/persistence/DbBackfillRun.kt index 2e6d4fea8..75782e390 100644 --- a/service/src/main/kotlin/app/cash/backfila/service/persistence/DbBackfillRun.kt +++ b/service/src/main/kotlin/app/cash/backfila/service/persistence/DbBackfillRun.kt @@ -90,6 +90,9 @@ class DbBackfillRun() : DbUnsharded, DbTimestampedEntity { @Column(nullable = false) var dry_run: Boolean = false + @Column(nullable = true) + var deleted_at: Instant? = null + /** Comma separated list of delays for consecutive retries in milliseconds, e.g. 1000,2000 */ @Column var backoff_schedule: String? = null diff --git a/service/src/main/kotlin/app/cash/backfila/ui/actions/BackfillShowButtonHandlerAction.kt b/service/src/main/kotlin/app/cash/backfila/ui/actions/BackfillShowButtonHandlerAction.kt index 426734dae..c79822267 100644 --- a/service/src/main/kotlin/app/cash/backfila/ui/actions/BackfillShowButtonHandlerAction.kt +++ b/service/src/main/kotlin/app/cash/backfila/ui/actions/BackfillShowButtonHandlerAction.kt @@ -1,6 +1,7 @@ package app.cash.backfila.ui.actions import app.cash.backfila.dashboard.CancelBackfillAction +import app.cash.backfila.dashboard.SoftDeleteBackfillAction import app.cash.backfila.dashboard.StartBackfillAction import app.cash.backfila.dashboard.StartBackfillRequest import app.cash.backfila.dashboard.StopBackfillAction @@ -33,6 +34,7 @@ class BackfillShowButtonHandlerAction @Inject constructor( private val stopBackfillAction: StopBackfillAction, private val updateBackfillAction: UpdateBackfillAction, private val cancelBackfillAction: CancelBackfillAction, + private val softDeleteBackfillAction: SoftDeleteBackfillAction, ) : WebAction { @Get(PATH) @ResponseContentType(MediaTypes.TEXT_HTML) @@ -56,6 +58,9 @@ class BackfillShowButtonHandlerAction @Inject constructor( BackfillState.CANCELLED.name -> { cancelBackfillAction.cancel(id.toLong()) } + "soft_delete" -> { + softDeleteBackfillAction.softDelete(id.toLong()) + } } } diff --git a/service/src/main/kotlin/app/cash/backfila/ui/components/BackfillsTable.kt b/service/src/main/kotlin/app/cash/backfila/ui/components/BackfillsTable.kt index 49ba61dde..1710f764f 100644 --- a/service/src/main/kotlin/app/cash/backfila/ui/components/BackfillsTable.kt +++ b/service/src/main/kotlin/app/cash/backfila/ui/components/BackfillsTable.kt @@ -2,10 +2,15 @@ package app.cash.backfila.ui.components import app.cash.backfila.dashboard.UiBackfillRun import app.cash.backfila.ui.pages.BackfillShowAction +import kotlinx.html.InputType import kotlinx.html.TagConsumer import kotlinx.html.a import kotlinx.html.div +import kotlinx.html.form import kotlinx.html.h1 +import kotlinx.html.input +import kotlinx.html.label +import kotlinx.html.section import kotlinx.html.table import kotlinx.html.tbody import kotlinx.html.td @@ -13,13 +18,30 @@ import kotlinx.html.th import kotlinx.html.thead import kotlinx.html.tr -fun TagConsumer<*>.BackfillsTable(running: Boolean, backfills: List) { +fun TagConsumer<*>.BackfillsTable( + running: Boolean, + backfills: List, + showDeleted: Boolean = false, +) { val title = if (running) "Running" else "Paused" div("px-4 sm:px-6 lg:px-8 py-5") { - div("sm:flex sm:items-center") { - div("sm:flex-auto") { - h1("text-base font-semibold leading-6 text-gray-900") { +"""Backfills ($title)""" } + section("sm:flex sm:items-center justify-between") { + h1("text-base font-semibold leading-6 text-gray-900") { +"""Backfills ($title)""" } + + if (!running) { // Only show toggle for Paused backfills + form { + label("text-sm font-medium text-gray-600 flex items-center") { + +"Show deleted" + input(classes = "ml-2 h-4 w-4 rounded border-gray-300 text-indigo-600 focus:ring-indigo-600") { + type = InputType.checkBox + name = "showDeleted" + value = "true" + if (showDeleted) attributes["checked"] = "checked" + attributes["onchange"] = "this.form.submit()" + } + } + } } } div("mt-8 flow-root") { diff --git a/service/src/main/kotlin/app/cash/backfila/ui/pages/BackfillShowAction.kt b/service/src/main/kotlin/app/cash/backfila/ui/pages/BackfillShowAction.kt index 9350f170b..0ca1b55b2 100644 --- a/service/src/main/kotlin/app/cash/backfila/ui/pages/BackfillShowAction.kt +++ b/service/src/main/kotlin/app/cash/backfila/ui/pages/BackfillShowAction.kt @@ -11,6 +11,7 @@ import app.cash.backfila.ui.components.PageTitle import app.cash.backfila.ui.components.Pagination import app.cash.backfila.ui.components.ProgressBar import app.cash.backfila.ui.pages.BackfillCreateAction.BackfillCreateField.CUSTOM_PARAMETER_PREFIX +import java.time.Instant import javax.inject.Inject import javax.inject.Singleton import kotlinx.html.ButtonType @@ -268,6 +269,7 @@ class BackfillShowAction @Inject constructor( val button: Link? = null, val updateFieldId: String? = null, val cancelButton: Link? = null, + val deleteButton: Link? = null, ) private fun getStateButton(state: BackfillState): Link? { @@ -296,6 +298,19 @@ class BackfillShowAction @Inject constructor( } } + private fun getDeleteButton(state: BackfillState, deletedAt: Instant?): Link? { + if (deletedAt != null) { + return null + } + return when (state) { + BackfillState.COMPLETE, BackfillState.CANCELLED -> Link( + label = DELETE_STATE_BUTTON_LABEL, + href = "soft_delete", + ) + else -> null + } + } + private fun GetBackfillStatusResponse.toConfigurationRows(id: Long) = listOf( DescriptionListRow( label = "State", @@ -303,6 +318,7 @@ class BackfillShowAction @Inject constructor( button = getStateButton(state), updateFieldId = "state", cancelButton = getCancelButton(state), + deleteButton = getDeleteButton(state, deleted_at), ), DescriptionListRow( label = "Dry Run", @@ -541,6 +557,35 @@ class BackfillShowAction @Inject constructor( } } } + + it.deleteButton?.let { deleteButton -> + span("ml-2") { + form { + action = BackfillShowButtonHandlerAction.path(id) + + it.updateFieldId?.let { + input { + type = InputType.hidden + name = "field_id" + value = it + } + + input { + type = InputType.hidden + name = "field_value" + value = deleteButton.href + } + } + + button( + classes = "rounded-full bg-gray-600 px-3 py-1.5 text-sm font-semibold text-white shadow-sm hover:bg-gray-500 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-gray-600", + ) { + type = ButtonType.submit + +deleteButton.label + } + } + } + } } } } @@ -590,6 +635,7 @@ class BackfillShowAction @Inject constructor( const val START_STATE_BUTTON_LABEL = "Start" const val PAUSE_STATE_BUTTON_LABEL = "Pause" const val CANCEL_STATE_BUTTON_LABEL = "Cancel" + const val DELETE_STATE_BUTTON_LABEL = "Delete" const val UPDATE_BUTTON_LABEL = "Update" const val VIEW_LOGS_BUTTON_LABEL = "View Logs" } diff --git a/service/src/main/kotlin/app/cash/backfila/ui/pages/ServiceShowAction.kt b/service/src/main/kotlin/app/cash/backfila/ui/pages/ServiceShowAction.kt index 4aaace693..cc0f74a9a 100644 --- a/service/src/main/kotlin/app/cash/backfila/ui/pages/ServiceShowAction.kt +++ b/service/src/main/kotlin/app/cash/backfila/ui/pages/ServiceShowAction.kt @@ -45,6 +45,7 @@ class ServiceShowAction @Inject constructor( @PathParam variantOrBlank: String? = "", @QueryParam offset: String? = null, @QueryParam lastOffset: String? = null, + @QueryParam showDeleted: Boolean = false, ): Response { if (service.isNullOrBlank()) { return Response( @@ -55,7 +56,12 @@ class ServiceShowAction @Inject constructor( } val variant = variantOrBlank.orEmpty().ifBlank { "default" } - val backfillRuns = getBackfillRunsAction.backfillRuns(service, variant, offset) + val backfillRuns = getBackfillRunsAction.backfillRuns( + service = service, + variant = variant, + pagination_token = offset, + show_deleted = showDeleted, + ) // TODO show default if other variants and probably link to a switcher val label = if (variant == "default") service else "$service ($variant)" @@ -79,7 +85,7 @@ class ServiceShowAction @Inject constructor( } BackfillsTable(true, backfillRuns.running_backfills) - BackfillsTable(false, backfillRuns.paused_backfills) + BackfillsTable(false, backfillRuns.paused_backfills, showDeleted) Pagination(backfillRuns.next_pagination_token, offset, lastOffset, path(service, variantOrBlank)) } } diff --git a/service/src/main/resources/migrations/v019__backfila.sql b/service/src/main/resources/migrations/v019__backfila.sql new file mode 100644 index 000000000..7b6bd1ada --- /dev/null +++ b/service/src/main/resources/migrations/v019__backfila.sql @@ -0,0 +1,2 @@ +ALTER TABLE backfill_runs + ADD COLUMN `deleted_at` timestamp(3) NULL DEFAULT NULL; \ No newline at end of file diff --git a/service/src/test/kotlin/app/cash/backfila/actions/SoftDeleteBackfillActionTest.kt b/service/src/test/kotlin/app/cash/backfila/actions/SoftDeleteBackfillActionTest.kt new file mode 100644 index 000000000..a6f32fb8c --- /dev/null +++ b/service/src/test/kotlin/app/cash/backfila/actions/SoftDeleteBackfillActionTest.kt @@ -0,0 +1,176 @@ +package app.cash.backfila.actions + +import app.cash.backfila.BackfilaTestingModule +import app.cash.backfila.api.ConfigureServiceAction +import app.cash.backfila.client.Connectors +import app.cash.backfila.dashboard.CreateBackfillAction +import app.cash.backfila.dashboard.GetBackfillStatusAction +import app.cash.backfila.dashboard.SoftDeleteBackfillAction +import app.cash.backfila.fakeCaller +import app.cash.backfila.protos.service.ConfigureServiceRequest +import app.cash.backfila.protos.service.CreateBackfillRequest +import app.cash.backfila.service.persistence.BackfilaDb +import app.cash.backfila.service.persistence.BackfillState +import app.cash.backfila.service.persistence.DbBackfillRun +import com.google.inject.Module +import javax.inject.Inject +import misk.exceptions.BadRequestException +import misk.hibernate.Id +import misk.hibernate.Query +import misk.hibernate.Transacter +import misk.hibernate.load +import misk.scope.ActionScope +import misk.testing.MiskTest +import misk.testing.MiskTestModule +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test + +@MiskTest(startService = true) +class SoftDeleteBackfillActionTest { + @Suppress("unused") + @MiskTestModule + val module: Module = BackfilaTestingModule() + + @Inject lateinit var configureServiceAction: ConfigureServiceAction + + @Inject lateinit var createBackfillAction: CreateBackfillAction + + @Inject lateinit var getBackfillStatusAction: GetBackfillStatusAction + + @Inject lateinit var softDeleteBackfillAction: SoftDeleteBackfillAction + + @Inject @BackfilaDb + lateinit var transacter: Transacter + + @Inject lateinit var queryFactory: Query.Factory + + @Inject lateinit var scope: ActionScope + + @Test + fun `soft delete completed backfill`() { + // Setup test backfill + val backfillId = setupTestBackfill() + + // Set state to COMPLETE + transacter.transaction { session -> + val backfill = session.load(Id(backfillId)) + backfill.setState(session, queryFactory, BackfillState.COMPLETE) + } + + // Soft delete the backfill + scope.fakeCaller(user = "molly") { + softDeleteBackfillAction.softDelete(backfillId) + } + + // Verify the backfill is soft deleted + transacter.transaction { session -> + val backfill = session.load(Id(backfillId)) + assertThat(backfill.deleted_at).isNotNull() + } + + // Verify event log + val status = getBackfillStatusAction.status(backfillId) + assertThat(status.event_logs[0].message).isEqualTo("backfill soft deleted") + assertThat(status.event_logs[0].user).isEqualTo("molly") + } + + @Test + fun `soft delete cancelled backfill`() { + // Setup test backfill + val backfillId = setupTestBackfill() + + // Set state to CANCELLED + transacter.transaction { session -> + val backfill = session.load(Id(backfillId)) + backfill.setState(session, queryFactory, BackfillState.CANCELLED) + } + + // Soft delete the backfill + scope.fakeCaller(user = "molly") { + softDeleteBackfillAction.softDelete(backfillId) + } + + // Verify the backfill is soft deleted + transacter.transaction { session -> + val backfill = session.load(Id(backfillId)) + assertThat(backfill.deleted_at).isNotNull() + } + } + + @Test + fun `cannot soft delete running backfill`() { + // Setup test backfill + val backfillId = setupTestBackfill() + + // Set state to RUNNING + transacter.transaction { session -> + val backfill = session.load(Id(backfillId)) + backfill.setState(session, queryFactory, BackfillState.RUNNING) + } + + // Attempt to soft delete running backfill + scope.fakeCaller(user = "molly") { + assertThatThrownBy { + softDeleteBackfillAction.softDelete(backfillId) + }.isInstanceOf(BadRequestException::class.java) + .hasMessageContaining("Can only delete completed or cancelled backfills") + } + + // Verify the backfill is not deleted + transacter.transaction { session -> + val backfill = session.load(Id(backfillId)) + assertThat(backfill.deleted_at).isNull() + } + } + + @Test + fun `cannot soft delete paused backfill`() { + // Setup test backfill (default state is PAUSED) + val backfillId = setupTestBackfill() + + // Attempt to soft delete paused backfill + scope.fakeCaller(user = "molly") { + assertThatThrownBy { + softDeleteBackfillAction.softDelete(backfillId) + }.isInstanceOf(BadRequestException::class.java) + .hasMessageContaining("Can only delete completed or cancelled backfills") + } + + // Verify the backfill is not deleted + transacter.transaction { session -> + val backfill = session.load(Id(backfillId)) + assertThat(backfill.deleted_at).isNull() + } + } + + private fun setupTestBackfill(): Long { + scope.fakeCaller(service = "deep-fryer") { + configureServiceAction.configureService( + ConfigureServiceRequest.Builder() + .backfills( + listOf( + ConfigureServiceRequest.BackfillData( + "ChickenSandwich", "Description", listOf(), null, + null, false, null, + ), + ), + ) + .connector_type(Connectors.ENVOY) + .build(), + ) + } + + val response = scope.fakeCaller(user = "molly") { + createBackfillAction.create( + "deep-fryer", + ConfigureServiceAction.RESERVED_VARIANT, + CreateBackfillRequest.Builder() + .backfill_name("ChickenSandwich") + .build(), + ) + } + + return response.backfill_run_id + } +}