Refactor department LDAP sync around canonical service flows
This commit is contained in:
parent
3dd13ec549
commit
81b8d29b38
@ -172,6 +172,36 @@ class DepartmentViewSet(viewsets.ModelViewSet):
|
||||
qs = qs.order_by("name")
|
||||
return qs
|
||||
|
||||
def _extract_department_ldap_changes(
|
||||
self, instance: Department, data: Dict[str, Any]
|
||||
) -> Dict[str, Any]:
|
||||
"""Собирает diff значимых LDAP-полей до сохранения."""
|
||||
changes: Dict[str, Any] = {}
|
||||
for field in ("name", "description"):
|
||||
if field not in data:
|
||||
continue
|
||||
new_value = data.get(field)
|
||||
if new_value != getattr(instance, field):
|
||||
changes[field] = new_value
|
||||
return changes
|
||||
|
||||
def _mark_department_sync_attrs(
|
||||
self,
|
||||
instance: Department,
|
||||
*,
|
||||
changes: Dict[str, Any] | None = None,
|
||||
sync_head: bool = False,
|
||||
) -> None:
|
||||
"""Передаёт post_save сигналу точный diff отдела."""
|
||||
if changes:
|
||||
existing = getattr(instance, "_ldap_changes", {}) or {}
|
||||
if not isinstance(existing, dict):
|
||||
existing = {}
|
||||
existing.update(changes)
|
||||
instance._ldap_changes = existing
|
||||
if sync_head:
|
||||
instance._ldap_sync_head = True
|
||||
|
||||
# --- частичное изменение ---
|
||||
|
||||
def _perform_set_head(
|
||||
@ -223,10 +253,27 @@ class DepartmentViewSet(viewsets.ModelViewSet):
|
||||
instance.head_appointed_at = timezone.now()
|
||||
else:
|
||||
instance.head_appointed_at = None
|
||||
self._mark_department_sync_attrs(instance, sync_head=True)
|
||||
instance.save(update_fields=["head", "head_appointed_at"])
|
||||
|
||||
return instance
|
||||
|
||||
def perform_update(self, serializer):
|
||||
"""Обновление отдела с фиксацией LDAP diff до save()."""
|
||||
instance = serializer.instance
|
||||
validated_data = serializer.validated_data
|
||||
self._mark_department_sync_attrs(
|
||||
instance,
|
||||
changes=self._extract_department_ldap_changes(
|
||||
instance, validated_data
|
||||
),
|
||||
sync_head=(
|
||||
"head" in validated_data
|
||||
and validated_data.get("head") != instance.head
|
||||
),
|
||||
)
|
||||
serializer.save()
|
||||
|
||||
def partial_update(self, request, *args, **kwargs) -> Response:
|
||||
"""Частичный апдейт отдела."""
|
||||
instance = self.get_object()
|
||||
@ -268,14 +315,12 @@ class DepartmentViewSet(viewsets.ModelViewSet):
|
||||
instance = result
|
||||
|
||||
# --- NAME / DESCRIPTION ---
|
||||
changes: Dict[str, Any] = {}
|
||||
for k in ("name", "description"):
|
||||
if k in data:
|
||||
changes[k] = data.get(k)
|
||||
changes = self._extract_department_ldap_changes(instance, data)
|
||||
|
||||
if changes:
|
||||
for k, v in changes.items():
|
||||
setattr(instance, k, v)
|
||||
self._mark_department_sync_attrs(instance, changes=changes)
|
||||
instance.save(update_fields=list(changes.keys()))
|
||||
|
||||
return Response(
|
||||
|
||||
@ -1,5 +1,22 @@
|
||||
# Чеклист рефакторинга LDAP-сервисов
|
||||
|
||||
## ✅ Завершено
|
||||
|
||||
### Обновление 10 апреля 2026: canonical Department sync
|
||||
|
||||
- [x] `DepartmentService` получил sync-oriented API:
|
||||
- `sync_department_state()`
|
||||
- `sync_department_delete()`
|
||||
- `sync_member_state()`
|
||||
- [x] Signals и Celery retry для Department используют один и тот же service-path
|
||||
- [x] Удалены вызовы устаревших private-методов retry-пути (`_move_user_to_department`, `_move_user_to_base_ou`)
|
||||
- [x] `Department` diff теперь формируется до `save()` через `_ldap_changes`
|
||||
- [x] Head sync теперь передаётся явно через `_ldap_sync_head`
|
||||
- [x] Добавлены регрессионные тесты для:
|
||||
- `department_member` retry (`is_active=True/False`)
|
||||
- `department_save` retry
|
||||
- API/view diff для `name`, `description`, `head`
|
||||
|
||||
## ✅ Завершено (19 марта 2026)
|
||||
|
||||
### Фаза 1: Фундамент архитектуры
|
||||
@ -44,9 +61,11 @@
|
||||
- [x] Добавить логирование через _log_operation()
|
||||
- [x] Рефакторить _touch_state на использование BaseService
|
||||
- [ ] Использовать GroupServiceRefactored
|
||||
- [x] Ввести единый Department sync contract для signals + retry
|
||||
- [x] Перенести `department_save`, `department_delete`, `department_member` на canonical service-path
|
||||
|
||||
**Статус:** ✅ Завершено (кроме миграции на GroupServiceRefactored)
|
||||
**Дата:** 19 марта 2026 г.
|
||||
**Дата:** 10 апреля 2026 г.
|
||||
|
||||
#### PositionService
|
||||
- [x] Наследовать от BaseService
|
||||
@ -83,7 +102,7 @@
|
||||
- [ ] Создать unit-тесты для BaseService
|
||||
- [ ] Создать unit-тесты для подсервисов (Password, Login, Mapper)
|
||||
- [ ] Создать интеграционные тесты для GroupServiceRefactored
|
||||
- [ ] Обновить существующие тесты
|
||||
- [x] Обновить существующие тесты для Department retry/diff
|
||||
- [ ] Проверить покрытие тестами (цель: >80%)
|
||||
|
||||
**Оценка:** 6-8 часов
|
||||
@ -150,12 +169,12 @@
|
||||
| UserLoginService | ✅ Готов | 100% |
|
||||
| UserMapperService | ✅ Готов | 100% |
|
||||
| GroupServiceRefactored | ✅ Готов | 100% |
|
||||
| DepartmentService | ✅ Рефакторен | 90% |
|
||||
| DepartmentService | ✅ Рефакторен | 95% |
|
||||
| PositionService | ✅ Рефакторен | 90% |
|
||||
| UserService | 🔄 Legacy | 0% |
|
||||
| SyncService | 🔄 Legacy | 0% |
|
||||
|
||||
**Общий прогресс:** 75% завершено (было 60%)
|
||||
**Общий прогресс:** 80% завершено
|
||||
|
||||
---
|
||||
|
||||
@ -210,5 +229,5 @@
|
||||
|
||||
---
|
||||
|
||||
**Последнее обновление:** 19 марта 2026 г.
|
||||
**Последнее обновление:** 10 апреля 2026 г.
|
||||
**Статус проекта:** 🟢 Активный рефакторинг
|
||||
|
||||
@ -1,9 +1,24 @@
|
||||
# Рефакторинг LDAP-сервисов: Архитектурные улучшения
|
||||
|
||||
**Дата:** 19 марта 2026 г.
|
||||
**Дата:** 10 апреля 2026 г.
|
||||
**Статус:** В процессе миграции
|
||||
**Цель:** Улучшение архитектуры, уменьшение дублирования, следование SOLID принципам
|
||||
|
||||
## Обновление 10 апреля 2026: canonical Department sync
|
||||
|
||||
Для департаментов и членства в отделах введён единый контракт синхронизации:
|
||||
|
||||
- `DepartmentService.sync_department_state(dept, *, created, changes, sync_head)`
|
||||
- `DepartmentService.sync_department_delete(*, object_pk, dept_dn=None)`
|
||||
- `DepartmentService.sync_member_state(employee, department, *, is_active, role=None)`
|
||||
|
||||
Ключевые правила после обновления:
|
||||
|
||||
- Django DB является источником истины для `Department`, `EmployeeDepartment.is_active` и role assignment.
|
||||
- `employees/signals/ldap/department.py` и retry-исполнители в `employees/tasks.py` больше не содержат отдельной LDAP-логики для отдела.
|
||||
- View-слой фиксирует diff через `_ldap_changes` и `_ldap_sync_head` до `save()`, чтобы post-save не вычислял изменения по уже обновлённой строке.
|
||||
- Retry-очередь воспроизводит тот же canonical sync, что и live signals, а не отдельный legacy-сценарий.
|
||||
|
||||
---
|
||||
|
||||
## Обзор изменений
|
||||
@ -200,7 +215,8 @@ with _ldap() as conn:
|
||||
|
||||
### Этап 2: Постепенная миграция (следующий шаг)
|
||||
|
||||
🔲 Обновить DepartmentService для использования BaseService
|
||||
✅ Обновить DepartmentService для использования BaseService
|
||||
✅ Выделить canonical sync path для Department signals + retry
|
||||
🔲 Обновить PositionService для использования BaseService
|
||||
🔲 Рефакторить UserService с использованием подсервисов
|
||||
🔲 Обновить вызовы GroupService на новый API
|
||||
|
||||
@ -1,7 +1,25 @@
|
||||
# Аудит модуля employees/ldap
|
||||
|
||||
**Дата:** 18 марта 2026 г.
|
||||
**Контекст:** После рефакторинга ViewSets (удалено -5576 строк)
|
||||
**Дата:** 10 апреля 2026 г.
|
||||
**Контекст:** Аудит после выравнивания Department LDAP sync и retry-очереди
|
||||
|
||||
## Обновление 10 апреля 2026
|
||||
|
||||
### Каноническая модель Department sync
|
||||
|
||||
- Источник истины для `Department`, `EmployeeDepartment.is_active` и ролей отдела теперь находится в Django DB.
|
||||
- `employees/signals/ldap/department.py` и `employees/tasks.py` больше не содержат отдельную handwritten LDAP-логику для отдела.
|
||||
- Оба пути делегируют в `DepartmentService`:
|
||||
- `sync_department_state()`
|
||||
- `sync_department_delete()`
|
||||
- `sync_member_state()`
|
||||
- Retry-очередь больше не вызывает устаревшие private-методы вроде `_move_user_to_department` и `_move_user_to_base_ou`.
|
||||
- Diff для `Department` формируется до `save()` через временные атрибуты `_ldap_changes` и `_ldap_sync_head`; post-save больше не пытается вычислять изменения повторным чтением уже сохранённой строки.
|
||||
|
||||
### Остаточные замечания
|
||||
|
||||
- `DepartmentService` по-прежнему содержит legacy CRUD API, которое мутирует DB (`add_member`, `remove_member`, `set_member_role`), но live sync и retry используют только sync-oriented методы.
|
||||
- `UserService` и часть legacy service-слоя всё ещё требуют отдельной консолидации.
|
||||
|
||||
## Текущее использование
|
||||
|
||||
@ -10,17 +28,14 @@
|
||||
#### DirectoryService (employees/ldap/directory_service.py)
|
||||
Используется в:
|
||||
- `api/v1/employees/views/auth.py` - RegisterAPIView
|
||||
- `employees/signals_ldap.py` - синхронизация Employee
|
||||
- `employees/signals_department.py` - синхронизация Department
|
||||
- `employees/signals_group.py` - синхронизация Group
|
||||
- `employees/signals/ldap/employee.py` - синхронизация Employee
|
||||
- `employees/signals/ldap/group.py` - синхронизация Group
|
||||
- `common/ldap_password_mixin.py` - смена пароля
|
||||
|
||||
**Используемые методы:**
|
||||
- `create_user()` - auth.py (регистрация новых пользователей)
|
||||
- `update_user()` - signals_ldap.py, ldap_password_mixin.py
|
||||
- `delete_user()` - signals_ldap.py
|
||||
- `update_department()` - signals_department.py
|
||||
- `delete_department()` - signals_department.py
|
||||
- `group_create()` - signals_group.py
|
||||
- `group_delete()` - signals_group.py
|
||||
- `group_rename()` - signals_group.py
|
||||
@ -31,9 +46,9 @@
|
||||
|
||||
#### Services (employees/ldap/services/*)
|
||||
Используются в сигналах через ленивый импорт:
|
||||
- `DepartmentService` - signals_department.py
|
||||
- `GroupService` - signals_department.py
|
||||
- `UserService` - signals_department.py
|
||||
- `DepartmentService` - `employees/signals/ldap/department.py`, `employees/tasks.py`
|
||||
- `GroupService` - через `DepartmentService`, `employees/signals/ldap/group.py`
|
||||
- `UserService` - через `DepartmentService`, `employees/signals/ldap/employee.py`
|
||||
- `SyncService` - management/commands/sync_directory.py
|
||||
|
||||
#### ORM Models (employees/ldap/orm_models.py)
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@ -90,8 +90,6 @@ class TestDepartmentServiceHelpers:
|
||||
@pytest.mark.django_db
|
||||
def test_reconcile_department_group(
|
||||
self,
|
||||
mock_ldap_context,
|
||||
mock_ldap_connection,
|
||||
mock_ldap_repository,
|
||||
mock_group_service,
|
||||
mock_user_service,
|
||||
@ -113,23 +111,38 @@ class TestDepartmentServiceHelpers:
|
||||
mock_query = mock_emp_dept.objects.filter.return_value
|
||||
mock_query.values_list.return_value = [1, 2]
|
||||
|
||||
with patch.object(
|
||||
service,
|
||||
'_ensure_department_group',
|
||||
return_value='CN=DEP_IT,DC=example,DC=com'
|
||||
):
|
||||
with patch.object(
|
||||
mock_user_service,
|
||||
'employee_ids_to_dns',
|
||||
return_value=['CN=Test,OU=Users,DC=example,DC=com']
|
||||
):
|
||||
# Act
|
||||
result = service._reconcile_department_group(
|
||||
mock_ldap_connection,
|
||||
sample_department,
|
||||
dept_dn
|
||||
)
|
||||
with patch(
|
||||
'employees.ldap.services.department_service.LdapSyncState.objects'
|
||||
) as mock_sync_objects:
|
||||
mock_sync_filter = Mock()
|
||||
mock_sync_filter.values_list.return_value = [
|
||||
('1', 'CN=One,OU=Users,DC=example,DC=com'),
|
||||
('2', 'CN=Two,OU=Users,DC=example,DC=com'),
|
||||
]
|
||||
mock_sync_objects.filter.return_value = mock_sync_filter
|
||||
|
||||
# Assert
|
||||
assert 'DEP_' in result
|
||||
assert mock_group_service.replace_members.called
|
||||
mock_group = Mock()
|
||||
|
||||
with patch.object(
|
||||
service,
|
||||
'_ensure_department_group',
|
||||
return_value='CN=DEP_IT,DC=example,DC=com'
|
||||
):
|
||||
with patch(
|
||||
'employees.ldap.services.department_service.'
|
||||
'LdapOrganizationalUnitGroup.objects.get',
|
||||
return_value=mock_group
|
||||
):
|
||||
# Act
|
||||
result = service._reconcile_department_group(
|
||||
sample_department,
|
||||
dept_dn
|
||||
)
|
||||
|
||||
# Assert
|
||||
assert 'DEP_' in result
|
||||
assert mock_group.member == [
|
||||
'CN=One,OU=Users,DC=example,DC=com',
|
||||
'CN=Two,OU=Users,DC=example,DC=com',
|
||||
]
|
||||
assert mock_group.save.called
|
||||
|
||||
@ -8,7 +8,6 @@ import logging
|
||||
from django.conf import settings
|
||||
from django.db.models.signals import post_delete, post_save
|
||||
from django.dispatch import receiver
|
||||
from django.utils import timezone
|
||||
|
||||
from employees.ldap import DepartmentService
|
||||
from employees.ldap.errors import (
|
||||
@ -27,177 +26,127 @@ def _is_ldap_enabled():
|
||||
return getattr(settings, "LDAP_ENABLED", False)
|
||||
|
||||
|
||||
def _department_changes(instance) -> dict:
|
||||
"""Возвращает временный diff для Department-синхронизации."""
|
||||
raw_changes = getattr(instance, "_ldap_changes", {}) or {}
|
||||
if not isinstance(raw_changes, dict):
|
||||
return {}
|
||||
return {
|
||||
key: value
|
||||
for key, value in raw_changes.items()
|
||||
if key in {"name", "description"}
|
||||
}
|
||||
|
||||
|
||||
@receiver(post_save, sender="employees.Department")
|
||||
def sync_department_to_ldap_on_save(sender, instance, created, **kwargs):
|
||||
"""Синхронизирует отдел с LDAP при создании/обновлении.
|
||||
|
||||
ВАЖНО: Создание/обновление LDAP OU
|
||||
происходит автоматически при сохранении Department.
|
||||
"""
|
||||
"""Синхронизирует отдел с LDAP при создании/обновлении."""
|
||||
if not _is_ldap_enabled():
|
||||
return
|
||||
|
||||
# Игнорируем вспомогательные сохранения
|
||||
if getattr(instance, "_skip_ldap_sync", False):
|
||||
return
|
||||
|
||||
changes = {} if created else _department_changes(instance)
|
||||
sync_head = bool(getattr(instance, "_ldap_sync_head", False))
|
||||
|
||||
if not created and not changes and not sync_head:
|
||||
return
|
||||
|
||||
try:
|
||||
svc = DepartmentService()
|
||||
|
||||
if created:
|
||||
# Создание OU в LDAP
|
||||
# DirectoryService.create_department создаст OU
|
||||
# и вернет обновленный Department
|
||||
# Но мы уже в post_save, поэтому просто создаем OU
|
||||
try:
|
||||
# Проверяем, есть ли уже LDAP sync state
|
||||
sync_state = LdapSyncState.objects.filter(
|
||||
model="department", object_pk=str(instance.pk)
|
||||
).first()
|
||||
|
||||
if not sync_state or not sync_state.ldap_dn:
|
||||
# Создаем OU в LDAP
|
||||
from employees.ldap.services.department_service import (
|
||||
DepartmentService,
|
||||
)
|
||||
from employees.ldap.services.group_service import (
|
||||
GroupService,
|
||||
)
|
||||
from employees.ldap.services.user_service import UserService
|
||||
|
||||
group_service = GroupService()
|
||||
user_service = UserService(group_service)
|
||||
dept_service = DepartmentService(
|
||||
group_service, user_service
|
||||
)
|
||||
|
||||
dept_service._ensure_department_ou(instance)
|
||||
|
||||
if instance.description:
|
||||
dept_service._set_ou_description(
|
||||
instance, instance.description
|
||||
)
|
||||
|
||||
if instance.head:
|
||||
dept_service._set_ou_managed_by(instance, instance.head)
|
||||
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
f"LDAP OU creation failed for Department {instance.id}: {
|
||||
e
|
||||
}",
|
||||
exc_info=True,
|
||||
)
|
||||
_enqueue(
|
||||
"department_save",
|
||||
"department",
|
||||
instance.pk,
|
||||
{
|
||||
"object_pk": str(instance.pk),
|
||||
"created": True,
|
||||
},
|
||||
)
|
||||
else:
|
||||
# Обновление OU в LDAP
|
||||
# Проверяем что изменилось
|
||||
changes = {}
|
||||
|
||||
# Проверим, есть ли изменения в name/description
|
||||
old_instance = sender.objects.filter(pk=instance.pk).first()
|
||||
if old_instance:
|
||||
if instance.name != old_instance.name:
|
||||
changes["name"] = instance.name
|
||||
if instance.description != old_instance.description:
|
||||
changes["description"] = instance.description
|
||||
|
||||
if changes or instance.head_id:
|
||||
# Есть изменения - синхронизируем
|
||||
try:
|
||||
if changes:
|
||||
svc.update_department(instance, changes)
|
||||
|
||||
# Обновляем managedBy если изменился head
|
||||
if instance.head_id:
|
||||
from employees.ldap.services.department_service import (
|
||||
DepartmentService,
|
||||
)
|
||||
from employees.ldap.services.group_service import (
|
||||
GroupService,
|
||||
)
|
||||
from employees.ldap.services.user_service import (
|
||||
UserService,
|
||||
)
|
||||
|
||||
group_service = GroupService()
|
||||
user_service = UserService(group_service)
|
||||
dept_service = DepartmentService(
|
||||
group_service, user_service
|
||||
)
|
||||
dept_service._set_ou_managed_by(instance, instance.head)
|
||||
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
f"LDAP OU update failed for Department {instance.id}: "
|
||||
f"{e}",
|
||||
exc_info=True,
|
||||
)
|
||||
_enqueue(
|
||||
"department_save",
|
||||
"department",
|
||||
instance.pk,
|
||||
{
|
||||
"object_pk": str(instance.pk),
|
||||
"created": False,
|
||||
"changes": changes,
|
||||
},
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
DepartmentService().sync_department_state(
|
||||
instance,
|
||||
created=created,
|
||||
changes=changes,
|
||||
sync_head=sync_head,
|
||||
)
|
||||
except (DirectoryLdapError, DirectoryServiceError, DirectoryDbError) as e:
|
||||
logger.error(
|
||||
f"Unexpected error in LDAP sync for Department {instance.id}: {e}",
|
||||
"LDAP sync failed for Department %s (created=%s): %s",
|
||||
instance.id,
|
||||
created,
|
||||
e,
|
||||
exc_info=True,
|
||||
)
|
||||
payload = {
|
||||
"object_pk": str(instance.pk),
|
||||
"created": created,
|
||||
"changes": changes,
|
||||
}
|
||||
if sync_head:
|
||||
payload["sync_head"] = True
|
||||
_enqueue(
|
||||
"department_save",
|
||||
"department",
|
||||
instance.pk,
|
||||
payload,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
"Unexpected error in LDAP sync for Department %s: %s",
|
||||
instance.id,
|
||||
e,
|
||||
exc_info=True,
|
||||
)
|
||||
finally:
|
||||
if hasattr(instance, "_ldap_changes"):
|
||||
delattr(instance, "_ldap_changes")
|
||||
if hasattr(instance, "_ldap_sync_head"):
|
||||
delattr(instance, "_ldap_sync_head")
|
||||
if hasattr(instance, "_skip_ldap_sync"):
|
||||
delattr(instance, "_skip_ldap_sync")
|
||||
|
||||
|
||||
@receiver(post_delete, sender="employees.Department")
|
||||
def sync_department_to_ldap_on_delete(sender, instance, **kwargs):
|
||||
"""Удаляет OU из LDAP при удалении Department."""
|
||||
"""Удаляет LDAP-следы отдела при удалении Department."""
|
||||
if not _is_ldap_enabled():
|
||||
return
|
||||
|
||||
dept_dn = (
|
||||
LdapSyncState.objects.filter(
|
||||
model="department", object_pk=str(instance.pk)
|
||||
)
|
||||
.values_list("ldap_dn", flat=True)
|
||||
.first()
|
||||
)
|
||||
|
||||
try:
|
||||
svc = DepartmentService()
|
||||
svc.delete_department(instance)
|
||||
logger.info(f"Deleted Department {instance.id} from LDAP")
|
||||
DepartmentService().sync_department_delete(
|
||||
object_pk=instance.pk,
|
||||
dept_dn=dept_dn,
|
||||
)
|
||||
logger.info("Deleted Department %s from LDAP", instance.id)
|
||||
except (DirectoryLdapError, DirectoryServiceError, DirectoryDbError) as e:
|
||||
logger.error(
|
||||
f"LDAP OU delete failed for Department {instance.id}: {e}",
|
||||
"LDAP delete failed for Department %s: %s",
|
||||
instance.id,
|
||||
e,
|
||||
exc_info=True,
|
||||
)
|
||||
payload = {"object_pk": str(instance.pk)}
|
||||
if dept_dn:
|
||||
payload["dept_dn"] = dept_dn
|
||||
_enqueue(
|
||||
"department_delete",
|
||||
"department",
|
||||
instance.pk,
|
||||
{
|
||||
"object_pk": str(instance.pk),
|
||||
},
|
||||
payload,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
f"Unexpected error in LDAP delete for Department {instance.id}: {
|
||||
e
|
||||
}",
|
||||
"Unexpected error in LDAP delete for Department %s: %s",
|
||||
instance.id,
|
||||
e,
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
|
||||
@receiver(post_save, sender="employees.EmployeeDepartment")
|
||||
def sync_department_member_to_ldap(sender, instance, created, **kwargs):
|
||||
"""Синхронизирует добавление/изменение члена отдела с LDAP.
|
||||
"""Синхронизирует членство сотрудника в отделе с LDAP."""
|
||||
del created, sender, kwargs
|
||||
|
||||
is_active=True → перемещаем в OU отдела + добавляем в группу
|
||||
is_active=False → перемещаем в Users/Dismissed OU + убираем из группы
|
||||
"""
|
||||
if not _is_ldap_enabled():
|
||||
return
|
||||
|
||||
@ -205,120 +154,17 @@ def sync_department_member_to_ldap(sender, instance, created, **kwargs):
|
||||
return
|
||||
|
||||
try:
|
||||
from employees.ldap.infrastructure.connections import _ldap
|
||||
from employees.ldap.repositories.ldap_repository import (
|
||||
ensure_container_exists,
|
||||
DepartmentService().sync_member_state(
|
||||
instance.employee,
|
||||
instance.department,
|
||||
is_active=instance.is_active,
|
||||
role=instance.role,
|
||||
)
|
||||
from employees.ldap.services.department_service import DepartmentService
|
||||
from employees.ldap.services.group_service import GroupService
|
||||
from employees.ldap.services.user_service import UserService
|
||||
from employees.ldap.utils.dn_utils import _move_to_department
|
||||
from employees.ldap.utils.ldap_utils import get_base_dn_for_employee
|
||||
from employees.ldap.services.constants import SyncDirection
|
||||
|
||||
group_service = GroupService()
|
||||
user_service = UserService(group_service)
|
||||
dept_service = DepartmentService(group_service, user_service)
|
||||
|
||||
employee = instance.employee
|
||||
department = instance.department
|
||||
|
||||
# Получаем DN сотрудника
|
||||
try:
|
||||
emp_dn = user_service._get_employee_dn(employee)
|
||||
except Exception:
|
||||
logger.debug(
|
||||
f"Employee {employee.id} has no LDAP DN, skipping member sync"
|
||||
)
|
||||
return
|
||||
|
||||
if instance.is_active:
|
||||
# === Активация: перемещаем в OU отдела + добавляем в группу ===
|
||||
try:
|
||||
dept_dn = dept_service._get_department_dn(department)
|
||||
except Exception:
|
||||
dept_dn = None
|
||||
|
||||
with _ldap() as conn:
|
||||
ensured_dn = dept_service._ensure_department_ou(
|
||||
conn, department.name
|
||||
)
|
||||
if not dept_dn or dept_dn != ensured_dn:
|
||||
dept_service._touch_state(
|
||||
model="department",
|
||||
object_pk=department.pk,
|
||||
ldap_dn=ensured_dn,
|
||||
last_django_modify_ts=timezone.now(),
|
||||
sync_dir=SyncDirection.AUTO,
|
||||
)
|
||||
dept_dn = ensured_dn
|
||||
|
||||
# Перемещаем пользователя в OU отдела
|
||||
new_dn = _move_to_department(conn, emp_dn, dept_dn)
|
||||
if new_dn != emp_dn:
|
||||
dept_service._touch_state(
|
||||
model="employee",
|
||||
object_pk=employee.pk,
|
||||
ldap_dn=new_dn,
|
||||
sync_dir=SyncDirection.LDAP,
|
||||
)
|
||||
|
||||
# Добавляем в группу отдела
|
||||
group_dn = dept_service._ensure_department_group(
|
||||
conn, department, dept_dn
|
||||
)
|
||||
if group_dn:
|
||||
group_service.add_members(group_dn, [new_dn])
|
||||
|
||||
# Устанавливаем роль если есть
|
||||
if instance.role:
|
||||
dept_service.set_member_role(
|
||||
department, employee, instance.role
|
||||
)
|
||||
else:
|
||||
# === Деактивация: убираем из группы
|
||||
# + перемещаем в Users/Dismissed OU ===
|
||||
with _ldap() as conn:
|
||||
# Убираем из группы отдела
|
||||
grp_dn = (department.ldap_group_dn or "").strip()
|
||||
if grp_dn:
|
||||
try:
|
||||
group_service.remove_members(grp_dn, [emp_dn])
|
||||
except Exception as grp_err:
|
||||
logger.warning(
|
||||
f"Failed to remove {employee.id} "
|
||||
f"from group {grp_dn}: {grp_err}"
|
||||
)
|
||||
|
||||
# Определяем целевой OU
|
||||
target_base = get_base_dn_for_employee(employee)
|
||||
|
||||
# Проверяем: уже в целевом OU?
|
||||
parts = emp_dn.split(",", 1)
|
||||
already_there = (
|
||||
len(parts) == 2 and parts[1].lower() == target_base.lower()
|
||||
)
|
||||
if not already_there:
|
||||
ensure_container_exists(conn, target_base)
|
||||
new_dn = user_service._move_user_to_base(
|
||||
conn, emp_dn, target_base
|
||||
)
|
||||
dept_service._touch_state(
|
||||
model="employee",
|
||||
object_pk=employee.pk,
|
||||
ldap_dn=new_dn,
|
||||
sync_dir=SyncDirection.LDAP,
|
||||
)
|
||||
logger.info(
|
||||
f"Moved employee {employee.id} "
|
||||
f"from {emp_dn} to {new_dn}"
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
f"LDAP member sync failed for EmployeeDepartment {instance.id}: {
|
||||
e
|
||||
}",
|
||||
"LDAP member sync failed for EmployeeDepartment %s: %s",
|
||||
instance.id,
|
||||
e,
|
||||
exc_info=True,
|
||||
)
|
||||
_enqueue(
|
||||
@ -332,3 +178,6 @@ def sync_department_member_to_ldap(sender, instance, created, **kwargs):
|
||||
"role": str(instance.role) if instance.role else None,
|
||||
},
|
||||
)
|
||||
finally:
|
||||
if hasattr(instance, "_skip_ldap_sync"):
|
||||
delattr(instance, "_skip_ldap_sync")
|
||||
|
||||
@ -13,6 +13,22 @@ from django.utils import timezone
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _require_payload_value(payload: dict, key: str):
|
||||
"""Возвращает обязательное поле payload или поднимает ValueError."""
|
||||
value = payload.get(key)
|
||||
if value in (None, ""):
|
||||
raise ValueError(f"LDAP queue payload missing required field: {key}")
|
||||
return value
|
||||
|
||||
|
||||
def _payload_changes(payload: dict) -> dict:
|
||||
"""Нормализует поле changes и валидирует его тип."""
|
||||
changes = payload.get("changes") or {}
|
||||
if not isinstance(changes, dict):
|
||||
raise ValueError("LDAP queue payload field 'changes' must be a dict")
|
||||
return changes
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Исполнители отложенных LDAP-операций
|
||||
# ---------------------------------------------------------------------------
|
||||
@ -58,71 +74,88 @@ def _execute_employee_delete(payload: dict) -> None:
|
||||
|
||||
def _execute_department_save(payload: dict) -> None:
|
||||
"""Повторяет синхронизацию Department → LDAP."""
|
||||
from employees.ldap.services.department_service import DepartmentService
|
||||
from employees.ldap.services.group_service import GroupService
|
||||
from employees.ldap.services.user_service import UserService
|
||||
from employees.ldap import DepartmentService
|
||||
from employees.models import Department
|
||||
|
||||
dept = Department.objects.get(pk=payload["object_pk"])
|
||||
group_service = GroupService()
|
||||
user_service = UserService(group_service)
|
||||
dept_service = DepartmentService(group_service, user_service)
|
||||
object_pk = _require_payload_value(payload, "object_pk")
|
||||
dept = Department.objects.filter(pk=object_pk).first()
|
||||
if dept is None:
|
||||
logger.info(
|
||||
"Department %s no longer exists, skipping queued sync",
|
||||
object_pk,
|
||||
)
|
||||
return
|
||||
|
||||
if payload.get("created"):
|
||||
dept_service._ensure_department_ou(dept)
|
||||
if dept.description:
|
||||
dept_service._set_ou_description(dept, dept.description)
|
||||
if dept.head:
|
||||
dept_service._set_ou_managed_by(dept, dept.head)
|
||||
else:
|
||||
changes = payload.get("changes", {})
|
||||
if changes:
|
||||
svc = DepartmentService()
|
||||
svc.update_department(dept, changes)
|
||||
if dept.head_id:
|
||||
dept_service._set_ou_managed_by(dept, dept.head)
|
||||
DepartmentService().sync_department_state(
|
||||
dept,
|
||||
created=bool(payload.get("created")),
|
||||
changes=_payload_changes(payload),
|
||||
sync_head=bool(payload.get("sync_head")),
|
||||
)
|
||||
|
||||
|
||||
def _execute_department_delete(payload: dict) -> None:
|
||||
"""Повторяет удаление Department OU из LDAP."""
|
||||
from employees.ldap import DepartmentService
|
||||
from employees.models import LdapSyncState
|
||||
|
||||
sync_state = LdapSyncState.objects.filter(
|
||||
model="department", object_pk=payload["object_pk"]
|
||||
).first()
|
||||
if not sync_state or not sync_state.ldap_dn:
|
||||
return
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
dept_stub = SimpleNamespace(
|
||||
pk=payload["object_pk"], id=payload["object_pk"]
|
||||
DepartmentService().sync_department_delete(
|
||||
object_pk=_require_payload_value(payload, "object_pk"),
|
||||
dept_dn=payload.get("dept_dn"),
|
||||
)
|
||||
svc = DepartmentService()
|
||||
svc.delete_department(dept_stub)
|
||||
|
||||
|
||||
def _execute_department_member(payload: dict) -> None:
|
||||
"""Повторяет sync члена отдела → LDAP."""
|
||||
from employees.ldap.services.department_service import DepartmentService
|
||||
from employees.ldap.services.group_service import GroupService
|
||||
from employees.ldap.services.user_service import UserService
|
||||
from employees.models import Employee, Department
|
||||
from employees.ldap import DepartmentService
|
||||
from employees.models import Department, Employee, EmployeeDepartment
|
||||
|
||||
emp = Employee.objects.get(pk=payload["employee_pk"])
|
||||
dept = Department.objects.get(pk=payload["department_pk"])
|
||||
employee_pk = _require_payload_value(payload, "employee_pk")
|
||||
department_pk = _require_payload_value(payload, "department_pk")
|
||||
|
||||
group_service = GroupService()
|
||||
user_service = UserService(group_service)
|
||||
dept_service = DepartmentService(group_service, user_service)
|
||||
emp = Employee.objects.filter(pk=employee_pk).first()
|
||||
if emp is None:
|
||||
logger.info(
|
||||
"Employee %s no longer exists, skipping queued department member sync",
|
||||
employee_pk,
|
||||
)
|
||||
return
|
||||
|
||||
if payload.get("is_active"):
|
||||
dept_service._move_user_to_department(emp, dept)
|
||||
if payload.get("role"):
|
||||
dept_service.set_member_role(dept, emp, payload["role"])
|
||||
dept = Department.objects.filter(pk=department_pk).first()
|
||||
if dept is None:
|
||||
logger.info(
|
||||
"Department %s no longer exists, skipping queued member sync",
|
||||
department_pk,
|
||||
)
|
||||
return
|
||||
|
||||
link = (
|
||||
EmployeeDepartment.objects.filter(
|
||||
employee_id=employee_pk,
|
||||
department_id=department_pk,
|
||||
)
|
||||
.select_related("role")
|
||||
.first()
|
||||
)
|
||||
|
||||
is_active = bool(payload.get("is_active"))
|
||||
role_hint = payload.get("role")
|
||||
if link is not None:
|
||||
is_active = link.is_active
|
||||
role_hint = link.role or role_hint
|
||||
else:
|
||||
dept_service._move_user_to_base_ou(emp)
|
||||
logger.warning(
|
||||
"EmployeeDepartment link missing for employee=%s department=%s, "
|
||||
"falling back to queued payload state",
|
||||
employee_pk,
|
||||
department_pk,
|
||||
)
|
||||
|
||||
DepartmentService().sync_member_state(
|
||||
emp,
|
||||
dept,
|
||||
is_active=is_active,
|
||||
role=role_hint,
|
||||
)
|
||||
|
||||
|
||||
def _execute_group_save(payload: dict) -> None:
|
||||
@ -278,7 +311,9 @@ def process_ldap_queue_item(self, queue_id: int):
|
||||
item.save(update_fields=["status", "updated_at"])
|
||||
|
||||
try:
|
||||
executor(item.payload)
|
||||
payload = dict(item.payload or {})
|
||||
payload.setdefault("object_pk", item.object_pk)
|
||||
executor(payload)
|
||||
item.mark_completed()
|
||||
logger.info(
|
||||
"LDAP queue item %d (%s %s:%s) completed on attempt %d",
|
||||
|
||||
@ -46,8 +46,10 @@
|
||||
# * `extract_results` — вытаскивает `results` из пагинированного ответа DRF или возвращает список как есть.
|
||||
|
||||
import itertools
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from django.test import override_settings
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.urls import reverse
|
||||
from employees.models import (
|
||||
@ -314,6 +316,128 @@ def test_partial_update_head_same_value_does_not_require_extra_perm(
|
||||
d.refresh_from_db()
|
||||
assert d.head_id == a.id
|
||||
|
||||
|
||||
@override_settings(LDAP_ENABLED=True)
|
||||
def test_partial_update_passes_name_description_diff_to_ldap_sync(
|
||||
api_client: APIClient,
|
||||
):
|
||||
with override_settings(LDAP_ENABLED=False):
|
||||
head = make_user("patch-head@example.com")
|
||||
d = Department.objects.create(
|
||||
name="Dept",
|
||||
description="old description",
|
||||
head=head,
|
||||
)
|
||||
emp = make_user("patch-manager@example.com")
|
||||
role = make_role(d, "mgr", ["manage_department"])
|
||||
EmployeeDepartment.objects.create(
|
||||
employee=emp,
|
||||
department=d,
|
||||
is_active=True,
|
||||
role=role,
|
||||
)
|
||||
|
||||
api_client.force_authenticate(user=emp)
|
||||
url = reverse("api:v1:departments-detail", args=[d.pk])
|
||||
|
||||
with patch(
|
||||
"employees.signals.ldap.department.DepartmentService.sync_department_state"
|
||||
) as mock_sync:
|
||||
resp = api_client.patch(
|
||||
url,
|
||||
{"name": "Dept Renamed", "description": "new description"},
|
||||
format="json",
|
||||
)
|
||||
|
||||
assert resp.status_code == status.HTTP_200_OK
|
||||
mock_sync.assert_called_once_with(
|
||||
d,
|
||||
created=False,
|
||||
changes={
|
||||
"name": "Dept Renamed",
|
||||
"description": "new description",
|
||||
},
|
||||
sync_head=False,
|
||||
)
|
||||
|
||||
|
||||
@override_settings(LDAP_ENABLED=True)
|
||||
def test_partial_update_head_passes_sync_head_to_ldap_sync(
|
||||
api_client: APIClient,
|
||||
):
|
||||
with override_settings(LDAP_ENABLED=False):
|
||||
head = make_user("head-old@example.com")
|
||||
candidate = make_user("head-new@example.com")
|
||||
d = Department.objects.create(name="Dept", head=head)
|
||||
emp = make_user("head-manager@example.com")
|
||||
role = make_role(
|
||||
d, "mgr", ["manage_department", "change_department_head"]
|
||||
)
|
||||
EmployeeDepartment.objects.create(
|
||||
employee=emp,
|
||||
department=d,
|
||||
is_active=True,
|
||||
role=role,
|
||||
)
|
||||
|
||||
api_client.force_authenticate(user=emp)
|
||||
url = reverse("api:v1:departments-detail", args=[d.pk])
|
||||
|
||||
with patch(
|
||||
"employees.signals.ldap.department.DepartmentService.sync_department_state"
|
||||
) as mock_sync:
|
||||
resp = api_client.patch(url, {"head_id": candidate.id}, format="json")
|
||||
|
||||
assert resp.status_code == status.HTTP_200_OK
|
||||
mock_sync.assert_called_once_with(
|
||||
d,
|
||||
created=False,
|
||||
changes={},
|
||||
sync_head=True,
|
||||
)
|
||||
|
||||
|
||||
@override_settings(LDAP_ENABLED=True)
|
||||
def test_full_update_passes_department_diff_to_ldap_sync(
|
||||
api_client: APIClient,
|
||||
):
|
||||
with override_settings(LDAP_ENABLED=False):
|
||||
d = Department.objects.create(
|
||||
name="Dept Full",
|
||||
description="before update",
|
||||
)
|
||||
emp = make_user("put-manager@example.com")
|
||||
role = make_role(d, "mgr", ["manage_department"])
|
||||
EmployeeDepartment.objects.create(
|
||||
employee=emp,
|
||||
department=d,
|
||||
is_active=True,
|
||||
role=role,
|
||||
)
|
||||
|
||||
api_client.force_authenticate(user=emp)
|
||||
url = reverse("api:v1:departments-detail", args=[d.pk])
|
||||
|
||||
with patch(
|
||||
"employees.signals.ldap.department.DepartmentService.sync_department_state"
|
||||
) as mock_sync:
|
||||
resp = api_client.put(
|
||||
url,
|
||||
{"name": "Dept Full Updated", "description": "after update"},
|
||||
format="json",
|
||||
)
|
||||
|
||||
assert resp.status_code == status.HTTP_200_OK
|
||||
mock_sync.assert_called_once_with(
|
||||
d,
|
||||
created=False,
|
||||
changes={
|
||||
"name": "Dept Full Updated",
|
||||
"description": "after update",
|
||||
},
|
||||
sync_head=False,
|
||||
)
|
||||
|
||||
# ---------- tests: action set_head ----------
|
||||
|
||||
def test_set_head_by_role_with_perm(api_client: APIClient):
|
||||
|
||||
@ -240,6 +240,143 @@ class TestProcessQueue:
|
||||
mock_task.delay.assert_not_called()
|
||||
|
||||
|
||||
class TestDepartmentQueueExecutors:
|
||||
|
||||
@override_settings(LDAP_ENABLED=True)
|
||||
def test_department_member_retry_reconciles_active_membership(
|
||||
self, db, user_factory
|
||||
):
|
||||
from employees.models import Department, EmployeeDepartment, LdapSyncQueue
|
||||
from employees.tasks import process_ldap_queue_item
|
||||
|
||||
with override_settings(LDAP_ENABLED=False):
|
||||
employee = user_factory(email="member-active@example.com")
|
||||
department = Department.objects.create(name="Dept Active Queue")
|
||||
link = EmployeeDepartment.objects.create(
|
||||
employee=employee,
|
||||
department=department,
|
||||
is_active=True,
|
||||
)
|
||||
|
||||
item = LdapSyncQueue.objects.create(
|
||||
operation="department_member",
|
||||
model_name="employee_department",
|
||||
object_pk=str(link.pk),
|
||||
payload={
|
||||
"employee_pk": str(employee.pk),
|
||||
"department_pk": str(department.pk),
|
||||
"is_active": True,
|
||||
"role": None,
|
||||
},
|
||||
)
|
||||
|
||||
with patch(
|
||||
"employees.ldap.services.department_service.DepartmentService.sync_member_state"
|
||||
) as mock_sync:
|
||||
process_ldap_queue_item(item.pk)
|
||||
|
||||
item.refresh_from_db()
|
||||
assert item.status == LdapSyncQueue.Status.COMPLETED
|
||||
mock_sync.assert_called_once_with(
|
||||
employee,
|
||||
department,
|
||||
is_active=True,
|
||||
role=None,
|
||||
)
|
||||
|
||||
@override_settings(LDAP_ENABLED=True)
|
||||
def test_department_member_retry_reconciles_inactive_membership(
|
||||
self, db, user_factory
|
||||
):
|
||||
from employees.models import Department, EmployeeDepartment, LdapSyncQueue
|
||||
from employees.tasks import process_ldap_queue_item
|
||||
|
||||
with override_settings(LDAP_ENABLED=False):
|
||||
employee = user_factory(email="member-inactive@example.com")
|
||||
department = Department.objects.create(name="Dept Inactive Queue")
|
||||
link = EmployeeDepartment.objects.create(
|
||||
employee=employee,
|
||||
department=department,
|
||||
is_active=False,
|
||||
)
|
||||
|
||||
item = LdapSyncQueue.objects.create(
|
||||
operation="department_member",
|
||||
model_name="employee_department",
|
||||
object_pk=str(link.pk),
|
||||
payload={
|
||||
"employee_pk": str(employee.pk),
|
||||
"department_pk": str(department.pk),
|
||||
"is_active": False,
|
||||
"role": None,
|
||||
},
|
||||
)
|
||||
|
||||
with patch(
|
||||
"employees.ldap.services.department_service.DepartmentService.sync_member_state"
|
||||
) as mock_sync:
|
||||
process_ldap_queue_item(item.pk)
|
||||
|
||||
item.refresh_from_db()
|
||||
assert item.status == LdapSyncQueue.Status.COMPLETED
|
||||
mock_sync.assert_called_once_with(
|
||||
employee,
|
||||
department,
|
||||
is_active=False,
|
||||
role=None,
|
||||
)
|
||||
|
||||
@override_settings(LDAP_ENABLED=True)
|
||||
def test_department_save_retry_uses_canonical_sync(
|
||||
self, db, user_factory
|
||||
):
|
||||
from employees.models import Department, LdapSyncQueue
|
||||
from employees.tasks import process_ldap_queue_item
|
||||
|
||||
with override_settings(LDAP_ENABLED=False):
|
||||
head = user_factory(email="dept-head-queue@example.com")
|
||||
department = Department.objects.create(
|
||||
name="Dept Queue Original",
|
||||
description="old description",
|
||||
)
|
||||
department.name = "Dept Queue Updated"
|
||||
department.description = "new description"
|
||||
department.head = head
|
||||
department.save(update_fields=["name", "description", "head"])
|
||||
|
||||
item = LdapSyncQueue.objects.create(
|
||||
operation="department_save",
|
||||
model_name="department",
|
||||
object_pk=str(department.pk),
|
||||
payload={
|
||||
"object_pk": str(department.pk),
|
||||
"created": False,
|
||||
"changes": {
|
||||
"name": "Dept Queue Updated",
|
||||
"description": "new description",
|
||||
},
|
||||
"sync_head": True,
|
||||
},
|
||||
)
|
||||
|
||||
with patch(
|
||||
"employees.ldap.services.department_service.DepartmentService.sync_department_state"
|
||||
) as mock_sync:
|
||||
process_ldap_queue_item(item.pk)
|
||||
|
||||
item.refresh_from_db()
|
||||
assert item.status == LdapSyncQueue.Status.COMPLETED
|
||||
mock_sync.assert_called_once_with(
|
||||
department,
|
||||
created=False,
|
||||
changes={
|
||||
"name": "Dept Queue Updated",
|
||||
"description": "new description",
|
||||
},
|
||||
sync_head=True,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Сигналы: проверка enqueue при ошибке LDAP
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user