Add: RenameRequest for entry (LDAP object) #918
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an API endpoint to rename an LDAP entry by running ModifyDN first and then Modify, including a rollback path when Modify fails.
Changes:
- Introduced
RenameRequestschema that orchestrates ModifyDN → Modify with rollback on Modify failure - Added
/entry/renameroute wired to the new request handler - Added API tests plus a fixture to create a computer entry for rollback testing
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_api/test_main/test_router/test_rename.py | Adds happy-path rename test and rollback-on-failure test |
| tests/test_api/test_main/conftest.py | Adds fixture to create a test “computer” LDAP entry |
| app/api/main/schema.py | Implements RenameRequest combining ModifyDN + Modify and rollback logic |
| app/api/main/router.py | Exposes the new /entry/rename endpoint |
| interface | Updates submodule pointer (interface dependency) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/api/main/schema.py
Outdated
| return await modify_dn_request.handle_api(container) | ||
|
|
||
| async def _clear_session_cache(self, container: AsyncContainer) -> None: | ||
| session = await container.get(AsyncSession) |
There was a problem hiding this comment.
не пон, зачем?
название еще не совсем удачное мне показалось, сделал так: _expire_session_objects
| assert data["resultCode"] == LDAPCodes.SUCCESS | ||
| assert data["search_result"][0]["object_name"] == "cn=admin2,dc=md,dc=test" | ||
|
|
||
| for attr in data["search_result"][0]["partial_attributes"]: |
There was a problem hiding this comment.
тут тоже можно сжать. либо собрать в 1 цикл, либо сделать функцию проверки и снизу тоже заюзать
There was a problem hiding this comment.
выписал это и отдельным пр сделаю по всем тестам
There was a problem hiding this comment.
из вариантов такое могу предложить, вролде должно работать
sAMAccountName_found = False
displayName_found = False
for attr in data["search_result"][0]["partial_attributes"]:
if attr["type"] == "sAMAccountName":
assert attr["vals"][0] == "admin2"
sAMAccountName_found = True
elif attr["type"] == "displayName":
assert attr["vals"][0] == "Administrator"
displayName_found = True
if sAMAccountName_found and displayName_found:
break
if not sAMAccountName_found:
raise Exception("User without sAMAccountName")
if not displayName_found:
raise Exception("User without displayName")
#----------------------------
attrs_dict = {attr["type"]: attr["vals"][0]
for attr in data["search_result"][0]["partial_attributes"]}
assert attrs_dict.get("sAMAccountName") == "admin2"
assert attrs_dict.get("displayName") == "Administrator"
app/api/main/schema.py
Outdated
| Combines ModifyDN and Modify operations. | ||
| """ | ||
|
|
||
| object: str |
There was a problem hiding this comment.
сделай _object, чтобы не трогать зарезервированное
There was a problem hiding this comment.
вообще, в ModifyDNRequest аналогичное поле называется entry. Мб сделать более менее едино?
There was a problem hiding this comment.
делал по аналогии с modify + делал по схеме как в ТЗ
нам бы везде поменять нейминг у реквестов, сейчас предлагаю везде делать по одному неидеальному шаблону
There was a problem hiding this comment.
это надо в общий рефакторинг реквестов уносить, сейчас мы это красиво не обыграем, только разменом костыль на костыль
milov-dmitriy
requested review from
Misha-Shvets and
rimu-stack
and removed request for
Misha-Shvets
Сделал ручку для rename директории: в ручке совмещена логика Modify + ModifyDN.
Сначала делаем ModifyDN, а затем Modify.
Именно такой порядок потому что если сделать rollback изменений (в случае падения в Modify) для ModifyDN сделать проще, чем наоборот.
"Проще" из-за того, что для ModifyDN мы можем узнать и старое значение и новое значение. Для Modify мы это так просто не узнаем.
Задача: 1110