Cascade deletes for attachments properly when deleting a topic or deleting all of a user's posts.

Eliminate some allocs for maxAgeYear in ShowAttachment.
Split DeleteAttachment out of deleteAttachment for reuse elsewhere.

common/attachments.go

@@ -4,8 +4,9 @@ import (
 	"database/sql"
 	"errors"
 	"strings"
+	"os"
 
-	"github.com/Azareal/Gosora/query_gen"
+	qgen "github.com/Azareal/Gosora/query_gen"
 )
 
 var Attachments AttachmentStore
@@ -26,9 +27,9 @@ type AttachmentStore interface {
 	Get(id int) (*MiniAttachment, error)
 	MiniGetList(originTable string, originID int) (alist []*MiniAttachment, err error)
 	BulkMiniGetList(originTable string, ids []int) (amap map[int][]*MiniAttachment, err error)
-	Add(sectionID int, sectionTable string, originID int, originTable string, uploadedBy int, path string, extra string) (int, error)
+	Add(sectionID int, sectionTable string, originID int, originTable string, uploadedBy int, path, extra string) (int, error)
 	MoveTo(sectionID int, originID int, originTable string) error
-	MoveToByExtra(sectionID int, originTable string, extra string) error
+	MoveToByExtra(sectionID int, originTable, extra string) error
 	Count() int
 	CountIn(originTable string, oid int) int
 	CountInPath(path string) int
@@ -50,7 +51,7 @@ type DefaultAttachmentStore struct {
 func NewDefaultAttachmentStore(acc *qgen.Accumulator) (*DefaultAttachmentStore, error) {
 	a := "attachments"
 	return &DefaultAttachmentStore{
-		get:         acc.Select(a).Columns("originID, sectionID, uploadedBy, path, extra").Where("attachID = ?").Prepare(),
+		get:         acc.Select(a).Columns("originID, sectionID, uploadedBy, path, extra").Where("attachID=?").Prepare(),
 		getByObj:    acc.Select(a).Columns("attachID, sectionID, uploadedBy, path, extra").Where("originTable = ? AND originID = ?").Prepare(),
 		add:         acc.Insert(a).Columns("sectionID, sectionTable, originID, originTable, uploadedBy, path, extra").Fields("?,?,?,?,?,?,?").Prepare(),
 		count:       acc.Count(a).Prepare(),
@@ -58,7 +59,7 @@ func NewDefaultAttachmentStore(acc *qgen.Accumulator) (*DefaultAttachmentStore,
 		countInPath: acc.Count(a).Where("path = ?").Prepare(),
 		move:        acc.Update(a).Set("sectionID = ?").Where("originID = ? AND originTable = ?").Prepare(),
 		moveByExtra: acc.Update(a).Set("sectionID = ?").Where("originTable = ? AND extra = ?").Prepare(),
-		delete:      acc.Delete(a).Where("attachID = ?").Prepare(),
+		delete:      acc.Delete(a).Where("attachID=?").Prepare(),
 	}, acc.FirstError()
 }
 
@@ -141,7 +142,7 @@ func (s *DefaultAttachmentStore) Get(id int) (*MiniAttachment, error) {
 	return a, nil
 }
 
-func (s *DefaultAttachmentStore) Add(sectionID int, sectionTable string, originID int, originTable string, uploadedBy int, path string, extra string) (int, error) {
+func (s *DefaultAttachmentStore) Add(sectionID int, sectionTable string, originID int, originTable string, uploadedBy int, path, extra string) (int, error) {
 	res, err := s.add.Exec(sectionID, sectionTable, originID, originTable, uploadedBy, path, extra)
 	if err != nil {
 		return 0, err
@@ -155,7 +156,7 @@ func (s *DefaultAttachmentStore) MoveTo(sectionID int, originID int, originTable
 	return err
 }
 
-func (s *DefaultAttachmentStore) MoveToByExtra(sectionID int, originTable string, extra string) error {
+func (s *DefaultAttachmentStore) MoveToByExtra(sectionID int, originTable, extra string) error {
 	_, err := s.moveByExtra.Exec(sectionID, originTable, extra)
 	return err
 }
@@ -188,3 +189,25 @@ func (s *DefaultAttachmentStore) Delete(aid int) error {
 	_, err := s.delete.Exec(aid)
 	return err
 }
+
+// TODO: Add a table for the files and lock the file row when performing tasks related to the file
+func DeleteAttachment(aid int) error {
+	attach, err := Attachments.Get(aid)
+	if err != nil {
+		return err
+	}
+	err = Attachments.Delete(aid)
+	if err != nil {
+		return err
+	}
+
+	count := Attachments.CountInPath(attach.Path)
+	if count == 0 {
+		err := os.Remove("./attachs/" + attach.Path)
+		if err != nil {
+			return err
+		}
+	}
+	
+	return nil
+}

common/topic.go

@@ -350,6 +350,35 @@ func handleLikedTopicReplies(tid int) error {
 	return rows.Err()
 }
 
+func handleTopicAttachments(tid int) error {
+	f := func(stmt *sql.Stmt) error {
+		rows, err := stmt.Query(tid)
+		if err != nil {
+			return err
+		}
+		defer rows.Close()
+
+		for rows.Next() {
+			var aid int
+			err := rows.Scan(&aid)
+			if err != nil {
+				return err
+			}
+			err = DeleteAttachment(aid)
+			if err != nil && err != sql.ErrNoRows {
+				return err
+			}
+		}
+
+		return rows.Err()
+	}
+	err := f(userStmts.getAttachmentsOfTopic)
+	if err != nil {
+		return err
+	}
+	return f(userStmts.getAttachmentsOfTopic2)
+}
+
 // TODO: Use a transaction here
 func (t *Topic) Delete() error {
 	creator, err := Users.Get(t.CreatedBy)
@@ -380,6 +409,10 @@ func (t *Topic) Delete() error {
 	if err != nil {
 		return err
 	}
+	err = handleTopicAttachments(t.ID)
+	if err != nil {
+		return err
+	}
 	_, err = topicStmts.deleteActivitySubs.Exec(t.ID)
 	if err != nil {
 		return err
@@ -389,7 +422,7 @@ func (t *Topic) Delete() error {
 		return err
 	}
 	if t.Poll > 0 {
-		err = (&Poll{ID:t.Poll}).Delete()
+		err = (&Poll{ID: t.Poll}).Delete()
 		if err != nil {
 			return err
 		}
@@ -523,7 +556,7 @@ func (ru *ReplyUser) Init() error {
 }
 
 // TODO: Factor TopicUser into a *Topic and *User, as this starting to become overly complicated x.x
-func (t *TopicUser) Replies(offset int, pFrag int, user *User) (rlist []*ReplyUser, ogdesc string, err error) {
+func (t *TopicUser) Replies(offset, pFrag int, user *User) (rlist []*ReplyUser, ogdesc string, err error) {
 	var likedMap map[int]int
 	if user.Liked > 0 {
 		likedMap = make(map[int]int)

common/user.go

@@ -150,6 +150,8 @@ type UserStmts struct {
 	deleteProfilePosts     *sql.Stmt
 	deleteReplyPosts       *sql.Stmt
 	getLikedRepliesOfTopic *sql.Stmt
+	getAttachmentsOfTopic  *sql.Stmt
+	getAttachmentsOfTopic2 *sql.Stmt
 }
 
 var userStmts UserStmts
@@ -191,6 +193,8 @@ func init() {
 			deleteProfilePosts:     acc.Select("users_replies").Columns("rid").Where("createdBy=?").Prepare(),
 			deleteReplyPosts:       acc.Select("replies").Columns("rid,tid").Where("createdBy=?").Prepare(),
 			getLikedRepliesOfTopic: acc.Select("replies").Columns("rid").Where("tid=? AND likeCount>0").Prepare(),
+			getAttachmentsOfTopic:  acc.Select("attachments").Columns("attachID").Where("originID=? AND originTable='topics'").Prepare(),
+			getAttachmentsOfTopic2: acc.Select("attachments").Columns("attachID").Where("extra=? AND originTable='replies'").Prepare(),
 		}
 		return acc.FirstError()
 	})
@@ -356,6 +360,10 @@ func (u *User) DeletePosts() error {
 		if err != nil {
 			return err
 		}
+		err = handleTopicAttachments(tid)
+		if err != nil {
+			return err
+		}
 		_, err = topicStmts.deleteActivitySubs.Exec(tid)
 		if err != nil {
 			return err
@@ -365,7 +373,7 @@ func (u *User) DeletePosts() error {
 			return err
 		}
 		if poll > 0 {
-			err = (&Poll{ID:poll}).Delete()
+			err = (&Poll{ID: poll}).Delete()
 			if err != nil {
 				return err
 			}

routes/attachments.go

@@ -3,13 +3,12 @@ package routes
 import (
 	"database/sql"
 	"net/http"
-	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 
 	c "github.com/Azareal/Gosora/common"
-	"github.com/Azareal/Gosora/query_gen"
+	qgen "github.com/Azareal/Gosora/query_gen"
 )
 
 type AttachmentStmts struct {
@@ -22,12 +21,14 @@ var attachmentStmts AttachmentStmts
 func init() {
 	c.DbInits.Add(func(acc *qgen.Accumulator) error {
 		attachmentStmts = AttachmentStmts{
-			get: acc.Select("attachments").Columns("sectionID, sectionTable, originID, originTable, uploadedBy, path").Where("path = ? AND sectionID = ? AND sectionTable = ?").Prepare(),
+			get: acc.Select("attachments").Columns("sectionID, sectionTable, originID, originTable, uploadedBy, path").Where("path=? AND sectionID=? AND sectionTable=?").Prepare(),
 		}
 		return acc.FirstError()
 	})
 }
 
+var maxAgeYear = "max-age=" + strconv.Itoa(int(c.Year))
+
 func ShowAttachment(w http.ResponseWriter, r *http.Request, user c.User, filename string) c.RouteError {
 	filename = c.Stripslashes(filename)
 	ext := filepath.Ext("./attachs/" + filename)
@@ -67,7 +68,7 @@ func ShowAttachment(w http.ResponseWriter, r *http.Request, user c.User, filenam
 	}
 
 	if !user.Loggedin {
-		w.Header().Set("Cache-Control", "max-age="+strconv.Itoa(int(c.Year)))
+		w.Header().Set("Cache-Control", maxAgeYear)
 	} else {
 		guest := c.GuestUser
 		_, ferr := c.SimpleForumUserCheck(w, r, &guest, sid)
@@ -76,7 +77,7 @@ func ShowAttachment(w http.ResponseWriter, r *http.Request, user c.User, filenam
 		}
 		h := w.Header()
 		if guest.Perms.ViewTopic {
-			h.Set("Cache-Control", "max-age="+strconv.Itoa(int(c.Year)))
+			h.Set("Cache-Control", maxAgeYear)
 		} else {
 			h.Set("Cache-Control", "private")
 		}
@@ -87,32 +88,12 @@ func ShowAttachment(w http.ResponseWriter, r *http.Request, user c.User, filenam
 	return nil
 }
 
-// TODO: Add a table for the files and lock the file row when performing tasks related to the file
 func deleteAttachment(w http.ResponseWriter, r *http.Request, user c.User, aid int, js bool) c.RouteError {
-	attach, err := c.Attachments.Get(aid)
+	err := c.DeleteAttachment(aid)
 	if err == sql.ErrNoRows {
 		return c.NotFoundJSQ(w, r, nil, js)
-	} else if err != nil {
+	}
 	return c.InternalErrorJSQ(err, w, r, js)
-	}
-
-	err = c.Attachments.Delete(aid)
-	if err != nil {
-		return c.InternalErrorJSQ(err, w, r, js)
-	}
-
-	count := c.Attachments.CountInPath(attach.Path)
-	if err != nil {
-		return c.InternalErrorJSQ(err, w, r, js)
-	}
-	if count == 0 {
-		err := os.Remove("./attachs/" + attach.Path)
-		if err != nil {
-			return c.InternalErrorJSQ(err, w, r, js)
-		}
-	}
-
-	return nil
 }
 
 // TODO: Stop duplicating this code