diff --git a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala index b9b29dff72f..bafe7a178a5 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala @@ -22,6 +22,7 @@ package org.apache.texera.web.service import com.typesafe.scalalogging.LazyLogging import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity} import org.apache.texera.dao.SqlServer +import org.jooq.exception.DataAccessException import org.apache.texera.dao.jooq.generated.tables.daos.WorkflowExecutionsDao import org.apache.texera.dao.jooq.generated.tables.pojos.WorkflowExecutions import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowVersionResource._ @@ -52,7 +53,7 @@ object ExecutionsMetadataPersistService extends LazyLogging { def insertNewExecution( workflowId: WorkflowIdentity, - uid: Option[Integer], + uid: Integer, executionName: String, environmentVersion: String, computingUnitId: Integer @@ -64,14 +65,25 @@ object ExecutionsMetadataPersistService extends LazyLogging { newExecution.setName(executionName) } newExecution.setVid(vid) - newExecution.setUid(uid.orNull) + newExecution.setUid(uid) newExecution.setStartingTime(new Timestamp(System.currentTimeMillis())) newExecution.setEnvironmentVersion(environmentVersion) // Set computing unit ID if provided newExecution.setCuid(computingUnitId) - workflowExecutionsDao.insert(newExecution) + try { + workflowExecutionsDao.insert(newExecution) + } catch { + // A NOT NULL column (e.g. uid, vid, cuid) was null. Postgres reports this + // as SQLState 23502; surface a readable message instead of the raw + // jOOQ/JDBC stack trace, while preserving the original as the cause. + case e: DataAccessException if e.sqlState() == "23502" => + throw new IllegalArgumentException( + "Cannot persist workflow execution: a required field (uid, vid, or cuid) was null.", + e + ) + } ExecutionIdentity(newExecution.getEid.longValue()) } diff --git a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala index 809faf6a520..0044c959678 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala @@ -192,6 +192,13 @@ class WorkflowService( val (uidOpt, userEmailOpt) = userOpt.map(user => (user.getUid, user.getEmail)).unzip + // uid is NOT NULL in the DB; fail early here rather than letting the insert fail downstream. + val uid = uidOpt.getOrElse( + throw new IllegalArgumentException( + "Cannot start execution: a user id (uid) is required but none was provided." + ) + ) + val workflowContext: WorkflowContext = createWorkflowContext() var controllerConf = ControllerConfig.default @@ -204,7 +211,7 @@ class WorkflowService( workflowContext.executionId = ExecutionsMetadataPersistService.insertNewExecution( workflowContext.workflowId, - uidOpt, + uid, req.executionName, convertToJson(req.engineVersion), req.computingUnitId diff --git a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala index 5c7a687728f..1d67fa03310 100644 --- a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala @@ -166,7 +166,7 @@ class ExecutionsMetadataPersistServiceSpec "insertNewExecution" should "insert a row tied to the latest workflow version" in { val id = ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - Some(testUid), + testUid, executionName = "named-execution", environmentVersion = "env-1", computingUnitId = seededCuid @@ -185,7 +185,7 @@ class ExecutionsMetadataPersistServiceSpec it should "skip setName when executionName is the empty string" in { val id = ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - Some(testUid), + testUid, executionName = "", environmentVersion = "env-2", computingUnitId = seededCuid @@ -198,23 +198,29 @@ class ExecutionsMetadataPersistServiceSpec stored.getName shouldBe "Untitled Execution" } - it should "throw a DB constraint violation when uid is None" in { - // The method signature accepts Option[Integer] for uid and calls - // `newExecution.setUid(uid.orNull)`, but workflow_executions.uid is - // NOT NULL per texera_ddl.sql, so passing None propagates a jOOQ - // DataAccessException. Pinning the current behavior so a future fix — - // either tightening the signature to a required Integer or making the - // column nullable — breaks the spec deliberately. See follow-up bug. - val ex = intercept[org.jooq.exception.DataAccessException] { + it should "let the DB NOT NULL constraint reject a null uid (surfaced as a readable error)" in { + // No more pre-insert require: a null uid is passed straight to jOOQ and the + // DB's NOT NULL constraint rejects it. insertNewExecution catches the + // resulting DataAccessException (SQLState 23502) and rethrows a readable + // IllegalArgumentException. Assert both the readable message and that the + // original DB exception is preserved as the cause, and that nothing was + // written. + val before = workflowExecutionsDao.fetchByVid(seededVid).size() + + val ex = intercept[IllegalArgumentException] { ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - None, + null, // uid is NOT NULL in the DB executionName = "anonymous", environmentVersion = "env-3", computingUnitId = seededCuid ) } ex.getMessage should include("uid") + ex.getCause shouldBe a[org.jooq.exception.DataAccessException] + + // The failed insert must not have persisted a row. + workflowExecutionsDao.fetchByVid(seededVid).size() shouldBe before } // -- tryGetExistingExecution ------------------------------------------------