Improve usage and optimize lookups of get_profile() and get_site_profile().

Review Request #10388 — Created Jan. 22, 2019 and submitted — Latest diff uploaded

Information

Review Board
release-3.0.x

Reviewers

User.get_profile() and User.get_site_profile() have diverged over
time, in terms of implementation and usage. get_profile() used to only
fetch profiles, requiring them to be created through some other means,
but these days it's guaranteed to return a result. It also gained the
ability to fetch via cache. get_site_profile() still worked like the
old get_profile() did, however, and required a lot more work from
callers. Much of the code in the codebase using these methods were
outdated and doing too much, creating extra SQL queries and ignoring any
previously-cached profiles.

This change works to modernize profile lookups and usage. Both methods
now accept the same arguments.

  • cached_only controls whether to only consider profile results in the
    existing cache.

  • create_if_missing controls whether to create an entry in the
    database if it doesn't already exist (which now defaults to True for
    both, creating consistent behavior).

  • return_is_new alters the return value to return a tuple containing
    the profile and an is_new boolean, letting callers be a bit smarter
    about whether operations on a profile need to be performed.

Call sites have been updated to take advantage of the new arguments and
behavior. Many callers that previously did their own get_or_create()
now benefit from the cache. Old calls checking for exceptions no longer
need to, or now pass create_if_missing=False. Default values computed
in the absence of a profile have been removed.

We also had many places where we were saving the entirety of profiles
unnecessarily. In some cases, we'd be performing a double-save of the
same data. We now no longer do this, and when we do want to save, we're
explicit about what fields we're updating. This helps avoid conflicts
and reduces database writes.

All unit tests pass.

Went through the product and tested most affected areas. There is a
chance of some subtle fallout/behavioral changes, which will require
further testing on reviews.reviewboard.org.

Commits

Files

    Loading...