endpoint selector tests (#58)

* add detection warnings for selected resources (#57)

* adds basic warnings framework with primary key not detected warning

* add tests for primary key warning and unresolved path params warning

* update ci config

* add warnings for missing response body

* move config to config object for tests

* fix ids to render

* add test for deselected parent

* add basic test for endpoint filtering

* fix test error

Author: sh-rp

dlt_openapi/__init__.py

@@ -10,7 +10,7 @@
 from dlt_openapi.utils.misc import import_class_from_string
 
 from .config import Config
-from .detector.base_detector import BaseDetector
+from .detector.base_detector import GLOBAL_WARNING_KEY, BaseDetector
 from .parser.openapi_parser import OpenapiParser
 from .renderer.base_renderer import BaseRenderer
 
@@ -53,11 +53,32 @@ def render(self, dry: bool = False) -> None:
         logger.info("Rendering project")
         if self.config.endpoint_filter:
             filtered_endpoints = self.config.endpoint_filter(self.openapi.endpoints)
-            self.openapi.endpoints.set_names_to_render(filtered_endpoints)
+            self.openapi.endpoints.set_ids_to_render(filtered_endpoints)
         self.renderer.run(self.openapi, dry=dry)
         logger.success(f"Rendered project to: {self.config.project_dir}")
         logger.info("You can now run your pipeline from this folder with 'python pipeline.py'.")
 
+    def print_warnings(self) -> None:
+        """print warnings to logger if any where encountered for endpoints that are being rendered"""
+        warnings = self.detector.get_warnings()
+        if not warnings:
+            logger.info("No warnings generated during parsing and detection")
+            return
+
+        # print the global warnings
+        if global_warnings := warnings.get(GLOBAL_WARNING_KEY):
+            logger.warning("Global warnings:")
+            for w in global_warnings:
+                logger.warning(w.msg)
+
+        # print warnings, but only for endpoints that where rendered
+        for endpoint_id, endpoint_warnings in warnings.items():
+            if endpoint_id in self.openapi.endpoints.endpoint_ids_to_render:
+                e = self.openapi.endpoints.endpoints_by_id[endpoint_id]
+                logger.warning(f"Warnings for endpoint {e.method} {e.path}:")
+                for w in endpoint_warnings:
+                    logger.warning(w.msg)
+
 
 def _get_project_for_url_or_path(  # pylint: disable=too-many-arguments
     url: Optional[str],
@@ -95,4 +116,5 @@ def create_new_client(
     project.parse()
     project.detect()
     project.render()
+    project.print_warnings()
     return project

dlt_openapi/cli/cli_endpoint_selection.py

@@ -25,4 +25,4 @@ def questionary_endpoint_selection(endpoints: EndpointCollection) -> Set[str]:
     ).ask()
 
     # return resource names of selected endpoints
-    return {e.detected_resource_name for e in selected_endpoints}
+    return {e.id for e in selected_endpoints}

dlt_openapi/config.py

@@ -43,13 +43,18 @@ class Config(BaseModel):
     global_limit: int = 0
     """Set a limit on how many items are emitted from a resource"""
 
+    # internal, do not set via config file
     project_dir: Path = None
 
     def __init__(self, *args: Any, **kwargs: Any) -> None:
         super(Config, self).__init__(*args, **kwargs)
-        base_dir = Path.cwd() if not self.output_path else Path.cwd() / self.output_path
-        project_folder = self.project_name + self.project_folder_suffix
-        self.project_dir = base_dir / project_folder
+        self.prepare()
+
+    def prepare(self) -> None:
+        if self.project_name and self.project_folder_suffix:
+            base_dir = Path.cwd() if not self.output_path else Path.cwd() / self.output_path
+            project_folder = self.project_name + self.project_folder_suffix
+            self.project_dir = base_dir / project_folder
 
     @staticmethod
     def load_from_path(path: Path, *args: Any, **kwargs: Any) -> "Config":

dlt_openapi/detector/base_detector.py

@@ -3,11 +3,17 @@
 """
 
 from abc import abstractmethod
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Dict, List
 
 if TYPE_CHECKING:
     from dlt_openapi.parser.openapi_parser import OpenapiParser
 
+GLOBAL_WARNING_KEY = "global"
+
+
+class BaseDetectionWarning:
+    msg: str = ""
+
 
 class BaseDetector:
     @abstractmethod
@@ -18,3 +24,8 @@ def run(self, open_api: "OpenapiParser") -> None:
             open_api (OpenapiParser): OpenAPI parser instance
         """
         ...
+
+    @abstractmethod
+    def get_warnings(self) -> Dict[str, List[BaseDetectionWarning]]:
+        """Get all warnings encountered during detection run"""
+        ...

dlt_openapi/detector/default/__init__.py

@@ -5,7 +5,7 @@
 from typing import Dict, List, Optional, Tuple, Union, cast
 
 from dlt_openapi.config import Config
-from dlt_openapi.detector.base_detector import BaseDetector
+from dlt_openapi.detector.base_detector import GLOBAL_WARNING_KEY, BaseDetector
 from dlt_openapi.detector.default import utils
 from dlt_openapi.detector.default.primary_key import detect_primary_key_by_name
 from dlt_openapi.parser.endpoints import Endpoint, EndpointCollection, Response, TransformerSetting
@@ -36,16 +36,27 @@
     RE_UNIQUE_KEY,
 )
 from .utils import to_int
+from .warnings import (
+    BaseDetectionWarning,
+    DataResponseNoBodyWarning,
+    DataResponseUndetectedWarning,
+    PrimaryKeyNotFoundWarning,
+    UnresolvedPathParametersWarning,
+)
 
 Tree = Dict[str, Union["str", "Tree"]]
 
 
 class DefaultDetector(BaseDetector):
+
+    warnings: Dict[str, List[BaseDetectionWarning]] = {}
+
     def __init__(self, config: Config) -> None:
         self.config = config
 
     def run(self, open_api: OpenapiParser) -> None:
         """Run the detector"""
+        self.warnings = {}
 
         # discover stuff from responses
         self.detect_paginators_and_responses(open_api.endpoints)
@@ -62,6 +73,11 @@ def run(self, open_api: OpenapiParser) -> None:
         # and sort resources by table name
         open_api.endpoints.endpoints.sort(key=lambda e: e.detected_table_name)
 
+        # add some warnings
+        for e in open_api.endpoints.endpoints:
+            if params := e.unresolvable_path_param_names:
+                self._add_warning(UnresolvedPathParametersWarning(params), e)
+
     def detect_resource_names(self, endpoints: EndpointCollection) -> None:
         """iterate all endpoints and find a strategy to select the right resource name"""
 
@@ -144,9 +160,9 @@ def detect_paginators_and_responses(self, endpoints: EndpointCollection) -> None
                 endpoint.detected_data_response.detected_payload = self.detect_response_payload(
                     endpoint.detected_data_response, expect_list=expect_list
                 )
-                self.detect_primary_key(endpoint.detected_data_response, endpoint.path)
+                self.detect_primary_key(endpoint, endpoint.detected_data_response, endpoint.path)
 
-    def detect_primary_key(self, response: Response, path: str) -> None:
+    def detect_primary_key(self, e: Endpoint, response: Response, path: str) -> None:
         """detect the primary key from the payload"""
         if not response.detected_payload:
             return
@@ -179,6 +195,9 @@ def detect_primary_key(self, response: Response, path: str) -> None:
         elif uuid_paths:
             response.detected_primary_key = uuid_paths[0]
 
+        if not response.detected_primary_key:
+            self._add_warning(PrimaryKeyNotFoundWarning(), e)
+
     def detect_main_response(self, endpoint: Endpoint) -> Optional[Response]:
         """Get main response and pagination for endpoint"""
 
@@ -191,6 +210,12 @@ def detect_main_response(self, endpoint: Endpoint) -> Optional[Response]:
             if response.status_code.startswith("2") and not main_response:
                 main_response = response
 
+        if not main_response:
+            self._add_warning(DataResponseUndetectedWarning(), endpoint)
+
+        if main_response and not main_response.schema:
+            self._add_warning(DataResponseNoBodyWarning(), endpoint)
+
         return main_response
 
     def detect_response_payload(self, response: Response, expect_list: bool) -> Optional[DataPropertyPath]:
@@ -397,3 +422,11 @@ def find_nearest_list_parent(endpoint: Endpoint) -> Optional[Endpoint]:
             endpoint.detected_parent = find_nearest_list_parent(endpoint)
             if endpoint.detected_parent:
                 endpoint.detected_parent.detected_children.append(endpoint)
+
+    def get_warnings(self) -> Dict[str, List[BaseDetectionWarning]]:
+        return self.warnings
+
+    def _add_warning(self, warning: BaseDetectionWarning, e: Optional[Endpoint] = None) -> None:
+        key = e.id if e else GLOBAL_WARNING_KEY
+        warning_list = self.warnings.setdefault(key, [])
+        warning_list.append(warning)

dlt_openapi/detector/default/warnings.py

@@ -0,0 +1,28 @@
+from typing import List
+
+from dlt_openapi.detector.base_detector import BaseDetectionWarning
+
+
+class PrimaryKeyNotFoundWarning(BaseDetectionWarning):
+    msg: str = "Primary key could not be detected"
+
+
+class UnresolvedPathParametersWarning(BaseDetectionWarning):
+
+    def __init__(self, params: List[str]) -> None:
+        self.params = params
+        self.msg = f"Could not resolve all path params, setting default values for: {','.join(params)}"
+
+
+class DataResponseUndetectedWarning(BaseDetectionWarning):
+    msg: str = (
+        "Could not detect the main data response with a status code 2xx. "
+        + "Will not be able to detect primary key and some paginators."
+    )
+
+
+class DataResponseNoBodyWarning(BaseDetectionWarning):
+    msg: str = (
+        "No json response schema defined on main data response. "
+        + "Will not be able to detect primary key and some paginators."
+    )

dlt_openapi/parser/endpoints.py

@@ -61,6 +61,11 @@ class Endpoint:
     detected_children: List["Endpoint"] = field(default_factory=list)
     detected_transformer_settings: Optional[TransformerSetting] = None
 
+    @property
+    def id(self) -> str:
+        """unique identifier"""
+        return self.operation_id
+
     @property
     def payload(self) -> Optional[DataPropertyPath]:
         """gets payload dataproperty path if detected"""
@@ -159,46 +164,39 @@ def from_operation(
 @dataclass
 class EndpointCollection:
     endpoints: List[Endpoint]
-    names_to_render: Set[str] = field(default_factory=set)
-    names_to_deselect: Set[str] = field(default_factory=set)
+    endpoint_ids_to_render: Set[str] = field(default_factory=set)
+    endpoint_ids_to_deselect: Set[str] = field(default_factory=set)
 
     @property
     def all_endpoints_to_render(self) -> List[Endpoint]:
         """get all endpoints we want to render"""
-        if not self.names_to_render:
-            return self.endpoints
-        return [e for e in self.endpoints if e.detected_resource_name in self.names_to_render]
-
-    @property
-    def all_endpoints_for_selector(self) -> List[Endpoint]:
-        pass
+        return [e for e in self.endpoints if e.id in self.endpoint_ids_to_render]
 
     @property
     def endpoints_by_path(self) -> Dict[str, Endpoint]:
         """Endpoints by path"""
         return {ep.path: ep for ep in self.endpoints}
 
     @property
-    def endpoints_by_detected_resource_name(self) -> Dict[str, Endpoint]:
+    def endpoints_by_id(self) -> Dict[str, Endpoint]:
         """Endpoints by path"""
-        return {ep.detected_resource_name: ep for ep in self.endpoints}
+        return {ep.id: ep for ep in self.endpoints}
 
-    def set_names_to_render(self, names: Set[str]) -> None:
-        selected_names = set()
-        render_names = set()
+    def set_ids_to_render(self, ids: Set[str]) -> None:
+        selected_ids = set()
+        ids_to_render = set()
 
         # traverse ancestry chain and make sure parent endpoints are also marked for rendering
         # but deselected
-        for name in names:
-            ep = self.endpoints_by_detected_resource_name[name]
-            render_names.add(ep.detected_resource_name)
-            selected_names.add(ep.detected_resource_name)
+        for endpoint_id in ids:
+            ep = self.endpoints_by_id[endpoint_id]
+            ids_to_render.add(ep.id)
+            selected_ids.add(ep.id)
             while ep.transformer and ep.parent:
-                render_names.add(ep.parent.detected_resource_name)
+                ids_to_render.add(ep.parent.id)
                 ep = ep.parent
-
-        self.names_to_render = render_names
-        self.names_to_deselect = render_names - selected_names
+        self.endpoint_ids_to_render = ids_to_render
+        self.endpoint_ids_to_deselect = ids_to_render - selected_ids
 
     @classmethod
     def from_context(cls, context: OpenapiContext) -> "EndpointCollection":
@@ -221,4 +219,4 @@ def from_context(cls, context: OpenapiContext) -> "EndpointCollection":
                         context=context,
                     )
                 )
-        return cls(endpoints=endpoints)
+        return cls(endpoints=endpoints, endpoint_ids_to_render={e.id for e in endpoints})

dlt_openapi/renderer/default/templates/source.py.j2

@@ -32,7 +32,7 @@ def {{ source_name }}(
             {% if endpoint.primary_key %}
             "primary_key": "{{ endpoint.primary_key }}",
             "write_disposition": "merge",
-            {% if endpoint.detected_resource_name in endpoint_collection.names_to_deselect %}
+            {% if endpoint.id in endpoint_collection.endpoint_ids_to_deselect %}
             "selected": False,
             {% endif %}
             {% endif %}