Skip to content

Commit

Permalink
spruce up to meet ruff standards
Browse files Browse the repository at this point in the history
  • Loading branch information
briehl committed Jun 3, 2024
1 parent e14a71e commit d41b26a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 22 deletions.
45 changes: 29 additions & 16 deletions lib/NarrativeService/sharing/sharemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,36 @@
from installed_clients.baseclient import ServerError
from NarrativeService import feeds

# from storage.mongo import (
# save_share_request,
# find_existing_share_request
# )
SERVICE_TOKEN_KEY = "service-token"
WS_TOKEN_KEY = "ws-admin-token"
SERVICE_TOKEN_KEY = "service-token" # noqa: S105
WS_TOKEN_KEY = "ws-admin-token" # noqa: S105


class ShareRequester:
def __init__(self, params, config):
def __init__(self, params: dict[str, str], config: dict[str, str | int]) -> None:
"""This class handles requesting that a Narrative is shared with another
user.
params is a dict with keys:
* ws_id - the (int) workspace to share with
* share_level - the requested sharing level
* user - the user to be shared with (not necessarily the user requesting the share)
config is a dict with expected keys
* SERVICE_TOKEN_KEY
* WS_TOKEN_KEY
* workspace-url
"""
self.validate_request_params(params)
self.ws_id = params["ws_id"]
self.user = params["user"]
self.share_level = params["share_level"]
self.config = config

def request_share(self):
def request_share(self) -> dict[str, str | int]:
"""
params is a dict with keys:
ws_id - the (int) workspace to share with
share_level - the requested sharing level
user - the user to be shared with (not necessarily the user requesting the share)
This requests that a narrative is shared with a user, and returns a small dictionary
with sharing request results. The request is made via the Feeds service, which notifies
the owner(s) of the requested narrative that someone else wants to view it.
"""
service_token = self.config.get(SERVICE_TOKEN_KEY)
if service_token is None:
Expand Down Expand Up @@ -65,16 +73,21 @@ def request_share(self):
"level": self.share_level
},
"level": "request",
"users": [{"id": u, "type": "user"} for u in requestees + [self.user]]
"users": [{"id": u, "type": "user"} for u in [*requestees, self.user]]
}

feeds.make_notification(note, self.config["feeds-url"], service_token)

# Store that we made the request, uh, somewhere
# save_share_request(self.ws_id, self.user, self.level, note_id)
return ret_value

def validate_request_params(self, params):
def validate_request_params(self, params: dict[str, str]) -> None:
"""Checks the given set of parameters for missing values or, in the case of share_level,
incorrect values.
Expected keys should be ws_id, share_level, and user. share_level should be "a", "n", or "r".
If value is missing, or if share_level is incorrect, this raises a ValueError.
"""
reqd = ["ws_id", "share_level", "user"]
for r in reqd:
if r not in params or params[r] is None:
Expand Down
24 changes: 18 additions & 6 deletions test/ShareManager_test.py → test/test_sharemanager.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
from unittest import mock
from unittest.mock import MagicMock

import pytest
from installed_clients.baseclient import ServerError
from NarrativeService.sharing.sharemanager import ShareRequester

NARRATIVE_TYPE = "KBaseNarrative.Narrative-4.0"
FAKE_ADMINS = ["some_user"]
def test_valid_params(config):


def test_valid_params(config: dict[str, str]):
params = {
"user": "foo",
"ws_id": 123,
Expand All @@ -16,17 +19,21 @@ def test_valid_params(config):
assert isinstance(requester, ShareRequester)
assert requester.ws_id == params["ws_id"]


invalid_combos = [
({"user": "foo", "share_level": "a"}, "ws_id"),
({"ws_id": 123, "share_level": "a"}, "user"),
({"user": "foo", "ws_id": 123}, "share_level"),
]


@pytest.mark.parametrize("params,missing", invalid_combos)
def test_invalid_params(params, missing, config):
def test_invalid_params(params: dict[str, str | int], missing: str, config: dict[str, str]):
with pytest.raises(ValueError, match=f'Missing required parameter "{missing}"'):
ShareRequester(params, config)

def test_invalid_share_level(config):

def test_invalid_share_level(config: dict[str, str]):
params = {
"user": "foo",
"share_level": "lol",
Expand All @@ -35,22 +42,26 @@ def test_invalid_share_level(config):
with pytest.raises(ValueError, match=f"Invalid share level: {params['share_level']}. Should be one of a, n, r."):
ShareRequester(params, config)


@mock.patch("NarrativeService.sharing.sharemanager.feeds")
@mock.patch("NarrativeService.sharing.sharemanager.ws.get_ws_admins", return_value=FAKE_ADMINS)
def test_make_notification_ok(mock_ws, mock_post, config):
def test_make_notification_ok(mock_ws, mock_post, config: dict[str, str]): # noqa: ARG001
config["service-token"] = "fake-service-token"
config["ws-admin-token"] = "fake-admin-token"
req = ShareRequester({"user": "kbasetest", "ws_id": 123, "share_level": "r"}, config)
res = req.request_share()
assert "ok" in res
assert res["ok"] == 1


token_allowance = [
("service-token", "missing permission to find Narrative owners."),
("ws-admin-token", "missing authorization to make request.")
]


@pytest.mark.parametrize("token_name,expected_error", token_allowance)
def test_make_notification_token_fail(token_name, expected_error, config):
def test_make_notification_token_fail(token_name: str, expected_error: str, config: dict[str, str]):
config[token_name] = "fake-token"
req = ShareRequester({"user": "kbasetest", "ws_id": 123, "share_level": "r"}, config)
res = req.request_share()
Expand All @@ -59,8 +70,9 @@ def test_make_notification_token_fail(token_name, expected_error, config):
assert "error" in res
assert res["error"] == f"Unable to request share - NarrativeService is {expected_error}"


@mock.patch("NarrativeService.sharing.sharemanager.ws.get_ws_admins")
def test_make_notification_fail(mock_ws, config):
def test_make_notification_fail(mock_ws: MagicMock, config: dict[str, str]):
mock_ws.side_effect = ServerError("error", 500, "not working")
config["service-token"] = "fake-token"
config["ws-admin-token"] = "fake-ws-token"
Expand Down

0 comments on commit d41b26a

Please sign in to comment.