Repository navigation
fix(migrations): scope leaf/root nodes to same-app edges #2307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 2 commits
736def4
688c261
623a769
3502453
a677e7e
398f114
df983cc
1e88310
8116caf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| """Tests for app-scoped `MigrationGraph.leaf_nodes()` / `root_nodes()`.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from tortoise.migrations.graph import MigrationGraph, MigrationKey | ||
| from tortoise.migrations.migration import Migration | ||
|
|
||
|
|
||
| def _build_cross_app_graph() -> MigrationGraph: | ||
| graph = MigrationGraph() | ||
| a1 = MigrationKey("a", "0001_initial") | ||
| a2 = MigrationKey("a", "0002_second") | ||
| b1 = MigrationKey("b", "0001_initial") | ||
| for key in (a1, a2, b1): | ||
| graph.add_node(key, Migration(key.name, key.app_label)) | ||
| graph.add_dependency(a2, a2, a1) # a.0002 depends on a.0001 | ||
| graph.add_dependency(b1, b1, a2) # b.0001 depends on a.0002 (cross-app edge) | ||
| return graph | ||
|
|
||
|
|
||
| def test_leaf_and_root_nodes_ignore_cross_app_edges() -> None: | ||
| graph = _build_cross_app_graph() | ||
| # a.0002 is depended on by b.0001, but that edge is not an `a` migration, so `a` still | ||
| # has a leaf; the same holds for the root of `b`. | ||
| assert graph.leaf_nodes("a") == [MigrationKey("a", "0002_second")] | ||
| assert graph.root_nodes("b") == [MigrationKey("b", "0001_initial")] | ||
|
|
||
|
|
||
| def test_leaf_and_root_nodes_without_app_label_keep_any_app_edges() -> None: | ||
| graph = _build_cross_app_graph() | ||
| # The global call still counts edges from every app. | ||
| assert graph.leaf_nodes() == [MigrationKey("b", "0001_initial")] | ||
| assert graph.root_nodes() == [MigrationKey("a", "0001_initial")] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ | |
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing import Literal | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| from tortoise.migrations.migration import Migration | ||
|
|
||
|
|
||
|
|
@@ -110,21 +112,32 @@ def validate_consistency(self) -> None: | |
| if isinstance(node, DummyNode): | ||
| node.raise_error() | ||
|
|
||
| def root_nodes(self, app_label: str | None = None) -> list[MigrationKey]: | ||
| nodes = [ | ||
| def _filter_boundary_keys( | ||
| self, attr: Literal["parents", "children"], app_label: str | None | ||
| ) -> list[MigrationKey]: | ||
| nodes = self.node_map.values() | ||
| if app_label is None: | ||
| return sorted([node.key for node in nodes if not getattr(node, attr)]) | ||
| keys = [ | ||
| node.key | ||
| for node in self.node_map.values() | ||
| if not node.parents and (app_label is None or node.key.app_label == app_label) | ||
| for node in nodes | ||
| if node.key.app_label == app_label | ||
| and not self._has_related_node(getattr(node, attr), app_label) | ||
| ] | ||
| return sorted(nodes) | ||
| return sorted(keys) | ||
|
|
||
| def root_nodes(self, app_label: str | None = None) -> list[MigrationKey]: | ||
| return self._filter_boundary_keys("parents", app_label) | ||
|
|
||
| def leaf_nodes(self, app_label: str | None = None) -> list[MigrationKey]: | ||
| nodes = [ | ||
| node.key | ||
| for node in self.node_map.values() | ||
| if not node.children and (app_label is None or node.key.app_label == app_label) | ||
| ] | ||
| return sorted(nodes) | ||
| return self._filter_boundary_keys("children", app_label) | ||
|
|
||
| @staticmethod | ||
| def _has_related_node(related: set[Node], app_label: str | None) -> bool: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call — adopted. |
||
| # For a single-app request only same-app edges decide whether that app has a root or | ||
| # a leaf: an app whose newest migration is depended on from another app, or whose | ||
| # first migration depends on another app, still has its own target. | ||
| return any(node.key.app_label == app_label for node in related) | ||
|
|
||
| def forwards_plan(self, target: MigrationKey) -> list[MigrationKey]: | ||
| if target not in self.nodes: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
#2307instead of#2305