From 54abbe980f7ccaa67f870a0f780e33ecf2affee3 Mon Sep 17 00:00:00 2001 From: Azareal Date: Tue, 18 Jun 2019 20:02:45 +1000 Subject: [PATCH] Add some test cases to avoid breaking CanSee with GroupStore.Reload again. Shorten some variable names. --- common/forum_perms_store.go | 40 +++++++++++++++++----------------- common/forum_store.go | 43 ++++++++++++++++++------------------- misc_test.go | 38 ++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 42 deletions(-) diff --git a/common/forum_perms_store.go b/common/forum_perms_store.go index bd111f83..7981dbc5 100644 --- a/common/forum_perms_store.go +++ b/common/forum_perms_store.go @@ -43,19 +43,19 @@ func NewMemoryForumPermsStore() (*MemoryForumPermsStore, error) { }, acc.FirstError() } -func (fps *MemoryForumPermsStore) Init() error { +func (s *MemoryForumPermsStore) Init() error { DebugLog("Initialising the forum perms store") - return fps.ReloadAll() + return s.ReloadAll() } -func (fps *MemoryForumPermsStore) ReloadAll() error { +func (s *MemoryForumPermsStore) ReloadAll() error { DebugLog("Reloading the forum perms") fids, err := Forums.GetAllIDs() if err != nil { return err } for _, fid := range fids { - err := fps.Reload(fid) + err := s.Reload(fid) if err != nil { return err } @@ -63,7 +63,7 @@ func (fps *MemoryForumPermsStore) ReloadAll() error { return nil } -func (fps *MemoryForumPermsStore) parseForumPerm(perms []byte) (pperms *ForumPerms, err error) { +func (s *MemoryForumPermsStore) parseForumPerm(perms []byte) (pperms *ForumPerms, err error) { DebugDetail("perms: ", string(perms)) pperms = BlankForumPerms() err = json.Unmarshal(perms, &pperms) @@ -73,9 +73,9 @@ func (fps *MemoryForumPermsStore) parseForumPerm(perms []byte) (pperms *ForumPer } // TODO: Need a more thread-safe way of doing this. Possibly with sync.Map? -func (fps *MemoryForumPermsStore) Reload(fid int) error { +func (s *MemoryForumPermsStore) Reload(fid int) error { DebugLogf("Reloading the forum permissions for forum #%d", fid) - rows, err := fps.getByForum.Query(fid) + rows, err := s.getByForum.Query(fid) if err != nil { return err } @@ -92,7 +92,7 @@ func (fps *MemoryForumPermsStore) Reload(fid int) error { DebugLog("gid: ", gid) DebugLogf("perms: %+v\n", perms) - pperms, err := fps.parseForumPerm(perms) + pperms, err := s.parseForumPerm(perms) if err != nil { return err } @@ -101,13 +101,13 @@ func (fps *MemoryForumPermsStore) Reload(fid int) error { } DebugLogf("forumPerms: %+v\n", forumPerms) if fid%2 == 0 { - fps.evenLock.Lock() - fps.evenForums[fid] = forumPerms - fps.evenLock.Unlock() + s.evenLock.Lock() + s.evenForums[fid] = forumPerms + s.evenLock.Unlock() } else { - fps.oddLock.Lock() - fps.oddForums[fid] = forumPerms - fps.oddLock.Unlock() + s.oddLock.Lock() + s.oddForums[fid] = forumPerms + s.oddLock.Unlock() } groups, err := Groups.GetAll() @@ -127,13 +127,13 @@ func (fps *MemoryForumPermsStore) Reload(fid int) error { var forumPerms map[int]*ForumPerms var ok bool if fid%2 == 0 { - fps.evenLock.RLock() - forumPerms, ok = fps.evenForums[fid] - fps.evenLock.RUnlock() + s.evenLock.RLock() + forumPerms, ok = s.evenForums[fid] + s.evenLock.RUnlock() } else { - fps.oddLock.RLock() - forumPerms, ok = fps.oddForums[fid] - fps.oddLock.RUnlock() + s.oddLock.RLock() + forumPerms, ok = s.oddForums[fid] + s.oddLock.RUnlock() } var forumPerm *ForumPerms diff --git a/common/forum_store.go b/common/forum_store.go index 49cb52e1..1cd469a5 100644 --- a/common/forum_store.go +++ b/common/forum_store.go @@ -92,16 +92,16 @@ func NewMemoryForumStore() (*MemoryForumStore, error) { // TODO: Rename to ReloadAll? // TODO: Add support for subforums -func (mfs *MemoryForumStore) LoadForums() error { +func (s *MemoryForumStore) LoadForums() error { var forumView []*Forum addForum := func(forum *Forum) { - mfs.forums.Store(forum.ID, forum) + s.forums.Store(forum.ID, forum) if forum.Active && forum.Name != "" && forum.ParentType == "" { forumView = append(forumView, forum) } } - rows, err := mfs.getAll.Query() + rows, err := s.getAll.Query() if err != nil { return err } @@ -109,25 +109,24 @@ func (mfs *MemoryForumStore) LoadForums() error { var i = 0 for ; rows.Next(); i++ { - forum := &Forum{ID: 0, Active: true, Preset: "all"} - err = rows.Scan(&forum.ID, &forum.Name, &forum.Desc, &forum.Tmpl, &forum.Active, &forum.Order, &forum.Preset, &forum.ParentID, &forum.ParentType, &forum.TopicCount, &forum.LastTopicID, &forum.LastReplyerID) + f := &Forum{ID: 0, Active: true, Preset: "all"} + err = rows.Scan(&f.ID, &f.Name, &f.Desc, &f.Tmpl, &f.Active, &f.Order, &f.Preset, &f.ParentID, &f.ParentType, &f.TopicCount, &f.LastTopicID, &f.LastReplyerID) if err != nil { return err } - if forum.Name == "" { + if f.Name == "" { DebugLog("Adding a placeholder forum") } else { - log.Printf("Adding the '%s' forum", forum.Name) + log.Printf("Adding the '%s' forum", f.Name) } - forum.Link = BuildForumURL(NameToSlug(forum.Name), forum.ID) - forum.LastTopic = Topics.DirtyGet(forum.LastTopicID) - forum.LastReplyer = Users.DirtyGet(forum.LastReplyerID) - - addForum(forum) + f.Link = BuildForumURL(NameToSlug(f.Name), f.ID) + f.LastTopic = Topics.DirtyGet(f.LastTopicID) + f.LastReplyer = Users.DirtyGet(f.LastReplyerID) + addForum(f) } - mfs.forumView.Store(forumView) + s.forumView.Store(forumView) TopicListThaw.Thaw() return rows.Err() } @@ -184,20 +183,20 @@ func (s *MemoryForumStore) Get(id int) (*Forum, error) { } func (s *MemoryForumStore) BypassGet(id int) (*Forum, error) { - var forum = &Forum{ID: id} - err := s.get.QueryRow(id).Scan(&forum.Name, &forum.Desc, &forum.Tmpl,&forum.Active, &forum.Order, &forum.Preset, &forum.ParentID, &forum.ParentType, &forum.TopicCount, &forum.LastTopicID, &forum.LastReplyerID) + var f = &Forum{ID: id} + err := s.get.QueryRow(id).Scan(&f.Name, &f.Desc, &f.Tmpl,&f.Active, &f.Order, &f.Preset, &f.ParentID, &f.ParentType, &f.TopicCount, &f.LastTopicID, &f.LastReplyerID) if err != nil { return nil, err } - if forum.Name == "" { + if f.Name == "" { return nil, ErrNoRows } - forum.Link = BuildForumURL(NameToSlug(forum.Name), forum.ID) - forum.LastTopic = Topics.DirtyGet(forum.LastTopicID) - forum.LastReplyer = Users.DirtyGet(forum.LastReplyerID) + f.Link = BuildForumURL(NameToSlug(f.Name), f.ID) + f.LastTopic = Topics.DirtyGet(f.LastTopicID) + f.LastReplyer = Users.DirtyGet(f.LastReplyerID) //TopicListThaw.Thaw() - return forum, err + return f, err } // TODO: Optimise this @@ -263,10 +262,10 @@ func (s *MemoryForumStore) GetAllVisibleIDs() ([]int, error) { } // TODO: Implement sub-forums. -/*func (mfs *MemoryForumStore) GetChildren(parentID int, parentType string) ([]*Forum,error) { +/*func (s *MemoryForumStore) GetChildren(parentID int, parentType string) ([]*Forum,error) { return nil, nil } -func (mfs *MemoryForumStore) GetFirstChild(parentID int, parentType string) (*Forum,error) { +func (s *MemoryForumStore) GetFirstChild(parentID int, parentType string) (*Forum,error) { return nil, nil }*/ diff --git a/misc_test.go b/misc_test.go index fdd6d2f8..6c97469c 100644 --- a/misc_test.go +++ b/misc_test.go @@ -8,6 +8,7 @@ import ( "strconv" "testing" "time" + "database/sql" c "github.com/Azareal/Gosora/common" "github.com/Azareal/Gosora/common/phrases" @@ -752,6 +753,7 @@ func TestGroupStore(t *testing.T) { group, 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, !c.Groups.Exists(-1), "GID #-1 shouldn't exist") // 0 aka Unknown, for system posts and other oddities @@ -771,6 +773,7 @@ func TestGroupStore(t *testing.T) { 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") + // TODO: Add a proper CanSee test isAdmin = false isMod = true @@ -806,16 +809,33 @@ func TestGroupStore(t *testing.T) { 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") err = group.ChangeRank(false, true, true) expectNilErr(t, err) + forum, err := c.Forums.Get(2) + expectNilErr(t, err) + forumPerms, err := c.FPStore.GetCopy(2, gid) + if err == sql.ErrNoRows { + forumPerms = *c.BlankForumPerms() + } else if err != nil { + expectNilErr(t, err) + } + forumPerms.ViewTopic = true + + err = forum.SetPerms(&forumPerms, "custom", gid) + expectNilErr(t, err) + group, 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) > 0, "group.CanSee should not be zero") + canSee := group.CanSee // Make sure the data is static c.Groups.Reload(gid) @@ -827,6 +847,24 @@ func TestGroupStore(t *testing.T) { expect(t, group.IsMod, "This should be a mod group") expect(t, !group.IsBanned, "This shouldn't be a ban group") + // TODO: Don't enforce a specific order here + canSeeTest := func(a, b []int) bool { + if (a == nil) != (b == nil) { + return false + } + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true + } + + expect(t, canSeeTest(group.CanSee,canSee), "group.CanSee is not being reused") + // TODO: Test group deletion // TODO: Test group reload // TODO: Test group cache set