Repository caching: Fallback early when repository does not exist but fallback is nil
The following discussion from !21789 (merged) should be addressed:
-
@stanhu started a discussion: (+4 comments) It is interesting that in the case of
avatar
and other cached fields, we don't have a fallback. So this will cause those requests to always run. Should the result just returnnil
?The question is whether we should
fallback_early?
withnil
, right? I thought about this too but played it conservative and kept the same behavior. But I think it would be safe and reasonable to changereturn fallback if fallback && fallback_early?(name)
toreturn fallback if fallback_early?(name)
.It turns out
RepositoryCacheAdapter.cache_method
is reused inGitlab::Conflict::FileCollection
but it doesn't define theexists?
method. I thinkcache_method
wasn't intended to be reused outside ofRepository
.At this point I plan to revert
Fallback early to nil fallback values
and open a tech debt issue to reconcile this problem.
This issue asks for two items:
-
Fallback early when repository does not exist but fallback is nil (e.g. https://gitlab.com/gitlab-org/gitlab-ce/commit/f31bcad3367f39eabf0aec0bfd277ae61377d3d0) -
Stop reusing RepositoryCacheAdapter
outside ofRepository
(e.g. extract a more generic class that providescache_method
) or rewrite it to be intentionally reusable (e.g. checkexists?
for fallback only ifexists?
is defined, or e.g. add afallback_to_nil: true
option)