refactor: simplify dataset source ownership
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user