Re: [PR] feat: add custom cover image support for datasets [texera]
aicam merged PR #4117: URL: https://github.com/apache/texera/pull/4117 -- 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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2680426917
##
file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala:
##
@@ -1328,4 +1328,180 @@ class DatasetResourceSpec
val part1 = fetchPartRows(uploadId).find(_.getPartNumber == 1).get
part1.getEtag.trim should not be ""
}
+
+ //
===
+ // Cover Image Tests
+ //
===
+
+ "updateDatasetCoverImage" should "reject path traversal attempts" in {
+val maliciousPaths = Seq(
+ "../../../etc/passwd",
+ "v1/../../secret.txt",
+ "../escape.jpg"
+)
+
+maliciousPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject absolute paths" in {
+val absolutePaths = Seq(
+ "/etc/passwd",
+ "/var/log/system.log"
+)
+
+absolutePaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject invalid file types" in {
+val invalidPaths = Seq(
+ "v1/script.js",
+ "v1/document.pdf",
+ "v1/data.csv"
+)
+
+invalidPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject empty or null cover image path" in {
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(""),
+sessionUser
+ )
+}
+
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(null),
+sessionUser
+ )
+}
+ }
+
+ it should "reject when user lacks WRITE access" in {
+val request = DatasetResource.CoverImageRequest("v1/cover.jpg")
+
+assertThrows[ForbiddenException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+request,
+sessionUser2
+ )
+}
+ }
+
+ "getDatasetCover" should "reject private dataset cover for anonymous users"
in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setIsPublic(false)
+dataset.setCoverImage("v1/cover.jpg")
+datasetDao.update(dataset)
+
+assertThrows[ForbiddenException] {
+ datasetResource.getDatasetCover(baseDataset.getDid, Optional.empty())
+}
+ }
+
+ it should "reject private dataset cover for users without access" in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setOwnerUid(ownerUser.getUid)
+dataset.setIsPublic(false)
+dataset.setCoverImage("v1/cover.jpg")
+datasetDao.update(dataset)
+
+assertThrows[ForbiddenException] {
+ datasetResource.getDatasetCover(baseDataset.getDid,
Optional.of(sessionUser2))
+}
+ }
+
+ it should "return 404 when no cover image is set" in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setCoverImage(null)
+dataset.setIsPublic(true)
+datasetDao.update(dataset)
+
+assertThrows[NotFoundException] {
+ datasetResource.getDatasetCover(baseDataset.getDid,
Optional.of(sessionUser))
+}
+ }
+
+ "validateSafePath" should "accept valid relative paths" in {
+DatasetResource.validateSafePath("v1/image.jpg") shouldEqual "v1/image.jpg"
+DatasetResource.validateSafePath("v1/folder/photo.png") shouldEqual
"v1/folder/photo.png"
+ }
+
+ it should "normalize safe internal navigation" in {
+DatasetResource.validateSafePath("v1/../v2/img.jpg") shouldEqual
"v2/img.jpg"
+DatasetResource.validateSafePath("./v1/image.jpg") shouldEqual
"v1/image.jpg"
+DatasetResource.validateSafePath("v1/./image.jpg") shouldEqual
"v1/image.jpg"
+ }
+
+ it should "reject path traversal" in {
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("../escape.txt")
+}
+
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("../../etc/passwd")
+}
+
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("v1/../../escape.txt")
+}
+ }
+
+ it should "reject absolute paths" in {
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("/e
Re: [PR] feat: add custom cover image support for datasets [texera]
aicam commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2678885711
##
file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala:
##
@@ -1328,4 +1328,180 @@ class DatasetResourceSpec
val part1 = fetchPartRows(uploadId).find(_.getPartNumber == 1).get
part1.getEtag.trim should not be ""
}
+
+ //
===
+ // Cover Image Tests
+ //
===
+
+ "updateDatasetCoverImage" should "reject path traversal attempts" in {
+val maliciousPaths = Seq(
+ "../../../etc/passwd",
+ "v1/../../secret.txt",
+ "../escape.jpg"
+)
+
+maliciousPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject absolute paths" in {
+val absolutePaths = Seq(
+ "/etc/passwd",
+ "/var/log/system.log"
+)
+
+absolutePaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject invalid file types" in {
+val invalidPaths = Seq(
+ "v1/script.js",
+ "v1/document.pdf",
+ "v1/data.csv"
+)
+
+invalidPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject empty or null cover image path" in {
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(""),
+sessionUser
+ )
+}
+
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(null),
+sessionUser
+ )
+}
+ }
+
+ it should "reject when user lacks WRITE access" in {
+val request = DatasetResource.CoverImageRequest("v1/cover.jpg")
+
+assertThrows[ForbiddenException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+request,
+sessionUser2
+ )
+}
+ }
+
+ "getDatasetCover" should "reject private dataset cover for anonymous users"
in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setIsPublic(false)
+dataset.setCoverImage("v1/cover.jpg")
+datasetDao.update(dataset)
+
+assertThrows[ForbiddenException] {
+ datasetResource.getDatasetCover(baseDataset.getDid, Optional.empty())
+}
+ }
+
+ it should "reject private dataset cover for users without access" in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setOwnerUid(ownerUser.getUid)
+dataset.setIsPublic(false)
+dataset.setCoverImage("v1/cover.jpg")
+datasetDao.update(dataset)
+
+assertThrows[ForbiddenException] {
+ datasetResource.getDatasetCover(baseDataset.getDid,
Optional.of(sessionUser2))
+}
+ }
+
+ it should "return 404 when no cover image is set" in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setCoverImage(null)
+dataset.setIsPublic(true)
+datasetDao.update(dataset)
+
+assertThrows[NotFoundException] {
+ datasetResource.getDatasetCover(baseDataset.getDid,
Optional.of(sessionUser))
+}
+ }
+
+ "validateSafePath" should "accept valid relative paths" in {
+DatasetResource.validateSafePath("v1/image.jpg") shouldEqual "v1/image.jpg"
+DatasetResource.validateSafePath("v1/folder/photo.png") shouldEqual
"v1/folder/photo.png"
+ }
+
+ it should "normalize safe internal navigation" in {
+DatasetResource.validateSafePath("v1/../v2/img.jpg") shouldEqual
"v2/img.jpg"
+DatasetResource.validateSafePath("./v1/image.jpg") shouldEqual
"v1/image.jpg"
+DatasetResource.validateSafePath("v1/./image.jpg") shouldEqual
"v1/image.jpg"
+ }
+
+ it should "reject path traversal" in {
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("../escape.txt")
+}
+
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("../../etc/passwd")
+}
+
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("v1/../../escape.txt")
+}
+ }
+
+ it should "reject absolute paths" in {
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("/et
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2669719765
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -144,6 +145,25 @@ object DatasetResource {
.toScala
}
+ /**
+* Validates a file path using Apache Commons IO.
+*/
+ def validateSafePath(path: String): String = {
Review Comment:
Renamed, thanks!
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2669690982
##
file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala:
##
@@ -1328,4 +1328,180 @@ class DatasetResourceSpec
val part1 = fetchPartRows(uploadId).find(_.getPartNumber == 1).get
part1.getEtag.trim should not be ""
}
+
+ //
===
+ // Cover Image Tests
+ //
===
+
+ "updateDatasetCoverImage" should "reject path traversal attempts" in {
+val maliciousPaths = Seq(
+ "../../../etc/passwd",
+ "v1/../../secret.txt",
+ "../escape.jpg"
+)
+
+maliciousPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject absolute paths" in {
+val absolutePaths = Seq(
+ "/etc/passwd",
+ "/var/log/system.log"
+)
+
+absolutePaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject invalid file types" in {
+val invalidPaths = Seq(
+ "v1/script.js",
+ "v1/document.pdf",
+ "v1/data.csv"
+)
+
+invalidPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject empty or null cover image path" in {
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(""),
+sessionUser
+ )
+}
+
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(null),
+sessionUser
+ )
+}
+ }
+
+ it should "reject when user lacks WRITE access" in {
+val request = DatasetResource.CoverImageRequest("v1/cover.jpg")
+
+assertThrows[ForbiddenException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+request,
+sessionUser2
+ )
+}
+ }
+
+ "getDatasetCover" should "reject private dataset cover for anonymous users"
in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setIsPublic(false)
+dataset.setCoverImage("v1/cover.jpg")
+datasetDao.update(dataset)
+
+assertThrows[ForbiddenException] {
+ datasetResource.getDatasetCover(baseDataset.getDid, Optional.empty())
+}
+ }
+
+ it should "reject private dataset cover for users without access" in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setOwnerUid(ownerUser.getUid)
+dataset.setIsPublic(false)
+dataset.setCoverImage("v1/cover.jpg")
+datasetDao.update(dataset)
+
+assertThrows[ForbiddenException] {
+ datasetResource.getDatasetCover(baseDataset.getDid,
Optional.of(sessionUser2))
+}
+ }
+
+ it should "return 404 when no cover image is set" in {
+val dataset = datasetDao.fetchOneByDid(baseDataset.getDid)
+dataset.setCoverImage(null)
+dataset.setIsPublic(true)
+datasetDao.update(dataset)
+
+assertThrows[NotFoundException] {
+ datasetResource.getDatasetCover(baseDataset.getDid,
Optional.of(sessionUser))
+}
+ }
+
+ "validateSafePath" should "accept valid relative paths" in {
+DatasetResource.validateSafePath("v1/image.jpg") shouldEqual "v1/image.jpg"
+DatasetResource.validateSafePath("v1/folder/photo.png") shouldEqual
"v1/folder/photo.png"
+ }
+
+ it should "normalize safe internal navigation" in {
+DatasetResource.validateSafePath("v1/../v2/img.jpg") shouldEqual
"v2/img.jpg"
+DatasetResource.validateSafePath("./v1/image.jpg") shouldEqual
"v1/image.jpg"
+DatasetResource.validateSafePath("v1/./image.jpg") shouldEqual
"v1/image.jpg"
+ }
+
+ it should "reject path traversal" in {
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("../escape.txt")
+}
+
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("../../etc/passwd")
+}
+
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("v1/../../escape.txt")
+}
+ }
+
+ it should "reject absolute paths" in {
+assertThrows[BadRequestException] {
+ DatasetResource.validateSafePath("/e
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2669666392
##
file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala:
##
@@ -1328,4 +1328,180 @@ class DatasetResourceSpec
val part1 = fetchPartRows(uploadId).find(_.getPartNumber == 1).get
part1.getEtag.trim should not be ""
}
+
+ //
===
+ // Cover Image Tests
+ //
===
+
+ "updateDatasetCoverImage" should "reject path traversal attempts" in {
+val maliciousPaths = Seq(
+ "../../../etc/passwd",
+ "v1/../../secret.txt",
+ "../escape.jpg"
+)
+
+maliciousPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject absolute paths" in {
+val absolutePaths = Seq(
+ "/etc/passwd",
+ "/var/log/system.log"
+)
+
+absolutePaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject invalid file types" in {
+val invalidPaths = Seq(
+ "v1/script.js",
+ "v1/document.pdf",
+ "v1/data.csv"
+)
+
+invalidPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject empty or null cover image path" in {
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(""),
+sessionUser
+ )
+}
+
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(null),
+sessionUser
+ )
+}
+ }
+
+ it should "reject when user lacks WRITE access" in {
+val request = DatasetResource.CoverImageRequest("v1/cover.jpg")
+
+assertThrows[ForbiddenException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+request,
+sessionUser2
+ )
+}
+ }
+
+ "getDatasetCover" should "reject private dataset cover for anonymous users"
in {
Review Comment:
Yes, this is a test case for a private dataset cover image being accessed by
an anonymous user.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
aicam commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2669208296
##
file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala:
##
@@ -1328,4 +1328,180 @@ class DatasetResourceSpec
val part1 = fetchPartRows(uploadId).find(_.getPartNumber == 1).get
part1.getEtag.trim should not be ""
}
+
+ //
===
+ // Cover Image Tests
+ //
===
+
+ "updateDatasetCoverImage" should "reject path traversal attempts" in {
+val maliciousPaths = Seq(
+ "../../../etc/passwd",
+ "v1/../../secret.txt",
+ "../escape.jpg"
+)
+
+maliciousPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject absolute paths" in {
+val absolutePaths = Seq(
+ "/etc/passwd",
+ "/var/log/system.log"
+)
+
+absolutePaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject invalid file types" in {
+val invalidPaths = Seq(
+ "v1/script.js",
+ "v1/document.pdf",
+ "v1/data.csv"
+)
+
+invalidPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject empty or null cover image path" in {
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(""),
+sessionUser
+ )
+}
+
+assertThrows[BadRequestException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+DatasetResource.CoverImageRequest(null),
+sessionUser
+ )
+}
+ }
+
+ it should "reject when user lacks WRITE access" in {
+val request = DatasetResource.CoverImageRequest("v1/cover.jpg")
+
+assertThrows[ForbiddenException] {
+ datasetResource.updateDatasetCoverImage(
+baseDataset.getDid,
+request,
+sessionUser2
+ )
+}
+ }
+
+ "getDatasetCover" should "reject private dataset cover for anonymous users"
in {
Review Comment:
for anonymous and non-public datasets?
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -144,6 +145,25 @@ object DatasetResource {
.toScala
}
+ /**
+* Validates a file path using Apache Commons IO.
+*/
+ def validateSafePath(path: String): String = {
Review Comment:
I think you can use `validateAndNormalizeFilePathOrThrow`
##
file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala:
##
@@ -1328,4 +1328,180 @@ class DatasetResourceSpec
val part1 = fetchPartRows(uploadId).find(_.getPartNumber == 1).get
part1.getEtag.trim should not be ""
}
+
+ //
===
+ // Cover Image Tests
+ //
===
+
+ "updateDatasetCoverImage" should "reject path traversal attempts" in {
+val maliciousPaths = Seq(
+ "../../../etc/passwd",
+ "v1/../../secret.txt",
+ "../escape.jpg"
+)
+
+maliciousPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject absolute paths" in {
+val absolutePaths = Seq(
+ "/etc/passwd",
+ "/var/log/system.log"
+)
+
+absolutePaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetResource.updateDatasetCoverImage(
+ baseDataset.getDid,
+ request,
+ sessionUser
+)
+ }
+}
+ }
+
+ it should "reject invalid file types" in {
+val invalidPaths = Seq(
+ "v1/script.js",
+ "v1/document.pdf",
+ "v1/data.csv"
+)
+
+invalidPaths.foreach { path =>
+ val request = DatasetResource.CoverImageRequest(path)
+
+ assertThrows[BadRequestException] {
+datasetRes
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2666494633
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ val document = DocumentFactory
+.openReadonlyDocument(
+ FileResolver.resolve(s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
+ val coverSizeLimit = 10 * 1024 * 1024 // 10 MB
+
+ if (file.length() > coverSizeLimit) {
+throw new BadRequestException(
+ s"Cover image must be less than ${coverSizeLimit / (1024 * 1024)} MB"
+)
+ }
+
+ dataset.setCoverImage(coverImage)
+ new DatasetDao(ctx.configuration()).update(dataset)
+ Response.ok().build()
+}
+ }
Review Comment:
fixed, including added test cases including: path traversal rejection,
absolute path rejection, invalid file type rejection, empty/null path
rejection, unauthorized user rejection, private dataset access checks, and
validateSafePath coverage.
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ val document = DocumentFactory
+.openReadonlyDocument(
+ FileResolver.resolve(s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
+ val coverSizeLimit = 10 * 1024 * 1024 // 10 MB
+
+ if (file.length() > coverSizeLimit) {
+throw new BadRequestException(
+ s"Cover image must be less than ${coverSizeLimit / (1024 * 1024)} MB"
+)
+ }
+
+ dataset.setCoverImage(coverImage)
+ new DatasetDao(ctx.configuration()).update(dataset)
+ Response.ok().build()
+}
+ }
Review Comment:
Fixed, including added test cases including: path traversal rejection,
absolute path rejection, invalid file type rejection, empty/null path
rejection, unauthorized user rejection, private dataset access checks, and
validateSafePath coverage.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2666488848
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1742,4 +1747,106 @@ class DatasetResource {
Response.ok(Map("message" -> "Multipart upload aborted
successfully")).build()
}
}
+
+ /**
+* Updates the cover image for a dataset.
+*
+* @param did Dataset ID
+* @param request Cover image request containing the relative file path
+* @param sessionUser Authenticated user session
+* @return Response with updated cover image path
+*
+* Expected coverImage format: "version/folder/image.jpg" (relative to
dataset root)
+*/
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ @Consumes(Array(MediaType.APPLICATION_JSON))
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ request: CoverImageRequest,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ if (request == null || request.coverImage == null ||
request.coverImage.trim.isEmpty) {
+throw new BadRequestException("Cover image path is required")
+ }
+
+ val normalized = Paths.get(request.coverImage).normalize().toString
+ if (normalized.startsWith("..") || normalized.startsWith("/")) {
Review Comment:
Fixed. Used FilenameUtils to validate the path.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2666487526
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1742,4 +1747,106 @@ class DatasetResource {
Response.ok(Map("message" -> "Multipart upload aborted
successfully")).build()
}
}
+
+ /**
+* Updates the cover image for a dataset.
+*
+* @param did Dataset ID
+* @param request Cover image request containing the relative file path
+* @param sessionUser Authenticated user session
+* @return Response with updated cover image path
+*
+* Expected coverImage format: "version/folder/image.jpg" (relative to
dataset root)
+*/
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ @Consumes(Array(MediaType.APPLICATION_JSON))
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ request: CoverImageRequest,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ if (request == null || request.coverImage == null ||
request.coverImage.trim.isEmpty) {
+throw new BadRequestException("Cover image path is required")
+ }
+
+ val normalized = Paths.get(request.coverImage).normalize().toString
+ if (normalized.startsWith("..") || normalized.startsWith("/")) {
+throw new BadRequestException("Invalid file path")
+ }
+
+ if (!ALLOWED_IMAGE_EXTENSIONS.exists(ext =>
normalized.toLowerCase.endsWith(ext))) {
+throw new BadRequestException("Invalid file type")
+ }
+
+ val owner = getOwner(ctx, did)
+ val document = DocumentFactory
+.openReadonlyDocument(
+
FileResolver.resolve(s"${owner.getEmail}/${dataset.getName}/$normalized")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
Review Comment:
Fixed.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2666486985
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1742,4 +1747,106 @@ class DatasetResource {
Response.ok(Map("message" -> "Multipart upload aborted
successfully")).build()
}
}
+
+ /**
+* Updates the cover image for a dataset.
+*
+* @param did Dataset ID
+* @param request Cover image request containing the relative file path
+* @param sessionUser Authenticated user session
+* @return Response with updated cover image path
+*
+* Expected coverImage format: "version/folder/image.jpg" (relative to
dataset root)
+*/
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ @Consumes(Array(MediaType.APPLICATION_JSON))
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ request: CoverImageRequest,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ if (request == null || request.coverImage == null ||
request.coverImage.trim.isEmpty) {
+throw new BadRequestException("Cover image path is required")
+ }
+
+ val normalized = Paths.get(request.coverImage).normalize().toString
+ if (normalized.startsWith("..") || normalized.startsWith("/")) {
+throw new BadRequestException("Invalid file path")
+ }
+
+ if (!ALLOWED_IMAGE_EXTENSIONS.exists(ext =>
normalized.toLowerCase.endsWith(ext))) {
+throw new BadRequestException("Invalid file type")
+ }
+
+ val owner = getOwner(ctx, did)
+ val document = DocumentFactory
+.openReadonlyDocument(
+
FileResolver.resolve(s"${owner.getEmail}/${dataset.getName}/$normalized")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
+
+ if (file.length() > COVER_IMAGE_SIZE_LIMIT_BYTES) {
+throw new BadRequestException(
+ s"Cover image must be less than ${COVER_IMAGE_SIZE_LIMIT_BYTES /
(1024 * 1024)} MB"
+)
+ }
+
+ dataset.setCoverImage(normalized)
+ new DatasetDao(ctx.configuration()).update(dataset)
+ Response.ok(Map("coverImage" -> normalized)).build()
+}
+ }
+
+ /**
+* Get the cover image for a public dataset.
+* Returns a 307 redirect to the presigned S3 URL.
+*
+* @param did Dataset ID
+* @return 307 Temporary Redirect to cover image
+*/
+ @GET
+ @Path("/{did}/cover")
+ def getDatasetCover(@PathParam("did") did: Integer): Response = {
+withTransaction(context) { ctx =>
+ val dataset = getDatasetByID(ctx, did)
+
+ if (!dataset.getIsPublic) {
Review Comment:
Fixed. Updated the authorization to: public accessible; private requires
login + READ.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
aicam commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2665723107
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ val document = DocumentFactory
+.openReadonlyDocument(
+ FileResolver.resolve(s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
+ val coverSizeLimit = 10 * 1024 * 1024 // 10 MB
+
+ if (file.length() > coverSizeLimit) {
+throw new BadRequestException(
+ s"Cover image must be less than ${coverSizeLimit / (1024 * 1024)} MB"
+)
+ }
+
+ dataset.setCoverImage(coverImage)
+ new DatasetDao(ctx.configuration()).update(dataset)
+ Response.ok().build()
+}
+ }
Review Comment:
@xuang7 add testing please
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1742,4 +1747,106 @@ class DatasetResource {
Response.ok(Map("message" -> "Multipart upload aborted
successfully")).build()
}
}
+
+ /**
+* Updates the cover image for a dataset.
+*
+* @param did Dataset ID
+* @param request Cover image request containing the relative file path
+* @param sessionUser Authenticated user session
+* @return Response with updated cover image path
+*
+* Expected coverImage format: "version/folder/image.jpg" (relative to
dataset root)
+*/
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ @Consumes(Array(MediaType.APPLICATION_JSON))
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ request: CoverImageRequest,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ if (request == null || request.coverImage == null ||
request.coverImage.trim.isEmpty) {
+throw new BadRequestException("Cover image path is required")
+ }
+
+ val normalized = Paths.get(request.coverImage).normalize().toString
+ if (normalized.startsWith("..") || normalized.startsWith("/")) {
Review Comment:
I think there are some builtin or helper functions in code base or libraries
to check validity of a path, this if statement is very simple and specific
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1742,4 +1747,106 @@ class DatasetResource {
Response.ok(Map("message" -> "Multipart upload aborted
successfully")).build()
}
}
+
+ /**
+* Updates the cover image for a dataset.
+*
+* @param did Dataset ID
+* @param request Cover image request containing the relative file path
+* @param sessionUser Authenticated user session
+* @return Response with updated cover image path
+*
+* Expected coverImage format: "version/folder/image.jpg" (relative to
dataset root)
+*/
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ @Consumes(Array(MediaType.APPLICATION_JSON))
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ request: CoverImageRequest,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ if (request == null || request.coverImage == null ||
request.coverImage.trim.isEmpty) {
+throw new BadRequestException("Cover image path is required")
+ }
+
+ val normalized = Paths.get(request.coverImage).normalize().toString
+ if (normalized.startsWith("..") || normalized.startsWith("/")) {
+throw new BadRequestException("Invalid file path")
+ }
+
+ if (!ALLOWED_IMAGE_EXTENSIONS.exists(ext =>
normalized.toLowerCase.endsW
Re: [PR] feat: add custom cover image support for datasets [texera]
chenlica commented on PR #4117: URL: https://github.com/apache/texera/pull/4117#issuecomment-3713088313 @aicam Please take the lead to finish this PR. -- 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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117: URL: https://github.com/apache/texera/pull/4117#discussion_r2663166348 ## sql/updates/16.sql: ## @@ -0,0 +1,34 @@ +-- Licensed to the Apache Software Foundation (ASF) under one Review Comment: Fixed, thanks! -- 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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2663147922
##
frontend/src/app/hub/component/browse-section/browse-section.component.ts:
##
@@ -89,4 +89,12 @@ export class BrowseSectionComponent implements OnInit,
OnChanges {
throw new Error("Unexpected type in DashboardEntry.");
}
}
+
+ getCoverImage(entity: DashboardEntry): string {
+if (entity.type === "dataset" && entity.coverImageUrl) {
+ const fullPath =
`${entity.ownerEmail}/${entity.name}/${entity.coverImageUrl}`;
Review Comment:
Validation is implemented on the backend.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2663142860
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ val document = DocumentFactory
+.openReadonlyDocument(
+ FileResolver.resolve(s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
Review Comment:
fixed.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2663121084
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
Review Comment:
fixed.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2663120390
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ val document = DocumentFactory
+.openReadonlyDocument(
+ FileResolver.resolve(s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
+ val coverSizeLimit = 10 * 1024 * 1024 // 10 MB
+
+ if (file.length() > coverSizeLimit) {
+throw new BadRequestException(
+ s"Cover image must be less than ${coverSizeLimit / (1024 * 1024)} MB"
+)
+ }
Review Comment:
Fixed, validation of file extension/file size/file path added.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2663119446
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ val document = DocumentFactory
+.openReadonlyDocument(
+ FileResolver.resolve(s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
+ val coverSizeLimit = 10 * 1024 * 1024 // 10 MB
Review Comment:
fixed, a constant added.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2663118651
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
Review Comment:
fixed.
##
frontend/src/app/dashboard/service/user/dataset/dataset.service.ts:
##
@@ -539,4 +539,8 @@ export class DatasetService {
public retrieveOwners(): Observable {
return
this.http.get(`${AppSettings.getApiEndpoint()}/${DATASET_GET_OWNERS_URL}`);
}
+
+ public updateDatasetCoverImage(did: number, coverImagePath: string):
Observable {
+return
this.http.post(`${AppSettings.getApiEndpoint()}/dataset/${did}/update/cover`,
coverImagePath);
Review Comment:
fixed.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
aglinxinyuan commented on PR #4117: URL: https://github.com/apache/texera/pull/4117#issuecomment-3707605951 Please address the comments. -- 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]
Re: [PR] feat: add custom cover image support for datasets [texera]
Copilot commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2659288382
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ val document = DocumentFactory
+.openReadonlyDocument(
+ FileResolver.resolve(s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
+ val coverSizeLimit = 10 * 1024 * 1024 // 10 MB
+
+ if (file.length() > coverSizeLimit) {
+throw new BadRequestException(
+ s"Cover image must be less than ${coverSizeLimit / (1024 * 1024)} MB"
+)
+ }
+
+ dataset.setCoverImage(coverImage)
+ new DatasetDao(ctx.configuration()).update(dataset)
+ Response.ok().build()
+}
+ }
Review Comment:
The new `updateDatasetCoverImage` endpoint lacks test coverage. Since this
repository has test coverage for other dataset endpoints in
DatasetResourceSpec, tests should be added to verify the behavior of this new
endpoint, including: successful cover image update, validation of file size
limits, permission checks for write access, and error handling for invalid file
paths.
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
+withTransaction(context) { ctx =>
+ val uid = sessionUser.getUid
+ val dataset = getDatasetByID(ctx, did)
+ if (!userHasWriteAccess(ctx, did, uid)) {
+throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
+ }
+
+ val document = DocumentFactory
+.openReadonlyDocument(
+ FileResolver.resolve(s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage")
+)
+.asInstanceOf[OnDataset]
+
+ val file = LakeFSStorageClient.getFileFromRepo(
+document.getRepositoryName(),
+document.getVersionHash(),
+document.getFileRelativePath()
+ )
Review Comment:
The code does not handle potential exceptions that could be thrown by
`DocumentFactory.openReadonlyDocument()` or
`LakeFSStorageClient.getFileFromRepo()`. If the file path is invalid or the
file doesn't exist, these calls could throw exceptions that aren't properly
caught. Consider adding appropriate error handling to provide more informative
error messages when the specified cover image path is invalid or the file
cannot be accessed.
```suggestion
val (document, file) =
try {
val doc = DocumentFactory
.openReadonlyDocument(
FileResolver.resolve(
s"${getOwner(ctx,
did).getEmail}/${dataset.getName}/$coverImage"
)
)
.asInstanceOf[OnDataset]
val f = LakeFSStorageClient.getFileFromRepo(
doc.getRepositoryName(),
doc.getVersionHash(),
doc.getFileRelativePath()
)
(doc, f)
} catch {
case e: Exception =>
throw new BadRequestException(
s"Unable to access cover image '$coverImage'. Please verify
that the path is valid and the file exists.",
e
)
}
```
##
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##
@@ -1326,4 +1326,44 @@ class DatasetResource {
Right(response)
}
}
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{did}/update/cover")
+ def updateDatasetCoverImage(
+ @PathParam("did") did: Integer,
+ coverImage: String,
+ @Auth sessionUser: SessionUser
+ ): Response = {
Review Comment:
The endpoint lacks documentation explaining its purpose, parameters,
expected behavior, and error conditions. Consider adding scaladoc comments to
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on PR #4117: URL: https://github.com/apache/texera/pull/4117#issuecomment-3707567912 This PR is using a recently removed API (dataset/file). I’ll update the code soon. -- 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]
Re: [PR] feat: add custom cover image support for datasets [texera]
chenlica commented on PR #4117: URL: https://github.com/apache/texera/pull/4117#issuecomment-3707562648 @aglinxinyuan Please review this PR. -- 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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2659248365
##
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.ts:
##
@@ -74,4 +80,14 @@ export class UserDatasetVersionFiletreeComponent implements
AfterViewInit {
this.tree.treeModel.expandAll();
}
}
+
+ isImageFile(fileName: string): boolean {
+const imageExts = [".jpg", ".jpeg", ".png", ".gif", ".webp"];
Review Comment:
fixed, thanks!
##
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.ts:
##
@@ -74,4 +80,14 @@ export class UserDatasetVersionFiletreeComponent implements
AfterViewInit {
this.tree.treeModel.expandAll();
}
}
+
+ isImageFile(fileName: string): boolean {
+const imageExts = [".jpg", ".jpeg", ".png", ".gif", ".webp"];
+return imageExts.some(ext => fileName.toLowerCase().endsWith(ext));
+ }
+
+ onSetCover(nodeData: DatasetFileNode): void {
+const path = getRelativePathFromDatasetFileNode(nodeData);
Review Comment:
fixed.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
chenlica commented on PR #4117: URL: https://github.com/apache/texera/pull/4117#issuecomment-3707252505 @xuang7 Please resolve those comments after they are taken care of. -- 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]
Re: [PR] feat: add custom cover image support for datasets [texera]
xuang7 commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2658776478
##
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.scss:
##
@@ -22,7 +22,7 @@
}
/* Styles for the delete button */
-.delete-button {
+.icon-button {
Review Comment:
The cover icon uses the same styling, so I renamed the delete icon class for
clarity. Functionality and styling remain the same.
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
aicam commented on code in PR #4117:
URL: https://github.com/apache/texera/pull/4117#discussion_r2658243286
##
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.scss:
##
@@ -22,7 +22,7 @@
}
/* Styles for the delete button */
-.delete-button {
+.icon-button {
Review Comment:
Why we need to change this?
##
sql/updates/16.sql:
##
@@ -0,0 +1,34 @@
+-- Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
New PRs have added this number, please update it based on the latest number
not taken before merge
##
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.ts:
##
@@ -74,4 +80,14 @@ export class UserDatasetVersionFiletreeComponent implements
AfterViewInit {
this.tree.treeModel.expandAll();
}
}
+
+ isImageFile(fileName: string): boolean {
+const imageExts = [".jpg", ".jpeg", ".png", ".gif", ".webp"];
Review Comment:
Add constant on top of file
##
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.ts:
##
@@ -74,4 +80,14 @@ export class UserDatasetVersionFiletreeComponent implements
AfterViewInit {
this.tree.treeModel.expandAll();
}
}
+
+ isImageFile(fileName: string): boolean {
+const imageExts = [".jpg", ".jpeg", ".png", ".gif", ".webp"];
+return imageExts.some(ext => fileName.toLowerCase().endsWith(ext));
+ }
+
+ onSetCover(nodeData: DatasetFileNode): void {
+const path = getRelativePathFromDatasetFileNode(nodeData);
Review Comment:
This can be written in one line
--
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]
Re: [PR] feat: add custom cover image support for datasets [texera]
chenlica commented on PR #4117: URL: https://github.com/apache/texera/pull/4117#issuecomment-3622836274 @aicam please review it first before @aglinxinyuan does it. -- 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]
