Skip to content

Commit e3ff279

Browse files
committed
refactor(client): extract _validate_property_dict helper, add Cypher identifier validation
- Extract per-property validation into _validate_property_dict static method, reducing _build_property_definitions cognitive complexity from 16 → ~10 (fixes SonarCloud threshold of 15) - Add _CYPHER_IDENT_RE regex and _validate_cypher_identifier() module helper; validate name, label, and each property in create_text_index(), and name in drop_text_index() before interpolating into DDL Cypher strings - Add missing port 37084 (metrics/health) to demo/README.md port table to match docker-compose.yml
1 parent c049966 commit e3ff279

2 files changed

Lines changed: 43 additions & 16 deletions

File tree

coordinode/coordinode/client.py

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,21 @@
2828
# be reliably distinguished from a "host:port" pair.
2929
_HOST_PORT_RE = re.compile(r"^(\[.+\]|[^:]+):(\d+)$")
3030

31+
# Cypher identifier: must start with a letter or underscore, followed by
32+
# letters, digits, or underscores. Validated before interpolating user-supplied
33+
# names/labels/properties into DDL strings to surface clear errors early.
34+
_CYPHER_IDENT_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
35+
36+
37+
def _validate_cypher_identifier(value: str, param_name: str) -> None:
38+
"""Raise :exc:`ValueError` if *value* is not a valid Cypher identifier."""
39+
if not isinstance(value, str) or not _CYPHER_IDENT_RE.match(value):
40+
raise ValueError(
41+
f"{param_name} must be a valid Cypher identifier (letters, digits, underscores, "
42+
f"starting with a letter or underscore); got {value!r}"
43+
)
44+
45+
3146
# ── Low-level helpers ────────────────────────────────────────────────────────
3247

3348

@@ -411,6 +426,27 @@ async def get_edge_types(self) -> list[EdgeTypeInfo]:
411426
resp = await self._schema_stub.ListEdgeTypes(ListEdgeTypesRequest(), timeout=self._timeout)
412427
return [EdgeTypeInfo(et) for et in resp.edge_types]
413428

429+
@staticmethod
430+
def _validate_property_dict(p: Any, idx: int) -> tuple[str, str, bool, bool]:
431+
"""Validate a single property dict and return ``(name, type_str, required, unique)``."""
432+
if not isinstance(p, dict):
433+
raise ValueError(f"Property at index {idx} must be a dict; got {p!r}")
434+
name = p.get("name")
435+
if not isinstance(name, str) or not name:
436+
raise ValueError(f"Property at index {idx} must have a non-empty 'name' key; got {p!r}")
437+
raw_type = p.get("type", "string")
438+
if "type" in p and not isinstance(raw_type, str):
439+
raise ValueError(f"Property {name!r} must use a string value for 'type'; got {raw_type!r}")
440+
type_str = str(raw_type).strip().lower()
441+
required = p.get("required", False)
442+
unique = p.get("unique", False)
443+
if not isinstance(required, bool) or not isinstance(unique, bool):
444+
raise ValueError(
445+
f"Property {name!r} must use boolean values for 'required' and 'unique'; got "
446+
f"required={required!r}, unique={unique!r}"
447+
)
448+
return name, type_str, required, unique
449+
414450
@staticmethod
415451
def _build_property_definitions(
416452
properties: list[dict[str, Any]] | None,
@@ -439,27 +475,12 @@ def _build_property_definitions(
439475
raise ValueError(f"'properties' must be a list of property dicts or None; got {type(properties).__name__}")
440476
result = []
441477
for idx, p in enumerate(properties):
442-
if not isinstance(p, dict):
443-
raise ValueError(f"Property at index {idx} must be a dict; got {p!r}")
444-
name = p.get("name")
445-
if not isinstance(name, str) or not name:
446-
raise ValueError(f"Property at index {idx} must have a non-empty 'name' key; got {p!r}")
447-
raw_type = p.get("type", "string")
448-
if "type" in p and not isinstance(raw_type, str):
449-
raise ValueError(f"Property {name!r} must use a string value for 'type'; got {raw_type!r}")
450-
type_str = str(raw_type).strip().lower()
478+
name, type_str, required, unique = AsyncCoordinodeClient._validate_property_dict(p, idx)
451479
if type_str not in type_map:
452480
raise ValueError(
453481
f"Unknown property type {type_str!r} for property {name!r}. "
454482
f"Expected 'type' to be one of: {sorted(type_map)}"
455483
)
456-
required = p.get("required", False)
457-
unique = p.get("unique", False)
458-
if not isinstance(required, bool) or not isinstance(unique, bool):
459-
raise ValueError(
460-
f"Property {name!r} must use boolean values for 'required' and 'unique'; got "
461-
f"required={required!r}, unique={unique!r}"
462-
)
463484
result.append(
464485
property_definition_cls(
465486
name=name,
@@ -568,10 +589,14 @@ async def create_text_index(
568589
info = await client.create_text_index("article_body", "Article", "body")
569590
# then: results = await client.text_search("Article", "machine learning")
570591
"""
592+
_validate_cypher_identifier(name, "name")
593+
_validate_cypher_identifier(label, "label")
571594
if isinstance(properties, str):
572595
prop_list = [properties]
573596
else:
574597
prop_list = list(properties)
598+
for prop in prop_list:
599+
_validate_cypher_identifier(prop, "property")
575600
props_expr = ", ".join(prop_list)
576601
lang_clause = f" DEFAULT LANGUAGE {language}" if language else ""
577602
cypher = f"CREATE TEXT INDEX {name} ON :{label}({props_expr}){lang_clause}"
@@ -590,6 +615,7 @@ async def drop_text_index(self, name: str) -> None:
590615
591616
await client.drop_text_index("article_body")
592617
"""
618+
_validate_cypher_identifier(name, "name")
593619
await self.cypher(f"DROP TEXT INDEX {name}")
594620

595621
async def traverse(

demo/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Open: http://localhost:38888 (token: `demo`)
3030
| Port | Service |
3131
|------|---------|
3232
| 37080 | CoordiNode gRPC |
33+
| 37084 | CoordiNode metrics/health (`/metrics`, `/health`) |
3334
| 38888 | Jupyter Lab |
3435

3536
## With OpenAI (optional)

0 commit comments

Comments
 (0)