Skip to content

Commit f8563b7

Browse files
AurelioloAureliolo
committed
refactor: extract helpers for function length, fix import placement
- Extract _deduplicate_departments and _persist_merged_config from apply_template_pack (was 93 lines, now ~30) - Extract _validate_pack_name from load_pack (was 57 lines, now ~40) - Move Sequence and TemplateDepartmentConfig to TYPE_CHECKING block
1 parent 69dd0ba commit f8563b7

2 files changed

Lines changed: 79 additions & 57 deletions

File tree

‎src/synthorg/api/controllers/template_packs.py‎

Lines changed: 58 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Template packs controller -- listing and live application."""
22

33
import json
4-
from typing import Any, Literal
4+
from typing import TYPE_CHECKING, Any, Literal
55

66
from litestar import Controller, get, post
77
from litestar.datastructures import State # noqa: TC002
@@ -28,6 +28,11 @@
2828
from synthorg.templates.errors import TemplateNotFoundError
2929
from synthorg.templates.pack_loader import PackInfo, list_packs, load_pack
3030

31+
if TYPE_CHECKING:
32+
from collections.abc import Sequence
33+
34+
from synthorg.templates.schema import TemplateDepartmentConfig
35+
3136
logger = get_logger(__name__)
3237

3338

@@ -98,6 +103,48 @@ async def _read_setting_list(
98103
return []
99104

100105

106+
def _deduplicate_departments(
107+
pack_name: str,
108+
pack_depts: Sequence[TemplateDepartmentConfig],
109+
current_depts: list[dict[str, Any]],
110+
) -> list[dict[str, Any]]:
111+
"""Return pack departments that don't conflict with existing ones."""
112+
existing_names = {str(d.get("name", "")).lower() for d in current_depts}
113+
if not pack_depts:
114+
return []
115+
raw: list[dict[str, Any]] = json.loads(
116+
departments_to_json(pack_depts),
117+
)
118+
new_depts = [d for d in raw if str(d.get("name", "")).lower() not in existing_names]
119+
if len(new_depts) < len(raw):
120+
logger.warning(
121+
TEMPLATE_PACK_APPLY_ERROR,
122+
pack_name=pack_name,
123+
action="departments_skipped",
124+
skipped=len(raw) - len(new_depts),
125+
)
126+
return new_depts
127+
128+
129+
async def _persist_merged_config(
130+
app_state: AppState,
131+
agents: list[dict[str, Any]],
132+
departments: list[dict[str, Any]],
133+
) -> None:
134+
"""Write merged agents and departments to settings."""
135+
settings_svc = app_state.settings_service
136+
await settings_svc.set(
137+
"company",
138+
"agents",
139+
json.dumps(agents),
140+
)
141+
await settings_svc.set(
142+
"company",
143+
"departments",
144+
json.dumps(departments),
145+
)
146+
147+
101148
# ---- Controller -----------------------------------------------------------
102149

103150

@@ -134,9 +181,6 @@ async def apply_template_pack(
134181
) -> ApiResponse[ApplyTemplatePackResponse]:
135182
"""Apply a template pack to the running organization.
136183
137-
Loads the pack, expands its agents and departments, and
138-
appends them to the current company configuration.
139-
140184
Args:
141185
data: Pack name and optional variables.
142186
state: Application state.
@@ -150,7 +194,6 @@ async def apply_template_pack(
150194
pack_name=data.pack_name,
151195
)
152196

153-
# Load and validate the pack.
154197
try:
155198
loaded = load_pack(data.pack_name)
156199
except TemplateNotFoundError as exc:
@@ -162,52 +205,20 @@ async def apply_template_pack(
162205
msg = f"Template pack {data.pack_name!r} not found"
163206
raise NotFoundError(msg) from exc
164207

165-
# Expand pack agents into persistable dicts.
166208
pack_agents = expand_template_agents(loaded.template)
167-
168-
# Read current state.
169209
current_agents = await _read_setting_list(app_state, "agents")
170-
current_depts = await _read_setting_list(
171-
app_state,
172-
"departments",
173-
)
210+
current_depts = await _read_setting_list(app_state, "departments")
174211

175-
# Merge departments (skip existing by name).
176-
existing_dept_names = {str(d.get("name", "")).lower() for d in current_depts}
177-
if loaded.template.departments:
178-
new_depts_raw: list[dict[str, Any]] = json.loads(
179-
departments_to_json(loaded.template.departments),
180-
)
181-
else:
182-
new_depts_raw = []
183-
new_depts = [
184-
d
185-
for d in new_depts_raw
186-
if str(d.get("name", "")).lower() not in existing_dept_names
187-
]
188-
if len(new_depts) < len(new_depts_raw):
189-
skipped = len(new_depts_raw) - len(new_depts)
190-
logger.warning(
191-
TEMPLATE_PACK_APPLY_ERROR,
192-
pack_name=data.pack_name,
193-
action="departments_skipped",
194-
skipped=skipped,
195-
)
196-
197-
# Persist.
198-
merged_agents = current_agents + pack_agents
199-
merged_depts = current_depts + new_depts
200-
201-
settings_svc = app_state.settings_service
202-
await settings_svc.set(
203-
"company",
204-
"agents",
205-
json.dumps(merged_agents),
212+
new_depts = _deduplicate_departments(
213+
data.pack_name,
214+
loaded.template.departments,
215+
current_depts,
206216
)
207-
await settings_svc.set(
208-
"company",
209-
"departments",
210-
json.dumps(merged_depts),
217+
218+
await _persist_merged_config(
219+
app_state,
220+
current_agents + pack_agents,
221+
current_depts + new_depts,
211222
)
212223

213224
logger.info(

‎src/synthorg/templates/pack_loader.py‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -111,29 +111,40 @@ def list_packs() -> tuple[PackInfo, ...]:
111111
return tuple(info for _, info in sorted(seen.items()))
112112

113113

114-
def load_pack(name: str) -> LoadedTemplate:
115-
"""Load a template pack by name: user directory first, then builtins.
116-
117-
Args:
118-
name: Pack name (e.g. ``"security-team"``).
114+
def _validate_pack_name(name: str) -> str:
115+
"""Normalize and validate a pack name, rejecting path traversal.
119116
120117
Returns:
121-
:class:`LoadedTemplate` with validated data and raw YAML.
118+
Normalized (lowercase, stripped) name.
122119
123120
Raises:
124-
TemplateNotFoundError: If no pack with *name* exists.
121+
TemplateNotFoundError: If the name contains path separators.
125122
"""
126123
name_clean = name.strip().lower()
127-
logger.debug(TEMPLATE_PACK_LOAD_START, pack_name=name_clean)
128-
129-
# Sanitize to prevent path traversal (OS-independent).
130124
if "/" in name_clean or "\\" in name_clean or ".." in name_clean:
131125
msg = f"Invalid pack name {name!r}: must not contain path separators"
132126
logger.warning(TEMPLATE_PACK_LOAD_NOT_FOUND, pack_name=name)
133127
raise TemplateNotFoundError(
134128
msg,
135129
locations=(ConfigLocation(file_path=f"<pack:{name}>"),),
136130
)
131+
return name_clean
132+
133+
134+
def load_pack(name: str) -> LoadedTemplate:
135+
"""Load a template pack by name: user directory first, then builtins.
136+
137+
Args:
138+
name: Pack name (e.g. ``"security-team"``).
139+
140+
Returns:
141+
:class:`LoadedTemplate` with validated data and raw YAML.
142+
143+
Raises:
144+
TemplateNotFoundError: If no pack with *name* exists.
145+
"""
146+
name_clean = _validate_pack_name(name)
147+
logger.debug(TEMPLATE_PACK_LOAD_START, pack_name=name_clean)
137148

138149
# Try user directory first.
139150
if _USER_PACKS_DIR.is_dir():

0 commit comments

Comments
 (0)