From 3571dd12b542a307f0bdd1a4a5db4635b48ca03b Mon Sep 17 00:00:00 2001 From: Jinghao-coding Date: Tue, 3 Nov 2026 23:07:46 +0800 Subject: [PATCH] fix(download): link completed downloads by resource ID Resolve the destination from storage path and resource type so same-name resources cannot redirect download details to another card. Closes #558 Signed-off-by: Jinghao-coding --- backend/docs/docs.go | 6 +- backend/docs/swagger.json | 6 +- backend/docs/swagger.yaml | 5 +- backend/internal/handler/modeldownload.go | 30 ++- .../handler/modeldownload_resource_test.go | 216 ++++++++++++++++++ .../model/model-download-detail.tsx | 20 +- frontend/src/services/api/modeldownload.ts | 1 + 7 files changed, 263 insertions(+), 21 deletions(-) create mode 100644 backend/internal/handler/modeldownload_resource_test.go diff --git a/backend/docs/docs.go b/backend/docs/docs.go index 03184e52f..ae8f61c3c 100644 --- a/backend/docs/docs.go +++ b/backend/docs/docs.go @@ -6904,7 +6904,7 @@ const docTemplate = `{ "Bearer": [] } ], - "description": "根据 ID 获取模型下载任务详情", + "description": "根据 ID 获取模型下载任务详情，已完成任务包含实际关联资源的 resourceId（资源存在时）", "consumes": [ "application/json" ], @@ -13328,6 +13328,10 @@ const docTemplate = `{ "$ref": "#/definitions/github_com_raids-lab_crater_dao_model.UserInfo" } }, + "resourceId": { + "description": "Linked resource, when available in a Ready download detail.", + "type": "integer" + }, "revision": { "type": "string" }, diff --git a/backend/docs/swagger.json b/backend/docs/swagger.json index 8c0359bd5..0f5723e32 100644 --- a/backend/docs/swagger.json +++ b/backend/docs/swagger.json @@ -6896,7 +6896,7 @@ "Bearer": [] } ], - "description": "根据 ID 获取模型下载任务详情", + "description": "根据 ID 获取模型下载任务详情，已完成任务包含实际关联资源的 resourceId（资源存在时）", "consumes": [ "application/json" ], @@ -13320,6 +13320,10 @@ "$ref": "#/definitions/github_com_raids-lab_crater_dao_model.UserInfo" } }, + "resourceId": { + "description": "Linked resource, when available in a Ready download detail.", + "type": "integer" + }, "revision": { "type": "string" }, diff --git a/backend/docs/swagger.yaml b/backend/docs/swagger.yaml index c3155996e..562a0ec9f 100644 --- a/backend/docs/swagger.yaml +++ b/backend/docs/swagger.yaml @@ -1815,6 +1815,9 @@ definitions: items: $ref: '#/definitions/github_com_raids-lab_crater_dao_model.UserInfo' type: array + resourceId: + description: Linked resource, when available in a Ready download detail. + type: integer revision: type: string sizeBytes: @@ -7468,7 +7471,7 @@ paths: get: consumes: - application/json - description: 根据 ID 获取模型下载任务详情 + description: 根据 ID 获取模型下载任务详情，已完成任务包含实际关联资源的 resourceId（资源存在时） parameters: - description: 下载任务ID in: path diff --git a/backend/internal/handler/modeldownload.go b/backend/internal/handler/modeldownload.go index df90564b7..058719bcd 100644 --- a/backend/internal/handler/modeldownload.go +++ b/backend/internal/handler/modeldownload.go @@ -116,6 +116,7 @@ type DownloadActionReq struct { type ModelDownloadResp struct { ID uint `json:"id"` + ResourceID *uint `json:"resourceId,omitempty"` // Linked resource, when available in a Ready download detail. Name string `json:"name"` Source string `json:"source"` Category string `json:"category"` @@ -785,7 +786,7 @@ func (mgr *ModelDownloadMgr) downloadStatusSummary(c *gin.Context, category stri // GetDownload godoc // // @Summary 获取单个模型下载任务详情 -// @Description 根据 ID 获取模型下载任务详情 +// @Description 根据 ID 获取模型下载任务详情，已完成任务包含实际关联资源的 resourceId（资源存在时） // @Tags ModelDownload // @Accept json // @Produce json @@ -821,10 +822,37 @@ func (mgr *ModelDownloadMgr) GetDownload(c *gin.Context) { return } resp = responses[0] + resp.ResourceID, err = mgr.findDownloadResourceID(c, query.Q, download) + if err != nil { + resputil.HandleError(c, bizerr.Internal.DatabaseError.Wrap(err, "get linked download resource failed")) + return + } resputil.Success(c, resp) } +// findDownloadResourceID uses the same storage-path/type identity as resource +// publication. Resolving it on read also covers existing downloads and avoids +// returning a stale link after the resource is deleted or renamed. +func (mgr *ModelDownloadMgr) findDownloadResourceID( + ctx context.Context, db *query.Query, download *model.ModelDownload, +) (*uint, error) { + if download.Status != model.ModelDownloadStatusReady || download.Path == "" { + return nil, nil + } + q := db.Dataset + linkedDataset, err := q.WithContext(ctx).Select(q.ID). + Where(q.URL.Eq(mgr.convertToPhysicalPath(download.Path)), q.Type.Eq(string(download.Category))). + First() + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, nil + } + if err != nil { + return nil, err + } + return &linkedDataset.ID, nil +} + // requireManagePermission allows only the creator or a platform administrator // to mutate a download task. func (mgr *ModelDownloadMgr) requireManagePermission(c *gin.Context, downloadID uint) (*model.ModelDownload, error) { diff --git a/backend/internal/handler/modeldownload_resource_test.go b/backend/internal/handler/modeldownload_resource_test.go new file mode 100644 index 000000000..52c1fc389 --- /dev/null +++ b/backend/internal/handler/modeldownload_resource_test.go @@ -0,0 +1,216 @@ +// Copyright 2026 The Crater Project Team, RAIDS-Lab +// +// Licensed 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. + +package handler + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/gin-gonic/gin" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + + "github.com/raids-lab/crater/dao/model" + "github.com/raids-lab/crater/dao/query" + "github.com/raids-lab/crater/internal/util" +) + +const ( + testDownloadResourceName = "owner/repository" + testDownloadResourcePath = "public/Models/owner/repository" + testUnrelatedResourcePath = "user/alice/repository" +) + +func newDownloadResourceTestDB(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open(filepath.Join(t.TempDir(), "resources.db")), &gorm.Config{ + DisableForeignKeyConstraintWhenMigrating: true, + IgnoreRelationshipsWhenMigrating: true, + }) + if err != nil { + t.Fatal(err) + } + sqlDB, err := db.DB() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := sqlDB.Close(); err != nil { + t.Error(err) + } + }) + if err := db.AutoMigrate(&model.Dataset{}, &model.ModelDownload{}, &model.User{}, &model.UserModelDownload{}); err != nil { + t.Fatal(err) + } + return db +} + +func TestDownloadResourceUsesStorageIdentityInsteadOfName(t *testing.T) { + for _, category := range []model.DownloadCategory{model.DownloadCategoryModel, model.DownloadCategoryDataset} { + t.Run(string(category), func(t *testing.T) { + t.Parallel() + db := newDownloadResourceTestDB(t) + mgr := &ModelDownloadMgr{} + download := model.ModelDownload{ + Name: testDownloadResourceName, Category: category, Status: model.ModelDownloadStatusReady, + Path: modelDownloadStoragePath(modelDownloadBasePath(category), testDownloadResourceName), + } + physicalPath := mgr.convertToPhysicalPath(download.Path) + otherType := model.DataTypeModel + if category == model.DownloadCategoryModel { + otherType = model.DataTypeDataset + } + resources := []model.Dataset{ + {Name: download.Name, URL: testUnrelatedResourcePath, Type: model.DataType(category)}, + {Name: download.Name, URL: physicalPath, Type: otherType}, + {Name: "renamed resource", URL: physicalPath, Type: model.DataType(category)}, + } + if err := db.Create(&resources).Error; err != nil { + t.Fatal(err) + } + id, err := mgr.findDownloadResourceID(t.Context(), query.Use(db), &download) + if err != nil { + t.Fatal(err) + } + if id == nil || *id != resources[2].ID { + t.Fatalf("linked resource = %v, want renamed resource ID %d", id, resources[2].ID) + } + }) + } +} + +func TestDownloadResourceOmitsUnavailableResource(t *testing.T) { + for _, tc := range []struct { + name string + status model.ModelDownloadStatus + path string + }{ + {name: "pending", status: model.ModelDownloadStatusPending, path: testDownloadResourcePath}, + {name: "downloading", status: model.ModelDownloadStatusDownloading, path: testDownloadResourcePath}, + {name: "paused", status: model.ModelDownloadStatusPaused, path: testDownloadResourcePath}, + {name: "failed", status: model.ModelDownloadStatusFailed, path: testDownloadResourcePath}, + {name: "missing path", status: model.ModelDownloadStatusReady}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + db := newDownloadResourceTestDB(t) + mgr := &ModelDownloadMgr{} + resource := model.Dataset{Name: testDownloadResourceName, URL: mgr.convertToPhysicalPath(tc.path), Type: model.DataTypeModel} + if err := db.Create(&resource).Error; err != nil { + t.Fatal(err) + } + download := model.ModelDownload{ + Name: resource.Name, Category: model.DownloadCategoryModel, Status: tc.status, Path: tc.path, + } + id, err := mgr.findDownloadResourceID(t.Context(), query.Use(db), &download) + if err != nil || id != nil { + t.Fatalf("unavailable resource = %v, error = %v, want no link", id, err) + } + }) + } +} + +func TestDownloadResourceDoesNotFallbackToSameNameAfterDeletion(t *testing.T) { + db := newDownloadResourceTestDB(t) + mgr := &ModelDownloadMgr{} + download := model.ModelDownload{ + Name: testDownloadResourceName, Category: model.DownloadCategoryDataset, + Status: model.ModelDownloadStatusReady, Path: "public/Datasets/owner/repository/huggingface/old-revision", + } + resources := []model.Dataset{ + {Name: download.Name, URL: testUnrelatedResourcePath, Type: model.DataTypeDataset}, + {Name: download.Name, URL: mgr.convertToPhysicalPath(download.Path), Type: model.DataTypeDataset}, + } + if err := db.Create(&resources).Error; err != nil { + t.Fatal(err) + } + id, err := mgr.findDownloadResourceID(t.Context(), query.Use(db), &download) + if err != nil || id == nil || *id != resources[1].ID { + t.Fatalf("historical download resource = %v, error = %v, want ID %d", id, err, resources[1].ID) + } + if err := db.Delete(&resources[1]).Error; err != nil { + t.Fatal(err) + } + id, err = mgr.findDownloadResourceID(t.Context(), query.Use(db), &download) + if err != nil || id != nil { + t.Fatalf("deleted resource = %v, error = %v, want no link", id, err) + } +} + +func TestGetDownloadReturnsResourceIDAndOmitsDeletedResource(t *testing.T) { + db := newDownloadResourceTestDB(t) + // This handler uses the application's default query. Keep this test serial + // and restore it before closing its database so other tests stay isolated. + previous := *query.Q + query.SetDefault(db) + t.Cleanup(func() { *query.Q = previous }) + + mgr := &ModelDownloadMgr{} + download := model.ModelDownload{ + Name: testDownloadResourceName, Category: model.DownloadCategoryModel, + Status: model.ModelDownloadStatusReady, Path: testDownloadResourcePath, CreatorID: 7, + } + if err := db.Create(&download).Error; err != nil { + t.Fatal(err) + } + resources := []model.Dataset{ + {Name: download.Name, URL: testUnrelatedResourcePath, Type: model.DataTypeModel}, + {Name: download.Name, URL: mgr.convertToPhysicalPath(download.Path), Type: model.DataTypeModel}, + } + if err := db.Create(&resources).Error; err != nil { + t.Fatal(err) + } + data := requestDownloadResourceDetail(t, mgr, download.ID) + var resourceID uint + if err := json.Unmarshal(data["resourceId"], &resourceID); err != nil { + t.Fatal(err) + } + if resourceID != resources[1].ID { + t.Fatalf("detail resourceId = %d, want %d", resourceID, resources[1].ID) + } + if err := db.Delete(&resources[1]).Error; err != nil { + t.Fatal(err) + } + data = requestDownloadResourceDetail(t, mgr, download.ID) + if value, ok := data["resourceId"]; ok { + t.Fatalf("detail returned resourceId %s for a deleted resource", value) + } +} + +func requestDownloadResourceDetail(t *testing.T, mgr *ModelDownloadMgr, id uint) map[string]json.RawMessage { + t.Helper() + router := gin.New() + router.Use(func(c *gin.Context) { + util.SetJWTContext(c, util.JWTMessage{UserID: 7, Username: "alice", RolePlatform: model.RoleUser}) + c.Next() + }) + mgr.RegisterProtected(router.Group("/v1/model-download")) + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, fmt.Sprintf("/v1/model-download/models/downloads/%d", id), http.NoBody)) + if recorder.Code != http.StatusOK { + t.Fatalf("GET download returned HTTP %d: %s", recorder.Code, recorder.Body.String()) + } + var response struct { + Data map[string]json.RawMessage `json:"data"` + } + if err := json.Unmarshal(recorder.Body.Bytes(), &response); err != nil { + t.Fatal(err) + } + return response.Data +} diff --git a/frontend/src/components/model/model-download-detail.tsx b/frontend/src/components/model/model-download-detail.tsx index a0a9b0afd..b64e7e717 100644 --- a/frontend/src/components/model/model-download-detail.tsx +++ b/frontend/src/components/model/model-download-detail.tsx @@ -50,7 +50,6 @@ import ModelDownloadProgress from '@/components/model/model-download-progress' import ModelDownloadTokenDialog from '@/components/model/model-download-token-dialog' import RepositorySourceMark from '@/components/model/repository-source-mark' -import { apiGetDataset } from '@/services/api/dataset' import { apiGetModelDownload, apiGetModelDownloadLogs, @@ -84,19 +83,6 @@ export function ModelDownloadDetail({ ...props }: DetailPageCoreProps) { }, }) - const { data: resources = [] } = useQuery({ - queryKey: ['data', download?.category || 'model'], - queryFn: () => apiGetDataset(), - select: (res) => res.data, - enabled: download?.status === 'Ready', - }) - - const linkedResource = resources.find( - (resource) => - resource.type === download?.category && - resource.name.toLowerCase() === download?.name.toLowerCase() - ) - const refetchDownload = async () => { try { await queryClient.invalidateQueries({ queryKey: ['model-downloads', id] }) @@ -143,8 +129,8 @@ export function ModelDownloadDetail({ ...props }: DetailPageCoreProps) { } const navigateToResourceCard = () => { - if (!linkedResource || !download) return - const params = { id: String(linkedResource.id) } + if (!download?.resourceId) return + const params = { id: String(download.resourceId) } if (download.category === 'dataset') { navigate({ to: '/portal/data/datasets/$id', params, search: { tab: '' } }) return @@ -182,7 +168,7 @@ export function ModelDownloadDetail({ ...props }: DetailPageCoreProps) { } >
- {download.status === 'Ready' && linkedResource && ( + {download.status === 'Ready' && Boolean(download.resourceId) && (