Add: Denormalization: delete ace attributeTypeId, add attribute_type_name

[milov-dmitriy]

С целью упрощения запросов и БЛ проводим денормализацию: удаляем колонку attributeTypeId и удаляем связь между AccessControlEntry и Directory, зато добавляем просто attribute_type_name с типом str.

Подготовка для задачи с ldf файлами.

[Copilot]

Pull request overview

This PR aims to denormalize AccessControlEntry “attribute type” handling by moving from an FK-based attributeTypeId approach toward storing a plain string attribute_type_name, simplifying authorization queries and business logic (prep for upcoming LDF-related work).

Changes:

• Update role/ACE DTOs and access-check logic to use attribute_type_name instead of attribute_type_id.
• Remove eager-loading of the attribute-type relationship in DAOs and request handlers.
• Add an Alembic migration to introduce/backfill attribute_type_name, and update LDAP-related tests accordingly.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
tests/test_ldap/test_util/test_search.py	Updates ACE test setup to pass attribute_type_name.
tests/test_ldap/test_util/test_modify.py	Updates modify-related ACE fixtures/assertions to use attribute_type_name.
tests/test_ldap/test_util/test_delete.py	Updates delete-related ACE fixtures to use attribute_type_name.
tests/test_ldap/test_util/test_add.py	Updates add-related ACE fixtures to use attribute_type_name.
tests/test_ldap/test_roles/test_search.py	Updates role-search tests to use attribute_type_name.
tests/test_ldap/test_roles/test_multiple_access.py	Updates multi-ACE test cases to use attribute_type_name.
app/repo/pg/tables.py	Adds attribute_type_name column to AccessControlEntries schema definition.
app/ldap_protocol/roles/role_use_case.py	Creates default roles/ACEs using attribute_type_name.
app/ldap_protocol/roles/role_dao.py	Stops joined-loading the attribute-type relationship for ACEs.
app/ldap_protocol/roles/migrations_ace_dao.py	Switches ACE remap queries to join by attribute type name.
app/ldap_protocol/roles/dataclasses.py	Renames DTO field from attribute_type_id to attribute_type_name.
app/ldap_protocol/roles/ace_dao.py	Writes attribute_type_name into ACE entities; removes joined-load of attribute-type relationship.
app/ldap_protocol/roles/access_manager.py	Updates access checks and query loader to rely on attribute_type_name.
app/ldap_protocol/ldap_requests/search.py	Removes load_attribute_type usage when loading ACEs for search.
app/ldap_protocol/ldap_requests/modify_dn.py	Removes load_attribute_type usage and updates checks to attribute_type_name.
app/ldap_protocol/ldap_requests/modify.py	Removes load_attribute_type usage when loading ACEs for modify.
app/entities.py	Adds attribute_type_name field to AccessControlEntry and removes computed property.
app/alembic/versions/1b71cafba681_replace_attributeTypeId.py	Adds/backfills attribute_type_name via migration (but currently does not actually drop attributeTypeId).

Comments suppressed due to low confidence (3)

app/ldap_protocol/roles/access_manager.py:71

• Access checks treat attribute_type_name as a direct comparable key, but LDAP/request attribute names are lower-cased elsewhere (e.g., SearchRequest.requested_attrs, SearchRequest filtering uses attr_name.lower()). Here you add raw ace.attribute_type_name into the allowed/forbidden sets, which will break matching for mixed-case names. Normalize to a consistent case (e.g., store ace.attribute_type_name.lower() in these sets) so filtering behaves correctly.

        for ace in aces:
            if not ace.is_allow and ace.attribute_type_name is None:
                if allowed_attributes:
                    return True, set(), allowed_attributes
                else:
                    return False, set(), set()

            elif not ace.is_allow and ace.attribute_type_name is not None:
                forbidden_attributes.add(ace.attribute_type_name)

            elif ace.is_allow and ace.attribute_type_name is None:
                return True, forbidden_attributes, set()

            else:
                allowed_attributes.add(ace.attribute_type_name)  # type: ignore

app/ldap_protocol/roles/access_manager.py:304

• The mutate_query_with_ace_load docstring still documents load_attribute_type and “null attribute_type_id”, but the parameter was removed and the code now filters by attribute_type_name. Please update the docstring so it matches the actual signature/behavior.

        """Mutate query to load access control entries.

        :param user_role_ids: list of user role ids
        :param query: SQLAlchemy query to mutate
        :param ace_types: single AceType or list of AceTypes to filter by
        :param load_attribute_type: whether to joinedload attribute_type
        :param require_attribute_type_null: whether to filter by
            null attribute_type_id
        :return: mutated query with access control entries loaded

app/entities.py:383

• AccessControlEntry still contains attribute_type_id and the attribute_type: Directory | None relationship field even though the PR intent says attributeTypeId and the ACE↔Directory attribute-type link are being removed. If the plan is to fully drop that relationship, the model/table mapping should be updated accordingly (and the legacy column removed via migration) to avoid having two competing sources of truth.

    depth: int | None = None
    path: str = ""
    attribute_type_id: int | None = None
    attribute_type_name: str | None = None
    entity_type_id: int | None = None
    is_allow: bool = False

    role: Role | None = field(init=False, default=None, repr=False)
    attribute_type: Directory | None = field(
        init=False,
        default=None,
        repr=False,
    )

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

app/ldap_protocol/roles/access_manager.py:304

• The docstring for mutate_query_with_ace_load() still documents the removed load_attribute_type parameter and refers to attribute_type_id being NULL. Please update the docstring to reflect the current signature and that the NULL-filter now applies to attribute_type_name.

        """Mutate query to load access control entries.

        :param user_role_ids: list of user role ids
        :param query: SQLAlchemy query to mutate
        :param ace_types: single AceType or list of AceTypes to filter by
        :param load_attribute_type: whether to joinedload attribute_type
        :param require_attribute_type_null: whether to filter by
            null attribute_type_id
        :return: mutated query with access control entries loaded

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rimu-stack]

не такой запрос, мы же хотим сделать на ldapDisplayName связь, как жто было до этого. В AccessControlEntry.attribute_type.name лежал твой новый ldap_display_name, а directory.name у того же атрибута, который мы перенесли из таблицы в ldap дерево, может отличаться и сильно. Например, Directory.name = 'Address-Book-Container', а его lDAPDisplayName = addressBookContainer - разница колоссальная

[rimu-stack]

не совсем понял, зачем это удалено

[rimu-stack]

1. не по directory.name
2. у тебя изменение ролевки будет после переноса в ldap схемы, если я правильно уловил, т.е. на момент вызова миграции с переносом AccessControlEntry.attribute_type_name будет у всех пустой, не?

[rimu-stack]

снова ошибка, description_attr.attrs_dict['ldapDisplayName'] чет такое