From 8a719bd13db941d7d03d0f01b0fb07729cc9226d Mon Sep 17 00:00:00 2001 From: Azareal Date: Sat, 18 May 2019 22:17:50 +1000 Subject: [PATCH] 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 | 41 +++++++++----------- misc_test.go | 87 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 101 insertions(+), 27 deletions(-) diff --git a/common/forum_store.go b/common/forum_store.go index 2e2f5da3..4f788915 100644 --- a/common/forum_store.go +++ b/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) { - fint, ok := mfs.forums.Load(id) +func (s *MemoryForumStore) Get(id int) (*Forum, error) { + 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} - 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, err := s.BypassGet(id) if err != nil { - return forum, err + 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) { - mfs.forums.Range(func(_ interface{}, value interface{}) bool { +func (s *MemoryForumStore) Length() (length int) { + s.forums.Range(func(_ interface{}, value interface{}) bool { length++ return true }) diff --git a/misc_test.go b/misc_test.go index fd53c3de..659f5cbf 100644 --- a/misc_test.go +++ b/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? }