launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Review: Approve db
Database patch needs an index:

CREATE INDEX nameblacklist__admin__idx ON NameBlackList(admin);

I don't really like that there is no warning when you create something with a blacklisted name that you are allowed to override. I'm not sure if a warning is desirable enough to complicate the UI.

Should the admins team in Launchpad be allowed ignore the blacklist entirely? Membership in this team needs to be checked for explicitly.

Here is a much more efficient version of the stored procedure that also performs the admin check:


CREATE OR REPLACE FUNCTION name_blacklist_match(text, integer) RETURNS int4
LANGUAGE plpythonu STABLE RETURNS NULL ON NULL INPUT
EXTERNAL SECURITY DEFINER SET search_path TO public AS
$$
    import re
    name = args[0].decode("UTF-8")
    user_id = args[1]

    # Initialize shared storage, shared between invocations.
    if not SD.has_key("regexp_select_plan"):

        # All the blacklist regexps except the ones we are an admin
        # for. These we do not check since they are not blacklisted to us.
        SD["regexp_select_plan"] = plpy.prepare("""
            SELECT id, regexp FROM NameBlacklist
            WHERE admin IS NULL OR admin NOT IN (
                SELECT team FROM TeamParticipation
                WHERE person = $1)
            ORDER BY id
            """, ["integer"])

        # Storage for compiled regexps
        SD["compiled"] = {}

        # admins is a celebrity and its id is immutable.
        admins_id = plpy.execute(
            "SELECT id FROM Person WHERE name='admins'")[0]["id"]

        SD["admin_select_plan"] = plpy.prepare("""
            SELECT TRUE FROM TeamParticipation
            WHERE
                TeamParticipation.team = %d
                AND TeamParticipation.person = $1
            LIMIT 1
            """ % admins_id, ["integer"])

    # Names are never blacklisted for admins.
    if user_id is not None and plpy.execute(
        SD["admin_select_plan"], [user_id]).nrows() > 0:
        return None

    compiled = SD["compiled"]

    for row in plpy.execute(SD["regexp_select_plan"], [user_id]):
        regexp_id = row["id"]
        regexp_txt = row["regexp"]
        if (compiled.get(regexp_id) is None
            or compiled[regexp_id][0] != regexp_txt):
            regexp = re.compile(
                regexp_txt, re.IGNORECASE | re.UNICODE | re.VERBOSE
                )
            compiled[regexp_id] = (regexp_txt, regexp)
        else:
            regexp = compiled[regexp_id][1]
        if regexp.search(name) is not None:
            return regexp_id
    return None
$$;



-- 
https://code.launchpad.net/~sinzui/launchpad/override-blacklist-0/+merge/45499
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/override-blacklist-0 into lp:launchpad/db-devel.