diff --git a/CHANGELOG.md b/CHANGELOG.md index b74f290b..5ff84b29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## Unreleased + +Python client generation improvements: + +- Generated Python clients with nested namespaces now import cleanly when models refer to parent or sibling namespaces, including cases like `offer_rules.InsurerCategory`. +- Namespace names containing characters that are not valid in Python identifiers, such as dashes or leading digits, now generate valid Python classes. +- Schemas with a root namespace named `sys` no longer conflict with Python's standard `sys` module. +- Re-running `reflectapi codegen` into an existing output directory now removes generated files from older schemas while preserving hand-written files. + ## 0.17.4 Python client codegen fixes: diff --git a/reflectapi-cli/src/main.rs b/reflectapi-cli/src/main.rs index d51871fb..33bd4b2f 100644 --- a/reflectapi-cli/src/main.rs +++ b/reflectapi-cli/src/main.rs @@ -2,7 +2,10 @@ use anyhow::Context; use clap::{Parser, Subcommand, ValueEnum}; use std::collections::BTreeSet; use std::io::Write; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; + +const GENERATED_MARKER: &str = "This file was generated by reflectapi-cli"; +const GENERATED_MANIFEST: &str = ".reflectapi-generated-files"; #[derive(Parser)] #[command(version, about, long_about = None)] @@ -252,12 +255,15 @@ fn main() -> anyhow::Result<()> { std::fs::create_dir_all(&output_path).context(format!( "Failed to create output directory: {output_path:?}" ))?; + let expected_files = generated_file_set(files.keys())?; + cleanup_stale_generated_files(&output_path, &expected_files, &language)?; for (filename, content) in &files { write_file( &output_path.join(generated_relative_path(filename)?), content, )?; } + write_generated_manifest(&output_path, &expected_files)?; } else { let parent = parent_or_dot(&output_path); std::fs::create_dir_all(&parent) @@ -276,6 +282,131 @@ fn main() -> anyhow::Result<()> { } } +fn generated_file_set<'a>( + filenames: impl Iterator, +) -> anyhow::Result> { + filenames + .map(|filename| generated_relative_path(filename).map(Path::to_path_buf)) + .collect() +} + +fn cleanup_stale_generated_files( + output_dir: &Path, + expected_files: &BTreeSet, + language: &Language, +) -> anyhow::Result<()> { + let manifest_path = output_dir.join(GENERATED_MANIFEST); + let stale_candidates = if manifest_path.is_file() { + read_generated_manifest(&manifest_path)? + } else { + let mut candidates = BTreeSet::new(); + collect_legacy_generated_files(output_dir, output_dir, language, &mut candidates)?; + candidates + }; + + for relative_path in stale_candidates { + if expected_files.contains(&relative_path) { + continue; + } + let path = output_dir.join(&relative_path); + if path.is_file() && is_generated_file(&path, language) { + std::fs::remove_file(&path) + .context(format!("Failed to remove stale generated file: {path:?}"))?; + prune_empty_generated_dirs(output_dir, path.parent()); + } + } + + Ok(()) +} + +fn read_generated_manifest(path: &Path) -> anyhow::Result> { + let content = std::fs::read_to_string(path) + .context(format!("Failed to read generated file manifest: {path:?}"))?; + let mut files = BTreeSet::new(); + for line in content.lines() { + let line = line.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + files.insert(generated_relative_path(line)?.to_path_buf()); + } + Ok(files) +} + +fn collect_legacy_generated_files( + output_dir: &Path, + dir: &Path, + language: &Language, + files: &mut BTreeSet, +) -> anyhow::Result<()> { + for entry in std::fs::read_dir(dir).context(format!("Failed to read directory: {dir:?}"))? { + let entry = entry?; + let path = entry.path(); + let file_type = entry + .file_type() + .context(format!("Failed to inspect directory entry: {path:?}"))?; + if file_type.is_symlink() { + continue; + } + if file_type.is_dir() { + collect_legacy_generated_files(output_dir, &path, language, files)?; + continue; + } + if path.file_name().and_then(|name| name.to_str()) == Some(GENERATED_MANIFEST) { + continue; + } + if is_generated_file(&path, language) { + let relative = path + .strip_prefix(output_dir) + .context(format!("Generated path escaped output directory: {path:?}"))?; + files.insert(generated_relative_path(&relative.to_string_lossy())?.to_path_buf()); + } + } + Ok(()) +} + +fn is_generated_file(path: &Path, language: &Language) -> bool { + if !matches_language_extension(path, language) { + return false; + } + let Ok(content) = std::fs::read_to_string(path) else { + return false; + }; + content.contains(GENERATED_MARKER) +} + +fn matches_language_extension(path: &Path, language: &Language) -> bool { + let extension = path.extension().and_then(|extension| extension.to_str()); + match language { + Language::Typescript => extension == Some("ts"), + Language::Rust => extension == Some("rs"), + Language::Python => extension == Some("py"), + Language::Openapi => extension == Some("json"), + } +} + +fn prune_empty_generated_dirs(output_dir: &Path, mut dir: Option<&Path>) { + while let Some(current) = dir { + if current == output_dir || !current.starts_with(output_dir) { + break; + } + match std::fs::remove_dir(current) { + Ok(()) => dir = current.parent(), + Err(_) => break, + } + } +} + +fn write_generated_manifest(output_dir: &Path, files: &BTreeSet) -> anyhow::Result<()> { + let mut content = format!("# {GENERATED_MARKER}\n"); + content.push_str("# Relative files generated during the last reflectapi codegen run.\n"); + for file in files { + content.push_str(&file.to_string_lossy()); + content.push('\n'); + } + write_file(&output_dir.join(GENERATED_MANIFEST), &content) +} + fn parent_or_dot(path: &std::path::Path) -> std::path::PathBuf { path.parent() .filter(|p| !p.as_os_str().is_empty()) diff --git a/reflectapi-cli/tests/output_paths.rs b/reflectapi-cli/tests/output_paths.rs index 920f390b..16a7e385 100644 --- a/reflectapi-cli/tests/output_paths.rs +++ b/reflectapi-cli/tests/output_paths.rs @@ -8,7 +8,7 @@ //! - stdout via `--output -`, which must print the language's *primary* //! file rather than the alphabetically-first one. -use std::process::Command; +use std::{fs, process::Command}; fn cargo_bin() -> std::path::PathBuf { let bin = env!("CARGO_BIN_EXE_reflectapi"); @@ -30,6 +30,49 @@ fn run(args: &[&str]) -> std::process::Output { .expect("spawn reflectapi") } +fn write_minimal_python_schema(path: &std::path::Path, type_name: &str) { + let schema = format!( + r#"{{ + "name": "CLI stale cleanup test", + "description": "", + "functions": [ + {{ + "name": "items.get", + "path": "", + "output_kind": "complete", + "output_type": {{ "name": "{type_name}" }}, + "serialization": ["json"], + "readonly": true + }} + ], + "input_types": {{ "types": [] }}, + "output_types": {{ + "types": [ + {{ + "kind": "primitive", + "name": "std::string::String", + "description": "String" + }}, + {{ + "kind": "struct", + "name": "{type_name}", + "fields": {{ + "named": [ + {{ + "name": "value", + "type": {{ "name": "std::string::String" }}, + "required": true + }} + ] + }} + }} + ] + }} +}}"# + ); + fs::write(path, schema).unwrap(); +} + #[test] fn ts_output_into_fresh_directory() { let tmp = tempfile::tempdir().unwrap(); @@ -80,6 +123,112 @@ fn python_output_into_fresh_directory() { assert!(target.join("__init__.py").is_file()); } +#[test] +fn python_directory_output_removes_stale_generated_files() { + let tmp = tempfile::tempdir().unwrap(); + let target = tmp.path().join("python-client"); + fs::create_dir_all(target.join("legacy")).unwrap(); + fs::write( + target.join("legacy/__init__.py"), + "\"\"\"\nDO NOT MODIFY THIS FILE MANUALLY\nThis file was generated by reflectapi-cli\n\"\"\"\n", + ) + .unwrap(); + fs::write(target.join("user_notes.py"), "# hand written\n").unwrap(); + + let schema_one = tmp.path().join("schema-one.json"); + let schema_two = tmp.path().join("schema-two.json"); + write_minimal_python_schema(&schema_one, "first::child::Thing"); + write_minimal_python_schema(&schema_two, "second::Thing"); + + let out = run(&[ + "codegen", + "--language", + "python", + "--schema", + schema_one.to_str().unwrap(), + "--output", + target.to_str().unwrap(), + ]); + assert!( + out.status.success(), + "stderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + !target.join("legacy/__init__.py").exists(), + "pre-manifest generated files should be removed by header scan" + ); + assert!(target.join("first/child/__init__.py").is_file()); + assert!(target.join("user_notes.py").is_file()); + + let out = run(&[ + "codegen", + "--language", + "python", + "--schema", + schema_two.to_str().unwrap(), + "--output", + target.to_str().unwrap(), + ]); + assert!( + out.status.success(), + "stderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + !target.join("first/child/__init__.py").exists(), + "manifest-listed files absent from the new generation should be removed" + ); + assert!(target.join("second/__init__.py").is_file()); + assert!(target.join("user_notes.py").is_file()); + + let manifest = fs::read_to_string(target.join(".reflectapi-generated-files")).unwrap(); + assert!(manifest.contains("second/__init__.py")); + assert!(!manifest.contains("first/child/__init__.py")); +} + +#[cfg(unix)] +#[test] +fn python_legacy_cleanup_skips_symlinked_directories() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + let target = tmp.path().join("python-client"); + let external = tmp.path().join("external"); + fs::create_dir_all(&target).unwrap(); + fs::create_dir_all(&external).unwrap(); + fs::write( + external.join("__init__.py"), + "\"\"\"\nDO NOT MODIFY THIS FILE MANUALLY\nThis file was generated by reflectapi-cli\n\"\"\"\n", + ) + .unwrap(); + symlink(&external, target.join("linked")).unwrap(); + + let schema = tmp.path().join("schema.json"); + write_minimal_python_schema(&schema, "current::Thing"); + + let out = run(&[ + "codegen", + "--language", + "python", + "--schema", + schema.to_str().unwrap(), + "--output", + target.to_str().unwrap(), + ]); + assert!( + out.status.success(), + "stderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + external.join("__init__.py").is_file(), + "legacy cleanup should not follow directory symlinks out of the output tree" + ); + assert!(target.join("linked").is_symlink()); + assert!(target.join("current/__init__.py").is_file()); +} + #[test] fn ts_output_to_file_path_writes_siblings() { // --output …/generated.ts should still work; transport file lands diff --git a/reflectapi-demo/clients/python/api_client/.reflectapi-generated-files b/reflectapi-demo/clients/python/api_client/.reflectapi-generated-files new file mode 100644 index 00000000..8ec82d92 --- /dev/null +++ b/reflectapi-demo/clients/python/api_client/.reflectapi-generated-files @@ -0,0 +1,11 @@ +# This file was generated by reflectapi-cli +# Relative files generated during the last reflectapi codegen run. +__init__.py +_client.py +_rebuild.py +generated.py +myapi/__init__.py +myapi/model/__init__.py +myapi/model/input/__init__.py +myapi/model/output/__init__.py +myapi/proto/__init__.py diff --git a/reflectapi-demo/clients/python/api_client/myapi/coverage/__init__.py b/reflectapi-demo/clients/python/api_client/myapi/coverage/__init__.py deleted file mode 100644 index 5bc89634..00000000 --- a/reflectapi-demo/clients/python/api_client/myapi/coverage/__init__.py +++ /dev/null @@ -1,343 +0,0 @@ -""" -DO NOT MODIFY THIS FILE MANUALLY -This file was generated by reflectapi-cli - -Schema name: Demo application -This is a demo application -""" - -from __future__ import annotations - - -# Standard library imports -import warnings -from collections.abc import AsyncIterator, Iterator -from datetime import datetime -from enum import Enum -from typing import Annotated, Any, Generic, Literal, Optional, TypeVar, Union - -# Third-party imports -from pydantic import ( - BaseModel, - ConfigDict, - Field, - RootModel, - model_serializer, - model_validator, -) - -# Runtime imports -from reflectapi_runtime import AsyncClientBase, ClientBase, ApiResponse -from reflectapi_runtime import ReflectapiDuration -from reflectapi_runtime import ReflectapiEmpty -from reflectapi_runtime import ReflectapiInfallible -from reflectapi_runtime import ReflectapiPartialModel -from reflectapi_runtime import ( - parse_externally_tagged as _parse_externally_tagged, - serialize_externally_tagged as _serialize_externally_tagged, -) - - -# Type variables for generic types - - -C = TypeVar("C") - -T = TypeVar("T") - - -# External type definitions -StdNumNonZeroU32 = Annotated[int, "Rust NonZero u32 type"] -StdNumNonZeroU64 = Annotated[int, "Rust NonZero u64 type"] -StdNumNonZeroI32 = Annotated[int, "Rust NonZero i32 type"] -StdNumNonZeroI64 = Annotated[int, "Rust NonZero i64 type"] - - -class MyapiCoverageBaseModel(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - label: str - - -class MyapiCoverageRequest(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - keywords: myapi.coverage.PyKeywordFields - reserved: myapi.coverage.PydanticReservedFields - tree: myapi.coverage.TreeNode - mutual: myapi.coverage.MutualA - generic_tree: myapi.coverage.GenericTree[int] - keyword_variants: myapi.coverage.KeywordVariants - int_keyed: myapi.coverage.IntKeyedMap - user_id: int - deep_option: myapi.coverage.DeepOption - shadowing: myapi.coverage.ShadowingFields - empty: myapi.coverage.EmptyStruct - weird_doc: myapi.coverage.WeirdDocstring - shadow_base_model: myapi.coverage.BaseModel - wrapper_int: myapi.coverage.Wrapper[int] - wrapper_str: myapi.coverage.Wrapper[str] - defaulted: myapi.coverage.DefaultedField - - -class MyapiCoverageResponse(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - ok: bool - - -class MyapiCoverageDeepOption(ReflectapiPartialModel): - model_config = ConfigDict( - extra="ignore", - populate_by_name=True, - validate_assignment=True, - protected_namespaces=(), - ) - - maybe_maybe: str | None | None = None - - -class MyapiCoverageDefaultedField(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - count: int | None = None - - -class MyapiCoverageEmptyStruct(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - -class MyapiCoverageGenericTree(BaseModel, Generic[T]): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - value: T - children: list[myapi.coverage.GenericTree[T]] - - -class MyapiCoverageIntKeyedMap(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - by_id: dict[int, str] - uuid_keyed: dict[str, str] - - -class MyapiCoverageMutualA(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - name: str - b: myapi.coverage.MutualB | None = None - - -class MyapiCoverageMutualB(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - name: str - a: myapi.coverage.MutualA | None = None - - -class MyapiCoveragePyKeywordFields(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - class_: str = Field(serialization_alias="class", validation_alias="class") - from_: int = Field(serialization_alias="from", validation_alias="from") - import_: bool = Field(serialization_alias="import", validation_alias="import") - lambda_: int = Field(serialization_alias="lambda", validation_alias="lambda") - return_: list[int] = Field(serialization_alias="return", validation_alias="return") - yield_: str | None = Field( - default=None, serialization_alias="yield", validation_alias="yield" - ) - none: int | None = Field( - default=None, serialization_alias="None", validation_alias="None" - ) - true: bool = Field(serialization_alias="True", validation_alias="True") - type_: str = Field(serialization_alias="type", validation_alias="type") - match: str - - -class MyapiCoveragePydanticReservedFields(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - model_config_: str = Field( - serialization_alias="model_config", validation_alias="model_config" - ) - model_fields_set_: str = Field( - serialization_alias="model_fields_set", validation_alias="model_fields_set" - ) - model_dump_json_: str = Field( - serialization_alias="model_dump_json", validation_alias="model_dump_json" - ) - - -class MyapiCoverageShadowingFields(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - field: str - annotated: str - generic: str - base_model: str - - -class MyapiCoverageTreeNode(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - value: str - children: list[myapi.coverage.TreeNode] - parent: myapi.coverage.TreeNode | None = None - - -class MyapiCoverageWeirdDocstring(BaseModel): - """A docstring with "quotes" and 'apostrophes' and a backslash: \\\\ -And a \"\"\"triple quote\"\"\" inside.""" - - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - value: str - mixed_quotes: str = Field( - description="Field description with \"double quotes\" and 'single quotes'." - ) - doubles_only: str = Field( - description='Field description with only "double quotes" — should use single-quoted Python literal.' - ) - - -class MyapiCoverageWrapper(BaseModel, Generic[T]): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - inner: T - - -class MyapiCoverageKeywordVariantsClass(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - kind: Literal["class"] = Field(default="class", description="Discriminator field") - - -class MyapiCoverageKeywordVariantsLambda(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - kind: Literal["lambda"] = Field(default="lambda", description="Discriminator field") - name: str - - -class MyapiCoverageKeywordVariantsReturn(BaseModel): - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - kind: Literal["return"] = Field(default="return", description="Discriminator field") - value: int - - -class MyapiCoverageKeywordVariants(RootModel): - root: Annotated[ - Union[ - MyapiCoverageKeywordVariantsClass, - MyapiCoverageKeywordVariantsLambda, - MyapiCoverageKeywordVariantsReturn, - ], - Field(discriminator="kind"), - ] - - -# Public aliases for this module -BaseModel = MyapiCoverageBaseModel -CoverageRequest = MyapiCoverageRequest -CoverageResponse = MyapiCoverageResponse -DeepOption = MyapiCoverageDeepOption -DefaultedField = MyapiCoverageDefaultedField -EmptyStruct = MyapiCoverageEmptyStruct -GenericTree = MyapiCoverageGenericTree -IntKeyedMap = MyapiCoverageIntKeyedMap -KeywordVariants = MyapiCoverageKeywordVariants -KeywordVariantsClass = MyapiCoverageKeywordVariantsClass -KeywordVariantsLambda = MyapiCoverageKeywordVariantsLambda -KeywordVariantsReturn = MyapiCoverageKeywordVariantsReturn -MutualA = MyapiCoverageMutualA -MutualB = MyapiCoverageMutualB -PyKeywordFields = MyapiCoveragePyKeywordFields -PydanticReservedFields = MyapiCoveragePydanticReservedFields -Request = MyapiCoverageRequest -Response = MyapiCoverageResponse -ShadowingFields = MyapiCoverageShadowingFields -TreeNode = MyapiCoverageTreeNode -WeirdDocstring = MyapiCoverageWeirdDocstring -Wrapper = MyapiCoverageWrapper - -__all__ = [ - "BaseModel", - "CoverageRequest", - "CoverageResponse", - "DeepOption", - "DefaultedField", - "EmptyStruct", - "GenericTree", - "IntKeyedMap", - "KeywordVariants", - "KeywordVariantsClass", - "KeywordVariantsLambda", - "KeywordVariantsReturn", - "MutualA", - "MutualB", - "MyapiCoverageBaseModel", - "MyapiCoverageDeepOption", - "MyapiCoverageDefaultedField", - "MyapiCoverageEmptyStruct", - "MyapiCoverageGenericTree", - "MyapiCoverageIntKeyedMap", - "MyapiCoverageKeywordVariants", - "MyapiCoverageKeywordVariantsClass", - "MyapiCoverageKeywordVariantsLambda", - "MyapiCoverageKeywordVariantsReturn", - "MyapiCoverageMutualA", - "MyapiCoverageMutualB", - "MyapiCoveragePyKeywordFields", - "MyapiCoveragePydanticReservedFields", - "MyapiCoverageRequest", - "MyapiCoverageResponse", - "MyapiCoverageShadowingFields", - "MyapiCoverageTreeNode", - "MyapiCoverageWeirdDocstring", - "MyapiCoverageWrapper", - "PyKeywordFields", - "PydanticReservedFields", - "Request", - "Response", - "ShadowingFields", - "TreeNode", - "WeirdDocstring", - "Wrapper", -] diff --git a/reflectapi-demo/clients/python/api_client/myapi/model/input/__init__.py b/reflectapi-demo/clients/python/api_client/myapi/model/input/__init__.py index ee87b5fb..d09b36a1 100644 --- a/reflectapi-demo/clients/python/api_client/myapi/model/input/__init__.py +++ b/reflectapi-demo/clients/python/api_client/myapi/model/input/__init__.py @@ -42,6 +42,12 @@ T = TypeVar("T") +import sys +from .... import myapi + +myapi.model = sys.modules[__name__.rsplit(".", 1)[0]] +myapi.model.input = sys.modules[__name__] + # External type definitions StdNumNonZeroU32 = Annotated[int, "Rust NonZero u32 type"] StdNumNonZeroU64 = Annotated[int, "Rust NonZero u64 type"] diff --git a/reflectapi-demo/clients/python/api_client/myapi/model/output/__init__.py b/reflectapi-demo/clients/python/api_client/myapi/model/output/__init__.py index 3ee45aeb..ad1b1616 100644 --- a/reflectapi-demo/clients/python/api_client/myapi/model/output/__init__.py +++ b/reflectapi-demo/clients/python/api_client/myapi/model/output/__init__.py @@ -42,6 +42,12 @@ T = TypeVar("T") +import sys +from .... import myapi + +myapi.model = sys.modules[__name__.rsplit(".", 1)[0]] +myapi.model.output = sys.modules[__name__] + # External type definitions StdNumNonZeroU32 = Annotated[int, "Rust NonZero u32 type"] StdNumNonZeroU64 = Annotated[int, "Rust NonZero u64 type"] diff --git a/reflectapi-demo/clients/python/api_client/myapi/order/__init__.py b/reflectapi-demo/clients/python/api_client/myapi/order/__init__.py deleted file mode 100644 index 3c760bdc..00000000 --- a/reflectapi-demo/clients/python/api_client/myapi/order/__init__.py +++ /dev/null @@ -1,109 +0,0 @@ -""" -DO NOT MODIFY THIS FILE MANUALLY -This file was generated by reflectapi-cli - -Schema name: Demo application -This is a demo application -""" - -from __future__ import annotations - - -# Standard library imports -import warnings -from collections.abc import AsyncIterator, Iterator -from datetime import datetime -from enum import Enum -from typing import Annotated, Any, Generic, Literal, Optional, TypeVar, Union - -# Third-party imports -from pydantic import ( - BaseModel, - ConfigDict, - Field, - RootModel, - model_serializer, - model_validator, -) - -# Runtime imports -from reflectapi_runtime import AsyncClientBase, ClientBase, ApiResponse -from reflectapi_runtime import ReflectapiDuration -from reflectapi_runtime import ReflectapiEmpty -from reflectapi_runtime import ReflectapiInfallible -from reflectapi_runtime import ReflectapiPartialModel -from reflectapi_runtime import ( - parse_externally_tagged as _parse_externally_tagged, - serialize_externally_tagged as _serialize_externally_tagged, -) - - -# Type variables for generic types - - -C = TypeVar("C") - -T = TypeVar("T") - - -# External type definitions -StdNumNonZeroU32 = Annotated[int, "Rust NonZero u32 type"] -StdNumNonZeroU64 = Annotated[int, "Rust NonZero u64 type"] -StdNumNonZeroI32 = Annotated[int, "Rust NonZero i32 type"] -StdNumNonZeroI64 = Annotated[int, "Rust NonZero i64 type"] - - -class MyapiOrderInsertData(BaseModel): - """A struct whose name begins with the parent namespace cap. - Exercises the namespace-alias path for both the stripped - (`order.InsertData`) and Rust-leaf (`order.OrderInsertData`) - forms.""" - - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - identity: str - alternative_part_number: tuple[str, str] | None = Field( - default=None, description="Exercises the `tuple[A, B]` rendering for `(A, B)`." - ) - - -class MyapiOrderPolicy(BaseModel, Generic[C, T]): - """Exercises `PhantomData` elision (the field carries no wire - data and must not appear in the Python model).""" - - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - name: str - - -class MyapiOrderRateLimit(BaseModel): - """Exercises the `std::time::Duration` ↔ `{secs, nanos}` wire - adapter.""" - - model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=() - ) - - retry_after: ReflectapiDuration - max_wait: ReflectapiDuration | None = None - - -# Public aliases for this module -InsertData = MyapiOrderInsertData -OrderInsertData = MyapiOrderInsertData -Policy = MyapiOrderPolicy -RateLimit = MyapiOrderRateLimit - -__all__ = [ - "InsertData", - "MyapiOrderInsertData", - "MyapiOrderPolicy", - "MyapiOrderRateLimit", - "OrderInsertData", - "Policy", - "RateLimit", -] diff --git a/reflectapi-demo/clients/python/api_client/myapi/proto/__init__.py b/reflectapi-demo/clients/python/api_client/myapi/proto/__init__.py index 3fc1cc4b..37ae33d7 100644 --- a/reflectapi-demo/clients/python/api_client/myapi/proto/__init__.py +++ b/reflectapi-demo/clients/python/api_client/myapi/proto/__init__.py @@ -54,6 +54,15 @@ StdNumNonZeroI64 = Annotated[int, "Rust NonZero i64 type"] +class _ReflectapiDeferredNamespace: + def __getattr__(self, name: str) -> None: + raise NameError(name) + + +if not hasattr(myapi, "model"): + myapi.model = _ReflectapiDeferredNamespace() + + class MyapiProtoHeaders(BaseModel): model_config = ConfigDict( extra="ignore", populate_by_name=True, protected_namespaces=(), defer_build=True diff --git a/reflectapi-demo/clients/python/api_client/std/__init__.py b/reflectapi-demo/clients/python/api_client/std/__init__.py deleted file mode 100644 index ad318f5a..00000000 --- a/reflectapi-demo/clients/python/api_client/std/__init__.py +++ /dev/null @@ -1,64 +0,0 @@ -""" -DO NOT MODIFY THIS FILE MANUALLY -This file was generated by reflectapi-cli - -Schema name: Demo application -This is a demo application -""" - -from __future__ import annotations - - -# Standard library imports -import warnings -from collections.abc import AsyncIterator, Iterator -from datetime import datetime, timedelta -from enum import Enum -from typing import Annotated, Any, Generic, Literal, Optional, TypeVar, Union - -# Third-party imports -from pydantic import ( - BaseModel, - ConfigDict, - Field, - RootModel, - model_serializer, - model_validator, -) - -# Runtime imports -from reflectapi_runtime import AsyncClientBase, ClientBase, ApiResponse -from reflectapi_runtime import ReflectapiEmpty -from reflectapi_runtime import ReflectapiInfallible -from reflectapi_runtime import ReflectapiPartialModel -from reflectapi_runtime import ( - parse_externally_tagged as _parse_externally_tagged, - serialize_externally_tagged as _serialize_externally_tagged, -) - - -# Type variables for generic types - - -C = TypeVar("C") - -T = TypeVar("T") - - -# External type definitions -StdNumNonZeroU32 = Annotated[int, "Rust NonZero u32 type"] -StdNumNonZeroU64 = Annotated[int, "Rust NonZero u64 type"] -StdNumNonZeroI32 = Annotated[int, "Rust NonZero i32 type"] -StdNumNonZeroI64 = Annotated[int, "Rust NonZero i64 type"] - - -from . import time - -try: - from .._rebuild import rebuild_models as _rebuild_models - - _rebuild_models() -except Exception: - pass - -__all__ = ["time"] diff --git a/reflectapi-demo/clients/python/api_client/std/time/__init__.py b/reflectapi-demo/clients/python/api_client/std/time/__init__.py deleted file mode 100644 index 30d24dc5..00000000 --- a/reflectapi-demo/clients/python/api_client/std/time/__init__.py +++ /dev/null @@ -1,67 +0,0 @@ -""" -DO NOT MODIFY THIS FILE MANUALLY -This file was generated by reflectapi-cli - -Schema name: Demo application -This is a demo application -""" - -from __future__ import annotations - - -# Standard library imports -import warnings -from collections.abc import AsyncIterator, Iterator -from datetime import datetime, timedelta -from enum import Enum -from typing import Annotated, Any, Generic, Literal, Optional, TypeVar, Union - -# Third-party imports -from pydantic import ( - BaseModel, - ConfigDict, - Field, - RootModel, - model_serializer, - model_validator, -) - -# Runtime imports -from reflectapi_runtime import AsyncClientBase, ClientBase, ApiResponse -from reflectapi_runtime import ReflectapiEmpty -from reflectapi_runtime import ReflectapiInfallible -from reflectapi_runtime import ReflectapiPartialModel -from reflectapi_runtime import ( - parse_externally_tagged as _parse_externally_tagged, - serialize_externally_tagged as _serialize_externally_tagged, -) - - -# Type variables for generic types - - -C = TypeVar("C") - -T = TypeVar("T") - - -# External type definitions -StdNumNonZeroU32 = Annotated[int, "Rust NonZero u32 type"] -StdNumNonZeroU64 = Annotated[int, "Rust NonZero u64 type"] -StdNumNonZeroI32 = Annotated[int, "Rust NonZero i32 type"] -StdNumNonZeroI64 = Annotated[int, "Rust NonZero i64 type"] - - -class StdTimeDuration(BaseModel): - """Time duration type""" - - model_config = ConfigDict(extra="ignore", populate_by_name=True) - - secs: int - nanos: int - - -# Public aliases for this module -Duration = StdTimeDuration - -__all__ = ["Duration", "StdTimeDuration"] diff --git a/reflectapi-demo/src/tests/namespace.rs b/reflectapi-demo/src/tests/namespace.rs index 1f66f321..a100802f 100644 --- a/reflectapi-demo/src/tests/namespace.rs +++ b/reflectapi-demo/src/tests/namespace.rs @@ -284,6 +284,266 @@ fn test_python_split_modules_bind_referenced_namespaces() { assert!(root_types_file.contains("order: order.Order")); } +#[test] +fn test_python_split_modules_import_parent_for_top_level_refs() { + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct InsurerCategory { + id: String, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct InsurerCategorySummary { + name: String, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct CategoriesGetResponse { + category: InsurerCategory, + summaries: Vec, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct FormData { + value: String, + category: InsurerCategory, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct ConfigsInsertRequest { + name: String, + form_data: FormData, + } + + async fn categories_get( + _s: S, + _: reflectapi::Empty, + _: reflectapi::Empty, + ) -> CategoriesGetResponse { + CategoriesGetResponse { + category: InsurerCategory { id: String::new() }, + summaries: vec![InsurerCategorySummary { + name: String::new(), + }], + } + } + + async fn configs_insert( + _s: S, + _request: ConfigsInsertRequest, + _: reflectapi::Empty, + ) -> reflectapi::Empty { + reflectapi::Empty {} + } + + let (schema, _) = reflectapi::Builder::<()>::new() + .route(categories_get, |b| b.name("offer_rules.categories.get")) + .route(configs_insert, |b| b.name("offer_rules.configs.insert")) + .rename_types( + "reflectapi_demo::tests::namespace::InsurerCategory", + "offer_rules::InsurerCategory", + ) + .rename_types( + "reflectapi_demo::tests::namespace::InsurerCategorySummary", + "offer_rules::InsurerCategorySummary", + ) + .rename_types( + "reflectapi_demo::tests::namespace::CategoriesGetResponse", + "offer_rules::categories::GetResponse", + ) + .rename_types( + "reflectapi_demo::tests::namespace::FormData", + "offer_rules::model::input::FormData", + ) + .rename_types( + "reflectapi_demo::tests::namespace::ConfigsInsertRequest", + "offer_rules::configs::InsertRequest", + ) + .build() + .unwrap(); + + let files = reflectapi::codegen::python::generate_files( + schema, + &reflectapi::codegen::python::Config::default(), + ) + .unwrap(); + + let categories_file = files.get("offer_rules/categories/__init__.py").unwrap(); + assert!(categories_file.contains("import sys"), "{categories_file}"); + assert!( + categories_file.contains("from ... import offer_rules"), + "{categories_file}" + ); + assert!( + categories_file.contains("offer_rules.categories = sys.modules[__name__]"), + "{categories_file}" + ); + assert!( + categories_file.contains("category: offer_rules.InsurerCategory"), + "{categories_file}" + ); + assert!( + categories_file.contains("summaries: list[offer_rules.InsurerCategorySummary]"), + "{categories_file}" + ); + + let configs_file = files.get("offer_rules/configs/__init__.py").unwrap(); + assert!( + configs_file.contains("from ... import offer_rules"), + "{configs_file}" + ); + assert!( + configs_file.contains("offer_rules.configs = sys.modules[__name__]"), + "{configs_file}" + ); + assert!( + configs_file.contains("if not hasattr(offer_rules, \"model\"):"), + "{configs_file}" + ); + assert!( + configs_file.contains(" offer_rules.model = _ReflectapiDeferredNamespace()"), + "{configs_file}" + ); + assert!( + configs_file.contains("form_data: offer_rules.model.input.FormData"), + "{configs_file}" + ); + + let form_data_file = files.get("offer_rules/model/input/__init__.py").unwrap(); + assert!( + form_data_file.contains("from .... import offer_rules"), + "{form_data_file}" + ); + assert!( + form_data_file.contains("offer_rules.model = sys.modules[__name__.rsplit(\".\", 1)[0]]"), + "{form_data_file}" + ); + assert!( + form_data_file.contains("offer_rules.model.input = sys.modules[__name__]"), + "{form_data_file}" + ); + assert!( + form_data_file.contains("category: offer_rules.InsurerCategory"), + "{form_data_file}" + ); +} + +#[test] +fn test_python_split_modules_handles_sys_root_and_sanitized_class_names() { + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct SysTop { + value: String, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct SysUsesTop { + top: SysTop, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct KeywordTop { + value: String, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct KeywordThing { + top: KeywordTop, + } + + async fn sys_get(_s: S, _: reflectapi::Empty, _: reflectapi::Empty) -> SysUsesTop { + SysUsesTop { + top: SysTop { + value: String::new(), + }, + } + } + + async fn keyword_get(_s: S, _: reflectapi::Empty, _: reflectapi::Empty) -> KeywordThing { + KeywordThing { + top: KeywordTop { + value: String::new(), + }, + } + } + + let (schema, _) = reflectapi::Builder::<()>::new() + .route(sys_get, |b| b.name("sys.child.get")) + .route(keyword_get, |b| b.name("class.1-bad.for.get")) + .rename_types("reflectapi_demo::tests::namespace::SysTop", "sys::Top") + .rename_types( + "reflectapi_demo::tests::namespace::SysUsesTop", + "sys::child::UsesTop", + ) + .rename_types( + "reflectapi_demo::tests::namespace::KeywordTop", + "class::Top", + ) + .rename_types( + "reflectapi_demo::tests::namespace::KeywordThing", + "class::1-bad::for::Thing", + ) + .build() + .unwrap(); + + let files = reflectapi::codegen::python::generate_files( + schema, + &reflectapi::codegen::python::Config::default(), + ) + .unwrap(); + + let sys_child_file = files.get("sys/child/__init__.py").unwrap(); + assert!( + sys_child_file.contains("import sys as _reflectapi_sys"), + "{sys_child_file}" + ); + assert!( + sys_child_file.contains("from ... import sys"), + "{sys_child_file}" + ); + assert!( + sys_child_file.contains("sys.child = _reflectapi_sys.modules[__name__]"), + "{sys_child_file}" + ); + assert!(sys_child_file.contains("top: sys.Top"), "{sys_child_file}"); + + let keyword_file = files.get("class_/_1_bad/for_/__init__.py").unwrap(); + assert!( + keyword_file.contains("class Class_1BadForThing(BaseModel):"), + "{keyword_file}" + ); + assert!( + keyword_file.contains("Thing = Class_1BadForThing"), + "{keyword_file}" + ); + assert!( + !keyword_file.contains("Class1-badForThing"), + "{keyword_file}" + ); + + let rebuild_file = files.get("_rebuild.py").unwrap(); + assert!( + rebuild_file.contains("from .class_._1_bad.for_ import Class_1BadForThing"), + "{rebuild_file}" + ); +} + #[test] fn test_python_split_modules_defer_peer_namespace_imports() { #[derive( @@ -417,7 +677,14 @@ fn test_python_split_modules_client_uses_hashed_namespace_class_name() { assert!(class_name.len() <= 80); assert_ne!(class_name, format!("Tier1{long_leaf_name}")); - assert!(tier1_file.contains(&format!("{alias_name} = (\n {class_name}\n)"))); + let alias_assignment = format!("{alias_name} = "); + let alias_start = tier1_file.find(&alias_assignment).unwrap(); + let alias_block = tier1_file[alias_start..] + .lines() + .take_while(|line| !line.trim().is_empty()) + .collect::>() + .join("\n"); + assert!(alias_block.contains(class_name), "{alias_block}"); let client_file = files.get("_client.py").unwrap(); assert!(client_file.contains(&format!("tier1.{alias_name}"))); diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 3726c680..e1080396 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -2271,21 +2271,46 @@ fn render_namespace_bindings(module: &templates::Module, ns_path: &[String]) -> let rendered_code = module_rendered_code(module); let current_module = module_qualified_name(ns_path); let references_current_module = contains_qualified_name(&rendered_code, ¤t_module); + let references_current_root = + ns_path.len() > 1 && contains_qualified_name(&rendered_code, &root_module(ns_path)); - if !references_current_module { + if !references_current_module && !references_current_root { return String::new(); } + let root = root_module(ns_path); + let sys_import = if root == "sys" { + "import sys as _reflectapi_sys" + } else { + "import sys" + }; + let sys_module = if root == "sys" { + "_reflectapi_sys" + } else { + "sys" + }; + let mut lines = Vec::new(); - lines.push("import sys".to_string()); + lines.push(sys_import.to_string()); if ns_path.len() > 1 { lines.push(format!( "from {} import {}", root_relative_prefix(ns_path.len()), - root_module(ns_path) + root )); } - lines.push(format!("{current_module} = sys.modules[__name__]")); + if ns_path.len() > 2 { + // During package import, Python may not have attached partially-loaded + // intermediate packages to the root module yet. + for depth in 2..ns_path.len() { + let parent_module = module_qualified_name(&ns_path[..depth]); + let split_count = ns_path.len() - depth; + lines.push(format!( + "{parent_module} = {sys_module}.modules[__name__.rsplit(\".\", {split_count})[0]]" + )); + } + } + lines.push(format!("{current_module} = {sys_module}.modules[__name__]")); lines.join("\n") } @@ -2345,13 +2370,43 @@ fn deferred_namespace_names( names } +fn deferred_root_namespace_names( + generation: &PythonGeneration, + module: &templates::Module, + ns_path: &[String], +) -> BTreeSet { + if ns_path.len() <= 1 { + return BTreeSet::new(); + } + + let rendered_code = module_rendered_code(module); + let current_child = ns_path + .get(1) + .map(|segment| safe_python_module_segment(segment)); + let root_name = root_module(ns_path); + let Some(root) = generation.module_tree.submodules.get(&ns_path[0]) else { + return BTreeSet::new(); + }; + + // Same-root sibling references such as `myapi.model.Input` need a + // placeholder on `myapi` until the real sibling package is imported. + root.submodules + .values() + .filter(|sub| !sub.is_empty()) + .map(|sub| safe_python_module_segment(&sub.name)) + .filter(|child| Some(child) != current_child.as_ref()) + .filter(|child| contains_qualified_name(&rendered_code, &format!("{root_name}.{child}"))) + .collect() +} + fn render_deferred_namespace_placeholders( generation: &PythonGeneration, module: &templates::Module, ns_path: &[String], ) -> String { let names = deferred_namespace_names(generation, module, ns_path); - if names.is_empty() { + let root_names = deferred_root_namespace_names(generation, module, ns_path); + if names.is_empty() && root_names.is_empty() { return String::new(); } @@ -2365,6 +2420,14 @@ fn render_deferred_namespace_placeholders( .into_iter() .map(|name| format!("{name} = _ReflectapiDeferredNamespace()")), ); + let root_name = root_module(ns_path); + lines.extend( + root_names.into_iter().map(|name| { + format!( + "if not hasattr({root_name}, \"{name}\"):\n {root_name}.{name} = _ReflectapiDeferredNamespace()" + ) + }), + ); lines.join("\n") } @@ -4739,10 +4802,21 @@ fn to_snake_case(s: &str) -> String { } fn to_pascal_case(s: &str) -> String { - // First, replace :: with _ to handle Rust module paths - let normalized = s.replace("::", "_"); + // First, replace Rust/module separators and any other invalid Python + // identifier characters with `_`. + let normalized: String = s + .replace("::", "_") + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '_' { + c + } else { + '_' + } + }) + .collect(); - normalized + let mut result = normalized .split('_') .map(|word| { let mut chars = word.chars(); @@ -4751,7 +4825,14 @@ fn to_pascal_case(s: &str) -> String { Some(first) => first.to_uppercase().chain(chars).collect(), } }) - .collect() + .collect::(); + if result.is_empty() { + result.push('_'); + } + if result.chars().next().is_some_and(|c| c.is_ascii_digit()) { + result.insert(0, '_'); + } + result } /// Produce a unique flat Python class name from a potentially-qualified Rust