Improve tear down speed

app/services/data/tear-down/README.md

@@ -0,0 +1,89 @@
+# Tear down
+
+This function supports our [water-abstraction-acceptance-tests](https://github.com/DEFRA/water-abstraction-acceptance-tests). At the start of each test, it will make an HTTP call to `POST /data/tear-down` resulting in `TearDownService.go()` being called.
+
+The service and those it calls are responsible for clearing the various schemas we use of test data. Some tables have a `is_test` field that immediately identifies the record as being created for a test.
+
+But each test will create data that won't be flagged this way. So, the tear-down services also remove records we can identify as being test data.
+
+- it might be linked to a `is_test` record
+- it has a known value, for example, `name`, that is specific to the acceptance tests
+
+Ideally, we wouldn't need to remove anything between the tests. We could then avoid this step and the delay it causes. But until we re-architect fully the acceptance tests we've inherited we have to work with what we've got.
+
+When we took on WRLS [water-abstraction-service](https://github.com/DEFRA/water-abstraction-service) had a tear-down endpoint. But it would take more than a minute to complete. This caused the tests to take a long time to finish and often caused them to fail.
+
+We built a more performant version. It would typically get the job done in 15 to 20 seconds. But sometimes this would creep to 40 seconds which again would cause test runs to fail.
+
+We are adding this README at the point we've taken a second shot at an even more performant option. We wanted to capture what we tried and the results we got to help
+
+- explain why this is the current solution
+- avoid anyone wasting time trying anything we've already tried
+- spark inspiration of an _even better_ way of doing this!
+
+## Strategies tried
+
+Including sticking with what we had, we tried 8 separate ways of clearing down the test data. We would run the supplementary billing `journey.cy.js` acceptance test and record the time it took in milliseconds to
+
+- wipe each of the schemas
+- complete all schemas
+
+We did this a number of times and then averaged out the results. We then selected the one that was most performant overall.
+
+> This was tested on an Apple M1 Mac. The DB was a complete import of the NALD data including all returns. This explains why the times recorded here (especially for the 'No changes' strategy) might be greater than you see if doing the same thing yourself.
+
+### No change
+
+As a bench mark we recorded what the times were using the existing solution. The key issue was clearing the `WATER` schema, which we suspected was due to the numerous foreign key constraints the previous team added.
+
+When a record is deleted these constraints are triggered and checked.
+
+### Disable the triggers
+
+In this attempt the only change we made was to disable all the triggers in a schema first, then leave the existing code run, followed by re-enabling the triggers.
+
+This was the critical change that got the average completion time down from 42 secs to less than 7 secs.
+
+### Single query
+
+The existing functionality would clear down all schemas at the same time using a [Promise.all()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all). But within each schema's tear-down service the tables would be cleared down one at a time using [Knex.js](https://knexjs.org/).
+
+We wanted to test if performance was better if we fired a single [raw query](https://knexjs.org/guide/raw.html#raw-queries) at the DB for each schema instead. We retained the trigger disabling strategy because of the impact it had. We simply combined the calls for these with the delete table calls into a single DB query.
+
+This time we got a small 1 sec improvement in the average time to complete.
+
+### Promise all
+
+Next we wanted to test what would happen if instead of firing a [single query per schema](#single-query), we adopted the same `Promise.all()` pattern used at the schema level to the queries inside each schema service.
+
+We rebuilt all the existing delete functions to just use 'raw' queries. This allowed us to bring the disabling of a table's triggers together with it's delete statement as one request to the DB.
+
+We then fired them all at the same time using `Promise.all()`, whilst clearing down all schemas at the same time.
+
+We were concerned that we might be pushing _too much_ at the DB at the same time. But we did see a minor improvement in the average completion time of 100ms. Though it should be noted that the average schema completion time degraded for some schemas.
+
+### Best of
+
+Having seen that some schemas performed better when fired as a [single query](#single-query) whereas others got their best times using the [promise all strategy](#promise-all) we tried a 'best of'.
+
+With this we saw a degradation in completion time of 2 secs. We were also uncomfortable with mixing patterns in the schemas so were happy to see this didn't win out!
+
+### All in one
+
+Finally, we tried lumping all the queries we were firing into a single query. Essentially, let the DB do it all and perhaps optimise it better than what we were attempting.
+
+Again, this confirmed disabling the table triggers still had a massive impact on average completion time as it was vastly quicker than our current implementation. But it was still 2 secs off [Promise all](#promise-all).
+
+## Results
+
+> The average times are in milliseconds. Just divide by 1000 for secs, for example, 1770 / 1000 = 1.7 secs
+
+| Schema  | No change | Just triggers | Single Query | Promise all | Best of | All in one |
+|---------|-----------|---------------|--------------|-------------|---------|------------|
+| CRM     | 1770      | 2106          | 1185         | **964**     | 1444    | N/A        |
+| IDM     | 58        | 73            | 46           | **852**     | 1199    | N/A        |
+| PERMIT  | 165       | 266           | 156          | **893**     | 1282    | N/A        |
+| RETURNS | 7176      | 6121          | 3517         | **4474**    | 7528    | N/A        |
+| WATER   | 42268     | 4413          | 5266         | **5106**    | 5289    | N/A        |
+| -       | -         | -             | -            | -           | -       |            |
+| Total   | 42269     | 6556          | 5266         | **5106**    | 7529    | 7457       |

app/services/data/tear-down/crm-schema.service.js

@@ -8,79 +8,216 @@
 const { db } = require('../../../../db/db.js')
 
 async function go () {
-  await _deleteTestData('crm_v2.documentRoles')
-  await _deleteTestData('crm_v2.companyAddresses')
-  await _deleteEntities()
-  await _deleteInvoiceAccounts()
-  await _deleteTestData('crm_v2.companyContacts')
-  await _deleteTestData('crm_v2.companies')
-  await _deleteTestData('crm_v2.addresses')
-  await _deleteDocuments()
-  await _deleteTestData('crm_v2.contacts')
-
-  await _deleteCompanies()
+  return Promise.all([
+    _crmEntityRoles(),
+    _crmEntity(),
+    _crmDocumentHeader(),
+    _crmV2DocumentRoles(),
+    _crmV2CompanyAddresses(),
+    _crmV2InvoiceAccountAddresses(),
+    _crmV2InvoiceAccounts(),
+    _crmV2CompanyContacts(),
+    _crmV2Companies(),
+    _crmV2Addresses(),
+    _crmV2Documents(),
+    _crmV2Contacts()
+  ])
 }
 
-async function _deleteCompanies () {
-  await db
-    .from('crm_v2.companies')
-    .whereLike('name', 'Big Farm Co Ltd%')
-    .del()
+async function _crmEntityRoles () {
+  return db.raw(`
+  DELETE
+  FROM
+    crm.entity_roles
+  WHERE
+    created_by = 'acceptance-test-setup';
+  `)
 }
 
-async function _deleteEntities () {
-  await db
-    .from('crm.entityRoles')
-    .where('createdBy', 'acceptance-test-setup')
-    .del()
-
-  await db
-    .from('crm.entity')
-    .whereLike('entityNm', 'acceptance-test.%')
-    .orWhereLike('entityNm', '%@example.com')
-    .orWhereLike('entityNm', 'regression.tests.%')
-    .orWhereLike('entityNm', 'Big Farm Co Ltd%')
-    .orWhere('source', 'acceptance-test-setup')
-    .del()
+async function _crmEntity () {
+  return db.raw(`
+  DELETE
+  FROM
+    crm.entity
+  WHERE
+    entity_nm LIKE 'acceptance-test.%'
+    OR entity_nm LIKE '%@example.com'
+    OR entity_nm LIKE 'regression.tests.%'
+    OR entity_nm LIKE 'Big Farm Co Ltd%'
+    OR SOURCE = 'acceptance-test-setup';
+  `)
 }
 
-async function _deleteInvoiceAccounts () {
-  await db
-    .from('crm_v2.invoiceAccountAddresses as iaa')
-    .innerJoin('crm_v2.invoiceAccounts as ia', 'iaa.invoiceAccountId', 'ia.invoiceAccountId')
-    .innerJoin('crm_v2.companies as c', 'ia.companyId', 'c.companyId')
-    .where('c.isTest', true)
-    .del()
-
-  await _deleteTestData('crm_v2.invoiceAccountAddresses')
-
-  await db
-    .from('crm_v2.invoiceAccounts as ia')
-    .innerJoin('crm_v2.companies as c', 'ia.companyId', 'c.companyId')
-    .where('c.isTest', true)
-    .del()
+async function _crmDocumentHeader () {
+  return db.raw(`
+  ALTER TABLE crm.document_header DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    crm.document_header
+  WHERE
+    jsonb_path_query_first(
+      metadata,
+      '$.dataType'
+    ) #>> '{}' = 'acceptance-test-setup';
+
+  ALTER TABLE crm.document_header ENABLE TRIGGER ALL;
+  `)
+}
+
+async function _crmV2DocumentRoles () {
+  return db.raw(`
+  ALTER TABLE crm_v2.document_roles DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."document_roles"
+  WHERE
+    "is_test" = TRUE;
+
+  ALTER TABLE crm_v2.document_roles ENABLE TRIGGER ALL;
+  `)
+}
+
+async function _crmV2CompanyAddresses () {
+  return db.raw(`
+  ALTER TABLE crm_v2.company_addresses DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."company_addresses"
+  WHERE
+    "is_test" = TRUE;
+
+  ALTER TABLE crm_v2.company_addresses ENABLE TRIGGER ALL;
+  `)
 }
 
-async function _deleteDocuments () {
-  await _deleteTestData('crm_v2.documents')
+async function _crmV2InvoiceAccountAddresses () {
+  return db.raw(`
+  ALTER TABLE crm_v2.invoice_account_addresses DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."invoice_account_addresses" AS "iaa"
+      USING "crm_v2"."invoice_accounts" AS "ia",
+    "crm_v2"."companies" AS "c"
+  WHERE
+    "c"."is_test" = TRUE
+    AND "iaa"."invoice_account_id" = "ia"."invoice_account_id"
+    AND "ia"."company_id" = "c"."company_id";
 
-  await db
-    .from('crm_v2.documents as d')
-    .innerJoin('crm.documentHeader as dh', 'd.documentRef', 'dh.systemExternalId')
-    .whereJsonPath('dh.metadata', '$.dataType', '=', 'acceptance-test-setup')
-    .del()
+  DELETE
+  FROM
+    "crm_v2"."invoice_account_addresses"
+  WHERE
+    "is_test" = TRUE;
 
-  await db
-    .from('crm.documentHeader')
-    .whereJsonPath('metadata', '$.dataType', '=', 'acceptance-test-setup')
-    .del()
+  ALTER TABLE crm_v2.invoice_account_addresses ENABLE TRIGGER ALL;
+  `)
 }
 
-async function _deleteTestData (tableName) {
-  await db
-    .from(tableName)
-    .where('isTest', true)
-    .del()
+async function _crmV2InvoiceAccounts () {
+  return db.raw(`
+  ALTER TABLE crm_v2.invoice_accounts DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."invoice_accounts" AS "ia"
+      USING "crm_v2"."companies" AS "c"
+  WHERE
+    "c"."is_test" = TRUE
+    AND "ia"."company_id" = "c"."company_id";
+
+  ALTER TABLE crm_v2.invoice_accounts ENABLE TRIGGER ALL;
+  `)
+}
+
+async function _crmV2CompanyContacts () {
+  return db.raw(`
+  ALTER TABLE crm_v2.company_contacts DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."company_contacts"
+  WHERE
+    "is_test" = TRUE;
+
+  ALTER TABLE crm_v2.company_contacts ENABLE TRIGGER ALL;
+  `)
+}
+
+async function _crmV2Companies () {
+  return db.raw(`
+  ALTER TABLE crm_v2.companies DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."companies"
+  WHERE
+    "is_test" = TRUE;
+
+  DELETE
+  FROM
+    "crm_v2"."companies"
+  WHERE
+    "name" LIKE 'Big Farm Co Ltd%';
+
+  ALTER TABLE crm_v2.companies ENABLE TRIGGER ALL;
+  `)
+}
+
+async function _crmV2Addresses () {
+  return db.raw(`
+  ALTER TABLE crm_v2.addresses DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."addresses"
+  WHERE
+    "is_test" = TRUE;
+
+  ALTER TABLE crm_v2.addresses ENABLE TRIGGER ALL;
+  `)
+}
+
+async function _crmV2Documents () {
+  return db.raw(`
+  ALTER TABLE crm_v2.documents DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."documents"
+  WHERE
+    "is_test" = TRUE;
+
+  DELETE
+  FROM
+    "crm_v2"."documents" AS "d"
+      USING "crm"."document_header" AS "dh"
+  WHERE
+    jsonb_path_query_first(
+      "dh"."metadata",
+      '$.dataType'
+    ) #>> '{}' = 'acceptance-test-setup'
+    AND "d"."document_ref" = "dh"."system_external_id";
+
+  ALTER TABLE crm_v2.documents ENABLE TRIGGER ALL;
+  `)
+}
+
+async function _crmV2Contacts () {
+  return db.raw(`
+  ALTER TABLE crm_v2.contacts DISABLE TRIGGER ALL;
+
+  DELETE
+  FROM
+    "crm_v2"."contacts"
+  WHERE
+    "is_test" = TRUE;
+
+  ALTER TABLE crm_v2.contacts ENABLE TRIGGER ALL;
+  `)
 }
 
 module.exports = {

app/services/data/tear-down/idm-schema.service.js

@@ -8,11 +8,21 @@
 const { db } = require('../../../../db/db.js')
 
 async function go () {
-  await db
-    .from('idm.users')
-    .whereJsonPath('user_data', '$.source', '=', 'acceptance-test-setup')
-    .orWhereLike('userName', '%@example.com')
-    .del()
+  return _users()
+}
+
+async function _users () {
+  return db.raw(`
+  DELETE
+  FROM
+    "idm"."users"
+  WHERE
+    jsonb_path_query_first(
+      "user_data",
+      '$.source'
+    ) #>> '{}' = 'acceptance-test-setup'
+    OR "user_name" LIKE '%@example.com';
+  `)
 }
 
 module.exports = {