Add: LDAP schema move from models to directories

[milov-dmitriy]

Нужно перевести представление LDAP схемы из двух таблиц (не считая двух линковочных) AttributeTypes и ObjectClasses на Директории, т.е. чтобы было полностью по аналогии.

!!! Обязательное требование: API ручки не трогать, не менять, контракты оставить прежними.

Задача: 1258

[Copilot]

Pull request overview

This PR continues the migration of LDAP schema handling from in-model entities to directory-backed storage, introducing new schema directory creation use-cases and “deprecated” appendix DAOs/use-cases to support migration paths.

Changes:

• Reworked AttributeType / ObjectClass persistence to rely on Directory + Attribute rows, with new “create directory like …” use-cases.
• Updated LDAP search/subschema generation to use schema use-cases and raw-definition renderers (*RawDisplay).
• Adjusted setup/migrations/tests to seed schema and entity types accordingly (with several tests temporarily stubbed).

Reviewed changes

Copilot reviewed 62 out of 64 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/test_shedule.py	Switches test dependency from DAO to EntityType use-case.
tests/test_ldap/test_util/test_search.py	Temporarily disables some LDAP utility integration tests.
tests/test_ldap/test_util/test_modify.py	Temporarily disables some modify-related integration tests.
tests/test_ldap/test_util/test_add.py	Temporarily disables ldapadd access-control integration test.
tests/test_ldap/test_roles/test_search.py	Temporarily disables multiple role-based search tests.
tests/test_ldap/test_roles/test_multiple_access.py	Temporarily disables multiple-ACE role test.
tests/test_ldap/test_ldap_schema/test_attribute_type_use_case.py	Updates replication-flag tests to create test attribute types first.
tests/test_ldap/test_ldap3_definition_parse.py	Updates raw-definition parsing tests to operate on DTOs + RawDisplay renderers.
tests/test_api/test_main/test_router/test_search.py	Expands expected subtree dirs to include Configuration.
tests/test_api/test_main/test_router/conftest.py	Updates test setup to use EntityTypeUseCase in SetupGateway wiring.
tests/test_api/test_ldap_schema/test_object_class_router.py	Adjusts pagination size; temporarily disables part of modification assertions.
tests/test_api/test_ldap_schema/test_attribute_type_router_datasets.py	Uses AttributeTypeUpdateSchema objects for patch datasets.
tests/test_api/test_ldap_schema/test_attribute_type_router.py	Uses model_dump() for patch JSON, adjusts page size.
tests/test_api/test_auth/test_router.py	Adds setup_session fixture for disabled-user auth test.
tests/constants.py	Adds configuration directory + explicit entity_type_name fields across test seed data.
tests/conftest.py	Major test-container wiring changes for new schema flow + deprecated appendix use-cases.
interface	Updates git submodule pointer.
docker-compose.test.yml	Adds -W ignore::SyntaxWarning during pytest in container.
app/repo/pg/tables.py	Imports schema entities from entities_appendix instead of entities.
app/ldap_protocol/utils/raw_definition_parser.py	Refactors raw-definition parser to produce DTOs (no DB session).
app/ldap_protocol/policies/password/dao.py	Minor formatting change for create() signature.
app/ldap_protocol/ldap_schema/object_class_use_case.py	Moves object-class creation logic to directory-backed creation + validation.
app/ldap_protocol/ldap_schema/object_class_raw_display.py	New: render ObjectClass DTO as RFC4512-style raw definition.
app/ldap_protocol/ldap_schema/object_class_dir_create_use_case.py	New: creates object-class directories under Configuration.
app/ldap_protocol/ldap_schema/object_class_dao.py	Reworks DAO to read from Directories/Attributes instead of schema tables.
app/ldap_protocol/ldap_schema/entity_type_use_case.py	Adds attach-to-directories logic and optional validation bypass for first setup.
app/ldap_protocol/ldap_schema/entity_type_dao.py	Removes DAO-level attach logic; shifts object-class validation out of DAO.
app/ldap_protocol/ldap_schema/constants.py	Removes compiled regex constant; keeps pattern.
app/ldap_protocol/ldap_schema/attribute_type_use_case.py	Reworks attribute-type creation via directory-backed creation and deprecated exceptions.
app/ldap_protocol/ldap_schema/attribute_type_raw_display.py	New: render AttributeType DTO as RFC4512-ish raw definition.
app/ldap_protocol/ldap_schema/attribute_type_dir_create_use_case.py	New: creates attribute-type directories under Configuration.
app/ldap_protocol/ldap_schema/attribute_type_dao.py	Reworks DAO to read from Directories/Attributes instead of schema tables.
app/ldap_protocol/ldap_schema/appendix/object_class_appendix/object_class_appendix_use_case.py	New: deprecated object-class use-case for migration.
app/ldap_protocol/ldap_schema/appendix/object_class_appendix/object_class_appendix_dao.py	New: deprecated object-class DAO for migration + table drop helper.
app/ldap_protocol/ldap_schema/appendix/attribute_type_appendix/attribute_type_appendix_use_case.py	New: deprecated attribute-type use-case for migration.
app/ldap_protocol/ldap_schema/appendix/attribute_type_appendix/attribute_type_appendix_dao.py	New: deprecated attribute-type DAO for migration + table drop helper.
app/ldap_protocol/ldap_requests/search.py	Switches subschema generation to use schema use-cases + RawDisplay.
app/ldap_protocol/ldap_requests/modify.py	Switches entity-type attach from DAO to use-case.
app/ldap_protocol/ldap_requests/contexts.py	Updates request contexts to carry schema use-cases and EntityTypeUseCase.
app/ldap_protocol/ldap_requests/add.py	Switches entity-type lookup/attach from DAO to use-case.
app/ldap_protocol/filter_interpreter.py	Reworks ANR attribute lookup to query directory-backed attribute-type flags.
app/ldap_protocol/auth/use_cases.py	Seeds new directory-based schema from deprecated tables during setup.
app/ldap_protocol/auth/setup_gateway.py	Uses EntityTypeUseCase to attach entity types during environment setup.
app/ioc.py	Adds wiring for deprecated appendix DAOs/use-cases and dir-create use-cases.
app/extra/scripts/add_domain_controller.py	Uses EntityTypeUseCase instead of EntityTypeDAO.
app/extra/alembic_utils.py	Adds temporary_stub_column2() helper for migrations.
app/enums.py	Adds EntityTypeNames for Configuration/Attribute Type/Object Class; removes auth rule constant.
app/entities_appendix.py	New: deprecated schema dataclasses (AttributeType/ObjectClass).
app/entities.py	Moves schema dataclasses out; keeps AttributeType type via appendix import.
app/constants.py	Adds Configuration dir in first setup data + entity types for schema entities.
app/api/ldap_schema/schema.py	Switches constants import to API-local constants module.
app/api/ldap_schema/attribute_type_router.py	Formatting-only change for update call.
app/api/ldap_schema/adapters/object_class.py	Replaces adaptix converter with manual mapping supporting Directory/DTO.
app/api/ldap_schema/adapters/entity_type.py	Switches constants import to API-local constants module.
app/api/ldap_schema/adapters/attribute_type.py	Switches constants import to API-local constants module.
app/alembic/versions/f24ed0e49df2_add_filter_anr.py	Migration now uses deprecated use-case via dishka + add temp stub column.
app/alembic/versions/c4888c68e221_fix_admin_attr_and_policy.py	Uses EntityTypeUseCase for attaching entity types during migration.
app/alembic/versions/ba78cef9700a_initial_entity_type.py	Uses EntityTypeUseCase; defers creation of new schema-related entity types.
app/alembic/versions/759d196145ae_.py	New migration to create new schema-related entity types + copy schema to directories.
app/alembic/versions/2dadf40c026a_add_system_flags_to_attribute_types.py	Migration now uses deprecated use-case for replication flags.
app/alembic/versions/275222846605_initial_ldap_schema.py	Refactors initial schema seed to use DTO collection + deprecated use-cases.
app/alembic/versions/01f3f05a5b11_add_primary_group_id.py	Uses EntityTypeUseCase for directory entity-type attachment in migration.

Comments suppressed due to low confidence (8)

app/ldap_protocol/utils/raw_definition_parser.py:1

• tmp.values() yields AttributeTypeInfo/ObjectClassInfo objects, but _list_to_string() is meant for iterables of strings and returns str | None. As written, both methods will return the wrong type (and may return None) despite their return annotations, breaking parsing at runtime. Fix by returning the single parsed info object directly (e.g., pick the only value from the mapping) and keep _list_to_string() for actual string-list fields like info.name/info.superior.
  app/ldap_protocol/utils/raw_definition_parser.py:1
• This error message is misleading in the object-class path (collect_object_class_dto_from_info). It should reference 'Object Class name' (not 'Attribute Type name') to help diagnose parsing failures correctly.
  app/ldap_protocol/ldap_schema/entity_type_use_case.py:1
• EntityTypeUseCase uses self.__session, but the diff does not show it being initialized in __init__. This will raise AttributeError when attach_entity_type_to_directories() runs (e.g., from migrations). Inject AsyncSession into the use-case (like other use-cases) and assign it, or refactor to execute these queries via a DAO that owns the session.
  app/ldap_protocol/ldap_schema/object_class_dir_create_use_case.py:1
• This uses the loop variable value from the prior for value in values loop, so the isinstance(value, str) check is unrelated to dir_.object_class and will be wrong (or even reference a stale value). Use dir_.object_class consistently in the check (as done in the AttributeType variant), or remove this extra objectClass insertion entirely if data['attributes']['objectClass'] already sets it. Also consider setting Directory.object_class to a meaningful value (e.g., classSchema) instead of \"\" to avoid creating an empty objectClass attribute.
  app/ldap_protocol/ldap_schema/object_class_dao.py:1
• This update path is inconsistent with the new directory-backed schema storage: self.get(name) returns a DTO, so mutating obj.attribute_types_* won't persist changes. Additionally, AttributeType here is imported from entities_appendix (a dataclass), so select(AttributeType) is not a valid SQLAlchemy selectable and will fail at runtime. To fix, load the underlying Directory + its Attribute rows (via get_dir()), then update/replace the appropriate Attribute records (attribute_types_must / attribute_types_may, superior_name, etc.), and flush.
  app/ldap_protocol/ldap_schema/object_class_dao.py:1
• The method name get_object_class_names_include_attribute_type1 looks like a temporary name and is unclear/confusing. Rename it to a stable API name (or remove it if superseded by get_object_class_names_include_attribute_type).
  tests/test_ldap/test_util/test_search.py:1
• Early return effectively disables the test while still reporting it as passed, which can hide regressions in the schema-migration work. Prefer pytest.skip(\"TODO: ...\") (or pytest.mark.xfail) so the test suite clearly reports the missing coverage; the same applies to the other newly-added early returns in this PR.
  docker-compose.test.yml:1
• Ignoring SyntaxWarning in CI can mask real issues introduced by the refactor (especially around SQLAlchemy queries and deprecated APIs). Consider removing this ignore, or narrowing it to a specific known warning source (module/class) so newly introduced warnings still fail or are visible.

---

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

[Copilot]

Pull request overview

Copilot reviewed 62 out of 64 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (11)

app/ldap_protocol/utils/raw_definition_parser.py:1

• _get_attribute_type_info() / get_object_class_info() now return _list_to_string(tmp.values()), but _list_to_string() returns str | None (and is intended for strings), not AttributeTypeInfo / ObjectClassInfo. This will break parsing at runtime. Restore returning the single ...Info instance (e.g., take the only element from tmp.values()), or generalize _list_to_string() with a TypeVar so it can return the single element of any iterable while preserving the return type.
  app/ldap_protocol/utils/raw_definition_parser.py:1
• The error message is incorrect for this context: this is parsing an Object Class name, but it raises ValueError(\"Attribute Type name is required\"). Update the message to refer to the Object Class name to make debugging actionable.
  app/ldap_protocol/ldap_schema/object_class_dir_create_use_case.py:1
• value here comes from the for name, values in data[\"attributes\"].items(): for value in values: loop above, so after the loop it will be the last iterated attribute value (or may be undefined if attributes is empty). This makes the objectClass attribute value incorrect/non-deterministic. Use dir_.object_class (and check isinstance(dir_.object_class, str)), or explicitly set the intended objectClass value(s) without relying on a loop-scoped variable.
  app/ldap_protocol/ldap_schema/object_class_dao.py:1
• attributes_dict.get(\"...\") can return None, and indexing [0] will raise TypeError. This is especially likely for optional fields like superior_name. Consider using a safe default (e.g., dir_.attributes_dict.get(\"superior_name\", [None])[0]) and similarly guard required fields with a clearer error when missing.
  app/ldap_protocol/ldap_schema/object_class_dao.py:1
• update() now loads obj via self.get(name), which returns an ObjectClassDTO, not a mutable ORM model/Directory entity. The method then tries to mutate obj.attribute_types_must with ORM AttributeType objects, which will either fail (DTO shape) or corrupt the DTO type (list[str] vs list[AttributeType]). Refactor update() to load and mutate the underlying Directory + Attribute rows (or delegate updates to the new Directory-backed create/update use case) instead of using the DTO.
  app/ldap_protocol/ldap_schema/object_class_dao.py:1
• update() now loads obj via self.get(name), which returns an ObjectClassDTO, not a mutable ORM model/Directory entity. The method then tries to mutate obj.attribute_types_must with ORM AttributeType objects, which will either fail (DTO shape) or corrupt the DTO type (list[str] vs list[AttributeType]). Refactor update() to load and mutate the underlying Directory + Attribute rows (or delegate updates to the new Directory-backed create/update use case) instead of using the DTO.
  app/ldap_protocol/ldap_schema/attribute_type_dao.py:1
• get_all() converts each Directory using attributes_dict, but the query does not eager-load Directory.attributes. That risks N+1 lazy loads and may produce incomplete attributes_dict depending on how it's computed. Align with other queries in this PR by adding selectinload(qa(Directory.attributes)) (and any other required relationships) before converting.
  app/ldap_protocol/ldap_schema/attribute_type_use_case.py:1
• set_attr_replication_flag() still exists on the use case, but its permission mapping was removed. If authorization enforcement relies on PERMISSIONS, this creates an authorization gap or breaks expected policy checks. Either re-add a permission mapping for set_attr_replication_flag (and keep/restore the corresponding AuthorizationRules entry), or remove/relocate the method if it is intentionally no longer part of the public service surface.
  app/ldap_protocol/ldap_schema/object_class_use_case.py:1
• superior_name is always stored as [str(dto.superior_name)]. When dto.superior_name is None, this persists the literal string 'None', which will later be treated as a real superior name. Only include superior_name when it is not None, or store an empty list/omit the attribute entirely.
  tests/test_ldap/test_util/test_search.py:1
• Returning early from a test makes it pass without exercising behavior or assertions, which can hide regressions in CI. Prefer pytest.skip(\"...\") (explicitly marking it as skipped with a reason) or pytest.xfail(\"...\") if it is expected to fail until the refactor is complete.
  docker-compose.test.yml:1
• Ignoring SyntaxWarning globally can mask real issues (especially around invalid escapes, deprecated syntax, or runtime parsing warnings) and makes the test signal weaker. Consider scoping the filter to the specific warning source/module or fixing the underlying warnings instead of suppressing them across the entire test run.

---

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

[Copilot]

Pull request overview

Copilot reviewed 82 out of 84 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (11)

app/ldap_protocol/ldap_schema/entity_type/entity_type_use_case.py:1

• EntityTypeUseCase calls self.__session.execute(...), but __session is never defined/initialized in this class (only DAOs are injected). This will raise AttributeError at runtime. Inject AsyncSession into the use-case (and assign it), or rewrite the method to execute via the DAO (e.g., add a DAO method to fetch directories missing entity_type_id).
  app/ldap_protocol/ldap_schema/object_class/object_class_dir_create_use_case.py:1
• This uses the loop variable value from the prior for value in values: loop, so the isinstance(value, str) check depends on the last attribute value seen, not on dir_.object_class. This can incorrectly store NULL for objectClass. Use dir_.object_class consistently (as in the attribute-type dir-create use-case) and avoid referencing the prior loop variable.
  app/ldap_protocol/ldap_schema/object_class/object_class_use_case.py:1
• Two schema fields are being serialized in a way that can persist invalid values into LDAP-like storage: (1) superior_name becomes the string 'None' when dto.superior_name is None; (2) kind is stored via str(dto.kind) which for Enums typically yields 'KindType.STRUCTURAL' rather than the expected value (e.g. 'STRUCTURAL'). Only include superior_name when it is not None, and serialize kind using dto.kind.value (or otherwise the enum value that the DTO/API expects).
  app/ldap_protocol/ldap_schema/object_class/object_class_dao.py:1
• dir_.attributes_dict.get('superior_name') (and kind) can be None or an empty list depending on how the Directory entry was created/seeded. Indexing [0] will raise TypeError/IndexError. Handle optional attributes safely (e.g., default to None when the key is missing or list is empty), matching the DTO's optionality for superior_name.
  app/ldap_protocol/ldap_schema/attribute_type/attribute_type_dao.py:1
• Per the docstring, is_included_anr should be modifiable for all attributes (system and non-system). In the current logic, is_included_anr is only updated for system attributes, and never updated for non-system attributes. Update is_included_anr regardless of dir_.is_system (and restrict only syntax/single_value/no_user_modification for system attributes).
  app/ldap_protocol/utils/raw_definition_parser.py:1
• _list_to_string is typed as Iterable[str] -> str|None, but it is used with tmp.values() where elements are AttributeTypeInfo/ObjectClassInfo, and the callers annotate the return type as AttributeTypeInfo/ObjectClassInfo. This is a type/contract mismatch that can hide real bugs (including returning None). Consider making the helper generic (e.g., Iterable[T] -> T) and raising a clearer error when the iterable is empty.
  app/ldap_protocol/utils/raw_definition_parser.py:1
• _list_to_string is typed as Iterable[str] -> str|None, but it is used with tmp.values() where elements are AttributeTypeInfo/ObjectClassInfo, and the callers annotate the return type as AttributeTypeInfo/ObjectClassInfo. This is a type/contract mismatch that can hide real bugs (including returning None). Consider making the helper generic (e.g., Iterable[T] -> T) and raising a clearer error when the iterable is empty.
  app/ldap_protocol/utils/raw_definition_parser.py:1
• _list_to_string is typed as Iterable[str] -> str|None, but it is used with tmp.values() where elements are AttributeTypeInfo/ObjectClassInfo, and the callers annotate the return type as AttributeTypeInfo/ObjectClassInfo. This is a type/contract mismatch that can hide real bugs (including returning None). Consider making the helper generic (e.g., Iterable[T] -> T) and raising a clearer error when the iterable is empty.
  app/ldap_protocol/utils/raw_definition_parser.py:1
• This error message is incorrect for the object-class code path: it says 'Attribute Type name is required' while validating an Object Class name. Update the message to 'Object Class name is required' (or similar) to aid debugging.
  app/extra/alembic_utils.py:1
• If func(*args, **kwargs) raises, the temporary stub column will never be dropped, potentially leaving the DB in an inconsistent state for subsequent retries. Wrap the call in try/finally so op.drop_column(...) always executes.
  app/ldap_protocol/ldap_schema/attribute_type/attribute_type_use_case.py:1
• set_attr_replication_flag remains a public use-case method that mutates schema state, but it is no longer present in the PERMISSIONS mapping. If authorization is enforced via AbstractService.PERMISSIONS, this can accidentally remove protection for this operation (or make it inconsistently enforced). Either add a dedicated authorization rule back and include it in PERMISSIONS, or make the method private/internal if it should not be externally callable.

---

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

[Copilot]

Pull request overview

Copilot reviewed 87 out of 89 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (8)

app/ldap_protocol/ldap_schema/raw_definition_parser.py:1

• _get_attribute_type_info() / get_object_class_info() return str | None (via _list_to_string) but are annotated/used as AttributeTypeInfo / ObjectClassInfo. This will crash later when accessing fields like .name/.oid. Return the parsed info object (e.g., first element of tmp.values()), and keep _list_to_string only for converting lists of names/superiors.
  app/ldap_protocol/ldap_schema/entity_type/entity_type_use_case.py:1
• EntityTypeUseCase uses self.__session but the class never defines/initializes __session. Calling attach_entity_type_to_directories() will raise AttributeError. Inject AsyncSession into this use-case (like other use-cases), or execute via a DAO method that already owns the session.
  app/ldap_protocol/ldap_schema/object_class/object_class_use_case.py:1
• superior_name is optional, but it is always persisted with values=[str(dto.superior_name)]. When dto.superior_name is None, this stores the literal string 'None', which breaks semantic meaning and will leak into API responses / LDAP raw definitions. Build attributes dynamically: only include subClassOf when dto.superior_name is not None (and/or store an empty values list, depending on how attributes_dict is expected to behave).
  app/ldap_protocol/ldap_schema/object_class/object_class_dao.py:1
• attributes_dict.get(...) can return None (or an empty list), and indexing [0] will raise at runtime. This is especially relevant for optional fields like SUPERIOR_NAME (root object classes). Consider using safer defaults (e.g., superior_vals = ...get(..., []) then superior_name = superior_vals[0] if superior_vals else None) and similarly guard required fields with a clearer ObjectClassNotFoundError/validation error if missing.
  app/ldap_protocol/ldap_schema/attribute_type/attribute_type_dao.py:1
• The docstring states ANR (is_included_anr) can be modified for all attributes (including system ones). Current logic updates ANR only for system attributes, but for non-system attributes it never updates Names.IS_INCLUDED_ANR. Update the loop to always handle IS_INCLUDED_ANR, while still restricting SYNTAX / SINGLE_VALUE / NO_USER_MODIFICATION updates to non-system attributes.
  app/ldap_protocol/ldap_schema/attribute_type/attribute_type_use_case.py:1
• set_attr_replication_flag() is still a public method on the service, but it is missing from the PERMISSIONS map. If authorization is enforced via AbstractService.PERMISSIONS, this can lead to the endpoint being unintentionally unprotected or unintentionally blocked (depending on framework behavior). Add an explicit permission mapping for set_attr_replication_flag (and ensure the corresponding AuthorizationRules value still exists).
  app/ldap_protocol/roles/migrations_ace_dao.py:1
• This performs one UPDATE statement per ACE row, which can be slow on large datasets during migration. Prefer a single bulk update strategy (e.g., UPDATE ... FROM (VALUES ...) / temporary table join, or a CASE expression keyed by ACE id) to reduce round-trips and transaction time.
  app/ldap_protocol/ldap_schema/raw_definition_parser.py:1
• This error is raised while collecting an ObjectClass DTO, but the message says "Attribute Type name is required". Update it to something like "Object Class name is required" to make debugging accurate.

---

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

[Copilot]

Pull request overview

Copilot reviewed 87 out of 89 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (10)

app/ldap_protocol/ldap_schema/entity_type/entity_type_use_case.py:1

• EntityTypeUseCase uses self.__session, but no session field is defined/initialized in this class. This will raise AttributeError at runtime (and affects multiple migrations calling attach_entity_type_to_directories). Inject AsyncSession into the use-case (like other use-cases), or move the query/flush logic into the DAO which already owns the session.
  app/ldap_protocol/ldap_schema/object_class/object_class_use_case.py:1
• When dto.superior_name is None, this stores the literal string 'None' as subClassOf. That will leak into API responses and LDAP raw definitions (e.g., SUP None) and breaks the meaning of “no superior”. Make SUPERIOR_NAME attribute conditional: only include it when dto.superior_name is not None (or store an empty list / omit the attribute).
  app/ldap_protocol/ldap_schema/object_class/object_class_dao.py:1
• superior_name=dir_.attributes_dict.get(...)[0] will throw when subClassOf is absent (which is valid for root object classes). This will also blow up if earlier migrations accidentally omit the attribute. Handle absence explicitly (e.g., read the list via .get(..., []) and set superior_name=None when empty), and consider normalizing a stored 'None' string back to None while data is being migrated.
  app/ldap_protocol/ldap_schema/attribute_type/attribute_type_dao.py:1
• The docstring above says ANR inclusion can be modified for all attributes (including non-system). Current logic updates aNR only for system attributes and never updates it for non-system attributes. Update the logic so Names.IS_INCLUDED_ANR is handled regardless of dir_.is_system, while keeping the restrictions for syntax/single_value/no_user_modification on system attributes.
  app/ldap_protocol/roles/migrations_ace_dao.py:1
• Unmapped ACEs are silently skipped (if row.name in directory_by_name). After the FK is switched to Directory.id, any remaining legacy ids in AccessControlEntries.attributeTypeId can cause foreign key violations (or worse: point to unrelated directories if IDs overlap). Consider enforcing completeness (raise if any row.name is missing), or explicitly setting attribute_type_id to NULL for unmapped rows, or creating missing ATTRIBUTE_TYPE directories before rebinding.
  app/ldap_protocol/roles/migrations_ace_dao.py:1
• This performs one UPDATE per ACE row, which can be slow on large datasets. Prefer a bulk update strategy (e.g., executemany with bind params, or a single UPDATE ... FROM (VALUES ...) style statement) to reduce roundtrips and lock time during migrations.
  app/ldap_protocol/ldap_schema/raw_definition_parser.py:1
• The error message is incorrect for this context: this is parsing an ObjectClass definition, not an AttributeType. Please change it to something like Object Class name is required to make debugging clearer.
  app/ldap_protocol/ldap_schema/schema_create_use_case.py:1
• Selecting the Configuration container by Directory.name alone is ambiguous: regular user data could also contain a cn=Configuration somewhere else. Since this method defines the parent for all schema directories, it should be uniquely identified (e.g., join EntityType and require EntityType.name == EntityTypeNames.CONFIGURATION, and/or constrain by being a base child). Otherwise schema objects may be created under the wrong subtree.
  docker-compose.test.yml:1
• Ignoring SyntaxWarning in the test runner can hide real issues (e.g., invalid escapes in regex strings) and make regressions harder to detect. Consider fixing the underlying warnings and removing this filter, or scoping it to the specific module/message that triggers the warning.
  app/ldap_protocol/ldap_schema/attribute_type/attribute_type_use_case.py:1
• set_attr_replication_flag is still part of the use-case API, but it is not included in the PERMISSIONS mapping anymore. If authorization relies on AbstractService.PERMISSIONS, this can lead to inconsistent access control (e.g., missing permission enforcement or runtime errors when resolving permissions). Either add an appropriate permission mapping back, or remove/relocate this method if it’s intentionally no longer exposed.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 88 out of 90 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (7)

app/ldap_protocol/ldap_schema/entity_type/entity_type_use_case.py:1

• EntityTypeUseCase uses self.__session, but the session is never set in __init__, so calling attach_entity_type_to_directories() will raise AttributeError at runtime. Fix by injecting AsyncSession into the constructor (like other use-cases) and assigning it to self.__session, or by moving the query logic into EntityTypeDAO (which already has a session) and calling it from the use-case.
  app/ldap_protocol/ldap_schema/object_class/object_class_use_case.py:1
• When dto.superior_name is None, this stores the literal string 'None' in Directory attributes, which breaks the API contract (superior_name should be null/None, not 'None'). Recommended fix: only include the SUPERIOR_NAME attribute when dto.superior_name is not None (or use an empty values=() and adjust DAO conversion accordingly).
  app/ldap_protocol/ldap_schema/object_class/object_class_dao.py:1
• attributes_dict.get(...)[0] will throw when the key is missing (e.g., superior_name is optional). This also returns raw strings for fields that are likely enums (kind) and should be converted consistently. Suggested fix: use safe defaults (e.g. None for SUPERIOR_NAME) and explicitly coerce/validate types (e.g. map kind string back to KindType).
  app/ldap_protocol/ldap_schema/attribute_type/attribute_type_dao.py:1
• The docstring says ANR inclusion can be modified for all attributes (including system ones), but the implementation only updates aNR for system attributes and never updates it for non-system attributes. Also, break stops scanning attributes early, which is risky if other update logic is added later. Fix: update aNR regardless of dir_.is_system, and keep the system/non-system restriction only for syntax/single_value/no_user_modification.
  app/ldap_protocol/roles/migrations_ace_dao.py:1
• This migration updates ACE rows one-by-one, which can be very slow on large databases. Prefer a set-based update (e.g., a single UPDATE ... FROM join between AccessControlEntries and Directory/EntityType, or a CASE WHEN update over a VALUES table) to reduce round-trips and lock time.
  app/ldap_protocol/ldap_schema/raw_definition_parser.py:1
• The error message is misleading for object class parsing (collect_object_class_dto_from_info): it should refer to the object class name (e.g., 'Object Class name is required') rather than 'Attribute Type'.
  docker-compose.test.yml:1
• Ignoring SyntaxWarning in CI can mask real issues (including invalid escape sequences and some forward-compat problems). Prefer fixing the underlying warnings and keeping CI strict, or suppress only specific known-warning modules/lines if needed.

---

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