aicam commented on code in PR #3728:
URL: https://github.com/apache/texera/pull/3728#discussion_r2392295748


##########
core/amber/src/main/scala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala:
##########
@@ -688,76 +690,76 @@ class WorkflowExecutionsResource {
   }
 
   @POST
-  @Path("/result/export")
+  @Path("/result/export/dataset")
   @RolesAllowed(Array("REGULAR", "ADMIN"))
-  def exportResult(
-      request: ResultExportRequest,
-      @Auth user: SessionUser
-  ): Response = {
+  def exportResult(request: ResultExportRequest, @Auth user: SessionUser): 
Response = {
+    try {
+      val resultExportService =
+        new ResultExportService(WorkflowIdentity(request.workflowId), 
request.computingUnitId)
+      resultExportService.validateExportRequest(request) match {
+        case Some(errorResponse) => errorResponse
+        case None                => 
resultExportService.exportToDataset(user.user, request)
+      }
+    } catch {
+      case ex: Exception =>
+        Response
+          .status(Response.Status.INTERNAL_SERVER_ERROR)
+          .`type`(MediaType.APPLICATION_JSON)
+          .entity(Map("error" -> ex.getMessage).asJava)
+          .build()
+    }
+  }
 
-    if (request.operators.size <= 0)
-      Response
-        .status(Response.Status.BAD_REQUEST)
-        .`type`(MediaType.APPLICATION_JSON)
-        .entity(Map("error" -> "No operator selected").asJava)
-        .build()
+  @POST
+  @Path("/result/export/local")
+  @Consumes(Array(MediaType.APPLICATION_FORM_URLENCODED))
+  def exportResultViaBrowser(
+      @FormParam("exportType") exportType: String,
+      @FormParam("workflowId") workflowId: Int,
+      @FormParam("workflowName") workflowName: String,
+      @FormParam("operators") operatorsJson: String,
+      @FormParam("datasetIds") datasetIdsJson: String,
+      @FormParam("rowIndex") rowIndex: Int,
+      @FormParam("columnIndex") columnIndex: Int,
+      @FormParam("filename") filename: String,
+      @FormParam("destination") destination: String,
+      @FormParam("computingUnitId") computingUnitId: Int,
+      @FormParam("token") token: String
+  ): Response = {
 
     try {
-      request.destination match {
-        case "local" =>
-          // CASE A: multiple operators => produce ZIP
-          if (request.operators.size > 1) {
-            val resultExportService =
-              new ResultExportService(WorkflowIdentity(request.workflowId), 
request.computingUnitId)
-            val (zipStream, zipFileNameOpt) =
-              resultExportService.exportOperatorsAsZip(request)
-
-            if (zipStream == null) {
-              throw new RuntimeException("Zip stream is null")
-            }
-
-            val finalFileName = zipFileNameOpt.getOrElse("operators.zip")
-            return Response
-              .ok(zipStream, "application/zip")
-              .header("Content-Disposition", "attachment; filename=\"" + 
finalFileName + "\"")
-              .build()
-          }
+      val userOpt = JwtParser.parseToken(token)
+      if (userOpt.isPresent) {
+        val user = userOpt.get()
+        val role = user.getUser.getRole
+        val RolesAllowed = Set(UserRoleEnum.REGULAR, UserRoleEnum.ADMIN)
+        if (!RolesAllowed.contains(role)) {
+          throw new RuntimeException("User role is not allowed to perform this 
download")
+        }
+      } else {
+        throw new RuntimeException("Invalid or expired token")
+      }
 
-          // CASE B: exactly one operator => single file
-          if (request.operators.size != 1) {
-            return Response
-              .status(Response.Status.BAD_REQUEST)
-              .`type`(MediaType.APPLICATION_JSON)
-              .entity(Map("error" -> "Local download does not support no 
operator.").asJava)
-              .build()
-          }
-          val singleOp = request.operators.head
-
-          val resultExportService =
-            new ResultExportService(WorkflowIdentity(request.workflowId), 
request.computingUnitId)
-          val (streamingOutput, fileNameOpt) =
-            resultExportService.exportOperatorResultAsStream(request, singleOp)
-
-          if (streamingOutput == null) {
-            return Response
-              .status(Response.Status.INTERNAL_SERVER_ERROR)
-              .`type`(MediaType.APPLICATION_JSON)
-              .entity(Map("error" -> "Failed to export operator").asJava)
-              .build()
-          }
+      val resultExportService =
+        new ResultExportService(WorkflowIdentity(workflowId), computingUnitId)
+      val operators = resultExportService.parseOperators(operatorsJson)
+      val datasetIds: List[Int] = List()
+      val request = ResultExportRequest(

Review Comment:
   Please do some search and see if you can convert form data into an object in 
Java using some built-in functions instead of manually creating



##########
core/gui/src/app/dashboard/service/user/download/download.service.ts:
##########
@@ -150,30 +155,99 @@ export class DownloadService {
 
     const urlPath =
       unit && unit.computingUnit.type == "kubernetes" && 
unit.computingUnit?.cuid
-        ? 
`${WORKFLOW_EXECUTIONS_API_BASE_URL}/${EXPORT_BASE_URL}?cuid=${unit.computingUnit.cuid}`
-        : `${WORKFLOW_EXECUTIONS_API_BASE_URL}/${EXPORT_BASE_URL}`;
-    if (destination === "local") {
-      return this.http.post(urlPath, requestBody, {
-        responseType: "blob",
-        observe: "response",
-        headers: {
-          "Content-Type": "application/json",
-          Accept: "application/octet-stream",
-        },
-      });
-    } else {
-      // dataset => return JSON
-      return this.http.post<ExportWorkflowJsonResponse>(urlPath, requestBody, {
-        responseType: "json",
-        observe: "response",
-        headers: {
-          "Content-Type": "application/json",
-          Accept: "application/json",
-        },
-      });
+        ? 
`${WORKFLOW_EXECUTIONS_API_BASE_URL}/${EXPORT_BASE_URL}/dataset?cuid=${unit.computingUnit.cuid}`
+        : `${WORKFLOW_EXECUTIONS_API_BASE_URL}/${EXPORT_BASE_URL}/dataset`;
+
+    return this.http.post<ExportWorkflowJsonResponse>(urlPath, requestBody, {
+      responseType: "json",
+      observe: "response",
+      headers: {
+        "Content-Type": "application/json",
+        Accept: "application/json",
+      },
+    });
+  }
+
+  /**
+   * Export the workflow result to local filesystem. The export is handled by 
the browser.
+   */
+  public exportWorkflowResultToLocal(
+    exportType: string,
+    workflowId: number,
+    workflowName: string,
+    operators: {
+      id: string;
+      outputType: string;
+    }[],
+    datasetIds: number[],

Review Comment:
   Ditto, why we need datasetIds?



##########
core/amber/src/main/scala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala:
##########
@@ -688,76 +690,76 @@ class WorkflowExecutionsResource {
   }
 
   @POST
-  @Path("/result/export")
+  @Path("/result/export/dataset")
   @RolesAllowed(Array("REGULAR", "ADMIN"))
-  def exportResult(
-      request: ResultExportRequest,
-      @Auth user: SessionUser
-  ): Response = {
+  def exportResult(request: ResultExportRequest, @Auth user: SessionUser): 
Response = {
+    try {
+      val resultExportService =
+        new ResultExportService(WorkflowIdentity(request.workflowId), 
request.computingUnitId)
+      resultExportService.validateExportRequest(request) match {
+        case Some(errorResponse) => errorResponse
+        case None                => 
resultExportService.exportToDataset(user.user, request)
+      }
+    } catch {
+      case ex: Exception =>
+        Response
+          .status(Response.Status.INTERNAL_SERVER_ERROR)
+          .`type`(MediaType.APPLICATION_JSON)
+          .entity(Map("error" -> ex.getMessage).asJava)
+          .build()
+    }
+  }
 
-    if (request.operators.size <= 0)
-      Response
-        .status(Response.Status.BAD_REQUEST)
-        .`type`(MediaType.APPLICATION_JSON)
-        .entity(Map("error" -> "No operator selected").asJava)
-        .build()
+  @POST
+  @Path("/result/export/local")
+  @Consumes(Array(MediaType.APPLICATION_FORM_URLENCODED))

Review Comment:
   Why do we need datasetId here? I see the endpoint for `local` and `dataset` 
are separated.



##########
core/gui/src/app/dashboard/service/user/download/download.service.ts:
##########
@@ -150,30 +155,99 @@ export class DownloadService {
 
     const urlPath =
       unit && unit.computingUnit.type == "kubernetes" && 
unit.computingUnit?.cuid
-        ? 
`${WORKFLOW_EXECUTIONS_API_BASE_URL}/${EXPORT_BASE_URL}?cuid=${unit.computingUnit.cuid}`
-        : `${WORKFLOW_EXECUTIONS_API_BASE_URL}/${EXPORT_BASE_URL}`;
-    if (destination === "local") {
-      return this.http.post(urlPath, requestBody, {
-        responseType: "blob",
-        observe: "response",
-        headers: {
-          "Content-Type": "application/json",
-          Accept: "application/octet-stream",
-        },
-      });
-    } else {
-      // dataset => return JSON
-      return this.http.post<ExportWorkflowJsonResponse>(urlPath, requestBody, {
-        responseType: "json",
-        observe: "response",
-        headers: {
-          "Content-Type": "application/json",
-          Accept: "application/json",
-        },
-      });
+        ? 
`${WORKFLOW_EXECUTIONS_API_BASE_URL}/${EXPORT_BASE_URL}/dataset?cuid=${unit.computingUnit.cuid}`
+        : `${WORKFLOW_EXECUTIONS_API_BASE_URL}/${EXPORT_BASE_URL}/dataset`;
+
+    return this.http.post<ExportWorkflowJsonResponse>(urlPath, requestBody, {
+      responseType: "json",
+      observe: "response",
+      headers: {
+        "Content-Type": "application/json",
+        Accept: "application/json",
+      },
+    });
+  }
+
+  /**
+   * Export the workflow result to local filesystem. The export is handled by 
the browser.
+   */
+  public exportWorkflowResultToLocal(
+    exportType: string,
+    workflowId: number,
+    workflowName: string,
+    operators: {
+      id: string;
+      outputType: string;
+    }[],
+    datasetIds: number[],
+    rowIndex: number,
+    columnIndex: number,
+    filename: string,
+    destination: "local" | "dataset" = "local", // "local" or "dataset" => 
default to "local"
+    unit: DashboardWorkflowComputingUnit        // computing unit for cluster 
setting
+  ): void {
+    if (destination != 'local') {

Review Comment:
   Why do we need this `if` here? in `workflow-result-export.service.ts` we 
pass request to this function only if the dest is `local`



-- 
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]

Reply via email to