diff --git a/README.md b/README.md index ba43b7f..e055a44 100644 --- a/README.md +++ b/README.md @@ -4,11 +4,13 @@ Code Coverage

-This package implements a fluent interface to the Discourse API. +This package implements a [fluent interface](https://en.wikipedia.org/wiki/Fluent_interface) to the Discourse API. + +What does that mean? Instead of mapping every endpoint and method to a unique function, we present a framework for making any request. -This means, with very little code, this package is fully compatible with the Discourse API. This includes undocumented endpoints as well as endpoints that have yet to be created. +This means, with very little code, **this package is fully compatible with the Discourse API, including undocumented endpoints, endpoints from plugins, and endpoints that have yet to be created.** ## Installation The easiest way to install is via PyPI @@ -16,26 +18,43 @@ The easiest way to install is via PyPI pip install fluent-discourse ``` - ## Usage +### Setting up a client Set up a client by specifying a base_url, username, and api_key for it to use. ```python from fluent_discourse import Discourse -client = Discourse(base_url="http://localhost:3000", username="test_user", api_key="a0a2d176b3cfbadd36ac2f46ccbd701bf45dfd6f47836e99d570bf7e0ae04af8") +client = Discourse(base_url="http://localhost:3000", username="test_user", api_key="a0a2d176b3cfbadd36ac2f46ccbd701bf45dfd6f47836e99d570bf7e0ae04af8", raise_for_rate_limit=True) +``` + +Or, you can set three environment variables: +```language +export DISCOURSE_URL=http://localhost:3000 +export DISCOURSE_USERNAME=test_user +export DISCOURSE_API_KEY=a0a2d176b3cfbadd36ac2f46ccbd701bf45dfd6f47836e99d570bf7e0ae04af8 ``` +Then you can use the `from_env()` class method to instantiate the client: +```python +from fluent_discourse import Discourse +client = Discourse.from_env(raise_for_rate_limit=False) +``` + +In either case, the `raise_for_rate_limit` parameter is optional (defaults to True) and controls how the client will respond to RateLimitErrors. If `True`, the client will raise a `RateLimitError`; if `False`, it will wait the suggested time for the RateLimit counter to reset and retry the request. -Once your client is initialized, you can can begin making requests. Let's take an example to see how this works. +Once your client is initialized, you can can begin making requests. Let's first take an example to see how this works. +### Basic Example Let's say we want to get the latest posts, [here's the appropriate endpoint](https://docs.discourse.org/#tag/Posts/paths/~1posts.json/get). We need to make a `GET` request to `/posts.json`. Here's how you do it with the client we've set up. ```python latest = client.posts.json.get() ``` -I hope that gives you an idea of how this works. Instead of calling a specific function that is mapped to this endpoint/method combination, we construct the request dynamically using a form of method chaining. +I hope that gives you an idea of how this works. Instead of calling a specific function that is mapped to this endpoint/method combination, we construct the request dynamically using a form of method chaining. Finally, we use the special methods `get()`, `put()`, `post()`, or `delete()` to trigger a request to the specified endpoint. + +### Passing IDs and Python Reserved Words Let's look at another example. This time we want to add users to a group, [here's the endpoint we want to hit](https://docs.discourse.org/#tag/Groups/paths/~1groups~1{id}~1members.json/put). Specifically, we want to add users to the group with `id=5`, so we need to send a `PUT` request to `/groups/5/members.json`. Here's how to do that with this package: @@ -43,26 +62,69 @@ Here's how to do that with this package: data = { "usernames": "username1,username2" } -client.groups[5].members.json.put(data=data) +client.groups[5].members.json.put(data) ``` +Notice that we use a slightly different syntax ("indexed at" brackets) to pass in numbers. This is because of the naming constraints of python attributes. We also run into problems with reserved words like `for` and `is`. + +If you ever need to construct a URL that contains numbers or reserved words, there are two methods. + +```python +# These throw Syntax errors +## Numbers are not allowed +client.groups.5.members.json.put(data) +## "is" and "for" are reserved words +client.is.this.for.me.get() + +# Valid approaches +## Using brackets +client.groups[5].members.json.put(data) +## Using the _() method +client._("is").this._("for").me.get() +``` + +As you can see, you can either use brackets `[]` or the underscore method `_()` to handle integers or reserved words. + +### Passing data +The `get()`, `put()`, `post()`, and `delete()` methods each take a single optional argument, `data`, which is a dictionary of data to pass along with the request. -A few things to note here: -* We can inject numbers into the endpoint using the "index at" bracket syntax `...groups[5]...`. -* Data for the request is passed as a dictionary to the `data` parameter. +For the `put()`, `post()`, and `delete()` methods the data is sent in the body of the request (as JSON). +For the `get()` method the data is added to the url as query parameters. + +### Exceptions +There are a few custom exceptions defined in this class. + +`DiscourseError` +* A catch all, and parent class for errors resulting from this package. Raised when Discourse responds with an error that doesn't fall into the other, more specific, categories (e.g. a 500 error). + +`UnauthorizedError` +* Raised when Discourse responds with a 403 error, and indicates that invalid credentials were used to set up the client. + +`RateLimitError` +* Triggered when Discourse responds with a 429 response and the client is configured with `raise_for_rate_limit=True`. + +`PageNotFoundError` +* Raised when Discourse responds with a 404, indicating either that the page does not exist or the current user does not have access to that page. + +You can import any of these errors directly from the package. +```python +from fluent_discourse import DiscourseError +``` ## Contributing Thanks for your interest in contributing to this project! For bug tracking and other issues please use the issue tracker on GitHub. ### Testing -Tests are run through tox. They are split into unit and integration tests. The integration tests require a live discourse. As such all tests should strive to be idempotent; however, despite striving to be idempotent it's still recommended to set up a local install of Discourse to run the tests against. All that's needed is a single admin user and an api key. +This package strives for 100% test coverage. Tests are run through tox. They are split into unit and integration tests. Unit tests are self contained, integration tests send requests to a server. + +Although all integration tests have been tested against a live Discourse server, we set up a mock-server for CI testing. Three environment variables are required to set up the test client: * `DISCOURSE_URL`: The base url of the discourse (e.g. `http://localhost:4200`) * `DISCOURSE_USERNAME`: The username of the user to interact as, importantly the tests require that this user has admin privileges. * `DISCOURSE_API_KEY`: An API key configured to work with the specified user. This key should have global scopes. -To run **all** tests: +To run **all** tests (against a live discourse): ```bash tox ``` @@ -72,11 +134,18 @@ To run just **unit** tests: tox -- -m "not integration" ``` -To run just **integration** tests: +To run just **integration** tests (against a live discourse): ```bash tox -- -m "integration" ``` +To set up the mock server and run the tests against that, set your `DISCOURSE_URL` env variable to `http://127.0.0.1:5000` and run: +```bash +./run_tests_w_mock_server.sh +``` + +100% test coverage is important. If you make changes, please ensure those changes are reflected in the tests as well. Particularly for integration tests, run them both against a live discourse server and the mock server. Please extend and adjust the mock server as necessary to reproduce the test results on a live server accurately. + ### Style Linting Please use [black](https://github.com/psf/black) to reformat any code changes before committing those changes. @@ -85,7 +154,6 @@ Just run: black . ``` - ## Acknowledgements I stole the idea for a fluent API interface (and some code as a starting point) from SendGrid's Python API. [Here's a resouce that explains their approach](https://sendgrid.com/blog/using-python-to-implement-a-fluent-interface-to-any-rest-api/). diff --git a/setup.cfg b/setup.cfg index 93c2b98..509960f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,15 +1,14 @@ [metadata] name = fluent-discourse -version = 0.0.1 +version = 1.0.0 author = Grayden Shand author_email = graydenshand@gmail.com description = A fluent interface to the Discourse API long_description = file: README.md long_description_content_type = text/markdown -url = https://github.com/graydenshand/fluid_discourse +url = https://github.com/graydenshand/fluent_discourse project_urls = - Bug Tracker = https://github.com/graydenshand/fluid_discourse/issues - Discussions = https://github.com/graydenshand/fluid_discourse/discussions + Bug Tracker = https://github.com/graydenshand/fluent_discourse/issues classifiers = Programming Language :: Python :: 3 License :: OSI Approved :: MIT License diff --git a/src/fluent_discourse/__init__.py b/src/fluent_discourse/__init__.py index 5f4721b..9523717 100644 --- a/src/fluent_discourse/__init__.py +++ b/src/fluent_discourse/__init__.py @@ -1 +1,2 @@ from .discourse import Discourse +from .errors import * diff --git a/src/fluent_discourse/discourse.py b/src/fluent_discourse/discourse.py index 0f54470..818f77f 100644 --- a/src/fluent_discourse/discourse.py +++ b/src/fluent_discourse/discourse.py @@ -1,9 +1,19 @@ import requests from json.decoder import JSONDecodeError +from .errors import * +import time +import logging +import os +from copy import deepcopy + +logger = logging.getLogger(__name__) +logger.addHandler(logging.NullHandler()) class Discourse: - def __init__(self, base_url, username, api_key, cache=None): + def __init__( + self, base_url, username, api_key, cache=None, raise_for_rate_limit=True + ): if base_url[-1] == "/": # Remove trailing slash from base_url base_url = base_url[:-1] @@ -11,28 +21,37 @@ def __init__(self, base_url, username, api_key, cache=None): self._username = username self._api_key = api_key self._cache = cache or [] + self._raise_for_rate_limit = raise_for_rate_limit self._headers = { "Content-Type": "application/json", "Api-Username": self._username, "Api-Key": self._api_key, } + @staticmethod + def from_env(raise_for_rate_limit=True): + base_url = os.environ.get("DISCOURSE_URL") + username = os.environ.get("DISCOURSE_USERNAME") + api_key = os.environ.get("DISCOURSE_API_KEY") + return Discourse( + base_url, username, api_key, raise_for_rate_limit=raise_for_rate_limit + ) + def _(self, name): # Add name to cache, return self - self._cache += [str(name)] - return self - - def request(self, method, data=None, params=None): - # Make a request - url = self._make_url() + return Discourse( + self._base_url, + self._username, + self._api_key, + self._cache + [str(name)], + self._raise_for_rate_limit, + ) + def _request(self, method, url, data=None, params=None): r = requests.request( method, url, json=data, params=params, headers=self._headers ) - # Clear cache - self._cache = [] - if r.status_code == 200: try: return r.json() @@ -40,23 +59,61 @@ def request(self, method, data=None, params=None): # Request succeeded but response body was not valid JSON return r.text else: - r.raise_for_status() + return self._handle_error(r, method, url, data, params) + + def _handle_error(self, response, method, url, data, params): + self._cache = [] + if response.status_code == 404: + raise PageNotFoundError( + f"The requested page was not found, or you do not have permission to access it: {response.url}" + ) + elif response.status_code == 403: + raise UnauthorizedError("Invalid credentials") + elif response.status_code == 429: + if self._raise_for_rate_limit: + raise RateLimitError("Rate limit hit") + else: + self._wait_for_rate_limit(response, method, url, data, params) + return self._request(method, url, data, params) + else: + raise DiscourseError( + f"Unhandled discourse exception: {response.status_code} - {response.text}" + ) + + def _wait_for_rate_limit(self, response, method, url, data, params): + # get the number of seconds to wait before retrying, add 1 for 0 errors + wait_seconds = int(response.json()["extras"]["wait_seconds"]) + 1 + # add piece to rate limit and then try again + logger.warning( + f"Discourse rate limit hit, trying again in {wait_seconds} seconds" + ) + # sleep for wait_seconds + time.sleep(wait_seconds) + return def get(self, data=None): # Make a get request - return self.request("GET", params=data) + url = self._make_url() + self._cache = [] + return self._request("GET", url, params=data) def post(self, data=None): # Make a post request - return self.request("POST", data=data) + url = self._make_url() + self._cache = [] + return self._request("POST", url, data=data) def put(self, data=None): # Make a put request - return self.request("PUT", data=data) + url = self._make_url() + self._cache = [] + return self._request("PUT", url, data=data) def delete(self, data=None): # Make a delete request - return self.request("DELETE", data=data) + url = self._make_url() + self._cache = [] + return self._request("DELETE", url, data=data) def _make_url(self): # Build the request url from cache segments diff --git a/src/fluent_discourse/errors.py b/src/fluent_discourse/errors.py new file mode 100644 index 0000000..cd07f0c --- /dev/null +++ b/src/fluent_discourse/errors.py @@ -0,0 +1,14 @@ +class DiscourseError(Exception): + pass + + +class RateLimitError(DiscourseError): + pass + + +class UnauthorizedError(DiscourseError): + pass + + +class PageNotFoundError(DiscourseError): + pass diff --git a/tests/0_unit_test.py b/tests/0_unit_test.py new file mode 100644 index 0000000..980486d --- /dev/null +++ b/tests/0_unit_test.py @@ -0,0 +1,74 @@ +from conf_tests import BASE_URL, client, USERNAME, API_KEY +from fluent_discourse import * +import pytest +import json + + +def test_accumulate_strings(client): + endpoint = client.test.a.path + assert endpoint._cache == ["test", "a", "path"] + + +def test_accumulate_integers(client): + endpoint = client.test.a.path[5] + assert endpoint._cache == ["test", "a", "path", "5"] + + +def test_make_url(client): + client._cache = ["this", "is", "a", "test", ".json"] + assert client._make_url() == f"{BASE_URL}/this/is/a/test.json" + + +def test_format_base_url(): + # add trailing slash to BASE URL + url = BASE_URL + "/" + client = Discourse(url, USERNAME, API_KEY) + assert client._base_url == BASE_URL + + +def test_from_env(): + client = Discourse.from_env() + + +def test_set_raise_for_rate_limit(): + client = Discourse.from_env(raise_for_rate_limit=False) + assert client._raise_for_rate_limit == False + + +class MockResponse: + status_code = 404 + url = BASE_URL + text = '{"test":"Hello World", "extras": {"wait_seconds": "3"}}' + + @classmethod + def json(cls): + return json.loads(cls.text) + + +def test_page_not_found_error(client): + with pytest.raises(PageNotFoundError): + client._handle_error(MockResponse, "GET", None, None, None) + + +def test_unauthorized_error(client): + MockResponse.status_code = 403 + with pytest.raises(UnauthorizedError): + client._handle_error(MockResponse, "GET", None, None, None) + + +def test_rate_limit_error(client): + MockResponse.status_code = 429 + with pytest.raises(RateLimitError): + client._handle_error(MockResponse, "GET", None, None, None) + + +def test_unhandled_error(client): + MockResponse.status_code = 500 + with pytest.raises(DiscourseError): + client._handle_error(MockResponse, "GET", None, None, None) + + +def test_wait_for_rate_limit(): + MockResponse.status_code = 429 + client = Discourse.from_env(raise_for_rate_limit=False) + client._wait_for_rate_limit(MockResponse, "GET", None, None, None) diff --git a/tests/integration_test.py b/tests/1_integration_test.py similarity index 65% rename from tests/integration_test.py rename to tests/1_integration_test.py index 3d0b19e..a1feaab 100644 --- a/tests/integration_test.py +++ b/tests/1_integration_test.py @@ -1,7 +1,7 @@ from conf_tests import BASE_URL, client import pytest import os -from requests.exceptions import HTTPError +from fluent_discourse import * @pytest.mark.integration @@ -50,6 +50,28 @@ def test_delete_group(client): @pytest.mark.integration -def test_bad_request(client): - with pytest.raises(HTTPError): +def test_page_not_found_error(client): + with pytest.raises(PageNotFoundError): response = client.nonexistant.endpoint.get() + + +@pytest.mark.integration +def test_unauthorized_error(): + with pytest.raises(UnauthorizedError): + client = Discourse(BASE_URL, "graydenshand", "bad_api_key") + client.posts.json.get() + + +@pytest.mark.integration +def test_rate_limit_error(client): + with pytest.raises(RateLimitError): + for i in range(70): + client.categories.json.get() + + +@pytest.mark.integration +def test_wait_for_rate_limit(): + # Should hit rate limit, wait and ultimately retrigger the connection + client = Discourse.from_env(raise_for_rate_limit=False) + categories = client.categories.json.get() + assert "category_list" in categories.keys() diff --git a/tests/mock_server/discourse.py b/tests/mock_server/discourse.py index 7754f67..ad2d0f8 100644 --- a/tests/mock_server/discourse.py +++ b/tests/mock_server/discourse.py @@ -2,6 +2,9 @@ discourse = Blueprint("discourse", __name__) +global N_REQUESTS_FOR_RATE_LIMIT +N_REQUESTS_FOR_RATE_LIMIT = 2 + @discourse.route("/") def healthcheck(): @@ -405,3 +408,181 @@ def create_a_group(): def delete_a_group(group_id): response = {"success": "OK"} return jsonify(response), 200 + + +@discourse.route("/categories.json", methods=["GET"]) +def raise_rate_limit_error(): + global N_REQUESTS_FOR_RATE_LIMIT + N_REQUESTS_FOR_RATE_LIMIT -= 1 + if N_REQUESTS_FOR_RATE_LIMIT >= 0: + response = {"extras": {"wait_seconds": "3"}} + return jsonify(response), 429 + else: + response = { + "category_list": { + "can_create_category": True, + "can_create_topic": True, + "categories": [ + { + "id": 3, + "name": "Staff", + "color": "E45735", + "text_color": "FFFFFF", + "slug": "staff", + "topic_count": 4, + "post_count": 7, + "position": 2, + "description": "

Private category for staff discussions. Topics are only visible to admins and moderators.

", + "description_text": "Private category for staff discussions. Topics are only visible to admins and moderators.", + "description_excerpt": "Private category for staff discussions. Topics are only visible to admins and moderators.", + "topic_url": "/t/about-the-staff-category/2", + "read_restricted": True, + "permission": 1, + "notification_level": 1, + "can_edit": True, + "topic_template": None, + "has_children": False, + "sort_order": None, + "sort_ascending": None, + "show_subcategory_list": False, + "num_featured_topics": 3, + "default_view": None, + "subcategory_list_style": "rows_with_featured_topics", + "default_top_period": "all", + "default_list_filter": "all", + "minimum_required_tags": 0, + "navigate_to_first_post_after_read": False, + "topics_day": 0, + "topics_week": 4, + "topics_month": 4, + "topics_year": 4, + "topics_all_time": 4, + "subcategory_ids": [], + "uploaded_logo": None, + "uploaded_background": None, + }, + { + "id": 4, + "name": "Lounge", + "color": "A461EF", + "text_color": "652D90", + "slug": "lounge", + "topic_count": 1, + "post_count": 1, + "position": 3, + "description": "

A category exclusive to members with trust level 3 and higher.

", + "description_text": "A category exclusive to members with trust level 3 and higher.", + "description_excerpt": "A category exclusive to members with trust level 3 and higher.", + "topic_url": "/t/about-the-lounge-category/3", + "read_restricted": True, + "permission": 1, + "notification_level": 1, + "can_edit": True, + "topic_template": None, + "has_children": False, + "sort_order": None, + "sort_ascending": None, + "show_subcategory_list": False, + "num_featured_topics": 3, + "default_view": None, + "subcategory_list_style": "rows_with_featured_topics", + "default_top_period": "all", + "default_list_filter": "all", + "minimum_required_tags": 0, + "navigate_to_first_post_after_read": False, + "topics_day": 0, + "topics_week": 1, + "topics_month": 1, + "topics_year": 1, + "topics_all_time": 1, + "subcategory_ids": [], + "uploaded_logo": None, + "uploaded_background": None, + }, + { + "id": 1, + "name": "Uncategorized", + "color": "0088CC", + "text_color": "FFFFFF", + "slug": "uncategorized", + "topic_count": 1, + "post_count": 1, + "position": 0, + "description": "Topics that don't need a category, or don't fit into any other existing category.", + "description_text": "Topics that don't need a category, or don't fit into any other existing category.", + "description_excerpt": "Topics that don't need a category, or don't fit into any other existing category.", + "topic_url": None, + "read_restricted": False, + "permission": 1, + "notification_level": 1, + "can_edit": True, + "topic_template": None, + "has_children": False, + "sort_order": None, + "sort_ascending": None, + "show_subcategory_list": False, + "num_featured_topics": 3, + "default_view": None, + "subcategory_list_style": "rows_with_featured_topics", + "default_top_period": "all", + "default_list_filter": "all", + "minimum_required_tags": 0, + "navigate_to_first_post_after_read": False, + "topics_day": 0, + "topics_week": 1, + "topics_month": 1, + "topics_year": 1, + "topics_all_time": 1, + "is_uncategorized": True, + "subcategory_ids": [], + "uploaded_logo": None, + "uploaded_background": None, + }, + { + "id": 2, + "name": "Site Feedback", + "color": "808281", + "text_color": "FFFFFF", + "slug": "site-feedback", + "topic_count": 0, + "post_count": 0, + "position": 1, + "description": "

Discussion about this site, its organization, how it works, and how we can improve it.

", + "description_text": "Discussion about this site, its organization, how it works, and how we can improve it.", + "description_excerpt": "Discussion about this site, its organization, how it works, and how we can improve it.", + "topic_url": "/t/about-the-site-feedback-category/1", + "read_restricted": False, + "permission": 1, + "notification_level": 1, + "can_edit": True, + "topic_template": None, + "has_children": False, + "sort_order": None, + "sort_ascending": None, + "show_subcategory_list": False, + "num_featured_topics": 3, + "default_view": None, + "subcategory_list_style": "rows_with_featured_topics", + "default_top_period": "all", + "default_list_filter": "all", + "minimum_required_tags": 0, + "navigate_to_first_post_after_read": False, + "topics_day": 0, + "topics_week": 0, + "topics_month": 0, + "topics_year": 0, + "topics_all_time": 0, + "subcategory_ids": [], + "uploaded_logo": None, + "uploaded_background": None, + }, + ], + } + } + return jsonify(response), 200 + + +@discourse.route("/posts.json", methods=["GET"]) +def raise_unauthorized_error(): + response = {"error": "Invalid Credentials"} + return jsonify(response), 403 diff --git a/tests/unit_test.py b/tests/unit_test.py deleted file mode 100644 index a43e1ca..0000000 --- a/tests/unit_test.py +++ /dev/null @@ -1,24 +0,0 @@ -from conf_tests import BASE_URL, client, USERNAME, API_KEY -from fluent_discourse import Discourse - - -def test_accumulate_strings(client): - endpoint = client.test.a.path - assert endpoint._cache == ["test", "a", "path"] - - -def test_accumulate_integers(client): - endpoint = client.test.a.path[5] - assert endpoint._cache == ["test", "a", "path", "5"] - - -def test_make_url(client): - client._cache = ["this", "is", "a", "test", ".json"] - assert client._make_url() == f"{BASE_URL}/this/is/a/test.json" - - -def test_format_base_url(): - # add trailing slash to BASE URL - url = BASE_URL + "/" - client = Discourse(url, USERNAME, API_KEY) - assert client._base_url == BASE_URL