Apps.is_installed() incorrectly assumes app label is last portion of app dotted path

[Carl Meyer]

The app-registry tracks appconfigs indexed by app-label, but is_installed() needs to get an app-config by app name instead (that is, full dotted path). Currently it does this by chopping off everything but the last portion, which used to be a valid approach but no longer is, since custom AppConfig subclasses can define custom app labels.

Instead it could iterate over all app-configs looking for a name match, or (probably better) the registry could explicitly track the mapping from name to label.

21:51 <+carljm> mYk: This is a side-note, but I think it might be worth tracking appconfigs-by-appname in Apps
21:51 <+mYk> yeah
21:51 <+carljm> or explicitly tracking the translation from name-to-label and vice versa
21:51 <+mYk> mumble something cache invalidation something
21:51 <+carljm> I note that is_installed() has to reimplement that
21:52 <+carljm> because it takes a name
21:52 <+mYk> because of modify_settings and override_settings and friends
21:52 <+carljm> yeah
21:52 <+mYk> yes, but is_installed is mostly for import-time checks, so I don't care too much about performance
21:52 <+carljm> but I think is_installed would currently break on apps with a custom label
21:52 <+carljm> or am I missing something?
21:52 <+mYk> let me checkt
21:53 <+carljm> because for those apps, label != name.rpartition('.')[2]
21:53 <+mYk> oh yes it wolud
21:54 <+mYk> anyway, the correct implementation is what I said earlier -- just iterate through all apss
21:54 <+carljm> yeah
21:54 <+mYk> or add a cache by name
21:54 <+carljm> I'll file that (and maybe work on it too)
21:54 <+mYk> we have apps.clear_cache() that's supposed to be called whenever the app registry changes
21:55 <+mYk> that should make the cache invalidation easy
21:55 <+carljm> yes
21:55 <+carljm> if we have a reasonable solution for cache invalidation, I think keeping the mapping explicitly would be nice
21:55 <+carljm> because referring to apps by full dotted path isn't going away, we're going to keep hitting the "damn I have to iterate" problem
21:56 <+mYk> you can create a function that returns this mapping
21:56 <+mYk> @lru_cache.lru_cache(maxsize=None) it
21:56 <+carljm> true story
21:56 <+mYk> and clear the cache
21:57 <+mYk> sounds easy, unless I've missed something

[Carl Meyer]

I decided to fix this for now with a simple short-circuiting loop in is_installed(), rather than with new API to get an app-config by name. The reason is that is_installed() should run successfully even when the app-registry isn't fully populated, which makes it a bit special. If there were new API to get an app-config by name, it should, I think, require the app registry to be populated, for consistency with get_app_config() et al. And if this new API requires a fully-populated registry, is_installed() can't use it. So if performance of is_installed() isn't critical, this seemed the simpler approach. The hypothetical new API can be added if/when we have a motivating use case that would actually use it.

[Carl Meyer <carl@…>]

In 29ddae7436e84f4713c7babeabdf9a1fa62d6543:

Fixed #21871 -- Fixed Apps.is_installed() for apps with custom label.

Thanks Aymeric for design discussion.