diff --git a/database/migrations.go b/database/migrations.go
index a9d2dae3a26d48db2ec7e501903d386fa274cb88..55f805296222a8d333684cbb4f08152427552ded 100644
--- a/database/migrations.go
+++ b/database/migrations.go
@@ -674,4 +674,32 @@ var migrations = []func(tx *sql.Tx) error{
_, err = tx.Exec(sql)
return err
},
+ func(tx *sql.Tx) (err error) {
+ // Delete duplicated rows
+ sql := `
+ DELETE FROM enclosures a USING enclosures b
+ WHERE a.id < b.id
+ AND a.user_id = b.user_id
+ AND a.entry_id = b.entry_id
+ AND a.url = b.url;
+ `
+ _, err = tx.Exec(sql)
+ if err != nil {
+ return err
+ }
+
+ // Remove previous index
+ _, err = tx.Exec(`DROP INDEX enclosures_user_entry_url_idx`)
+ if err != nil {
+ return err
+ }
+
+ // Create unique index
+ _, err = tx.Exec(`CREATE UNIQUE INDEX enclosures_user_entry_url_unique_idx ON enclosures(user_id, entry_id, md5(url))`)
+ if err != nil {
+ return err
+ }
+
+ return nil
+ },
}
diff --git a/storage/enclosure.go b/storage/enclosure.go
index 048c3fde260320b45c427ee53ca96b3b24dc06ea..3573609b9c78b1317316861890de4b14d4da52c4 100644
--- a/storage/enclosure.go
+++ b/storage/enclosure.go
@@ -6,26 +6,13 @@ package storage // import "miniflux.app/storage"
import (
"database/sql"
"fmt"
+ "strings"
"miniflux.app/model"
)
// GetEnclosures returns all attachments for the given entry.
func (s *Storage) GetEnclosures(entryID int64) (model.EnclosureList, error) {
- tx, err := s.db.Begin()
- if err != nil {
- return nil, fmt.Errorf(`store: unable to start transaction: %v`, err)
- }
- // As the transaction is only created to use the txGetEnclosures function, we can commit it and close it.
- // to avoid leaving an open transaction as I don't have any idea if it will be closed automatically,
- // I manually close it. I chose `commit` over `rollback` because I assumed it cost less on SGBD, but I'm no Database
- // administrator so any better solution is welcome.
- defer tx.Commit()
- return s.txGetEnclosures(tx, entryID)
-}
-
-// GetEnclosures returns all attachments for the given entry within a Database transaction
-func (s *Storage) txGetEnclosures(tx *sql.Tx, entryID int64) (model.EnclosureList, error) {
query := `
SELECT
id,
@@ -42,7 +29,7 @@ func (s *Storage) txGetEnclosures(tx *sql.Tx, entryID int64) (model.EnclosureLis
ORDER BY id ASC
`
- rows, err := tx.Query(query, entryID)
+ rows, err := s.db.Query(query, entryID)
if err != nil {
return nil, fmt.Errorf(`store: unable to fetch enclosures: %v`, err)
}
@@ -109,7 +96,8 @@ func (s *Storage) GetEnclosure(enclosureID int64) (*model.Enclosure, error) {
}
func (s *Storage) createEnclosure(tx *sql.Tx, enclosure *model.Enclosure) error {
- if enclosure.URL == "" {
+ enclosureURL := strings.TrimSpace(enclosure.URL)
+ if enclosureURL == "" {
return nil
}
@@ -118,76 +106,60 @@ func (s *Storage) createEnclosure(tx *sql.Tx, enclosure *model.Enclosure) error
(url, size, mime_type, entry_id, user_id, media_progression)
VALUES
($1, $2, $3, $4, $5, $6)
- RETURNING
- id
+ ON CONFLICT (user_id, entry_id, md5(url)) DO NOTHING
`
- err := tx.QueryRow(
+ _, err := tx.Exec(
query,
- enclosure.URL,
+ enclosureURL,
enclosure.Size,
enclosure.MimeType,
enclosure.EntryID,
enclosure.UserID,
enclosure.MediaProgression,
- ).Scan(&enclosure.ID)
+ )
if err != nil {
- return fmt.Errorf(`store: unable to create enclosure %q: %v`, enclosure.URL, err)
+ return fmt.Errorf(`store: unable to create enclosure: %v`, err)
}
return nil
}
func (s *Storage) updateEnclosures(tx *sql.Tx, userID, entryID int64, enclosures model.EnclosureList) error {
- originalEnclosures, err := s.txGetEnclosures(tx, entryID)
- if err != nil {
- return fmt.Errorf(`store: unable fetch enclosures for entry #%d : %v`, entryID, err)
- }
-
- // this map will allow to identify enclosure already in the database based on their URL.
- originalEnclosuresByURL := map[string]*model.Enclosure{}
- for _, enclosure := range originalEnclosures {
- originalEnclosuresByURL[enclosure.URL] = enclosure
+ if len(enclosures) == 0 {
+ return nil
}
- // in order to keep enclosure ID consistent I need to identify already existing one to keep them as is, and only
- // add/delete enclosure that need to be
- enclosuresToAdd := map[string]*model.Enclosure{}
- enclosuresToDelete := map[string]*model.Enclosure{}
- enclosuresToKeep := map[string]*model.Enclosure{}
+ sqlValues := []any{userID, entryID}
+ sqlPlaceholders := []string{}
for _, enclosure := range enclosures {
- originalEnclosure, alreadyExist := originalEnclosuresByURL[enclosure.URL]
- if alreadyExist {
- enclosuresToKeep[originalEnclosure.URL] = originalEnclosure // we keep the original already in the database
- } else {
- enclosuresToAdd[enclosure.URL] = enclosure // we insert the new one
- }
- }
-
- // we know what to keep, and add. We need to find what's in the database that need to be deleted
- for _, enclosure := range originalEnclosures {
- _, existToAdd := enclosuresToAdd[enclosure.URL]
- _, existToKeep := enclosuresToKeep[enclosure.URL]
- if !existToKeep && !existToAdd { // if it does not exist to keep or add this mean it has been deleted.
- enclosuresToDelete[enclosure.URL] = enclosure
- }
- }
+ sqlPlaceholders = append(sqlPlaceholders, fmt.Sprintf(`$%d`, len(sqlValues)+1))
+ sqlValues = append(sqlValues, strings.TrimSpace(enclosure.URL))
- for _, enclosure := range enclosuresToDelete {
- if _, err := tx.Exec(`DELETE FROM enclosures WHERE user_id=$1 AND entry_id=$2 and id=$3`, userID, entryID, enclosure.ID); err != nil {
+ if err := s.createEnclosure(tx, enclosure); err != nil {
return err
}
}
- for _, enclosure := range enclosuresToAdd {
- if err := s.createEnclosure(tx, enclosure); err != nil {
- return err
- }
+ query := `
+ DELETE FROM enclosures
+ WHERE
+ user_id=$1 AND
+ entry_id=$2 AND
+ url NOT IN (%s)
+ `
+
+ query = fmt.Sprintf(query, strings.Join(sqlPlaceholders, `,`))
+
+ _, err := tx.Exec(query, sqlValues...)
+ if err != nil {
+ return fmt.Errorf(`store: unable to delete old enclosures: %v`, err)
}
return nil
}
+
func (s *Storage) UpdateEnclosure(enclosure *model.Enclosure) error {
query := `
UPDATE
@@ -211,8 +183,10 @@ func (s *Storage) UpdateEnclosure(enclosure *model.Enclosure) error {
enclosure.MediaProgression,
enclosure.ID,
)
+
if err != nil {
return fmt.Errorf(`store: unable to update enclosure #%d : %v`, enclosure.ID, err)
}
+
return nil
}
diff --git a/ui/entry_enclosure_save_position.go b/ui/entry_enclosure_save_position.go
index d74e3745895931286a8b27f4cc071c1339855f84..91fd0078eff1f6f617932c130afa9acedf28c679 100644
--- a/ui/entry_enclosure_save_position.go
+++ b/ui/entry_enclosure_save_position.go
@@ -12,10 +12,6 @@ import (
"miniflux.app/http/response/json"
)
-type enclosurePositionSaveRequest struct {
- Progression int64 `json:"progression"`
-}
-
func (h *handler) saveEnclosureProgression(w http.ResponseWriter, r *http.Request) {
enclosureID := request.RouteInt64Param(r, "enclosureID")
enclosure, err := h.store.GetEnclosure(enclosureID)
@@ -23,6 +19,16 @@ func (h *handler) saveEnclosureProgression(w http.ResponseWriter, r *http.Reques
json.ServerError(w, r, err)
return
}
+
+ if enclosure == nil {
+ json.NotFound(w, r)
+ return
+ }
+
+ type enclosurePositionSaveRequest struct {
+ Progression int64 `json:"progression"`
+ }
+
var postData enclosurePositionSaveRequest
body, err := io.ReadAll(r.Body)
if err != nil {