From bc956ed4c1aec1da6485adf07297184b7d313b05 Mon Sep 17 00:00:00 2001 From: tdstein Date: Wed, 12 Jun 2024 15:35:40 -0400 Subject: [PATCH 1/4] feat: always resolve content owner --- integration/Makefile | 2 +- .../tests/posit/connect/test_client.py | 3 - .../tests/posit/connect/test_content.py | 28 +++++++++ src/posit/connect/content.py | 13 ++-- .../f2f37341-e21d-3d80-c698-a935ad614066.json | 8 ++- .../20a79ce3-6e87-4522-9faf-be24228800a4.json | 2 +- tests/posit/connect/test_content.py | 59 ++++++++++++++++++- 7 files changed, 102 insertions(+), 13 deletions(-) create mode 100644 integration/tests/posit/connect/test_content.py diff --git a/integration/Makefile b/integration/Makefile index 9c0eaabb..af636a25 100644 --- a/integration/Makefile +++ b/integration/Makefile @@ -138,4 +138,4 @@ help: # on local network. test: mkdir -p logs - CONNECT_VERSION=${CONNECT_VERSION} CONNECT_API_KEY="$(shell rsconnect bootstrap -i -s http://connect:3939 --raw)" $(PYTHON) -m pytest --junit-xml=./reports/$(CONNECT_VERSION).xml > ./logs/$(CONNECT_VERSION).log + CONNECT_VERSION=${CONNECT_VERSION} CONNECT_API_KEY="$(shell rsconnect bootstrap -i -s http://connect:3939 --raw)" $(PYTHON) -m pytest -s --junit-xml=./reports/$(CONNECT_VERSION).xml | tee ./logs/$(CONNECT_VERSION).log diff --git a/integration/tests/posit/connect/test_client.py b/integration/tests/posit/connect/test_client.py index 1352007f..e7ac3647 100644 --- a/integration/tests/posit/connect/test_client.py +++ b/integration/tests/posit/connect/test_client.py @@ -1,6 +1,3 @@ -import os -import pytest - from posit import connect diff --git a/integration/tests/posit/connect/test_content.py b/integration/tests/posit/connect/test_content.py new file mode 100644 index 00000000..53422742 --- /dev/null +++ b/integration/tests/posit/connect/test_content.py @@ -0,0 +1,28 @@ +from posit import connect + + +class TestContent: + def setup_class(cls): + cls.client = connect.Client() + cls.item = cls.client.content.create( + name="Sample", + description="Simple sample content for testing", + access_type="acl", + ) + + def test_count(self): + assert self.client.content.count() == 1 + + def test_get(self): + assert self.client.content.get(self.item.guid) == self.item + + def test_find(self): + assert self.client.content.find() + + def test_find_one(self): + assert self.client.content.find_one() + + def test_content_item_owner(self): + item = self.client.content.find_one(include=None) + owner = item.owner + assert owner.guid == self.client.me.guid diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index 188c6a4c..4461a508 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -13,6 +13,7 @@ from .bundles import Bundles from .permissions import Permissions from .resources import Resources, Resource +from .users import Users class ContentItemOwner(Resource): @@ -146,6 +147,14 @@ def bundles(self) -> Bundles: def permissions(self) -> Permissions: return Permissions(self.config, self.session, self.guid) + @property + def owner(self) -> ContentItemOwner: + if "owner" in self: + return self["owner"] + user = Users(self.config, self.session).get(self.owner_guid) + owner = ContentItemOwner(self.config, self.session, **user) + return owner + # Properties @property @@ -308,10 +317,6 @@ def run_as_current_user(self) -> bool: def owner_guid(self) -> str: return self.get("owner_guid") # type: ignore - @property - def owner(self) -> ContentItemOwner: - return self.get("owner", {}) # type: ignore - @property def content_url(self) -> str: return self.get("content_url") # type: ignore diff --git a/tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json b/tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json index 07158938..f8bc5b26 100644 --- a/tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json +++ b/tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json @@ -37,7 +37,13 @@ "default_py_environment_management": null, "run_as": null, "run_as_current_user": false, - "owner_guid": "87c12c08-11cd-4de1-8da3-12a7579c4998", + "owner_guid": "20a79ce3-6e87-4522-9faf-be24228800a4", + "owner": { + "username": "carlos12", + "first_name": "Carlos", + "last_name": "User", + "guid": "20a79ce3-6e87-4522-9faf-be24228800a4" + }, "content_url": "https://connect.example/content/f2f37341-e21d-3d80-c698-a935ad614066/", "dashboard_url": "https://connect.example/connect/#/apps/f2f37341-e21d-3d80-c698-a935ad614066", "app_role": "viewer", diff --git a/tests/posit/connect/__api__/v1/users/20a79ce3-6e87-4522-9faf-be24228800a4.json b/tests/posit/connect/__api__/v1/users/20a79ce3-6e87-4522-9faf-be24228800a4.json index 5561d5ae..8a6f6db9 100644 --- a/tests/posit/connect/__api__/v1/users/20a79ce3-6e87-4522-9faf-be24228800a4.json +++ b/tests/posit/connect/__api__/v1/users/20a79ce3-6e87-4522-9faf-be24228800a4.json @@ -10,4 +10,4 @@ "confirmed": true, "locked": false, "guid": "20a79ce3-6e87-4522-9faf-be24228800a4" -} \ No newline at end of file +} diff --git a/tests/posit/connect/test_content.py b/tests/posit/connect/test_content.py index 21c2a5b0..0a753592 100644 --- a/tests/posit/connect/test_content.py +++ b/tests/posit/connect/test_content.py @@ -5,12 +5,34 @@ from posit.connect.client import Client from posit.connect.config import Config -from posit.connect.content import ContentItem +from posit.connect.content import ContentItem, ContentItemOwner from posit.connect.permissions import Permissions from .api import load_mock # type: ignore +class TestContentOwnerAttributes: + @classmethod + def setup_class(cls): + guid = "20a79ce3-6e87-4522-9faf-be24228800a4" + config = Config(api_key="12345", url="https://connect.example/") + session = requests.Session() + fake_item = load_mock(f"v1/users/{guid}.json") + cls.item = ContentItemOwner(config, session, **fake_item) + + def test_guid(self): + assert self.item.guid == "20a79ce3-6e87-4522-9faf-be24228800a4" + + def test_username(self): + assert self.item.username == "carlos12" + + def test_first_name(self): + assert self.item.first_name == "Carlos" + + def test_last_name(self): + assert self.item.last_name == "User" + + class TestContentItemAttributes: @classmethod def setup_class(cls): @@ -138,7 +160,7 @@ def test_run_as_current_user(self): assert self.item.run_as_current_user is False def test_owner_guid(self): - assert self.item.owner_guid == "87c12c08-11cd-4de1-8da3-12a7579c4998" + assert self.item.owner_guid == "20a79ce3-6e87-4522-9faf-be24228800a4" def test_content_url(self): assert ( @@ -156,7 +178,12 @@ def test_app_role(self): assert self.item.app_role == "viewer" def test_owner(self): - assert self.item.owner == {} + assert self.item.owner == { + "username": "carlos12", + "first_name": "Carlos", + "last_name": "User", + "guid": "20a79ce3-6e87-4522-9faf-be24228800a4", + } def test_permissions(self): assert isinstance(self.item.permissions, Permissions) @@ -165,6 +192,32 @@ def test_tags(self): assert self.item.tags == [] +class TestContentItemGetContentOwner: + @responses.activate + def test_owner(self): + mock_content = load_mock( + "v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json" + ) + # remove owner from response to force fetch on attribute + del mock_content["owner"] + responses.get( + "https://connect.example/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066", + json=mock_content, + ) + + responses.get( + f"https://connect.example/__api__/v1/users/20a79ce3-6e87-4522-9faf-be24228800a4", + json=load_mock( + f"v1/users/20a79ce3-6e87-4522-9faf-be24228800a4.json" + ), + ) + + c = Client("12345", "https://connect.example") + item = c.content.get("f2f37341-e21d-3d80-c698-a935ad614066") + owner = item.owner + assert owner.guid == "20a79ce3-6e87-4522-9faf-be24228800a4" + + class TestContentItemDelete: @responses.activate def test(self): From f20aefee6817632de2cb3f91e508b86cb7c75344 Mon Sep 17 00:00:00 2001 From: Taylor Steinberg Date: Thu, 13 Jun 2024 09:10:58 -0400 Subject: [PATCH 2/4] Update src/posit/connect/content.py Co-authored-by: Neal Richardson --- src/posit/connect/content.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index 4461a508..58edec42 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -149,11 +149,12 @@ def permissions(self) -> Permissions: @property def owner(self) -> ContentItemOwner: - if "owner" in self: - return self["owner"] - user = Users(self.config, self.session).get(self.owner_guid) - owner = ContentItemOwner(self.config, self.session, **user) - return owner + if "owner" not in self: + # It is possible to get a content item that does not contain owner. + # "owner" is an optional additional request param. + # If it's not included, we can retrieve the information by `owner_guid` + self["owner"] = Users(self.config, self.session).get(self.owner_guid) + return ContentItemOwner(self.config, self.session, **self["owner"]) # Properties From 78ccaf6ae7b080cda1f2cebb2223a3ab5348ddeb Mon Sep 17 00:00:00 2001 From: tdstein Date: Thu, 13 Jun 2024 09:22:33 -0400 Subject: [PATCH 3/4] revert mocked data --- src/posit/connect/content.py | 4 +++- .../v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json | 6 ------ tests/posit/connect/test_content.py | 9 +-------- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index 58edec42..50c3963b 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -153,7 +153,9 @@ def owner(self) -> ContentItemOwner: # It is possible to get a content item that does not contain owner. # "owner" is an optional additional request param. # If it's not included, we can retrieve the information by `owner_guid` - self["owner"] = Users(self.config, self.session).get(self.owner_guid) + self["owner"] = Users(self.config, self.session).get( + self.owner_guid + ) return ContentItemOwner(self.config, self.session, **self["owner"]) # Properties diff --git a/tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json b/tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json index f8bc5b26..48f4b8b5 100644 --- a/tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json +++ b/tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json @@ -38,12 +38,6 @@ "run_as": null, "run_as_current_user": false, "owner_guid": "20a79ce3-6e87-4522-9faf-be24228800a4", - "owner": { - "username": "carlos12", - "first_name": "Carlos", - "last_name": "User", - "guid": "20a79ce3-6e87-4522-9faf-be24228800a4" - }, "content_url": "https://connect.example/content/f2f37341-e21d-3d80-c698-a935ad614066/", "dashboard_url": "https://connect.example/connect/#/apps/f2f37341-e21d-3d80-c698-a935ad614066", "app_role": "viewer", diff --git a/tests/posit/connect/test_content.py b/tests/posit/connect/test_content.py index 0a753592..a85e9ba5 100644 --- a/tests/posit/connect/test_content.py +++ b/tests/posit/connect/test_content.py @@ -178,12 +178,7 @@ def test_app_role(self): assert self.item.app_role == "viewer" def test_owner(self): - assert self.item.owner == { - "username": "carlos12", - "first_name": "Carlos", - "last_name": "User", - "guid": "20a79ce3-6e87-4522-9faf-be24228800a4", - } + assert "owner" not in self.item def test_permissions(self): assert isinstance(self.item.permissions, Permissions) @@ -198,8 +193,6 @@ def test_owner(self): mock_content = load_mock( "v1/content/f2f37341-e21d-3d80-c698-a935ad614066.json" ) - # remove owner from response to force fetch on attribute - del mock_content["owner"] responses.get( "https://connect.example/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066", json=mock_content, From b6b358f8502bb70fa0c2483a155e5cf373c94b10 Mon Sep 17 00:00:00 2001 From: tdstein Date: Thu, 13 Jun 2024 09:56:05 -0400 Subject: [PATCH 4/4] assert user endpoint called once --- integration/tests/posit/connect/test_content.py | 5 +++++ tests/posit/connect/test_content.py | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/integration/tests/posit/connect/test_content.py b/integration/tests/posit/connect/test_content.py index 53422742..d7fa2fbd 100644 --- a/integration/tests/posit/connect/test_content.py +++ b/integration/tests/posit/connect/test_content.py @@ -26,3 +26,8 @@ def test_content_item_owner(self): item = self.client.content.find_one(include=None) owner = item.owner assert owner.guid == self.client.me.guid + + def test_content_item_owner_from_include(self): + item = self.client.content.find_one(include="owner") + owner = item.owner + assert owner.guid == self.client.me.guid diff --git a/tests/posit/connect/test_content.py b/tests/posit/connect/test_content.py index a85e9ba5..4980029f 100644 --- a/tests/posit/connect/test_content.py +++ b/tests/posit/connect/test_content.py @@ -198,7 +198,7 @@ def test_owner(self): json=mock_content, ) - responses.get( + mock_user_get = responses.get( f"https://connect.example/__api__/v1/users/20a79ce3-6e87-4522-9faf-be24228800a4", json=load_mock( f"v1/users/20a79ce3-6e87-4522-9faf-be24228800a4.json" @@ -210,6 +210,11 @@ def test_owner(self): owner = item.owner assert owner.guid == "20a79ce3-6e87-4522-9faf-be24228800a4" + # load a second time, assert tha owner is loaded from cached result + owner = item.owner + assert owner.guid == "20a79ce3-6e87-4522-9faf-be24228800a4" + assert mock_user_get.call_count == 1 + class TestContentItemDelete: @responses.activate