a
/
gosora
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix a bug where reloaded groups lost their CanSee data. 

Remove a couple of redundant queries in group reload.
Exit from group reload early if there's an error.
Remove a couple of redundant assignments in the forum list.
Shorten some variable names.
This commit is contained in:
Azareal 2019-06-18 14:13:47 +10:00
parent 89ca5ee823
commit 97fb4af723
6 changed files with 74 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
common/forum_perms_store.go
View File

@ -166,35 +166,35 @@ func (fps *MemoryForumPermsStore) Reload(fid int) error {
} }
// ! Throughput on this might be bad due to the excessive locking // ! Throughput on this might be bad due to the excessive locking
func (fps *MemoryForumPermsStore) GetAllMap() (bigMap map[int]map[int]*ForumPerms) { func (s *MemoryForumPermsStore) GetAllMap() (bigMap map[int]map[int]*ForumPerms) {
bigMap = make(map[int]map[int]*ForumPerms) bigMap = make(map[int]map[int]*ForumPerms)
fps.evenLock.RLock() s.evenLock.RLock()
for fid, subMap := range fps.evenForums { for fid, subMap := range s.evenForums {
bigMap[fid] = subMap bigMap[fid] = subMap
} }
fps.evenLock.RUnlock() s.evenLock.RUnlock()
fps.oddLock.RLock() s.oddLock.RLock()
for fid, subMap := range fps.oddForums { for fid, subMap := range s.oddForums {
bigMap[fid] = subMap bigMap[fid] = subMap
} }
fps.oddLock.RUnlock() s.oddLock.RUnlock()
return bigMap return bigMap
} }
// TODO: Add a hook here and have plugin_guilds use it // TODO: Add a hook here and have plugin_guilds use it
// TODO: Check if the forum exists? // TODO: Check if the forum exists?
// TODO: Fix the races // TODO: Fix the races
func (fps *MemoryForumPermsStore) Get(fid int, gid int) (fperms *ForumPerms, err error) { func (s *MemoryForumPermsStore) Get(fid int, gid int) (fperms *ForumPerms, err error) {
var fmap map[int]*ForumPerms var fmap map[int]*ForumPerms
var ok bool var ok bool
if fid%2 == 0 { if fid%2 == 0 {
fps.evenLock.RLock() s.evenLock.RLock()
fmap, ok = fps.evenForums[fid] fmap, ok = s.evenForums[fid]
fps.evenLock.RUnlock() s.evenLock.RUnlock()
} else { } else {
fps.oddLock.RLock() s.oddLock.RLock()
fmap, ok = fps.oddForums[fid] fmap, ok = s.oddForums[fid]
fps.oddLock.RUnlock() s.oddLock.RUnlock()
} }
if !ok { if !ok {
return fperms, ErrNoRows return fperms, ErrNoRows
@ -209,8 +209,8 @@ func (fps *MemoryForumPermsStore) Get(fid int, gid int) (fperms *ForumPerms, err
// TODO: Check if the forum exists? // TODO: Check if the forum exists?
// TODO: Fix the races // TODO: Fix the races
func (fps *MemoryForumPermsStore) GetCopy(fid int, gid int) (fperms ForumPerms, err error) { func (s *MemoryForumPermsStore) GetCopy(fid int, gid int) (fperms ForumPerms, err error) {
fPermsPtr, err := fps.Get(fid, gid) fPermsPtr, err := s.Get(fid, gid)
if err != nil { if err != nil {
return fperms, err return fperms, err
} }

92
common/group_store.go
View File

@ -8,6 +8,7 @@ import (
"log" "log"
"sort" "sort"
"sync" "sync"
"strconv"
"github.com/Azareal/Gosora/query_gen" "github.com/Azareal/Gosora/query_gen"
) )
@ -95,8 +96,8 @@ func (mgs *MemoryGroupStore) LoadGroups() error {
} }
// TODO: Hit the database when the item isn't in memory // TODO: Hit the database when the item isn't in memory
func (mgs *MemoryGroupStore) dirtyGetUnsafe(gid int) *Group { func (s *MemoryGroupStore) dirtyGetUnsafe(gid int) *Group {
group, ok := mgs.groups[gid] group, ok := s.groups[gid]
if !ok { if !ok {
return &blankGroup return &blankGroup
} }
@ -104,10 +105,10 @@ func (mgs *MemoryGroupStore) dirtyGetUnsafe(gid int) *Group {
} }
// TODO: Hit the database when the item isn't in memory // TODO: Hit the database when the item isn't in memory
func (mgs *MemoryGroupStore) DirtyGet(gid int) *Group { func (s *MemoryGroupStore) DirtyGet(gid int) *Group {
mgs.RLock() s.RLock()
group, ok := mgs.groups[gid] group, ok := s.groups[gid]
mgs.RUnlock() s.RUnlock()
if !ok { if !ok {
return &blankGroup return &blankGroup
} }
@ -115,10 +116,10 @@ func (mgs *MemoryGroupStore) DirtyGet(gid int) *Group {
} }
// TODO: Hit the database when the item isn't in memory // TODO: Hit the database when the item isn't in memory
func (mgs *MemoryGroupStore) Get(gid int) (*Group, error) { func (s *MemoryGroupStore) Get(gid int) (*Group, error) {
mgs.RLock() s.RLock()
group, ok := mgs.groups[gid] group, ok := s.groups[gid]
mgs.RUnlock() s.RUnlock()
if !ok { if !ok {
return nil, ErrNoRows return nil, ErrNoRows
} }
@ -126,38 +127,49 @@ func (mgs *MemoryGroupStore) Get(gid int) (*Group, error) {
} }
// TODO: Hit the database when the item isn't in memory // TODO: Hit the database when the item isn't in memory
func (mgs *MemoryGroupStore) GetCopy(gid int) (Group, error) { func (s *MemoryGroupStore) GetCopy(gid int) (Group, error) {
mgs.RLock() s.RLock()
group, ok := mgs.groups[gid] group, ok := s.groups[gid]
mgs.RUnlock() s.RUnlock()
if !ok { if !ok {
return blankGroup, ErrNoRows return blankGroup, ErrNoRows
} }
return *group, nil return *group, nil
} }
func (mgs *MemoryGroupStore) Reload(id int) error { func (s *MemoryGroupStore) Reload(id int) error {
var group = &Group{ID: id} // TODO: Reload this data too
err := mgs.get.QueryRow(id).Scan(&group.Name, &group.PermissionsText, &group.PluginPermsText, &group.IsMod, &group.IsAdmin, &group.IsBanned, &group.Tag) group, err := s.Get(id)
if err != nil {
LogError(errors.New("can't get cansee data for group #" + strconv.Itoa(id)))
return nil
}
canSee := group.CanSee
group = &Group{ID: id, CanSee: canSee}
err = s.get.QueryRow(id).Scan(&group.Name, &group.PermissionsText, &group.PluginPermsText, &group.IsMod, &group.IsAdmin, &group.IsBanned, &group.Tag)
if err != nil { if err != nil {
return err return err
} }
err = mgs.initGroup(group) err = s.initGroup(group)
if err != nil { if err != nil {
LogError(err) LogError(err)
return nil
} }
mgs.CacheSet(group)
err = RebuildGroupPermissions(id) // TODO: Do we really need this?
/*err = RebuildGroupPermissions(group)
if err != nil { if err != nil {
LogError(err) LogError(err)
} return nil
}*/
s.CacheSet(group)
TopicListThaw.Thaw() TopicListThaw.Thaw()
return nil return nil
} }
func (mgs *MemoryGroupStore) initGroup(group *Group) error { func (s *MemoryGroupStore) initGroup(group *Group) error {
err := json.Unmarshal(group.PermissionsText, &group.Perms) err := json.Unmarshal(group.PermissionsText, &group.Perms)
if err != nil { if err != nil {
log.Printf("group: %+v\n", group) log.Printf("group: %+v\n", group)
@ -180,25 +192,25 @@ func (mgs *MemoryGroupStore) initGroup(group *Group) error {
group.IsBanned = false group.IsBanned = false
} }
err = mgs.userCount.QueryRow(group.ID).Scan(&group.UserCount) err = s.userCount.QueryRow(group.ID).Scan(&group.UserCount)
if err != sql.ErrNoRows { if err != sql.ErrNoRows {
return err return err
} }
return nil return nil
} }
func (mgs *MemoryGroupStore) CacheSet(group *Group) error { func (s *MemoryGroupStore) CacheSet(group *Group) error {
mgs.Lock() s.Lock()
mgs.groups[group.ID] = group s.groups[group.ID] = group
mgs.Unlock() s.Unlock()
return nil return nil
} }
// TODO: Hit the database when the item isn't in memory // TODO: Hit the database when the item isn't in memory
func (mgs *MemoryGroupStore) Exists(gid int) bool { func (s *MemoryGroupStore) Exists(gid int) bool {
mgs.RLock() s.RLock()
group, ok := mgs.groups[gid] group, ok := s.groups[gid]
mgs.RUnlock() s.RUnlock()
return ok && group.Name != "" return ok && group.Name != ""
} }
@ -287,23 +299,23 @@ func (mgs *MemoryGroupStore) Create(name string, tag string, isAdmin bool, isMod
//return gid, TopicList.RebuildPermTree() //return gid, TopicList.RebuildPermTree()
} }
func (mgs *MemoryGroupStore) GetAll() (results []*Group, err error) { func (s *MemoryGroupStore) GetAll() (results []*Group, err error) {
var i int var i int
mgs.RLock() s.RLock()
results = make([]*Group, len(mgs.groups)) results = make([]*Group, len(s.groups))
for _, group := range mgs.groups { for _, group := range s.groups {
results[i] = group results[i] = group
i++ i++
} }
mgs.RUnlock() s.RUnlock()
sort.Sort(SortGroup(results)) sort.Sort(SortGroup(results))
return results, nil return results, nil
} }
func (mgs *MemoryGroupStore) GetAllMap() (map[int]*Group, error) { func (s *MemoryGroupStore) GetAllMap() (map[int]*Group, error) {
mgs.RLock() s.RLock()
defer mgs.RUnlock() defer s.RUnlock()
return mgs.groups, nil return s.groups, nil
} }
// ? - Set the lower and higher numbers to 0 to remove the bounds // ? - Set the lower and higher numbers to 0 to remove the bounds

9
common/permissions.go
View File

@ -171,7 +171,7 @@ func PresetToLang(preset string) string {
// TODO: Is this racey? // TODO: Is this racey?
// TODO: Test this along with the rest of the perms system // TODO: Test this along with the rest of the perms system
func RebuildGroupPermissions(gid int) error { func RebuildGroupPermissions(group *Group) error {
var permstr []byte var permstr []byte
log.Print("Reloading a group") log.Print("Reloading a group")
@ -182,7 +182,7 @@ func RebuildGroupPermissions(gid int) error {
} }
defer getGroupPerms.Close() defer getGroupPerms.Close()
err = getGroupPerms.QueryRow(gid).Scan(&permstr) err = getGroupPerms.QueryRow(group.ID).Scan(&permstr)
if err != nil { if err != nil {
return err return err
} }
@ -194,11 +194,6 @@ func RebuildGroupPermissions(gid int) error {
if err != nil { if err != nil {
return err return err
} }
group, err := Groups.Get(gid)
if err != nil {
return err
}
group.Perms = tmpPerms group.Perms = tmpPerms
return nil return nil
} }

6
routes/forum_list.go
View File

@ -20,7 +20,6 @@ func ForumList(w http.ResponseWriter, r *http.Request, user c.User, header *c.He
header.MetaDesc = header.Settings["meta_desc"].(string) header.MetaDesc = header.Settings["meta_desc"].(string)
var err error var err error
var forumList []c.Forum
var canSee []int var canSee []int
if user.IsSuperAdmin { if user.IsSuperAdmin {
canSee, err = c.Forums.GetAllVisibleIDs() canSee, err = c.Forums.GetAllVisibleIDs()
@ -36,6 +35,7 @@ func ForumList(w http.ResponseWriter, r *http.Request, user c.User, header *c.He
canSee = group.CanSee canSee = group.CanSee
} }
var forumList []c.Forum
for _, fid := range canSee { for _, fid := range canSee {
// Avoid data races by copying the struct into something we can freely mold without worrying about breaking something somewhere else // Avoid data races by copying the struct into something we can freely mold without worrying about breaking something somewhere else
var forum = c.Forums.DirtyGet(fid).Copy() var forum = c.Forums.DirtyGet(fid).Copy()
@ -43,11 +43,7 @@ func ForumList(w http.ResponseWriter, r *http.Request, user c.User, header *c.He
if forum.LastTopicID != 0 { if forum.LastTopicID != 0 {
if forum.LastTopic.ID != 0 && forum.LastReplyer.ID != 0 { if forum.LastTopic.ID != 0 && forum.LastReplyer.ID != 0 {
forum.LastTopicTime = c.RelativeTime(forum.LastTopic.LastReplyAt) forum.LastTopicTime = c.RelativeTime(forum.LastTopic.LastReplyAt)
} else {
forum.LastTopicTime = ""
} }
} else {
forum.LastTopicTime = ""
} }
header.Hooks.Hook("forums_frow_assign", &forum) header.Hooks.Hook("forums_frow_assign", &forum)
forumList = append(forumList, forum) forumList = append(forumList, forum)

3
routes/panel/groups.go
View File

@ -131,7 +131,6 @@ func GroupsEditPerms(w http.ResponseWriter, r *http.Request, user c.User, sgid s
} else if err != nil { } else if err != nil {
return c.InternalError(err, w, r) return c.InternalError(err, w, r)
} }
if group.IsAdmin && !user.Perms.EditGroupAdmin { if group.IsAdmin && !user.Perms.EditGroupAdmin {
return c.LocalError(phrases.GetErrorPhrase("panel_groups_cannot_edit_admin"), w, r, user) return c.LocalError(phrases.GetErrorPhrase("panel_groups_cannot_edit_admin"), w, r, user)
} }
@ -211,7 +210,6 @@ func GroupsEditSubmit(w http.ResponseWriter, r *http.Request, user c.User, sgid
} else if err != nil { } else if err != nil {
return c.InternalError(err, w, r) return c.InternalError(err, w, r)
} }
if group.IsAdmin && !user.Perms.EditGroupAdmin { if group.IsAdmin && !user.Perms.EditGroupAdmin {
return c.LocalError(phrases.GetErrorPhrase("panel_groups_cannot_edit_admin"), w, r, user) return c.LocalError(phrases.GetErrorPhrase("panel_groups_cannot_edit_admin"), w, r, user)
} }
@ -300,7 +298,6 @@ func GroupsEditPermsSubmit(w http.ResponseWriter, r *http.Request, user c.User,
} else if err != nil { } else if err != nil {
return c.InternalError(err, w, r) return c.InternalError(err, w, r)
} }
if group.IsAdmin && !user.Perms.EditGroupAdmin { if group.IsAdmin && !user.Perms.EditGroupAdmin {
return c.LocalError(phrases.GetErrorPhrase("panel_groups_cannot_edit_admin"), w, r, user) return c.LocalError(phrases.GetErrorPhrase("panel_groups_cannot_edit_admin"), w, r, user)
} }

2
templates/topic.html
View File

@ -94,7 +94,7 @@
<button form="quick_post_form" name="reply-button" class="formbutton">{{lang "topic.reply_button"}}</button> <button form="quick_post_form" name="reply-button" class="formbutton">{{lang "topic.reply_button"}}</button>
<button form="quick_post_form" class="formbutton" id="add_poll_button">{{lang "topic.reply_add_poll_button"}}</button> <button form="quick_post_form" class="formbutton" id="add_poll_button">{{lang "topic.reply_add_poll_button"}}</button>
{{if .CurrentUser.Perms.UploadFiles}} {{if .CurrentUser.Perms.UploadFiles}}
<input name="upload_files" form="quick_post_form" id="upload_files" multiple type="file" style="display: none;" /> <input name="upload_files" form="quick_post_form" id="upload_files" multiple type="file" style="display:none;" />
<label for="upload_files" class="formbutton add_file_button">{{lang "topic.reply_add_file_button"}}</label> <label for="upload_files" class="formbutton add_file_button">{{lang "topic.reply_add_file_button"}}</label>
<div id="upload_file_dock"></div>{{end}} <div id="upload_file_dock"></div>{{end}}
</div> </div>