From 0c9f37262acfe7b58778b337764e7dabdd165ebf Mon Sep 17 00:00:00 2001
From: tdstein <taylor.steinberg@posit.co>
Date: Tue, 12 Nov 2024 12:59:35 -0500
Subject: [PATCH] apply pr feedback

---
 src/posit/connect/packages.py  |  4 +-
 src/posit/connect/resources.py | 70 ++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/src/posit/connect/packages.py b/src/posit/connect/packages.py
index 863b866e..7e0d9279 100644
--- a/src/posit/connect/packages.py
+++ b/src/posit/connect/packages.py
@@ -128,7 +128,7 @@ class _Package(TypedDict):
         """The numerical identifier of the application this package is associated with"""
 
         app_guid: Required[str]
-        """The numerical identifier of the application this package is associated with"""
+        """The unique identifier of the application this package is associated with"""
 
     def __init__(self, ctx, /, **attributes: Unpack[_Package]):
         # todo - passing "" is a hack since path isn't needed. Instead, this class should inherit from Resource, but ActiveSequence is designed to operate on Active. That should change.
@@ -192,7 +192,7 @@ class _FindBy(TypedDict, total=False):
         """The numerical identifier of the application this package is associated with"""
 
         app_guid: NotRequired[str]
-        """The numerical identifier of the application this package is associated with"""
+        """The unique identifier of the application this package is associated with"""
 
     @overload
     def find_by(self, **conditions: Unpack[_FindBy]) -> "Package | None":
diff --git a/src/posit/connect/resources.py b/src/posit/connect/resources.py
index cf774a19..17c9894c 100644
--- a/src/posit/connect/resources.py
+++ b/src/posit/connect/resources.py
@@ -4,7 +4,6 @@
 import warnings
 from abc import ABC, abstractmethod
 from dataclasses import dataclass
-from itertools import islice
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -17,6 +16,8 @@
     overload,
 )
 
+from typing_extensions import Self
+
 if TYPE_CHECKING:
     import requests
 
@@ -112,6 +113,7 @@ def __init__(self, ctx: Context, path: str, uid: str = "guid"):
         self._ctx = ctx
         self._path = path
         self._uid = uid
+        self._cache: Optional[List[T]] = None
 
     @abstractmethod
     def _create_instance(self, path: str, /, **kwargs: Any) -> T:
@@ -132,57 +134,61 @@ def fetch(self, **conditions) -> Iterable[T]:
         results = response.json()
         return [self._to_instance(result) for result in results]
 
+    def reload(self) -> Self:
+        """Reloads the collection from Connect.
+
+        Returns
+        -------
+        Self
+        """
+        self._cache = None
+        return self
+
     def _to_instance(self, result: dict) -> T:
         """Converts a result into an instance of T."""
         uid = result[self._uid]
         path = posixpath.join(self._path, uid)
         return self._create_instance(path, **result)
 
+    @property
+    def _data(self) -> List[T]:
+        """Get the collection.
+
+        Fetches the collection from Connect and caches the result. Subsequent invocations return the cached results unless the cache is explicitly reset.
+
+        Returns
+        -------
+        List[T]
+
+        See Also
+        --------
+        cached
+        reload
+        """
+        if self._cache is None:
+            self._cache = list(self.fetch())
+        return self._cache
+
     @overload
     def __getitem__(self, index: int) -> T: ...
 
     @overload
     def __getitem__(self, index: slice) -> Sequence[T]: ...
 
-    def __getitem__(self, index) -> Sequence[T] | T:
-        data = self.fetch()
-
-        if isinstance(index, int):
-            if index < 0:
-                # Handle negative indexing
-                data = list(data)
-                return data[index]
-            for i, value in enumerate(data):
-                if i == index:
-                    return value
-            raise KeyError(f"Index {index} is out of range.")
-
-        if isinstance(index, slice):
-            # Handle slicing with islice
-            start = index.start or 0
-            stop = index.stop
-            step = index.step or 1
-            if step == 0:
-                raise ValueError("slice step cannot be zero")
-            return [
-                value
-                for i, value in enumerate(islice(data, start, stop))
-                if (i + start) % step == 0
-            ]
-
-        raise TypeError(f"Index must be int or slice, not {type(index).__name__}.")
+    def __getitem__(self, index):
+        return self._data[index]
 
     def __len__(self) -> int:
-        return len(list(self.fetch()))
+        return len(self._data)
 
     def __iter__(self):
-        return iter(self.fetch())
+        return iter(self._data)
 
     def __str__(self) -> str:
-        return str(list(self.fetch()))
+        return str(self._data)
 
     def __repr__(self) -> str:
-        return repr(list(self.fetch()))
+        return repr(self._data)
 
 
 class ActiveFinderMethods(ActiveSequence[T]):