From 76e8eee38668a03694d85ed061bd9d97316b2915 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 16 Dec 2024 10:34:25 -0500 Subject: [PATCH 01/12] Add all files to get help from @tdstein --- .../connect/test_content_item_repository.py | 3 +- src/posit/connect/_api.py | 278 ------------------ src/posit/connect/_api_call.py | 72 ----- src/posit/connect/content.py | 141 ++++----- src/posit/connect/resources.py | 26 +- tests/posit/connect/test_api_endpoint.py | 18 -- tests/posit/connect/test_content.py | 12 +- 7 files changed, 91 insertions(+), 459 deletions(-) delete mode 100644 src/posit/connect/_api.py delete mode 100644 src/posit/connect/_api_call.py delete mode 100644 tests/posit/connect/test_api_endpoint.py diff --git a/integration/tests/posit/connect/test_content_item_repository.py b/integration/tests/posit/connect/test_content_item_repository.py index 6260b6e3..7db79efc 100644 --- a/integration/tests/posit/connect/test_content_item_repository.py +++ b/integration/tests/posit/connect/test_content_item_repository.py @@ -3,6 +3,7 @@ from posit import connect from posit.connect.content import ContentItem, ContentItemRepository +from posit.connect.resources import _Resource from . import CONNECT_VERSION @@ -65,7 +66,7 @@ def test_create_get_update_delete(self): assert content_repo is not None def assert_repo(r: ContentItemRepository): - assert isinstance(content_repo, ContentItemRepository) + assert isinstance(content_repo, _Resource) assert r["repository"] == self.repo_repository assert r["branch"] == self.repo_branch assert r["directory"] == self.repo_directory diff --git a/src/posit/connect/_api.py b/src/posit/connect/_api.py deleted file mode 100644 index b602c7b0..00000000 --- a/src/posit/connect/_api.py +++ /dev/null @@ -1,278 +0,0 @@ -# TODO-barret-future; Piecemeal migrate everything to leverage `ApiDictEndpoint` and `ApiListEndpoint` classes. -# TODO-barret-future; Merge any trailing behavior of `Active` or `ActiveList` into the new classes. - -from __future__ import annotations - -import itertools -import posixpath -from abc import ABC, abstractmethod -from collections.abc import Mapping -from typing import TYPE_CHECKING, Any, Generator, Generic, Optional, TypeVar, cast, overload - -from ._api_call import ApiCallMixin, get_api -from ._json import Jsonifiable, JsonifiableDict, ResponseAttrs - -if TYPE_CHECKING: - from .context import Context - - -# Design Notes: -# * Perform API calls on property retrieval. e.g. `my_content.repository` -# * Dictionary endpoints: Retrieve all attributes during init unless provided -# * List endpoints: Do not retrieve until `.fetch()` is called directly. Avoids cache invalidation issues. -# * While slower, all ApiListEndpoint helper methods should `.fetch()` on demand. -# * Only expose methods needed for `ReadOnlyDict`. -# * Ex: When inheriting from `dict`, we'd need to shut down `update`, `pop`, etc. -# * Use `ApiContextProtocol` to ensure that the class has the necessary attributes for API calls. -# * Inherit from `ApiCallMixin` to add all helper methods for API calls. -# * Classes should write the `path` only once within its init method. -# * Through regular interactions, the path should only be written once. - -# When making a new class, -# * Use a class to define the parameters and their types -# * If attaching the type info class to the parent class, start with `_`. E.g.: `ContentItemRepository._Attrs` -# * Document all attributes like normal -# * When the time comes that there are multiple attribute types, we can use overloads with full documentation and unpacking of type info class for each overload method. -# * Inherit from `ApiDictEndpoint` or `ApiListEndpoint` as needed -# * Init signature should be `def __init__(self, ctx: Context, path: str, /, **attrs: Jsonifiable) -> None:` - - -# This class should not have typing about the class keys as that would fix the class's typing. If -# for some reason, we _know_ the keys are fixed (as we've moved on to a higher version), we can add -# `Generic[AttrsT]` to the class. -class ReadOnlyDict(Mapping): - _attrs: ResponseAttrs - """Resource attributes passed.""" - - def __init__(self, attrs: ResponseAttrs) -> None: - """ - A read-only dict abstraction for any HTTP endpoint that returns a singular resource. - - Parameters - ---------- - attrs : dict - Resource attributes passed - """ - super().__init__() - self._attrs = attrs - - def get(self, key: str, default: Any = None) -> Any: - return self._attrs.get(key, default) - - def __getitem__(self, key: str) -> Any: - return self._attrs[key] - - def __setitem__(self, key: str, value: Any) -> None: - raise NotImplementedError( - "Resource attributes are locked. " - "To retrieve updated values, please retrieve the parent object again." - ) - - def __len__(self) -> int: - return self._attrs.__len__() - - def __iter__(self): - return self._attrs.__iter__() - - def __contains__(self, key: object) -> bool: - return self._attrs.__contains__(key) - - def __repr__(self) -> str: - return repr(self._attrs) - - def __str__(self) -> str: - return str(self._attrs) - - def keys(self): - return self._attrs.keys() - - def values(self): - return self._attrs.values() - - def items(self): - return self._attrs.items() - - -class ApiDictEndpoint(ApiCallMixin, ReadOnlyDict): - _ctx: Context - """The context object containing the session and URL for API interactions.""" - _path: str - """The HTTP path component for the resource endpoint.""" - - def _get_api(self, *path) -> JsonifiableDict | None: - super()._get_api(*path) - - def __init__( - self, - ctx: Context, - path: str, - get_data: Optional[bool] = None, - /, - **attrs: Jsonifiable, - ) -> None: - """ - A dict abstraction for any HTTP endpoint that returns a singular resource. - - Adds helper methods to interact with the API with reduced boilerplate. - - Parameters - ---------- - ctx : Context - The context object containing the session and URL for API interactions. - path : str - The HTTP path component for the resource endpoint - get_data : Optional[bool] - If `True`, fetch the API and set the attributes from the response. If `False`, only set - the provided attributes. If `None` [default], fetch the API if no attributes are - provided. - attrs : dict - Resource attributes passed - """ - # If no attributes are provided, fetch the API and set the attributes from the response - if get_data is None: - get_data = len(attrs) == 0 - - # If we should get data, fetch the API and set the attributes from the response - if get_data: - init_attrs: Jsonifiable = get_api(ctx, path) - init_attrs_dict = cast(ResponseAttrs, init_attrs) - # Overwrite the initial attributes with `attrs`: e.g. {'key': value} | {'content_guid': '123'} - init_attrs_dict.update(attrs) - attrs = init_attrs_dict - - super().__init__(attrs) - self._ctx = ctx - self._path = path - - -T = TypeVar("T", bound="ReadOnlyDict") -"""A type variable that is bound to the `Active` class""" - - -class ApiListEndpoint(ApiCallMixin, Generic[T], ABC, object): - """A HTTP GET endpoint that can fetch a collection.""" - - def __init__(self, *, ctx: Context, path: str, uid_key: str = "guid") -> None: - """A sequence abstraction for any HTTP GET endpoint that returns a collection. - - Parameters - ---------- - ctx : Context - The context object containing the session and URL for API interactions. - path : str - The HTTP path component for the collection endpoint - uid_key : str, optional - The field name of that uniquely identifiers an instance of T, by default "guid" - """ - super().__init__() - self._ctx = ctx - self._path = path - self._uid_key = uid_key - - @abstractmethod - def _create_instance(self, path: str, /, **kwargs: Any) -> T: - """Create an instance of 'T'.""" - raise NotImplementedError() - - def fetch(self) -> Generator[T, None, None]: - """Fetch the collection. - - Fetches the collection directly from Connect. This operation does not effect the cache state. - - Returns - ------- - List[T] - """ - results: Jsonifiable = self._get_api() - results_list = cast(list[JsonifiableDict], results) - for result in results_list: - yield self._to_instance(result) - - def __iter__(self) -> Generator[T, None, None]: - return self.fetch() - - def _to_instance(self, result: dict) -> T: - """Converts a result into an instance of T.""" - uid = result[self._uid_key] - path = posixpath.join(self._path, uid) - return self._create_instance(path, **result) - - @overload - def __getitem__(self, index: int) -> T: ... - - @overload - def __getitem__(self, index: slice) -> Generator[T, None, None]: ... - - def __getitem__(self, index: int | slice) -> T | Generator[T, None, None]: - if isinstance(index, slice): - results = itertools.islice(self.fetch(), index.start, index.stop, index.step) - for result in results: - yield result - else: - return list(itertools.islice(self.fetch(), index, index + 1))[0] - - # def __len__(self) -> int: - # return len(self.fetch()) - - def __str__(self) -> str: - return self.__repr__() - - def __repr__(self) -> str: - # Jobs - 123 items - return repr( - f"{self.__class__.__name__} - { len(list(self.fetch())) } items - {self._path}" - ) - - def find(self, uid: str) -> T | None: - """ - Find a record by its unique identifier. - - Fetches the record from Connect by it's identifier. - - Parameters - ---------- - uid : str - The unique identifier of the record. - - Returns - ------- - : - Single instance of T if found, else None - """ - result: Jsonifiable = self._get_api(uid) - result_obj = cast(JsonifiableDict, result) - - return self._to_instance(result_obj) - - def find_by(self, **conditions: Any) -> T | None: - """ - Find the first record matching the specified conditions. - - There is no implied ordering, so if order matters, you should specify it yourself. - - Parameters - ---------- - **conditions : Any - - Returns - ------- - T - The first record matching the conditions, or `None` if no match is found. - """ - results = self.fetch() - - conditions_items = conditions.items() - - # Get the first item of the generator that matches the conditions - # If no item is found, return None - return next( - ( - # Return result - result - # Iterate through `results` generator - for result in results - # If all `conditions`'s key/values are found in `result`'s key/values... - if result.items() >= conditions_items - ), - None, - ) diff --git a/src/posit/connect/_api_call.py b/src/posit/connect/_api_call.py deleted file mode 100644 index f90244aa..00000000 --- a/src/posit/connect/_api_call.py +++ /dev/null @@ -1,72 +0,0 @@ -from __future__ import annotations - -import posixpath -from typing import TYPE_CHECKING, Protocol - -if TYPE_CHECKING: - from ._json import Jsonifiable - from .context import Context - - -class ApiCallProtocol(Protocol): - _ctx: Context - _path: str - - def _endpoint(self, *path) -> str: ... - def _get_api(self, *path) -> Jsonifiable: ... - def _delete_api(self, *path) -> Jsonifiable | None: ... - def _patch_api(self, *path, json: Jsonifiable | None) -> Jsonifiable: ... - def _put_api(self, *path, json: Jsonifiable | None) -> Jsonifiable: ... - - -def endpoint(ctx: Context, *path) -> str: - return ctx.url + posixpath.join(*path) - - -# Helper methods for API interactions -def get_api(ctx: Context, *path) -> Jsonifiable: - response = ctx.session.get(endpoint(ctx, *path)) - return response.json() - - -def put_api( - ctx: Context, - *path, - json: Jsonifiable | None, -) -> Jsonifiable: - response = ctx.session.put(endpoint(ctx, *path), json=json) - return response.json() - - -# Mixin class for API interactions - - -class ApiCallMixin: - def _endpoint(self: ApiCallProtocol, *path) -> str: - return endpoint(self._ctx, self._path, *path) - - def _get_api(self: ApiCallProtocol, *path) -> Jsonifiable: - response = self._ctx.session.get(self._endpoint(*path)) - return response.json() - - def _delete_api(self: ApiCallProtocol, *path) -> Jsonifiable | None: - response = self._ctx.session.delete(self._endpoint(*path)) - if len(response.content) == 0: - return None - return response.json() - - def _patch_api( - self: ApiCallProtocol, - *path, - json: Jsonifiable | None, - ) -> Jsonifiable: - response = self._ctx.session.patch(self._endpoint(*path), json=json) - return response.json() - - def _put_api( - self: ApiCallProtocol, - *path, - json: Jsonifiable | None, - ) -> Jsonifiable: - response = self._ctx.session.put(self._endpoint(*path), json=json) - return response.json() diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index 77a31320..59c0db5a 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -4,28 +4,41 @@ import posixpath import time +from abc import abstractmethod +from collections.abc import Mapping from posixpath import dirname -from typing import ( + +from typing_extensions import ( TYPE_CHECKING, Any, List, Literal, + NotRequired, Optional, - cast, + Protocol, + Required, + TypedDict, + Unpack, overload, ) -from typing_extensions import NotRequired, Required, TypedDict, Unpack - from . import tasks -from ._api import ApiDictEndpoint, JsonifiableDict from .bundles import Bundles from .context import requires from .env import EnvVars from .errors import ClientError from .oauth.associations import ContentItemAssociations from .permissions import Permissions -from .resources import Active, Resource, ResourceParameters, Resources, _ResourceSequence +from .resources import ( + Active, + Resource, + ResourceParameters, + Resources, + _Resource, + _ResourcePatch, + _ResourceSequence, + _ResourceUpdatePatchMixin, +) from .tags import ContentItemTags from .vanities import VanityMixin from .variants import Variants @@ -42,12 +55,11 @@ def _assert_guid(guid: str): assert len(guid) > 0, "Expected 'guid' to be non-empty" -def _assert_content_guid(content_guid: str): - assert isinstance(content_guid, str), "Expected 'content_guid' to be a string" - assert len(content_guid) > 0, "Expected 'content_guid' to be non-empty" +# ContentItem Repository uses a PATCH method, not a PUT for updating. +class _ContentItemRepositoryResource(_ResourceUpdatePatchMixin, _Resource): ... -class ContentItemRepository(ApiDictEndpoint): +class ContentItemRepository(Mapping[str, Any]): """ Content items GitHub repository information. @@ -58,65 +70,7 @@ class ContentItemRepository(ApiDictEndpoint): * Update info: https://docs.posit.co/connect/api/#patch-/v1/content/-guid-/repository """ - class _Attrs(TypedDict, total=False): - repository: str - """URL for the repository.""" - branch: NotRequired[str] - """The tracked Git branch.""" - directory: NotRequired[str] - """Directory containing the content.""" - polling: NotRequired[bool] - """Indicates that the Git repository is regularly polled.""" - - def __init__( - self, - ctx: Context, - /, - *, - content_guid: str, - # By default, the `attrs` will be retrieved from the API if no `attrs` are supplied. - **attrs: Unpack[ContentItemRepository._Attrs], - ) -> None: - """Content items GitHub repository information. - - Parameters - ---------- - ctx : Context - The context object containing the session and URL for API interactions. - content_guid : str - The unique identifier of the content item. - **attrs : ContentItemRepository._Attrs - Attributes for the content item repository. If not supplied, the attributes will be - retrieved from the API upon initialization - """ - _assert_content_guid(content_guid) - - path = self._api_path(content_guid) - # Only fetch data if `attrs` are not supplied - get_data = len(attrs) == 0 - super().__init__(ctx, path, get_data, **{"content_guid": content_guid, **attrs}) - - @classmethod - def _api_path(cls, content_guid: str) -> str: - return f"v1/content/{content_guid}/repository" - - @classmethod - def _create( - cls, - ctx: Context, - content_guid: str, - **attrs: Unpack[ContentItemRepository._Attrs], - ) -> ContentItemRepository: - from ._api_call import put_api - - result = put_api(ctx, cls._api_path(content_guid), json=cast(JsonifiableDict, attrs)) - - return ContentItemRepository( - ctx, - content_guid=content_guid, - **result, # pyright: ignore[reportCallIssue] - ) - + @abstractmethod def destroy(self) -> None: """ Delete the content's git repository location. @@ -125,13 +79,17 @@ def destroy(self) -> None: -------- * https://docs.posit.co/connect/api/#delete-/v1/content/-guid-/repository """ - self._delete_api() + ... + @abstractmethod def update( self, - # *, - **attrs: Unpack[ContentItemRepository._Attrs], - ) -> ContentItemRepository: + *, + repository: Optional[str] = None, + branch: str = "main", + directory: str = ".", + polling: bool = False, + ) -> None: """Update the content's repository. Parameters @@ -153,12 +111,7 @@ def update( -------- * https://docs.posit.co/connect/api/#patch-/v1/content/-guid-/repository """ - result = self._patch_api(json=cast(JsonifiableDict, dict(attrs))) - return ContentItemRepository( - self._ctx, - content_guid=self["content_guid"], - **result, # pyright: ignore[reportCallIssue] - ) + ... class ContentItemOAuth(Resource): @@ -265,14 +218,28 @@ def oauth(self) -> ContentItemOAuth: @property def repository(self) -> ContentItemRepository | None: try: - return ContentItemRepository(self._ctx, content_guid=self["guid"]) + return _Resource( + self._ctx, + f"v1/content/{self['guid']}/repository", + ) except ClientError: return None + @overload def create_repository( self, - **attrs: Unpack[ContentItemRepository._Attrs], - ) -> ContentItemRepository: + /, + *, + repository: Optional[str] = None, + branch: str = "main", + directory: str = ".", + polling: bool = False, + ) -> ContentItemRepository: ... + + @overload + def create_repository(self, /, **attributes) -> ContentItemRepository: ... + + def create_repository(self, /, **attributes) -> ContentItemRepository: """Create repository. Parameters @@ -290,7 +257,15 @@ def create_repository( ------- ContentItemRepository """ - return ContentItemRepository._create(self._ctx, self["guid"], **attrs) + path = f"v1/content/{self['guid']}/repository" + response = self._ctx.session.put(path, json=attributes) + result = response.json() + + return _ContentItemRepositoryResource( + self._ctx, + path, + **result, + ) def delete(self) -> None: """Delete the content item.""" diff --git a/src/posit/connect/resources.py b/src/posit/connect/resources.py index dc843273..a0582f5b 100644 --- a/src/posit/connect/resources.py +++ b/src/posit/connect/resources.py @@ -8,6 +8,7 @@ TYPE_CHECKING, Any, Iterable, + Protocol, Sequence, ) @@ -85,8 +86,17 @@ def __init__(self, ctx: Context, path: str, /, **attributes): self._path = path +class _ResourceP(Protocol): + _ctx: Context + _path: str + + def destroy(self) -> None: ... + + def update(self, **attributes): ... + + class _Resource(dict): - def __init__(self, ctx: Context, path: str, **attributes): + def __init__(self, ctx: Context, path: str, /, **attributes): self._ctx = ctx self._path = path super().__init__(**attributes) @@ -100,6 +110,20 @@ def update(self, **attributes): # type: ignore[reportIncompatibleMethodOverride super().update(**result) +class _ResourceUpdatePatchMixin: + def update(self: _ResourceP, **attributes): # type: ignore[reportIncompatibleMethodOverride] + response = self._ctx.client.patch(self._path, json=attributes) + result = response.json() + super().update(**result) + + +class _ResourcePatch(_Resource): + def update(self, **attributes): # type: ignore[reportIncompatibleMethodOverride] + response = self._ctx.client.patch(self._path, json=attributes) + result = response.json() + super().update(**result) + + class _ResourceSequence(Sequence): def __init__(self, ctx: Context, path: str, *, uid: str = "guid"): self._ctx = ctx diff --git a/tests/posit/connect/test_api_endpoint.py b/tests/posit/connect/test_api_endpoint.py deleted file mode 100644 index 2281084f..00000000 --- a/tests/posit/connect/test_api_endpoint.py +++ /dev/null @@ -1,18 +0,0 @@ -import pytest - -from posit.connect._api import ReadOnlyDict - - -class TestApiEndpoint: - def test_read_only(self): - obj = ReadOnlyDict({}) - - assert len(obj) == 0 - - assert obj.get("foo", "bar") == "bar" - - with pytest.raises(NotImplementedError): - obj["foo"] = "baz" - - eq_obj = ReadOnlyDict({"foo": "bar", "a": 1}) - assert eq_obj == {"foo": "bar", "a": 1} diff --git a/tests/posit/connect/test_content.py b/tests/posit/connect/test_content.py index f18b46f5..d432168d 100644 --- a/tests/posit/connect/test_content.py +++ b/tests/posit/connect/test_content.py @@ -3,9 +3,9 @@ from responses import matchers from posit.connect.client import Client -from posit.connect.content import ContentItem, ContentItemRepository +from posit.connect.content import ContentItem from posit.connect.context import Context -from posit.connect.resources import ResourceParameters +from posit.connect.resources import ResourceParameters, _Resource from .api import load_mock, load_mock_dict @@ -587,7 +587,7 @@ def mock_repository_info(self): ) repository_info = content_item.repository - assert isinstance(repository_info, ContentItemRepository) + assert isinstance(repository_info, _Resource) assert mock_get.call_count == 1 return repository_info @@ -605,14 +605,14 @@ def test_repository_update(self): self.endpoint, json=load_mock_dict(f"v1/content/{self.content_guid}/repository_patch.json"), ) - new_repository_info = repository_info.update(branch="testing-main") + repository_info.update(branch="testing-main") assert mock_patch.call_count == 1 for key, value in repository_info.items(): if key == "branch": - assert new_repository_info[key] == "testing-main" + assert repository_info[key] == "testing-main" else: - assert new_repository_info[key] == value + assert repository_info[key] == value @responses.activate def test_repository_delete(self): From 666a18614ae92c6b3fce14629d58ac7a5d251f8b Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 16 Dec 2024 16:41:37 -0500 Subject: [PATCH 02/12] Delete _json.py --- src/posit/connect/_json.py | 37 ------------------------------------- tests/posit/connect/api.py | 2 +- 2 files changed, 1 insertion(+), 38 deletions(-) delete mode 100644 src/posit/connect/_json.py diff --git a/src/posit/connect/_json.py b/src/posit/connect/_json.py deleted file mode 100644 index 62f28f82..00000000 --- a/src/posit/connect/_json.py +++ /dev/null @@ -1,37 +0,0 @@ -from typing import Dict, List, Tuple, TypeVar, Union - -# Implemented in https://github.com/posit-dev/py-shiny/blob/415ced034e6c500adda524abb7579731c32088b5/shiny/types.py#L357-L386 -# Table from: https://github.com/python/cpython/blob/df1eec3dae3b1eddff819fd70f58b03b3fbd0eda/Lib/json/encoder.py#L77-L95 -# +-------------------+---------------+ -# | Python | JSON | -# +===================+===============+ -# | dict | object | -# +-------------------+---------------+ -# | list, tuple | array | -# +-------------------+---------------+ -# | str | string | -# +-------------------+---------------+ -# | int, float | number | -# +-------------------+---------------+ -# | True | true | -# +-------------------+---------------+ -# | False | false | -# +-------------------+---------------+ -# | None | null | -# +-------------------+---------------+ -Jsonifiable = Union[ - str, - int, - float, - bool, - None, - List["Jsonifiable"], - Tuple["Jsonifiable", ...], - "JsonifiableDict", -] - -JsonifiableT = TypeVar("JsonifiableT", bound="Jsonifiable") -JsonifiableDict = Dict[str, Jsonifiable] -JsonifiableList = List[JsonifiableT] - -ResponseAttrs = Dict[str, Jsonifiable] diff --git a/tests/posit/connect/api.py b/tests/posit/connect/api.py index 06b5f6cc..1b99badb 100644 --- a/tests/posit/connect/api.py +++ b/tests/posit/connect/api.py @@ -20,7 +20,7 @@ def load_mock(path: str): Returns ------- - Jsonifiable + dict | list The parsed data from the JSONC file. Examples From 15f854ae56f1629786961e2b4017eb4b03bb2759 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 16 Dec 2024 16:43:42 -0500 Subject: [PATCH 03/12] Fix class duck typing using inspiration from #364 --- src/posit/connect/content.py | 31 ++++++++++++++++--------------- src/posit/connect/resources.py | 23 ----------------------- 2 files changed, 16 insertions(+), 38 deletions(-) diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index 7678d1b1..d45c8ee4 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -5,12 +5,11 @@ import posixpath import time from abc import abstractmethod -from collections.abc import Mapping -from posixpath import dirname from typing_extensions import ( TYPE_CHECKING, Any, + Hashable, List, Literal, NotRequired, @@ -29,14 +28,7 @@ from .errors import ClientError from .oauth.associations import ContentItemAssociations from .permissions import Permissions -from .resources import ( - Active, - Resource, - Resources, - _Resource, - _ResourceSequence, - _ResourceUpdatePatchMixin, -) +from .resources import Active, Resource, Resources, _Resource, _ResourceSequence from .tags import ContentItemTags from .vanities import VanityMixin from .variants import Variants @@ -48,16 +40,25 @@ from .tasks import Task +# TODO-barret: Replace with Resource class from https://github.com/posit-dev/posit-sdk-py/pull/364/files#diff-94b7dc3c7d7d7c7b1a5f25e06c37df5fc53e1921cb10d41d4f04b18a715fae55R72 +class ResourceP(Protocol): + def __getitem__(self, key: Hashable, /) -> Any: ... + + def _assert_guid(guid: str): assert isinstance(guid, str), "Expected 'guid' to be a string" assert len(guid) > 0, "Expected 'guid' to be non-empty" # ContentItem Repository uses a PATCH method, not a PUT for updating. -class _ContentItemRepositoryResource(_ResourceUpdatePatchMixin, _Resource): ... +class _ContentItemRepository(_Resource): + def update(self, **attributes): # type: ignore[reportIncompatibleMethodOverride] + response = self._ctx.client.patch(self._path, json=attributes) + result = response.json() + super().update(**result) -class ContentItemRepository(Mapping[str, Any]): +class ContentItemRepository(ResourceP): """ Content items GitHub repository information. @@ -216,7 +217,7 @@ def oauth(self) -> ContentItemOAuth: @property def repository(self) -> ContentItemRepository | None: try: - return _Resource( + return _ContentItemRepository( self._ctx, f"v1/content/{self['guid']}/repository", ) @@ -256,10 +257,10 @@ def create_repository(self, /, **attributes) -> ContentItemRepository: ContentItemRepository """ path = f"v1/content/{self['guid']}/repository" - response = self._ctx.session.put(path, json=attributes) + response = self._ctx.client.session.put(path, json=attributes) result = response.json() - return _ContentItemRepositoryResource( + return _ContentItemRepository( self._ctx, path, **result, diff --git a/src/posit/connect/resources.py b/src/posit/connect/resources.py index 8f09ccb8..5c8c1d69 100644 --- a/src/posit/connect/resources.py +++ b/src/posit/connect/resources.py @@ -63,15 +63,6 @@ def __init__(self, ctx: Context, path: str, /, **attributes): self._path = path -class _ResourceP(Protocol): - _ctx: Context - _path: str - - def destroy(self) -> None: ... - - def update(self, **attributes): ... - - class _Resource(dict): def __init__(self, ctx: Context, path: str, /, **attributes): self._ctx = ctx @@ -87,20 +78,6 @@ def update(self, **attributes): # type: ignore[reportIncompatibleMethodOverride super().update(**result) -class _ResourceUpdatePatchMixin: - def update(self: _ResourceP, **attributes): # type: ignore[reportIncompatibleMethodOverride] - response = self._ctx.client.patch(self._path, json=attributes) - result = response.json() - super().update(**result) - - -class _ResourcePatch(_Resource): - def update(self, **attributes): # type: ignore[reportIncompatibleMethodOverride] - response = self._ctx.client.patch(self._path, json=attributes) - result = response.json() - super().update(**result) - - class _ResourceSequence(Sequence): def __init__(self, ctx: Context, path: str, *, uid: str = "guid"): self._ctx = ctx From 96feb7a57a2800710bf85fc1952a523a1debee0d Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 16 Dec 2024 17:02:06 -0500 Subject: [PATCH 04/12] Be sure to retrieve data before init'ing --- src/posit/connect/content.py | 15 +++++++++++---- src/posit/connect/resources.py | 1 - 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index d45c8ee4..a4987f70 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -52,13 +52,16 @@ def _assert_guid(guid: str): # ContentItem Repository uses a PATCH method, not a PUT for updating. class _ContentItemRepository(_Resource): - def update(self, **attributes): # type: ignore[reportIncompatibleMethodOverride] + def update(self, **attributes): response = self._ctx.client.patch(self._path, json=attributes) result = response.json() - super().update(**result) + # # Calling this method will call `_Resource.update` which will try to PUT to the path. + # super().update(**result) + # Instead, update the dict directly. + dict.update(self, **result) -class ContentItemRepository(ResourceP): +class ContentItemRepository(ResourceP, Protocol): """ Content items GitHub repository information. @@ -217,9 +220,13 @@ def oauth(self) -> ContentItemOAuth: @property def repository(self) -> ContentItemRepository | None: try: + path = f"v1/content/{self['guid']}/repository" + response = self._ctx.client.get(path) + result = response.json() return _ContentItemRepository( self._ctx, - f"v1/content/{self['guid']}/repository", + path, + **result, ) except ClientError: return None diff --git a/src/posit/connect/resources.py b/src/posit/connect/resources.py index 5c8c1d69..d4d7cf69 100644 --- a/src/posit/connect/resources.py +++ b/src/posit/connect/resources.py @@ -7,7 +7,6 @@ TYPE_CHECKING, Any, Iterable, - Protocol, Sequence, ) From 9e971c1b310873ed76b796453e5fa9d60eb334a6 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 16 Dec 2024 17:07:33 -0500 Subject: [PATCH 05/12] Typo --- src/posit/connect/content.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index a4987f70..bc1cf822 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -264,7 +264,7 @@ def create_repository(self, /, **attributes) -> ContentItemRepository: ContentItemRepository """ path = f"v1/content/{self['guid']}/repository" - response = self._ctx.client.session.put(path, json=attributes) + response = self._ctx.client.put(path, json=attributes) result = response.json() return _ContentItemRepository( From 01cebb0c213d1f8a15d946a40a564ac1eb645867 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 16 Dec 2024 17:18:22 -0500 Subject: [PATCH 06/12] Move repository content to `._repository.py` --- src/posit/connect/_repository.py | 145 +++++++++++++++++++++++++++++++ src/posit/connect/content.py | 136 +---------------------------- 2 files changed, 148 insertions(+), 133 deletions(-) create mode 100644 src/posit/connect/_repository.py diff --git a/src/posit/connect/_repository.py b/src/posit/connect/_repository.py new file mode 100644 index 00000000..e0175ddb --- /dev/null +++ b/src/posit/connect/_repository.py @@ -0,0 +1,145 @@ +"""Content item repository.""" + +from __future__ import annotations + +from abc import abstractmethod + +from typing_extensions import ( + Any, + Hashable, + Optional, + Protocol, + overload, +) + +from .errors import ClientError +from .resources import Resource, _Resource + + +# TODO-barret: Replace with Resource class from https://github.com/posit-dev/posit-sdk-py/pull/364/files#diff-94b7dc3c7d7d7c7b1a5f25e06c37df5fc53e1921cb10d41d4f04b18a715fae55R72 +class ResourceP(Protocol): + def __getitem__(self, key: Hashable, /) -> Any: ... + + +# ContentItem Repository uses a PATCH method, not a PUT for updating. +class _ContentItemRepository(_Resource): + def update(self, **attributes): + response = self._ctx.client.patch(self._path, json=attributes) + result = response.json() + # # Calling this method will call `_Resource.update` which will try to PUT to the path. + # super().update(**result) + # Instead, update the dict directly. + dict.update(self, **result) + + +class ContentItemRepository(ResourceP, Protocol): + """ + Content items GitHub repository information. + + See Also + -------- + * Get info: https://docs.posit.co/connect/api/#get-/v1/content/-guid-/repository + * Delete info: https://docs.posit.co/connect/api/#delete-/v1/content/-guid-/repository + * Update info: https://docs.posit.co/connect/api/#patch-/v1/content/-guid-/repository + """ + + @abstractmethod + def destroy(self) -> None: + """ + Delete the content's git repository location. + + See Also + -------- + * https://docs.posit.co/connect/api/#delete-/v1/content/-guid-/repository + """ + ... + + @abstractmethod + def update( + self, + *, + repository: Optional[str] = None, + branch: str = "main", + directory: str = ".", + polling: bool = False, + ) -> None: + """Update the content's repository. + + Parameters + ---------- + repository: str, optional + URL for the repository. Default is None. + branch: str, optional + The tracked Git branch. Default is 'main'. + directory: str, optional + Directory containing the content. Default is '.' + polling: bool, optional + Indicates that the Git repository is regularly polled. Default is False. + + Returns + ------- + None + + See Also + -------- + * https://docs.posit.co/connect/api/#patch-/v1/content/-guid-/repository + """ + ... + + +class ContentItemRepositoryMixin(Resource): + @property + def repository(self) -> ContentItemRepository | None: + try: + path = f"v1/content/{self['guid']}/repository" + response = self._ctx.client.get(path) + result = response.json() + return _ContentItemRepository( + self._ctx, + path, + **result, + ) + except ClientError: + return None + + @overload + def create_repository( + self, + /, + *, + repository: Optional[str] = None, + branch: str = "main", + directory: str = ".", + polling: bool = False, + ) -> ContentItemRepository: ... + + @overload + def create_repository(self, /, **attributes) -> ContentItemRepository: ... + + def create_repository(self, /, **attributes) -> ContentItemRepository: + """Create repository. + + Parameters + ---------- + repository : str + URL for the respository. + branch : str, optional + The tracked Git branch. Default is 'main'. + directory : str, optional + Directory containing the content. Default is '.'. + polling : bool, optional + Indicates that the Git repository is regularly polled. Default is False. + + Returns + ------- + ContentItemRepository + """ + path = f"v1/content/{self['guid']}/repository" + response = self._ctx.client.put(path, json=attributes) + result = response.json() + + return _ContentItemRepository( + self._ctx, + path, + **result, + ) diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index bc1cf822..c8bf8da4 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -4,17 +4,14 @@ import posixpath import time -from abc import abstractmethod from typing_extensions import ( TYPE_CHECKING, Any, - Hashable, List, Literal, NotRequired, Optional, - Protocol, Required, TypedDict, Unpack, @@ -22,13 +19,13 @@ ) from . import tasks +from ._repository import ContentItemRepositoryMixin from .bundles import Bundles from .context import requires from .env import EnvVars -from .errors import ClientError from .oauth.associations import ContentItemAssociations from .permissions import Permissions -from .resources import Active, Resource, Resources, _Resource, _ResourceSequence +from .resources import Active, Resource, Resources, _ResourceSequence from .tags import ContentItemTags from .vanities import VanityMixin from .variants import Variants @@ -40,82 +37,11 @@ from .tasks import Task -# TODO-barret: Replace with Resource class from https://github.com/posit-dev/posit-sdk-py/pull/364/files#diff-94b7dc3c7d7d7c7b1a5f25e06c37df5fc53e1921cb10d41d4f04b18a715fae55R72 -class ResourceP(Protocol): - def __getitem__(self, key: Hashable, /) -> Any: ... - - def _assert_guid(guid: str): assert isinstance(guid, str), "Expected 'guid' to be a string" assert len(guid) > 0, "Expected 'guid' to be non-empty" -# ContentItem Repository uses a PATCH method, not a PUT for updating. -class _ContentItemRepository(_Resource): - def update(self, **attributes): - response = self._ctx.client.patch(self._path, json=attributes) - result = response.json() - # # Calling this method will call `_Resource.update` which will try to PUT to the path. - # super().update(**result) - # Instead, update the dict directly. - dict.update(self, **result) - - -class ContentItemRepository(ResourceP, Protocol): - """ - Content items GitHub repository information. - - See Also - -------- - * Get info: https://docs.posit.co/connect/api/#get-/v1/content/-guid-/repository - * Delete info: https://docs.posit.co/connect/api/#delete-/v1/content/-guid-/repository - * Update info: https://docs.posit.co/connect/api/#patch-/v1/content/-guid-/repository - """ - - @abstractmethod - def destroy(self) -> None: - """ - Delete the content's git repository location. - - See Also - -------- - * https://docs.posit.co/connect/api/#delete-/v1/content/-guid-/repository - """ - ... - - @abstractmethod - def update( - self, - *, - repository: Optional[str] = None, - branch: str = "main", - directory: str = ".", - polling: bool = False, - ) -> None: - """Update the content's repository. - - Parameters - ---------- - repository: str, optional - URL for the repository. Default is None. - branch: str, optional - The tracked Git branch. Default is 'main'. - directory: str, optional - Directory containing the content. Default is '.' - polling: bool, optional - Indicates that the Git repository is regularly polled. Default is False. - - Returns - ------- - None - - See Also - -------- - * https://docs.posit.co/connect/api/#patch-/v1/content/-guid-/repository - """ - ... - - class ContentItemOAuth(Resource): def __init__(self, ctx: Context, content_guid: str) -> None: super().__init__(ctx) @@ -130,7 +56,7 @@ class ContentItemOwner(Resource): pass -class ContentItem(Active, VanityMixin, Resource): +class ContentItem(Active, ContentItemRepositoryMixin, VanityMixin, Resource): class _AttrsBase(TypedDict, total=False): # # `name` will be set by other _Attrs classes # name: str @@ -217,62 +143,6 @@ def __getitem__(self, key: Any) -> Any: def oauth(self) -> ContentItemOAuth: return ContentItemOAuth(self._ctx, content_guid=self["guid"]) - @property - def repository(self) -> ContentItemRepository | None: - try: - path = f"v1/content/{self['guid']}/repository" - response = self._ctx.client.get(path) - result = response.json() - return _ContentItemRepository( - self._ctx, - path, - **result, - ) - except ClientError: - return None - - @overload - def create_repository( - self, - /, - *, - repository: Optional[str] = None, - branch: str = "main", - directory: str = ".", - polling: bool = False, - ) -> ContentItemRepository: ... - - @overload - def create_repository(self, /, **attributes) -> ContentItemRepository: ... - - def create_repository(self, /, **attributes) -> ContentItemRepository: - """Create repository. - - Parameters - ---------- - repository : str - URL for the respository. - branch : str, optional - The tracked Git branch. Default is 'main'. - directory : str, optional - Directory containing the content. Default is '.'. - polling : bool, optional - Indicates that the Git repository is regularly polled. Default is False. - - Returns - ------- - ContentItemRepository - """ - path = f"v1/content/{self['guid']}/repository" - response = self._ctx.client.put(path, json=attributes) - result = response.json() - - return _ContentItemRepository( - self._ctx, - path, - **result, - ) - def delete(self) -> None: """Delete the content item.""" path = f"v1/content/{self['guid']}" From d319c5bac6642e4cbc69cb26cbc729278dcbc510 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 16 Dec 2024 17:18:36 -0500 Subject: [PATCH 07/12] Update integration repository update code --- .../posit/connect/test_content_item_repository.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/integration/tests/posit/connect/test_content_item_repository.py b/integration/tests/posit/connect/test_content_item_repository.py index 7db79efc..0558328a 100644 --- a/integration/tests/posit/connect/test_content_item_repository.py +++ b/integration/tests/posit/connect/test_content_item_repository.py @@ -2,7 +2,8 @@ from packaging import version from posit import connect -from posit.connect.content import ContentItem, ContentItemRepository +from posit.connect._repository import ContentItemRepository +from posit.connect.content import ContentItem from posit.connect.resources import _Resource from . import CONNECT_VERSION @@ -77,12 +78,12 @@ def assert_repo(r: ContentItemRepository): # Update ex_branch = "main" - updated_repo = content_repo.update(branch=ex_branch) - assert updated_repo["branch"] == ex_branch + content_repo.update(branch=ex_branch) + assert content_repo["branch"] == ex_branch - assert updated_repo["repository"] == self.repo_repository - assert updated_repo["directory"] == self.repo_directory - assert updated_repo["polling"] is self.repo_polling + assert content_repo["repository"] == self.repo_repository + assert content_repo["directory"] == self.repo_directory + assert content_repo["polling"] is self.repo_polling # Delete content_repo.destroy() From 30e8e4b3bc8c4f63448d8d22e845db27cf7d52d2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 17 Dec 2024 10:31:43 -0500 Subject: [PATCH 08/12] Remove `@abstractmethod` on `Protocol` class --- src/posit/connect/_repository.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/posit/connect/_repository.py b/src/posit/connect/_repository.py index e0175ddb..b88abbb9 100644 --- a/src/posit/connect/_repository.py +++ b/src/posit/connect/_repository.py @@ -2,8 +2,6 @@ from __future__ import annotations -from abc import abstractmethod - from typing_extensions import ( Any, Hashable, @@ -43,7 +41,6 @@ class ContentItemRepository(ResourceP, Protocol): * Update info: https://docs.posit.co/connect/api/#patch-/v1/content/-guid-/repository """ - @abstractmethod def destroy(self) -> None: """ Delete the content's git repository location. @@ -54,7 +51,6 @@ def destroy(self) -> None: """ ... - @abstractmethod def update( self, *, From a3453e3bbae2e09dca3400ccc7773e455053ee54 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 17 Dec 2024 10:34:32 -0500 Subject: [PATCH 09/12] Make `ContentItemRepository` class `@runtime_checkable` --- .../tests/posit/connect/test_content_item_repository.py | 3 +-- src/posit/connect/_repository.py | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/integration/tests/posit/connect/test_content_item_repository.py b/integration/tests/posit/connect/test_content_item_repository.py index 0558328a..bb891ef9 100644 --- a/integration/tests/posit/connect/test_content_item_repository.py +++ b/integration/tests/posit/connect/test_content_item_repository.py @@ -4,7 +4,6 @@ from posit import connect from posit.connect._repository import ContentItemRepository from posit.connect.content import ContentItem -from posit.connect.resources import _Resource from . import CONNECT_VERSION @@ -67,7 +66,7 @@ def test_create_get_update_delete(self): assert content_repo is not None def assert_repo(r: ContentItemRepository): - assert isinstance(content_repo, _Resource) + assert isinstance(content_repo, ContentItemRepository) assert r["repository"] == self.repo_repository assert r["branch"] == self.repo_branch assert r["directory"] == self.repo_directory diff --git a/src/posit/connect/_repository.py b/src/posit/connect/_repository.py index b88abbb9..404bdb5b 100644 --- a/src/posit/connect/_repository.py +++ b/src/posit/connect/_repository.py @@ -8,6 +8,7 @@ Optional, Protocol, overload, + runtime_checkable, ) from .errors import ClientError @@ -30,6 +31,7 @@ def update(self, **attributes): dict.update(self, **result) +@runtime_checkable class ContentItemRepository(ResourceP, Protocol): """ Content items GitHub repository information. From 00c10d710e6157686a7333573bd82cdbb17a6b8c Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 17 Dec 2024 10:37:16 -0500 Subject: [PATCH 10/12] Rename `_repository.py` to `repository.py` --- integration/tests/posit/connect/test_content_item_repository.py | 2 +- src/posit/connect/content.py | 2 +- src/posit/connect/{_repository.py => repository.py} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/posit/connect/{_repository.py => repository.py} (100%) diff --git a/integration/tests/posit/connect/test_content_item_repository.py b/integration/tests/posit/connect/test_content_item_repository.py index bb891ef9..7911847d 100644 --- a/integration/tests/posit/connect/test_content_item_repository.py +++ b/integration/tests/posit/connect/test_content_item_repository.py @@ -2,8 +2,8 @@ from packaging import version from posit import connect -from posit.connect._repository import ContentItemRepository from posit.connect.content import ContentItem +from posit.connect.repository import ContentItemRepository from . import CONNECT_VERSION diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index c8bf8da4..881fa9ae 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -19,12 +19,12 @@ ) from . import tasks -from ._repository import ContentItemRepositoryMixin from .bundles import Bundles from .context import requires from .env import EnvVars from .oauth.associations import ContentItemAssociations from .permissions import Permissions +from .repository import ContentItemRepositoryMixin from .resources import Active, Resource, Resources, _ResourceSequence from .tags import ContentItemTags from .vanities import VanityMixin diff --git a/src/posit/connect/_repository.py b/src/posit/connect/repository.py similarity index 100% rename from src/posit/connect/_repository.py rename to src/posit/connect/repository.py From 923c0e8aff71a98ab1c05e0be585cc9d04def288 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 17 Dec 2024 11:08:10 -0500 Subject: [PATCH 11/12] Make `ContentItemRepository` inherit from resources `Resource` --- src/posit/connect/repository.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/posit/connect/repository.py b/src/posit/connect/repository.py index 404bdb5b..809b9eb5 100644 --- a/src/posit/connect/repository.py +++ b/src/posit/connect/repository.py @@ -3,8 +3,6 @@ from __future__ import annotations from typing_extensions import ( - Any, - Hashable, Optional, Protocol, overload, @@ -15,11 +13,6 @@ from .resources import Resource, _Resource -# TODO-barret: Replace with Resource class from https://github.com/posit-dev/posit-sdk-py/pull/364/files#diff-94b7dc3c7d7d7c7b1a5f25e06c37df5fc53e1921cb10d41d4f04b18a715fae55R72 -class ResourceP(Protocol): - def __getitem__(self, key: Hashable, /) -> Any: ... - - # ContentItem Repository uses a PATCH method, not a PUT for updating. class _ContentItemRepository(_Resource): def update(self, **attributes): @@ -32,7 +25,7 @@ def update(self, **attributes): @runtime_checkable -class ContentItemRepository(ResourceP, Protocol): +class ContentItemRepository(Resource, Protocol): """ Content items GitHub repository information. From e36eb94293ba617c951203aa6a22b1aa3c2b2ca2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 17 Dec 2024 11:08:35 -0500 Subject: [PATCH 12/12] Turn `ContentItemRepositoryMixin` into a true mixin class --- src/posit/connect/repository.py | 10 +++++----- src/posit/connect/resources.py | 3 +++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/posit/connect/repository.py b/src/posit/connect/repository.py index 809b9eb5..1c562393 100644 --- a/src/posit/connect/repository.py +++ b/src/posit/connect/repository.py @@ -78,9 +78,9 @@ def update( ... -class ContentItemRepositoryMixin(Resource): +class ContentItemRepositoryMixin: @property - def repository(self) -> ContentItemRepository | None: + def repository(self: Resource) -> ContentItemRepository | None: try: path = f"v1/content/{self['guid']}/repository" response = self._ctx.client.get(path) @@ -95,7 +95,7 @@ def repository(self) -> ContentItemRepository | None: @overload def create_repository( - self, + self: Resource, /, *, repository: Optional[str] = None, @@ -105,9 +105,9 @@ def create_repository( ) -> ContentItemRepository: ... @overload - def create_repository(self, /, **attributes) -> ContentItemRepository: ... + def create_repository(self: Resource, /, **attributes) -> ContentItemRepository: ... - def create_repository(self, /, **attributes) -> ContentItemRepository: + def create_repository(self: Resource, /, **attributes) -> ContentItemRepository: """Create repository. Parameters diff --git a/src/posit/connect/resources.py b/src/posit/connect/resources.py index d6650e07..1916539d 100644 --- a/src/posit/connect/resources.py +++ b/src/posit/connect/resources.py @@ -70,6 +70,9 @@ def __init__(self, ctx: Context, path: str, /, **attributes): class Resource(Protocol): + _ctx: Context + _path: str + def __getitem__(self, key: Hashable, /) -> Any: ...