This is an automated email from the ASF dual-hosted git repository.
linxinyuan 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 77673b317b fix: private dataset can be set as downloadable (#3666)
77673b317b is described below
commit 77673b317bee0af6606df05ca80452e5d711091c
Author: Seongjin Yoon <[email protected]>
AuthorDate: Mon Aug 18 11:16:06 2025 -0700
fix: private dataset can be set as downloadable (#3666)
---
.../texera/service/resource/DatasetResource.scala | 50 ++++++++++++----------
.../dataset-detail.component.html | 3 +-
.../dataset-detail.component.scss | 3 +-
.../dataset-detail.component.ts | 24 +++++------
.../user-dataset-version-creator.component.html | 1 -
.../user-dataset-version-creator.component.ts | 5 ---
.../service/user/download/download.service.spec.ts | 8 ++--
.../service/user/download/download.service.ts | 4 +-
8 files changed, 47 insertions(+), 51 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 3570f05020..a551b79900 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,8 +39,7 @@ import edu.uci.ics.texera.dao.jooq.generated.tables.daos.{
import edu.uci.ics.texera.dao.jooq.generated.tables.pojos.{
Dataset,
DatasetUserAccess,
- DatasetVersion,
- User
+ DatasetVersion
}
import edu.uci.ics.texera.service.`type`.DatasetFileNode
import edu.uci.ics.texera.service.resource.DatasetAccessResource.{
@@ -196,6 +195,7 @@ 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
@@ -247,11 +247,6 @@ class DatasetResource {
val isDatasetPublic = request.isDatasetPublic
val isDatasetDownloadable = request.isDatasetDownloadable
- // Validate business rule: downloadable can only be true if dataset is
public
- if (isDatasetDownloadable && !isDatasetPublic) {
- throw new BadRequestException("Dataset can only be downloadable if it
is public")
- }
-
// Check if a dataset with the same name already exists
if (!datasetDao.fetchByName(datasetName).isEmpty) {
throw new BadRequestException("Dataset with the same name already
exists")
@@ -573,6 +568,19 @@ 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
+ generatePresignedResponse(encodedUrl, datasetName, commitHash, uid)
+ }
+
@GET
@Path("/public-presign-download")
def getPublicPresignedUrl(
@@ -580,9 +588,17 @@ class DatasetResource {
@QueryParam("datasetName") datasetName: String,
@QueryParam("commitHash") commitHash: String
): Response = {
- val user = new SessionUser(new User())
- val uid = user.getUid
- generatePresignedResponse(encodedUrl, datasetName, commitHash, uid)
+ 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 = {
+ generatePresignedResponse(encodedUrl, datasetName, commitHash, null)
}
@DELETE
@@ -771,11 +787,6 @@ class DatasetResource {
val newPublicStatus = !existedDataset.getIsPublic
existedDataset.setIsPublic(newPublicStatus)
- // If dataset becomes private, it must not be downloadable
- if (!newPublicStatus) {
- existedDataset.setIsDownloadable(false)
- }
-
datasetDao.update(existedDataset)
Response.ok().build()
}
@@ -799,11 +810,6 @@ class DatasetResource {
val existedDataset = getDatasetByID(ctx, did)
val newDownloadableStatus = !existedDataset.getIsDownloadable
- // Validate business rule: can only set downloadable to true if dataset
is public
- if (newDownloadableStatus && !existedDataset.getIsPublic) {
- throw new BadRequestException("Dataset can only be downloadable if it
is public")
- }
-
existedDataset.setIsDownloadable(newDownloadableStatus)
datasetDao.update(existedDataset)
@@ -1046,8 +1052,8 @@ class DatasetResource {
// Retrieve dataset and check download permission
val dataset = getDatasetByID(ctx, did)
- // Non-owners can only download public and downloadable datasets
- if (!userOwnDataset(ctx, did, uid) && (!dataset.getIsPublic ||
!dataset.getIsDownloadable)) {
+ // Non-owners can download if dataset is downloadable and they have read
access
+ if (!userOwnDataset(ctx, did, uid) && !dataset.getIsDownloadable) {
throw new ForbiddenException("Dataset download is not allowed")
}
diff --git
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html
index ad982cf99a..348f5daf42 100644
---
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html
+++
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html
@@ -44,11 +44,10 @@
<div
class="control-row"
*ngIf="isOwner">
- <label>Downloads:</label>
+ <label>Download:</label>
<nz-switch
[ngModel]="datasetIsDownloadable"
(ngModelChange)="onDownloadableStatusChange($event)"
- [nzDisabled]="!datasetIsPublic"
nzCheckedChildren="allowed"
nzUnCheckedChildren="blocked"></nz-switch>
</div>
diff --git
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.scss
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.scss
index 3bddc6de15..7e1ba30bdf 100644
---
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.scss
+++
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.scss
@@ -199,6 +199,7 @@ nz-select {
flex-direction: column;
gap: 12px;
margin-right: 20px;
+ margin-bottom: 20px;
}
.control-row {
@@ -213,7 +214,7 @@ nz-select {
}
nz-switch {
- margin-left: auto;
+ margin-left: 8px;
}
.help-text {
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 f9d6d475b3..2d7d73c017 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
@@ -218,10 +218,6 @@ export class DatasetDetailComponent implements OnInit {
.subscribe({
next: (res: Response) => {
this.datasetIsPublic = checked;
- // If dataset becomes private, it cannot be downloadable
- if (!checked) {
- this.datasetIsDownloadable = false;
- }
let state = "public";
if (!this.datasetIsPublic) {
state = "private";
@@ -236,12 +232,6 @@ export class DatasetDetailComponent implements OnInit {
}
onDownloadableStatusChange(checked: boolean): void {
- // Only allow downloadable change if dataset is public
- if (checked && !this.datasetIsPublic) {
- this.notificationService.error("Dataset can only be downloadable if it
is public");
- return;
- }
-
// Handle the change in dataset downloadable status
if (this.did) {
this.datasetService
@@ -307,8 +297,12 @@ export class DatasetDetailComponent implements OnInit {
onClickDownloadCurrentFile = (): void => {
if (!this.did || !this.selectedVersion?.dvid) return;
-
-
this.downloadService.downloadSingleFile(this.currentDisplayedFileName).pipe(untilDestroyed(this)).subscribe();
+ // For public datasets accessed by non-owners, use public endpoint
+ const shouldUsePublicEndpoint = this.datasetIsPublic && !this.isOwner;
+ this.downloadService
+ .downloadSingleFile(this.currentDisplayedFileName,
!shouldUsePublicEndpoint)
+ .pipe(untilDestroyed(this))
+ .subscribe();
};
onClickScaleTheView() {
@@ -353,8 +347,10 @@ export class DatasetDetailComponent implements OnInit {
if (this.isOwner) {
return true;
}
- // Non-owners can only download if dataset is public and downloadable
- return this.datasetIsPublic && this.datasetIsDownloadable;
+ // Non-owners can download if dataset is downloadable and they have access
+ // For public datasets, users have access even if userDatasetAccessLevel
is 'NONE'
+ // For private datasets, users need explicit access
(userDatasetAccessLevel !== 'NONE')
+ return this.datasetIsDownloadable && (this.datasetIsPublic ||
this.userDatasetAccessLevel !== "NONE");
}
// Track multiple file by unique key
diff --git
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.html
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.html
index cfc530b65a..8320f91772 100644
---
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.html
+++
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.html
@@ -45,7 +45,6 @@
<nz-switch
[ngModel]="isDatasetDownloadable"
(ngModelChange)="onDownloadableStatusChange($event)"
- [nzDisabled]="!isDatasetPublic"
nzCheckedChildren="enabled"
nzUnCheckedChildren="disabled"></nz-switch>
</div>
diff --git
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.ts
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.ts
index f5912d4af1..5e18d198a6 100644
---
a/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.ts
+++
b/core/gui/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.ts
@@ -203,11 +203,6 @@ export class UserDatasetVersionCreatorComponent implements
OnInit {
onPublicStatusChange(newValue: boolean): void {
// Handle the change in dataset public status
this.isDatasetPublic = newValue;
-
- // If dataset becomes private, disable downloads for security
- if (!newValue) {
- this.isDatasetDownloadable = false;
- }
}
onDownloadableStatusChange(newValue: boolean): void {
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 f6d5a4b4b1..3f59997078 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
@@ -66,11 +66,11 @@ describe("DownloadService", () => {
datasetServiceSpy.retrieveDatasetVersionSingleFile.and.returnValue(of(mockBlob));
- downloadService.downloadSingleFile(filePath).subscribe({
+ downloadService.downloadSingleFile(filePath, true).subscribe({
next: blob => {
expect(blob).toBe(mockBlob);
expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download file test/file.txt");
-
expect(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith(filePath);
+
expect(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith(filePath,
true);
expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"file.txt");
expect(notificationServiceSpy.success).toHaveBeenCalledWith("File
test/file.txt has been downloaded");
done();
@@ -87,14 +87,14 @@ describe("DownloadService", () => {
datasetServiceSpy.retrieveDatasetVersionSingleFile.and.returnValue(throwError(()
=> new Error(errorMessage)));
- downloadService.downloadSingleFile(filePath).subscribe({
+ downloadService.downloadSingleFile(filePath, true).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(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith(filePath,
true);
expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error
downloading file 'test/file.txt'");
done();
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 813dea08e2..9c19cf19e2 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,11 +93,11 @@ export class DownloadService {
);
}
- downloadSingleFile(filePath: string): Observable<Blob> {
+ downloadSingleFile(filePath: string, isLogin: boolean = true):
Observable<Blob> {
const DEFAULT_FILE_NAME = "download";
const fileName = filePath.split("/").pop() || DEFAULT_FILE_NAME;
return this.downloadWithNotification(
- () => this.datasetService.retrieveDatasetVersionSingleFile(filePath),
+ () => this.datasetService.retrieveDatasetVersionSingleFile(filePath,
isLogin),
fileName,
`Starting to download file ${filePath}`,
`File ${filePath} has been downloaded`,
