为用户和公司分配多个端点的权限

Question

原始代码：

# add this class to make the code can be a reproducible example.In acutually environment all attribute could be True or False.
class Capabilities:
    def __init__(self) -> None:
        self.canEditCompany = True
        self.canDeleteCompany = True
        self.canShareCompany = True
        self.canAddFiscal = True
        self.canAddCategory = True
        self.canEditCategory = True
        self.canGetCategory = True
        self.canAddAccount = True
        self.canGetAccount = True
        self.canEditAccount = True
        self.canAddTransaction = True
        self.canAddScan = True
        self.canAddTax = True

def permission_control(user_email,company):
    capabilities = Capabilities()
    casbin_roles = []
    if capabilities.canEditCompany:
        casbin_roles.append((user_email, company, 'PUT'))
    if capabilities.canDeleteCompany:
        casbin_roles.append((user_email, company, 'Delete'))
    if capabilities.canShareCompany:
        casbin_roles.append((user_email, f'{company}/sharing', 'Post'))
    if capabilities.canAddFiscal:
        casbin_roles.append((user_email, f'{company}/fiscal_year', 'POST'))
    if capabilities.canAddCategory:
        casbin_roles.append((user_email, f'{company}/fiscals/*/categories', 'Post'))
    if capabilities.canEditCategory:
        casbin_roles.append((user_email, f'{company}/fiscals/*/categories/*', 'PUT'))
    if capabilities.canGetCategory:
        casbin_roles.append((user_email, f'{company}/fiscals/*/categories', 'GET'))
    if capabilities.canAddAccount:
        casbin_roles.append((user_email, f'{company}/fiscals/*/accounts', 'POST'))
    if capabilities.canGetAccount:
        casbin_roles.append((user_email, f'{company}/fiscals/*/accounts', 'GET'))
    if capabilities.canEditAccount:
        casbin_roles.append((user_email, f'{company}/fiscals/*/accounts/*', 'PUT'))
    if capabilities.canAddAccount:
        casbin_roles.append((user_email, f'{company}/fiscals/*/account', 'POST'))
    if capabilities.canAddTax:
        casbin_roles.append((user_email, f'{company}/tax', 'POST'))
    if capabilities.canAddScan:
        casbin_roles.append((user_email, f'{company}/scan', 'POST'))
    if capabilities.canAddTransaction:
        casbin_roles.append((user_email, f'{company}/accounts/*', 'POST'))
    return casbin_roles

我的尝试：

import functools
def permission_control(user_email,company):

    def partial(attribute_name):
        return functools.partial(getattr,capabilities,attribute_name)
        
    capabilities = Capabilities()
    permissions = ["canEditCompany","canDeleteCompany","canShareCompany","canAddFiscal","canAddCategory","canEditCategory","canGetCategory","canAddAccount","canGetAccount","canEditAccount","canAddAccount","canAddTax","canAddScan","canAddTransaction"]
    urls = [(company, 'PUT'),(company, 'Delete'),(f'{company}/sharing', 'Post'),(f'{company}/fiscal_year', 'POST'),(f'{company}/fiscals/*/categories', 'Post'),(f'{company}/fiscals/*/categories/*', 'PUT'),(f'{company}/fiscals/*/categories', 'GET'),(f'{company}/fiscals/*/accounts', 'POST'),(f'{company}/fiscals/*/accounts', 'GET'),(f'{company}/fiscals/*/accounts/*', 'PUT'),(f'{company}/fiscals/*/account', 'POST'),(f'{company}/tax', 'POST'),(f'{company}/scan', 'POST'),(f'{company}/accounts/*', 'POST')]
    return [((user_email,) + url) for permission,url in zip(permissions,urls) if partial(permission)()]

问题

• 有更好的，更多的琵琶方法来实现它吗？
• 我认为我的代码可能比原始代码更糟糕，因为我的同事很难理解/维护它。我说的对吗？

Answer 1

我会把它说得简单一点:没有lambdas，没有部分，没有格式(只有后缀，没有其他)。使您的HTTP方法具有一致性，并且可以使用生成器而不是列表理解。对于您的Capabilities，您可以接受布尔kwargs。预注册所有可能的功能，在一个常数字典，大致类似于亚历克斯的actions_dict，但更强的类型。

还请注意，permission_control是一个名词，其中的方法名称应该是动词，即control_permissions或，如我所建议的.evaluate()

建议

from dataclasses import dataclass
from typing import Tuple, Iterable

CapTuple = Tuple[
    str,  # email
    str,  # URL path
    str,  # method
]


@dataclass(frozen=True)
class Capability:
    name: str
    method: str
    path: str


CAPABILITIES = {
    name: Capability(name, method, path)
    for name, method, path in (
        ("canEditCompany",    'PUT',    ''),
        ("canDeleteCompany",  'DELETE', ''),
        ("canShareCompany",   'POST',   '/sharing'),
        ("canAddFiscal",      'POST',   '/fiscal_year'),
        ("canAddCategory",    'POST',   '/fiscals/*/categories'),
        ("canEditCategory",   'PUT',    '/fiscals/*/categories/*'),
        ("canGetCategory",    'GET',    '/fiscals/*/categories'),
        ("canAddAccount",     'POST',   '/fiscals/*/accounts'),
        ("canGetAccount",     'GET',    '/fiscals/*/accounts'),
        ("canEditAccount",    'PUT',    '/fiscals/*/accounts/*'),
        ("canAddAccount",     'POST',   '/fiscals/*/account'),
        ("canAddTax",         'POST',   '/tax'),
        ("canAddScan",        'POST',   '/scan'),
        ("canAddTransaction", 'POST',   '/accounts/*'),
    )
}


class Capabilities:
    def __init__(self, **kwargs: bool):
        self.allowed: Tuple[Capability] = tuple(
            CAPABILITIES[name]
            for name, allowed in kwargs.items()
            if allowed
        )

    def evaluate(self, user_email: str, company: str) -> Iterable[CapTuple]:
        for cap in self.allowed:
            yield user_email, company + cap.path, cap.method


def test() -> None:
    caps = Capabilities(canAddAccount=True, canEditAccount=True, canDeleteCompany=False)
    for user_email, path, method in caps.evaluate('me@here.com', 'here_inc'):
        print(user_email, path, method)


if __name__ == '__main__':
    test()

Answer 2

我可能会做这样的事：

class Capabilities:
    def __init__(self) -> None:
        self.canEditCompany = True
        self.canDeleteCompany = True
        self.canShareCompany = True
        self.canAddFiscal = True
        self.canAddCategory = True
        self.canEditCategory = True
        self.canGetCategory = True
        self.canAddAccount = True
        self.canGetAccount = True
        self.canEditAccount = True
        self.canAddTransaction = True
        self.canAddScan = True
        self.canAddTax = True

 actions_dict = {
    "canEditCompany":    lambda company: (company, 'PUT'),
    "canDeleteCompany":  lambda company: (company, 'Delete'),
    "canShareCompany":   lambda company: (f'{company}/sharing', 'Post'),
    "canAddFiscal":      lambda company: (f'{company}/fiscal_year', 'POST'),
    "canAddCategory":    lambda company: (f'{company}/fiscals/*/categories', 'Post'),
    "canEditCategory":   lambda company: (f'{company}/fiscals/*/categories/*', 'PUT'),
    "canGetCategory":    lambda company: (f'{company}/fiscals/*/categories', 'GET'),
    "canAddAccount":     lambda company: (f'{company}/fiscals/*/accounts', 'POST'),
    "canGetAccount":     lambda company: (f'{company}/fiscals/*/accounts', 'GET'),
    "canEditAccount":    lambda company: (f'{company}/fiscals/*/accounts/*', 'PUT'),
    "canAddAccount":     lambda company: (f'{company}/fiscals/*/account', 'POST'),
    "canAddTax":         lambda company: (f'{company}/tax', 'POST'),
    "canAddScan":        lambda company: (f'{company}/scan', 'POST'),
    "canAddTransaction": lambda company: (f'{company}/accounts/*', 'POST')
}


def permission_control(user_email, company):
    capabilities = Capabilities()
    return [(user_email, *action(company)) for capability, action in actions_dict.items() if getattr(capabilities, capability)]

我同意，在某些方面，您的代码的可读性较低，但我认为您是朝着正确的方向前进，去掉了那棵糟糕的if-tree，在我看来，这绝对是反模式。我认为将输入和结果放在字典中比将它们保存在两个单独的列表中要容易读得多，就像您拥有它们一样--通过使用lambda函数，我们可以在全局命名空间中构造dict，这样就不必每次调用函数时都重新生成dict，这就是您的代码对两个列表所做的。

编辑:按照注释中的建议，更好的解决方案是使用format-strings而不是lambdas：

class Capabilities:
    def __init__(self) -> None:
        self.canEditCompany = True
        self.canDeleteCompany = True
        self.canShareCompany = True
        self.canAddFiscal = True
        self.canAddCategory = True
        self.canEditCategory = True
        self.canGetCategory = True
        self.canAddAccount = True
        self.canGetAccount = True
        self.canEditAccount = True
        self.canAddTransaction = True
        self.canAddScan = True
        self.canAddTax = True

actions_dict = {
    "canEditCompany":    ('{}', 'PUT'),
    "canDeleteCompany":  ('{}', 'Delete'),
    "canShareCompany":   ('{}/sharing', 'Post'),
    "canAddFiscal":      ('{}/fiscal_year', 'POST'),
    "canAddCategory":    ('{}/fiscals/*/categories', 'Post'),
    "canEditCategory":   ('{}/fiscals/*/categories/*', 'PUT'),
    "canGetCategory":    ('{}/fiscals/*/categories', 'GET'),
    "canAddAccount":     ('{}/fiscals/*/accounts', 'POST'),
    "canGetAccount":     ('{}/fiscals/*/accounts', 'GET'),
    "canEditAccount":    ('{}/fiscals/*/accounts/*', 'PUT'),
    "canAddAccount":     ('{}/fiscals/*/account', 'POST'),
    "canAddTax":         ('{}/tax', 'POST'),
    "canAddScan":        ('{}/scan', 'POST'),
    "canAddTransaction": ('{}/accounts/*', 'POST')
}


def permission_control(user_email, company):
    capabilities = Capabilities()

    return [
        (user_email, url.format(company), method)
        for capability, (url, method) in actions_dict.items() 
        if getattr(capabilities, capability)
    ]

第二个编辑:正如注释中所指出的，通过预绑定格式字符串的format方法，可以更好地解决格式字符串的问题。这稍微更有表现力，并且将字符串限制在一个单一的用途上。

我还将数据结构从str: tuple[str, str]字典更改为NamedTuples列表。在这个阶段，字典已不再有意义了。与元组相比，NamedTuples并不是绝对必要的，但我认为使用它们可以使代码的意图更加清晰，并提高可读性。

from typing import NamedTuple, Callable, Literal

class Capabilities:
    def __init__(self) -> None:
        self.canEditCompany = True
        self.canDeleteCompany = True
        self.canShareCompany = True
        self.canAddFiscal = True
        self.canAddCategory = True
        self.canEditCategory = True
        self.canGetCategory = True
        self.canAddAccount = True
        self.canGetAccount = True
        self.canEditAccount = True
        self.canAddTransaction = True
        self.canAddScan = True
        self.canAddTax = True


class CapabilityTuple(NamedTuple):
    capability: str
    url_factory: Callable[[str], str]

    # Any reason why some of these aren't all-uppercase in your question?
    # HTTP methods are usually all-uppercase
    method: Literal['PUT', 'Delete', 'POST', 'GET', 'Post'] 


actions_list = [
    CapabilityTuple(capability, url.format, method) for  capability, url, method in (
        ("canEditCompany",    '{}',                        'PUT'),
        ("canDeleteCompany",  '{}',                        'Delete'),
        ("canShareCompany",   '{}/sharing',                'Post'),
        ("canAddFiscal",      '{}/fiscal_year',            'POST'),
        ("canAddCategory",    '{}/fiscals/*/categories',   'Post'),
        ("canEditCategory",   '{}/fiscals/*/categories/*', 'PUT'),
        ("canGetCategory",    '{}/fiscals/*/categories',   'GET'),
        ("canAddAccount",     '{}/fiscals/*/accounts',     'POST'),
        ("canGetAccount",     '{}/fiscals/*/accounts',     'GET'),
        ("canEditAccount",    '{}/fiscals/*/accounts/*',   'PUT'),
        ("canAddAccount",     '{}/fiscals/*/account',      'POST'),
        ("canAddTax",         '{}/tax',                    'POST'),
        ("canAddScan",        '{}/scan',                   'POST'),
        ("canAddTransaction", '{}/accounts/*',             'POST')
    )
]


def permission_control(user_email, company):
    capabilities = Capabilities()

    return [
        (user_email, url_factory(company), method)
        for capability, url_factory, method in actions_list
        if getattr(capabilities, capability)
    ]