applies feedback discussed in pull requests

tdstein · tdstein · commit 82b9b7e1cf89 · 2024-10-24T17:37:46.000-04:00
diff --git a/src/posit/connect/jobs.py b/src/posit/connect/jobs.py
@@ -2,7 +2,7 @@
 
 from typing_extensions import NotRequired, Required, Unpack
 
-from .resources import Active, ActiveFinderMethods, Resource
+from .resources import Active, ActiveFinderMethods, ActiveSequence, Resource
 
 JobTag = Literal[
     "unknown",
@@ -123,22 +123,41 @@ def destroy(self) -> None:
         self._ctx.session.delete(self._endpoint)
 
 
-class Jobs(ActiveFinderMethods[Job]):
-    """A collection of jobs."""
+class Jobs(
+    ActiveFinderMethods[Job],
+    ActiveSequence[Job],
+):
+    def __init__(self, ctx, parent: Active, uid="key"):
+        """A collection of jobs.
 
-    _uid = "key"
-
-    def __init__(self, cls, ctx, parent: Active):
-        super().__init__(cls, ctx, parent)
+        Parameters
+        ----------
+        ctx : Context
+            The context containing the HTTP session used to interact with the API.
+        parent : Active
+            Parent resource for maintaining hierarchical relationships
+        uid : str, optional
+            The default field name used to uniquely identify records, by default "key"
+        """
+        super().__init__(ctx, parent, uid)
         self._parent = parent
 
     @property
     def _endpoint(self) -> str:
         return self._ctx.url + f"v1/content/{self._parent['guid']}/jobs"
 
+    def _create_instance(self, **kwargs) -> Job:
+        """Creates a `Job` instance.
+
+        Returns
+        -------
+        Job
+        """
+        return Job(self._ctx, self._parent, **kwargs)
+
     class _FindByRequest(TypedDict, total=False):
         # Identifiers
-        id: NotRequired[str]
+        id: Required[str]
         """A unique identifier for the job."""
 
         ppid: NotRequired[Optional[str]]
@@ -270,4 +289,4 @@ class JobsMixin(Active, Resource):
 
     def __init__(self, ctx, **kwargs):
         super().__init__(ctx, **kwargs)
-        self.jobs = Jobs(Job, ctx, self)
+        self.jobs = Jobs(ctx, self)
diff --git a/src/posit/connect/resources.py b/src/posit/connect/resources.py
@@ -1,9 +1,10 @@
 import warnings
 from abc import ABC, abstractmethod
 from dataclasses import dataclass
-from typing import Any, Generic, List, Optional, Sequence, Type, TypeVar, overload
+from typing import Any, Generic, List, Optional, Sequence, TypeVar, overload
 
 import requests
+from typing_extensions import Self
 
 from .context import Context
 from .urls import Url
@@ -48,39 +49,18 @@ def __init__(self, params: ResourceParameters) -> None:
         self.params = params
 
 
-class Active(Resource):
-    """
-    A base class representing an active resource.
-
-    Extends the `Resource` class and provides additional functionality for via the session context and an optional parent resource.
-
-    Parameters
-    ----------
-    ctx : Context
-        The context object containing the session and URL for API interactions.
-    parent : Optional[Active], optional
-        An optional parent resource that establishes a hierarchical relationship, by default None.
-    **kwargs : dict
-        Additional keyword arguments passed to the parent `Resource` class.
-
-    Attributes
-    ----------
-    _ctx : Context
-        The session context.
-    _parent : Optional[Active]
-        The parent resource, if provided, which establishes a hierarchical relationship.
-    """
-
+class Active(ABC, Resource):
     def __init__(self, ctx: Context, parent: Optional["Active"] = None, **kwargs):
-        """
-        Initialize the `Active` resource.
+        """A base class representing an active resource.
+
+        Extends the `Resource` class and provides additional functionality for via the session context and an optional parent resource.
 
         Parameters
         ----------
         ctx : Context
-            The context object containing session and URL for API interactions.
+            The context object containing the session and URL for API interactions.
         parent : Optional[Active], optional
-            An optional parent resource to establish a hierarchical relationship, by default None.
+            An optional parent resource that establishes a hierarchical relationship, by default None.
         **kwargs : dict
             Additional keyword arguments passed to the parent `Resource` class.
         """
@@ -90,69 +70,27 @@ def __init__(self, ctx: Context, parent: Optional["Active"] = None, **kwargs):
         self._parent = parent
 
 
-T_co = TypeVar("T_co", bound="Active", covariant=True)
-"""A covariant type variable that is bound to the `Active` class and must inherit from it."""
+T = TypeVar("T", bound="Active")
+"""A type variable that is bound to the `Active` class"""
 
 
-class ActiveSequence(ABC, Generic[T_co], Sequence[T_co]):
-    """
-    A sequence abstraction for any HTTP GET endpoint that returns a collection.
-
-    It lazily fetches data on demand, caches the results, and allows for standard sequence operations like indexing and slicing.
+class ActiveSequence(ABC, Generic[T], Sequence[T]):
+    def __init__(self, ctx: Context, parent: Optional[Active] = None):
+        """A sequence abstraction for any HTTP GET endpoint that returns a collection.
 
-    Parameters
-    ----------
-    cls : Type[T_co]
-        The class used to represent each item in the sequence.
-    ctx : Context
-        The context object that holds the HTTP session used for sending the GET request.
-    parent : Optional[Active], optional
-        An optional parent resource to establish a nested relationship, by default None.
+        It lazily fetches data on demand, caches the results, and allows for standard sequence operations like indexing and slicing.
 
-    Attributes
-    ----------
-    _cls : Type[T_co]
-        The class used to instantiate each item in the sequence.
-    _ctx : Context
-        The context containing the HTTP session used to interact with the API.
-    _parent : Optional[Active]
-        Optional parent resource for maintaining hierarchical relationships.
-    _cache : Optional[List[T_co]]
-        Cached list of items returned from the API. Set to None initially, and populated after the first request.
-
-    Abstract Properties
-    -------------------
-    _endpoint : str
-        The API endpoint URL for the HTTP GET request. Subclasses are required to implement this property.
-
-    Methods
-    -------
-    _data() -> List[T_co]
-        Fetch and cache the data from the API. This method sends a GET request to `_endpoint`, parses the
-        response as JSON, and instantiates each item using `cls`.
-
-    __getitem__(index) -> Union[T_co, Sequence[T_co]]
-        Retrieve an item or slice from the sequence. Indexing follows the standard Python sequence semantics.
-
-    __len__() -> int
-        Return the number of items in the sequence.
-
-    __str__() -> str
-        Return a string representation of the cached data.
-
-    __repr__() -> str
-        Return a detailed string representation of the cached data.
-
-    reload() -> ActiveSequence
-        Clear the cache and mark to reload the data from the API on the next operation.
-    """
-
-    def __init__(self, cls: Type[T_co], ctx: Context, parent: Optional[Active] = None):
+        Parameters
+        ----------
+        ctx : Context
+            The context object that holds the HTTP session used for sending the GET request.
+        parent : Optional[Active], optional
+            An optional parent resource to establish a nested relationship, by default None.
+        """
         super().__init__()
-        self._cls = cls
         self._ctx = ctx
         self._parent = parent
-        self._cache = None
+        self._cache: Optional[List[T]] = None
 
     @property
     @abstractmethod
@@ -171,32 +109,32 @@ def _endpoint(self) -> str:
         raise NotImplementedError()
 
     @property
-    def _data(self) -> List[T_co]:
+    def _data(self) -> List[T]:
         """
         Fetch and cache the data from the API.
 
         This method sends a GET request to the `_endpoint` and parses the response as a list of JSON objects.
-        Each JSON object is used to instantiate an item of type `T_co` using the class specified by `_cls`.
+        Each JSON object is used to instantiate an item of type `T` using the class specified by `_cls`.
         The results are cached after the first request and reused for subsequent access unless reloaded.
 
         Returns
         -------
-        List[T_co]
-            A list of items of type `T_co` representing the fetched data.
+        List[T]
+            A list of items of type `T` representing the fetched data.
         """
         if self._cache:
             return self._cache
 
         response = self._ctx.session.get(self._endpoint)
         results = response.json()
-        self._cache = [self._cls(self._ctx, self._parent, **result) for result in results]
+        self._cache = [self._create_instance(**result) for result in results]
         return self._cache
 
     @overload
-    def __getitem__(self, index: int) -> T_co: ...
+    def __getitem__(self, index: int) -> T: ...
 
     @overload
-    def __getitem__(self, index: slice) -> Sequence[T_co]: ...
+    def __getitem__(self, index: slice) -> Sequence[T]: ...
 
     def __getitem__(self, index):
         return self._data[index]
@@ -210,7 +148,17 @@ def __str__(self) -> str:
     def __repr__(self) -> str:
         return repr(self._data)
 
-    def reload(self) -> "ActiveSequence":
+    @abstractmethod
+    def _create_instance(self, **kwargs) -> T:
+        """Create an instance of 'T'.
+
+        Returns
+        -------
+        T
+        """
+        raise NotImplementedError()
+
+    def reload(self) -> Self:
         """
         Clear the cache and reload the data from the API on the next access.
 
@@ -223,31 +171,25 @@ def reload(self) -> "ActiveSequence":
         return self
 
 
-class ActiveFinderMethods(ActiveSequence[T_co], ABC, Generic[T_co]):
-    """
-    Finder methods.
+class ActiveFinderMethods(ActiveSequence[T], ABC, Generic[T]):
+    def __init__(self, ctx: Context, parent: Optional[Active] = None, uid: str = "guid"):
+        """Finder methods.
 
-    Provides various finder methods for locating records in any endpoint supporting HTTP GET requests.
+        Provides various finder methods for locating records in any endpoint supporting HTTP GET requests.
 
-    Attributes
-    ----------
-    _uid : str
-        The default field name used to uniquely identify records. Defaults to 'guid'.
-
-    Methods
-    -------
-    find(uid) -> T_co
-        Finds and returns a record by its unique identifier (`uid`). If a cached result exists, it searches within the cache;
-        otherwise, it makes a GET request to retrieve the data from the endpoint.
-
-    find_by(**conditions: Any) -> Optional[T_co]
-        Finds the first record that matches the provided conditions. If no record is found, returns None.
-    """
-
-    _uid: str = "guid"
-    """The default field name used to uniquely identify records. Defaults to 'guid'."""
+        Parameters
+        ----------
+        ctx : Context
+            The context containing the HTTP session used to interact with the API.
+        parent : Optional[Active], optional
+            Optional parent resource for maintaining hierarchical relationships, by default None
+        uid : str, optional
+            The default field name used to uniquely identify records, by default "guid"
+        """
+        super().__init__(ctx, parent)
+        self._uid = uid
 
-    def find(self, uid) -> T_co:
+    def find(self, uid) -> T:
         """
         Find a record by its unique identifier.
 
@@ -260,28 +202,29 @@ def find(self, uid) -> T_co:
 
         Returns
         -------
-        T_co
+        T
 
         Raises
         ------
         ValueError
             If no record is found.
         """
+        # todo - add some more comments about this
         if self._cache:
             conditions = {self._uid: uid}
             result = self.find_by(**conditions)
         else:
             endpoint = self._endpoint + uid
             response = self._ctx.session.get(endpoint)
             result = response.json()
-            result = self._cls(self._ctx, self._parent, **result)
+            result = self._create_instance(**result)
 
         if not result:
             raise ValueError(f"Failed to find instance where {self._uid} is '{uid}'")
 
         return result
 
-    def find_by(self, **conditions: Any) -> Optional[T_co]:
+    def find_by(self, **conditions: Any) -> Optional[T]:
         """
         Find the first record matching the specified conditions.
 
@@ -293,7 +236,7 @@ def find_by(self, **conditions: Any) -> Optional[T_co]:
 
         Returns
         -------
-        Optional[T_co]
+        Optional[T]
             The first record matching the conditions, or `None` if no match is found.
         """
         return next((v for v in self._data if v.items() >= conditions.items()), None)
