Skip to content

Commit 2b9d23e

Browse files
authored
♻️ Migrate use of extra_links to Schema-Based Access (#1638)
This PR migrates all post-config-resolution uses of `needs_config.extra_links` to use `needs_schema` methods, centralizing link type configuration in the schema system, similar to how fields are used. ## Key Changes ### Schema Enhancements (`needs_schema.py`) - **Added `LinkDisplayConfig`** dataclass for link rendering configuration: - `incoming`, `outgoing` (required): Display titles for link directions - `color`, `style`, `style_part`, `style_start`, `style_end`: Diagram styling with sensible defaults - **Extended `LinkSchema`** with new attributes: - `display: LinkDisplayConfig` - Rendering configuration (required) - `copy: bool` - Whether to copy links to common `links` field - `allow_dead_links: bool` - Whether to allow dead links without warning ### Configuration Changes (`config.py`, `needs.py`) - Renamed `extra_links` → `_extra_links` (internal use only, for config resolution phase) - Schema creation in `needs.py` now populates `LinkDisplayConfig` from link config, using dataclass defaults when values aren't explicitly set ### Updated Modules Migrated from dict-based `needs_config.extra_links` access to schema methods: | Module | Change | |--------|--------| | `layout.py` | Use `schema.iter_link_fields()` and `link.display.*` | | `api/need.py` | Use `schema.iter_link_fields()` and `link.copy` | | `directives/need.py` | Use schema for `allow_dead_links` lookup | | `directives/needtable.py` | Use `LinkSchema` objects instead of dicts | | `directives/needflow/_plantuml.py` | Use schema for link types and display config | | `directives/needflow/_graphviz.py` | Use schema for link types and display config | | `directives/needgantt.py` | Use schema for link type validation | | `directives/needsequence.py` | Use schema for link type names | | `directives/needreport.py` | Convert schema to dict for template compatibility | | `directives/list2need.py` | Use schema for link type list | | `roles/need_outgoing.py` | Use schema for `allow_dead_links` check | | `utils.py` | Use schema for link field iteration | ## Migration Pattern **Before:** ```python for link_type in needs_config.extra_links: name = link_type["option"] outgoing = link_type["outgoing"] ``` **After:** ```python for link in needs_schema.iter_link_fields(): name = link.name outgoing = link.display.outgoing ``` ## Benefits - **Single source of truth**: Link configuration is centralized in the schema after config resolution - **Type safety**: `LinkSchema` and `LinkDisplayConfig` provide typed access to link properties - **Cleaner separation**: `_extra_links` is internal for config merging; schema is the public API - **Consistent defaults**: `LinkDisplayConfig` dataclass defaults are used consistently
1 parent 6371b3a commit 2b9d23e

19 files changed

Lines changed: 177 additions & 131 deletions

docs/api.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ Schema
5959

6060
.. automodule:: sphinx_needs.needs_schema
6161
:members: FieldsSchema, FieldSchema, FieldFunctionArray, LinksFunctionArray,
62-
FieldLiteralValue, LinkSchema, LinksLiteralValue, AllowedTypes
62+
FieldLiteralValue, LinkSchema, LinkDisplayConfig, LinksLiteralValue, AllowedTypes
6363

6464
.. automodule:: sphinx_needs.schema.config
6565
:members: ExtraOptionStringSchemaType, ExtraOptionBooleanSchemaType,

sphinx_needs/api/need.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ def generate_need(
394394
else v
395395
for k, v in links_no_defaults.items()
396396
}
397-
_copy_links(links, needs_config)
397+
_copy_links(links, needs_schema)
398398

399399
title, title_func = _convert_to_str_func("title", title_converted)
400400
status, status_func = _convert_to_none_str_func(
@@ -1156,15 +1156,15 @@ def _make_hashed_id(
11561156

11571157
def _copy_links(
11581158
links: dict[str, LinksLiteralValue | LinksFunctionArray | None],
1159-
config: NeedsSphinxConfig,
1159+
schema: FieldsSchema,
11601160
) -> None:
11611161
"""Implement 'copy' logic for links."""
11621162
if "links" not in links:
11631163
return # should not happen, but be defensive
11641164
copy_links: list[str | DynamicFunctionParsed | VariantFunctionParsed] = []
1165-
for link_type in config.extra_links:
1166-
if link_type.get("copy", False) and (name := link_type["option"]) != "links":
1167-
other = links[name]
1165+
for link_field in schema.iter_link_fields():
1166+
if link_field.copy and link_field.name != "links":
1167+
other = links[link_field.name]
11681168
if isinstance(other, LinksLiteralValue | LinksFunctionArray):
11691169
copy_links.extend(other.value)
11701170
if any(

sphinx_needs/config.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -688,10 +688,10 @@ def functions(self) -> Mapping[str, NeedFunctionsType]:
688688
default="→\xa0", metadata={"rebuild": "html", "types": (str,)}
689689
)
690690
"""Prefix for need_part output in tables"""
691-
extra_links: list[LinkOptionsType] = field(
691+
_extra_links: list[LinkOptionsType] = field(
692692
default_factory=list, metadata={"rebuild": "html", "types": ()}
693693
)
694-
"""List of additional link types between needs"""
694+
"""List of additional link types between needs (internal config, use schema for access after config resolution)"""
695695
report_dead_links: bool = field(
696696
default=True, metadata={"rebuild": "html", "types": (bool,)}
697697
)

sphinx_needs/directives/list2need.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from sphinx.util.docutils import SphinxDirective
1414

1515
from sphinx_needs.config import NeedsSphinxConfig
16+
from sphinx_needs.data import SphinxNeedsData
1617

1718
NEED_TEMPLATE = """.. {{type}}:: {{title}}
1819
{% if need_id is not none %}:id: {{need_id}}{%endif%}
@@ -100,7 +101,8 @@ def run(self) -> Sequence[nodes.Node]:
100101
down_links_raw_list = []
101102
else:
102103
down_links_raw_list = [x.strip() for x in down_links_raw.split(",")]
103-
link_types = [x["option"] for x in needs_config.extra_links]
104+
needs_schema = SphinxNeedsData(self.env).get_schema()
105+
link_types = [link.name for link in needs_schema.iter_link_fields()]
104106
for i, down_link_raw in enumerate(down_links_raw_list):
105107
down_links_types[i] = down_link_raw
106108
if down_link_raw not in link_types:

sphinx_needs/directives/need.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from sphinx_needs.logging import WarningSubTypes, get_logger, log_warning
2525
from sphinx_needs.need_constraints import process_constraints
2626
from sphinx_needs.need_item import NeedItem, NeedItemSourceDirective
27+
from sphinx_needs.needs_schema import FieldsSchema
2728
from sphinx_needs.nodes import Need
2829
from sphinx_needs.utils import (
2930
DummyOptionSpec,
@@ -356,11 +357,12 @@ def post_process_needs_data(app: Sphinx) -> None:
356357
needs_data = SphinxNeedsData(app.env)
357358
if not needs_data.needs_is_post_processed:
358359
needs_config = NeedsSphinxConfig(app.config)
360+
needs_schema = needs_data.get_schema()
359361
needs = needs_data.get_needs_mutable()
360362
app.emit("needs-before-post-processing", needs)
361363
extend_needs_data(needs, needs_data.get_or_create_extends(), needs_config)
362364
resolve_functions(app, needs, needs_config)
363-
update_back_links(needs, needs_config)
365+
update_back_links(needs, needs_config, needs_schema)
364366
process_constraints(needs, needs_config)
365367
app.emit("needs-before-sealing", needs)
366368
# run a last check to ensure all needs are of the correct type
@@ -430,7 +432,9 @@ def format_need_nodes(
430432
node_need.parent.replace(node_need, rendered_node)
431433

432434

433-
def update_back_links(needs: NeedsMutable, config: NeedsSphinxConfig) -> None:
435+
def update_back_links(
436+
needs: NeedsMutable, config: NeedsSphinxConfig, schema: FieldsSchema
437+
) -> None:
434438
"""Update needs with back-links, i.e. for each need A that links to need B,"""
435439
for need in needs.values():
436440
need.reset_backlinks()
@@ -455,7 +459,7 @@ def update_back_links(needs: NeedsMutable, config: NeedsSphinxConfig) -> None:
455459

456460
need["has_dead_links"] = bool(dead_links)
457461
allow_dead_links = {
458-
li["option"]: li.get("allow_dead_links", False) for li in config.extra_links
462+
link.name: link.allow_dead_links for link in schema.iter_link_fields()
459463
}
460464
need["has_forbidden_dead_links"] = bool(
461465
any(not allow_dead_links.get(lt, False) for lt, _ in dead_links)

sphinx_needs/directives/needflow/_directive.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from sphinx_needs.data import (
1414
GraphvizStyleType,
1515
NeedsFlowType,
16+
SphinxNeedsData,
1617
)
1718
from sphinx_needs.debug import measure_time
1819
from sphinx_needs.filter_common import FilterBase
@@ -73,7 +74,8 @@ def run(self) -> Sequence[nodes.Node]:
7374
id = self.env.new_serialno("needflow")
7475
targetid = f"needflow-{self.env.docname}-{id}"
7576

76-
all_link_types = ",".join(x["option"] for x in needs_config.extra_links)
77+
needs_schema = SphinxNeedsData(self.env).get_schema()
78+
all_link_types = ",".join(link.name for link in needs_schema.iter_link_fields())
7779
link_types = split_link_types(
7880
self.options.get("link_types", all_link_types), location
7981
)

sphinx_needs/directives/needflow/_graphviz.py

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
)
1717
from sphinx.util.logging import getLogger
1818

19-
from sphinx_needs.config import LinkOptionsType, NeedsSphinxConfig
19+
from sphinx_needs.config import NeedsSphinxConfig
2020
from sphinx_needs.data import SphinxNeedsData
2121
from sphinx_needs.debug import measure_time
2222
from sphinx_needs.directives.needflow._directive import NeedflowGraphiz
@@ -28,6 +28,7 @@
2828
)
2929
from sphinx_needs.logging import log_warning
3030
from sphinx_needs.need_item import NeedItem, NeedPartItem
31+
from sphinx_needs.needs_schema import LinkSchema
3132
from sphinx_needs.utils import remove_node_from_tree
3233
from sphinx_needs.variants import match_variants
3334
from sphinx_needs.views import NeedsView
@@ -50,10 +51,11 @@ def process_needflow_graphviz(
5051
found_nodes: list[nodes.Element],
5152
) -> None:
5253
needs_config = NeedsSphinxConfig(app.config)
54+
needs_schema = SphinxNeedsData(app.env).get_schema()
5355
env_data = SphinxNeedsData(app.env)
5456
needs_view = env_data.get_needs_view()
5557

56-
link_type_names = [link["option"].upper() for link in needs_config.extra_links]
58+
link_type_names = [name.upper() for name in needs_schema.iter_link_field_names()]
5759
allowed_link_types_options = [link.upper() for link in needs_config.flow_link_types]
5860

5961
node: NeedflowGraphiz
@@ -93,29 +95,28 @@ def process_needflow_graphviz(
9395
None,
9496
)
9597

96-
# compute the allowed link names
97-
allowed_link_types: list[LinkOptionsType] = []
98-
for link_type in needs_config.extra_links:
98+
# compute the allowed link types
99+
allowed_link_types: list[LinkSchema] = []
100+
for link in needs_schema.iter_link_fields():
99101
# Skip link-type handling, if it is not part of a specified list of allowed link_types or
100102
# if not part of the overall configuration of needs_flow_link_types
101103
if (
102-
attributes["link_types"]
103-
and link_type["option"].upper() not in option_link_types
104+
attributes["link_types"] and link.name.upper() not in option_link_types
104105
) or (
105106
not attributes["link_types"]
106-
and link_type["option"].upper() not in allowed_link_types_options
107+
and link.name.upper() not in allowed_link_types_options
107108
):
108109
continue
109110
# skip creating links from child needs to their own parent need
110-
if link_type["option"] == "parent_needs":
111+
if link.name == "parent_needs":
111112
continue
112-
allowed_link_types.append(link_type)
113+
allowed_link_types.append(link)
113114

114115
init_filtered_needs = (
115116
filter_by_tree(
116117
needs_view,
117118
root_id,
118-
allowed_link_types,
119+
[lt.name for lt in allowed_link_types],
119120
attributes["root_direction"],
120121
attributes["root_depth"],
121122
)
@@ -169,7 +170,7 @@ def process_needflow_graphviz(
169170
content += "\n// edge definitions\n"
170171
for need in filtered_needs:
171172
for link_type in allowed_link_types:
172-
for link in need[link_type["option"]]:
173+
for link in need[link_type.name]:
173174
content += _render_edge(
174175
need, link, link_type, node, needs_config, rendered_nodes
175176
)
@@ -440,7 +441,7 @@ def _label(
440441
def _render_edge(
441442
need: NeedItem | NeedPartItem,
442443
link: str,
443-
link_type: LinkOptionsType,
444+
link_type: LinkSchema,
444445
node: NeedflowGraphiz,
445446
config: NeedsSphinxConfig,
446447
rendered_nodes: dict[str, _RenderedNode],
@@ -455,16 +456,15 @@ def _render_edge(
455456
params: list[tuple[str, str]] = []
456457

457458
if show_links:
458-
params.append(("label", _quote(link_type["outgoing"])))
459+
params.append(("label", _quote(link_type.display.outgoing)))
459460

460461
is_part = "." in link or "." in need["id_complete"]
461462
params.extend(
463+
# TODO also use link_type.display.color?
462464
_style_params_from_link_type(
463-
link_type.get("style_part", "dotted")
464-
if is_part
465-
else link_type.get("style", ""),
466-
link_type.get("style_start", "-"),
467-
link_type.get("style_end", "->"),
465+
link_type.display.style_part if is_part else link_type.display.style,
466+
link_type.display.style_start,
467+
link_type.display.style_end,
468468
)
469469
)
470470

sphinx_needs/directives/needflow/_plantuml.py

Lines changed: 24 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from jinja2 import Template
99
from sphinx.application import Sphinx
1010

11-
from sphinx_needs.config import LinkOptionsType, NeedsSphinxConfig
11+
from sphinx_needs.config import NeedsSphinxConfig
1212
from sphinx_needs.data import NeedsFlowType, SphinxNeedsData
1313
from sphinx_needs.debug import measure_time
1414
from sphinx_needs.diagrams_common import calculate_link, create_legend
@@ -17,6 +17,7 @@
1717
from sphinx_needs.filter_common import filter_single_need, process_filters
1818
from sphinx_needs.logging import get_logger, log_warning
1919
from sphinx_needs.need_item import NeedItem, NeedPartItem
20+
from sphinx_needs.needs_schema import LinkSchema
2021
from sphinx_needs.utils import remove_node_from_tree
2122
from sphinx_needs.variants import match_variants
2223
from sphinx_needs.views import NeedsView
@@ -193,8 +194,9 @@ def process_needflow_plantuml(
193194
needs_config = NeedsSphinxConfig(app.config)
194195
env_data = SphinxNeedsData(env)
195196
needs_view = env_data.get_needs_view()
197+
needs_schema = env_data.get_schema()
196198

197-
link_type_names = [link["option"].upper() for link in needs_config.extra_links]
199+
link_type_names = [link.name.upper() for link in needs_schema.iter_link_fields()]
198200
allowed_link_types_options = [link.upper() for link in needs_config.flow_link_types]
199201

200202
node: NeedflowPlantuml
@@ -219,23 +221,23 @@ def process_needflow_plantuml(
219221
location=node,
220222
)
221223

222-
# compute the allowed link names
223-
allowed_link_types: list[LinkOptionsType] = []
224-
for link_type in needs_config.extra_links:
224+
# compute the allowed link types
225+
allowed_link_types: list[LinkSchema] = []
226+
for link_field in needs_schema.iter_link_fields():
225227
# Skip link-type handling, if it is not part of a specified list of allowed link_types or
226228
# if not part of the overall configuration of needs_flow_link_types
227229
if (
228230
current_needflow["link_types"]
229-
and link_type["option"].upper() not in option_link_types
231+
and link_field.name.upper() not in option_link_types
230232
) or (
231233
not current_needflow["link_types"]
232-
and link_type["option"].upper() not in allowed_link_types_options
234+
and link_field.name.upper() not in allowed_link_types_options
233235
):
234236
continue
235237
# skip creating links from child needs to their own parent need
236-
if link_type["option"] == "parent_needs":
238+
if link_field.name == "parent_needs":
237239
continue
238-
allowed_link_types.append(link_type)
240+
allowed_link_types.append(link_field)
239241

240242
try:
241243
if "sphinxcontrib.plantuml" not in app.extensions:
@@ -256,7 +258,7 @@ def process_needflow_plantuml(
256258
filter_by_tree(
257259
needs_view,
258260
root_id,
259-
allowed_link_types,
261+
[lt.name for lt in allowed_link_types],
260262
current_needflow["root_direction"],
261263
current_needflow["root_depth"],
262264
)
@@ -386,7 +388,7 @@ def process_needflow_plantuml(
386388

387389
def render_connections(
388390
found_needs: list[NeedItem | NeedPartItem],
389-
allowed_link_types: list[LinkOptionsType],
391+
allowed_link_types: list[LinkSchema],
390392
show_links: bool,
391393
) -> str:
392394
"""
@@ -395,7 +397,7 @@ def render_connections(
395397
puml_connections = ""
396398
for need_info in found_needs:
397399
for link_type in allowed_link_types:
398-
for link in need_info[link_type["option"]]:
400+
for link in need_info[link_type.name]:
399401
# Do not create an links, if the link target is not part of the search result.
400402
if link not in [
401403
x["id"] for x in found_needs if x["is_need"]
@@ -405,40 +407,29 @@ def render_connections(
405407
continue
406408

407409
if show_links:
408-
desc = link_type["outgoing"] + "\\n"
410+
desc = link_type.display.outgoing + "\\n"
409411
comment = f": {desc}"
410412
else:
411413
comment = ""
412414

413415
# If source or target of link is a need_part, a specific style is needed
414416
if "." in link or "." in need_info["id_complete"]:
415-
if _style_part := link_type.get("style_part"):
416-
link_style = f"[{_style_part}]"
417-
else:
418-
link_style = "[dotted]"
419-
else:
420-
if _style := link_type.get("style"):
421-
link_style = f"[{_style}]"
422-
else:
423-
link_style = ""
424-
425-
if _style_start := link_type.get("style_start"):
426-
style_start = _style_start
427-
else:
428-
style_start = "-"
429-
430-
if _style_end := link_type.get("style_end"):
431-
style_end = _style_end
417+
link_style = f"[{link_type.display.style_part}]"
432418
else:
433-
style_end = "->"
419+
link_style = (
420+
f"[{link_type.display.style}]"
421+
if link_type.display.style
422+
else ""
423+
)
434424

425+
# TODO also use link_type.display.color?
435426
puml_connections += "{id} {style_start}{link_style}{style_end} {link}{comment}\n".format(
436427
id=make_entity_name(need_info["id_complete"]),
437428
link=make_entity_name(link),
438429
comment=comment,
439430
link_style=link_style,
440-
style_start=style_start,
441-
style_end=style_end,
431+
style_start=link_type.display.style_start,
432+
style_end=link_type.display.style_end,
442433
)
443434
return puml_connections
444435

0 commit comments

Comments
 (0)