Yicong-Huang commented on code in PR #8398:
URL: https://github.com/apache/texera/pull/8398#discussion_r3936820619
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala:
##########
@@ -848,6 +848,9 @@ class WorkflowResource extends LazyLogging {
@Path("/type/{wid}")
def getWorkflowType(@PathParam("wid") wid: Integer): String = {
val workflow: Workflow = workflowDao.fetchOneByWid(wid)
+ if (workflow == null) {
+ throw new NotFoundException(s"Workflow with id $wid not found")
Review Comment:
**Must fix:**
`retrievePublicWorkflow` (891 to 897) takes the same path this guard
removes. Its `.fetchOne()` returns null for a wid that is missing, deleted, or
no longer public, and 897 dereferences it. That one is more reachable: the
frontend calls it for shared public links (`workflow-persist.service.ts:47`),
so such a link renders 500 instead of 404. Same three-line fix.
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala:
##########
@@ -947,6 +947,12 @@ class WorkflowResourceSpec
assert(sizes.get(wid) == content.length)
}
+ it should "return NotFoundException for a missing workflow type" in {
Review Comment:
**Polish:**
The name says "return NotFoundException" while the body asserts a throw. The
suite's own sibling case for the same exception reads "throw NotFoundException
for a wid that does not exist" (line 1059).
```suggestion
it should "throw NotFoundException for a missing workflow type" in {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
