-
Notifications
You must be signed in to change notification settings - Fork 55
feat: separate user-facing schema (write/read) from internal model [INFP-234] #9814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 36 commits
fe0d631
3b79693
7976eb4
30a158a
0bbdd4a
234b483
c6b42e9
ab5bdf6
6cd7ac3
8e774c1
492e6ec
264fb46
9630b8b
f9c4c64
81bfb24
32bd22f
ae4c7c4
687373d
12a22a9
341b488
2b1e6a7
baad9b9
4ce7610
154c6ac
bcf6d31
2cfb3d1
cf25235
e18cee8
1d9945c
18d7f09
a4e00a4
b654344
b53b6ce
d0af4ec
55fe3d4
38f841a
f67c6ae
4b8ee94
408ba99
439b04a
8e4e234
65cde1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,9 +3,18 @@ | |
| from typing import TYPE_CHECKING, Any, Sequence | ||
|
|
||
| from fastapi import APIRouter, Depends, Query, Request | ||
| from infrahub_sdk.schema.generated.read import ( | ||
| GenericSchemaRead, | ||
| NodeSchemaRead, | ||
| ProfileSchemaRead, | ||
| TemplateSchemaRead, | ||
| ) | ||
| from infrahub_sdk.schema.generated.write import InfrahubSchemaWrite | ||
| from infrahub_sdk.schema.validate import validate_schema as validate_write_schema | ||
| from pydantic import ( | ||
| BaseModel, | ||
| Field, | ||
| PrivateAttr, | ||
| computed_field, | ||
| create_model, | ||
| model_validator, | ||
|
|
@@ -75,35 +84,26 @@ def from_schema(cls, schema: MainSchemaTypes) -> Self: | |
| data["relationships"] = [ | ||
| relationship.model_dump() for relationship in schema.relationships if not relationship.internal_peer | ||
| ] | ||
| # ``kind`` is a computed field on the generated read model (derived from namespace+name); | ||
| # only ``hash`` needs to be supplied here since it is computed by the server. | ||
| data["hash"] = schema.get_hash() | ||
| return cls(**data) | ||
|
|
||
| @model_validator(mode="before") | ||
| @classmethod | ||
| def set_kind(cls, values: Any) -> Any: | ||
| if isinstance(values, dict): | ||
| values["kind"] = f"{values['namespace']}{values['name']}" | ||
| return values | ||
|
|
||
| class APINodeSchema(NodeSchemaRead, APISchemaMixin): | ||
| pass | ||
|
|
||
| class APINodeSchema(NodeSchema, APISchemaMixin): | ||
| api_kind: str | None = Field(default=None, alias="kind", validate_default=True) | ||
| hash: str | ||
|
|
||
| class APIGenericSchema(GenericSchemaRead, APISchemaMixin): | ||
| pass | ||
|
|
||
| class APIGenericSchema(GenericSchema, APISchemaMixin): | ||
| api_kind: str | None = Field(default=None, alias="kind", validate_default=True) | ||
| hash: str | ||
|
|
||
| class APIProfileSchema(ProfileSchemaRead, APISchemaMixin): | ||
| pass | ||
|
|
||
| class APIProfileSchema(ProfileSchema, APISchemaMixin): | ||
| api_kind: str | None = Field(default=None, alias="kind", validate_default=True) | ||
| hash: str | ||
|
|
||
|
|
||
| class APITemplateSchema(TemplateSchema, APISchemaMixin): | ||
| api_kind: str | None = Field(default=None, alias="kind", validate_default=True) | ||
| hash: str | ||
| class APITemplateSchema(TemplateSchemaRead, APISchemaMixin): | ||
| pass | ||
|
|
||
|
|
||
| class SchemaReadAPI(BaseModel): | ||
|
|
@@ -115,8 +115,35 @@ class SchemaReadAPI(BaseModel): | |
| namespaces: list[SchemaNamespace] = Field(default_factory=list) | ||
|
|
||
|
|
||
| class SchemaLoadAPI(SchemaRoot): | ||
| class SchemaLoadAPI(InfrahubSchemaWrite): | ||
| version: str | ||
| _internal_schema: SchemaRoot = PrivateAttr() | ||
|
|
||
| @model_validator(mode="before") | ||
| @classmethod | ||
| def validate_write_contract(cls, data: Any) -> Any: | ||
| # The user-facing "write" contract lives in the SDK; each submitted node/generic is | ||
| # validated against it so that fields the user may not set (read-level, internal, or | ||
| # unknown) and out-of-range constrained values are rejected at the boundary with a | ||
| # field-level message, before the payload is coerced into the richer internal models. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this comment is unnecessary. There's no information here that is not covered by either where |
||
| if isinstance(data, dict): | ||
| result = validate_write_schema(schema=data) | ||
| if not result.valid: | ||
| raise ValueError("; ".join(result.messages)) | ||
| return data | ||
|
|
||
| @model_validator(mode="after") | ||
| def build_internal_schema(self) -> Self: | ||
| # The published request contract only exposes user-settable fields; the processing | ||
| # engine operates on the richer internal SchemaRoot. Building it here surfaces | ||
| # internal-only invariants (e.g. reserved keywords) as request-validation errors, | ||
| # and lets the endpoints feed the pipeline without re-deriving the conversion. | ||
|
Comment on lines
+175
to
+178
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4-line comment is overkill |
||
| self._internal_schema = SchemaRoot.model_validate(self.model_dump(exclude_none=True)) | ||
| return self | ||
|
|
||
| @property | ||
| def internal_schema(self) -> SchemaRoot: | ||
| return self._internal_schema | ||
|
|
||
|
|
||
| class SchemasLoadAPI(BaseModel): | ||
|
|
@@ -170,11 +197,11 @@ def _merge_candidate_schemas(schemas: Sequence[SchemaRoot]) -> SchemaRoot: | |
|
|
||
|
|
||
| def evaluate_candidate_schemas( | ||
| branch_schema: SchemaBranch, schemas_to_evaluate: SchemasLoadAPI | ||
| branch_schema: SchemaBranch, schemas_to_evaluate: Sequence[SchemaRoot] | ||
| ) -> tuple[SchemaBranch, SchemaUpdateValidationResult]: | ||
| candidate_schema = branch_schema.duplicate() | ||
| try: | ||
| schema = _merge_candidate_schemas(schemas=schemas_to_evaluate.schemas) | ||
| schema = _merge_candidate_schemas(schemas=schemas_to_evaluate) | ||
|
|
||
| candidate_schema.load_schema(schema=schema) | ||
| candidate_schema.process() | ||
|
|
@@ -357,10 +384,13 @@ async def load_schema( | |
|
|
||
| errors: list[str] = [] | ||
| warnings: list[SchemaWarning] = [] | ||
| candidate_schemas: list[SchemaRoot] = [] | ||
| for schema in schemas.schemas: | ||
| errors += schema.validate_namespaces() | ||
| errors += schema.validate_reserved_suffixes() | ||
| warnings += schema.gather_warnings() | ||
| internal_schema = schema.internal_schema | ||
| candidate_schemas.append(internal_schema) | ||
| errors += internal_schema.validate_namespaces() | ||
| errors += internal_schema.validate_reserved_suffixes() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like these two should be one function on |
||
| warnings += internal_schema.gather_warnings() | ||
|
|
||
| if errors: | ||
| raise SchemaNotValidError(message=", ".join(errors)) | ||
|
|
@@ -369,7 +399,9 @@ async def load_schema( | |
| branch_schema = registry.schema.get_schema_branch(name=branch.name) | ||
| original_hash = branch_schema.get_hash() | ||
|
|
||
| candidate_schema, result = evaluate_candidate_schemas(branch_schema=branch_schema, schemas_to_evaluate=schemas) | ||
| candidate_schema, result = evaluate_candidate_schemas( | ||
| branch_schema=branch_schema, schemas_to_evaluate=candidate_schemas | ||
| ) | ||
|
|
||
| if not result.diff.all: | ||
| return SchemaUpdate(hash=original_hash, previous_hash=original_hash, diff=result.diff) | ||
|
|
@@ -445,17 +477,22 @@ async def check_schema( | |
|
|
||
| errors: list[str] = [] | ||
| warnings: list[SchemaWarning] = [] | ||
| candidate_schemas: list[SchemaRoot] = [] | ||
| for schema in schemas.schemas: | ||
| errors += schema.validate_namespaces() | ||
| errors += schema.validate_reserved_suffixes() | ||
| warnings += schema.gather_warnings() | ||
| internal_schema = schema.internal_schema | ||
| candidate_schemas.append(internal_schema) | ||
| errors += internal_schema.validate_namespaces() | ||
| errors += internal_schema.validate_reserved_suffixes() | ||
| warnings += internal_schema.gather_warnings() | ||
|
|
||
| if errors: | ||
| raise SchemaNotValidError(message=", ".join(errors)) | ||
|
|
||
| branch_schema = registry.schema.get_schema_branch(name=branch.name) | ||
|
|
||
| candidate_schema, result = evaluate_candidate_schemas(branch_schema=branch_schema, schemas_to_evaluate=schemas) | ||
| candidate_schema, result = evaluate_candidate_schemas( | ||
| branch_schema=branch_schema, schemas_to_evaluate=candidate_schemas | ||
| ) | ||
|
|
||
| # ---------------------------------------------------------- | ||
| # Validate if the new schema is valid with the content of the database | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this comment is necessary