Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 20 additions & 19 deletions src/extensions/score_metamodel/checks/check_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,20 @@ def get_need_type(needs_types: list[ScoreNeedType], directive: str) -> ScoreNeed
raise ValueError(f"Need type {directive} not found in needs_types")


def _get_normalized(need: NeedItem, key: str, remove_prefix: bool = False) -> list[str]:
def _get_normalized(need: NeedItem, key: str) -> list[str]:
"""Normalize a raw value into a list of strings."""
raw_value = need.get(key, None)
if not raw_value:
return []
if isinstance(raw_value, str):
if remove_prefix:
return [_remove_namespace_prefix_(raw_value)]
return [raw_value]
if isinstance(raw_value, list):
# Verify all elements are strings
raw_list = cast(list[object], raw_value)
for item in raw_list:
if not isinstance(item, str):
raise ValueError
str_list = cast(list[str], raw_value)
if remove_prefix:
return [_remove_namespace_prefix_(v) for v in str_list]
return str_list
return cast(list[str], raw_value)
raise ValueError


Expand All @@ -73,11 +68,6 @@ def _validate_value_pattern(
) from e


def _remove_namespace_prefix_(word: str) -> str:
# If the word starts with uppercase letters followed by an underscore, remove them.
return re.sub(r"^[A-Z]+_", "", word)


def validate_options(
log: CheckLogger,
need_type: ScoreNeedType,
Expand Down Expand Up @@ -144,6 +134,22 @@ def _validate(attributes_to_allowed_values: dict[str, str], mandatory: bool):
# )


def _to_link_pattern(value: "str | ScoreNeedType") -> str:
"""Convert a link constraint to a regex pattern.

- A plain type name like ``"stkh_req"`` becomes ``^stkh_req__``
(matching IDs that start with the type name followed by ``__``).
- A string already starting with ``^`` is treated as an explicit regex
and returned unchanged.
- A ScoreNeedType dict uses its ``mandatory_options.id`` pattern.
"""
if isinstance(value, str):
if value.startswith("^"):
return value
return f"^{value}__"
return value["mandatory_options"]["id"]
Comment on lines +149 to +150

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strings will start with ^ while the others with the id directly? ^ should be outside this function?!



def validate_links(
log: CheckLogger,
need_type: ScoreNeedType,
Expand All @@ -159,16 +165,11 @@ def _validate(
treat_as_info: bool = False,
):
for attribute, allowed_values in attributes_to_allowed_values.items():
values = _get_normalized(need, attribute, remove_prefix=True)
values = _get_normalized(need, attribute)
if mandatory and not values:
log.warning_for_need(need, f"is missing required link: `{attribute}`.")

allowed_regex = "|".join(
[
v if isinstance(v, str) else v["mandatory_options"]["id"]
for v in allowed_values
]
)
allowed_regex = "|".join(_to_link_pattern(v) for v in allowed_values)

# regex based validation
for value in values:
Expand Down
123 changes: 123 additions & 0 deletions src/extensions/score_metamodel/tests/test_check_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
check_extra_options,
check_options,
parse_milestone,
validate_links,
)
from score_metamodel.tests import fake_check_logger, need
from sphinx.application import Sphinx # type: ignore[import-untyped]
Expand Down Expand Up @@ -124,3 +125,125 @@ def test_milestone_parsing():
assert parse_milestone("v0.5") == (0, 5, 0)
assert parse_milestone("v1.0") == (1, 0, 0)
assert parse_milestone("v1.0.1") == (1, 0, 1)


class TestValidateLinks:
"""Tests for validate_links() — specifically the prefix-stripping bug (#588)."""

NEED_TYPE_WITH_LINK_BY_NAME: ScoreNeedType = {
"title": "Test Type",
"prefix": "TT",
"tags": [],
"parts": 1,
"directive": "test_type",
"mandatory_options": {
"id": "^test_type__.*$",
"status": "^(draft|valid)$",
},
"optional_options": {},
"mandatory_links": {},
"optional_links": {
"satisfies": ["product"],
},
}

NEED_TYPE_WITH_LINK_BY_REGEX: ScoreNeedType = {
"title": "Test Type",
"prefix": "TT",
"tags": [],
"parts": 1,
"directive": "test_type",
"mandatory_options": {
"id": "^test_type__.*$",
"status": "^(draft|valid)$",
},
"optional_options": {},
"mandatory_links": {},
"optional_links": {
"satisfies": ["^product__.*$"],
},
}

def test_link_matches_by_type_name_no_prefix(self):
"""A link value that starts with the type name should pass validation."""
need_1 = need(
id="test_type__001",
type="test_type",
satisfies="product__foo",
)
logger = fake_check_logger()
validate_links(
cast(CheckLogger, logger),
self.NEED_TYPE_WITH_LINK_BY_NAME,
need_1,
)
logger.assert_no_warnings()

def test_link_with_uppercase_prefix_no_longer_stripped(self):
"""
After the fix for #588, uppercase prefixes are no longer stripped.
P_FOO stays as P_FOO and correctly fails to match the 'product' type pattern.
"""
need_1 = need(
id="test_type__001",
type="test_type",
satisfies="P_FOO",
)
logger = fake_check_logger()
validate_links(
cast(CheckLogger, logger),
self.NEED_TYPE_WITH_LINK_BY_NAME,
need_1,
)
# P_FOO is preserved as-is (no stripping) and doesn't match ^product__
logger.assert_warning("references 'P_FOO' as 'satisfies'")

def test_link_with_uppercase_prefix_not_stripped_for_regex(self):
"""
After the fix for #588, uppercase prefixes are no longer stripped.
P_product__foo stays as P_product__foo and does NOT match ^product__.*$
because the P_ prefix is preserved.
"""
need_1 = need(
id="test_type__001",
type="test_type",
satisfies="P_product__foo",
)
logger = fake_check_logger()
validate_links(
cast(CheckLogger, logger),
self.NEED_TYPE_WITH_LINK_BY_REGEX,
need_1,
)
# P_product__foo is preserved as-is and doesn't match ^product__.*$
logger.assert_warning("references 'P_product__foo' as 'satisfies'")

def test_link_without_prefix_matches_by_name(self):
"""A link value without any prefix that starts with the type name passes."""
need_1 = need(
id="test_type__001",
type="test_type",
satisfies="product__bar",
)
logger = fake_check_logger()
validate_links(
cast(CheckLogger, logger),
self.NEED_TYPE_WITH_LINK_BY_NAME,
need_1,
)
logger.assert_no_warnings()

def test_link_not_matching_any_type_warns(self):
"""A link value that doesn't match any allowed type should warn."""
need_1 = need(
id="test_type__001",
type="test_type",
satisfies="unknown__thing",
)
logger = fake_check_logger()
validate_links(
cast(CheckLogger, logger),
self.NEED_TYPE_WITH_LINK_BY_NAME,
need_1,
)
logger.assert_warning("references 'unknown__thing' as 'satisfies'")
Loading