From d2da66c244cc2480c278e5ee06a487995105041f Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 10 Oct 2024 12:19:51 +0100 Subject: [PATCH 1/3] Fix error when seeding in AWS non-production MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unlike migrations, which are only ever expected to be run once against a DB, seeders may be run repeatedly. For example, a new charge version 'change reason' is required in the future. We would add it to our `db/seeds/data/change-reasons.js` and simply re-run `npm run seed`. Because of this each seeder needs to be able to handle being run against an environment where the records may already exist. In most cases, we can exploit constraints in the DB and PostgreSQL's [ON CONFLICT clause](https://www.postgresql.org/docs/current/sql-insert.html), also known as an 'upsert'. We try to insert a record, but when a conflict error is thrown because a matching record already exists, we can instruct PostgreSQL to update certain fields instead. That is unless it is `idm.users`! The previous team appear to have opted instead to create their own custom sequence and use a number as the ID. 😩 Instead of the expected `on conflict` being raised, we're getting a primary key violation. Simplest solution is to fall back to checking if a record exists first, and then running the insert or update separately depending on the result. --- db/seeds/09-users.seed.js | 62 ++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/db/seeds/09-users.seed.js b/db/seeds/09-users.seed.js index 71037796aa..e896f99de0 100644 --- a/db/seeds/09-users.seed.js +++ b/db/seeds/09-users.seed.js @@ -18,10 +18,29 @@ async function seed () { const password = _generateHashedPassword() for (const user of users) { - await _upsert(user, password) + const exists = await _exists(user) + + if (exists) { + await _update(user, password) + } else { + await _insert(user) + } } } +async function _exists (user) { + const { application, username } = user + + const result = await UserModel.query() + .select('id') + .where('application', application) + .andWhere('username', username) + .limit(1) + .first() + + return !!result +} + function _generateHashedPassword () { // 10 is the number of salt rounds to perform to generate the salt. The legacy code uses // const salt = bcrypt.genSaltSync(10) to pre-generate the salt before passing it to hashSync(). But this is @@ -31,20 +50,35 @@ function _generateHashedPassword () { return bcrypt.hashSync(DatabaseConfig.defaultUserPassword, 10) } -async function _upsert (user, password) { +async function _insert (user, password) { + return UserModel.query().insert({ ...user, password }) +} + +async function _update (user, password) { + const { + application, + badLogins, + enabled, + lastLogin, + resetGuid, + resetGuidCreatedAt, + resetRequired, + username + } = user + return UserModel.query() - .insert({ ...user, password, updatedAt: timestampForPostgres() }) - .onConflict(['application', 'username']) - .merge([ - 'badLogins', - 'enabled', - 'lastLogin', - 'password', - 'resetGuid', - 'resetGuidCreatedAt', - 'resetRequired', - 'updatedAt' - ]) + .patch({ + badLogins, + enabled, + lastLogin, + password, + resetGuid, + resetGuidCreatedAt, + resetRequired, + updatedAt: timestampForPostgres() + }) + .where('application', application) + .andWhere('username', username) } module.exports = { From a1529796f167b1fae280585a80925a359403f914 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 10 Oct 2024 12:35:16 +0100 Subject: [PATCH 2/3] Oops! --- db/seeds/09-users.seed.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/seeds/09-users.seed.js b/db/seeds/09-users.seed.js index e896f99de0..479f0ed01f 100644 --- a/db/seeds/09-users.seed.js +++ b/db/seeds/09-users.seed.js @@ -23,7 +23,7 @@ async function seed () { if (exists) { await _update(user, password) } else { - await _insert(user) + await _insert(user, password) } } } From 1a250cab985df4078f0b42395574c43544704a06 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 10 Oct 2024 13:06:43 +0100 Subject: [PATCH 3/3] Handle another issue with seeding in AWS non-prod --- db/seeds/09-users.seed.js | 42 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/db/seeds/09-users.seed.js b/db/seeds/09-users.seed.js index 479f0ed01f..ddc71a9ab6 100644 --- a/db/seeds/09-users.seed.js +++ b/db/seeds/09-users.seed.js @@ -50,7 +50,49 @@ function _generateHashedPassword () { return bcrypt.hashSync(DatabaseConfig.defaultUserPassword, 10) } +async function _idInUse (id) { + const result = await UserModel.query() + .findById(id) + + return !!result +} + async function _insert (user, password) { + const { + application, + badLogins, + enabled, + id, + lastLogin, + resetGuid, + resetGuidCreatedAt, + resetRequired, + username + } = user + + // NOTE: Seeding users is a pain (!) because of the previous teams choice to use a custom sequence for the ID instead + // of sticking with UUIDs. This means it is possible that, for example, a user with + // + // `username = 'admin-internal@wrls.gov.uk' && application = 'water_admin'` + // + // does not exist. _But_ a user with ID 100000 does! So, we do want to insert our record, but we can't use the ID + // because it is already in use. We only really face this problem when running the seed in our AWS environments. + const idInUse = await _idInUse(id) + + if (idInUse) { + return UserModel.query().insert({ + application, + badLogins, + enabled, + lastLogin, + password, + resetGuid, + resetGuidCreatedAt, + resetRequired, + username + }) + } + return UserModel.query().insert({ ...user, password }) }