ForumStore.Create now rejects blank names.

ForumStore.Get should now respond appropriately when a forum has been deleted.
Removed a bit of boilerplate in the ForumStore tests.

Added forumstore tests for forum deletion and trying to create a forum with a blank name.

common/forum_store.go

@@ -1,7 +1,7 @@
 /*
 *
 *	Gosora Forum Store
-* 	Copyright Azareal 2017 - 2019
+* 	Copyright Azareal 2017 - 2020
 *
  */
 package common
@@ -20,6 +20,7 @@ import (
 var forumCreateMutex sync.Mutex
 var forumPerms map[int]map[int]*ForumPerms // [gid][fid]*ForumPerms // TODO: Add an abstraction around this and make it more thread-safe
 var Forums ForumStore
+var ErrBlankName = errors.New("The name must not be blank")
 
 // ForumStore is an interface for accessing the forums and the metadata stored on them
 type ForumStore interface {
@@ -67,7 +68,7 @@ type MemoryForumStore struct {
 	updateCache  *sql.Stmt
 	addTopics    *sql.Stmt
 	removeTopics *sql.Stmt
-	updateOrder *sql.Stmt
+	updateOrder  *sql.Stmt
 }
 
 // NewMemoryForumStore gives you a new instance of MemoryForumStore
@@ -83,7 +84,7 @@ func NewMemoryForumStore() (*MemoryForumStore, error) {
 		updateCache:  acc.Update("forums").Set("lastTopicID = ?, lastReplyerID = ?").Where("fid = ?").Prepare(),
 		addTopics:    acc.Update("forums").Set("topicCount = topicCount + ?").Where("fid = ?").Prepare(),
 		removeTopics: acc.Update("forums").Set("topicCount = topicCount - ?").Where("fid = ?").Prepare(),
-		updateOrder: acc.Update("forums").Set("order = ?").Where("fid = ?").Prepare(),
+		updateOrder:  acc.Update("forums").Set("order = ?").Where("fid = ?").Prepare(),
 	}, acc.FirstError()
 }
 
@@ -164,21 +165,28 @@ func (mfs *MemoryForumStore) CacheGet(id int) (*Forum, error) {
 
 func (mfs *MemoryForumStore) Get(id int) (*Forum, error) {
 	fint, ok := mfs.forums.Load(id)
-	if !ok || fint.(*Forum).Name == "" {
+	if ok {
-		var forum = &Forum{ID: id}
+		forum := fint.(*Forum)
-		err := mfs.get.QueryRow(id).Scan(&forum.Name, &forum.Desc, &forum.Active, &forum.Order, &forum.Preset, &forum.ParentID, &forum.ParentType, &forum.TopicCount, &forum.LastTopicID, &forum.LastReplyerID)
+		if forum.Name == "" {
-		if err != nil {
+			return nil, ErrNoRows
-			return forum, err
 		}
+		return forum, nil
+	}
 
-		forum.Link = BuildForumURL(NameToSlug(forum.Name), forum.ID)
+	var forum = &Forum{ID: id}
-		forum.LastTopic = Topics.DirtyGet(forum.LastTopicID)
+	err := mfs.get.QueryRow(id).Scan(&forum.Name, &forum.Desc, &forum.Active, &forum.Order, &forum.Preset, &forum.ParentID, &forum.ParentType, &forum.TopicCount, &forum.LastTopicID, &forum.LastReplyerID)
-		forum.LastReplyer = Users.DirtyGet(forum.LastReplyerID)
+	if err != nil {
-
-		mfs.CacheSet(forum)
 		return forum, err
 	}
-	return fint.(*Forum), nil
+	if forum.Name == "" {
+		return nil, ErrNoRows
+	}
+	forum.Link = BuildForumURL(NameToSlug(forum.Name), forum.ID)
+	forum.LastTopic = Topics.DirtyGet(forum.LastTopicID)
+	forum.LastReplyer = Users.DirtyGet(forum.LastReplyerID)
+
+	mfs.CacheSet(forum)
+	return forum, err
 }
 
 func (mfs *MemoryForumStore) BypassGet(id int) (*Forum, error) {
@@ -274,7 +282,10 @@ func (mfs *MemoryForumStore) GetFirstChild(parentID int, parentType string) (*Fo
 // TODO: Add a query for this rather than hitting cache
 func (mfs *MemoryForumStore) Exists(id int) bool {
 	forum, ok := mfs.forums.Load(id)
-	return ok && forum.(*Forum).Name != ""
+	if !ok {
+		return false
+	}
+	return forum.(*Forum).Name != ""
 }
 
 // TODO: Batch deletions with name blanking? Is this necessary?
@@ -284,12 +295,12 @@ func (mfs *MemoryForumStore) CacheDelete(id int) {
 }
 
 // TODO: Add a hook to allow plugin_guilds to detect when one of it's forums has just been deleted?
-func (mfs *MemoryForumStore) Delete(id int) error {
+func (s *MemoryForumStore) Delete(id int) error {
 	if id == ReportForumID {
 		return errors.New("You cannot delete the Reports forum")
 	}
-	_, err := mfs.delete.Exec(id)
+	_, err := s.delete.Exec(id)
-	mfs.CacheDelete(id)
+	s.CacheDelete(id)
 	return err
 }
 
@@ -329,6 +340,9 @@ func (mfs *MemoryForumStore) UpdateLastTopic(tid int, uid int, fid int) error {
 }
 
 func (mfs *MemoryForumStore) Create(forumName string, forumDesc string, active bool, preset string) (int, error) {
+	if forumName == "" {
+		return 0, ErrBlankName
+	}
 	forumCreateMutex.Lock()
 	res, err := mfs.create.Exec(forumName, forumDesc, active, preset)
 	if err != nil {

misc_test.go

@@ -537,6 +537,9 @@ func TestForumStore(t *testing.T) {
 		c.InitPlugins()
 	}
 
+	expect(t, c.Forums.GlobalCount() == 2, "The forumstore global count should be 2")
+	//expect(t, c.Forums.Length() == 2, "The forumstore length should be 2")
+
 	_, err := c.Forums.Get(-1)
 	recordMustNotExist(t, err, "FID #-1 shouldn't exist")
 	_, err = c.Forums.Get(0)
@@ -560,22 +563,22 @@ func TestForumStore(t *testing.T) {
 	expectDesc = "A place for general discussions which don't fit elsewhere"
 	expect(t, forum.Desc == expectDesc, fmt.Sprintf("The forum description should be '%s' not '%s'", expectDesc, forum.Desc))
 
-	ok := c.Forums.Exists(-1)
+	expect(t, !c.Forums.Exists(-1), "FID #-1 shouldn't exist")
-	expect(t, !ok, "FID #-1 shouldn't exist")
+	expect(t, !c.Forums.Exists(0), "FID #0 shouldn't exist")
-	ok = c.Forums.Exists(0)
+	expect(t, c.Forums.Exists(1), "FID #1 should exist")
-	expect(t, !ok, "FID #0 shouldn't exist")
+	expect(t, c.Forums.Exists(2), "FID #2 should exist")
-	ok = c.Forums.Exists(1)
+	expect(t, !c.Forums.Exists(3), "FID #3 shouldn't exist")
-	expect(t, ok, "FID #1 should exist")
+
-	ok = c.Forums.Exists(2)
+	_, err = c.Forums.Create("", "", true, "all")
-	expect(t, ok, "FID #2 should exist")
+	expect(t, err != nil, "A forum shouldn't be successfully created, if it has a blank name")
-	ok = c.Forums.Exists(3)
-	expect(t, !ok, "FID #3 shouldn't exist")
 
 	fid, err := c.Forums.Create("Test Forum", "", true, "all")
 	expectNilErr(t, err)
 	expect(t, fid == 3, "The first forum we create should have an ID of 3")
-	ok = c.Forums.Exists(3)
+	expect(t, c.Forums.Exists(3), "FID #2 should exist")
-	expect(t, ok, "FID #2 should exist")
+
+	expect(t, c.Forums.GlobalCount() == 3, "The forumstore global count should be 3")
+	//expect(t, c.Forums.Length() == 3, "The forumstore length should be 3")
 
 	forum, err = c.Forums.Get(3)
 	recordMustExist(t, err, "Couldn't find FID #3")
@@ -586,8 +589,18 @@ func TestForumStore(t *testing.T) {
 	expect(t, forum.Desc == "", fmt.Sprintf("The forum description should be blank not '%s'", forum.Desc))
 
 	// TODO: More forum creation tests
-	// TODO: Test forum deletion
+
+	expectNilErr(t, c.Forums.Delete(3))
+	expect(t, forum.ID == 3, fmt.Sprintf("forum pointer shenanigans"))
+	expect(t, c.Forums.GlobalCount() == 2, "The forumstore global count should be 2")
+	//expect(t, c.Forums.Length() == 2, "The forumstore length should be 2")
+	expect(t, !c.Forums.Exists(3), "FID #3 shouldn't exist after being deleted")
+	_, err = c.Forums.Get(3)
+	recordMustNotExist(t, err, "FID #3 shouldn't exist after being deleted")
+
+	// TODO: Test deleting the reports forum
 	// TODO: Test forum update
+	// TODO: Other forumstore stuff and forumcache?
 }
 
 // TODO: Implement this