This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 51b6af3574 feat(python-notebook-migration): add backend endpoint to
delete a workflow's stored notebook and mapping (#7132)
51b6af3574 is described below
commit 51b6af3574f41fa3f477a50eee7c7c0c5bc7bee4
Author: Ryan Zhang <[email protected]>
AuthorDate: Thu Jul 30 15:58:13 2026 -0700
feat(python-notebook-migration): add backend endpoint to delete a
workflow's stored notebook and mapping (#7132)
### What changes were proposed in this PR?
Adds a single backend REST endpoint to the notebook-migration service
that deletes a workflow's stored notebook and its workflow-to-notebook
mapping.
Until now the service could store and fetch a workflow's notebook
(`store-notebook-and-mapping`, `fetch-notebook-and-mapping`) but had no
way to remove one. Once stored, a notebook and its mapping stayed in the
database with no server-side operation to clear them. This left the
persisted rows behind whenever a user closed or discarded a migrated
notebook, and it blocked the end-to-end deletion flow, which needs a
backend call to remove the notebook before the UI can reset the state
that reflects whether a notebook exists.
New endpoint: `POST /notebook-migration/delete-notebook-and-mapping`.
Behavior:
- Reads `wid` from the JSON body. `vid` is not required: `notebook.wid`
is UNIQUE (one notebook per workflow), so `wid` alone identifies the
row.
- Requires write access to the workflow. Returns 403 when the caller
lacks it, consistent with the store and fetch endpoints.
- Deletes the `notebook` row for the workflow. The
`workflow_notebook_mapping` rows are removed by the existing `ON DELETE
CASCADE` foreign key, so a single delete clears both tables and cannot
leave them inconsistent.
- Returns `{"success": true, "deleted": <count>}`, where the count is 1
when a notebook was removed and 0 when nothing was stored, so the caller
can tell a real deletion from a no-op.
- Idempotent: deleting when nothing is stored returns success with
`deleted: 0`.
POST with a JSON body is used to stay consistent with the existing store
and fetch endpoints, which already read `wid` from a JSON body.
This is backend only. The frontend service method, the close-panel
wiring, and any Jupyter-server cleanup are intentionally left to the
follow-on end-to-end deletion work that consumes this endpoint.
Note: if the workflow itself is deleted, the notebook and mapping are
already removed by existing cascades, so this endpoint targets the case
where the workflow survives but its notebook should be discarded.
### Any related issues, documentation, discussions?
Closes #7131
Parent issue #4301
### How was this PR tested?
Added resource spec cases in `NotebookMigrationResourceSpec` covering:
- Deleting a workflow that has a stored notebook removes both the
notebook row and its mapping rows (cascade verified) and reports
`deleted: 1`.
- Idempotent delete: returns success with `deleted: 0` when nothing is
stored.
- Returns 403 and deletes nothing when the caller has only read access.
- Returns 500 on a malformed JSON body.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
---
.../resource/NotebookMigrationResource.scala | 86 +++++++++++++++++++++-
.../resource/NotebookMigrationResourceSpec.scala | 78 ++++++++++++++++++++
2 files changed, 161 insertions(+), 3 deletions(-)
diff --git
a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
index b8ebbb9f19..ac41ccd28d 100644
---
a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
+++
b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
@@ -17,7 +17,7 @@
package org.apache.texera.service.resource
-import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.databind.{JsonNode, ObjectMapper}
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import com.typesafe.scalalogging.LazyLogging
import io.dropwizard.auth.Auth
@@ -49,6 +49,30 @@ object NotebookMigrationResource extends LazyLogging {
private def successUrlJson(url: String): String =
mapper.writeValueAsString(mapper.createObjectNode().put("success",
true).put("url", url))
+ // Build a {"success": true, "deleted": <count>} body via the mapper. The
count lets the
+ // caller distinguish a real deletion (1) from a no-op when nothing was
stored (0).
+ private def successDeletedJson(deleted: Int): String =
+ mapper.writeValueAsString(
+ mapper.createObjectNode().put("success", true).put("deleted", deleted)
+ )
+
+ // Read the required integer `wid` from a request body. Returns Left(400)
when the field is
+ // missing or not an integer so the caller can short-circuit. Without this a
missing wid NPEs
+ // into a 500 and a non-integer wid silently coerces to 0 via asInt().
+ private def readWid(json: JsonNode): Either[Response, java.lang.Integer] = {
+ val widNode = json.get("wid")
+ if (widNode == null || !widNode.isInt) {
+ Left(
+ Response
+ .status(Response.Status.BAD_REQUEST)
+ .entity(errorJson("Missing or invalid 'wid'"))
+ .build()
+ )
+ } else {
+ Right(widNode.asInt())
+ }
+ }
+
private val jupyterUrl = StorageConfig.jupyterURL
private val jupyterToken = StorageConfig.jupyterToken
// The token is passed as a URL param so the browser iframe can authenticate
when loading the notebook.
@@ -223,7 +247,10 @@ object NotebookMigrationResource extends LazyLogging {
try {
val json = mapper.readTree(body)
- val wid: java.lang.Integer = json.get("wid").asInt()
+ val wid: java.lang.Integer = readWid(json) match {
+ case Left(badRequest) => return badRequest
+ case Right(w) => w
+ }
val vid: java.lang.Integer = json.get("vid").asInt()
val mappingNode = json.get("mapping")
val notebookNode = json.get("notebook")
@@ -311,7 +338,10 @@ object NotebookMigrationResource extends LazyLogging {
try {
val json = mapper.readTree(body)
- val wid: java.lang.Integer = json.get("wid").asInt()
+ val wid: java.lang.Integer = readWid(json) match {
+ case Left(badRequest) => return badRequest
+ case Right(w) => w
+ }
val vid: java.lang.Integer = json.get("vid").asInt()
// Only a user with write access to the workflow may fetch its notebook.
@@ -374,6 +404,49 @@ object NotebookMigrationResource extends LazyLogging {
.build()
}
}
+
+ // Delete notebook + mapping for a workflow. The notebook ->
workflow_notebook_mapping FK is
+ // ON DELETE CASCADE, so deleting the notebook row removes its mapping rows
too. notebook.wid
+ // is UNIQUE (one notebook per workflow), so wid alone identifies the row
and vid is not needed.
+ def deleteNotebookAndMapping(body: String, uid: java.lang.Integer): Response
= {
+ try {
+ val json = mapper.readTree(body)
+
+ val wid: java.lang.Integer = readWid(json) match {
+ case Left(badRequest) => return badRequest
+ case Right(w) => w
+ }
+
+ // Only a user with write access to the workflow may delete its notebook.
+ if (!WorkflowAccessResource.hasWriteAccess(wid, uid)) {
+ return Response
+ .status(Response.Status.FORBIDDEN)
+ .entity(errorJson(s"No write access to workflow $wid"))
+ .build()
+ }
+
+ val dsl = SqlServer.getInstance().createDSLContext()
+
+ // execute() returns the affected row count: 1 when a notebook was
removed, 0 when the
+ // workflow had nothing stored (idempotent no-op).
+ val deleted: Int = SqlServer.withTransaction(dsl) { ctx =>
+ ctx
+ .deleteFrom(Notebook.NOTEBOOK)
+ .where(Notebook.NOTEBOOK.WID.eq(wid))
+ .execute()
+ }
+
+ Response.ok(successDeletedJson(deleted)).build()
+
+ } catch {
+ case NonFatal(e) =>
+ logger.error("Error deleting notebook and mapping", e)
+ Response
+ .status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity(errorJson(e.getMessage))
+ .build()
+ }
+ }
}
@Path("/notebook-migration")
@@ -416,4 +489,11 @@ class NotebookMigrationResource extends LazyLogging {
logger.info("Fetching notebook and mapping")
NotebookMigrationResource.fetchNotebookAndMapping(body, user.getUid)
}
+
+ @POST
+ @Path("/delete-notebook-and-mapping")
+ def deleteNotebookAndMapping(body: String, @Auth user: SessionUser):
Response = {
+ logger.info("Deleting notebook and mapping")
+ NotebookMigrationResource.deleteNotebookAndMapping(body, user.getUid)
+ }
}
diff --git
a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
index 35c3f47377..ea6505a86b 100644
---
a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
+++
b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
@@ -161,6 +161,9 @@ class NotebookMigrationResourceSpec
private def fetchPayload(vid: Integer = seededVid): String =
s"""{"wid": $testWid, "vid": $vid}"""
+ private def deletePayload(): String =
+ s"""{"wid": $testWid}"""
+
private val resource = new NotebookMigrationResource()
private def sessionUser(uid: Integer): SessionUser = {
@@ -326,6 +329,81 @@ class NotebookMigrationResourceSpec
entity should include("\"v1\"")
}
+ // -- deleteNotebookAndMapping
-----------------------------------------------
+
+ "deleteNotebookAndMapping" should "remove the notebook and cascade to its
mapping, reporting deleted=1" in {
+ NotebookMigrationResource.storeNotebookAndMapping(storePayload(),
writerUid)
+ getDSLContext.fetchCount(NOTEBOOK) shouldBe 1
+ getDSLContext.fetchCount(WORKFLOW_NOTEBOOK_MAPPING) shouldBe 1
+
+ val response =
NotebookMigrationResource.deleteNotebookAndMapping(deletePayload(), writerUid)
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getEntity.toString should include("\"deleted\":1")
+
+ // Deleting the notebook row cascades to workflow_notebook_mapping via the
FK.
+ getDSLContext.fetchCount(NOTEBOOK) shouldBe 0
+ getDSLContext.fetchCount(WORKFLOW_NOTEBOOK_MAPPING) shouldBe 0
+ }
+
+ it should "be idempotent, returning success with deleted=0 when nothing is
stored" in {
+ val response =
NotebookMigrationResource.deleteNotebookAndMapping(deletePayload(), writerUid)
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getEntity.toString should include("\"deleted\":0")
+ }
+
+ it should "return 403 Forbidden and delete nothing when the user lacks write
access" in {
+ NotebookMigrationResource.storeNotebookAndMapping(storePayload(),
writerUid)
+
+ // readerUid holds only READ access; delete requires WRITE.
+ NotebookMigrationResource
+ .deleteNotebookAndMapping(deletePayload(), readerUid)
+ .getStatus shouldBe Response.Status.FORBIDDEN.getStatusCode
+
+ getDSLContext.fetchCount(NOTEBOOK) shouldBe 1
+ getDSLContext.fetchCount(WORKFLOW_NOTEBOOK_MAPPING) shouldBe 1
+ }
+
+ it should "return 500 when the request body is malformed JSON" in {
+ // Exercises the NonFatal catch path in deleteNotebookAndMapping.
+ resource
+ .deleteNotebookAndMapping("not json", sessionUser(writerUid))
+ .getStatus shouldBe 500
+ }
+
+ // -- wid validation
---------------------------------------------------------
+
+ "store/fetch/delete" should "return 400 Bad Request when 'wid' is missing
from the body" in {
+ // A missing wid must be a client error, not a 500 from the null.asInt()
NPE.
+ val noWid = s"""{"vid": $seededVid}"""
+ NotebookMigrationResource
+ .storeNotebookAndMapping(noWid, writerUid)
+ .getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
+ NotebookMigrationResource
+ .fetchNotebookAndMapping(noWid, writerUid)
+ .getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
+ NotebookMigrationResource
+ .deleteNotebookAndMapping("""{}""", writerUid)
+ .getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
+
+ getDSLContext.fetchCount(NOTEBOOK) shouldBe 0
+ }
+
+ it should "return 400 Bad Request when 'wid' is not an integer" in {
+ // A non-integer wid must be rejected rather than silently coerced to 0 by
asInt().
+ val badWid = s"""{"wid": "not-an-int", "vid": $seededVid}"""
+ NotebookMigrationResource
+ .storeNotebookAndMapping(badWid, writerUid)
+ .getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
+ NotebookMigrationResource
+ .fetchNotebookAndMapping(badWid, writerUid)
+ .getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
+ NotebookMigrationResource
+ .deleteNotebookAndMapping(badWid, writerUid)
+ .getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
+
+ getDSLContext.fetchCount(NOTEBOOK) shouldBe 0
+ }
+
// -- workflow write-access enforcement
--------------------------------------
"store/fetch" should "return 403 Forbidden when the user lacks write access
to the workflow" in {
