more attachment tests

Add UpdateLinked method to AttachmentStore.

common/attachments.go

@@ -3,6 +3,8 @@ package common
 import (
 	"database/sql"
 	"errors"
+
+	//"fmt"
 	"os"
 	"strings"
 
@@ -34,6 +36,8 @@ type AttachmentStore interface {
 	CountIn(originTable string, oid int) int
 	CountInPath(path string) int
 	Delete(id int) error
+
+	UpdateLinked(otable string, oid int) (err error)
 }
 
 type DefaultAttachmentStore struct {
@@ -46,20 +50,27 @@ type DefaultAttachmentStore struct {
 	move        *sql.Stmt
 	moveByExtra *sql.Stmt
 	delete      *sql.Stmt
+
+	replyUpdateAttachs *sql.Stmt
+	topicUpdateAttachs *sql.Stmt
 }
 
 func NewDefaultAttachmentStore(acc *qgen.Accumulator) (*DefaultAttachmentStore, error) {
 	a := "attachments"
 	return &DefaultAttachmentStore{
 		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(),
+		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(),
 		countIn:     acc.Count(a).Where("originTable=? and originID=?").Prepare(),
-		countInPath: acc.Count(a).Where("path = ?").Prepare(),
+		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(),
+
+		// TODO: Less race-y attachment count updates
+		replyUpdateAttachs: acc.Update("replies").Set("attachCount=?").Where("rid=?").Prepare(),
+		topicUpdateAttachs: acc.Update("topics").Set("attachCount=?").Where("tid=?").Prepare(),
 	}, acc.FirstError()
 }
 
@@ -80,7 +91,14 @@ func (s *DefaultAttachmentStore) MiniGetList(originTable string, originID int) (
 		a.Image = ImageFileExts.Contains(a.Ext)
 		alist = append(alist, a)
 	}
-	return alist, rows.Err()
+	err = rows.Err()
+	if err != nil {
+		return nil, err
+	}
+	if len(alist) == 0 {
+		err = sql.ErrNoRows
+	}
+	return alist, err
 }
 
 func (s *DefaultAttachmentStore) BulkMiniGetList(originTable string, ids []int) (amap map[int][]*MiniAttachment, err error) {
@@ -190,14 +208,34 @@ func (s *DefaultAttachmentStore) Delete(id int) error {
 	return err
 }
 
+// TODO: Split this out of this store
+func (s *DefaultAttachmentStore) UpdateLinked(otable string, oid int) (err error) {
+	switch otable {
+	case "topics":
+		_, err = s.topicUpdateAttachs.Exec(s.CountIn(otable, oid), oid)
+		if err != nil {
+			return err
+		}
+		err = Topics.Reload(oid)
+	case "replies":
+		_, err = s.replyUpdateAttachs.Exec(s.CountIn(otable, oid), oid)
+	}
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 // 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 {
+		//fmt.Println("o1")
 		return err
 	}
 	err = Attachments.Delete(aid)
 	if err != nil {
+		//fmt.Println("o2")
 		return err
 	}
 
@@ -205,9 +243,11 @@ func DeleteAttachment(aid int) error {
 	if count == 0 {
 		err := os.Remove("./attachs/" + attach.Path)
 		if err != nil {
+			//fmt.Println("o3")
 			return err
 		}
 	}
+	//fmt.Println("o4")
 
 	return nil
 }

common/page_store.go

@@ -90,10 +90,11 @@ type DefaultPageStore struct {
 
 func NewDefaultPageStore(acc *qgen.Accumulator) (*DefaultPageStore, error) {
 	pa := "pages"
+	allCols := "pid, name, title, body, allowedGroups, menuID"
 	return &DefaultPageStore{
 		get:       acc.Select(pa).Columns("name, title, body, allowedGroups, menuID").Where("pid=?").Prepare(),
-		getByName: acc.Select(pa).Columns("pid, name, title, body, allowedGroups, menuID").Where("name=?").Prepare(),
-		getOffset: acc.Select(pa).Columns("pid, name, title, body, allowedGroups, menuID").Orderby("pid DESC").Limit("?,?").Prepare(),
+		getByName: acc.Select(pa).Columns(allCols).Where("name=?").Prepare(),
+		getOffset: acc.Select(pa).Columns(allCols).Orderby("pid DESC").Limit("?,?").Prepare(),
 		count:     acc.Count(pa).Prepare(),
 		delete:    acc.Delete(pa).Where("pid=?").Prepare(),
 	}, acc.FirstError()

docs/configuration.md

@@ -88,7 +88,7 @@ LastIPCutoff - The number of months which need to pass before the last IP stored
 
 PostIPCutoff - The number of days which need to pass before the IP data for a post is automatically deleted. 0 defaults to whatever the current default is, currently 120 and -1 disables this feature.
 
-PollIPCutoff - The number of days which need to pass before the IP data for a poll is automatically deleted. 0 defaults to whatever the current default is, currently 365 and -1 disables this feature.
+PollIPCutoff - The number of days which need to pass before the IP data for a poll is automatically deleted. 0 defaults to whatever the current default is, currently 90 and -1 disables this feature.
 
 DisableLastIP - Disable storing last IPs for users and purge any existing user last IP data. Default: false
 

misc_test.go

@@ -561,92 +561,6 @@ func topicStoreTest(t *testing.T, newID int, ip string) {
 	// TODO: Test topic creation and retrieving that created topic plus reload and inspecting the cache
 }
 
-// TODO: Implement this
-func TestAttachments(t *testing.T) {
-	miscinit(t)
-	if !c.PluginsInited {
-		c.InitPlugins()
-	}
-
-	filename := "n0-48.png"
-	srcFile := "./test_data/" + filename
-	destFile := "./attachs/" + filename
-
-	expect(t, c.Attachments.Count() == 0, "the number of attachments should be 0")
-	expect(t, c.Attachments.CountIn("topics", 1) == 0, "the number of attachments in topic 1 should be 0")
-	expect(t, c.Attachments.CountInPath(filename) == 0, fmt.Sprintf("the number of attachments with path '%s' should be 0", filename))
-	_, err := c.Attachments.Get(1)
-	if err != nil && err != sql.ErrNoRows {
-		t.Error(err)
-	}
-	expect(t, err == sql.ErrNoRows, ".Get should have no results")
-
-	// Sim an upload, try a proper upload through the proper pathway later on
-	_, err = os.Stat(destFile)
-	if err != nil && !os.IsNotExist(err) {
-		expectNilErr(t, err)
-	} else if err == nil {
-		err := os.Remove(destFile)
-		expectNilErr(t, err)
-	}
-
-	input, err := ioutil.ReadFile(srcFile)
-	expectNilErr(t, err)
-	err = ioutil.WriteFile(destFile, input, 0644)
-	expectNilErr(t, err)
-
-	tid, err := c.Topics.Create(2, "Attach Test", "Fillter Body", 1, "")
-	expectNilErr(t, err)
-	aid, err := c.Attachments.Add(2, "forums", tid, "topics", 1, filename, "")
-	expectNilErr(t, err)
-	expect(t, c.Attachments.Count() == 1, "the number of attachments should be 1")
-	expect(t, c.Attachments.CountIn("topics", tid) == 1, fmt.Sprintf("the number of attachments in topic %d should be 1", tid))
-	expect(t, c.Attachments.CountInPath(filename) == 1, fmt.Sprintf("the number of attachments with path '%s' should be 1", filename))
-
-	a, err := c.Attachments.Get(aid)
-	expectNilErr(t, err)
-	expect(t, a.ID == aid, fmt.Sprintf("a.ID should be %d not %d", aid, a.ID))
-	expect(t, a.SectionID == 2, fmt.Sprintf("a.SectionID should be %d not %d", 2, a.SectionID))
-	expect(t, a.OriginID == tid, fmt.Sprintf("a.OriginID should be %d not %d", tid, a.OriginID))
-	expect(t, a.UploadedBy == 1, fmt.Sprintf("a.UploadedBy should be %d not %d", 1, a.UploadedBy))
-	expect(t, a.Path == filename, fmt.Sprintf("a.Path should be %s not %s", filename, a.Path))
-	expect(t, a.Extra == "", fmt.Sprintf("a.Extra should be blank not %s", a.Extra))
-	expect(t, a.Image, "a.Image should be true")
-	expect(t, a.Ext == "png", fmt.Sprintf("a.Ext should be png not %s", a.Ext))
-
-	alist, err := c.Attachments.MiniGetList("topics", tid)
-	expectNilErr(t, err)
-	expect(t, len(alist) == 1, fmt.Sprintf("len(alist) should be 1 not %d", len(alist)))
-	a = alist[0]
-	expect(t, a.ID == aid, fmt.Sprintf("a.ID should be %d not %d", aid, a.ID))
-	expect(t, a.SectionID == 2, fmt.Sprintf("a.SectionID should be %d not %d", 2, a.SectionID))
-	expect(t, a.OriginID == tid, fmt.Sprintf("a.OriginID should be %d not %d", tid, a.OriginID))
-	expect(t, a.UploadedBy == 1, fmt.Sprintf("a.UploadedBy should be %d not %d", 1, a.UploadedBy))
-	expect(t, a.Path == filename, fmt.Sprintf("a.Path should be %s not %s", filename, a.Path))
-	expect(t, a.Extra == "", fmt.Sprintf("a.Extra should be blank not %s", a.Extra))
-	expect(t, a.Image, "a.Image should be true")
-	expect(t, a.Ext == "png", fmt.Sprintf("a.Ext should be png not %s", a.Ext))
-
-	// TODO: Cover the other bits of creation / deletion not covered in the AttachmentStore like updating the reply / topic attachCount
-
-	// TODO: Get attachment tests
-	// TODO: Move attachment tests
-
-	expectNilErr(t, c.Attachments.Delete(aid))
-	expect(t, c.Attachments.Count() == 0, "the number of attachments should be 0")
-	expect(t, c.Attachments.CountIn("topics", tid) == 0, fmt.Sprintf("the number of attachments in topic %d should be 0", tid))
-	expect(t, c.Attachments.CountInPath(filename) == 0, fmt.Sprintf("the number of attachments with path '%s' should be 0", filename))
-	_, err = c.Attachments.Get(aid)
-	if err != nil && err != sql.ErrNoRows {
-		t.Error(err)
-	}
-	expect(t, err == sql.ErrNoRows, ".Get should have no results")
-
-	// TODO: Reply attachment test
-	// TODO: Delete reply attachment
-	// TODO: Path overlap tests
-}
-
 func TestForumStore(t *testing.T) {
 	miscinit(t)
 	if !c.PluginsInited {
@@ -656,7 +570,6 @@ func TestForumStore(t *testing.T) {
 
 	fcache, ok := c.Forums.(c.ForumCache)
 	expect(t, ok, "Unable to cast ForumStore to ForumCache")
-
 	expect(t, c.Forums.Count() == 2, "The forumstore global count should be 2")
 	expect(t, fcache.Length() == 2, "The forum cache length should be 2")
 
@@ -861,16 +774,16 @@ func TestGroupStore(t *testing.T) {
 	recordMustNotExist(t, err, "GID #-1 shouldn't exist")
 
 	// TODO: Refactor the group store to remove GID #0
-	group, err := c.Groups.Get(0)
+	g, err := c.Groups.Get(0)
 	recordMustExist(t, err, "Couldn't find GID #0")
 
-	expect(t, group.ID == 0, fmt.Sprintf("group.ID doesn't not match the requested GID. Got '%d' instead.", group.ID))
-	expect(t, group.Name == "Unknown", fmt.Sprintf("GID #0 is named '%s' and not 'Unknown'", group.Name))
+	expect(t, g.ID == 0, fmt.Sprintf("g.ID doesn't not match the requested GID. Got '%d' instead.", g.ID))
+	expect(t, g.Name == "Unknown", fmt.Sprintf("GID #0 is named '%s' and not 'Unknown'", g.Name))
 
-	group, err = c.Groups.Get(1)
+	g, err = c.Groups.Get(1)
 	recordMustExist(t, err, "Couldn't find GID #1")
-	expect(t, group.ID == 1, fmt.Sprintf("group.ID doesn't not match the requested GID. Got '%d' instead.'", group.ID))
-	expect(t, len(group.CanSee) > 0, "group.CanSee should not be zero")
+	expect(t, g.ID == 1, fmt.Sprintf("g.ID doesn't not match the requested GID. Got '%d' instead.'", g.ID))
+	expect(t, len(g.CanSee) > 0, "g.CanSee should not be zero")
 
 	expect(t, !c.Groups.Exists(-1), "GID #-1 shouldn't exist")
 	// 0 aka Unknown, for system posts and other oddities
@@ -884,13 +797,13 @@ func TestGroupStore(t *testing.T) {
 	expectNilErr(t, err)
 	expect(t, c.Groups.Exists(gid), "The group we just made doesn't exist")
 
-	group, err = c.Groups.Get(gid)
+	g, err = c.Groups.Get(gid)
 	expectNilErr(t, err)
-	expect(t, group.ID == gid, "The group ID should match the requested ID")
-	expect(t, group.IsAdmin, "This should be an admin group")
-	expect(t, group.IsMod, "This should be a mod group")
-	expect(t, !group.IsBanned, "This shouldn't be a ban group")
-	expect(t, len(group.CanSee) == 0, "group.CanSee should be empty")
+	expect(t, g.ID == gid, "The group ID should match the requested ID")
+	expect(t, g.IsAdmin, "This should be an admin group")
+	expect(t, g.IsMod, "This should be a mod group")
+	expect(t, !g.IsBanned, "This shouldn't be a ban group")
+	expect(t, len(g.CanSee) == 0, "g.CanSee should be empty")
 
 	isAdmin = false
 	isMod = true
@@ -899,36 +812,36 @@ func TestGroupStore(t *testing.T) {
 	expectNilErr(t, err)
 	expect(t, c.Groups.Exists(gid), "The group we just made doesn't exist")
 
-	group, err = c.Groups.Get(gid)
+	g, err = c.Groups.Get(gid)
 	expectNilErr(t, err)
-	expect(t, group.ID == gid, "The group ID should match the requested ID")
-	expect(t, !group.IsAdmin, "This should not be an admin group")
-	expect(t, group.IsMod, "This should be a mod group")
-	expect(t, !group.IsBanned, "This shouldn't be a ban group")
+	expect(t, g.ID == gid, "The group ID should match the requested ID")
+	expect(t, !g.IsAdmin, "This should not be an admin group")
+	expect(t, g.IsMod, "This should be a mod group")
+	expect(t, !g.IsBanned, "This shouldn't be a ban group")
 
 	// TODO: Make sure this pointer doesn't change once we refactor the group store to stop updating the pointer
-	err = group.ChangeRank(false, false, true)
+	err = g.ChangeRank(false, false, true)
 	expectNilErr(t, err)
 
-	group, err = c.Groups.Get(gid)
+	g, err = c.Groups.Get(gid)
 	expectNilErr(t, err)
-	expect(t, group.ID == gid, "The group ID should match the requested ID")
-	expect(t, !group.IsAdmin, "This shouldn't be an admin group")
-	expect(t, !group.IsMod, "This shouldn't be a mod group")
-	expect(t, group.IsBanned, "This should be a ban group")
+	expect(t, g.ID == gid, "The group ID should match the requested ID")
+	expect(t, !g.IsAdmin, "This shouldn't be an admin group")
+	expect(t, !g.IsMod, "This shouldn't be a mod group")
+	expect(t, g.IsBanned, "This should be a ban group")
 
-	err = group.ChangeRank(true, true, true)
+	err = g.ChangeRank(true, true, true)
 	expectNilErr(t, err)
 
-	group, err = c.Groups.Get(gid)
+	g, err = c.Groups.Get(gid)
 	expectNilErr(t, err)
-	expect(t, group.ID == gid, "The group ID should match the requested ID")
-	expect(t, group.IsAdmin, "This should be an admin group")
-	expect(t, group.IsMod, "This should be a mod group")
-	expect(t, !group.IsBanned, "This shouldn't be a ban group")
-	expect(t, len(group.CanSee) == 0, "len(group.CanSee) should be 0")
+	expect(t, g.ID == gid, "The group ID should match the requested ID")
+	expect(t, g.IsAdmin, "This should be an admin group")
+	expect(t, g.IsMod, "This should be a mod group")
+	expect(t, !g.IsBanned, "This shouldn't be a ban group")
+	expect(t, len(g.CanSee) == 0, "len(g.CanSee) should be 0")
 
-	err = group.ChangeRank(false, true, true)
+	err = g.ChangeRank(false, true, true)
 	expectNilErr(t, err)
 
 	forum, err := c.Forums.Get(2)
@@ -944,26 +857,26 @@ func TestGroupStore(t *testing.T) {
 	err = forum.SetPerms(&forumPerms, "custom", gid)
 	expectNilErr(t, err)
 
-	group, err = c.Groups.Get(gid)
+	g, err = c.Groups.Get(gid)
 	expectNilErr(t, err)
-	expect(t, group.ID == gid, "The group ID should match the requested ID")
-	expect(t, !group.IsAdmin, "This shouldn't be an admin group")
-	expect(t, group.IsMod, "This should be a mod group")
-	expect(t, !group.IsBanned, "This shouldn't be a ban group")
-	expect(t, group.CanSee != nil, "group.CanSee must not be nil")
-	expect(t, len(group.CanSee) == 1, "len(group.CanSee) should not be one")
-	expect(t, group.CanSee[0] == 2, "group.CanSee[0] should be 2")
-	canSee := group.CanSee
+	expect(t, g.ID == gid, "The group ID should match the requested ID")
+	expect(t, !g.IsAdmin, "This shouldn't be an admin group")
+	expect(t, g.IsMod, "This should be a mod group")
+	expect(t, !g.IsBanned, "This shouldn't be a ban group")
+	expect(t, g.CanSee != nil, "g.CanSee must not be nil")
+	expect(t, len(g.CanSee) == 1, "len(g.CanSee) should not be one")
+	expect(t, g.CanSee[0] == 2, "g.CanSee[0] should be 2")
+	canSee := g.CanSee
 
 	// Make sure the data is static
 	c.Groups.Reload(gid)
 
-	group, err = c.Groups.Get(gid)
+	g, err = c.Groups.Get(gid)
 	expectNilErr(t, err)
-	expect(t, group.ID == gid, "The group ID should match the requested ID")
-	expect(t, !group.IsAdmin, "This shouldn't be an admin group")
-	expect(t, group.IsMod, "This should be a mod group")
-	expect(t, !group.IsBanned, "This shouldn't be a ban group")
+	expect(t, g.ID == gid, "The group ID should match the requested ID")
+	expect(t, !g.IsAdmin, "This shouldn't be an admin group")
+	expect(t, g.IsMod, "This should be a mod group")
+	expect(t, !g.IsBanned, "This shouldn't be a ban group")
 
 	// TODO: Don't enforce a specific order here
 	canSeeTest := func(a, b []int) bool {
@@ -981,7 +894,7 @@ func TestGroupStore(t *testing.T) {
 		return true
 	}
 
-	expect(t, canSeeTest(group.CanSee, canSee), "group.CanSee is not being reused")
+	expect(t, canSeeTest(g.CanSee, canSee), "g.CanSee is not being reused")
 
 	// TODO: Test group deletion
 	// TODO: Test group reload
@@ -1051,7 +964,6 @@ func TestReplyStore(t *testing.T) {
 	if !c.PluginsInited {
 		c.InitPlugins()
 	}
-
 	_, err := c.Rstore.Get(-1)
 	recordMustNotExist(t, err, "RID #-1 shouldn't exist")
 	_, err = c.Rstore.Get(0)
@@ -1074,10 +986,10 @@ func testReplyStore(t *testing.T, newID, newPostCount int, ip string) {
 	}
 
 	replyTest := func(rid, parentID, createdBy int, content, ip string) {
-		reply, err := c.Rstore.Get(rid)
-		replyTest2(reply, err, rid, parentID, createdBy, content, ip)
-		reply, err = c.Rstore.GetCache().Get(rid)
-		replyTest2(reply, err, rid, parentID, createdBy, content, ip)
+		r, err := c.Rstore.Get(rid)
+		replyTest2(r, err, rid, parentID, createdBy, content, ip)
+		r, err = c.Rstore.GetCache().Get(rid)
+		replyTest2(r, err, rid, parentID, createdBy, content, ip)
 	}
 	replyTest(1, 1, 1, "A reply!", "::1")
 
@@ -1145,6 +1057,143 @@ func testReplyStore(t *testing.T, newID, newPostCount int, ip string) {
 	// TODO: Add tests for ReplyCache
 }
 
+// TODO: Implement this
+func TestAttachments(t *testing.T) {
+	miscinit(t)
+	if !c.PluginsInited {
+		c.InitPlugins()
+	}
+
+	filename := "n0-48.png"
+	srcFile := "./test_data/" + filename
+	destFile := "./attachs/" + filename
+
+	expect(t, c.Attachments.Count() == 0, "the number of attachments should be 0")
+	expect(t, c.Attachments.CountIn("topics", 1) == 0, "the number of attachments in topic 1 should be 0")
+	expect(t, c.Attachments.CountInPath(filename) == 0, fmt.Sprintf("the number of attachments with path '%s' should be 0", filename))
+	_, err := c.Attachments.Get(1)
+	if err != nil && err != sql.ErrNoRows {
+		t.Error(err)
+	}
+	expect(t, err == sql.ErrNoRows, ".Get should have no results")
+	_, err = c.Attachments.MiniGetList("topics", 1)
+	if err != nil && err != sql.ErrNoRows {
+		t.Error(err)
+	}
+	expect(t, err == sql.ErrNoRows, ".MiniGetList should have no results")
+	_, err = c.Attachments.BulkMiniGetList("topics", []int{1})
+	if err != nil && err != sql.ErrNoRows {
+		t.Error(err)
+	}
+	expect(t, err == sql.ErrNoRows, ".BulkMiniGetList should have no results")
+
+	// Sim an upload, try a proper upload through the proper pathway later on
+	_, err = os.Stat(destFile)
+	if err != nil && !os.IsNotExist(err) {
+		expectNilErr(t, err)
+	} else if err == nil {
+		err := os.Remove(destFile)
+		expectNilErr(t, err)
+	}
+
+	input, err := ioutil.ReadFile(srcFile)
+	expectNilErr(t, err)
+	err = ioutil.WriteFile(destFile, input, 0644)
+	expectNilErr(t, err)
+
+	tid, err := c.Topics.Create(2, "Attach Test", "Fillter Body", 1, "")
+	expectNilErr(t, err)
+	aid, err := c.Attachments.Add(2, "forums", tid, "topics", 1, filename, "")
+	expectNilErr(t, err)
+	expectNilErr(t, c.Attachments.UpdateLinked("topics", tid))
+	expect(t, c.Attachments.Count() == 1, "the number of attachments should be 1")
+	expect(t, c.Attachments.CountIn("topics", tid) == 1, fmt.Sprintf("the number of attachments in topic %d should be 1", tid))
+	expect(t, c.Attachments.CountInPath(filename) == 1, fmt.Sprintf("the number of attachments with path '%s' should be 1", filename))
+
+	var a *c.MiniAttachment
+	f := func(aid, sid, oid, uploadedBy int, path, extra, ext string) {
+		expect(t, a.ID == aid, fmt.Sprintf("ID should be %d not %d", aid, a.ID))
+		expect(t, a.SectionID == sid, fmt.Sprintf("SectionID should be %d not %d", sid, a.SectionID))
+		expect(t, a.OriginID == oid, fmt.Sprintf("OriginID should be %d not %d", oid, a.OriginID))
+		expect(t, a.UploadedBy == uploadedBy, fmt.Sprintf("UploadedBy should be %d not %d", uploadedBy, a.UploadedBy))
+		expect(t, a.Path == path, fmt.Sprintf("Path should be %s not %s", path, a.Path))
+		expect(t, a.Extra == extra, fmt.Sprintf("Extra should be %s not %s", extra, a.Extra))
+		expect(t, a.Image, "Image should be true")
+		expect(t, a.Ext == ext, fmt.Sprintf("Ext should be %s not %s", ext, a.Ext))
+	}
+
+	f2 := func(aid, oid int, extra string, topic bool) {
+		var tbl string
+		if topic {
+			tbl = "topics"
+		} else {
+			tbl = "replies"
+		}
+		a, err = c.Attachments.Get(aid)
+		expectNilErr(t, err)
+		f(aid, 2, oid, 1, filename, extra, "png")
+
+		alist, err := c.Attachments.MiniGetList(tbl, oid)
+		expectNilErr(t, err)
+		expect(t, len(alist) == 1, fmt.Sprintf("len(alist) should be 1 not %d", len(alist)))
+		a = alist[0]
+		f(aid, 2, oid, 1, filename, extra, "png")
+
+		amap, err := c.Attachments.BulkMiniGetList(tbl, []int{oid})
+		expectNilErr(t, err)
+		expect(t, len(amap) == 1, fmt.Sprintf("len(amap) should be 1 not %d", len(amap)))
+		alist, ok := amap[oid]
+		if !ok {
+			t.Logf("key %d not found in amap", oid)
+		}
+		expect(t, len(alist) == 1, fmt.Sprintf("len(alist) should be 1 not %d", len(alist)))
+		a = alist[0]
+		f(aid, 2, oid, 1, filename, extra, "png")
+	}
+
+	topic, err := c.Topics.Get(tid)
+	expectNilErr(t, err)
+	expect(t, topic.AttachCount == 1, fmt.Sprintf("topic.AttachCount should be 1 not %d", topic.AttachCount))
+	f2(aid, tid, "", true)
+
+	// TODO: Cover the other bits of creation / deletion not covered in the AttachmentStore like updating the reply / topic attachCount
+
+	// TODO: Move attachment tests
+
+	expectNilErr(t, c.Attachments.Delete(aid))
+	expect(t, c.Attachments.Count() == 0, "the number of attachments should be 0")
+	expect(t, c.Attachments.CountIn("topics", tid) == 0, fmt.Sprintf("the number of attachments in topic %d should be 0", tid))
+	expect(t, c.Attachments.CountInPath(filename) == 0, fmt.Sprintf("the number of attachments with path '%s' should be 0", filename))
+	_, err = c.Attachments.Get(aid)
+	if err != nil && err != sql.ErrNoRows {
+		t.Error(err)
+	}
+	expect(t, err == sql.ErrNoRows, ".Get should have no results")
+	_, err = c.Attachments.MiniGetList("topics", tid)
+	if err != nil && err != sql.ErrNoRows {
+		t.Error(err)
+	}
+	expect(t, err == sql.ErrNoRows, ".MiniGetList should have no results")
+	_, err = c.Attachments.BulkMiniGetList("topics", []int{tid})
+	if err != nil && err != sql.ErrNoRows {
+		t.Error(err)
+	}
+	expect(t, err == sql.ErrNoRows, ".BulkMiniGetList should have no results")
+
+	rid, err := c.Rstore.Create(topic, "Reply Filler", "", 1)
+	expectNilErr(t, err)
+	aid, err = c.Attachments.Add(2, "forums", rid, "replies", 1, filename, strconv.Itoa(topic.ID))
+	expectNilErr(t, err)
+	expectNilErr(t, c.Attachments.UpdateLinked("replies", rid))
+	r, err := c.Rstore.Get(rid)
+	expectNilErr(t, err)
+	expect(t, r.AttachCount == 1, fmt.Sprintf("r.AttachCount should be 1 not %d", r.AttachCount))
+	f2(aid, rid, strconv.Itoa(topic.ID), false)
+
+	// TODO: Delete reply attachment
+	// TODO: Path overlap tests
+}
+
 func TestProfileReplyStore(t *testing.T) {
 	miscinit(t)
 	if !c.PluginsInited {

routes/attachments.go

@@ -101,7 +101,7 @@ func deleteAttachment(w http.ResponseWriter, r *http.Request, user c.User, aid i
 // TODO: Stop duplicating this code
 // TODO: Use a transaction here
 // TODO: Move this function to neutral ground
-func uploadAttachment(w http.ResponseWriter, r *http.Request, user c.User, sid int, sectionTable string, oid int, originTable, extra string) (pathMap map[string]string, rerr c.RouteError) {
+func uploadAttachment(w http.ResponseWriter, r *http.Request, user c.User, sid int, stable string, oid int, otable, extra string) (pathMap map[string]string, rerr c.RouteError) {
 	pathMap = make(map[string]string)
 	files, rerr := uploadFilesWithHash(w, r, user, "./attachs/")
 	if rerr != nil {
@@ -109,7 +109,7 @@ func uploadAttachment(w http.ResponseWriter, r *http.Request, user c.User, sid i
 	}
 
 	for _, filename := range files {
-		aid, err := c.Attachments.Add(sid, sectionTable, oid, originTable, user.ID, filename, extra)
+		aid, err := c.Attachments.Add(sid, stable, oid, otable, user.ID, filename, extra)
 		if err != nil {
 			return nil, c.InternalError(err, w, r)
 		}
@@ -121,22 +121,10 @@ func uploadAttachment(w http.ResponseWriter, r *http.Request, user c.User, sid i
 			pathMap[filename] = strconv.Itoa(aid)
 		}
 
-		switch originTable {
-		case "topics":
-			_, err = topicStmts.updateAttachs.Exec(c.Attachments.CountIn(originTable, oid), oid)
+		err = c.Attachments.UpdateLinked(otable, oid)
 		if err != nil {
 			return nil, c.InternalError(err, w, r)
 		}
-			err = c.Topics.Reload(oid)
-			if err != nil {
-				return nil, c.InternalError(err, w, r)
-			}
-		case "replies":
-			_, err = replyStmts.updateAttachs.Exec(c.Attachments.CountIn(originTable, oid), oid)
-			if err != nil {
-				return nil, c.InternalError(err, w, r)
-			}
-		}
 	}
 
 	return pathMap, nil

routes/reply.go

@@ -15,7 +15,6 @@ import (
 )
 
 type ReplyStmts struct {
-	updateAttachs     *sql.Stmt
 	createReplyPaging *sql.Stmt
 }
 
@@ -25,8 +24,6 @@ var replyStmts ReplyStmts
 func init() {
 	c.DbInits.Add(func(acc *qgen.Accumulator) error {
 		replyStmts = ReplyStmts{
-			// TODO: Less race-y attachment count updates
-			updateAttachs:     acc.Update("replies").Set("attachCount=?").Where("rid=?").Prepare(),
 			createReplyPaging: acc.Select("replies").Cols("rid").Where("rid >= ? - 1 AND tid=?").Orderby("rid ASC").Prepare(),
 		}
 		return acc.FirstError()

routes/topic.go

@@ -30,7 +30,6 @@ import (
 
 type TopicStmts struct {
 	getLikedTopic *sql.Stmt
-	updateAttachs *sql.Stmt
 }
 
 var topicStmts TopicStmts
@@ -40,8 +39,6 @@ func init() {
 	c.DbInits.Add(func(acc *qgen.Accumulator) error {
 		topicStmts = TopicStmts{
 			getLikedTopic: acc.Select("likes").Columns("targetItem").Where("sentBy=? && targetItem=? && targetType='topics'").Prepare(),
-			// TODO: Less race-y attachment count updates
-			updateAttachs: acc.Update("topics").Set("attachCount=?").Where("tid=?").Prepare(),
 		}
 		return acc.FirstError()
 	})
@@ -134,7 +131,7 @@ func ViewTopic(w http.ResponseWriter, r *http.Request, user c.User, header *c.He
 
 	if topic.AttachCount > 0 {
 		attachs, err := c.Attachments.MiniGetList("topics", topic.ID)
-		if err != nil {
+		if err != nil && err != sql.ErrNoRows {
 			// TODO: We might want to be a little permissive here in-case of a desync?
 			return c.InternalError(err, w, r)
 		}
@@ -452,6 +449,7 @@ func CreateTopicSubmit(w http.ResponseWriter, r *http.Request, user c.User) c.Ro
 	return nil
 }
 
+// TODO: Move this function
 func uploadFilesWithHash(w http.ResponseWriter, r *http.Request, user c.User, dir string) (filenames []string, rerr c.RouteError) {
 	files, ok := r.MultipartForm.File["upload_files"]
 	if !ok {