(texera) 01/01: test(file-service): cover DatasetResource's public presigned and file-node reads (#7881)

[gi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7881-1d1eee1ef431568824ccc2ba494dd5c24bfdf8ad
in repository https://gitbox.apache.org/repos/asf/texera.git

commit a557a1bcb33ebaca99321e98116f4a1ec5b6770a
Author: Meng Wang <me...@uci.edu>
AuthorDate: Sun Aug 23 07:29:29 2026 +0000

    test(file-service): cover DatasetResource's public presigned and file-node reads (#7881)
    
    ### What changes were proposed in this PR?
    
    Extends `DatasetResourceSpec` to cover `DatasetResource`'s public
    (anonymous)
    read path, which no test called before: the presigned-download
    endpoints, the
    anonymous dataset read, and the file-node tree behind the version reads.
    
    Measured with JaCoCo, this takes `DatasetResource.scala` from **73.10 %
    to
    83.10 %** line coverage (78 → 49 uncovered lines).
    
    8 added tests:
    
    - `getPublicPresignedUrl` — returns a presigned address for a public
    dataset's file, and returns **400** when only one of `repositoryName` /
    `commitHash` is supplied (the shared `resolveDatasetAndPath` error arm).
    - `getPublicPresignedUrlWithS3` — the same for the S3 variant, plus a
    percent-encoded path (spaces and an escaped separator) that only
    resolves because the endpoint decodes the query parameter first.
    - `getPublicDataset` — returns the dashboard dataset for a public
    dataset, and raises `ForbiddenException` for a private one.
    - `retrievePublicDatasetVersionRootFileNodes` — returns the version's
    tree to an anonymous caller, asserting it is **assembled rather than
    flattened** (a directory keeps its own children, two levels deep) across
    files seeded under more than one directory.
    - `retrieveDatasetVersionRootFileNodes` — the authenticated sibling,
    which shared the same uncovered helper.
    
    Every region the issue named is now fully covered.
    
    ### Notes from writing these
    
    - **The presigned URL carries no file name.** It is MinIO's *physical*
    object address built from LakeFS's internal object ids
    (`…/<bucket>/<repositoryName>/data/<objectId>?X-Amz-…`). The tests
    therefore assert that the address belongs to the dataset under test (its
    unique `repositoryName`) and is actually signed (`X-Amz-Signature`) — no
    port and no absolute host, per the issue's determinism note.
    - **The file-node reads list committed objects,** so the helper commits
    and pins the returned hash rather than reading a branch head.
    - **`getDatasetVersionZip` was already covered** — the baseline report
    shows no uncovered lines there — so it is not duplicated here.
    - The `(None, None)` arm of `resolveDatasetAndPath` (lines
    1644/1646/1667) is left uncovered: reaching it means driving
    `FileResolver.resolve` + `DocumentFactory.openReadonlyDocument` with a
    resolvable `texera://` URI, which is a different (and much heavier)
    fixture than this issue's scope.
    
    **Determinism.** Each test seeds its own dataset, repository and
    version, so
    nothing depends on another test's uploads or on the shared fixture's
    publicity,
    which existing tests mutate in place. Assertions are on structure — node
    names,
    nesting, status codes — never on byte sizes, timestamps, ports or
    absolute URLs.
    
    No production code was changed.
    
    ### Any related issues, documentation, discussions?
    
    Closes #7878
    
    ### How was this PR tested?
    
    Extended unit tests, run locally against the spec's existing embedded
    Postgres +
    LakeFS/MinIO mocks:
    
    ```
    sbt "FileService/testOnly *DatasetResourceSpec"
    # Tests: succeeded 138, failed 0
    sbt "FileService/test"      # whole module: 306 succeeded, 0 failed (twice)
    sbt "FileService/Test/scalafmtCheck" "FileService/Test/scalafix --check"   # clean
    sbt "FileService/jacoco"    # DatasetResource.scala: 73.10% -> 83.10%
    ```
    
    The failure path was verified by breaking the file-tree assertion and
    confirming
    the suite exits non-zero, then restoring it.
    
    ### Was this PR authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Code (Opus 4.8 [1M context])
---
 .../service/resource/DatasetResourceSpec.scala     | 169 +++++++++++++++++++++
 1 file changed, 169 insertions(+)

diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala
index 8d89ffe578..baac6770ae 100644
--- a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala
+++ b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala
@@ -3484,4 +3484,173 @@ class DatasetResourceSpec
     val commit = LakeFSStorageClient.createCommit(repoName, "main", "commit all files")
     LakeFSStorageClient.retrieveObjectsOfVersion(repoName, commit.getId).size shouldEqual totalFiles
   }
+
+  // ===========================================================================
+  // Public (anonymous) read path: presigned download, dataset read, file nodes
+  // ===========================================================================
+
+  /** Seeds a dataset owned by `ownerUser` plus a committed version holding `files`.
+    * Each call gets its own repository and rows, so these tests never depend on
+    * another test's uploads nor on a shared fixture's publicity being left in a
+    * particular state.
+    */
+  private def seedDatasetWithVersion(
+      prefix: String,
+      files: Seq[(String, String)],
+      isPublic: Boolean = true
+  ): (Dataset, DatasetVersion) = {
+    val repositoryName =
+      s"$prefix-${System.nanoTime()}-${Random.alphanumeric.take(6).mkString.toLowerCase}"
+
+    val dataset = new Dataset
+    dataset.setName(repositoryName)
+    dataset.setRepositoryName(repositoryName)
+    dataset.setIsPublic(isPublic)
+    dataset.setIsDownloadable(true)
+    dataset.setDescription("dataset for public-read tests")
+    dataset.setOwnerUid(ownerUser.getUid)
+    datasetDao.insert(dataset)
+
+    LakeFSStorageClient.initRepo(repositoryName)
+    files.foreach {
+      case (path, content) =>
+        LakeFSStorageClient.writeFileToRepo(
+          repositoryName,
+          path,
+          new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8))
+        )
+    }
+    // The file-node reads list COMMITTED objects, so commit before pinning the hash.
+    val commit = LakeFSStorageClient.createCommit(repositoryName, "main", s"seed $prefix")
+
+    val version = new DatasetVersion
+    version.setDid(dataset.getDid)
+    version.setCreatorUid(ownerUser.getUid)
+    version.setName("v1")
+    version.setVersionHash(commit.getId)
+    new DatasetVersionDao(getDSLContext.configuration()).insert(version)
+
+    (dataset, version)
+  }
+
+  "getPublicPresignedUrl" should "hand an anonymous caller a presigned URL for a public dataset's file" in {
+    val (dataset, version) =
+      seedDatasetWithVersion("public-presign", Seq("data/report.csv" -> "a,b"))
+
+    val response = datasetResource.getPublicPresignedUrl(
+      urlEnc("data/report.csv"),
+      dataset.getRepositoryName,
+      version.getVersionHash
+    )
+
+    response.getStatus shouldEqual 200
+    // The address is a physical object URL, so assert it belongs to this dataset and
+    // is actually signed — never on the mock's port or on the logical file name.
+    val presigned = entityAsScalaMap(response)("presignedUrl").toString
+    presigned should include(dataset.getRepositoryName)
+    presigned should include("X-Amz-Signature")
+  }
+
+  it should "reject a request that supplies a repository name without a commit hash" in {
+    val (dataset, _) = seedDatasetWithVersion("public-presign-half", Seq("f.txt" -> "x"))
+
+    val response =
+      datasetResource.getPublicPresignedUrl(urlEnc("f.txt"), dataset.getRepositoryName, null)
+
+    response.getStatus shouldEqual 400
+  }
+
+  "getPublicPresignedUrlWithS3" should "hand an anonymous caller a presigned URL for a public dataset's file" in {
+    val (dataset, version) =
+      seedDatasetWithVersion("public-presign-s3", Seq("model.bin" -> "weights"))
+
+    val response = datasetResource.getPublicPresignedUrlWithS3(
+      urlEnc("model.bin"),
+      dataset.getRepositoryName,
+      version.getVersionHash
+    )
+
+    response.getStatus shouldEqual 200
+    val presigned = entityAsScalaMap(response)("presignedUrl").toString
+    presigned should include(dataset.getRepositoryName)
+    presigned should include("X-Amz-Signature")
+  }
+
+  it should "decode a percent-encoded file path before resolving it" in {
+    val filePath = "nested dir/a b.txt"
+    val (dataset, version) =
+      seedDatasetWithVersion("public-presign-enc", Seq(filePath -> "encoded"))
+
+    // The encoded form differs from the raw path, so this only resolves if the
+    // endpoint decodes the query parameter first.
+    val encoded = urlEnc(filePath)
+    encoded should not equal filePath
+
+    val response = datasetResource.getPublicPresignedUrlWithS3(
+      encoded,
+      dataset.getRepositoryName,
+      version.getVersionHash
+    )
+
+    response.getStatus shouldEqual 200
+  }
+
+  "getPublicDataset" should "return the dashboard dataset for a public dataset" in {
+    val (dataset, _) = seedDatasetWithVersion("public-dataset", Seq("f.txt" -> "x"))
+
+    val dashboard = datasetResource.getPublicDataset(dataset.getDid)
+
+    dashboard.dataset.getDid shouldEqual dataset.getDid
+    dashboard.dataset.getName shouldEqual dataset.getName
+    dashboard.ownerEmail shouldEqual ownerUser.getEmail
+    dashboard.isOwner shouldBe false
+  }
+
+  it should "refuse an anonymous read of a private dataset" in {
+    val (dataset, _) =
+      seedDatasetWithVersion("private-dataset", Seq("f.txt" -> "x"), isPublic = false)
+
+    assertThrows[ForbiddenException] {
+      datasetResource.getPublicDataset(dataset.getDid)
+    }
+  }
+
+  "retrievePublicDatasetVersionRootFileNodes" should "return the version's nested file tree to an anonymous caller" in {
+    val (dataset, version) = seedDatasetWithVersion(
+      "public-nodes",
+      Seq(
+        "top.txt" -> "root file",
+        "docs/readme.md" -> "docs",
+        "docs/img/logo.png" -> "deeper",
+        "data/train.csv" -> "data"
+      )
+    )
+
+    val response =
+      datasetResource.retrievePublicDatasetVersionRootFileNodes(dataset.getDid, version.getDvid)
+
+    response.fileNodes.map(_.getName).toSet shouldEqual Set("top.txt", "docs", "data")
+
+    // The tree is assembled rather than flattened: docs/ keeps its own children.
+    val docs = response.fileNodes.find(_.getName == "docs").get
+    docs.getChildren.map(_.getName).toSet shouldEqual Set("readme.md", "img")
+    docs.getChildren
+      .find(_.getName == "img")
+      .get
+      .getChildren
+      .map(_.getName) shouldEqual List("logo.png")
+  }
+
+  "retrieveDatasetVersionRootFileNodes" should "return the same tree to the dataset's owner" in {
+    val (dataset, version) =
+      seedDatasetWithVersion("owner-nodes", Seq("a.txt" -> "1", "sub/b.txt" -> "2"))
+
+    val response = datasetResource.retrieveDatasetVersionRootFileNodes(
+      dataset.getDid,
+      version.getDvid,
+      sessionUser
+    )
+
+    response.fileNodes.map(_.getName).toSet shouldEqual Set("a.txt", "sub")
+  }
 }