Added several more ForumStore tests.

Refactor ForumStore.Get to use ForumStore.Get as a fallback rather than implementing the fallback itself.
ForumStore.BypassGet now handles deleted forums correctly.
Added ErrNoDeleteReports to make it easier to check for that error.

common/forum_store.go

@@ -21,6 +21,7 @@ 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")
+var ErrNoDeleteReports = errors.New("You cannot delete the Reports forum")
 
 // ForumStore is an interface for accessing the forums and the metadata stored on them
 type ForumStore interface {
@@ -163,8 +164,8 @@ func (mfs *MemoryForumStore) CacheGet(id int) (*Forum, error) {
 	return fint.(*Forum), nil
 }
 
-func (mfs *MemoryForumStore) Get(id int) (*Forum, error) {
+func (s *MemoryForumStore) Get(id int) (*Forum, error) {
-	fint, ok := mfs.forums.Load(id)
+	fint, ok := s.forums.Load(id)
 	if ok {
 		forum := fint.(*Forum)
 		if forum.Name == "" {
@@ -173,29 +174,23 @@ func (mfs *MemoryForumStore) Get(id int) (*Forum, error) {
 		return forum, nil
 	}
 
-	var forum = &Forum{ID: id}
+	forum, err := s.BypassGet(id)
-	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 err != nil {
+		return nil, err
+	}
+	s.CacheSet(forum)
 	return forum, err
 }
+
+func (s *MemoryForumStore) BypassGet(id int) (*Forum, error) {
+	var forum = &Forum{ID: id}
+	err := s.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 err != nil {
+		return nil, err
+	}
 	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) {
-	var forum = &Forum{ID: id}
-	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 err != nil {
-		return nil, err
-	}
-
 	forum.Link = BuildForumURL(NameToSlug(forum.Name), forum.ID)
 	forum.LastTopic = Topics.DirtyGet(forum.LastTopicID)
 	forum.LastReplyer = Users.DirtyGet(forum.LastReplyerID)
@@ -297,7 +292,7 @@ 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 (s *MemoryForumStore) Delete(id int) error {
 	if id == ReportForumID {
-		return errors.New("You cannot delete the Reports forum")
+		return ErrNoDeleteReports
 	}
 	_, err := s.delete.Exec(id)
 	s.CacheDelete(id)
@@ -378,8 +373,8 @@ func (s *MemoryForumStore) UpdateOrder(updateMap map[int]int) error {
 
 // ! Might be slightly inaccurate, if the sync.Map is constantly shifting and churning, but it'll stabilise eventually. Also, slow. Don't use this on every request x.x
 // Length returns the number of forums in the memory cache
-func (mfs *MemoryForumStore) Length() (length int) {
+func (s *MemoryForumStore) Length() (length int) {
-	mfs.forums.Range(func(_ interface{}, value interface{}) bool {
+	s.forums.Range(func(_ interface{}, value interface{}) bool {
 		length++
 		return true
 	})

misc_test.go

@@ -537,8 +537,11 @@ func TestForumStore(t *testing.T) {
 		c.InitPlugins()
 	}
 
+	fcache, ok := c.Forums.(c.ForumCache)
+	expect(t, ok, "Unable to cast ForumStore to ForumCache")
+
 	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, fcache.Length() == 2, "The forum cache length should be 2")
 
 	_, err := c.Forums.Get(-1)
 	recordMustNotExist(t, err, "FID #-1 shouldn't exist")
@@ -553,9 +556,13 @@ func TestForumStore(t *testing.T) {
 	expect(t, !forum.Active, fmt.Sprintf("The reports forum shouldn't be active"))
 	var expectDesc = "All the reports go here"
 	expect(t, forum.Desc == expectDesc, fmt.Sprintf("The forum description should be '%s' not '%s'", expectDesc, forum.Desc))
+	forum, err = c.Forums.BypassGet(1)
+	recordMustExist(t, err, "Couldn't find FID #1")
 
 	forum, err = c.Forums.Get(2)
 	recordMustExist(t, err, "Couldn't find FID #2")
+	forum, err = c.Forums.BypassGet(2)
+	recordMustExist(t, err, "Couldn't find FID #2")
 
 	expect(t, forum.ID == 2, fmt.Sprintf("The FID should be 2 not %d", forum.ID))
 	expect(t, forum.Name == "General", fmt.Sprintf("The name of the forum should be 'General' not '%s'", forum.Name))
@@ -578,10 +585,12 @@ func TestForumStore(t *testing.T) {
 	expect(t, c.Forums.Exists(3), "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")
+	expect(t, fcache.Length() == 3, "The forum cache length should be 3")
 
 	forum, err = c.Forums.Get(3)
 	recordMustExist(t, err, "Couldn't find FID #3")
+	forum, err = c.Forums.BypassGet(3)
+	recordMustExist(t, err, "Couldn't find FID #3")
 
 	expect(t, forum.ID == 3, fmt.Sprintf("The FID should be 3 not %d", forum.ID))
 	expect(t, forum.Name == "Test Forum", fmt.Sprintf("The name of the forum should be 'Test Forum' not '%s'", forum.Name))
@@ -593,12 +602,82 @@ func TestForumStore(t *testing.T) {
 	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, fcache.Length() == 2, "The forum cache 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")
+	_, err = c.Forums.BypassGet(3)
+	recordMustNotExist(t, err, "FID #3 shouldn't exist after being deleted")
+
+	expect(t, c.Forums.Delete(c.ReportForumID) != nil, "The reports forum shouldn't be deletable")
+	expect(t, c.Forums.Exists(c.ReportForumID), fmt.Sprintf("FID #%d should still exist", c.ReportForumID))
+	_, err = c.Forums.Get(c.ReportForumID)
+	expect(t, err == nil, fmt.Sprintf("FID #%d should still exist", c.ReportForumID))
+	_, err = c.Forums.BypassGet(c.ReportForumID)
+	expect(t, err == nil, fmt.Sprintf("FID #%d should still exist", c.ReportForumID))
+
+	eforums := map[int]bool{1: true, 2: true}
+	{
+		forums, err := c.Forums.GetAll()
+		expectNilErr(t, err)
+		found := make(map[int]*c.Forum)
+		for _, forum := range forums {
+			_, ok := eforums[forum.ID]
+			expect(t, ok, fmt.Sprintf("unknown forum #%d in forums", forum.ID))
+			found[forum.ID] = forum
+		}
+		for fid, _ := range eforums {
+			_, ok := found[fid]
+			expect(t, ok, fmt.Sprintf("unable to find expected forum #%d in forums", fid))
+		}
+	}
+
+	{
+		fids, err := c.Forums.GetAllIDs()
+		expectNilErr(t, err)
+		found := make(map[int]bool)
+		for _, fid := range fids {
+			_, ok := eforums[fid]
+			expect(t, ok, fmt.Sprintf("unknown fid #%d in fids", fid))
+			found[fid] = true
+		}
+		for fid, _ := range eforums {
+			_, ok := found[fid]
+			expect(t, ok, fmt.Sprintf("unable to find expected fid #%d in fids", fid))
+		}
+	}
+
+	vforums := map[int]bool{2: true}
+	{
+		forums, err := c.Forums.GetAllVisible()
+		expectNilErr(t, err)
+		found := make(map[int]*c.Forum)
+		for _, forum := range forums {
+			_, ok := vforums[forum.ID]
+			expect(t, ok, fmt.Sprintf("unknown forum #%d in forums", forum.ID))
+			found[forum.ID] = forum
+		}
+		for fid, _ := range vforums {
+			_, ok := found[fid]
+			expect(t, ok, fmt.Sprintf("unable to find expected forum #%d in forums", fid))
+		}
+	}
+
+	{
+		fids, err := c.Forums.GetAllVisibleIDs()
+		expectNilErr(t, err)
+		found := make(map[int]bool)
+		for _, fid := range fids {
+			_, ok := vforums[fid]
+			expect(t, ok, fmt.Sprintf("unknown fid #%d in fids", fid))
+			found[fid] = true
+		}
+		for fid, _ := range vforums {
+			_, ok := found[fid]
+			expect(t, ok, fmt.Sprintf("unable to find expected fid #%d in fids", fid))
+		}
+	}
 
-	// TODO: Test deleting the reports forum
 	// TODO: Test forum update
 	// TODO: Other forumstore stuff and forumcache?
 }