From b3c69087d8e6220e9dfb83bd5c7738cca2e2cd0c Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 16 Jun 2020 15:21:43 +0100
Subject: [PATCH] Serialise less stuff from the service object
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

By default Marshallow includes unknown properties. This means every time
a new property is added to the service model it gets included in the
JSON-serialised response sent to the admin app.

This is particuarly bad because it means that for returned letters the
ID of every returned letter. So the JSON stored in Redis for the
Check Your State Pension service is 86kb.

Similarly the JSON stored in Redis for a big user of inbound text
messaging is 458kb(!!!) because it has the ID of every received text
message. That’s ~8,500 UUIDs.

Luckily the admin app tells us exactly which keys it’s using here:
https://github.com/alphagov/notifications-admin/blob/5952d9c26da749e39d761e0d37dd72876e59fa6d/app/models/service.py#L31-L52

```python
- `active`
- `contact_link`
- `email_branding`
- `email_from`
- `id`
- `inbound_api`
- `letter_branding`
- `letter_contact_block`
- `message_limit`
- `name`
- `prefix_sms`
- `research_mode`
- `service_callback_api`
- `volume_email`
- `volume_sms`
- `volume_letter`
- `consent_to_research`
- `count_as_live`
- `go_live_user`
- `go_live_at`
}
```

Plus these which it does not get automatically:
- `email_branding`
- `letter_branding`
- `organisation`
- `organisation_type`
- `permissions`
- `restricted`

The API is returning all of these:
- `active`
- `all_template_folders`
- `annual_billing`
- `consent_to_research`
- `contact_link`
- `contact_list`
- `count_as_live`
- `created_by`
- `crown`
- `email_branding`
- `email_from`
- `go_live_at`
- `go_live_user`
- `id`
- `inbound_api`
- `inbound_number`
- `inbound_sms`
- `letter_branding`
- `letter_contact_block`
- `letter_logo_filename`
- `message_limit`
- `name`
- `organisation`
- `organisation_type`
- `permissions`
- `prefix_sms`
- `rate_limit`
- `research_mode`
- `restricted`
- `returned_letters`
- `service_callback_api`
- `users`
- `version`
- `volume_email`
- `volume_letter`
- `volume_sms`
- `whitelist`

So the ones that the admin is getting but not expecting are:
- `all_template_folders`
- `annual_billing`
- `contact_list`
- `created_by`
- `crown`
- `inbound_number`
- `inbound_sms`
- `letter_logo_filename`
- `rate_limit`
- `returned_letters`
- `users`
- `version`
- `whitelist`

Which is what this PR adds to the exclude list, except for `created_by`
which is keeps because it’s needed to validate the JSON provided when
creating a service.
---
 app/schemas.py                 | 25 ++++++++++++++++++++++++-
 tests/app/service/test_rest.py |  3 ---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/app/schemas.py b/app/schemas.py
index 10cf6e90f..c985d1812 100644
--- a/app/schemas.py
+++ b/app/schemas.py
@@ -235,6 +235,18 @@ class ServiceSchema(BaseSchema):
             'letter_contacts',
             'complaints',
             'data_retention',
+            'all_template_folders',
+            'annual_billing',
+            'contact_list',
+            'crown',
+            'inbound_number',
+            'inbound_sms',
+            'letter_logo_filename',
+            'rate_limit',
+            'returned_letters',
+            'users',
+            'version',
+            'whitelist',
         )
         strict = True
 
@@ -289,7 +301,18 @@ class DetailedServiceSchema(BaseSchema):
             'sms_sender',
             'permissions',
             'inbound_number',
-            'inbound_sms'
+            'inbound_sms',
+            'all_template_folders',
+            'annual_billing',
+            'contact_list',
+            'created_by',
+            'crown',
+            'letter_logo_filename',
+            'rate_limit',
+            'returned_letters',
+            'users',
+            'version',
+            'whitelist',
         )
 
 
diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py
index 31e27460c..159e45117 100644
--- a/tests/app/service/test_rest.py
+++ b/tests/app/service/test_rest.py
@@ -235,7 +235,6 @@ def test_get_service_by_id(admin_request, sample_service):
     assert json_resp['data']['email_branding'] is None
     assert 'branding' not in json_resp['data']
     assert json_resp['data']['prefix_sms'] is True
-    assert json_resp['data']['letter_logo_filename'] is None
 
 
 @pytest.mark.parametrize('detailed', [True, False])
@@ -347,7 +346,6 @@ def test_create_service(
     assert json_resp['data']['name'] == 'created service'
     assert json_resp['data']['email_from'] == 'created.service'
     assert not json_resp['data']['research_mode']
-    assert json_resp['data']['rate_limit'] == 3000
     assert json_resp['data']['letter_branding'] is None
     assert json_resp['data']['count_as_live'] is expected_count_as_live
 
@@ -1225,7 +1223,6 @@ def test_add_existing_user_to_another_service_with_all_permissions(
             )
             assert resp.status_code == 200
             json_resp = resp.json
-            assert str(user_to_add.id) in json_resp['data']['users']
 
             # check user has all permissions
             auth_header = create_authorization_header()