feat(file-service): add model management API and unify shared resource access by tanishqgandhi1908 · Pull Request #6869 · apache/texera

tanishqgandhi1908 · 2026-07-24T21:23:41Z

What changes were proposed in this PR?

Adds the metadata + access-control half of the model management API, and unifies the
code datasets and models share. Model version upload follows in a separate PR.

Schema: model_user_access table in texera_ddl.sql + idempotent migration sql/updates/31.sql (changeSet 31).
ModelResource (@Path("/model"))

Method	Path	Description
POST	`/create`	Create a model; provisions LakeFS repo `model-{mid}`, rolls the DB row back if repo init fails, grants the owner `WRITE`. Stores `framework` (defaults `pytorch`) and `format`.
DELETE	`/{mid}`	Delete a model (owner-only): removes the LakeFS repo, the S3 directory, and the row.
POST	`/update/name`	Rename (write access; validates name; rejects duplicate).
POST	`/update/description`	Update description (write access).
POST	`/{mid}/update/publicity`	Toggle `is_public` (write access).
POST	`/{mid}/update/downloadable`	Toggle `is_downloadable` (owner-only).
GET	`/list`	Models the user can access, plus public models.
GET	`/{mid}`	Get a model the user can read.
GET	`/public/{mid}`	Get a public model (unauthenticated).

ModelAccessResource (@Path("/access/model")) — owner/{mid}, list/{mid}, grant/{mid}/{email}/{privilege}, revoke/{mid}/{email} (all write-access-guarded), backed by model_user_access.

Both registered in FileService; storage/util layer reused unchanged.

Shared access + naming layer (touches dataset code) - Rather than duplicating the dataset rules for models, the shared rules now have one implementation. jOOQ generates an unrelated class per table with no common supertype, so the shared code is parameterized by a descriptor naming the relevant columns instead of by inheritance:

Unified file document - LakeFSFileDocument now takes a ResourceType and resolves its own presign endpoint, so ModelFileDocument and DatasetFileDocument are both deleted — one document class serves every resource type.

Stacked on #6860 (#6497); its commits show in this diff until that merges.

Any related issues, documentation, discussions?

Part of #6498, umbrella #6494.

How was this PR tested?

New tests: ModelResourceSpec (22 — create/get/list/delete/update/toggle, incl. access
filtering, duplicate-name and forbidden paths, LakeFS-404 → 404) and ModelAccessResourceSpec
(30 — privilege helpers, grant/list/revoke, placeholder-account and unknown-email rejection).
DocumentFactorySpec now asserts the resource type per URI scheme so scheme routing stays
covered, and DatasetFileDocumentSpec became LakeFSFileDocumentSpec with per-resource-type
presign-endpoint tests.

Because the refactors touch dataset code, the existing dataset suites are the regression guard
and were run in full:

sbt "FileService/test" — 283/283 (incl. upstream DatasetResourceSpec, DatasetAccessResourceSpec)
sbt "WorkflowCore/test" — 761/761
scalafmtAll + scalafixAll --check clean

Also verified manually in a running Texera: dataset create / rename (incl. renaming to the same
name) / duplicate rejection, sharing (grant READ→WRITE, access list, revoke), public visibility,
owner-only delete and downloadable toggle, and file upload + version creation.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 4.8)

… tests Review fixes on apache#5911: use dataset owner email in retrieveLatestDatasetVersion, replace brittle Option.get/.head with headOption, strip datasets prefix on selection-modal reopen, clarify FileResolver docs; add FileResolver/DatasetFileNode and frontend path-helper tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…rse tests

…l-path-prefix

The datasets logical-path prefix strips four leading segments (datasets/owner/dataset/version); update the cover-image test's input path to include the prefix so the extracted relative path is the file name, not an empty string. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Now that models are coming as a separate resource/table, the leading resource-type segment is what selects the backing table, so an unprefixed path can no longer be routed unambiguously. Make the "datasets" prefix required instead of a tolerated fallback, and migrate existing data. - FileResolver: a dataset path must start with the "datasets" segment; unprefixed paths are no longer treated as dataset paths. - pytexera DatasetFileDocument: same rule, mirroring the backend. - FileListerSourceOpExec: parse the now-prefixed datasetVersionPath (skip the "datasets" segment); extracted into a testable helper. - Migration (sql/updates/29.sql): prepend "datasets/" to legacy paths stored in workflow.content and workflow_version.content, covering both the fileName (scan sources) and datasetVersionPath (file lister) operator properties. Only values whose first two segments match an existing (user.email, dataset.name) are rewritten, so local paths and URLs are left untouched; email format is irrelevant (owner may be a username without "@"). Uses create_missing=false and is idempotent. - Example workflows: use datasets-prefixed paths. Tests: FileResolverSpec and WorkflowExecutionsResourceSpec updated; test_dataset_file_document.py updated; new FileListerSourceOpExecSpec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…l-path-prefix # Conflicts: # frontend/src/app/common/type/datasetVersionFileTree.spec.ts

…format python test - dataset-selection-modal.component.spec.ts: expect the datasets-prefixed selectedPath in version (non-file) mode, matching the emitted path. - test_dataset_file_document.py: apply ruff format (wrap an over-length line) so `ruff format --check` passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The cover-image endpoints resolve a dataset path built as {owner}/{name}/{coverImage}. With the prefix now required by FileResolver, these must carry the datasets/ segment; add it in the set-cover, get-cover redirect, and cover-url handlers so cover images resolve again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…l-path-prefix

…storage # Conflicts: # sql/changelog.xml

…l-path-prefix

…storage

…rage # Conflicts: # common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/DocumentFactorySpec.scala

…nt-api

…var rename The rename to FILE_SERVICE_GET_DATASET_PRESIGNED_URL_ENDPOINT pushed a few lines past black's 88-col limit; reformat to satisfy the pyamber black --check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…at/model-management-api

…nt-api # Conflicts: # common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/DocumentFactory.scala # common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/model/LakeFSFileDocument.scala # common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/DocumentFactorySpec.scala # common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/model/LakeFSFileDocumentSpec.scala # sql/changelog.xml # sql/texera_ddl.sql

aicam

LGTM! please address the comments. Also please keep two notes on this PR and add in the description:
1- We will most likely change the prefix from datasets to dataset
2- We also need to discuss if we should go with one table, I see lots of duplicate code
Please keep these two in mind to discuss further in the discussion.

tanishqgandhi1908 · 2026-08-17T21:11:12Z

/request-review @mengw15

Copilot

Pull request overview

Adds model metadata and access-management APIs while generalizing dataset/model resource handling.

Changes:

Adds model access-control schema and REST resources.
Shares naming and authorization logic across datasets and models.
Unifies LakeFS document routing with expanded tests.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`sql/updates/38.sql`	Adds model access migration.
`sql/texera_ddl.sql`	Adds model access table to base schema.
`sql/changelog.xml`	Registers changeSet 38.
`file-service/.../ModelResourceSpec.scala`	Tests model management APIs.
`file-service/.../ModelAccessResourceSpec.scala`	Tests model authorization and sharing.
`file-service/.../DatasetAccessResourceSpec.scala`	Updates dataset access tests.
`file-service/.../ResourceNaming.scala`	Centralizes resource naming rules.
`file-service/.../ResourceAccess.scala`	Centralizes resource authorization.
`file-service/.../ModelResource.scala`	Implements model metadata APIs.
`file-service/.../ModelAccessResource.scala`	Implements model sharing APIs.
`file-service/.../ManagedResource.scala`	Describes shared resource tables.
`file-service/.../DatasetResource.scala`	Adopts shared resource helpers.
`file-service/.../DatasetAccessResource.scala`	Adopts shared access logic.
`file-service/.../FileService.scala`	Registers model resources.
`common/.../LakeFSFileDocumentSpec.scala`	Tests unified LakeFS documents.
`common/.../DocumentFactorySpec.scala`	Tests resource-specific routing.
`common/.../ModelFileDocument.scala`	Removes model-specific wrapper.
`common/.../LakeFSFileDocument.scala`	Adds resource-aware endpoint selection.
`common/.../DatasetFileDocument.scala`	Removes dataset-specific wrapper.
`common/.../DocumentFactory.scala`	Creates unified LakeFS documents.

Suppressed comments (5)

file-service/src/main/scala/org/apache/texera/service/resource/ResourceAccess.scala:197

Ownership alone is not included in this query, although userHasReadAccess treats owners as authorized. Because any WRITE grantee can revoke the owner's explicit row, an owner can then lose the resource from /list while still being able to fetch it directly. Join only the caller's grant and select rows where the caller is either owner or grantee; report WRITE for owners.

      .where(resource.accessUidField.eq(uid))

file-service/src/main/scala/org/apache/texera/service/resource/ResourceAccess.scala:147

This read guard lets any authenticated user enumerate every grantee's name and email for a public model, because public resources satisfy userHasReadAccess. The PR contract says these access-management endpoints are write-guarded; require write access before returning the sharing list.

    requireReadAccess(ctx, resource, id, requesterUid)

file-service/src/main/scala/org/apache/texera/service/resource/ResourceAccess.scala:251

The path parameter is converted without validation. Unknown values throw IllegalArgumentException as a 500, while the valid enum value NONE creates a row that listVisible treats as accessible even though read checks reject it. Accept only READ or WRITE and return a 400 for everything else.

    val granted = PrivilegeEnum.valueOf(privilege)

file-service/src/main/scala/org/apache/texera/service/resource/ResourceAccess.scala:275

Revoking an unknown email dereferences null and returns a 500. Since this method documents revocation of a missing grant as a no-op, an email with no account should also return success rather than crashing.

    val granteeUid = new UserDao(ctx.configuration()).fetchOneByEmail(email).getUid

file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala:223

The LakeFS/S3 deletions happen before the database row is deleted or the transaction commits. If the S3 call, database delete, or commit fails after deleteRepo, the transaction preserves a model row whose repository is already gone; retries then stop on LakeFS 404, leaving the model undeletable. Use a persisted deleting state/outbox, or otherwise make cleanup retryable before finalizing the database deletion.

      withLakeFSErrorHandling(s"deleting the repository of model '${model.getName}'") {
        LakeFSStorageClient.deleteRepo(model.getRepositoryName)
      }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+CREATE TABLE IF NOT EXISTS model_user_access
+(
+    mid       INT NOT NULL,
+    uid       INT NOT NULL,
+    privilege privilege_enum NOT NULL DEFAULT 'NONE',
+    PRIMARY KEY (mid, uid),
+    FOREIGN KEY (mid) REFERENCES model(mid) ON DELETE CASCADE,
+    FOREIGN KEY (uid) REFERENCES "user"(uid) ON DELETE CASCADE
+);


+  def presignEndpointOf(resourceType: ResourceType.Value): String =
+    resourceType match {
+      case ResourceType.Datasets => datasetPresignEndpoint
+      case ResourceType.Models   => modelPresignEndpoint


+        withLakeFSErrorHandling(s"creating the repository of model '${model.getName}'") {
+          LakeFSStorageClient.initRepo(repositoryName)


+      .orNull
+  }
+
+  /** The owner's email, or an empty string when the resource does not exist. */


mengw15 · 2026-08-17T23:40:38Z

Can we split this pr? Since the change seems a lot

chenlica · 2026-08-18T05:53:01Z

@tanishqgandhi1908 I also think this PR is a bit too big. Can you split it? Also try to provide a more human-readable description at the beginning of the PR.

tanishqgandhi1908 · 2026-08-18T13:41:26Z

@tanishqgandhi1908 I also think this PR is a bit too big. Can you split it? Also try to provide a more human-readable description at the beginning of the PR.

Yes, I am working on splitting it such that both PR's are independent. Will update the description too accordingly

aicam and others added 22 commits July 1, 2026 13:57

feat: datasets prefix

0d80726

test+fix: extend datasets prefix to python UDF path + add frontend pa…

1431887

…rse tests

refactor: remove unused frontend path parser; reword prefix docs

1d9cbd1

Merge remote-tracking branch 'upstream/main' into feat/dataset-logica…

e4df21c

…l-path-prefix

Merge remote-tracking branch 'upstream/main' into feat/dataset-logica…

1ef2634

…l-path-prefix # Conflicts: # frontend/src/app/common/type/datasetVersionFileTree.spec.ts

consolidate the datasets literal

66a5aed

Merge remote-tracking branch 'upstream/main' into feat/dataset-logica…

8eccc88

…l-path-prefix

fixing test cases

e11e008

feat(storage): add model metadata tables

de806ba

Merge branch 'feat/dataset-logical-path-prefix' into feat/model-file-…

581ef1f

…storage # Conflicts: # sql/changelog.xml

addressing the PR comments

db4579a

Merge remote-tracking branch 'upstream/main' into feat/dataset-logica…

f3a3ad1

…l-path-prefix

Merge branch 'feat/dataset-logical-path-prefix' into feat/model-file-…

ed0914b

…storage

feat(storage): add model file storage and path resolution

144ee2f

Merge remote-tracking branch 'upstream/main' into feat/model-file-sto…

953d6fb

…rage # Conflicts: # common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/DocumentFactorySpec.scala

feat(storage): add model file storage and path resolution

aebfdb9

feat(file-service): add model management API (metadata + access control)

e3347c1

github-actions Bot assigned tanishqgandhi1908 Jul 24, 2026

github-actions Bot added feature engine ddl-change Changes to the TexeraDB DDL pyamber frontend Changes related to the frontend GUI infra common labels Jul 24, 2026

tanishqgandhi1908 added 3 commits August 13, 2026 13:29

Merge remote-tracking branch 'upstream/main' into feat/model-manageme…

f19ef65

…nt-api

addressing the review comments on presign-url

3354a09

refactoring the common code used by models and datasets

79e0588

github-actions Bot removed engine pyamber frontend Changes related to the frontend GUI infra labels Aug 13, 2026

tanishqgandhi1908 added 2 commits August 13, 2026 15:48

Merge branch 'feat/model-file-storage' into feat/model-management-api

fe0dd6b

refactoring the common code used by models and datasets

93eb784

github-actions Bot added pyamber infra labels Aug 13, 2026

tanishqgandhi1908 changed the title ~~feat(file-service): add model management API (metadata + access control)~~ feat(file-service): add model management API and unify shared resource access Aug 13, 2026

tanishqgandhi1908 marked this pull request as ready for review August 13, 2026 23:49

tanishqgandhi1908 and others added 4 commits August 13, 2026 16:58

renaming the .env file with presigned url

b4f5b2a

Merge remote-tracking branch 'origin/feat/model-file-storage' into fe…

db3c133

…at/model-management-api

github-actions Bot removed pyamber infra labels Aug 14, 2026

aicam approved these changes Aug 17, 2026

View reviewed changes

addressing PR review comments

6ac0cbc

github-actions Bot requested a review from mengw15 August 17, 2026 21:11

mengw15 requested a balanced review from Copilot August 17, 2026 21:15

Copilot started reviewing on behalf of mengw15 August 17, 2026 21:16 View session

Copilot AI reviewed Aug 17, 2026

View reviewed changes

addressing PR review comments

6ed828b

Navigation Menu

Uh oh!

feat(file-service): add model management API and unify shared resource access - #6869

feat(file-service): add model management API and unify shared resource access#6869
tanishqgandhi1908 wants to merge 50 commits into
apache:mainfrom
tanishqgandhi1908:feat/model-management-api

tanishqgandhi1908 commented Jul 24, 2026 •

edited

Loading

Uh oh!

aicam left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tanishqgandhi1908 commented Aug 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mengw15 commented Aug 17, 2026

Uh oh!

chenlica commented Aug 18, 2026

Uh oh!

tanishqgandhi1908 commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		withLakeFSErrorHandling(s"creating the repository of model '${model.getName}'") {
		LakeFSStorageClient.initRepo(repositoryName)

Uh oh!

Conversation

tanishqgandhi1908 commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this PR?

Any related issues, documentation, discussions?

How was this PR tested?

Was this PR authored or co-authored using generative AI tooling?

Uh oh!

aicam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tanishqgandhi1908 commented Aug 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mengw15 commented Aug 17, 2026

Uh oh!

chenlica commented Aug 18, 2026

Uh oh!

tanishqgandhi1908 commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tanishqgandhi1908 commented Jul 24, 2026 •

edited

Loading