ansys / ansys/pydynamicreporting
Add atomic reparenting for serverless templates
- Dominant language
- Python
- Stars
- 12
- Forks
- 5
- Avg merge
- 3d 14h
- Merged PRs (30d)
- 9
Description
### 📝 Description of the feature
Add a public atomic API for reparenting an existing serverless template while preserving tree integrity and child ordering.
Serverless templates expose mutable `parent` and `children` attributes, and `Template.save()` derives `master` plus `children_order`. `ADR.create_template()` also keeps both sides synchronized when a template is first created:
```python
template = template_type.create(**kwargs)
parent = kwargs.get("parent")
if parent is not None:
parent.children.append(template)
parent.save()
```
After creation, however, there is no equivalent public move operation. Assigning a new `template.parent` and calling `save()` updates only part of the tree contract unless callers also correctly:
- remove the template from the old parent's ordered children;
- save the old parent's `children_order`;
- insert it into the new parent's ordered children;
- save the new parent's `children_order`;
- derive the target's root/master state;
- prevent self-parenting and descendant cycles;
- keep all database changes atomic;
- handle concurrent edits to either parent.
`Template.reorder_child()` only changes position within one parent's in-memory children list. It does not move a node between parents or promote/demote a root.
This orchestration belongs in PyDynamicReporting because it owns the persisted parent foreign key, `master`, and `children_order` invariants. Every downstream adapter should not implement its own tree mutation transaction.
### 💡 Steps for implementing the feature
#### 1. Add one public move operation
Place the orchestration on `ADR`, alongside `create_template()`:
```python
def reparent_template(
self,
template: Template,
new_parent: Template | None,
*,
position: int | None = None,
) -> Template:
"""Move a saved template and its subtree to a new parent atomically."""
```
Contract:
- `new_parent=None` promotes the template to a root report;
- a non-`None` parent demotes or moves it below that saved template;
- `position=None` appends to the new parent's children;
- an integer position inserts at that zero-based index;
- moving within the same parent can reuse reorder semantics;
- the template GUID and its complete descendant subtree are preserved;
- the returned object is refreshed from persisted state.
Use `Template` objects consistently with the existing serverless API. GUID resolution can remain a responsibility of higher-level adapters.
#### 2. Validate the complete operation before mutation
Reject:
- unsaved target or parent objects;
- target and parent from different database aliases;
- `template is new_parent`;
- a parent contained anywhere in the target's descendant subtree;
- invalid positions (`position < 0` or `position > len(new_parent.children)` after removing the target for same-parent moves);
- a non-`None` position when promoting to root, unless root ordering is explicitly supported;
- type-specific parent/child combinations that existing template validation disallows.
Use a product-specific exception such as `TemplateReparentError` with target and parent GUIDs in the detail.
Cycle detection can walk parent links from the proposed parent to the root, avoiding a full subtree load:
```python
ancestor = new_parent
while ancestor is not None:
if ancestor.guid == template.guid:
raise TemplateReparentError("A template cannot be moved below its descendant")
ancestor = ancestor.parent
```
Also guard against pre-existing corrupt cycles by tracking visited GUIDs.
#### 3. Apply all persistence changes in one transaction
Lock the target, old parent, and new parent rows before calculating order. The implementation outline is:
```python
with transaction.atomic(using=database_alias):
target = get_locked_template(template.guid)
old_parent = get_locked_parent(target.parent)
destination = get_locked_parent(new_parent)
validate_reparent(target, destination, position)
if old_parent is not None:
old_parent.children = [
child for child in old_parent.children if child.guid != target.guid
]
old_parent.save(using=database_alias)
target.parent = destination
target.save(using=database_alias)
if destination is not None:
destination.children = [
child for child in destination.children if child.guid != target.guid
]
insertion_index = len(destination.children) if position is None else position
destination.children.insert(insertion_index, target)
destination.save(using=database_alias)
```
Use fresh persisted child collections while holding row locks; do not trust potentially stale in-memory `children` lists passed by the caller.
If any save or validation step fails, roll back the target parent, both parent order strings, and root/master state together.
#### 4. Define same-parent and no-op behavior
- Same parent plus a new position should reorder atomically and return the refreshed target.
- Same parent plus `position=None` should be an idempotent no-op.
- Moving a root to `None` should be an idempotent no-op.
- Duplicate child entries must never be introduced.
- Promoting a nested template to root must remove its GUID from the old parent's order.
Document whether the moved node becomes a report immediately when promoted and confirm that `ADR.get_report(guid=...)` can retrieve it.
#### 5. Add focused tests
Cover:
1. moving a leaf between two parents;
2. moving a subtree and preserving every descendant relation;
3. promoting a child to a root and demoting a root below another report;
4. insertion at the beginning, middle, end, and append default;
5. same-parent reorder and no-op behavior;
6. self-parenting and direct/indirect cycle rejection;
7. unsaved objects and cross-database rejection;
8. injected failure after the old-parent update, proving full rollback;
9. concurrent moves or stale callers, proving row-lock behavior;
10. re-fetching all affected templates and verifying `parent`, `master`, `children`, and `children_order` agree.
#### Acceptance criteria
- Reparenting is available through one documented public PyDynamicReporting call.
- Callers never need to edit both parents and the child manually.
- Cycles, duplicate child entries, stale order strings, and partial updates are prevented.
- Promotion and demotion correctly update root/master behavior.
- The operation preserves the moved subtree and GUIDs.
- Any failure rolls back every affected database change.
- Tests verify persisted state after re-fetch, not only in-memory lists.
### 🔗 Useful links and references
- [Template parent, children, and ordering fields](https://github.com/ansys/pydynamicreporting/blob/d98b45adad663d015d4e17ad93dcd3749f2f3d81/src/ansys/dynamicreporting/core/serverless/template.py#L93-L143)
- [`Template.save()` hierarchy persistence](https://github.com/ansys/pydynamicreporting/blob/d98b45adad663d015d4e17ad93dcd3749f2f3d81/src/ansys/dynamicreporting/core/serverless/template.py#L221-L259)
- [Current child-only reorder helper](https://github.com/ansys/pydynamicreporting/blob/d98b45adad663d015d4e17ad93dcd3749f2f3d81/src/ansys/dynamicreporting/core/serverless/template.py#L537-L567)
- [Creation-time parent synchronization](https://github.com/ansys/pydynamicreporting/blob/d98b45adad663d015d4e17ad93dcd3749f2f3d81/src/ansys/dynamicreporting/core/serverless/adr.py#L1002-L1041)
- Related but non-duplicate issue: #351 broadly mentions missing serverless template attributes/methods but does not identify reparenting, transactions, cycle prevention, or ordering semantics.
An open-and-closed issue audit found no existing issue defining an atomic template reparenting operation.
Contributor guide
Assessment
This issue has not been assessed yet.