Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 {
Expand Down
Loading