Fix/api dbs

[chrismclarke]

PR Checklist

• PR title descriptive (can be used in release notes)

Description

• Update api core packages (minor patches only to avoid breaking changes)
• Add testing infrastructure and recreate issue seen in production (entries not writing to correct db)
• Refactor way in requests get mapped to models that have specific db connection query interfaces
• Add global connection manager to simplify code in deployment service (and because deployment service is recreated on a per-request basis)

Review Notes

I doubt the code changes will make much sense (a suuuper deep dive into nestJS/sequelize issues), but it might be useful if you are able to confirm that you can at least setup the e2e tests (see notes in updated documentation) and run the test suite to confirm similar outcomes.

Dev Notes

This issue was really awful to debug, so many moving pieces on top of a lot of layers of obscurity with how nest-js -> sequelize-typescript -> sequelize -> postgres, with various levels at which services are instantiated (sometimes globally, sometimes per request, not always as expected (e.g. global services still created per-request when interacted with via middleware).

The new testing infrastructure should hopefully help a lot moving forwards, but it still feels like what we're trying to achieve with the database goes far beyond what the system really should be used for.

Ultimately there were a few core issues which should now be resolved, namely:

• Lack of testing infrastructure making it hard to debug. A small set of e2e tests have been created to help debug issues.
• Use of middleware to set default db client just wasn't working as expected. Instead middleware now only bootstraps db and specific connection determined at a later step
• The way in which nestJS models default to using default db connection - this is now manually when accessed through deployment service (but still may cause issue
• The fact that services declared globally were actually being instantiated on a per-request basis due to... reasons (eurgh, could go into details if interested but honestly a headache).
• Memory management/allocation when lots of requests in parallel writing to objects that have lots of deep/circular references (again, could go into the details but don't expect them to wow anyone)

Follow-up Tasks (Post Merge)

• Refactor remaining code that calls model constructor (e.g. new User(...)), in favour of deployment service methods (e.g. this.model.create(...)
• Testing additional methods (beyond user create)
• Documentation (?)
• Deploy to server
• Consider NestCLS in case might be able to help simplify some of the logic

Git Issues

Closes #2098

Screenshots/Videos

Set of tests created (passing)

Example output - 5 users written concurrently to 5 different databases

 [                                                                               
      {
        "status": 201,
        "body": {
          "id": 1,
          "app_user_id": "0803991a-3b5c-408d-9423-664ea5a0b5d6_0",
          "app_deployment_name": "test",
          "app_version": "0.0.0",
          "contact_fields": {},
          "device_info": {},
          "updatedAt": "2023-10-25T21:34:10.921Z",
          "createdAt": "2023-10-25T21:34:10.921Z"
        },
        "deploymentDBName": "test_e2e_ad695a_0"
      },
      {
        "status": 201,
        "body": {
          "id": 1,
          "app_user_id": "0803991a-3b5c-408d-9423-664ea5a0b5d6_1",
          "app_deployment_name": "test",
          "app_version": "0.0.0",
          "contact_fields": {},
          "device_info": {},
          "updatedAt": "2023-10-25T21:34:10.924Z",
          "createdAt": "2023-10-25T21:34:10.924Z"
        },
        "deploymentDBName": "test_e2e_ad695a_1"
      },
      {
        "status": 201,
        "body": {
          "id": 1,
          "app_user_id": "0803991a-3b5c-408d-9423-664ea5a0b5d6_2",
          "app_deployment_name": "test",
          "app_version": "0.0.0",
          "contact_fields": {},
          "device_info": {},
          "updatedAt": "2023-10-25T21:34:10.928Z",
          "createdAt": "2023-10-25T21:34:10.928Z"
        },
        "deploymentDBName": "test_e2e_ad695a_2"
      },
      {
        "status": 201,
        "body": {
          "id": 1,
          "app_user_id": "0803991a-3b5c-408d-9423-664ea5a0b5d6_3",
          "app_deployment_name": "test",
          "app_version": "0.0.0",
          "contact_fields": {},
          "device_info": {},
          "updatedAt": "2023-10-25T21:34:10.930Z",
          "createdAt": "2023-10-25T21:34:10.930Z"
        },
        "deploymentDBName": "test_e2e_ad695a_3"
      },
      {
        "status": 201,
        "body": {
          "id": 1,
          "app_user_id": "0803991a-3b5c-408d-9423-664ea5a0b5d6_4",
          "app_deployment_name": "test",
          "app_version": "0.0.0",
          "contact_fields": {},
          "device_info": {},
          "updatedAt": "2023-10-25T21:34:10.934Z",
          "createdAt": "2023-10-25T21:34:10.934Z"
        },
        "deploymentDBName": "test_e2e_ad695a_4"
      }
    ]

Responses above additionally verified by specific db queries, e.g. query all rows for test_e2e_ad695a_2

{
      deploymentDBName: 'test_e2e_ad695a_2',
      rowCount: 1,
      rows: [
        {
          id: 1,
          createdAt: 2023-10-25T21:34:10.928Z,
          updatedAt: 2023-10-25T21:34:10.928Z,
          deletedAt: null,
          version: null,
          app_user_id: '0803991a-3b5c-408d-9423-664ea5a0b5d6_2',
          contact_fields: {},
          app_version: '0.0.0',
          device_info: {},
          app_deployment_name: 'test'
        }
      ]
    }

[esmeetewinkel]

Does this resolve our discussion on issue #2098? If so let's close.

[jfmcquade]

Thanks for the notes @chrismclarke, seems like a big headache. I haven't dived into all the code, but I am able to run the e2e tests with positive results:

I did have an issue with the umzug package when I initially ran yarn workspace api start

I resolved this by resetting the required version of umzug in packages/api/package.json to "umzug": "3.2.1" (this PR otherwise bumps up the version number to "^3.3.1"). I'm not sure why that would cause issues on my machine and not yours. Looking at the package's release notes, there were some issues with release 3.3.0 but I'm not sure if that's relevant

[chrismclarke]

Thanks for the review @jfmcquade . Weird issue with umzug, but glad you could get working locally.
I'm going to merge here and raise a second PR (#2115) with some of additional housekeeping required.

@esmeetewinkel - just to clarify on the related issue. This PR should indeed fix, however only once the code has been deployed to the server. I think it would probably be best to wait until next week after the additional pr #2115 raised/reviewed/merged (to allow for some extra testing), although if that PR gets stalled or needed urgently then we could make separate server updates for each PR