This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new d1ac3a3eaf fix(file-service): reject invalid or duplicate dataset
renames (#6426)
d1ac3a3eaf is described below
commit d1ac3a3eaf0d49d7eb57565f040844abed6a711b
Author: Kunwoo (Chris) <[email protected]>
AuthorDate: Thu Jul 16 20:04:53 2026 -0400
fix(file-service): reject invalid or duplicate dataset renames (#6426)
### What changes were proposed in this PR?
The dataset rename endpoint previously skipped both checks that dataset
creation enforces: name validation and the per owner duplicate name
check. A rename could therefore produce two datasets with the same
`(owner, name)` pair, which breaks every lookup that resolves a dataset
by `(owner email, dataset name)`. File uploads and workflow file reads
fail with a `TooManyRowsException` for both datasets.
This PR adds the missing guards to the backend rename endpoint, and a
`UNIQUE (owner_uid, name)` constraint at the database level so that
concurrent writes racing past the application-level check cannot
reintroduce duplicates. The migration script `sql/updates/28.sql`
deduplicates pre-existing duplicates before adding the constraint (the
oldest dataset keeps its name, later ones get a `-<did>` suffix). The
frontend rename input now applies the same name validation client-side
and shows the detailed reason from the backend when a rename is
rejected, instead of the generic "Update dataset name failed".
Some additional changes:
- `validateDatasetName` now also enforces the 128-character column limit
- DEFAULT_DATASET_NAME changed from "Untitled dataset" to
"Untitled-dataset", since the fallback name used for empty rename input
must itself pass the new validation
### Any related issues, documentation, discussions?
Closes #6424
### How was this PR tested?
https://github.com/user-attachments/assets/823bd44d-fef3-4aa0-b2c5-e6d27d359d9b
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code, Claude Fable 5
---
.../texera/service/resource/DatasetResource.scala | 92 ++++++----
.../service/resource/DatasetResourceSpec.scala | 188 +++++++++++++++++++++
.../card-item/card-item.component.spec.ts | 42 ++++-
.../list-item/card-item/card-item.component.ts | 21 ++-
.../user/list-item/list-item.component.spec.ts | 58 +++++++
.../user/list-item/list-item.component.ts | 21 ++-
.../user-dataset-list-item.component.spec.ts | 32 +++-
.../user-dataset-list-item.component.ts | 14 +-
.../service/user/dataset/dataset.service.spec.ts | 40 ++++-
.../service/user/dataset/dataset.service.ts | 13 +-
sql/changelog.xml | 5 +
sql/texera_ddl.sql | 3 +-
sql/updates/28.sql | 75 ++++++++
13 files changed, 560 insertions(+), 44 deletions(-)
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 789f6373b9..c376e4ce04 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
@@ -279,21 +279,16 @@ class DatasetResource extends LazyLogging {
val isDatasetPublic = request.isDatasetPublic
val isDatasetDownloadable = request.isDatasetDownloadable
- // validate dataset name
- try {
- validateDatasetName(datasetName)
- } catch {
- case e: IllegalArgumentException =>
- throw new BadRequestException(e.getMessage)
- }
+ validateDatasetName(datasetName)
// Check if a dataset with the same name already exists
- val existingDatasets = context
- .selectFrom(DATASET)
- .where(DATASET.OWNER_UID.eq(uid))
- .and(DATASET.NAME.eq(datasetName))
- .fetch()
- if (!existingDatasets.isEmpty) {
+ val duplicateExists = ctx.fetchExists(
+ ctx
+ .selectFrom(DATASET)
+ .where(DATASET.OWNER_UID.eq(uid))
+ .and(DATASET.NAME.eq(datasetName))
+ )
+ if (duplicateExists) {
throw new BadRequestException("Dataset with the same name already
exists")
}
@@ -306,11 +301,13 @@ class DatasetResource extends LazyLogging {
dataset.setOwnerUid(uid)
// insert record and get created dataset with did
- val createdDataset = ctx
- .insertInto(DATASET)
- .set(ctx.newRecord(DATASET, dataset))
- .returning()
- .fetchOne()
+ val createdDataset = failOnDuplicateDatasetName {
+ ctx
+ .insertInto(DATASET)
+ .set(ctx.newRecord(DATASET, dataset))
+ .returning()
+ .fetchOne()
+ }
// Initialize the repository in LakeFS
val repositoryName = s"dataset-${createdDataset.getDid}"
@@ -514,8 +511,24 @@ class DatasetResource extends LazyLogging {
throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE)
}
+ validateDatasetName(modificator.name)
+
+ // Check if the owner already has another dataset with the same name
+ val duplicateExists = ctx.fetchExists(
+ ctx
+ .selectFrom(DATASET)
+ .where(DATASET.OWNER_UID.eq(dataset.getOwnerUid))
+ .and(DATASET.NAME.eq(modificator.name))
+ .and(DATASET.DID.notEqual(dataset.getDid))
+ )
+ if (duplicateExists) {
+ throw new BadRequestException("Dataset with the same name already
exists")
+ }
+
dataset.setName(modificator.name)
- datasetDao.update(dataset)
+ failOnDuplicateDatasetName {
+ datasetDao.update(dataset)
+ }
Response.ok().build()
}
}
@@ -1423,29 +1436,48 @@ class DatasetResource extends LazyLogging {
.fetchInto(classOf[String])
}
+ private val DATASET_NAME_MAX_LENGTH = 128
+ private val DATASET_NAME_PATTERN = "^[A-Za-z0-9_-]+$".r
+
/**
* Validates the dataset name.
*
* Rules:
- * - Must be at least 1 character long.
- * - Only lowercase letters, numbers, underscores, and hyphens are allowed.
- * - Cannot start with a hyphen.
+ * - Must be 1 to 128 characters long.
+ * - Only letters, numbers, underscores, and hyphens are allowed.
*
* @param name The dataset name to validate.
- * @throws java.lang.IllegalArgumentException if the name is invalid.
+ * @throws jakarta.ws.rs.BadRequestException if the name is invalid.
*/
private def validateDatasetName(name: String): Unit = {
- val datasetNamePattern = "^[A-Za-z0-9_-]+$".r
- if (!datasetNamePattern.matches(name)) {
- throw new IllegalArgumentException(
- s"Invalid dataset name: '$name'. " +
- "Dataset names must be at least 1 character long and " +
- "contain only lowercase letters, numbers, underscores, and hyphens,
" +
- "and cannot start with a hyphen."
+ if (name == null || !DATASET_NAME_PATTERN.matches(name)) {
+ throw new BadRequestException(
+ "Invalid dataset name: only letters, numbers, underscores, and hyphens
are allowed."
+ )
+ }
+ if (name.length > DATASET_NAME_MAX_LENGTH) {
+ throw new BadRequestException(
+ s"Invalid dataset name: name must be at most $DATASET_NAME_MAX_LENGTH
characters long."
)
}
}
+ /**
+ * Runs a dataset write and translates a (owner_uid, name) unique-constraint
+ * violation into the same BadRequestException the pre-checks throw, so
+ * requests losing a concurrent race get a 400 instead of a 500.
+ */
+ private[resource] def failOnDuplicateDatasetName[T](op: => T): T = {
+ try op
+ catch {
+ case e: DataAccessException =>
+ if (e.sqlState() == "23505") {
+ throw new BadRequestException("Dataset with the same name already
exists")
+ }
+ throw e
+ }
+ }
+
private def fetchDatasetVersions(ctx: DSLContext, did: Integer):
List[DatasetVersion] = {
ctx
.selectFrom(DATASET_VERSION)
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 e4e6165241..99d1fd8311 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
@@ -801,6 +801,194 @@ class DatasetResourceSpec
reloaded.getRepositoryName shouldEqual "rename-keeps-repo-stable"
}
+ it should "refuse to rename dataset to an invalid name" in {
+ val dataset = new Dataset
+ dataset.setName("rename-invalid-src")
+ dataset.setRepositoryName("rename-invalid-src-repo")
+ dataset.setDescription("for rename invalid-name test")
+ dataset.setOwnerUid(ownerUser.getUid)
+ dataset.setIsPublic(true)
+ dataset.setIsDownloadable(true)
+ datasetDao.insert(dataset)
+
+ Seq("", "a/b", "has space", "名前", "dot.dot", "a" * 129, null) foreach {
invalidName =>
+ withClue(s"renaming to '$invalidName': ") {
+ assertThrows[BadRequestException] {
+ datasetResource.updateDatasetName(
+ DatasetResource.DatasetNameModification(dataset.getDid,
invalidName),
+ sessionUser
+ )
+ }
+ }
+ }
+
+ datasetDao.fetchOneByDid(dataset.getDid).getName shouldEqual
"rename-invalid-src"
+ }
+
+ it should "refuse to rename dataset to a name already used by another
dataset of the same owner" in {
+ val existing = new Dataset
+ existing.setName("rename-dup-existing")
+ existing.setRepositoryName("rename-dup-existing-repo")
+ existing.setDescription("existing dataset for duplicate rename test")
+ existing.setOwnerUid(ownerUser.getUid)
+ existing.setIsPublic(true)
+ existing.setIsDownloadable(true)
+ datasetDao.insert(existing)
+
+ val renamed = new Dataset
+ renamed.setName("rename-dup-source")
+ renamed.setRepositoryName("rename-dup-source-repo")
+ renamed.setDescription("dataset being renamed in duplicate rename test")
+ renamed.setOwnerUid(ownerUser.getUid)
+ renamed.setIsPublic(true)
+ renamed.setIsDownloadable(true)
+ datasetDao.insert(renamed)
+
+ assertThrows[BadRequestException] {
+ datasetResource.updateDatasetName(
+ DatasetResource.DatasetNameModification(renamed.getDid,
"rename-dup-existing"),
+ sessionUser
+ )
+ }
+
+ datasetDao.fetchOneByDid(renamed.getDid).getName shouldEqual
"rename-dup-source"
+ }
+
+ it should "allow renaming a dataset to its own current name" in {
+ val dataset = new Dataset
+ dataset.setName("rename-self-noop")
+ dataset.setRepositoryName("rename-self-noop-repo")
+ dataset.setDescription("for rename-to-self test")
+ dataset.setOwnerUid(ownerUser.getUid)
+ dataset.setIsPublic(true)
+ dataset.setIsDownloadable(true)
+ datasetDao.insert(dataset)
+
+ val response = datasetResource.updateDatasetName(
+ DatasetResource.DatasetNameModification(dataset.getDid,
"rename-self-noop"),
+ sessionUser
+ )
+
+ response.getStatus shouldEqual 200
+ datasetDao.fetchOneByDid(dataset.getDid).getName shouldEqual
"rename-self-noop"
+ }
+
+ it should "allow renaming to a name used by a dataset of a different owner"
in {
+ val otherOwners = new Dataset
+ otherOwners.setName("rename-cross-owner")
+ otherOwners.setRepositoryName("rename-cross-owner-repo")
+ otherOwners.setDescription("other owner's dataset for cross-owner rename
test")
+ otherOwners.setOwnerUid(otherAdminUser.getUid)
+ otherOwners.setIsPublic(true)
+ otherOwners.setIsDownloadable(true)
+ datasetDao.insert(otherOwners)
+
+ val dataset = new Dataset
+ dataset.setName("rename-cross-source")
+ dataset.setRepositoryName("rename-cross-source-repo")
+ dataset.setDescription("dataset being renamed in cross-owner rename test")
+ dataset.setOwnerUid(ownerUser.getUid)
+ dataset.setIsPublic(true)
+ dataset.setIsDownloadable(true)
+ datasetDao.insert(dataset)
+
+ val response = datasetResource.updateDatasetName(
+ DatasetResource.DatasetNameModification(dataset.getDid,
"rename-cross-owner"),
+ sessionUser
+ )
+
+ response.getStatus shouldEqual 200
+ datasetDao.fetchOneByDid(dataset.getDid).getName shouldEqual
"rename-cross-owner"
+ }
+
+ "dataset table" should "enforce uniqueness of (owner_uid, name) at the
database level" in {
+ val first = new Dataset
+ first.setName("db-unique-ds")
+ first.setRepositoryName("db-unique-ds-repo")
+ first.setDescription("first dataset for unique constraint test")
+ first.setOwnerUid(ownerUser.getUid)
+ first.setIsPublic(true)
+ first.setIsDownloadable(true)
+ datasetDao.insert(first)
+
+ val duplicate = new Dataset
+ duplicate.setName("db-unique-ds")
+ duplicate.setRepositoryName("db-unique-ds-repo-2")
+ duplicate.setDescription("duplicate dataset for unique constraint test")
+ duplicate.setOwnerUid(ownerUser.getUid)
+ duplicate.setIsPublic(true)
+ duplicate.setIsDownloadable(true)
+
+ assertThrows[org.jooq.exception.DataAccessException] {
+ datasetDao.insert(duplicate)
+ }
+ }
+
+ "failOnDuplicateDatasetName" should "translate a unique-constraint violation
into BadRequestException" in {
+ val existing = new Dataset
+ existing.setName("race-existing")
+ existing.setRepositoryName("race-existing-repo")
+ existing.setDescription("existing dataset for constraint-translation test")
+ existing.setOwnerUid(ownerUser.getUid)
+ existing.setIsPublic(true)
+ existing.setIsDownloadable(true)
+ datasetDao.insert(existing)
+
+ val victim = new Dataset
+ victim.setName("race-victim")
+ victim.setRepositoryName("race-victim-repo")
+ victim.setDescription("dataset whose rename loses the race")
+ victim.setOwnerUid(ownerUser.getUid)
+ victim.setIsPublic(true)
+ victim.setIsDownloadable(true)
+ datasetDao.insert(victim)
+
+ // Simulate the write that loses the race: the constraint fires because the
+ // duplicate already exists, and the helper must map it to a 400.
+ victim.setName("race-existing")
+ assertThrows[BadRequestException] {
+ datasetResource.failOnDuplicateDatasetName {
+ datasetDao.update(victim)
+ }
+ }
+
+ datasetDao.fetchOneByDid(victim.getDid).getName shouldEqual "race-victim"
+ }
+
+ it should "rethrow DataAccessExceptions that are not unique violations" in {
+ assertThrows[org.jooq.exception.DataAccessException] {
+ datasetResource.failOnDuplicateDatasetName {
+ throw new org.jooq.exception.DataAccessException(
+ "lock timeout",
+ new java.sql.SQLException("lock timeout", "55P03")
+ )
+ }
+ }
+ }
+
+ it should "rethrow DataAccessExceptions whose cause carries no SQL state" in
{
+ assertThrows[org.jooq.exception.DataAccessException] {
+ datasetResource.failOnDuplicateDatasetName {
+ throw new org.jooq.exception.DataAccessException(
+ "no sql state",
+ new java.sql.SQLException("constructed without a SQL state")
+ )
+ }
+ }
+ }
+
+ it should "let exceptions other than DataAccessException propagate
unchanged" in {
+ assertThrows[IllegalStateException] {
+ datasetResource.failOnDuplicateDatasetName {
+ throw new IllegalStateException("unrelated failure")
+ }
+ }
+ }
+
+ it should "return the result of the operation when no exception is thrown"
in {
+ datasetResource.failOnDuplicateDatasetName(42) shouldEqual 42
+ }
+
//
===========================================================================
// Multipart upload tests (merged in)
//
===========================================================================
diff --git
a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts
b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts
index ff174e93bc..bc51a33198 100644
---
a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts
+++
b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts
@@ -81,7 +81,7 @@ describe("CardItemComponent", () => {
setCoverFromFile: vi.fn(),
clearCover: vi.fn().mockReturnValue(of(undefined)),
};
- const datasetServiceSpy = { getDatasetCoverUrl: vi.fn() };
+ const datasetServiceSpy = { getDatasetCoverUrl: vi.fn(),
updateDatasetName: vi.fn() };
await TestBed.configureTestingModule({
imports: [CardItemComponent, HttpClientTestingModule,
BrowserAnimationsModule, RouterTestingModule],
@@ -127,6 +127,46 @@ describe("CardItemComponent", () => {
expect(component.editingName).toBe(false);
});
+ it("should reject an invalid dataset name, revert to original, and exit
editing", () => {
+ component.entry = makeDatasetEntry({ id: 5, name: "invalid name" });
+ component.originalName = "original-name";
+ component.editingName = true;
+ const notificationService = TestBed.inject(NotificationService);
+ const errorSpy = vi.spyOn(notificationService, "error");
+
+ component.confirmUpdateCustomName("invalid name");
+
+ expect(datasetService.updateDatasetName).not.toHaveBeenCalled();
+ expect(errorSpy).toHaveBeenCalled();
+ expect(component.entry.name).toBe("original-name");
+ expect(component.editingName).toBe(false);
+ });
+
+ it("should call the dataset service for a valid dataset rename", () => {
+ component.entry = makeDatasetEntry({ id: 5, name: "new-valid-name" });
+ component.originalName = "old-name";
+ datasetService.updateDatasetName.mockReturnValue(of({} as any));
+
+ component.confirmUpdateCustomName("new-valid-name");
+
+ expect(datasetService.updateDatasetName).toHaveBeenCalledWith(5,
"new-valid-name");
+ });
+
+ it("should surface the error message and revert the name when a dataset
rename fails", () => {
+ component.entry = makeDatasetEntry({ id: 5, name: "new-valid-name" });
+ component.originalName = "old-name";
+ component.editingName = true;
+ datasetService.updateDatasetName.mockReturnValue(throwError(() => new
Error("boom")));
+ const notificationService = TestBed.inject(NotificationService);
+ const errorSpy = vi.spyOn(notificationService, "error");
+
+ component.confirmUpdateCustomName("new-valid-name");
+
+ expect(errorSpy).toHaveBeenCalledWith("boom");
+ expect(component.entry.name).toBe("old-name");
+ expect(component.editingName).toBe(false);
+ });
+
it("should update workflow description successfully", () => {
component.entry = makeWorkflowEntry({ id: 1, description: "Old
Description" });
workflowPersistService.updateWorkflowDescription.mockReturnValue(of({} as
Response));
diff --git
a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts
b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts
index 7048daec4c..c890bc4a25 100644
---
a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts
+++
b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts
@@ -52,8 +52,13 @@ import { ActionType, HubService } from
"../../../../../hub/service/hub.service";
import { DownloadService } from
"src/app/dashboard/service/user/download/download.service";
import { formatSize } from "src/app/common/util/size-formatter.util";
import { formatRelativeTime, formatCount } from
"src/app/common/util/format.util";
-import { DatasetService, DEFAULT_DATASET_NAME } from
"../../../../service/user/dataset/dataset.service";
+import {
+ DatasetService,
+ DEFAULT_DATASET_NAME,
+ validateDatasetName,
+} from "../../../../service/user/dataset/dataset.service";
import { NotificationService } from
"../../../../../common/service/notification/notification.service";
+import { extractErrorMessage } from "../../../../../common/util/error";
import { WorkflowCoverService } from
"../../../../service/user/workflow-cover/workflow-cover.service";
import {
HUB_DATASET_RESULT_DETAIL,
@@ -370,8 +375,8 @@ export class CardItemComponent implements OnChanges {
next: () => {
this.entry[propertyName] = newValue; // Dynamic property assignment
},
- error: () => {
- this.notificationService.error("Update failed");
+ error: (err: unknown) => {
+ this.notificationService.error(extractErrorMessage(err));
(this.entry as any)[propertyName] = originalValue ?? ""; // Fallback
to original value
this.setEditingState(propertyName, false);
},
@@ -396,6 +401,16 @@ export class CardItemComponent implements OnChanges {
}
const newName = this.entry.type === "workflow" ? name ||
DEFAULT_WORKFLOW_NAME : name || DEFAULT_DATASET_NAME;
+ if (this.entry.type === "dataset") {
+ const nameError = validateDatasetName(newName);
+ if (nameError) {
+ this.notificationService.error(nameError);
+ this.entry.name = this.originalName;
+ this.editingName = false;
+ return;
+ }
+ }
+
if (this.entry.type === "workflow") {
this.updateProperty(
this.workflowPersistService.updateWorkflowName.bind(this.workflowPersistService),
diff --git
a/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts
b/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts
index 44bec7e07f..10b07b82ce 100644
---
a/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts
+++
b/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts
@@ -30,6 +30,8 @@ import { UserService } from
"../../../../common/service/user/user.service";
import { commonTestProviders } from "../../../../common/testing/test-utils";
import type { Mocked } from "vitest";
import { DashboardEntry } from "src/app/dashboard/type/dashboard-entry";
+import { DatasetService } from "../../../service/user/dataset/dataset.service";
+import { NotificationService } from
"../../../../common/service/notification/notification.service";
import {
HUB_DATASET_RESULT_DETAIL,
HUB_WORKFLOW_RESULT_DETAIL,
@@ -42,14 +44,17 @@ describe("ListItemComponent", () => {
let component: ListItemComponent;
let fixture: ComponentFixture<ListItemComponent>;
let workflowPersistService: Mocked<WorkflowPersistService>;
+ let datasetService: Mocked<DatasetService>;
beforeEach(async () => {
const workflowPersistServiceSpy = { updateWorkflowName: vi.fn(),
updateWorkflowDescription: vi.fn() };
+ const datasetServiceSpy = { updateDatasetName: vi.fn() };
await TestBed.configureTestingModule({
imports: [ListItemComponent, HttpClientTestingModule,
BrowserAnimationsModule, RouterTestingModule],
providers: [
{ provide: WorkflowPersistService, useValue: workflowPersistServiceSpy
},
+ { provide: DatasetService, useValue: datasetServiceSpy },
{ provide: UserService, useClass: StubUserService },
NzModalService,
...commonTestProviders,
@@ -59,6 +64,7 @@ describe("ListItemComponent", () => {
fixture = TestBed.createComponent(ListItemComponent);
component = fixture.componentInstance;
workflowPersistService = TestBed.inject(WorkflowPersistService) as unknown
as Mocked<WorkflowPersistService>;
+ datasetService = TestBed.inject(DatasetService) as unknown as
Mocked<DatasetService>;
// initializeEntry() needs a fully-formed workflow entry to avoid throwing
// when the template renders for the first time. Each test below overwrites
// component.entry directly, which exercises confirm methods without going
@@ -189,4 +195,56 @@ describe("ListItemComponent", () => {
expect(component.entryLink).toEqual([HUB_DATASET_RESULT_DETAIL, "301"]);
});
});
+
+ it("should reject an invalid dataset name, revert to original, and exit
editing", () => {
+ component.entry = {
+ id: 5,
+ name: "has space",
+ type: "dataset",
+ } as unknown as DashboardEntry;
+ component.originalName = "original-name";
+ component.editingName = true;
+ const notificationService = TestBed.inject(NotificationService);
+ const errorSpy = vi.spyOn(notificationService, "error");
+
+ component.confirmUpdateCustomName("has space");
+
+ expect(datasetService.updateDatasetName).not.toHaveBeenCalled();
+ expect(errorSpy).toHaveBeenCalled();
+ expect(component.entry.name).toBe("original-name");
+ expect(component.editingName).toBe(false);
+ });
+
+ it("should call the dataset service for a valid dataset rename", () => {
+ component.entry = {
+ id: 5,
+ name: "new-valid-name",
+ type: "dataset",
+ } as unknown as DashboardEntry;
+ component.originalName = "old-name";
+ datasetService.updateDatasetName.mockReturnValue(of({} as any));
+
+ component.confirmUpdateCustomName("new-valid-name");
+
+ expect(datasetService.updateDatasetName).toHaveBeenCalledWith(5,
"new-valid-name");
+ });
+
+ it("should surface the error message and revert the name when a dataset
rename fails", () => {
+ component.entry = {
+ id: 5,
+ name: "new-valid-name",
+ type: "dataset",
+ } as unknown as DashboardEntry;
+ component.originalName = "old-name";
+ component.editingName = true;
+ datasetService.updateDatasetName.mockReturnValue(throwError(() => new
Error("boom")));
+ const notificationService = TestBed.inject(NotificationService);
+ const errorSpy = vi.spyOn(notificationService, "error");
+
+ component.confirmUpdateCustomName("new-valid-name");
+
+ expect(errorSpy).toHaveBeenCalledWith("boom");
+ expect(component.entry.name).toBe("old-name");
+ expect(component.editingName).toBe(false);
+ });
});
diff --git
a/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts
b/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts
index 5cca45ced4..5291b79b1d 100644
--- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts
+++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts
@@ -43,8 +43,13 @@ import { ActionType, HubService } from
"../../../../hub/service/hub.service";
import { DownloadService } from
"src/app/dashboard/service/user/download/download.service";
import { formatSize } from "src/app/common/util/size-formatter.util";
import { formatCount, formatRelativeTime } from
"src/app/common/util/format.util";
-import { DatasetService, DEFAULT_DATASET_NAME } from
"../../../service/user/dataset/dataset.service";
+import {
+ DatasetService,
+ DEFAULT_DATASET_NAME,
+ validateDatasetName,
+} from "../../../service/user/dataset/dataset.service";
import { NotificationService } from
"../../../../common/service/notification/notification.service";
+import { extractErrorMessage } from "../../../../common/util/error";
import {
HUB_DATASET_RESULT_DETAIL,
HUB_WORKFLOW_RESULT_DETAIL,
@@ -311,8 +316,8 @@ export class ListItemComponent implements OnChanges {
this.renderMarkdownPreview(newValue);
}
},
- error: () => {
- this.notificationService.error("Update failed");
+ error: (err: unknown) => {
+ this.notificationService.error(extractErrorMessage(err));
(this.entry as any)[propertyName] = originalValue ?? ""; // Fallback
to original value
if (propertyName === "description") {
this.renderMarkdownPreview(originalValue);
@@ -336,6 +341,16 @@ export class ListItemComponent implements OnChanges {
public confirmUpdateCustomName(name: string): void {
const newName = this.entry.type === "workflow" ? name ||
DEFAULT_WORKFLOW_NAME : name || DEFAULT_DATASET_NAME;
+ if (this.entry.type === "dataset") {
+ const nameError = validateDatasetName(newName);
+ if (nameError) {
+ this.notificationService.error(nameError);
+ this.entry.name = this.originalName;
+ this.editingName = false;
+ return;
+ }
+ }
+
if (this.entry.type === "workflow") {
this.updateProperty(
this.workflowPersistService.updateWorkflowName.bind(this.workflowPersistService),
diff --git
a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.spec.ts
b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.spec.ts
index eba371fdb4..513d803b87 100644
---
a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.spec.ts
+++
b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.spec.ts
@@ -189,7 +189,37 @@ describe("UserDatasetListItemComponent", () => {
component.confirmUpdateDatasetCustomName("renamed-dataset");
-
expect(notificationService.error).toHaveBeenCalledExactlyOnceWith("Update
dataset name failed");
+
expect(notificationService.error).toHaveBeenCalledExactlyOnceWith("boom");
+ expect(component.entry.dataset.name).toBe(originalName);
+ expect(component.editingName).toBe(false);
+ });
+
+ ["", "a/b", "has space", "名前"].forEach(invalidName => {
+ it(`rejects the invalid name '${invalidName}' without calling the
service`, () => {
+ const originalName = component.entry.dataset.name;
+ component.editingName = true;
+
+ component.confirmUpdateDatasetCustomName(invalidName);
+
+ expect(datasetService.updateDatasetName).not.toHaveBeenCalled();
+ expect(notificationService.error).toHaveBeenCalledExactlyOnceWith(
+ "Invalid dataset name: only letters, numbers, underscores, and
hyphens are allowed (max 128 characters)"
+ );
+ expect(component.entry.dataset.name).toBe(originalName);
+ expect(component.editingName).toBe(false);
+ });
+ });
+
+ it("surfaces the backend error message when the rename is rejected", () =>
{
+ const originalName = component.entry.dataset.name;
+ component.editingName = true;
+ datasetService.updateDatasetName.mockReturnValue(
+ throwError(() => ({ error: { message: "Dataset with the same name
already exists" } }))
+ );
+
+ component.confirmUpdateDatasetCustomName("renamed-dataset");
+
+
expect(notificationService.error).toHaveBeenCalledExactlyOnceWith("Dataset with
the same name already exists");
expect(component.entry.dataset.name).toBe(originalName);
expect(component.editingName).toBe(false);
});
diff --git
a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.ts
b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.ts
index 6846911823..e6e583c4f3 100644
---
a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.ts
+++
b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.ts
@@ -20,9 +20,10 @@
import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy";
import { Component, EventEmitter, Input, Output } from "@angular/core";
import { Dataset } from "../../../../../common/type/dataset";
-import { DatasetService } from
"../../../../service/user/dataset/dataset.service";
+import { DatasetService, validateDatasetName } from
"../../../../service/user/dataset/dataset.service";
import { ShareAccessComponent } from
"../../share-access/share-access.component";
import { NotificationService } from
"../../../../../common/service/notification/notification.service";
+import { extractErrorMessage } from "../../../../../common/util/error";
import { NzModalService } from "ng-zorro-antd/modal";
import { DashboardDataset } from
"../../../../type/dashboard-dataset.interface";
import { USER_DATASET } from "../../../../../app-routing.constant";
@@ -119,6 +120,13 @@ export class UserDatasetListItemComponent {
return;
}
+ const nameError = validateDatasetName(name);
+ if (nameError) {
+ this.notificationService.error(nameError);
+ this.editingName = false;
+ return;
+ }
+
if (this.entry.dataset.did)
this.datasetService
.updateDatasetName(this.entry.dataset.did, name)
@@ -128,8 +136,8 @@ export class UserDatasetListItemComponent {
this.entry.dataset.name = name;
this.editingName = false;
},
- error: () => {
- this.notificationService.error("Update dataset name failed");
+ error: (err: unknown) => {
+ this.notificationService.error(extractErrorMessage(err));
this.editingName = false;
},
});
diff --git
a/frontend/src/app/dashboard/service/user/dataset/dataset.service.spec.ts
b/frontend/src/app/dashboard/service/user/dataset/dataset.service.spec.ts
index 229ac32c5e..c5ec10cd7c 100644
--- a/frontend/src/app/dashboard/service/user/dataset/dataset.service.spec.ts
+++ b/frontend/src/app/dashboard/service/user/dataset/dataset.service.spec.ts
@@ -21,7 +21,7 @@ import { TestBed } from "@angular/core/testing";
import { HttpClientTestingModule, HttpTestingController } from
"@angular/common/http/testing";
import { firstValueFrom } from "rxjs";
-import { DATASET_BASE_URL, DatasetService } from "./dataset.service";
+import { DATASET_BASE_URL, DatasetService, validateDatasetName } from
"./dataset.service";
import { AppSettings } from "../../../../common/app-setting";
import { commonTestProviders } from "../../../../common/testing/test-utils";
import { Dataset, DatasetVersion } from "../../../../common/type/dataset";
@@ -116,6 +116,44 @@ class FakeXMLHttpRequest {
}
}
+describe("validateDatasetName", () => {
+ it("returns null for a valid name", () => {
+ expect(validateDatasetName("my-dataset_1")).toBeNull();
+ });
+
+ it("returns null for a single valid character", () => {
+ expect(validateDatasetName("a")).toBeNull();
+ });
+
+ it("returns null for a name exactly at the 128-character limit", () => {
+ expect(validateDatasetName("a".repeat(128))).toBeNull();
+ });
+
+ it("returns an error for an empty string", () => {
+ expect(validateDatasetName("")).not.toBeNull();
+ });
+
+ it("returns an error for names with spaces", () => {
+ expect(validateDatasetName("has space")).not.toBeNull();
+ });
+
+ it("returns an error for names with dots", () => {
+ expect(validateDatasetName("dot.dot")).not.toBeNull();
+ });
+
+ it("returns an error for names with slashes", () => {
+ expect(validateDatasetName("a/b")).not.toBeNull();
+ });
+
+ it("returns an error for names with non-ASCII characters", () => {
+ expect(validateDatasetName("名前")).not.toBeNull();
+ });
+
+ it("returns an error for names exceeding 128 characters", () => {
+ expect(validateDatasetName("a".repeat(129))).not.toBeNull();
+ });
+});
+
describe("DatasetService", () => {
let service: DatasetService;
let http: HttpTestingController;
diff --git a/frontend/src/app/dashboard/service/user/dataset/dataset.service.ts
b/frontend/src/app/dashboard/service/user/dataset/dataset.service.ts
index 6ec6a25aba..38cb3f2fce 100644
--- a/frontend/src/app/dashboard/service/user/dataset/dataset.service.ts
+++ b/frontend/src/app/dashboard/service/user/dataset/dataset.service.ts
@@ -43,7 +43,18 @@ export const DATASET_DELETE_URL = DATASET_BASE_URL +
"/delete";
export const DATASET_VERSION_BASE_URL = "version";
export const DATASET_VERSION_RETRIEVE_LIST_URL = DATASET_VERSION_BASE_URL +
"/list";
export const DATASET_VERSION_LATEST_URL = DATASET_VERSION_BASE_URL + "/latest";
-export const DEFAULT_DATASET_NAME = "Untitled dataset";
+export const DEFAULT_DATASET_NAME = "Untitled-dataset";
+
+export const DATASET_NAME_MAX_LENGTH = 128;
+const DATASET_NAME_PATTERN = /^[A-Za-z0-9_-]+$/;
+
+export function validateDatasetName(name: string): string | null {
+ if (!DATASET_NAME_PATTERN.test(name) || name.length >
DATASET_NAME_MAX_LENGTH) {
+ return "Invalid dataset name: only letters, numbers, underscores, and
hyphens are allowed (max 128 characters)";
+ }
+ return null;
+}
+
export const DATASET_PUBLIC_VERSION_BASE_URL = "publicVersion";
export const DATASET_PUBLIC_VERSION_RETRIEVE_LIST_URL =
DATASET_PUBLIC_VERSION_BASE_URL + "/list";
export const DATASET_GET_OWNERS_URL = DATASET_BASE_URL +
"/user-dataset-owners";
diff --git a/sql/changelog.xml b/sql/changelog.xml
index c9f4b0d191..469868f895 100644
--- a/sql/changelog.xml
+++ b/sql/changelog.xml
@@ -48,6 +48,11 @@
<sqlFile path="sql/updates/27.sql"/>
</changeSet>
+ <!-- Enforce unique dataset names per owner -->
+ <changeSet id="28" author="kunwp1">
+ <sqlFile path="sql/updates/28.sql"/>
+ </changeSet>
+
<!-- example changeSet
<changeSet id="1" author="author">
<sqlFile path="sql/updates/1.sql"/>
diff --git a/sql/texera_ddl.sql b/sql/texera_ddl.sql
index 8202614932..8a1e8782cd 100644
--- a/sql/texera_ddl.sql
+++ b/sql/texera_ddl.sql
@@ -287,7 +287,8 @@ CREATE TABLE IF NOT EXISTS dataset
description TEXT NOT NULL,
creation_time TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
cover_image varchar(255),
- FOREIGN KEY (owner_uid) REFERENCES "user"(uid) ON DELETE CASCADE
+ FOREIGN KEY (owner_uid) REFERENCES "user"(uid) ON DELETE CASCADE,
+ UNIQUE (owner_uid, name)
);
-- dataset_user_access
diff --git a/sql/updates/28.sql b/sql/updates/28.sql
new file mode 100644
index 0000000000..5fa0b9bc0e
--- /dev/null
+++ b/sql/updates/28.sql
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+\c texera_db
+
+SET search_path TO texera_db;
+
+BEGIN;
+
+-- Datasets are looked up by (owner, name) in the file service and the file
+-- resolver, so this pair must be unique. Before adding the constraint,
+-- deterministically rename any pre-existing duplicates (kept: the oldest did;
+-- renamed: name suffixed with "-<did>", truncated to fit VARCHAR(128)).
+-- Each rename is reported via RAISE NOTICE: this is a user-visible data
+-- change, and workflows that reference a renamed dataset by path will resolve
+-- to the surviving dataset afterward, so operators should review the notices
+-- and notify the affected dataset owners.
+DO $$
+DECLARE
+ rec RECORD;
+ renamed INT := 0;
+ iterations INT := 0;
+BEGIN
+ LOOP
+ FOR rec IN
+ UPDATE dataset d
+ SET name = LEFT(d.name, 128 - LENGTH('-' || d.did::text)) || '-'
|| d.did::text
+ FROM (
+ SELECT did, name AS old_name,
+ ROW_NUMBER() OVER (PARTITION BY owner_uid, name ORDER
BY did) AS rn
+ FROM dataset
+ ) dups
+ WHERE d.did = dups.did AND dups.rn > 1
+ RETURNING d.did, d.owner_uid, dups.old_name, d.name AS new_name
+ LOOP
+ renamed := renamed + 1;
+ RAISE NOTICE 'Renamed duplicate dataset did=% (owner_uid=%): "%"
-> "%"',
+ rec.did, rec.owner_uid, rec.old_name, rec.new_name;
+ END LOOP;
+
+ EXIT WHEN NOT EXISTS (
+ SELECT 1 FROM dataset GROUP BY owner_uid, name HAVING COUNT(*) > 1
+ );
+
+ iterations := iterations + 1;
+ IF iterations > 10 THEN
+ RAISE EXCEPTION 'Could not deduplicate dataset (owner_uid, name)
pairs after 10 passes; resolve duplicates manually before re-running.';
+ END IF;
+ END LOOP;
+
+ IF renamed > 0 THEN
+ RAISE NOTICE 'Renamed % duplicate dataset name(s) in total; workflows
referencing the old names now resolve to the surviving datasets.', renamed;
+ END IF;
+END $$;
+
+ALTER TABLE dataset
+ ADD CONSTRAINT dataset_owner_uid_name_key UNIQUE (owner_uid, name);
+
+COMMIT;
