I realized where the N+1 queries were actually coming from
project.protected_branches, but how come we cannot preload this,
or cache this at all?
Then I found that this is somehow a Rails limitation. What we're
doing before, eventually come to:
project.protected_branches.matching
But why it's not cached? (project.protected_branches.loaded? is always
false) It's because matching is a class method, which is called on
the proxy. In this case, Rails cannot cache the result. I don't know
if this is possible to implement or not, because clearly this would
require some tricks to implement class methods on associations.
So instead, we could just pass project.protected_branches to
ProtectedRef.matching, then it would work regularly.
With this change, there's no more N+1 queries.
This is allowed for existing instances so we don't end up 76 offenses
right away, but for new code one should _only_ use this if they _have_
to remove non database data. Even then it's usually better to do this in
a service class as this gives you more control over how to remove the
data (e.g. in bulk).
Improvements and refactorings were made while adding role based permissions for protected tags to EE. This doesn’t backport the feature, but should improve code quality and minimize divergence.
