fix a url generation error for default api helper, improve flexibility of the base consysapi object by subclassing based on request type

Author: tipatterson-dev

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "oshconnect"
-version = "0.5.1a3"
+version = "0.5.1a5"
 description = "Library for interfacing with OSH, helping guide visualization efforts, and providing a place to store configurations. Implements OGC CS API Part 3 (Pub/Sub) MQTT topic conventions including :data topics and resource event topics."
 readme = "README.md"
 authors = [

src/oshconnect/csapi4py/con_sys_api.py

@@ -6,29 +6,106 @@
 from .request_wrappers import post_request, put_request, get_request, delete_request
 
 
+class APIRequest(BaseModel):
+    """Base for per-verb request classes.
+
+    Holds the fields every HTTP method shares: ``url`` (required),
+    ``headers``, ``auth``. Subclasses (`GetRequest`, `PostRequest`,
+    `PutRequest`, `DeleteRequest`) extend with verb-specific fields —
+    ``params`` for GET/DELETE, ``body`` for POST/PUT — so the type
+    system rejects incoherent shapes (e.g. a GET carrying a body) at
+    construction time instead of silently sending them.
+
+    Subclasses implement ``execute()`` to dispatch through the
+    matching ``request_wrappers`` function.
+    """
+    url: HttpUrl = Field(...)
+    headers: Union[dict, None] = Field(None)
+    auth: Union[tuple, None] = Field(None)
+
+    def execute(self):
+        raise NotImplementedError("APIRequest subclasses must implement execute().")
+
+
+class GetRequest(APIRequest):
+    """GET — query parameters only; no body."""
+    params: Union[dict, None] = Field(None)
+
+    def execute(self):
+        return get_request(self.url, self.params, self.headers, self.auth)
+
+
+class PostRequest(APIRequest):
+    """POST — body, optional. ``dict`` lands in ``json``, ``str`` in ``data``."""
+    body: Union[dict, str, None] = Field(None)
+
+    def execute(self):
+        return post_request(self.url, self.body, self.headers, self.auth)
+
+
+class PutRequest(APIRequest):
+    """PUT — body, optional. Same body routing as POST."""
+    body: Union[dict, str, None] = Field(None)
+
+    def execute(self):
+        return put_request(self.url, self.body, self.headers, self.auth)
+
+
+class DeleteRequest(APIRequest):
+    """DELETE — query parameters only. HTTP allows a body but the
+    project's wrapper doesn't pass one, so we don't model it here."""
+    params: Union[dict, None] = Field(None)
+
+    def execute(self):
+        return delete_request(self.url, self.params, self.headers, self.auth)
+
+
 class ConnectedSystemAPIRequest(BaseModel):
-    url: HttpUrl = Field(None)
-    body: Union[dict, str] = Field(None)
-    params: dict = Field(None)
+    """Legacy single-class request shape used by the fluent
+    ``ConnectedSystemsRequestBuilder`` and the free helper functions
+    in ``oshconnect.api_helpers``. New code in ``APIHelper`` uses the
+    per-verb subclasses above.
+    """
+    url: Union[HttpUrl, None] = Field(None)
+    body: Union[dict, str, None] = Field(None)
+    params: Union[dict, None] = Field(None)
     request_method: str = Field('GET')
-    headers: dict = Field(None)
+    headers: Union[dict, None] = Field(None)
     auth: Union[tuple, None] = Field(None)
 
     def make_request(self):
+        self._validate_for_send()
         match self.request_method:
             case 'GET':
                 return get_request(self.url, self.params, self.headers, self.auth)
             case 'POST':
-                print(f'POST request: {self}')
                 return post_request(self.url, self.body, self.headers, self.auth)
             case 'PUT':
-                print(f'PUT request: {self}')
                 return put_request(self.url, self.body, self.headers, self.auth)
             case 'DELETE':
-                print(f'DELETE request: {self}')
                 return delete_request(self.url, self.params, self.headers, self.auth)
             case _:
-                raise ValueError('Invalid request method')
+                raise ValueError(f'Invalid request method: {self.request_method!r}')
+
+    def _validate_for_send(self):
+        """Final coherence check before dispatch.
+
+        ``url`` may be ``None`` during builder-style construction, but
+        an unset URL at send time is a programming error. ``GET`` with
+        a body is well-formed at the HTTP level but most servers ignore
+        the body — we reject it so the caller doesn't silently send
+        data that goes nowhere. ``POST``/``PUT`` bodies are optional;
+        ``DELETE`` with a body is allowed by HTTP and accepted here.
+        """
+        if self.url is None:
+            raise ValueError(
+                "ConnectedSystemAPIRequest cannot be sent: 'url' is not set."
+            )
+        if self.request_method == 'GET' and self.body is not None:
+            raise ValueError(
+                "GET requests must not carry a body; pass query parameters "
+                "via 'params' instead."
+            )
 
 
 class ConnectedSystemsRequestBuilder(BaseModel):

src/oshconnect/csapi4py/default_api_helpers.py

@@ -12,7 +12,7 @@
 
 from pydantic import BaseModel, Field
 
-from .con_sys_api import ConnectedSystemAPIRequest
+from .con_sys_api import DeleteRequest, GetRequest, PostRequest, PutRequest
 from .constants import APIResourceTypes, ContentTypes, APITerms
 
 
@@ -122,10 +122,9 @@ def create_resource(self, res_type: APIResourceTypes, json_data: any, parent_res
         if url_endpoint is None:
             url = self.resource_url_resolver(res_type, None, parent_res_id, from_collection)
         else:
-            url = f'{self.server_url}/{self.api_root}/{url_endpoint}'
-        api_request = ConnectedSystemAPIRequest(url=url, request_method='POST', auth=self.get_helper_auth(),
-                                                body=json_data, headers=req_headers)
-        return api_request.make_request()
+            url = f'{self.get_api_root_url()}/{url_endpoint}'
+        return PostRequest(url=url, body=json_data, headers=req_headers,
+                           auth=self.get_helper_auth()).execute()
 
     def retrieve_resource(self, res_type: APIResourceTypes, res_id: str = None, parent_res_id: str = None,
                           from_collection: bool = False,
@@ -145,10 +144,9 @@ def retrieve_resource(self, res_type: APIResourceTypes, res_id: str = None, pare
         if url_endpoint is None:
             url = self.resource_url_resolver(res_type, res_id, parent_res_id, from_collection)
         else:
-            url = f'{self.server_url}/{self.api_root}/{url_endpoint}'
-        api_request = ConnectedSystemAPIRequest(url=url, request_method='GET', auth=self.get_helper_auth(),
-                                                headers=req_headers)
-        return api_request.make_request()
+            url = f'{self.get_api_root_url()}/{url_endpoint}'
+        return GetRequest(url=url, headers=req_headers,
+                          auth=self.get_helper_auth()).execute()
 
     def get_resource(self, resource_type: APIResourceTypes, resource_id: str = None,
                      subresource_type: APIResourceTypes = None,
@@ -171,11 +169,8 @@ def get_resource(self, resource_type: APIResourceTypes, resource_id: str = None,
         res_id_str = f'/{resource_id}' if resource_id else ""
         sub_res_type_str = f'/{resource_type_to_endpoint(subresource_type)}' if subresource_type else ""
         complete_url = f'{base_api_url}/{resource_type_str}{res_id_str}{sub_res_type_str}'
-        api_request = ConnectedSystemAPIRequest(url=complete_url, request_method='GET', auth=self.get_helper_auth(),
-                                                headers=req_headers)
-        if params is not None:
-            api_request.params = params
-        return api_request.make_request()
+        return GetRequest(url=complete_url, params=params, headers=req_headers,
+                          auth=self.get_helper_auth()).execute()
 
     def update_resource(self, res_type: APIResourceTypes, res_id: str, json_data: any, parent_res_id: str = None,
                         from_collection: bool = False, url_endpoint: str = None, req_headers: dict = None):
@@ -192,12 +187,11 @@ def update_resource(self, res_type: APIResourceTypes, res_id: str, json_data: an
         :return:
         """
         if url_endpoint is None:
-            url = self.resource_url_resolver(res_type, None, parent_res_id, from_collection)
+            url = self.resource_url_resolver(res_type, res_id, parent_res_id, from_collection)
         else:
-            url = f'{self.server_url}/{self.api_root}/{url_endpoint}'
-        api_request = ConnectedSystemAPIRequest(url=url, request_method='PUT', auth=self.get_helper_auth(),
-                                                body=json_data, headers=req_headers)
-        return api_request.make_request()
+            url = f'{self.get_api_root_url()}/{url_endpoint}'
+        return PutRequest(url=url, body=json_data, headers=req_headers,
+                          auth=self.get_helper_auth()).execute()
 
     def delete_resource(self, res_type: APIResourceTypes, res_id: str, parent_res_id: str = None,
                         from_collection: bool = False, url_endpoint: str = None, req_headers: dict = None):
@@ -213,12 +207,11 @@ def delete_resource(self, res_type: APIResourceTypes, res_id: str, parent_res_id
         :return:
         """
         if url_endpoint is None:
-            url = self.resource_url_resolver(res_type, None, parent_res_id, from_collection)
+            url = self.resource_url_resolver(res_type, res_id, parent_res_id, from_collection)
         else:
-            url = f'{self.server_url}/{self.api_root}/{url_endpoint}'
-        api_request = ConnectedSystemAPIRequest(url=url, request_method='DELETE', auth=self.get_helper_auth(),
-                                                headers=req_headers)
-        return api_request.make_request()
+            url = f'{self.get_api_root_url()}/{url_endpoint}'
+        return DeleteRequest(url=url, headers=req_headers,
+                             auth=self.get_helper_auth()).execute()
 
     # Helpers
     def resource_url_resolver(self, subresource_type: APIResourceTypes, subresource_id: str = None,