From bc7fa409953ed786294ac2fbc4f64a5b7a4289c9 Mon Sep 17 00:00:00 2001 From: yanweidong Date: Thu, 23 Jul 2026 22:39:12 +0800 Subject: [PATCH] refactor: simplify dataset source ownership --- backend/internal/initdb/dataset.go | 74 +++++++++++++++++-- backend/internal/initdb/dataset_test.go | 39 ++++++---- backend/internal/logic/dataset/feed_test.go | 20 ++--- .../internal/logic/dataset/handlers_test.go | 12 ++- backend/internal/logic/dataset/service.go | 2 +- backend/internal/models/dataset_source.go | 30 ++++---- backend/internal/models/identity.go | 11 +-- backend/internal/models/identity_test.go | 2 - 8 files changed, 123 insertions(+), 67 deletions(-) diff --git a/backend/internal/initdb/dataset.go b/backend/internal/initdb/dataset.go index e5c8de2..6305528 100644 --- a/backend/internal/initdb/dataset.go +++ b/backend/internal/initdb/dataset.go @@ -3,6 +3,7 @@ package initdb import ( "errors" "fmt" + "strings" "gorm.io/gorm" "senlinai-agent/backend/internal/models" @@ -59,28 +60,87 @@ func InitDataset(database *gorm.DB) error { } func prepareDatasetSourceOwners(tx *gorm.DB) error { + hasCreatedBy, err := hasDatasetSourceColumn(tx, "created_by") + if err != nil { + return err + } + if hasCreatedBy { + if err := tx.Exec( + "UPDATE sa_dataset_sources SET owner_id = created_by WHERE owner_id = 0", + ).Error; err != nil { + return fmt.Errorf("backfill dataset source owners: %w", err) + } + } + var sources []models.SaDatasetSource if err := tx.Where("owner_id = ? OR owner_identity = ?", 0, "").Find(&sources).Error; err != nil { return err } for _, source := range sources { - ownerID := source.OwnerID - if ownerID == 0 { - ownerID = source.CreatedBy - } var owner models.SaUser - if err := tx.Select("identity").First(&owner, ownerID).Error; err != nil { + if err := tx.Select("identity").First(&owner, source.OwnerID).Error; err != nil { return err } if err := tx.Model(&source).Updates(map[string]any{ - "owner_id": ownerID, "owner_identity": owner.Identity, + "owner_identity": owner.Identity, }).Error; err != nil { return err } } + + if tx.Migrator().HasIndex(&models.SaDatasetSource{}, "idx_sa_dataset_source_seed") { + if err := tx.Migrator().DropIndex(&models.SaDatasetSource{}, "idx_sa_dataset_source_seed"); err != nil { + return fmt.Errorf("drop legacy dataset source seed index: %w", err) + } + } + for _, index := range []string{ + "idx_sa_dataset_sources_created_by", + "idx_sa_dataset_sources_created_by_identity", + } { + if tx.Migrator().HasIndex(&models.SaDatasetSource{}, index) { + if err := tx.Migrator().DropIndex(&models.SaDatasetSource{}, index); err != nil { + return fmt.Errorf("drop legacy dataset source creator index %s: %w", index, err) + } + } + } + hasCreatedByIdentity, err := hasDatasetSourceColumn(tx, "created_by_identity") + if err != nil { + return err + } + if hasCreatedByIdentity { + if err := tx.Exec( + "ALTER TABLE sa_dataset_sources DROP COLUMN created_by_identity", + ).Error; err != nil { + return fmt.Errorf("drop legacy dataset source creator identity: %w", err) + } + } + hasCreatedBy, err = hasDatasetSourceColumn(tx, "created_by") + if err != nil { + return err + } + if hasCreatedBy { + if err := tx.Exec( + "ALTER TABLE sa_dataset_sources DROP COLUMN created_by", + ).Error; err != nil { + return fmt.Errorf("drop legacy dataset source creator: %w", err) + } + } return nil } +func hasDatasetSourceColumn(tx *gorm.DB, name string) (bool, error) { + columns, err := tx.Migrator().ColumnTypes(&models.SaDatasetSource{}) + if err != nil { + return false, fmt.Errorf("inspect dataset source columns: %w", err) + } + for _, column := range columns { + if strings.EqualFold(column.Name(), name) { + return true, nil + } + } + return false, nil +} + func prepareDatasetItems(tx *gorm.DB) error { if tx.Migrator().HasIndex(&models.SaDatasetItem{}, "idx_sa_dataset_item_source_external") { return nil @@ -135,7 +195,7 @@ func upsertDefaultDatasetSource(tx *gorm.DB, root models.SaUser, definition defa seedKey := definition.Key return tx.Create(&models.SaDatasetSource{ - OwnerID: root.ID, CreatedBy: root.ID, SeedKey: &seedKey, Name: definition.Name, + OwnerID: root.ID, SeedKey: &seedKey, Name: definition.Name, Kind: "rss", URL: definition.URL, IconURL: definition.IconURL, Description: "默认资讯订阅", Enabled: true, }).Error diff --git a/backend/internal/initdb/dataset_test.go b/backend/internal/initdb/dataset_test.go index 75e602d..d26808e 100644 --- a/backend/internal/initdb/dataset_test.go +++ b/backend/internal/initdb/dataset_test.go @@ -33,8 +33,6 @@ func TestInitDatasetCreatesDefaultSourcesWhenTableIsEmpty(t *testing.T) { require.True(t, sources[index].Enabled) require.Equal(t, root.ID, sources[index].OwnerID) require.Equal(t, root.Identity, sources[index].OwnerIdentity) - require.Equal(t, root.ID, sources[index].CreatedBy) - require.Equal(t, root.Identity, sources[index].CreatedByIdentity) } } @@ -44,10 +42,10 @@ func TestInitDatasetAddsMissingDefaultsWithoutChangingCustomSources(t *testing.T var root models.SaUser require.NoError(t, database.Where("email = ?", rootUsername).First(&root).Error) existing := models.SaDatasetSource{ - CreatedBy: root.ID, - Name: "Existing source", - Kind: "manual", - Enabled: true, + OwnerID: root.ID, + Name: "Existing source", + Kind: "manual", + Enabled: true, } require.NoError(t, database.Create(&existing).Error) @@ -70,20 +68,33 @@ func TestInitDatasetBackfillsLegacySourceOwners(t *testing.T) { PasswordHash: "hash", Role: "user", } require.NoError(t, database.Create(&other).Error) + require.NoError(t, database.Exec( + "ALTER TABLE sa_dataset_sources ADD COLUMN created_by integer NOT NULL DEFAULT 0", + ).Error) + require.NoError(t, database.Exec( + "ALTER TABLE sa_dataset_sources ADD COLUMN created_by_identity text", + ).Error) source := models.SaDatasetSource{ - CreatedBy: other.ID, Name: "Existing source", Kind: "manual", Enabled: true, + OwnerID: other.ID, Name: "Existing source", Kind: "manual", Enabled: true, } require.NoError(t, database.Create(&source).Error) - require.NoError(t, database.Model(&source).Updates(map[string]any{ - "owner_id": 0, "owner_identity": "", - }).Error) + require.NoError(t, database.Exec(` + UPDATE sa_dataset_sources + SET owner_id = 0, owner_identity = '', created_by = ?, created_by_identity = ? + WHERE id = ? + `, other.ID, other.Identity, source.ID).Error) require.NoError(t, InitDataset(database)) require.NoError(t, database.First(&source, source.ID).Error) require.Equal(t, other.ID, source.OwnerID) require.Equal(t, other.Identity, source.OwnerIdentity) - require.Equal(t, other.ID, source.CreatedBy) + hasCreatedBy, err := hasDatasetSourceColumn(database, "created_by") + require.NoError(t, err) + require.False(t, hasCreatedBy) + hasCreatedByIdentity, err := hasDatasetSourceColumn(database, "created_by_identity") + require.NoError(t, err) + require.False(t, hasCreatedByIdentity) } func TestInitDatasetRejectsMissingRootInsteadOfAssigningDefaultsToAnotherUser(t *testing.T) { @@ -115,11 +126,11 @@ func TestInitDatasetRepairsLegacyRSSHubURLsForRootOnly(t *testing.T) { require.NoError(t, database.Create(&other).Error) rootLegacy := models.SaDatasetSource{ - CreatedBy: root.ID, Name: "财联社-热门", Kind: "rss", + OwnerID: root.ID, Name: "财联社-热门", Kind: "rss", URL: "https://rsshub.app/cls/hot", Enabled: true, } otherLegacy := models.SaDatasetSource{ - CreatedBy: other.ID, Name: "财联社-热门", Kind: "rss", + OwnerID: other.ID, Name: "财联社-热门", Kind: "rss", URL: "https://rsshub.app/cls/hot", Enabled: true, } require.NoError(t, database.Create(&rootLegacy).Error) @@ -142,7 +153,7 @@ func TestInitDatasetPreparesCollectedItemUniqueness(t *testing.T) { var root models.SaUser require.NoError(t, database.Where("email = ?", rootUsername).First(&root).Error) source := models.SaDatasetSource{ - CreatedBy: root.ID, Name: "Existing feed", Kind: "rss", + OwnerID: root.ID, Name: "Existing feed", Kind: "rss", URL: "https://example.com/feed", Enabled: true, } require.NoError(t, database.Create(&source).Error) diff --git a/backend/internal/logic/dataset/feed_test.go b/backend/internal/logic/dataset/feed_test.go index 618f25f..2d175fb 100644 --- a/backend/internal/logic/dataset/feed_test.go +++ b/backend/internal/logic/dataset/feed_test.go @@ -128,11 +128,11 @@ func TestSyncSourcesRecordsFeedFailure(t *testing.T) { database := newDatasetTestDatabase(t) user := createDatasetTestUser(t, database, "feed-owner@example.com") source := models.SaDatasetSource{ - CreatedBy: user.ID, - Name: "Broken feed", - Kind: "rss", - URL: "https://example.com/feed", - Enabled: true, + OwnerID: user.ID, + Name: "Broken feed", + Kind: "rss", + URL: "https://example.com/feed", + Enabled: true, } require.NoError(t, database.Create(&source).Error) @@ -150,9 +150,9 @@ func TestSyncAllSourcesCollectsEnabledRSSSourcesForEveryUser(t *testing.T) { firstUser := createDatasetTestUser(t, database, "first-feed-owner@example.com") secondUser := createDatasetTestUser(t, database, "second-feed-owner@example.com") for _, source := range []models.SaDatasetSource{ - {CreatedBy: firstUser.ID, Name: "First RSS", Kind: "rss", URL: "https://example.com/first", Enabled: true}, - {CreatedBy: secondUser.ID, Name: "Second RSS", Kind: "rss", URL: "https://example.com/second", Enabled: true}, - {CreatedBy: firstUser.ID, Name: "Manual", Kind: "manual", Enabled: true}, + {OwnerID: firstUser.ID, Name: "First RSS", Kind: "rss", URL: "https://example.com/first", Enabled: true}, + {OwnerID: secondUser.ID, Name: "Second RSS", Kind: "rss", URL: "https://example.com/second", Enabled: true}, + {OwnerID: firstUser.ID, Name: "Manual", Kind: "manual", Enabled: true}, } { require.NoError(t, database.Create(&source).Error) } @@ -171,7 +171,7 @@ func TestSyncSourcesCollectsMultipleSourcesForOneOwner(t *testing.T) { user := createDatasetTestUser(t, database, "multi-feed-owner@example.com") for index := 0; index < 5; index++ { require.NoError(t, database.Create(&models.SaDatasetSource{ - CreatedBy: user.ID, Name: fmt.Sprintf("Feed %d", index), Kind: "rss", + OwnerID: user.ID, Name: fmt.Sprintf("Feed %d", index), Kind: "rss", URL: fmt.Sprintf("https://example.com/feed-%d", index), Enabled: true, }).Error) } @@ -189,7 +189,7 @@ func TestQueueSourcesRunsInBackgroundAndRejectsOverlap(t *testing.T) { database := newDatasetTestDatabase(t) user := createDatasetTestUser(t, database, "queued-feed-owner@example.com") source := models.SaDatasetSource{ - CreatedBy: user.ID, Name: "Queued RSS", Kind: "rss", + OwnerID: user.ID, Name: "Queued RSS", Kind: "rss", URL: "https://example.com/feed", Enabled: true, } require.NoError(t, database.Create(&source).Error) diff --git a/backend/internal/logic/dataset/handlers_test.go b/backend/internal/logic/dataset/handlers_test.go index 64bf088..3e46da1 100644 --- a/backend/internal/logic/dataset/handlers_test.go +++ b/backend/internal/logic/dataset/handlers_test.go @@ -49,8 +49,6 @@ func TestDatasetAPIConnectsSourcesItemsAndSyncTasks(t *testing.T) { require.NoError(t, database.Where("identity = ?", source.ID).First(&storedSource).Error) require.Equal(t, owner.ID, storedSource.OwnerID) require.Equal(t, owner.Identity, storedSource.OwnerIdentity) - require.Equal(t, owner.ID, storedSource.CreatedBy) - require.Equal(t, owner.Identity, storedSource.CreatedByIdentity) createdItem := performDatasetRequest(t, ownerRouter, http.MethodPost, "/api/v1/dataset-items", map[string]any{ "sourceId": source.ID, "title": "A collected article", "summary": "Summary", @@ -143,8 +141,8 @@ func TestDatasetSourceAuthorizationUsesOwner(t *testing.T) { owner := createDatasetTestUser(t, database, "owner@example.com") creator := createDatasetTestUser(t, database, "creator@example.com") source := models.SaDatasetSource{ - OwnerID: owner.ID, CreatedBy: creator.ID, - Name: "Owned feed", Kind: "rss", URL: "https://example.com/feed", Enabled: true, + OwnerID: owner.ID, + Name: "Owned feed", Kind: "rss", URL: "https://example.com/feed", Enabled: true, } require.NoError(t, database.Create(&source).Error) @@ -180,8 +178,8 @@ func TestDatasetItemsArePaginatedByOwnedSource(t *testing.T) { database := newDatasetTestDatabase(t) owner := createDatasetTestUser(t, database, "owner@example.com") source := models.SaDatasetSource{ - OwnerID: owner.ID, CreatedBy: owner.ID, - Name: "Feed", Kind: "rss", URL: "https://example.com/feed", Enabled: true, + OwnerID: owner.ID, + Name: "Feed", Kind: "rss", URL: "https://example.com/feed", Enabled: true, } require.NoError(t, database.Create(&source).Error) for index := 0; index < 3; index++ { @@ -209,7 +207,7 @@ func TestBuiltInDatasetSourceKeepsManagedFieldsAndCannotBeDeleted(t *testing.T) owner := createDatasetTestUser(t, database, "owner@example.com") seedKey := "built-in" source := models.SaDatasetSource{ - OwnerID: owner.ID, CreatedBy: owner.ID, SeedKey: &seedKey, + OwnerID: owner.ID, SeedKey: &seedKey, Name: "Built-in", Kind: "rss", URL: "https://example.com/feed", IconURL: "https://example.com/icon.png", Enabled: true, } diff --git a/backend/internal/logic/dataset/service.go b/backend/internal/logic/dataset/service.go index 27fa9bd..30c4774 100644 --- a/backend/internal/logic/dataset/service.go +++ b/backend/internal/logic/dataset/service.go @@ -143,7 +143,7 @@ func (s *Service) CreateSource(userID uint, input SourceInput) (*models.SaDatase return nil, err } source := &models.SaDatasetSource{ - OwnerID: userID, CreatedBy: userID, Name: normalized.Name, Kind: normalized.Kind, + OwnerID: userID, Name: normalized.Name, Kind: normalized.Kind, URL: normalized.URL, IconURL: normalized.IconURL, Description: normalized.Description, Enabled: normalized.Enabled, } diff --git a/backend/internal/models/dataset_source.go b/backend/internal/models/dataset_source.go index 2193618..3aaa9b0 100644 --- a/backend/internal/models/dataset_source.go +++ b/backend/internal/models/dataset_source.go @@ -3,22 +3,20 @@ package models import "time" type SaDatasetSource struct { - ID uint `gorm:"primaryKey"` - Identity string `gorm:"type:char(36);uniqueIndex"` - OwnerID uint `gorm:"index;not null;default:0;uniqueIndex:idx_sa_dataset_source_owner_seed,priority:1"` - OwnerIdentity string `gorm:"type:char(36);index"` - CreatedBy uint `gorm:"index;not null"` - CreatedByIdentity string `gorm:"type:char(36);index"` - SeedKey *string `gorm:"size:64;uniqueIndex:idx_sa_dataset_source_owner_seed,priority:2"` - Name string `gorm:"size:160;not null"` - Kind string `gorm:"size:32;not null"` - URL string `gorm:"size:2048"` - IconURL string `gorm:"size:2048"` - Description string `gorm:"type:text"` - Enabled bool `gorm:"not null;default:true;index"` - LastSyncedAt *time.Time - CreatedAt time.Time - UpdatedAt time.Time + ID uint `gorm:"primaryKey"` + Identity string `gorm:"type:char(36);uniqueIndex"` + OwnerID uint `gorm:"index;not null;default:0;uniqueIndex:idx_sa_dataset_source_owner_seed,priority:1"` + OwnerIdentity string `gorm:"type:char(36);index"` + SeedKey *string `gorm:"size:64;uniqueIndex:idx_sa_dataset_source_owner_seed,priority:2"` + Name string `gorm:"size:160;not null"` + Kind string `gorm:"size:32;not null"` + URL string `gorm:"size:2048"` + IconURL string `gorm:"size:2048"` + Description string `gorm:"type:text"` + Enabled bool `gorm:"not null;default:true;index"` + LastSyncedAt *time.Time + CreatedAt time.Time + UpdatedAt time.Time } func (SaDatasetSource) TableName() string { diff --git a/backend/internal/models/identity.go b/backend/internal/models/identity.go index 8a5bc0e..5532dd7 100644 --- a/backend/internal/models/identity.go +++ b/backend/internal/models/identity.go @@ -23,19 +23,10 @@ func (m *SaDatasetSource) BeforeCreate(tx *gorm.DB) error { if err := ensureIdentity(&m.Identity); err != nil { return err } - if m.OwnerID == 0 { - m.OwnerID = m.CreatedBy - } - if m.CreatedBy == 0 { - m.CreatedBy = m.OwnerID - } if m.OwnerID == 0 { return errors.New("dataset source owner is required") } - if err := resolveIdentity(tx, &SaUser{}, m.OwnerID, &m.OwnerIdentity); err != nil { - return err - } - return resolveIdentity(tx, &SaUser{}, m.CreatedBy, &m.CreatedByIdentity) + return resolveIdentity(tx, &SaUser{}, m.OwnerID, &m.OwnerIdentity) } func (m *SaDatasetItem) BeforeCreate(tx *gorm.DB) error { diff --git a/backend/internal/models/identity_test.go b/backend/internal/models/identity_test.go index c1c046f..196e830 100644 --- a/backend/internal/models/identity_test.go +++ b/backend/internal/models/identity_test.go @@ -50,8 +50,6 @@ func TestDatasetSourceBeforeCreateAssociatesOwner(t *testing.T) { require.Equal(t, user.ID, source.OwnerID) require.Equal(t, user.Identity, source.OwnerIdentity) - require.Equal(t, user.ID, source.CreatedBy) - require.Equal(t, user.Identity, source.CreatedByIdentity) } func newIdentityTestDB(t *testing.T) *gorm.DB {