This is an automated email from the ASF dual-hosted git repository.
aicam pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/master by this push:
new eeaaa96d76 chore: revert "feat: switch single-file dataset downloads
to use browser native downloads (#3621)" (#3669)
eeaaa96d76 is described below
commit eeaaa96d76b40171b03b486a1f320dac60b51870
Author: Madison Lin <[email protected]>
AuthorDate: Sun Aug 17 19:44:35 2025 -0700
chore: revert "feat: switch single-file dataset downloads to use browser
native downloads (#3621)" (#3669)
This PR reverts commit f2214a1828370745fa4434a95f4b72cd96cd6c21 due to
issues with the single-file dataset download for Texera when hosted on
Kubernetes.
---
.../texera/service/resource/DatasetResource.scala | 78 ++--------------------
.../ics/texera/service/util/S3StorageClient.scala | 67 +------------------
.../dataset-detail.component.ts | 3 +-
.../service/user/dataset/dataset.service.ts | 23 -------
.../service/user/download/download.service.spec.ts | 42 ++++++++++++
.../service/user/download/download.service.ts | 13 +++-
6 files changed, 60 insertions(+), 166 deletions(-)
diff --git
a/core/file-service/src/main/scala/edu/uci/ics/texera/service/resource/DatasetResource.scala
b/core/file-service/src/main/scala/edu/uci/ics/texera/service/resource/DatasetResource.scala
index d279cfe12c..3570f05020 100644
---
a/core/file-service/src/main/scala/edu/uci/ics/texera/service/resource/DatasetResource.scala
+++
b/core/file-service/src/main/scala/edu/uci/ics/texera/service/resource/DatasetResource.scala
@@ -39,7 +39,8 @@ import edu.uci.ics.texera.dao.jooq.generated.tables.daos.{
import edu.uci.ics.texera.dao.jooq.generated.tables.pojos.{
Dataset,
DatasetUserAccess,
- DatasetVersion
+ DatasetVersion,
+ User
}
import edu.uci.ics.texera.service.`type`.DatasetFileNode
import edu.uci.ics.texera.service.resource.DatasetAccessResource.{
@@ -195,7 +196,6 @@ object DatasetResource {
class DatasetResource {
private val ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE = "User has no access
to this dataset"
private val ERR_DATASET_VERSION_NOT_FOUND_MESSAGE = "The version of the
dataset not found"
- private val EXPIRATION_MINUTES = 5
/**
* Helper function to get the dataset from DB with additional information
including user access privilege and owner email
@@ -573,19 +573,6 @@ class DatasetResource {
generatePresignedResponse(encodedUrl, datasetName, commitHash, uid)
}
- @GET
- @RolesAllowed(Array("REGULAR", "ADMIN"))
- @Path("/presign-download-s3")
- def getPresignedUrlWithS3(
- @QueryParam("filePath") encodedUrl: String,
- @QueryParam("datasetName") datasetName: String,
- @QueryParam("commitHash") commitHash: String,
- @Auth user: SessionUser
- ): Response = {
- val uid = user.getUid
- generatePresignedResponseWithS3(encodedUrl, datasetName, commitHash, uid)
- }
-
@GET
@Path("/public-presign-download")
def getPublicPresignedUrl(
@@ -593,17 +580,9 @@ class DatasetResource {
@QueryParam("datasetName") datasetName: String,
@QueryParam("commitHash") commitHash: String
): Response = {
- generatePresignedResponse(encodedUrl, datasetName, commitHash, null)
- }
-
- @GET
- @Path("/public-presign-download-s3")
- def getPublicPresignedUrlWithS3(
- @QueryParam("filePath") encodedUrl: String,
- @QueryParam("datasetName") datasetName: String,
- @QueryParam("commitHash") commitHash: String
- ): Response = {
- generatePresignedResponseWithS3(encodedUrl, datasetName, commitHash, null)
+ val user = new SessionUser(new User())
+ val uid = user.getUid
+ generatePresignedResponse(encodedUrl, datasetName, commitHash, uid)
}
@DELETE
@@ -1293,53 +1272,6 @@ class DatasetResource {
}
}
- private def generatePresignedResponseWithS3(
- encodedUrl: String,
- datasetName: String,
- commitHash: String,
- uid: Integer
- ): Response = {
- resolveDatasetAndPath(encodedUrl, datasetName, commitHash, uid) match {
- case Left(errorResponse) =>
- errorResponse
-
- case Right((resolvedDatasetName, resolvedCommitHash, resolvedFilePath))
=>
- // Additional download permission check for S3 downloads
- if (uid != null) {
- withTransaction(context) { ctx =>
- val datasetDao = new DatasetDao(ctx.configuration())
- val datasets =
datasetDao.fetchByName(resolvedDatasetName).asScala.toList
- if (datasets.nonEmpty) {
- val dataset = datasets.head
- // Non-owners can only download public and downloadable datasets
- if (
- !userOwnDataset(
- ctx,
- dataset.getDid,
- uid
- ) && (!dataset.getIsPublic || !dataset.getIsDownloadable)
- ) {
- throw new ForbiddenException("Dataset download is not allowed")
- }
- }
- }
- }
-
- val fileName =
resolvedFilePath.split("/").lastOption.getOrElse("download")
- val contentType = "application/octet-stream"
- val url = S3StorageClient.getFilePresignedUrl(
- resolvedDatasetName,
- resolvedCommitHash,
- resolvedFilePath,
- fileName,
- contentType,
- EXPIRATION_MINUTES
- )
-
- Response.ok(Map("presignedUrl" -> url)).build()
- }
- }
-
private def resolveDatasetAndPath(
encodedUrl: String,
datasetName: String,
diff --git
a/core/file-service/src/main/scala/edu/uci/ics/texera/service/util/S3StorageClient.scala
b/core/file-service/src/main/scala/edu/uci/ics/texera/service/util/S3StorageClient.scala
index ae110fec52..2b1afd1165 100644
---
a/core/file-service/src/main/scala/edu/uci/ics/texera/service/util/S3StorageClient.scala
+++
b/core/file-service/src/main/scala/edu/uci/ics/texera/service/util/S3StorageClient.scala
@@ -24,12 +24,7 @@ import
software.amazon.awssdk.auth.credentials.{AwsBasicCredentials, StaticCrede
import software.amazon.awssdk.regions.Region
import software.amazon.awssdk.services.s3.{S3Client, S3Configuration}
import software.amazon.awssdk.services.s3.model._
-import software.amazon.awssdk.services.s3.presigner.S3Presigner
-import
software.amazon.awssdk.services.s3.presigner.model.GetObjectPresignRequest
-import software.amazon.awssdk.services.s3.model.GetObjectRequest
-import java.net.URI
-import java.time.Duration
import java.security.MessageDigest
import scala.jdk.CollectionConverters._
@@ -41,10 +36,10 @@ import scala.jdk.CollectionConverters._
object S3StorageClient {
val MINIMUM_NUM_OF_MULTIPART_S3_PART: Long = 5L * 1024 * 1024 // 5 MiB
val MAXIMUM_NUM_OF_MULTIPART_S3_PARTS = 10_000
- val credentials = AwsBasicCredentials.create(StorageConfig.s3Username,
StorageConfig.s3Password)
// Initialize MinIO-compatible S3 Client
private lazy val s3Client: S3Client = {
+ val credentials = AwsBasicCredentials.create(StorageConfig.s3Username,
StorageConfig.s3Password)
S3Client
.builder()
.credentialsProvider(StaticCredentialsProvider.create(credentials))
@@ -56,29 +51,6 @@ object S3StorageClient {
.build()
}
- // Initialize S3-compatible presigner for LakeFS S3 Gateway
- private lazy val s3Presigner: S3Presigner = {
- val fullUri = new URI(StorageConfig.lakefsEndpoint)
- val baseUri = new URI(
- fullUri.getScheme,
- null,
- fullUri.getHost,
- fullUri.getPort,
- null,
- null,
- null
- ) // Extract just the base (scheme + host + port)
- S3Presigner
- .builder()
- .credentialsProvider(StaticCredentialsProvider.create(credentials))
- .region(Region.of(StorageConfig.s3Region))
- .endpointOverride(baseUri) // LakeFS base URL ("http://localhost:8000";
on local)
- .serviceConfiguration(
- S3Configuration.builder().pathStyleAccessEnabled(true).build()
- )
- .build()
- }
-
/**
* Checks if a directory (prefix) exists within an S3 bucket.
*
@@ -167,41 +139,4 @@ object S3StorageClient {
s3Client.deleteObjects(deleteObjectsRequest)
}
}
-
- /**
- * Retrieves file content from a specific commit and path.
- *
- * @param repoName Repository name.
- * @param commitHash Commit hash of the version.
- * @param filePath Path to the file in the repository.
- * @param fileName Name of the file downloaded via the presigned
URL.
- * @param contentType Type of the file downloaded via the presigned
URL.
- * @param expirationMinutes Duration in minutes that the presigned URL is
valid.
- */
- def getFilePresignedUrl(
- repoName: String,
- commitHash: String,
- filePath: String,
- fileName: String,
- contentType: String,
- expirationMinutes: Long
- ): String = {
- val getObjectRequest = GetObjectRequest
- .builder()
- .bucket(repoName)
- .key(s"$commitHash/$filePath")
- .responseContentDisposition(s"attachment; filename='$fileName'")
- .responseContentType(contentType)
- .build()
-
- val presignRequest = GetObjectPresignRequest
- .builder()
- .signatureDuration(Duration.ofMinutes(expirationMinutes))
- .getObjectRequest(getObjectRequest)
- .build()
-
- val presignedUrl =
s3Presigner.presignGetObject(presignRequest).url().toString
- s3Presigner.close()
- presignedUrl
- }
}
diff --git
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts
index f563142e0c..f9d6d475b3 100644
---
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts
+++
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts
@@ -307,7 +307,8 @@ export class DatasetDetailComponent implements OnInit {
onClickDownloadCurrentFile = (): void => {
if (!this.did || !this.selectedVersion?.dvid) return;
- this.downloadService.downloadSingleFile(this.currentDisplayedFileName);
+
+
this.downloadService.downloadSingleFile(this.currentDisplayedFileName).pipe(untilDestroyed(this)).subscribe();
};
onClickScaleTheView() {
diff --git a/core/gui/src/app/dashboard/service/user/dataset/dataset.service.ts
b/core/gui/src/app/dashboard/service/user/dataset/dataset.service.ts
index e4c9fbbf20..329a6e5947 100644
--- a/core/gui/src/app/dashboard/service/user/dataset/dataset.service.ts
+++ b/core/gui/src/app/dashboard/service/user/dataset/dataset.service.ts
@@ -95,29 +95,6 @@ export class DatasetService {
.pipe(switchMap(({ presignedUrl }) => this.http.get(presignedUrl, {
responseType: "blob" })));
}
- /**
- * Retrieves a single file from a dataset version using a pre-signed URL.
- * @param filePath Relative file path within the dataset.
- * @param isLogin Determine whether a user is currently logged in
- * @returns void File is downloaded natively by the browser.
- */
- public retrieveDatasetVersionSingleFileViaBrowser(filePath: string, isLogin:
boolean = true): void {
- const endpointSegment = isLogin ? "presign-download-s3" :
"public-presign-download-s3";
- const endpoint =
`${AppSettings.getApiEndpoint()}/${DATASET_BASE_URL}/${endpointSegment}?filePath=${encodeURIComponent(filePath)}`;
-
- this.http.get<{ presignedUrl: string }>(endpoint).subscribe({
- next: response => {
- const presignedUrl = response.presignedUrl;
- const downloadUrl = document.createElement("a");
-
- downloadUrl.href = presignedUrl;
- document.body.appendChild(downloadUrl);
- downloadUrl.click();
- downloadUrl.remove();
- },
- });
- }
-
/**
* Retrieves a zip file of a dataset version.
* @param did Dataset ID
diff --git
a/core/gui/src/app/dashboard/service/user/download/download.service.spec.ts
b/core/gui/src/app/dashboard/service/user/download/download.service.spec.ts
index d1c55a434f..f6d5a4b4b1 100644
--- a/core/gui/src/app/dashboard/service/user/download/download.service.spec.ts
+++ b/core/gui/src/app/dashboard/service/user/download/download.service.spec.ts
@@ -60,6 +60,48 @@ describe("DownloadService", () => {
notificationServiceSpy = TestBed.inject(NotificationService) as
jasmine.SpyObj<NotificationService>;
});
+ it("should download a single file successfully", (done: DoneFn) => {
+ const filePath = "test/file.txt";
+ const mockBlob = new Blob(["test content"], { type: "text/plain" });
+
+
datasetServiceSpy.retrieveDatasetVersionSingleFile.and.returnValue(of(mockBlob));
+
+ downloadService.downloadSingleFile(filePath).subscribe({
+ next: blob => {
+ expect(blob).toBe(mockBlob);
+ expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download file test/file.txt");
+
expect(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith(filePath);
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"file.txt");
+ expect(notificationServiceSpy.success).toHaveBeenCalledWith("File
test/file.txt has been downloaded");
+ done();
+ },
+ error: (error: unknown) => {
+ fail("Should not have thrown an error: " + error);
+ },
+ });
+ });
+
+ it("should handle download failure correctly", done => {
+ const filePath = "test/file.txt";
+ const errorMessage = "Download failed";
+
+
datasetServiceSpy.retrieveDatasetVersionSingleFile.and.returnValue(throwError(()
=> new Error(errorMessage)));
+
+ downloadService.downloadSingleFile(filePath).subscribe({
+ next: () => {
+ fail("Should have thrown an error");
+ },
+ error: (error: unknown) => {
+ expect(error).toBeTruthy();
+ expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download file test/file.txt");
+
expect(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith(filePath);
+ expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
+ expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error
downloading file 'test/file.txt'");
+ done();
+ },
+ });
+ });
+
it("should download a dataset successfully", done => {
const datasetId = 1;
const datasetName = "TestDataset";
diff --git
a/core/gui/src/app/dashboard/service/user/download/download.service.ts
b/core/gui/src/app/dashboard/service/user/download/download.service.ts
index 39b89f0f01..813dea08e2 100644
--- a/core/gui/src/app/dashboard/service/user/download/download.service.ts
+++ b/core/gui/src/app/dashboard/service/user/download/download.service.ts
@@ -93,9 +93,16 @@ export class DownloadService {
);
}
- downloadSingleFile(filePath: string): void {
- this.notificationService.info(`Starting to download file ${filePath}`);
- this.datasetService.retrieveDatasetVersionSingleFileViaBrowser(filePath);
+ downloadSingleFile(filePath: string): Observable<Blob> {
+ const DEFAULT_FILE_NAME = "download";
+ const fileName = filePath.split("/").pop() || DEFAULT_FILE_NAME;
+ return this.downloadWithNotification(
+ () => this.datasetService.retrieveDatasetVersionSingleFile(filePath),
+ fileName,
+ `Starting to download file ${filePath}`,
+ `File ${filePath} has been downloaded`,
+ `Error downloading file '${filePath}'`
+ );
}
downloadWorkflowsAsZip(workflowEntries: Array<{ id: number; name: string
}>): Observable<Blob> {
