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-8875-8bfe034af17f88e8396b4961714658a48f79cff2 in repository https://gitbox.apache.org/repos/asf/texera.git
commit ea0fc10cd8478b3b7368910680bede73a3246e73 Author: Kunwoo (Chris) <[email protected]> AuthorDate: Sat Oct 10 21:59:26 2026 +0000 fix(file-service): reject over-long dataset version names (#8875) ### What changes were proposed in this PR? Root cause. `createDatasetVersion` builds the name `v{N} - {description}` and commits to LakeFS first, then inserts the `dataset_version` row (`DatasetResource.scala`). `dataset_version.name` is `VARCHAR(128)`, so a long description makes the insert fail after the commit, and the commit is outside the DB transaction. The request returns a 500, the staged files are already committed with no version row, and the dataset page then shows neither a pending change nor a version until the next version is created. Fix. `createDatasetVersion` now rejects a name longer than 128 characters with a 400 before it creates the LakeFS commit, using the same check `createModelVersion` already has. The length is counted in code points rather than UTF-16 code units, since `VARCHAR(n)` counts characters and an emoji would otherwise be rejected early. The staged files stay pending, so the user can shorten the description and submit again. The frontend already shows the message from the response, so it needs no change. ``` Before: Submit -> LakeFS commit -> INSERT dataset_version fails -> 500, files committed, no version row After: Submit -> name over 128 characters -> 400 "Version name is too long: ..." -> files stay pending ``` ### Any related issues, documentation, discussions? Closes #8873 ### How was this PR tested? Added five tests to `DatasetResourceSpec`: version naming (null, empty and non-empty description), a name of exactly 128 characters (accepted), a name of 129 characters (rejected with a 400, nothing committed, and a retry succeeds), a 128-character name made of emoji (accepted) and a 129-character one (rejected), and the existing no-staged-changes 400. Before the fix, the over-long test failed with `org.jooq.exception.DataException` from the insert. A first version of the guard that used `String.length` failed the emoji test, which is why the length is counted in code points. `sbt "FileService/testOnly *DatasetResourceSpec"` gives 157 passed, 0 failed, and `scalafixAll` and `scalafmtAll` change nothing. This ran locally with a MinIO container in place of the RustFS image, which could not be pulled on that machine, so CI is the run against RustFS. It was not exercised through the UI. https://github.com/user-attachments/assets/f3e5d22d-89fb-477a-88a0-37f77c4e09c7 ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code, Claude Sonnet 5.5 --- .../texera/service/resource/DatasetResource.scala | 14 ++++ .../service/resource/DatasetResourceSpec.scala | 94 ++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala index 76eb3ebd74..4da407fd31 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala @@ -270,6 +270,9 @@ class DatasetResource extends LazyLogging { // varchar(246) on DBs upgraded through sql/updates/18.sql, narrower than model's. private val COVER_IMAGE_MAX_PATH_LENGTH = 246 + // Matches dataset_version.name VARCHAR(128). + private val MAX_VERSION_NAME_LENGTH = 128 + private val resourceType = ResourceType.Dataset /** @@ -450,6 +453,17 @@ class DatasetResource extends LazyLogging { s"v${versionCount + 1} - $sanitizedVersionName" } + // Before the commit: the commit is outside this transaction, so a name the insert + // rejects would leave a commit no version points at and strand the staged files. + // Code points, not String.length: VARCHAR(n) counts characters, so an emoji counts once. + val newVersionNameLength = newVersionName.codePointCount(0, newVersionName.length) + if (newVersionNameLength > MAX_VERSION_NAME_LENGTH) { + throw new BadRequestException( + s"Version name is too long: $newVersionNameLength characters, " + + s"maximum is $MAX_VERSION_NAME_LENGTH." + ) + } + // Create a commit in LakeFS val commit = withLakeFSErrorHandling { LakeFSStorageClient.createCommit( 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 b728b748c5..db4c2437eb 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 @@ -3881,4 +3881,98 @@ class DatasetResourceSpec .get .getFilePath should startWith("/dataset/") } + + // --------------------------------------------------------------------------- + // createDatasetVersion + // --------------------------------------------------------------------------- + + // The state right after a user drops a file on "Create New Version": a dataset whose + // repository holds one staged (uncommitted) file and no version row yet. + private def seedDatasetWithStagedFile(prefix: String): Dataset = { + val dataset = seedDataset(s"$prefix-${System.nanoTime()}", ownerUser.getUid) + LakeFSStorageClient.initRepo(dataset.getRepositoryName) + stageFile(dataset, "staged.txt") + dataset + } + + private def stageFile(dataset: Dataset, path: String): Unit = + LakeFSStorageClient.writeFileToRepo( + dataset.getRepositoryName, + path, + new ByteArrayInputStream("content".getBytes(StandardCharsets.UTF_8)) + ) + + private def createVersion( + dataset: Dataset, + description: String + ): DatasetResource.DashboardDatasetVersion = + datasetResource.createDatasetVersion(description, dataset.getDid, sessionUser) + + // "v1 - " takes 5 of the 128 characters dataset_version.name allows. + private val maxDescriptionLength = 128 - "v1 - ".length + + "createDatasetVersion" should "name versions by their number, followed by the description when given" in { + val dataset = seedDatasetWithStagedFile("version-naming") + + createVersion(dataset, null).datasetVersion.getName shouldBe "v1" + + stageFile(dataset, "second.txt") + createVersion(dataset, "second upload").datasetVersion.getName shouldBe "v2 - second upload" + + stageFile(dataset, "third.txt") + createVersion(dataset, "").datasetVersion.getName shouldBe "v3" + } + + it should "accept a description that makes the version name exactly 128 characters" in { + val dataset = seedDatasetWithStagedFile("version-name-at-limit") + + val created = createVersion(dataset, "d" * maxDescriptionLength) + + created.datasetVersion.getName should have length 128 + } + + it should "reject a description that pushes the version name past 128 characters without committing" in { + val dataset = seedDatasetWithStagedFile("version-name-too-long") + + val ex = intercept[WebApplicationException] { + createVersion(dataset, "d" * (maxDescriptionLength + 1)) + } + assertStatus(ex, 400) + ex.getMessage should include("too long") + + // Nothing was committed: the file is still staged, so the dataset page keeps showing it + // as a pending change. + LakeFSStorageClient + .retrieveUncommittedObjects(dataset.getRepositoryName) + .map(_.getPath) shouldBe List("staged.txt") + + // The user can fix the description and retry; no version row was created, so it is still v1. + createVersion(dataset, "shorter").datasetVersion.getName shouldBe "v1 - shorter" + } + + it should "count characters rather than UTF-16 code units against the 128 limit" in { + val dataset = seedDatasetWithStagedFile("version-name-unicode") + // U+1F600 is one character but two UTF-16 code units. + val emoji = new String(Character.toChars(0x1f600)) + + val ex = intercept[WebApplicationException] { + createVersion(dataset, emoji * (maxDescriptionLength + 1)) + } + assertStatus(ex, 400) + ex.getMessage should include("129 characters") + + val name = createVersion(dataset, emoji * maxDescriptionLength).datasetVersion.getName + name.codePointCount(0, name.length) shouldBe 128 + } + + it should "reject a version when there are no staged changes" in { + val dataset = seedDatasetWithStagedFile("version-no-changes") + createVersion(dataset, "first") + + val ex = intercept[WebApplicationException] { + createVersion(dataset, "empty") + } + assertStatus(ex, 400) + datasetResource.getDatasetVersionList(dataset.getDid, sessionUser) should have size 1 + } }
