fix(projects): load projects only one when fetching subscriptions for a bunch of projects at once

This change ensures already loaded projects are passed down when fetching their subscription  instead of re-loading each project with a single sql statement. When loading all projects, this meant all projects were loaded twice, which was highly inefficient. This roughly added 25ms to each request, assuming the per page limit was maxed out at 50 projects.

Empirical testing shows this change reduces load times by ~20ms. Because the request is already pretty fast, this is ~30% of the overall request time, making the loading of projects now even faster

pkg/models/project.go

@@ -316,8 +316,8 @@ func GetProjectSimplByTaskID(s *xorm.Session, taskID int64) (l *Project, err err
 	return &project, nil
 }
 
-// GetProjectsSimplByTaskIDs gets a list of projects by a task ids
+// GetProjectsMapSimplByTaskIDs gets a list of projects by a task ids
-func GetProjectsSimplByTaskIDs(s *xorm.Session, taskIDs []int64) (ps map[int64]*Project, err error) {
+func GetProjectsMapSimplByTaskIDs(s *xorm.Session, taskIDs []int64) (ps map[int64]*Project, err error) {
 	ps = make(map[int64]*Project)
 	err = s.
 		Select("projects.*").
@@ -328,8 +328,18 @@ func GetProjectsSimplByTaskIDs(s *xorm.Session, taskIDs []int64) (ps map[int64]*
 	return
 }
 
-// GetProjectsByIDs returns a map of projects from a slice with project ids
+func GetProjectsSimplByTaskIDs(s *xorm.Session, taskIDs []int64) (ps []*Project, err error) {
-func GetProjectsByIDs(s *xorm.Session, projectIDs []int64) (projects map[int64]*Project, err error) {
+	err = s.
+		Select("projects.*").
+		Table(Project{}).
+		Join("INNER", "tasks", "projects.id = tasks.project_id").
+		In("tasks.id", taskIDs).
+		Find(&ps)
+	return
+}
+
+// GetProjectsMapByIDs returns a map of projects from a slice with project ids
+func GetProjectsMapByIDs(s *xorm.Session, projectIDs []int64) (projects map[int64]*Project, err error) {
 	projects = make(map[int64]*Project, len(projectIDs))
 
 	if len(projectIDs) == 0 {
@@ -340,6 +350,17 @@ func GetProjectsByIDs(s *xorm.Session, projectIDs []int64) (projects map[int64]*
 	return
 }
 
+func GetProjectsByIDs(s *xorm.Session, projectIDs []int64) (projects []*Project, err error) {
+	projects = make([]*Project, 0, len(projectIDs))
+
+	if len(projectIDs) == 0 {
+		return
+	}
+
+	err = s.In("id", projectIDs).Find(&projects)
+	return
+}
+
 type projectOptions struct {
 	search      string
 	user        *user.User
@@ -559,7 +580,7 @@ func addProjectDetails(s *xorm.Session, projects []*Project, a web.Auth) (err er
 		return err
 	}
 
-	subscriptions, err := GetSubscriptions(s, SubscriptionEntityProject, projectIDs, a)
+	subscriptions, err := GetSubscriptionsForProjects(s, projects, a)
 	if err != nil {
 		log.Errorf("An error occurred while getting project subscriptions for a project: %s", err.Error())
 		subscriptions = make(map[int64][]*Subscription)

pkg/models/subscription.go

@@ -223,7 +223,11 @@ func GetSubscriptions(s *xorm.Session, entityType SubscriptionEntityType, entity
 
 	switch entityType {
 	case SubscriptionEntityProject:
-		return getSubscriptionsForProjects(s, entityIDs, u)
+		projects, err := GetProjectsByIDs(s, entityIDs)
+		if err != nil {
+			return nil, err
+		}
+		return GetSubscriptionsForProjects(s, projects, u)
 	case SubscriptionEntityTask:
 		subs, err := getSubscriptionsForTasks(s, entityIDs, u)
 		if err != nil {
@@ -232,22 +236,34 @@ func GetSubscriptions(s *xorm.Session, entityType SubscriptionEntityType, entity
 
 		// If the task does not have a subscription directly or from its project, get the one
 		// from the parent and return it instead.
+		var taskIDsWithoutSubscription []int64
 		for _, eID := range entityIDs {
 			if _, has := subs[eID]; has {
 				continue
 			}
 
-			task, err := GetTaskByIDSimple(s, eID)
+			taskIDsWithoutSubscription = append(taskIDsWithoutSubscription, eID)
-			if err != nil {
+		}
-				return nil, err
+
-			}
+		projects, err := GetProjectsSimplByTaskIDs(s, taskIDsWithoutSubscription)
-			projectSubscriptions, err := getSubscriptionsForProjects(s, []int64{task.ProjectID}, u)
+		if err != nil {
-			if err != nil {
+			return nil, err
-				return nil, err
+		}
-			}
+
-			for _, subscription := range projectSubscriptions {
+		tasks, err := GetTasksSimpleByIDs(s, taskIDsWithoutSubscription)
-				subs[eID] = subscription // The first project subscription is the subscription we're looking for
+		if err != nil {
-				break
+			return nil, err
+		}
+
+		projectSubscriptions, err := GetSubscriptionsForProjects(s, projects, u)
+		if err != nil {
+			return nil, err
+		}
+
+		for _, task := range tasks {
+			sub, has := projectSubscriptions[task.ProjectID]
+			if has {
+				subs[task.ID] = sub
 			}
 		}
 
@@ -257,48 +273,59 @@ func GetSubscriptions(s *xorm.Session, entityType SubscriptionEntityType, entity
 	return
 }
 
-func getSubscriptionsForProjects(s *xorm.Session, projectIDs []int64, u *user.User) (projectsToSubscriptions map[int64][]*Subscription, err error) {
+func GetSubscriptionsForProjects(s *xorm.Session, projects []*Project, a web.Auth) (projectsToSubscriptions map[int64][]*Subscription, err error) {
-	origEntityIDs := projectIDs
+	u, is := a.(*user.User)
-	var ps = make(map[int64]*Project)
+	if u != nil && !is {
+		return
+	}
 
-	for _, eID := range projectIDs {
+	var ps = make(map[int64]*Project)
-		if eID < 1 {
+	origProjectIDs := make([]int64, 0, len(projects))
+	allProjectIDs := make([]int64, 0, len(projects))
+
+	for _, p := range projects {
+		ps[p.ID] = p
+		origProjectIDs = append(origProjectIDs, p.ID)
+		allProjectIDs = append(allProjectIDs, p.ID)
+	}
+
+	// We can't just use the projects we have, we need to fetch the parents
+	// because they may not be loaded in the same object
+
+	for _, p := range projects {
+		if p.ParentProjectID == 0 {
 			continue
 		}
-		ps[eID], err = GetProjectSimpleByID(s, eID)
+
-		if err != nil && IsErrProjectDoesNotExist(err) {
+		if _, has := ps[p.ParentProjectID]; has {
-			// If the project does not exist, it might got deleted. There could still be subscribers though.
-			delete(ps, eID)
 			continue
 		}
-		if err != nil {
+
-			return nil, err
+		err = ps[p.ID].GetAllParentProjects(s)
-		}
-		err = ps[eID].GetAllParentProjects(s)
 		if err != nil {
 			return nil, err
 		}
 
 		parentIDs := []int64{}
-		var parent = ps[eID].ParentProject
+		var parent = ps[p.ID].ParentProject
 		for parent != nil {
 			parentIDs = append(parentIDs, parent.ID)
 			parent = parent.ParentProject
 		}
 
 		// Now we have all parent ids
-		projectIDs = append(projectIDs, parentIDs...) // the child project id is already in there
+		allProjectIDs = append(allProjectIDs, parentIDs...) // the child project id is already in there
 	}
 
 	var subscriptions []*Subscription
 	if u != nil {
 		err = s.
 			Where("user_id = ?", u.ID).
-			And(getSubscriberCondForEntities(SubscriptionEntityProject, projectIDs)).
+			And(getSubscriberCondForEntities(SubscriptionEntityProject, allProjectIDs)).
 			Find(&subscriptions)
 	} else {
 		err = s.
-			And(getSubscriberCondForEntities(SubscriptionEntityProject, projectIDs)).
+			And(getSubscriberCondForEntities(SubscriptionEntityProject, allProjectIDs)).
 			Find(&subscriptions)
 	}
 	if err != nil {
@@ -313,7 +340,7 @@ func getSubscriptionsForProjects(s *xorm.Session, projectIDs []int64, u *user.Us
 
 	// Rearrange so that subscriptions trickle down
 
-	for _, eID := range origEntityIDs {
+	for _, eID := range origProjectIDs {
 		// If the current project does not have a subscription, climb up the tree until a project has one,
 		// then use that subscription for all child projects
 		_, has := projectsToSubscriptions[eID]

pkg/models/task_overdue_reminder.go

@@ -139,7 +139,7 @@ func RegisterOverdueReminderCron() {
 			}
 		}
 
-		projects, err := GetProjectsSimplByTaskIDs(s, taskIDs)
+		projects, err := GetProjectsMapSimplByTaskIDs(s, taskIDs)
 		if err != nil {
 			log.Errorf("[Undone Overdue Tasks Reminder] Could not get projects for tasks: %s", err)
 			return

pkg/models/task_reminder.go

@@ -173,7 +173,7 @@ func getTasksWithRemindersDueAndTheirUsers(s *xorm.Session, now time.Time) (remi
 
 	seen := make(map[int64]map[int64]bool)
 
-	projects, err := GetProjectsSimplByTaskIDs(s, taskIDs)
+	projects, err := GetProjectsMapSimplByTaskIDs(s, taskIDs)
 	if err != nil {
 		return
 	}

pkg/models/tasks.go

@@ -356,6 +356,11 @@ func GetTaskSimple(s *xorm.Session, t *Task) (task Task, err error) {
 	return
 }
 
+func GetTasksSimpleByIDs(s *xorm.Session, ids []int64) (tasks []*Task, err error) {
+	err = s.In("id", ids).Find(&tasks)
+	return
+}
+
 // GetTasksByIDs returns all tasks for a project of ids
 func (bt *BulkTask) GetTasksByIDs(s *xorm.Session) (err error) {
 	for _, id := range bt.IDs {
@@ -586,7 +591,7 @@ func addMoreInfoToTasks(s *xorm.Session, taskMap map[int64]*Task, a web.Auth) (e
 	}
 
 	// Get all identifiers
-	projects, err := GetProjectsByIDs(s, projectIDs)
+	projects, err := GetProjectsMapByIDs(s, projectIDs)
 	if err != nil {
 		return err
 	}