refactor(model): remove create classmethod, set key with Field default factory

src/zulip_write_only_proxy/cli.py

@@ -1,20 +1,22 @@
-from typing import Annotated, Optional
+from typing import Annotated
 
 import typer
 
-from . import services
+from . import models, services
 
 app = typer.Typer()
 
 
 @app.command()
 def create(
     proposal_no: Annotated[int, typer.Argument()],
-    stream: Annotated[Optional[str], typer.Argument()] = None,
+    stream: Annotated[str, typer.Argument()],
 ):
     """Create a new scoped client for a proposal."""
     services.setup()
-    client = services.create_client(proposal_no, stream)
+    client = services.create_client(
+        models.ScopedClientCreate(proposal_no=proposal_no, stream=stream)
+    )
     typer.echo(client)
 
 

src/zulip_write_only_proxy/main.py

@@ -134,20 +134,14 @@ def healthcheck():
     }
 
 
-class ScopedClientWithKey(models.ScopedClient):
-    key: str  # type: ignore[assignment]
-
-
 @app.post("/create_client", tags=["Admin"])
 def create_client(
     admin_client: Annotated[models.AdminClient, fastapi.Depends(get_client)],
-    proposal_no: Annotated[int, fastapi.Query(...)],
-    stream: Annotated[Union[str, None], fastapi.Query()] = None,
-) -> ScopedClientWithKey:
+    client: Annotated[models.ScopedClient, fastapi.Depends(services.create_client)],
+) -> models.ScopedClientWithKey:
     try:
-        client = services.create_client(proposal_no, stream)
         dump = client.model_dump()
         dump["key"] = client.key.get_secret_value()
-        return ScopedClientWithKey(**dump)
+        return models.ScopedClientWithKey(**dump)
     except ValueError as e:
         raise fastapi.HTTPException(status_code=400, detail=str(e)) from e

src/zulip_write_only_proxy/models.py

@@ -6,8 +6,7 @@
 from typing import IO, Any, Union
 
 import zulip
-from pydantic import BaseModel, PrivateAttr, SecretStr, field_validator
-from typing_extensions import Self
+from pydantic import BaseModel, Field, PrivateAttr, SecretStr, field_validator
 
 log = logging.getLogger(__name__)
 
@@ -18,29 +17,18 @@ class PropagateMode(str, enum.Enum):
     change_later = "change_later"
 
 
-class ScopedClient(BaseModel):
-    key: SecretStr
-
+class ScopedClientCreate(BaseModel):
     proposal_no: int
-    stream: str
+    stream: str | None
 
-    _client: zulip.Client = PrivateAttr()
 
-    @classmethod
-    def create(
-        cls,
-        proposal_no: int,
-        stream: str | None = None,
-    ) -> Self:
-        self = cls(
-            key=SecretStr(secrets.token_urlsafe()),
-            proposal_no=proposal_no,
-            stream=stream or f"some-pattern-{proposal_no}",
-        )
+class ScopedClient(ScopedClientCreate):
+    key: SecretStr = Field(default_factory=lambda: SecretStr(secrets.token_urlsafe()))
 
-        self._client.add_subscriptions(streams=[{"name": self.stream}])
+    proposal_no: int
+    stream: str
 
-        return self
+    _client: zulip.Client = PrivateAttr()
 
     def upload_file(self, file: IO[Any]):
         return self._client.upload_file(file)
@@ -87,14 +75,14 @@ def update_message(
         return self._client.update_message(request)
 
 
+class ScopedClientWithKey(ScopedClient):
+    key: str  # type: ignore[assignment]
+
+
 class AdminClient(BaseModel):
-    key: SecretStr
+    key: SecretStr = Field(default_factory=lambda: SecretStr(secrets.token_urlsafe()))
     admin: bool
 
-    @classmethod
-    def create(cls) -> Self:
-        return cls(key=SecretStr(secrets.token_urlsafe()), admin=True)
-
     @field_validator("admin")
     def check_admin(cls, v: bool) -> bool:
         if not v:

src/zulip_write_only_proxy/services.py

@@ -1,7 +1,9 @@
 from __future__ import annotations
 
 from pathlib import Path
+from typing import Annotated
 
+import fastapi
 import zulip
 from pydantic.fields import ModelPrivateAttr
 from pydantic_core import PydanticUndefined
@@ -11,14 +13,16 @@
 REPOSITORY = repositories.JSONRepository(path=Path.cwd() / "config" / "clients.json")
 
 
-def create_client(proposal_no: int, stream: str | None = None) -> models.ScopedClient:
-    client = models.ScopedClient.create(proposal_no, stream)
+def create_client(
+    client: Annotated[models.ScopedClientCreate, fastapi.Depends()]
+) -> models.ScopedClient:
+    client = models.ScopedClient.model_validate(client, from_attributes=True)
     REPOSITORY.put(client)
     return client
 
 
 def create_admin() -> models.AdminClient:
-    client = models.AdminClient.create()
+    client = models.AdminClient(admin=True)
     REPOSITORY.put(client)
     return client
 

tests/conftest.py

@@ -41,7 +41,7 @@ def repository():
 
 @pytest.fixture
 def scoped_client():
-    return ScopedClient.create(1234, stream="Test Stream")
+    return ScopedClient(proposal_no=1234, stream="Test Stream")
 
 
 @pytest.fixture(autouse=True)

tests/test_fastapi.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from zulip_write_only_proxy import models, services
+from zulip_write_only_proxy import models
 
 if TYPE_CHECKING:
     from fastapi.testclient import TestClient
@@ -216,31 +216,15 @@ def test_create_client(fastapi_client, zulip_client):
     }
 
 
-def test_create_client_error(fastapi_client):
-    with patch(
-        "zulip_write_only_proxy.services.create_client",
-        MagicMock(side_effect=ValueError("Test Error")),
-    ):
-        # Call the API endpoint with invalid data
-        response = fastapi_client.post(
-            "/create_client",
-            headers={"X-API-key": "admin1"},
-            params={"proposal_no": 1234, "stream": "Test Stream"},
-        )
-
-        assert response.status_code == 400
-        assert response.json() == {"detail": "Test Error"}
-
-        # Check that the services module was called with the expected arguments
-        services.create_client.assert_called_once_with(1234, "Test Stream")
-
-
 @pytest.mark.parametrize(
     "client_type,kwargs",
-    [(models.AdminClient, {}), (models.ScopedClient, {"proposal_no": 0})],
+    [
+        (models.AdminClient, {"admin": True}),
+        (models.ScopedClient, {"proposal_no": 0, "stream": ""}),
+    ],
 )
 def test_get_me(client_type, kwargs, fastapi_client, zulip_client):
-    client = client_type.create(**kwargs)
+    client = client_type(**kwargs)
 
     with patch(
         "zulip_write_only_proxy.services.get_client",
@@ -252,3 +236,4 @@ def test_get_me(client_type, kwargs, fastapi_client, zulip_client):
         )
 
         assert response.status_code == 200
+        assert response.json() == client.model_dump(exclude="key")

tests/test_models.py

@@ -71,7 +71,7 @@ def test_send_message(scoped_client):
 
 
 def test_admin_client_create():
-    client = AdminClient.create()
+    client = AdminClient(admin=True)
 
     assert client.key is not None
 

tests/test_services.py

@@ -3,12 +3,14 @@
 import pytest
 
 from zulip_write_only_proxy import services
-from zulip_write_only_proxy.models import AdminClient, ScopedClient
+from zulip_write_only_proxy.models import AdminClient, ScopedClient, ScopedClientCreate
 from zulip_write_only_proxy.repositories import JSONRepository
 
 
 def test_create_client(repository: JSONRepository):
-    result = services.create_client(proposal_no=3)
+    result = services.create_client(
+        ScopedClientCreate(proposal_no=1234, stream="Test Stream")
+    )
     assert isinstance(result, ScopedClient)