Better efficency for loading teams (#128)

Fix staticcheck

Better performance for getting teams on a namespace

Better performance for getting teams on a list

Fix lint

Fix swagger

Signed-off-by: kolaente <k@knt.li>

Make loading a single full team more efficent

Signed-off-by: kolaente <k@knt.li>

Make loading teams more efficent

Signed-off-by: kolaente <k@knt.li>

Co-authored-by: kolaente <k@knt.li>
Reviewed-on: vikunja/api#128
This commit is contained in:
konrad 2020-01-27 17:28:17 +00:00
parent 2abb858859
commit f603b41d99
10 changed files with 124 additions and 47 deletions

1
go.mod
View File

@ -58,6 +58,7 @@ require (
github.com/onsi/gomega v1.4.3 // indirect
github.com/op/go-logging v0.0.0-20160315200505-970db520ece7
github.com/pelletier/go-toml v1.4.0 // indirect
github.com/pkg/errors v0.8.1 // indirect
github.com/prometheus/client_golang v0.9.2
github.com/samedi/caldav-go v3.0.0+incompatible
github.com/shurcooL/httpfs v0.0.0-20190527155220-6a4d4a70508b

View File

@ -186,6 +186,16 @@ func (tl *TeamList) ReadAll(a web.Auth, search string, page int, perPage int) (r
return nil, 0, 0, err
}
teams := []*Team{}
for _, t := range all {
teams = append(teams, &t.Team)
}
err = addMoreInfoToTeams(teams)
if err != nil {
return
}
totalItems, err = x.
Table("teams").
Join("INNER", "team_list", "team_id = teams.id").

View File

@ -171,6 +171,16 @@ func (tn *TeamNamespace) ReadAll(a web.Auth, search string, page int, perPage in
return nil, 0, 0, err
}
teams := []*Team{}
for _, t := range all {
teams = append(teams, &t.Team)
}
err = addMoreInfoToTeams(teams)
if err != nil {
return
}
numberOfTotalItems, err = x.Table("teams").
Join("INNER", "team_namespaces", "team_id = teams.id").
Where("team_namespaces.namespace_id = ?", tn.NamespaceID).

View File

@ -236,7 +236,7 @@ func (t *Task) addNewAssigneeByID(newAssigneeID int64, list *List) (err error) {
// @Param s query string false "Search assignees by their username."
// @Param taskID path int true "Task ID"
// @Security JWTKeyAuth
// @Success 200 {array} models.User "The assignees"
// @Success 200 {array} user.User "The assignees"
// @Failure 500 {object} models.Message "Internal error"
// @Router /tasks/{taskID}/assignees [get]
func (la *TaskAssginee) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) {

View File

@ -20,6 +20,7 @@ import (
"code.vikunja.io/api/pkg/metrics"
"code.vikunja.io/api/pkg/user"
"code.vikunja.io/web"
"github.com/go-xorm/builder"
)
// Team holds a team object
@ -53,15 +54,6 @@ func (Team) TableName() string {
// AfterLoad gets the created by user object
func (t *Team) AfterLoad() {
// Get the owner
t.CreatedBy, _ = user.GetUserByID(t.CreatedByID)
// Get all members
x.Select("*").
Table("users").
Join("INNER", "team_members", "team_members.user_id = users.id").
Where("team_id = ?", t.ID).
Find(&t.Members)
}
// TeamMember defines the relationship between a user and a team
@ -93,23 +85,76 @@ func (TeamMember) TableName() string {
type TeamUser struct {
user.User `xorm:"extends"`
// Whether or not the member is an admin of the team. See the docs for more about what a team admin can do
Admin bool `json:"admin"`
Admin bool `json:"admin"`
TeamID int64 `json:"-"`
}
// GetTeamByID gets a team by its ID
func GetTeamByID(id int64) (team Team, err error) {
func GetTeamByID(id int64) (team *Team, err error) {
if id < 1 {
return team, ErrTeamDoesNotExist{id}
}
exists, err := x.Where("id = ?", id).Get(&team)
t := Team{}
exists, err := x.
Where("id = ?", id).
Get(&t)
if err != nil {
return
}
if !exists {
return team, ErrTeamDoesNotExist{id}
return &t, ErrTeamDoesNotExist{id}
}
teamSlice := []*Team{&t}
err = addMoreInfoToTeams(teamSlice)
if err != nil {
return
}
team = &t
return
}
func addMoreInfoToTeams(teams []*Team) (err error) {
// Put the teams in a map to make assigning more info to it more efficient
teamMap := make(map[int64]*Team, len(teams))
var teamIDs []int64
var ownerIDs []int64
for _, team := range teams {
teamMap[team.ID] = team
teamIDs = append(teamIDs, team.ID)
ownerIDs = append(ownerIDs, team.CreatedByID)
}
// Get all owners and team members
users := []*TeamUser{}
err = x.Select("*").
Table("users").
Join("LEFT", "team_members", "team_members.user_id = users.id").
Join("LEFT", "teams", "team_members.team_id = teams.id").
Or(
builder.In("team_id", teamIDs),
builder.And(
builder.In("users.id", ownerIDs),
builder.Expr("teams.created_by_id = users.id"),
builder.In("teams.id", teamIDs),
),
).
Find(&users)
if err != nil {
return
}
for _, u := range users {
if _, exists := teamMap[u.TeamID]; !exists {
continue
}
u.Email = ""
teamMap[u.TeamID].CreatedBy = &u.User
teamMap[u.TeamID].Members = append(teamMap[u.TeamID].Members, u)
}
return
}
@ -124,9 +169,12 @@ func GetTeamByID(id int64) (team Team, err error) {
// @Success 200 {object} models.Team "The team"
// @Failure 403 {object} code.vikunja.io/web.HTTPError "The user does not have access to the team"
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id} [get]
// @Router /teams/{id} [get]
func (t *Team) ReadOne() (err error) {
*t, err = GetTeamByID(t.ID)
team, err := GetTeamByID(t.ID)
if team != nil {
*t = *team
}
return
}
@ -160,6 +208,11 @@ func (t *Team) ReadAll(a web.Auth, search string, page int, perPage int) (result
return nil, 0, 0, err
}
err = addMoreInfoToTeams(all)
if err != nil {
return nil, 0, 0, err
}
numberOfTotalItems, err = x.
Table("teams").
Join("INNER", "team_members", "team_members.team_id = teams.id").
@ -282,7 +335,10 @@ func (t *Team) Update() (err error) {
}
// Get the newly updated team
*t, err = GetTeamByID(t.ID)
team, err := GetTeamByID(t.ID)
if team != nil {
*t = *team
}
return
}

View File

@ -36,7 +36,7 @@ type Token struct {
// @tags user
// @Accept json
// @Produce json
// @Param credentials body models.UserLogin true "The login credentials"
// @Param credentials body user.UserLogin true "The login credentials"
// @Success 200 {object} v1.Token
// @Failure 400 {object} models.Message "Invalid user password model."
// @Failure 403 {object} models.Message "Invalid username or password."

View File

@ -31,8 +31,8 @@ import (
// @tags user
// @Accept json
// @Produce json
// @Param credentials body models.APIUserPassword true "The user credentials"
// @Success 200 {object} models.User
// @Param credentials body user.APIUserPassword true "The user credentials"
// @Success 200 {object} user.User
// @Failure 400 {object} code.vikunja.io/web.HTTPError "No or invalid user register object provided / User already exists."
// @Failure 500 {object} models.Message "Internal error"
// @Router /register [post]

View File

@ -33,7 +33,7 @@ import (
// @Produce json
// @Param s query string false "Search for a user by its name."
// @Security JWTKeyAuth
// @Success 200 {array} models.User "All (found) users."
// @Success 200 {array} user.User "All (found) users."
// @Failure 400 {object} code.vikunja.io/web.HTTPError "Something's invalid."
// @Failure 500 {object} models.Message "Internal server error."
// @Router /users [get]
@ -61,7 +61,7 @@ func UserList(c echo.Context) error {
// @Param s query string false "Search for a user by its name."
// @Security JWTKeyAuth
// @Param id path int true "List ID"
// @Success 200 {array} models.User "All (found) users."
// @Success 200 {array} user.User "All (found) users."
// @Failure 400 {object} code.vikunja.io/web.HTTPError "Something's invalid."
// @Failure 401 {object} code.vikunja.io/web.HTTPError "The user does not have the right to see the list."
// @Failure 500 {object} models.Message "Internal server error."

View File

@ -30,7 +30,7 @@ import (
// @Accept json
// @Produce json
// @Security JWTKeyAuth
// @Success 200 {object} models.User
// @Success 200 {object} user.User
// @Failure 404 {object} code.vikunja.io/web.HTTPError "User does not exist."
// @Failure 500 {object} models.Message "Internal server error."
// @Router /user [get]

View File

@ -664,7 +664,7 @@ var doc = `{
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
}
},
@ -1589,7 +1589,7 @@ var doc = `{
"required": true,
"schema": {
"type": "object",
"$ref": "#/definitions/models.UserLogin"
"$ref": "#/definitions/user.UserLogin"
}
}
],
@ -2688,7 +2688,7 @@ var doc = `{
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
},
"400": {
@ -3347,7 +3347,7 @@ var doc = `{
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
}
},
@ -4251,7 +4251,7 @@ var doc = `{
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
},
"404": {
@ -4529,7 +4529,7 @@ var doc = `{
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
}
},
@ -4626,7 +4626,7 @@ var doc = `{
"description": "A list with all assignees",
"type": "array",
"items": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
}
}
@ -4638,7 +4638,7 @@ var doc = `{
"description": "An array of users who are assigned to this task",
"type": "array",
"items": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
},
"attachments": {
@ -4655,7 +4655,7 @@ var doc = `{
"createdBy": {
"description": "The user who initially created the task.",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"description": {
"description": "The task description.",
@ -4771,7 +4771,7 @@ var doc = `{
"created_by": {
"description": "The user who created this label",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"description": {
"description": "The label description.",
@ -4847,7 +4847,7 @@ var doc = `{
"shared_by": {
"description": "The user who shared this list",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"sharing_type": {
"description": "The kind of this link. 0 = undefined, 1 = without password, 2 = with password (currently not implemented).",
@ -4885,7 +4885,7 @@ var doc = `{
"owner": {
"description": "The user who created this list.",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"title": {
"description": "The title of the list. You'll see this in the namespace overview.",
@ -4959,7 +4959,7 @@ var doc = `{
"owner": {
"description": "The user who owns this namespace",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"updated": {
"description": "A unix timestamp when this namespace was last updated. You cannot change this value.",
@ -5024,7 +5024,7 @@ var doc = `{
"owner": {
"description": "The user who owns this namespace",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"updated": {
"description": "A unix timestamp when this namespace was last updated. You cannot change this value.",
@ -5065,7 +5065,7 @@ var doc = `{
"description": "An array of users who are assigned to this task",
"type": "array",
"items": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
},
"attachments": {
@ -5082,7 +5082,7 @@ var doc = `{
"createdBy": {
"description": "The user who initially created the task.",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"description": {
"description": "The task description.",
@ -5181,7 +5181,7 @@ var doc = `{
"description": "An array of users who are assigned to this task",
"type": "array",
"items": {
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
}
},
"attachments": {
@ -5198,7 +5198,7 @@ var doc = `{
"createdBy": {
"description": "The user who initially created the task.",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"description": {
"description": "The task description.",
@ -5307,7 +5307,7 @@ var doc = `{
},
"created_by": {
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"file": {
"type": "object",
@ -5331,7 +5331,7 @@ var doc = `{
"created_by": {
"description": "The user who created this relation",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"other_task_id": {
"description": "The ID of the other task, the task which is being related.",
@ -5357,7 +5357,7 @@ var doc = `{
"createdBy": {
"description": "The user who created this team.",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"description": {
"description": "The team's description.",
@ -5507,7 +5507,7 @@ var doc = `{
"createdBy": {
"description": "The user who created this team.",
"type": "object",
"$ref": "#/definitions/models.User"
"$ref": "#/definitions/user.User"
},
"description": {
"description": "The team's description.",
@ -5539,7 +5539,7 @@ var doc = `{
}
}
},
"models.User": {
"user.User": {
"type": "object",
"properties": {
"avatarUrl": {
@ -5571,7 +5571,7 @@ var doc = `{
}
}
},
"models.UserLogin": {
"user.UserLogin": {
"type": "object",
"properties": {
"password": {