Skip to content

Commit c8a55ef

Browse files
committed
refactor: update PathData to use ConfigMetadata instance
- Add metadata property to return ConfigMetadata instance - Update tests to use ConfigMetadata properties directly - Improve test coverage in test_to_dict_from_dict - Maintain backward compatibility
1 parent 59360ba commit c8a55ef

File tree

4 files changed

+93
-47
lines changed

4 files changed

+93
-47
lines changed

helm_values_manager/models/path_data.py

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from typing import Any, Callable, Dict, Iterator, Optional
99

10+
from helm_values_manager.models.config_metadata import ConfigMetadata
1011
from helm_values_manager.models.value import Value
1112
from helm_values_manager.utils.logger import logger
1213

@@ -23,7 +24,11 @@ def __init__(self, path: str, metadata: Dict[str, Any]):
2324
metadata: Dictionary containing metadata fields
2425
"""
2526
self.path = path
26-
self.metadata = metadata
27+
self._metadata = ConfigMetadata(
28+
description=metadata.get("description"),
29+
required=metadata.get("required", False),
30+
sensitive=metadata.get("sensitive", False),
31+
)
2732
self._values: Dict[str, Value] = {}
2833

2934
def validate(self) -> None:
@@ -47,7 +52,7 @@ def validate(self) -> None:
4752
raise ValueError(f"Value for environment {env} has inconsistent path: {value.path} != {self.path}")
4853

4954
# Check required values
50-
if self.metadata.get("required", False):
55+
if self._metadata.required:
5156
val = value.get()
5257
if val is None or val == "":
5358
logger.error("Missing required value for path %s in environment %s", self.path, env)
@@ -106,12 +111,22 @@ def to_dict(self) -> Dict[str, Any]:
106111
"""
107112
return {
108113
"path": self.path,
109-
"description": self.metadata.get("description"),
110-
"required": self.metadata.get("required", False),
111-
"sensitive": self.metadata.get("sensitive", False),
114+
"description": self._metadata.description,
115+
"required": self._metadata.required,
116+
"sensitive": self._metadata.sensitive,
112117
"values": {env: value.get() for env, value in self._values.items()},
113118
}
114119

120+
@property
121+
def metadata(self) -> ConfigMetadata:
122+
"""
123+
Get metadata for this path.
124+
125+
Returns:
126+
ConfigMetadata: Metadata instance for this path
127+
"""
128+
return self._metadata
129+
115130
@classmethod
116131
def from_dict(
117132
cls, data: Dict[str, Any], create_value_fn: Callable[[str, str, Dict[str, Any]], Value]

tests/unit/models/test_helm_values_config.py

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ def test_add_config_path():
2626
path_data = config._path_map[path]
2727
assert isinstance(path_data, PathData)
2828
assert path_data.path == path
29-
assert path_data.metadata["description"] == description
30-
assert path_data.metadata["required"] is True
31-
assert path_data.metadata["sensitive"] is False
29+
assert path_data.metadata.description == description
30+
assert path_data.metadata.required is True
31+
assert path_data.metadata.sensitive is False
3232

3333

3434
def test_add_duplicate_path():
@@ -88,33 +88,66 @@ def test_set_value_without_path():
8888

8989
def test_to_dict_from_dict():
9090
"""Test serialization and deserialization."""
91-
config = HelmValuesConfig()
92-
path = "app.config.key1"
93-
description = "Test config"
94-
value = "test-value"
95-
environment = "dev"
96-
97-
config.add_config_path(path, description=description, required=True, sensitive=False)
98-
config.set_value(path, environment, value)
99-
91+
config_data = {
92+
"version": "1.0",
93+
"release": "test-release",
94+
"deployments": {
95+
"prod": {
96+
"backend": "aws",
97+
"auth": {"type": "env", "env_prefix": "AWS_"},
98+
"backend_config": {"region": "us-west-2"},
99+
}
100+
},
101+
"config": [
102+
{
103+
"path": "app.config.key1",
104+
"description": "Test config",
105+
"required": True,
106+
"sensitive": True,
107+
"values": {"default": "test-value"},
108+
}
109+
],
110+
}
111+
112+
# Test from_dict
113+
config = HelmValuesConfig.from_dict(config_data)
114+
assert config.release == "test-release"
115+
assert config.version == "1.0"
116+
assert len(config.deployments) == 1
117+
assert len(config._path_map) == 1
118+
119+
# Verify deployment data
120+
deployment = config.deployments["prod"]
121+
assert deployment.backend == "aws"
122+
assert deployment.auth == {"type": "env", "env_prefix": "AWS_"}
123+
assert deployment.backend_config == {"region": "us-west-2"}
124+
125+
# Verify config data
126+
path_data = config._path_map["app.config.key1"]
127+
assert path_data.path == "app.config.key1"
128+
assert path_data.metadata.description == "Test config"
129+
assert path_data.metadata.required is True
130+
assert path_data.metadata.sensitive is True
131+
132+
# Test to_dict
100133
config_dict = config.to_dict()
101134
assert config_dict["version"] == "1.0"
102-
assert config_dict["deployments"] == {}
135+
assert config_dict["release"] == "test-release"
136+
assert config_dict["deployments"] == {
137+
"prod": {
138+
"backend": "aws",
139+
"auth": {"type": "env", "env_prefix": "AWS_"},
140+
"backend_config": {"region": "us-west-2"},
141+
}
142+
}
103143
assert len(config_dict["config"]) == 1
104144

105145
config_item = config_dict["config"][0]
106-
assert config_item["path"] == path
107-
assert config_item["description"] == description
146+
assert config_item["path"] == "app.config.key1"
147+
assert config_item["description"] == "Test config"
108148
assert config_item["required"] is True
109-
assert config_item["sensitive"] is False
110-
assert config_item["values"] == {environment: value}
111-
112-
# Test deserialization
113-
new_config = HelmValuesConfig.from_dict(config_dict)
114-
assert new_config.get_value(path, environment) == value
115-
assert new_config._path_map[path].metadata["description"] == description
116-
assert new_config._path_map[path].metadata["required"] is True
117-
assert new_config._path_map[path].metadata["sensitive"] is False
149+
assert config_item["sensitive"] is True
150+
assert config_item["values"] == {"default": "test-value"}
118151

119152

120153
def test_validate():

tests/unit/models/test_path_data.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ def mock_value():
3232
def test_path_data_init(path_data):
3333
"""Test PathData initialization."""
3434
assert path_data.path == "test.path"
35-
assert path_data.metadata["description"] == "Test description"
36-
assert path_data.metadata["required"] is True
37-
assert path_data.metadata["sensitive"] is False
35+
assert path_data.metadata.description == "Test description"
36+
assert path_data.metadata.required is True
37+
assert path_data.metadata.sensitive is False
3838

3939

4040
def test_set_value(path_data, mock_value):
@@ -103,7 +103,7 @@ def test_validate_success(path_data, mock_value):
103103

104104
def test_validate_not_required(path_data, mock_value):
105105
"""Test validation when path is not required."""
106-
path_data.metadata["required"] = False
106+
path_data.metadata.required = False
107107
mock_value.get.return_value = None
108108
path_data.set_value("test_env", mock_value)
109109
path_data.validate() # Should not raise any error
@@ -140,11 +140,9 @@ def create_value_fn(path, env, value_data):
140140

141141
path_data = PathData.from_dict(data, create_value_fn)
142142
assert path_data.path == "test.path"
143-
assert path_data.metadata == {
144-
"description": "Test description",
145-
"required": True,
146-
"sensitive": False,
147-
}
143+
assert path_data.metadata.description == "Test description"
144+
assert path_data.metadata.required is True
145+
assert path_data.metadata.sensitive is False
148146
assert len(path_data._values) == 1
149147
assert "test_env" in path_data._values
150148

tests/unit/models/test_schema_validation.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def test_valid_minimal_config():
1919

2020

2121
def test_valid_full_config():
22-
"""Test loading a complete configuration with all optional fields."""
22+
"""Test loading a valid configuration with all fields."""
2323
config_data = {
2424
"version": "1.0",
2525
"release": "test-release",
@@ -56,9 +56,9 @@ def test_valid_full_config():
5656
# Verify config data
5757
path_data = config._path_map["app.config.key1"]
5858
assert path_data.path == "app.config.key1"
59-
assert path_data.metadata["description"] == "Test config"
60-
assert path_data.metadata["required"] is True
61-
assert path_data.metadata["sensitive"] is True
59+
assert path_data.metadata.description == "Test config"
60+
assert path_data.metadata.required is True
61+
assert path_data.metadata.sensitive is True
6262
assert config.get_value("app.config.key1", "default") == "test-value"
6363

6464

@@ -72,9 +72,9 @@ def test_default_values():
7272
}
7373
config = HelmValuesConfig.from_dict(config_data)
7474
path_data = config._path_map["app.config.key1"]
75-
assert path_data.metadata["description"] is None
76-
assert path_data.metadata["required"] is False
77-
assert path_data.metadata["sensitive"] is False
75+
assert path_data.metadata.description is None
76+
assert path_data.metadata.required is False
77+
assert path_data.metadata.sensitive is False
7878

7979

8080
def test_type_coercion():
@@ -94,8 +94,8 @@ def test_type_coercion():
9494
}
9595
config = HelmValuesConfig.from_dict(config_data)
9696
path_data = config._path_map["app.config.key1"]
97-
assert path_data.metadata["required"] is True
98-
assert path_data.metadata["sensitive"] is False
97+
assert path_data.metadata.required is True
98+
assert path_data.metadata.sensitive is False
9999

100100

101101
def test_missing_required_fields():

0 commit comments

Comments
 (0)