From 61f6eea6db8ee9ba7c6fcc22e59641d2001e7ecc Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sun, 19 Sep 2021 19:46:45 +0300 Subject: [PATCH 1/8] Reword the initial services section --- README.md | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 49cb2fb..ae2a040 100644 --- a/README.md +++ b/README.md @@ -363,18 +363,29 @@ A few things to note here: ## Services -A service is a simple function that: +Services are where business logic lives. -* Lives in `your_app/services.py` module -* Takes keyword-only arguments -* Is type-annotated (even if you are not using [`mypy`](https://github.com/python/mypy) at the moment) -* Works mostly with models & other services and selectors -* Does business logic - from simple model creation to complex cross-cutting concerns, to calling external services & tasks. +The service layer speaks the specific domain language of the software, can access the database & other resources & can interact with other parts of your system. + +A service can be: + +- A simple function. +- A class. +- An entire module. +- Whatever makes sense in your case. + +In most cases, a service can be simple function that: + +- Lives in `your_app/services.py` module. +- Takes keyword-only arguments, unless it requires no or one argument. +- Is type-annotated (even if you are not using [`mypy`](https://github.com/python/mypy) at the moment). +- Interacts with the database, other resources & other parts of your system. +- Does business logic - from simple model creation to complex cross-cutting concerns, to calling external services & tasks. An example service that creates a user: ```python -def create_user( +def user_create( *, email: str, name: str @@ -383,13 +394,15 @@ def create_user( user.full_clean() user.save() - create_profile(user=user, name=name) - send_confirmation_email(user=user) + profile_create(user=user, name=name) + confirmation_email_send(user=user) return user ``` -As you can see, this service calls 2 other services - `create_profile` and `send_confirmation_email` +As you can see, this service calls 2 other services - `profile_create` and `confirmation_email_send`. + +In this example, everything related to the user creation is in one place and can be traced. ### Naming convention From c7ba1be8bba90753117c54bc45d6485e754abe73 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sun, 19 Sep 2021 19:50:18 +0300 Subject: [PATCH 2/8] Reword the naming convention section --- README.md | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index ae2a040..c570a6e 100644 --- a/README.md +++ b/README.md @@ -408,30 +408,12 @@ In this example, everything related to the user creation is in one place and can Naming conventions depend on your taste. It pays off to have a consistent naming convention throughout a project. -If we take the example above, our service is named `create_user`. The pattern is - `_`. +If we take the example above, our service is named `user_create`. The pattern is - `_`. -What we usually prefer in our projects, again, depending on taste, is `_` or with the example above: `user_create`. This seems odd at first, but it has few nice features: +This is what we prefer in HackSoft's projects. This seems odd at first, but it has few nice features: -* Namespacing. It's easy to spot all services starting with `user_` and it's a good idea to put them in a `users.py` module. -* Greppability. Or in other words, if you want to see all actions for a specific entity, just grep for `user_`. - -A full example would look like this: - -```python -def user_create( - *, - email: str, - name: str -) -> User: - user = User(email=email) - user.full_clean() - user.save() - - profile_create(user=user, name=name) - confirmation_email_send(user=user) - - return user -``` +* **Namespacing.** It's easy to spot all services starting with `user_` and it's a good idea to put them in a `users.py` module. +* **Greppability.** Or in other words, if you want to see all actions for a specific entity, just grep for `user_`. ## Selectors From 199a5d698d4788af927ad8ef037ee92150468ecd Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sun, 19 Sep 2021 20:03:47 +0300 Subject: [PATCH 3/8] Move `selectors` as a sub-section to `services` --- README.md | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index c570a6e..2714915 100644 --- a/README.md +++ b/README.md @@ -22,10 +22,9 @@ Django styleguide that we use in [HackSoft](https://hacksoft.io). * [Testing](#testing) - [Services](#services) * [Naming convention](#naming-convention) -- [Selectors](#selectors) - * [Naming convention](#naming-convention-1) + * [Selectors](#selectors) - [APIs & Serializers](#apis--serializers) - * [Naming convention](#naming-convention-2) + * [Naming convention](#naming-convention-1) * [An example list API](#an-example-list-api) + [Plain](#plain) + [Filters + Pagination](#filters--pagination) @@ -376,7 +375,7 @@ A service can be: In most cases, a service can be simple function that: -- Lives in `your_app/services.py` module. +- Lives in `/services.py` module. - Takes keyword-only arguments, unless it requires no or one argument. - Is type-annotated (even if you are not using [`mypy`](https://github.com/python/mypy) at the moment). - Interacts with the database, other resources & other parts of your system. @@ -406,7 +405,7 @@ In this example, everything related to the user creation is in one place and can ### Naming convention -Naming conventions depend on your taste. It pays off to have a consistent naming convention throughout a project. +Naming convention depends on your taste. It pays off to have something consistent throughout a project. If we take the example above, our service is named `user_create`. The pattern is - `_`. @@ -415,33 +414,32 @@ This is what we prefer in HackSoft's projects. This seems odd at first, but it h * **Namespacing.** It's easy to spot all services starting with `user_` and it's a good idea to put them in a `users.py` module. * **Greppability.** Or in other words, if you want to see all actions for a specific entity, just grep for `user_`. -## Selectors +### Selectors -A selector is a simple function that: +In most of our projects, we distinguish between "Pushing data to the database" and "Pulling data from the database": -* Lives in `your_app/selectors.py` module -* Takes keyword-only arguments -* Is type-annotated (even if you are not using [`mypy`](https://github.com/python/mypy) at the moment) -* Works mostly with models & other services and selectors -* Does business logic around fetching data from your database +1. Services take care of the push. +1. **Selectors take care of the pull.** +1. Selectors can be viewed as a "sub-layer" to services, that's specialized in fetching data. -An example selector that lists users from the database: +> If this idea does not resonate well with you, you can just have services for both "kinds" of operations. + +A selector follows the same rules as a service. + +For example, in a module `/selectors.py`, we can have the following: ```python -def get_users(*, fetched_by: User) -> Iterable[User]: - user_ids = get_visible_users_for(user=fetched_by) +def users_list(*, fetched_by: User) -> Iterable[User]: + user_ids = users_get_visible_for(user=fetched_by) query = Q(id__in=user_ids) return User.objects.filter(query) ``` -As you can see, `get_visible_users_for` is another selector. +As you can see, `users_get_visible_for` is another selector. - -### Naming convention - -Read the section in services. The same rules apply here. +You can return querysets, or lists or whatever makes sense to your specific case. ## APIs & Serializers From 96db6f42f2eec9c70ee8e66eb842e8f1a794bc44 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sun, 19 Sep 2021 20:19:16 +0300 Subject: [PATCH 4/8] Add new section - `Modules` --- README.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/README.md b/README.md index 2714915..2d5a9a7 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ Django styleguide that we use in [HackSoft](https://hacksoft.io). * [Testing](#testing) - [Services](#services) * [Naming convention](#naming-convention) + * [Modules](#modules) * [Selectors](#selectors) - [APIs & Serializers](#apis--serializers) * [Naming convention](#naming-convention-1) @@ -414,6 +415,29 @@ This is what we prefer in HackSoft's projects. This seems odd at first, but it h * **Namespacing.** It's easy to spot all services starting with `user_` and it's a good idea to put them in a `users.py` module. * **Greppability.** Or in other words, if you want to see all actions for a specific entity, just grep for `user_`. +### Modules + +If you have a simple-enough Django app with a bunch of services, they can all live happily in the `service.py` module. + +But when things get big, you might want to split `services.py` into a folder with sub-modules, depending on the different sub-domains that you are dealing with in your app. + +For example, lets say we have an `authentication` app, where we have 1 sub-module in our `services` module, that deals with `jwt`, and one sub-module that deals with `oauth`. + +The structure may look like this: + +``` +services +├── __init__.py +├── jwt.py +└── oauth.py +``` + +There are lots of flavors here: + +- You can do the import-export dance in `services/__init__.py`, so you can import from `project.authentication.services` everywhere else +- You can create a folder-module, `jwt/__init__.py`, and put the code there. +- Basically, the structure is up to you. If you feel it's time to restructure and refactor - do so. + ### Selectors In most of our projects, we distinguish between "Pushing data to the database" and "Pulling data from the database": From 10eaba3319288e5e272afbfece8e84f6f509877d Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sat, 25 Sep 2021 19:10:54 +0300 Subject: [PATCH 5/8] Add a diagram for the service layer --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 2d5a9a7..899926c 100644 --- a/README.md +++ b/README.md @@ -367,6 +367,10 @@ Services are where business logic lives. The service layer speaks the specific domain language of the software, can access the database & other resources & can interact with other parts of your system. +Here's a very simple diagram, positioning the service layer in our Django apps: + +![Service layer](https://user-images.githubusercontent.com/387867/134778130-be168592-b953-4b74-8588-a3dbaa0b6871.png) + A service can be: - A simple function. From ce058477104bcd954399b88ba434f40ebe6581b5 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sat, 25 Sep 2021 19:24:54 +0300 Subject: [PATCH 6/8] Add section about testing in `Services` - Remove examples for testing services / selectors from the `Testing` section --- README.md | 395 +++++++++++++++--------------------------------------- 1 file changed, 108 insertions(+), 287 deletions(-) diff --git a/README.md b/README.md index 899926c..b5e66a7 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ Django styleguide that we use in [HackSoft](https://hacksoft.io). * [Naming convention](#naming-convention) * [Modules](#modules) * [Selectors](#selectors) + * [Testing](#testing-1) - [APIs & Serializers](#apis--serializers) * [Naming convention](#naming-convention-1) * [An example list API](#an-example-list-api) @@ -38,14 +39,8 @@ Django styleguide that we use in [HackSoft](https://hacksoft.io). * [Raising Exceptions in Services / Selectors](#raising-exceptions-in-services--selectors) * [Handle Exceptions in APIs](#handle-exceptions-in-apis) * [Error formatting](#error-formatting) -- [Testing](#testing-1) +- [Testing](#testing-2) * [Naming conventions](#naming-conventions) - * [Example](#example) - + [Example models](#example-models) - + [Example selectors](#example-selectors) - + [Example services](#example-services) - * [Testing services](#testing-services) - * [Testing selectors](#testing-selectors) - [Celery](#celery) * [Structure](#structure) + [Configuration](#configuration) @@ -469,6 +464,112 @@ As you can see, `users_get_visible_for` is another selector. You can return querysets, or lists or whatever makes sense to your specific case. +### Testing + +Since services hold our business logic, they are an ideal candidate for tests. + +If you decide to cover the service layer with tests, we have few general rules of thumb to follow: + +1. The tests should cover the business logic behind the services in an exhaustive manner. +1. The tests should hit the database - creating & reading from it. +1. The tests should mock async task calls & everything that goes outside the project. + +When creating the required state for a given test, one can use a combination of: + +* Fakes (We recommend using [`faker`](https://github.com/joke2k/faker)) +* Other services, to create the required objects. +* Special test utility & helper methods. +* Factories (We recommend using [`factory_boy`](https://factoryboy.readthedocs.io/en/latest/orms.html)) +* Plain `Model.objects.create()` calls, if factories are not yet introduced in the project. +* Usually, whatever suits you better. + +**Let's take a look at our service from the example:** + +```python +from django.contrib.auth.models import User +from django.core.exceptions import ValidationError + +from project.payments.selectors import items_get_for_user +from project.payments.models import Item, Payment +from project.payments.tasks import payment_charge + + +def item_buy( + *, + item: Item, + user: User, +) -> Payment: + if item in items_get_for_user(user=user): + raise ValidationError(f'Item {item} already in {user} items.') + + payment = Payment.objects.create( + item=item, + user=user, + successful=False + ) + + payment_charge.delay(payment_id=payment.id) + + return payment +``` + +The service: + +* Calls a selector for validation. +* Creates an object. +* Delays a task. + +**Those are our tests:** + +```python +from unittest.mock import patch + +from django.test import TestCase +from django.contrib.auth.models import User +from django.core.exceptions import ValidationError + +from django_styleguide.payments.services import item_buy +from django_styleguide.payments.models import Payment, Item + + +class ItemBuyTests(TestCase): + def setUp(self): + self.user = User.objects.create_user(username='Test User') + self.item = Item.objects.create( + name='Test Item', + description='Test Item description', + price=10.15 + ) + + @patch('project.payments.services.items_get_for_user') + def test_buying_item_that_is_already_bought_fails(self, items_get_for_user_mock): + """ + Since we already have tests for `items_get_for_user`, + we can safely mock it here and give it a proper return value. + """ + items_get_for_user_mock.return_value = [self.item] + + with self.assertRaises(ValidationError): + item_buy(user=self.user, item=self.item) + + @patch('project.payments.services.payment_charge.delay') + def test_buying_item_creates_a_payment_and_calls_charge_task( + self, + payment_charge_mock + ): + self.assertEqual(0, Payment.objects.count()) + + payment = item_buy(user=self.user, item=self.item) + + self.assertEqual(1, Payment.objects.count()) + self.assertEqual(payment, Payment.objects.first()) + + self.assertFalse(payment.successful) + + payment_charge_mock.assert_called() +``` + + ## APIs & Serializers When using services & selectors, all of your APIs should look simple & identical. @@ -1101,286 +1202,6 @@ If we are to split the `utils.py` module into submodules, the same will happen f We try to match the structure of our modules with the structure of their respective tests. -### Example - -We have a demo `django_styleguide` project. - -#### Example models - -```python -import uuid - -from django.db import models -from django.contrib.auth.models import User -from django.utils import timezone - -from djmoney.models.fields import MoneyField - - -class Item(models.Model): - id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) - - name = models.CharField(max_length=255) - description = models.TextField() - - price = MoneyField( - max_digits=14, - decimal_places=2, - default_currency='EUR' - ) - - def __str__(self): - return f'Item {self.id} / {self.name} / {self.price}' - - -class Payment(models.Model): - item = models.ForeignKey( - Item, - on_delete=models.CASCADE, - related_name='payments' - ) - - user = models.ForeignKey( - User, - on_delete=models.CASCADE, - related_name='payments' - ) - - successful = models.BooleanField(default=False) - - created_at = models.DateTimeField(default=timezone.now) - - def __str__(self): - return f'Payment for {self.item} / {self.user}' -``` - -#### Example selectors - -For implementation of `QuerySetType`, check `queryset_type.py`. - -```python -from django.contrib.auth.models import User - -from django_styleguide.common.types import QuerySetType - -from django_styleguide.payments.models import Item - - -def get_items_for_user( - *, - user: User -) -> QuerySetType[Item]: - return Item.objects.filter(payments__user=user) -``` - -#### Example services - -```python -from django.contrib.auth.models import User -from django.core.exceptions import ValidationError - -from django_styleguide.payments.selectors import get_items_for_user -from django_styleguide.payments.models import Item, Payment -from django_styleguide.payments.tasks import charge_payment - - -def buy_item( - *, - item: Item, - user: User, -) -> Payment: - if item in get_items_for_user(user=user): - raise ValidationError(f'Item {item} already in {user} items.') - - payment = Payment.objects.create( - item=item, - user=user, - successful=False - ) - - charge_payment.delay(payment_id=payment.id) - - return payment -``` - -### Testing services - -Service tests are the most important tests in the project. Usually, those are the heavier tests with most lines of code. - -General rule of thumb for service tests: - -* The tests should cover the business logic behind the services in an exhaustive manner. -* The tests should hit the database - creating & reading from it. -* The tests should mock async task calls & everything that goes outside the project. - -When creating the required state for a given test, one can use a combination of: - -* Fakes (We recommend using [`faker`](https://github.com/joke2k/faker)) -* Other services, to create the required objects. -* Special test utility & helper methods. -* Factories (We recommend using [`factory_boy`](https://factoryboy.readthedocs.io/en/latest/orms.html)) -* Plain `Model.objects.create()` calls, if factories are not yet introduced in the project. - -**Let's take a look at our service from the example:** - -```python -from django.contrib.auth.models import User -from django.core.exceptions import ValidationError - -from django_styleguide.payments.selectors import get_items_for_user -from django_styleguide.payments.models import Item, Payment -from django_styleguide.payments.tasks import charge_payment - - -def buy_item( - *, - item: Item, - user: User, -) -> Payment: - if item in get_items_for_user(user=user): - raise ValidationError(f'Item {item} already in {user} items.') - - payment = Payment.objects.create( - item=item, - user=user, - successful=False - ) - - charge_payment.delay(payment_id=payment.id) - - return payment - -``` - -The service: - -* Calls a selector for validation -* Creates ORM object -* Calls a task - -**Those are our tests:** - -```python -from unittest.mock import patch - -from django.test import TestCase -from django.contrib.auth.models import User -from django.core.exceptions import ValidationError - -from django_styleguide.payments.services import buy_item -from django_styleguide.payments.models import Payment, Item - - -class BuyItemTests(TestCase): - def setUp(self): - self.user = User.objects.create_user(username='Test User') - self.item = Item.objects.create( - name='Test Item', - description='Test Item description', - price=10.15 - ) - - self.service = buy_item - - @patch('django_styleguide.payments.services.get_items_for_user') - def test_buying_item_that_is_already_bought_fails(self, get_items_for_user_mock): - """ - Since we already have tests for `get_items_for_user`, - we can safely mock it here and give it a proper return value. - """ - get_items_for_user_mock.return_value = [self.item] - - with self.assertRaises(ValidationError): - self.service(user=self.user, item=self.item) - - @patch('django_styleguide.payments.services.charge_payment.delay') - def test_buying_item_creates_a_payment_and_calls_charge_task( - self, - charge_payment_mock - ): - self.assertEqual(0, Payment.objects.count()) - - payment = self.service(user=self.user, item=self.item) - - self.assertEqual(1, Payment.objects.count()) - self.assertEqual(payment, Payment.objects.first()) - - self.assertFalse(payment.successful) - - charge_payment_mock.assert_called() -``` - -### Testing selectors - -Testing selectors is also an important part of every project. - -Sometimes, the selectors can be really straightforward, and if we have to "cut corners", we can omit those tests. But it the end, it's important to cover our selectors too. - -Let's take another look at our example selector: - -```python -from django.contrib.auth.models import User - -from django_styleguide.common.types import QuerySetType - -from django_styleguide.payments.models import Item - - -def get_items_for_user( - *, - user: User -) -> QuerySetType[Item]: - return Item.objects.filter(payments__user=user) -``` - -As you can see, this is a very straightforward & simple selector. We can easily cover that with 2 to 3 tests. - -**Here are the tests:** - -```python -from django.test import TestCase -from django.contrib.auth.models import User - -from django_styleguide.payments.selectors import get_items_for_user -from django_styleguide.payments.models import Item, Payment - - -class GetItemsForUserTests(TestCase): - def test_selector_returns_nothing_for_user_without_items(self): - """ - This is a "corner case" test. - We should get nothing if the user has no items. - """ - user = User.objects.create_user(username='Test User') - - expected = [] - result = list(get_items_for_user(user=user)) - - self.assertEqual(expected, result) - - def test_selector_returns_item_for_user_with_that_item(self): - """ - This test will fail in case we change the model structure. - """ - user = User.objects.create_user(username='Test User') - - item = Item.objects.create( - name='Test Item', - description='Test Item description', - price=10.15 - ) - - Payment.objects.create( - item=item, - user=user - ) - - expected = [item] - result = list(get_items_for_user(user=user)) - - self.assertEqual(expected, result) -``` - ## Celery We use [Celery](http://www.celeryproject.org/) for the following general cases: From 7886bfa4327770c7ffcb3975267fa5f5d019f9be Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sat, 25 Sep 2021 19:26:23 +0300 Subject: [PATCH 7/8] Highlight important rules of thumb --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b5e66a7..e9b7303 100644 --- a/README.md +++ b/README.md @@ -470,9 +470,9 @@ Since services hold our business logic, they are an ideal candidate for tests. If you decide to cover the service layer with tests, we have few general rules of thumb to follow: -1. The tests should cover the business logic behind the services in an exhaustive manner. -1. The tests should hit the database - creating & reading from it. -1. The tests should mock async task calls & everything that goes outside the project. +1. The tests **should cover the business logic** in an exhaustive manner. +1. The tests **should hit the database** - creating & reading from it. +1. The tests **should mock async task calls & everything that goes outside the project.** When creating the required state for a given test, one can use a combination of: From ad51865d2591c0a3ec9341f25c5b354b8ff89964 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sat, 25 Sep 2021 19:27:15 +0300 Subject: [PATCH 8/8] Fix name of selectors --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e9b7303..b629e71 100644 --- a/README.md +++ b/README.md @@ -452,15 +452,15 @@ A selector follows the same rules as a service. For example, in a module `/selectors.py`, we can have the following: ```python -def users_list(*, fetched_by: User) -> Iterable[User]: - user_ids = users_get_visible_for(user=fetched_by) +def user_list(*, fetched_by: User) -> Iterable[User]: + user_ids = user_get_visible_for(user=fetched_by) query = Q(id__in=user_ids) return User.objects.filter(query) ``` -As you can see, `users_get_visible_for` is another selector. +As you can see, `user_get_visible_for` is another selector. You can return querysets, or lists or whatever makes sense to your specific case.