Fix FIXME and remove superfluous queries in models/org

[ethantkoenig]

I realize this is long, but I want to give some context for my changes. Please bear with me, or jump to the TL;DR at the bottom if you're impatient.

This PR addresses the following FIXME in models/org.go, line 474-475 (an identical FIXME is also in models/action.go, line 661):

// FIXME: only need to get IDs here, not all fields of repository.
repos, _, err := org.GetUserRepositories(user.ID, 1, org.NumRepos)

org.GetUserRepositories(uid, ..) first computes the necessary repoIDs, then loads the repos with those repoIDs, then finally computes a count (the second return value, which is ignored above). It would therefore make sense to create a separate function for computing the repoIDs (say org.GetUserRepositoryIDs(uid, ..)) that both org.GetUserRepositories(..) and models/org.go, 474-475 can use.

There's more: of the several places where org.GetUserRepositories(..) is called, the count is ignored in all but one place (routers/user/home.go, line 355). So most of the time when org.GetUserRepositories(..) is called, the count is calculated (which involves a join), and then the count is immediately thrown away. I therefore decided to not have org.GetUserRepositories(..) compute the count, but instead move that logic to another function (say org.GetAccessibleReposCount(uid)) to avoid wasting work. routers/user/home.go, line 355 would then make two calls: one to org.GetUserRepositories(..) and another to org.GetAccessibleRepoCount(..).

This seems promising, but there's one problem with this approach: both computing the repoIDs (in org.GetUserRepositoryIDs(..), and by extension org.GetUserRepositories(..)) and computing the count (org.GetAccessibleRepoCount(..)) require determining which teams the specified user belongs to. In routers/user/home.go, line 355, where I actually care about the count, I will need to compute the teams that a user belongs to twice (once for org.GetUserRepositories(), another time for org.GetAccessibleRepoCount()). The same problem occurs in places where both org.GetUserRepositories() and org.GetUserMirrorRepositories()) are called, (e.g. routers/user/home.go, line 116).

Taking a step back, there are three functions

• org.GetUserRepositoryIDs(uid, ..)
• org.GetAccessibleRepositoryCount(uid)
• org.GetUserMirrorRepositories(uid)

all of which require the same preprocessing, and we would like to avoid doing the preprocessing more than once if we call these functions multiple times with the same uid. The solution that occurred to me was to move these functions to a AccessibleRepoEnvironment interface. The implementation of the interface, accessibleRepoEnv, would store the results of the preprocessing (i.e. the teamIDs a particular user belongs to), and each function/method would just use those results instead of (re)computing them.

Under my changes, to call these functions/method, one can do the following

env, err := org.AccessibleRepoEnv(userID) // preprocessing done here
// check err
count, err := env.CountRepos() // org and uid not provided, they're specific to env
// check err
repos := env.Repos()
// check err
...

and no work will be wasted or duplicated 😃. The interface, or at least the idea behind, could also be used in other parts of the codebase.

TL;DR: I got rid of useless/duplicated queries by moving some functions to an interface.

I'm open to suggestions (particularly for naming) and would be happy to hear any feedback on this.

[Bwko]

I tried to fix #746 when I bumped into exactly the same issue.
Your point makes sense to me. But I'm no sql expert so maybe @lunny could take a look?

[lunny]

LGTM

[Bwko]

LGTM