From 32585023899b02b0b3864745b1e476c30f2cf9af Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 11:44:58 +0100 Subject: [PATCH 01/31] Save redis events Signed-off-by: John Gomersall --- src/domain/entities/settings.ts | 13 ----- src/domain/services/import.service.spec.ts | 56 +++++++++++++++++-- src/domain/services/import.service.ts | 13 +++++ src/domain/services/settings.service.ts | 28 ++++------ src/migrations/.snapshot-query.json | 48 ---------------- .../Migration20240719101700-redis.ts | 8 +++ 6 files changed, 84 insertions(+), 82 deletions(-) delete mode 100644 src/domain/entities/settings.ts create mode 100644 src/migrations/Migration20240719101700-redis.ts diff --git a/src/domain/entities/settings.ts b/src/domain/entities/settings.ts deleted file mode 100644 index 40de5ca..0000000 --- a/src/domain/entities/settings.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { Entity, PrimaryKey, Property } from '@mikro-orm/core'; - -@Entity() -export class Settings { - @PrimaryKey() - id = 1; - - @Property({ columnType: 'timestamptz' }) - lastModified?: Date; - - @Property() - lastMessageId?: string; -} diff --git a/src/domain/services/import.service.spec.ts b/src/domain/services/import.service.spec.ts index a63ac84..b6752b9 100644 --- a/src/domain/services/import.service.spec.ts +++ b/src/domain/services/import.service.spec.ts @@ -13,6 +13,7 @@ import { createClient } from 'redis'; import { GenericContainer } from 'testcontainers'; import { setTimeout } from 'timers/promises'; import { ProductIngredient } from '../entities/product-ingredient'; +import sql from '../../db'; const lastModified = 1692032161; @@ -402,6 +403,41 @@ describe('importWithFilter', () => { }); }); +describe('messageTime', () => { + it('should return a date from a message id', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const time = lastModified * 1000; + const date = importService.messageTime({ id: `${time}-0` }); + expect(date.getTime()).toBe(time); + }); + }); + it('should return the current date for an invalid message id', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const now = Date.now(); + const date = importService.messageTime({ id: 'invalid' }); + expect(date.getTime()).toBeGreaterThanOrEqual(now); + }); + }); + it('should cope with a null id', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const now = Date.now(); + const date = importService.messageTime({ id: null }); + expect(date.getTime()).toBeGreaterThanOrEqual(now); + }); + }); + it('should cope with no id', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const now = Date.now(); + const date = importService.messageTime({}); + expect(date.getTime()).toBeGreaterThanOrEqual(now); + }); + }); +}); + describe('receiveMessages', () => { it('should call importwithfilter when a message is received', async () => { await createTestingModule([DomainModule], async (app) => { @@ -424,13 +460,16 @@ describe('receiveMessages', () => { const client = createClient({ url: redisUrl }); await client.connect(); try { + const code1 = randomCode(); + const code2 = randomCode(); + // When: A message is sent const messageId = await client.xAdd('product_updates_off', '*', { - code: 'TEST1', + code: code1, }); // Wait for message to be delivered - await setTimeout(10); + await setTimeout(100); // Then the import is called expect(importSpy).toHaveBeenCalledTimes(1); @@ -439,17 +478,24 @@ describe('receiveMessages', () => { // If a new message is added importSpy.mockClear(); await client.xAdd('product_updates_off', '*', { - code: 'TEST2', + code: code2, }); // Wait for message to be delivered - await setTimeout(10); + await setTimeout(100); // Then import is called again but only with the new code expect(importSpy).toHaveBeenCalledTimes(1); const codes = importSpy.mock.calls[0][0].code.$in; expect(codes).toHaveLength(1); - expect(codes[0]).toBe('TEST2'); + expect(codes[0]).toBe(code2); + + // Update events are created + const events = + await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; + + expect(events).toHaveLength(1); + expect(events[0].id).toBe(messageId); } finally { await client.quit(); await importService.stopRedisConsumer(); diff --git a/src/domain/services/import.service.ts b/src/domain/services/import.service.ts index 9e502c6..e04bb32 100644 --- a/src/domain/services/import.service.ts +++ b/src/domain/services/import.service.ts @@ -368,6 +368,11 @@ export class ImportService { if (this.client && this.client.isOpen) await this.client.quit(); } + messageTime(message: any) { + const time = new Date(parseInt(message.id?.split('-')[0])); + return isNaN(time.getTime()) ? new Date() : time; + } + async receiveMessages() { const lastMessageId = await this.settings.getLastMessageId(); if (!this.client.isOpen) return; @@ -404,6 +409,14 @@ export class ImportService { diffs: "{\"fields\":{\"change\":[\"categories\"],\"delete\":[\"product_name\",\"product_name_es\"]}}", } */ + await sql`INSERT into product_update_event ${sql( + messages.map((m) => ({ + id: m.id, + updated_at: this.messageTime(m), + code: m.message.code, + message: m.message, + })), + )}`; const productCodes = messages.map((m) => m.message.code); const filter = { code: { $in: productCodes } }; await this.importWithFilter(filter, ProductSource.EVENT); diff --git a/src/domain/services/settings.service.ts b/src/domain/services/settings.service.ts index 66f2277..8e7a2be 100644 --- a/src/domain/services/settings.service.ts +++ b/src/domain/services/settings.service.ts @@ -1,36 +1,32 @@ -import { EntityManager } from '@mikro-orm/core'; import { Injectable } from '@nestjs/common'; -import { Settings } from '../entities/settings'; +import sql from '../../db'; @Injectable() export class SettingsService { - constructor(private readonly em: EntityManager) {} - - settings: Settings; - async find() { - this.settings = await this.em.findOne(Settings, 1); - if (!this.settings) { - this.settings = this.em.create(Settings, {}); + async updateSetting(settings: any) { + const result = await sql`UPDATE settings SET ${sql(settings)}`; + if (!result.count) { + await sql`INSERT INTO settings ${sql(settings)}`; } - return this.settings; } async getLastModified() { - return (await this.find()).lastModified; + return (await sql`SELECT last_modified FROM settings`)[0].last_modified; } async setLastModified(lastModified: Date) { - (await this.find()).lastModified = lastModified; - await this.em.flush(); + await this.updateSetting({ last_modified: lastModified }); } async getLastMessageId() { - return (await this.find()).lastMessageId || '$'; + return ( + (await sql`SELECT last_message_id FROM settings`)[0].last_message_id || + '$' + ); } async setLastMessageId(messageId: string) { - (await this.find()).lastMessageId = messageId; - await this.em.flush(); + await this.updateSetting({ last_message_id: messageId }); } getRedisUrl() { diff --git a/src/migrations/.snapshot-query.json b/src/migrations/.snapshot-query.json index 7f15460..f1a387c 100644 --- a/src/migrations/.snapshot-query.json +++ b/src/migrations/.snapshot-query.json @@ -4900,54 +4900,6 @@ "updateRule": "cascade" } } - }, - { - "columns": { - "id": { - "name": "id", - "type": "serial", - "unsigned": true, - "autoincrement": true, - "primary": true, - "nullable": false, - "default": "1", - "mappedType": "integer" - }, - "last_modified": { - "name": "last_modified", - "type": "timestamptz", - "unsigned": false, - "autoincrement": false, - "primary": false, - "nullable": true, - "length": 6, - "mappedType": "datetime" - }, - "last_message_id": { - "name": "last_message_id", - "type": "text", - "unsigned": false, - "autoincrement": false, - "primary": false, - "nullable": true, - "mappedType": "text" - } - }, - "name": "settings", - "schema": "query", - "indexes": [ - { - "keyName": "settings_pkey", - "columnNames": [ - "id" - ], - "composite": false, - "primary": true, - "unique": true - } - ], - "checks": [], - "foreignKeys": {} } ] } diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts new file mode 100644 index 0000000..364e198 --- /dev/null +++ b/src/migrations/Migration20240719101700-redis.ts @@ -0,0 +1,8 @@ +/* eslint-disable prettier/prettier */ +import { Migration } from '@mikro-orm/migrations'; + +export class Migration20240719101700Redis extends Migration { + async up(): Promise { + this.addSql(`CREATE TABLE IF NOT EXISTS query.product_update_event (id text NOT NULL PRIMARY KEY, updated_at timestamptz NOT NULL, code text NULL, message jsonb NOT NULL)`); + } +} From ba7de1e44680f6daf43957e808fb3ec7734f28e8 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 12:56:21 +0100 Subject: [PATCH 02/31] Test running query directly Signed-off-by: John Gomersall --- src/constants.ts | 2 + src/db.ts | 1 - src/domain/domain.module.ts | 9 ++- src/domain/services/import.service.spec.ts | 35 ---------- src/domain/services/import.service.ts | 16 +---- src/domain/services/messages.service.spec.ts | 28 ++++++++ src/domain/services/messages.service.ts | 21 ++++++ src/domain/services/views.spec.ts | 69 +++++++++++++++++++ .../Migration20240719101700-redis.ts | 2 + 9 files changed, 133 insertions(+), 50 deletions(-) create mode 100644 src/domain/services/messages.service.spec.ts create mode 100644 src/domain/services/messages.service.ts create mode 100644 src/domain/services/views.spec.ts diff --git a/src/constants.ts b/src/constants.ts index 2a1500e..c5493c7 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1 +1,3 @@ export const SCHEMA = 'query'; +export const VIEW_USER = 'viewer'; +export const VIEW_PASSWORD = 'off'; diff --git a/src/db.ts b/src/db.ts index c988f59..9406193 100644 --- a/src/db.ts +++ b/src/db.ts @@ -1,5 +1,4 @@ import postgres from 'postgres'; -import { SCHEMA } from './constants'; const sql = postgres({ host: process.env.POSTGRES_HOST, diff --git a/src/domain/domain.module.ts b/src/domain/domain.module.ts index e603134..3ef5e9a 100644 --- a/src/domain/domain.module.ts +++ b/src/domain/domain.module.ts @@ -6,10 +6,17 @@ import { TagService } from './services/tag.service'; import { SettingsService } from './services/settings.service'; import { EntityManager, RequestContext } from '@mikro-orm/core'; import { Cron, ScheduleModule } from '@nestjs/schedule'; +import { MessagesService } from './services/messages.service'; @Module({ imports: [MikroOrmModule.forRoot(), ScheduleModule.forRoot()], - providers: [ImportService, QueryService, TagService, SettingsService], + providers: [ + ImportService, + QueryService, + TagService, + SettingsService, + MessagesService, + ], exports: [ImportService, QueryService, TagService, SettingsService], }) export class DomainModule implements OnModuleInit, OnModuleDestroy { diff --git a/src/domain/services/import.service.spec.ts b/src/domain/services/import.service.spec.ts index b6752b9..a142c9d 100644 --- a/src/domain/services/import.service.spec.ts +++ b/src/domain/services/import.service.spec.ts @@ -403,41 +403,6 @@ describe('importWithFilter', () => { }); }); -describe('messageTime', () => { - it('should return a date from a message id', async () => { - await createTestingModule([DomainModule], async (app) => { - const importService = app.get(ImportService); - const time = lastModified * 1000; - const date = importService.messageTime({ id: `${time}-0` }); - expect(date.getTime()).toBe(time); - }); - }); - it('should return the current date for an invalid message id', async () => { - await createTestingModule([DomainModule], async (app) => { - const importService = app.get(ImportService); - const now = Date.now(); - const date = importService.messageTime({ id: 'invalid' }); - expect(date.getTime()).toBeGreaterThanOrEqual(now); - }); - }); - it('should cope with a null id', async () => { - await createTestingModule([DomainModule], async (app) => { - const importService = app.get(ImportService); - const now = Date.now(); - const date = importService.messageTime({ id: null }); - expect(date.getTime()).toBeGreaterThanOrEqual(now); - }); - }); - it('should cope with no id', async () => { - await createTestingModule([DomainModule], async (app) => { - const importService = app.get(ImportService); - const now = Date.now(); - const date = importService.messageTime({}); - expect(date.getTime()).toBeGreaterThanOrEqual(now); - }); - }); -}); - describe('receiveMessages', () => { it('should call importwithfilter when a message is received', async () => { await createTestingModule([DomainModule], async (app) => { diff --git a/src/domain/services/import.service.ts b/src/domain/services/import.service.ts index e04bb32..f906f43 100644 --- a/src/domain/services/import.service.ts +++ b/src/domain/services/import.service.ts @@ -12,6 +12,7 @@ import { SettingsService } from './settings.service'; import sql from '../../db'; import { ReservedSql } from 'postgres'; import { SerializableParameter } from 'postgres'; +import { MessagesService } from './messages.service'; @Injectable() export class ImportService { @@ -22,6 +23,7 @@ export class ImportService { private readonly em: EntityManager, private readonly tagService: TagService, private readonly settings: SettingsService, + private readonly messages: MessagesService, ) {} // Lowish batch size seems to work best, probably due to the size of the product document @@ -368,11 +370,6 @@ export class ImportService { if (this.client && this.client.isOpen) await this.client.quit(); } - messageTime(message: any) { - const time = new Date(parseInt(message.id?.split('-')[0])); - return isNaN(time.getTime()) ? new Date() : time; - } - async receiveMessages() { const lastMessageId = await this.settings.getLastMessageId(); if (!this.client.isOpen) return; @@ -409,14 +406,7 @@ export class ImportService { diffs: "{\"fields\":{\"change\":[\"categories\"],\"delete\":[\"product_name\",\"product_name_es\"]}}", } */ - await sql`INSERT into product_update_event ${sql( - messages.map((m) => ({ - id: m.id, - updated_at: this.messageTime(m), - code: m.message.code, - message: m.message, - })), - )}`; + await this.messages.create(messages); const productCodes = messages.map((m) => m.message.code); const filter = { code: { $in: productCodes } }; await this.importWithFilter(filter, ProductSource.EVENT); diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts new file mode 100644 index 0000000..7a633c7 --- /dev/null +++ b/src/domain/services/messages.service.spec.ts @@ -0,0 +1,28 @@ +import { MessagesService } from './messages.service'; + +describe('messageTime', () => { + it('should return a date from a message id', () => { + const importService = new MessagesService(); + const time = Date.now() - 1000; + const date = importService.messageTime({ id: `${time}-0` }); + expect(date.getTime()).toBe(time); + }); + it('should return the current date for an invalid message id', () => { + const importService = new MessagesService(); + const now = Date.now(); + const date = importService.messageTime({ id: 'invalid' }); + expect(date.getTime()).toBeGreaterThanOrEqual(now); + }); + it('should cope with a null id', async () => { + const importService = new MessagesService(); + const now = Date.now(); + const date = importService.messageTime({ id: null }); + expect(date.getTime()).toBeGreaterThanOrEqual(now); + }); + it('should cope with no id', async () => { + const importService = new MessagesService(); + const now = Date.now(); + const date = importService.messageTime({}); + expect(date.getTime()).toBeGreaterThanOrEqual(now); + }); +}); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts new file mode 100644 index 0000000..dfe6b15 --- /dev/null +++ b/src/domain/services/messages.service.ts @@ -0,0 +1,21 @@ +import { Injectable } from '@nestjs/common'; +import sql from '../../db'; + +@Injectable() +export class MessagesService { + messageTime(message: any) { + const time = new Date(parseInt(message.id?.split('-')[0])); + return isNaN(time.getTime()) ? new Date() : time; + } + + async create(messages: any[]) { + await sql`INSERT into product_update_event ${sql( + messages.map((m) => ({ + id: m.id, + updated_at: this.messageTime(m), + code: m.message.code, + message: m.message, + })), + )}`; + } +} diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts new file mode 100644 index 0000000..a981312 --- /dev/null +++ b/src/domain/services/views.spec.ts @@ -0,0 +1,69 @@ +import { randomCode } from '../../../test/test.helper'; +import sql from '../../db'; +import { MessagesService } from './messages.service'; + +describe('product_updates_view', () => { + it('should aggregate events by count and distinct products', async () => { + // Create some products + const code1 = randomCode(); + const code2 = randomCode(); + const owner1 = randomCode(); + + await sql`INSERT INTO product ${sql([ + { + code: code1, + owners_tags: owner1, + }, + { + code: code2, + owners_tags: owner1, + }, + ])}`; + + // Create some messages + const messages = new MessagesService(); + + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + await messages.create([ + { + id: nextId(), + message: { + code: code1, + }, + }, + { + id: nextId(), + message: { + code: code1, + }, + }, + { + id: nextId(), + message: { + code: code1, + }, + }, + { + id: nextId(), + message: { + code: code2, + }, + }, + ]); + + const results = await sql`SELECT + DATE_TRUNC('day', pe.updated_at) updated_day, + p.owners_tags, + count(*) update_count, + count(DISTINCT pe.code) product_count + FROM product_update_event pe + LEFT JOIN product p on p.code = pe.code + GROUP BY DATE_TRUNC('day', pe.updated_at), + p.owners_tags`; + + const myResult = results.find((r) => r.owners_tags === owner1); + expect(myResult.update_count).toBe('4'); + expect(myResult.product_count).toBe('2'); + }); +}); diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 364e198..25337ec 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -1,8 +1,10 @@ /* eslint-disable prettier/prettier */ import { Migration } from '@mikro-orm/migrations'; +import { VIEW_PASSWORD, VIEW_USER } from '../constants'; export class Migration20240719101700Redis extends Migration { async up(): Promise { this.addSql(`CREATE TABLE IF NOT EXISTS query.product_update_event (id text NOT NULL PRIMARY KEY, updated_at timestamptz NOT NULL, code text NULL, message jsonb NOT NULL)`); + this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); } } From 32d41fc270ae82df85d20d87bf55b7e9a116a699 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 13:00:35 +0100 Subject: [PATCH 03/31] Failing test using view only user Signed-off-by: John Gomersall --- src/domain/services/views.spec.ts | 21 +++++++++++-------- .../Migration20240719101700-redis.ts | 9 ++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index a981312..4fbe93c 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -1,6 +1,8 @@ +import postgres from 'postgres'; import { randomCode } from '../../../test/test.helper'; import sql from '../../db'; import { MessagesService } from './messages.service'; +import { VIEW_PASSWORD, VIEW_USER } from '../../constants'; describe('product_updates_view', () => { it('should aggregate events by count and distinct products', async () => { @@ -52,15 +54,16 @@ describe('product_updates_view', () => { }, ]); - const results = await sql`SELECT - DATE_TRUNC('day', pe.updated_at) updated_day, - p.owners_tags, - count(*) update_count, - count(DISTINCT pe.code) product_count - FROM product_update_event pe - LEFT JOIN product p on p.code = pe.code - GROUP BY DATE_TRUNC('day', pe.updated_at), - p.owners_tags`; + // Use viewer user + const viewer = postgres({ + host: process.env.POSTGRES_HOST, + database: process.env.POSTGRES_DB, + user: VIEW_USER, + password: VIEW_PASSWORD, + port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), + }); + + const results = await viewer`SELECT * from product_update_view`; const myResult = results.find((r) => r.owners_tags === owner1); expect(myResult.update_count).toBe('4'); diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 25337ec..0cee88e 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -6,5 +6,14 @@ export class Migration20240719101700Redis extends Migration { async up(): Promise { this.addSql(`CREATE TABLE IF NOT EXISTS query.product_update_event (id text NOT NULL PRIMARY KEY, updated_at timestamptz NOT NULL, code text NULL, message jsonb NOT NULL)`); this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); + this.addSql(`CREATE OR REPLACE VIEW product_update_view AS SELECT + DATE_TRUNC('day', pe.updated_at) updated_day, + p.owners_tags, + count(*) update_count, + count(DISTINCT pe.code) product_count + FROM product_update_event pe + LEFT JOIN product p on p.code = pe.code + GROUP BY DATE_TRUNC('day', pe.updated_at), + p.owners_tags`); } } From 12ecb0218b2632ff3d52e659fce2b612c3008591 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 13:25:46 +0100 Subject: [PATCH 04/31] Don't update tags if no products updated Signed-off-by: John Gomersall --- src/domain/services/import.service.ts | 135 +++++++++--------- .../Migration20240719101700-redis.ts | 5 +- 2 files changed, 73 insertions(+), 67 deletions(-) diff --git a/src/domain/services/import.service.ts b/src/domain/services/import.service.ts index f906f43..d7c9c96 100644 --- a/src/domain/services/import.service.ts +++ b/src/domain/services/import.service.ts @@ -230,45 +230,14 @@ export class ImportService { WHERE product.id = tp.id`; this.logger.debug(`Updated ${productResults.count} products`); - // Fix ingredients - let logText = `Updated ingredients`; - const deleted = await connection`delete from product_ingredient - where product_id in (select id from product_temp)`; - logText += ` deleted ${deleted.count},`; - const results = await connection`insert into product_ingredient ( - product_id, - sequence, - id, - ciqual_food_code, - ingredient_text, - percent, - percent_min, - percent_max, - percent_estimate, - data, - obsolete - ) - select - product.id, - ordinality, - tag.value->>'id', - tag.value->>'ciqual_food_code', - tag.value->>'ingredient_text', - tag.value->>'percent', - (tag.value->>'percent_min')::numeric, - (tag.value->>'percent_max')::numeric, - (tag.value->>'percent_estimate')::numeric, - tag.value->'ingredients', - ${obsolete} - from product_temp product - cross join jsonb_array_elements(data->'ingredients') with ordinality tag`; - let affectedRows = results.count; - logText += ` inserted ${affectedRows}`; - while (affectedRows > 0) { + if (productResults.count) { + // Fix ingredients + let logText = `Updated ingredients`; + const deleted = await connection`delete from product_ingredient + where product_id in (select id from product_temp)`; + logText += ` deleted ${deleted.count},`; const results = await connection`insert into product_ingredient ( product_id, - parent_product_id, - parent_sequence, sequence, id, ciqual_food_code, @@ -281,10 +250,8 @@ export class ImportService { obsolete ) select - pi.product_id, - pi.product_id, - pi.sequence, - pi.sequence || '.' || ordinality, + product.id, + ordinality, tag.value->>'id', tag.value->>'ciqual_food_code', tag.value->>'ingredient_text', @@ -294,37 +261,73 @@ export class ImportService { (tag.value->>'percent_estimate')::numeric, tag.value->'ingredients', ${obsolete} - from product_ingredient pi - join product_temp product on product.id = pi.product_id - cross join json_array_elements(pi.data) with ordinality tag - WHERE pi.data IS NOT NULL - AND NOT EXISTS (SELECT * FROM product_ingredient pi2 WHERE pi2.parent_product_id = pi.product_id AND pi2.parent_sequence = pi.sequence)`; - affectedRows = results.count; - logText += ` > ${affectedRows}`; - } - this.logger.debug(logText + ' rows'); + from product_temp product + cross join jsonb_array_elements(data->'ingredients') with ordinality tag`; + let affectedRows = results.count; + logText += ` inserted ${affectedRows}`; + while (affectedRows > 0) { + const results = await connection`insert into product_ingredient ( + product_id, + parent_product_id, + parent_sequence, + sequence, + id, + ciqual_food_code, + ingredient_text, + percent, + percent_min, + percent_max, + percent_estimate, + data, + obsolete + ) + select + pi.product_id, + pi.product_id, + pi.sequence, + pi.sequence || '.' || ordinality, + tag.value->>'id', + tag.value->>'ciqual_food_code', + tag.value->>'ingredient_text', + tag.value->>'percent', + (tag.value->>'percent_min')::numeric, + (tag.value->>'percent_max')::numeric, + (tag.value->>'percent_estimate')::numeric, + tag.value->'ingredients', + ${obsolete} + from product_ingredient pi + join product_temp product on product.id = pi.product_id + cross join json_array_elements(pi.data) with ordinality tag + WHERE pi.data IS NOT NULL + AND NOT EXISTS (SELECT * FROM product_ingredient pi2 WHERE pi2.parent_product_id = pi.product_id AND pi2.parent_sequence = pi.sequence)`; + affectedRows = results.count; + logText += ` > ${affectedRows}`; + } + this.logger.debug(logText + ' rows'); - for (const [tag, entity] of Object.entries(ProductTagMap.MAPPED_TAGS)) { - let logText = `Updated ${tag}`; - // Get the underlying table name for the entity - const tableName = this.em.getMetadata(entity).tableName; + for (const [tag, entity] of Object.entries(ProductTagMap.MAPPED_TAGS)) { + let logText = `Updated ${tag}`; + // Get the underlying table name for the entity + const tableName = this.em.getMetadata(entity).tableName; - // Delete existing tags for products that were imported on this run - const deleted = await connection`delete from ${sql(tableName)} - where product_id in (select id from product_temp)`; - logText += ` deleted ${deleted.count},`; + // Delete existing tags for products that were imported on this run + const deleted = await connection`delete from ${sql(tableName)} + where product_id in (select id from product_temp)`; + logText += ` deleted ${deleted.count},`; - // Add tags back in with the updated information - const results = await connection`insert into ${sql( - tableName, - )} (product_id, value, obsolete) - select DISTINCT id, tag.value, ${obsolete} from product_temp - cross join jsonb_array_elements_text(data->'${sql.unsafe(tag)}') tag`; + // Add tags back in with the updated information + const results = await connection`insert into ${sql( + tableName, + )} (product_id, value, obsolete) + select DISTINCT id, tag.value, ${obsolete} from product_temp + cross join jsonb_array_elements_text(data->'${sql.unsafe(tag)}') tag`; - logText += ` inserted ${results.count} rows`; + logText += ` inserted ${results.count} rows`; - this.logger.debug(logText); + this.logger.debug(logText); + } } + await connection`truncate table product_temp`; await connection`commit`; } diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 0cee88e..63bd8ff 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -1,11 +1,13 @@ /* eslint-disable prettier/prettier */ import { Migration } from '@mikro-orm/migrations'; -import { VIEW_PASSWORD, VIEW_USER } from '../constants'; +import { SCHEMA, VIEW_PASSWORD, VIEW_USER } from '../constants'; export class Migration20240719101700Redis extends Migration { async up(): Promise { this.addSql(`CREATE TABLE IF NOT EXISTS query.product_update_event (id text NOT NULL PRIMARY KEY, updated_at timestamptz NOT NULL, code text NULL, message jsonb NOT NULL)`); this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); + this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=${SCHEMA},public`, + ); this.addSql(`CREATE OR REPLACE VIEW product_update_view AS SELECT DATE_TRUNC('day', pe.updated_at) updated_day, p.owners_tags, @@ -15,5 +17,6 @@ export class Migration20240719101700Redis extends Migration { LEFT JOIN product p on p.code = pe.code GROUP BY DATE_TRUNC('day', pe.updated_at), p.owners_tags`); + this.addSql(`GRANT SELECT ON product_update_view TO ${VIEW_USER}`); } } From e47d484b0c686993e59ca502797203e51be87222 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 15:57:34 +0100 Subject: [PATCH 05/31] Make sure dotenv is imported before postgres is initialised Signed-off-by: John Gomersall --- package-lock.json | 20 ++++++++++++++++---- package.json | 1 + src/main.ts | 3 +++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0173bf2..28619fd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,6 +19,7 @@ "@nestjs/platform-express": "^9.0.0", "@nestjs/schedule": "^4.0.0", "@nestjs/terminus": "^10.1.1", + "dotenv": "^16.4.5", "fast-deep-equal": "^3.1.3", "id128": "^1.6.6", "mongodb": "^5.8.0", @@ -1584,6 +1585,17 @@ } } }, + "node_modules/@mikro-orm/core/node_modules/dotenv": { + "version": "16.3.1", + "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-16.3.1.tgz", + "integrity": "sha512-IPzF4w4/Rd94bA9imS68tZBaYyBWSCE47V1RGuMrB94iyTOIEwRmVL2x/4An+6mETpLrKJ5hQkB8W4kFAadeIQ==", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/motdotla/dotenv?sponsor=1" + } + }, "node_modules/@mikro-orm/core/node_modules/fs-extra": { "version": "11.1.1", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-11.1.1.tgz", @@ -4407,14 +4419,14 @@ } }, "node_modules/dotenv": { - "version": "16.3.1", - "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-16.3.1.tgz", - "integrity": "sha512-IPzF4w4/Rd94bA9imS68tZBaYyBWSCE47V1RGuMrB94iyTOIEwRmVL2x/4An+6mETpLrKJ5hQkB8W4kFAadeIQ==", + "version": "16.4.5", + "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-16.4.5.tgz", + "integrity": "sha512-ZmdL2rui+eB2YwhsWzjInR8LldtZHGDoQ1ugH85ppHKwpUHL7j7rN0Ti9NCnGiQbhaZ11FpR+7ao1dNsmduNUg==", "engines": { "node": ">=12" }, "funding": { - "url": "https://github.com/motdotla/dotenv?sponsor=1" + "url": "https://dotenvx.com" } }, "node_modules/ee-first": { diff --git a/package.json b/package.json index 020b204..81a086e 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "@nestjs/platform-express": "^9.0.0", "@nestjs/schedule": "^4.0.0", "@nestjs/terminus": "^10.1.1", + "dotenv": "^16.4.5", "fast-deep-equal": "^3.1.3", "id128": "^1.6.6", "mongodb": "^5.8.0", diff --git a/src/main.ts b/src/main.ts index 697349f..6da5b6e 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,3 +1,6 @@ +import { configDotenv } from 'dotenv'; +configDotenv(); + import { NestFactory } from '@nestjs/core'; import { AppModule } from './app.module'; import { MikroORM } from '@mikro-orm/core'; From 23efaa155106192179834907299711bd1d443e59 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 15:59:39 +0100 Subject: [PATCH 06/31] Need schema permission to select from view Signed-off-by: John Gomersall --- src/domain/services/import.service.ts | 16 ++++++++-------- src/migrations/Migration20240719101700-redis.ts | 2 ++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/domain/services/import.service.ts b/src/domain/services/import.service.ts index d7c9c96..49c7d77 100644 --- a/src/domain/services/import.service.ts +++ b/src/domain/services/import.service.ts @@ -41,6 +41,8 @@ export class ImportService { return; } this.importRunning = true; + const source = + from != null ? ProductSource.INCREMENTAL_LOAD : ProductSource.FULL_LOAD; try { // If the from parameter is supplied but it is empty then obtain the most // recent modified time from the database and query MongoDB for products @@ -55,11 +57,7 @@ export class ImportService { this.logger.debug(`Starting import from ${from}`); } - const latestModified = await this.importWithFilter( - filter, - from ? ProductSource.INCREMENTAL_LOAD : ProductSource.FULL_LOAD, - skip, - ); + const latestModified = await this.importWithFilter(filter, source, skip); if (latestModified) { await this.settings.setLastModified(new Date(latestModified)); } @@ -122,6 +120,11 @@ export class ImportService { i++; if (skip && i < skip) continue; + + if (!(i % this.importLogInterval)) { + this.logger.debug(`Fetched ${i}`); + } + // Find the product if it exists let results = await connection`select id, last_modified from product where code = ${data.code}`; @@ -176,9 +179,6 @@ export class ImportService { await this.applyProductChange(connection, obsolete, source, updateId); await connection`begin`; } - if (!(i % this.importLogInterval)) { - this.logger.debug(`Updated ${i}`); - } } await this.applyProductChange(connection, obsolete, source, updateId); await cursor.close(); diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 63bd8ff..ea4f77f 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -8,6 +8,8 @@ export class Migration20240719101700Redis extends Migration { this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=${SCHEMA},public`, ); + this.addSql(`GRANT USAGE ON SCHEMA ${SCHEMA} TO ${VIEW_USER}`); + this.addSql(`CREATE OR REPLACE VIEW product_update_view AS SELECT DATE_TRUNC('day', pe.updated_at) updated_day, p.owners_tags, From 33632ee82fb023585ac143d4e351f97e2cf43aea Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 16:09:08 +0100 Subject: [PATCH 07/31] Ignore duplicate events Signed-off-by: John Gomersall --- src/domain/services/messages.service.spec.ts | 27 ++++++++++++++++++++ src/domain/services/messages.service.ts | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 7a633c7..e8f619a 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -1,3 +1,5 @@ +import { randomCode } from '../../../test/test.helper'; +import sql from '../../db'; import { MessagesService } from './messages.service'; describe('messageTime', () => { @@ -26,3 +28,28 @@ describe('messageTime', () => { expect(date.getTime()).toBeGreaterThanOrEqual(now); }); }); + +describe('create', () => { + it('should ignore duplicate events', async () => { + const code1 = randomCode(); + const messages = new MessagesService(); + await messages.create([ + { + id: '1-0', + message: { + code: code1, + }, + }, + { + id: '1-0', + message: { + code: code1, + }, + }, + ]); + + const result = + await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; + expect(result).toHaveLength(1); + }); +}); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index dfe6b15..f436a08 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -16,6 +16,6 @@ export class MessagesService { code: m.message.code, message: m.message, })), - )}`; + )} ON CONFLICT DO NOTHING`; } } From b39dc7302bf77cbda59a14a88940563e09e02931 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 17:15:39 +0100 Subject: [PATCH 08/31] Don't do product updates for initial_import events Signed-off-by: John Gomersall --- src/domain/services/import.service.spec.ts | 115 +++++++++++++++++++++ src/domain/services/import.service.ts | 18 ++-- 2 files changed, 126 insertions(+), 7 deletions(-) diff --git a/src/domain/services/import.service.spec.ts b/src/domain/services/import.service.spec.ts index a142c9d..8b1f77a 100644 --- a/src/domain/services/import.service.spec.ts +++ b/src/domain/services/import.service.spec.ts @@ -469,3 +469,118 @@ describe('receiveMessages', () => { }); }); }); + +describe('receiveMessages', () => { + it('should call importwithfilter when a message is received', async () => { + await createTestingModule([DomainModule], async (app) => { + // GIVEN: Redis is running + const redis = await new GenericContainer('redis') + .withExposedPorts(6379) + .start(); + const redisUrl = `redis://localhost:${redis.getMappedPort(6379)}`; + const settings = app.get(SettingsService); + jest.spyOn(settings, 'getRedisUrl').mockImplementation(() => redisUrl); + + // And lastmessageid is zero + await settings.setLastMessageId('0'); + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + await importService.startRedisConsumer(); + + const client = createClient({ url: redisUrl }); + await client.connect(); + try { + const code1 = randomCode(); + const code2 = randomCode(); + + // When: A message is sent + const messageId = await client.xAdd('product_updates_off', '*', { + code: code1, + }); + + // Wait for message to be delivered + await setTimeout(100); + + // Then the import is called + expect(importSpy).toHaveBeenCalledTimes(1); + expect(await settings.getLastMessageId()).toBe(messageId); + + // If a new message is added + importSpy.mockClear(); + await client.xAdd('product_updates_off', '*', { + code: code2, + }); + + // Wait for message to be delivered + await setTimeout(100); + + // Then import is called again but only with the new code + expect(importSpy).toHaveBeenCalledTimes(1); + const codes = importSpy.mock.calls[0][0].code.$in; + expect(codes).toHaveLength(1); + expect(codes[0]).toBe(code2); + + // Update events are created + const events = + await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; + + expect(events).toHaveLength(1); + expect(events[0].id).toBe(messageId); + } finally { + await client.quit(); + await importService.stopRedisConsumer(); + await redis.stop(); + } + }); + }); +}); + +describe('processMessages', () => { + it('should not call importwithfilter when a message contains the initial_import diff', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + + const code1 = randomCode(); + const code2 = randomCode(); + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + const messages = [ + { + id: nextId(), + message: { + code: code1, + }, + }, + { + id: nextId(), + message: { + code: code2, + diffs: { + initial_import: 1, + }, + }, + }, + ]; + + await importService.processMessages(messages); + // Then the import is called only once for code1 + const codes = importSpy.mock.calls[0][0].code.$in; + expect(codes).toHaveLength(1); + expect(codes[0]).toBe(code1); + + // Update events are created for all codes + const events = + await sql`SELECT * FROM product_update_event WHERE code IN ${sql([ + code1, + code2, + ])}`; + + expect(events).toHaveLength(2); + }); + }); +}); diff --git a/src/domain/services/import.service.ts b/src/domain/services/import.service.ts index 49c7d77..c702149 100644 --- a/src/domain/services/import.service.ts +++ b/src/domain/services/import.service.ts @@ -409,13 +409,7 @@ export class ImportService { diffs: "{\"fields\":{\"change\":[\"categories\"],\"delete\":[\"product_name\",\"product_name_es\"]}}", } */ - await this.messages.create(messages); - const productCodes = messages.map((m) => m.message.code); - const filter = { code: { $in: productCodes } }; - await this.importWithFilter(filter, ProductSource.EVENT); - await this.settings.setLastMessageId( - messages[messages.length - 1].id, - ); + await this.processMessages(messages); } } setTimeout(() => { @@ -423,4 +417,14 @@ export class ImportService { }, 0); }); } + + async processMessages(messages: any[]) { + await this.messages.create(messages); + const productCodes = messages + .filter((m) => !m.message.diffs?.initial_import) // Don't trigger product updates on initial import + .map((m) => m.message.code); + const filter = { code: { $in: productCodes } }; + await this.importWithFilter(filter, ProductSource.EVENT); + await this.settings.setLastMessageId(messages[messages.length - 1].id); + } } From 87f5fec275361fd35fa2a4b159f870b219996428 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 17:26:18 +0100 Subject: [PATCH 09/31] Use message timestamp if provided Signed-off-by: John Gomersall --- src/domain/services/messages.service.spec.ts | 6 ++++++ src/domain/services/messages.service.ts | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index e8f619a..0f70f87 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -27,6 +27,12 @@ describe('messageTime', () => { const date = importService.messageTime({}); expect(date.getTime()).toBeGreaterThanOrEqual(now); }); + it('should use timestamp if provided', async () => { + const importService = new MessagesService(); + const time = Math.trunc((Date.now() - 1000) / 1000); + const date = importService.messageTime({ timestamp: time }); + expect(date.getTime()).toBe(time * 1000); + }); }); describe('create', () => { diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index f436a08..14cf466 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -4,7 +4,13 @@ import sql from '../../db'; @Injectable() export class MessagesService { messageTime(message: any) { - const time = new Date(parseInt(message.id?.split('-')[0])); + // First preference is to use timestamp in the message + let time = new Date(parseInt(message.timestamp) * 1000); + // Otherwise derive from message id + time = isNaN(time.getTime()) + ? new Date(parseInt(message.id?.split('-')[0])) + : time; + // Or use today's date/time if that doesn't work return isNaN(time.getTime()) ? new Date() : time; } From 8679f10f70f801860630dbde8fd0536af9479891 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 19 Jul 2024 17:28:27 +0100 Subject: [PATCH 10/31] Remove plural from owner_tag Signed-off-by: John Gomersall --- src/domain/services/views.spec.ts | 2 +- src/migrations/Migration20240719101700-redis.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index 4fbe93c..47e1d10 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -65,7 +65,7 @@ describe('product_updates_view', () => { const results = await viewer`SELECT * from product_update_view`; - const myResult = results.find((r) => r.owners_tags === owner1); + const myResult = results.find((r) => r.owner_tag === owner1); expect(myResult.update_count).toBe('4'); expect(myResult.product_count).toBe('2'); }); diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index ea4f77f..c944bda 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -12,7 +12,7 @@ export class Migration20240719101700Redis extends Migration { this.addSql(`CREATE OR REPLACE VIEW product_update_view AS SELECT DATE_TRUNC('day', pe.updated_at) updated_day, - p.owners_tags, + p.owners_tags owner_tag, count(*) update_count, count(DISTINCT pe.code) product_count FROM product_update_event pe From ec779100c926ce58ef98058fcf716755efa26f0e Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Sat, 20 Jul 2024 13:02:57 +0100 Subject: [PATCH 11/31] Cope with \u0000 characters and parse diffs Signed-off-by: John Gomersall --- src/domain/services/import.service.spec.ts | 56 ++++++++++++++++++-- src/domain/services/import.service.ts | 24 +++++++-- src/domain/services/messages.service.spec.ts | 27 +++++++++- src/domain/services/messages.service.ts | 8 +++ 4 files changed, 105 insertions(+), 10 deletions(-) diff --git a/src/domain/services/import.service.spec.ts b/src/domain/services/import.service.spec.ts index 8b1f77a..461e076 100644 --- a/src/domain/services/import.service.spec.ts +++ b/src/domain/services/import.service.spec.ts @@ -538,7 +538,7 @@ describe('receiveMessages', () => { }); describe('processMessages', () => { - it('should not call importwithfilter when a message contains the initial_import diff', async () => { + it('should not call importwithfilter for messages that contain the initial_import diff', async () => { await createTestingModule([DomainModule], async (app) => { const importService = app.get(ImportService); const importSpy = jest @@ -560,9 +560,10 @@ describe('processMessages', () => { id: nextId(), message: { code: code2, - diffs: { + // Note JSON properties in Redis come in as strings + diffs: JSON.stringify({ initial_import: 1, - }, + }), }, }, ]; @@ -583,4 +584,53 @@ describe('processMessages', () => { expect(events).toHaveLength(2); }); }); + + it('should not call importwithfilter at all if all messages contain the initial_import diff', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + + const code1 = randomCode(); + const code2 = randomCode(); + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + const messages = [ + { + id: nextId(), + message: { + code: code1, + // Note JSON properties in Redis come in as strings + diffs: JSON.stringify({ + initial_import: 1, + }), + }, + }, + { + id: nextId(), + message: { + code: code2, + // Note JSON properties in Redis come in as strings + diffs: JSON.stringify({ + initial_import: 1, + }), + }, + }, + ]; + + await importService.processMessages(messages); + // Then the import is not called at all + expect(importSpy).toHaveBeenCalledTimes(0); + + // Update events are created for all codes + const events = + await sql`SELECT * FROM product_update_event WHERE code IN ${sql([ + code1, + code2, + ])}`; + + expect(events).toHaveLength(2); + }); + }); }); diff --git a/src/domain/services/import.service.ts b/src/domain/services/import.service.ts index c702149..5c0228d 100644 --- a/src/domain/services/import.service.ts +++ b/src/domain/services/import.service.ts @@ -419,12 +419,26 @@ export class ImportService { } async processMessages(messages: any[]) { + // Fix JSON properties on each message to be objects rather than strings + for (const event of messages) { + if (event.message.diffs) + event.message.diffs = JSON.parse(event.message.diffs); + } await this.messages.create(messages); - const productCodes = messages - .filter((m) => !m.message.diffs?.initial_import) // Don't trigger product updates on initial import - .map((m) => m.message.code); - const filter = { code: { $in: productCodes } }; - await this.importWithFilter(filter, ProductSource.EVENT); + const productCodes = [ + ...new Set( + messages + .filter((m) => !m.message.diffs?.initial_import) // Don't trigger product updates on initial import + .map((m) => m.message.code), + ), + ]; + this.logger.log( + `Received ${messages.length} events with ${productCodes.length} products to import`, + ); + if (productCodes.length) { + const filter = { code: { $in: productCodes } }; + await this.importWithFilter(filter, ProductSource.EVENT); + } await this.settings.setLastMessageId(messages[messages.length - 1].id); } } diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 0f70f87..8c00c0b 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -35,21 +35,43 @@ describe('messageTime', () => { }); }); +let idCount = 0; + describe('create', () => { it('should ignore duplicate events', async () => { const code1 = randomCode(); const messages = new MessagesService(); + const messageId = `${Date.now()}-${idCount++}`; + await messages.create([ { - id: '1-0', + id: messageId, message: { code: code1, }, }, { - id: '1-0', + id: messageId, + message: { + code: code1, + }, + }, + ]); + + const result = + await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; + expect(result).toHaveLength(1); + }); + + it('should cope with null characters', async () => { + const code1 = randomCode(); + const messages = new MessagesService(); + await messages.create([ + { + id: `${Date.now()}-${idCount++}`, message: { code: code1, + comment: 'test \u0000 test2 \u0000 end', }, }, ]); @@ -57,5 +79,6 @@ describe('create', () => { const result = await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; expect(result).toHaveLength(1); + expect(result[0].message.comment).toBe('test test2 end'); }); }); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index 14cf466..df504b3 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -1,6 +1,8 @@ import { Injectable } from '@nestjs/common'; import sql from '../../db'; +const nulRegex = /\\u0000/g; + @Injectable() export class MessagesService { messageTime(message: any) { @@ -15,6 +17,12 @@ export class MessagesService { } async create(messages: any[]) { + // Strip out any \u0000 characters as PostgresSQL can't cope with them + const messageJson = JSON.stringify(messages); + if (messageJson.includes('\\u0000')) { + messages = JSON.parse(messageJson.replace(nulRegex, '')); + } + await sql`INSERT into product_update_event ${sql( messages.map((m) => ({ id: m.id, From 599027678dc9499273878cc60cda625b725bba7d Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 26 Jul 2024 09:28:30 +0100 Subject: [PATCH 12/31] Fix getting timestamp from message Signed-off-by: John Gomersall --- src/domain/services/messages.service.spec.ts | 5 ++++- src/domain/services/messages.service.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 8c00c0b..3562f8d 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -30,7 +30,10 @@ describe('messageTime', () => { it('should use timestamp if provided', async () => { const importService = new MessagesService(); const time = Math.trunc((Date.now() - 1000) / 1000); - const date = importService.messageTime({ timestamp: time }); + const date = importService.messageTime({ + id: '100-0', + message: { timestamp: time }, + }); expect(date.getTime()).toBe(time * 1000); }); }); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index df504b3..793c5bd 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -7,7 +7,7 @@ const nulRegex = /\\u0000/g; export class MessagesService { messageTime(message: any) { // First preference is to use timestamp in the message - let time = new Date(parseInt(message.timestamp) * 1000); + let time = new Date(parseInt(message.message?.timestamp) * 1000); // Otherwise derive from message id time = isNaN(time.getTime()) ? new Date(parseInt(message.id?.split('-')[0])) From cba3c2eaed05032610eff3baab2de9afd39ebc1f Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 26 Jul 2024 16:55:23 +0100 Subject: [PATCH 13/31] Added a decision around how to aggregate Signed-off-by: John Gomersall --- docs/decisions/event-aggregation.md | 56 +++++++++++++++++++++++ docs/decisions/template.md | 69 +++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 docs/decisions/event-aggregation.md create mode 100644 docs/decisions/template.md diff --git a/docs/decisions/event-aggregation.md b/docs/decisions/event-aggregation.md new file mode 100644 index 0000000..44cea2e --- /dev/null +++ b/docs/decisions/event-aggregation.md @@ -0,0 +1,56 @@ +# Product Event Aggregation + +## Context and Problem Statement + +We would like to be able to support a variety of queries on events that have been recorded on products over time. For example, for the producer's dashboard we want to be able to show the number of edits and distinct products updated over a month. + +## Decision Drivers + +* Queries should run quickly +* Database space consumed should not be excessive +* Data should be reasonably up to date, i.e. any ETL / ELT process should keep up with the rate at which events are being created + +## Considered Options + +* Query the raw event tables +* Create specific aggregate tables for each aggregate dimension +* Create a relational model of events against products + +## Decision Outcome + +Chosen option: "Create a relational model of events against products", because it offers the best compromise in terms of acceptable query performance with minimal storage space and does not require new tables to be created for every possible aggregate dimension. + +### Consequences + +In general we should try and map things to a relational model, but only at the most granular level of detail that makes sense, e.g. total count of actions in one day. + +It has been observed that PostgreSQL performs much better when dealing with small record sizes, so text fields should be normalised where possible so that an integer id can be stored instead. + +## Pros and Cons of the Options + +### Query the raw event tables + +In this option the raw events are simply loaded into a table and then views are created to query this table, joining to the product table to obtain the required dimension. + +* Good: Only the raw events are being stored +* Good: Import of data is as fast as possible +* Bad: Query performance is poor. Even with indexing typical queries were taking around 2 minutes + +### Create specific aggregate tables for each aggregate dimension + +With this option the raw events would be ingested and then a follow-up process would run to aggregate those events by the required dimension, e.g. for producer's dashboard this would be aggregating by day, action and owner with a total update count plus a count of distinct products updated. + +* Good: Queries run very quickly (sub 100ms) +* Bad: Additional tables, processes and storage need to be assigned for each new query dimension +* Bad: It is difficult to incrementally refresh tables where distinct counts are included (as cannot work out the new distinct count from the combination of new events plus existing distinct count) + +### Create a relational model of events against products + +With this option the raw events would be ingested and then a follow-up process would run to just aggregate those events by action, contributor and day against the product. Different views can then be provided to query this data, joining to the product to obtain the required dimension. + +With this option it was important to keep the size of the relational table as small as possible, so an enumeration was used for the action and the contributors were normalised into a separate table so that only the id needed to be stored in the event table. + +* Neutral: Queries performance is acceptable (sub 1s) +* Good: Queries to support different dimensions do not require addition storage or import processes +* Good: Aggregated counts are not distinct, so can be refreshed incrementally + diff --git a/docs/decisions/template.md b/docs/decisions/template.md new file mode 100644 index 0000000..77097eb --- /dev/null +++ b/docs/decisions/template.md @@ -0,0 +1,69 @@ +# {short title of solved problem and solution} + +## Context and Problem Statement + +{Describe the context and problem statement, e.g., in free form using two to three sentences or in the form of an illustrative story. + You may want to articulate the problem in form of a question and add links to collaboration boards or issue management systems.} + + +## Decision Drivers + +* {decision driver 1, e.g., a force, facing concern, …} +* {decision driver 2, e.g., a force, facing concern, …} +* … + +## Considered Options + +* {title of option 1} +* {title of option 2} +* {title of option 3} +* … + +## Decision Outcome + +Chosen option: "{title of option 1}", because +{justification. e.g., only option, which meets k.o. criterion decision driver | which resolves force {force} | … | comes out best (see below)}. + + +### Consequences + +{Provide detail on the implications of making this decision and how any forseen problems can be mitigated} + + +### Confirmation + +{Describe how the implementation of/compliance with the ADR is confirmed. E.g., by a review or an ArchUnit test. + Although we classify this element as optional, it is included in most ADRs.} + + +## Pros and Cons of the Options + +### {title of option 1} + + +{example | description | pointer to more information | …} + +* Good: {argument a} +* Good: {argument b} + +* Neutral: {argument c} +* Bad: {argument d} +* … + +### {title of other option} + +{example | description | pointer to more information | …} + +* Good: {argument a} +* Good: {argument b} +* Neutral: {argument c} +* Bad: {argument d} +* … + + +## More Information + +{You might want to provide additional evidence/confidence for the decision outcome here and/or + document the team agreement on the decision and/or + define when/how this decision the decision should be realized and if/when it should be re-visited. +Links to other decisions and resources might appear here as well.} \ No newline at end of file From 451160841f63364acc6031a3711c256b7440aca0 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 26 Jul 2024 18:03:56 +0100 Subject: [PATCH 14/31] Refactor redis into a separate class Signed-off-by: John Gomersall --- src/app.controller.ts | 7 +- src/domain/domain.module.ts | 20 +- src/domain/services/import.service.spec.ts | 242 ------------------ src/domain/services/import.service.ts | 113 +------- src/domain/services/messages.service.spec.ts | 124 +++++---- src/domain/services/messages.service.ts | 73 +++++- src/domain/services/redis.listener.spec.ts | 180 +++++++++++++ src/domain/services/redis.listener.ts | 94 +++++++ src/domain/services/views.spec.ts | 55 +++- .../Migration20240719101700-redis.ts | 38 ++- 10 files changed, 532 insertions(+), 414 deletions(-) create mode 100644 src/domain/services/redis.listener.spec.ts create mode 100644 src/domain/services/redis.listener.ts diff --git a/src/app.controller.ts b/src/app.controller.ts index 6288cc2..69f9e8f 100644 --- a/src/app.controller.ts +++ b/src/app.controller.ts @@ -1,12 +1,14 @@ import { Body, Controller, Get, Post, Query, All } from '@nestjs/common'; import { ImportService } from './domain/services/import.service'; import { QueryService } from './domain/services/query.service'; +import { RedisListener } from './domain/services/redis.listener'; @Controller() export class AppController { constructor( private readonly importService: ImportService, private readonly queryService: QueryService, + private readonly redisListener: RedisListener, ) {} @Get('importfrommongo') @@ -19,7 +21,10 @@ export class AppController { @Get('scheduledimportfrommongo') async scheduledImportFromMongo() { - await this.importService.scheduledImportFromMongo(); + // Pause redis while doing a scheduled import + await this.redisListener.pauseAndRun( + this.importService.scheduledImportFromMongo, + ); } parseBoolean(value) { diff --git a/src/domain/domain.module.ts b/src/domain/domain.module.ts index 3ef5e9a..a8224da 100644 --- a/src/domain/domain.module.ts +++ b/src/domain/domain.module.ts @@ -7,6 +7,7 @@ import { SettingsService } from './services/settings.service'; import { EntityManager, RequestContext } from '@mikro-orm/core'; import { Cron, ScheduleModule } from '@nestjs/schedule'; import { MessagesService } from './services/messages.service'; +import { RedisListener } from './services/redis.listener'; @Module({ imports: [MikroOrmModule.forRoot(), ScheduleModule.forRoot()], @@ -16,23 +17,32 @@ import { MessagesService } from './services/messages.service'; TagService, SettingsService, MessagesService, + RedisListener, + ], + exports: [ + ImportService, + QueryService, + TagService, + SettingsService, + RedisListener, + MessagesService, ], - exports: [ImportService, QueryService, TagService, SettingsService], }) export class DomainModule implements OnModuleInit, OnModuleDestroy { constructor( private readonly em: EntityManager, private readonly importService: ImportService, + private readonly redisListener: RedisListener, ) {} async onModuleInit() { RequestContext.create(this.em, () => { - this.importService.startRedisConsumer(); + this.redisListener.startRedisConsumer(); }); } async onModuleDestroy() { - await this.importService.stopRedisConsumer(); + await this.redisListener.stopRedisConsumer(); } // Refresh the PostgreSQL database from MongoDB at 2am every night @@ -42,7 +52,9 @@ export class DomainModule implements OnModuleInit, OnModuleDestroy { // The request context creates a separate entity manager instance // which avoids clashes with other requests await RequestContext.createAsync(this.em, async () => { - await this.importService.scheduledImportFromMongo(); + await this.redisListener.pauseAndRun( + this.importService.scheduledImportFromMongo, + ); }); } } diff --git a/src/domain/services/import.service.spec.ts b/src/domain/services/import.service.spec.ts index 461e076..5417661 100644 --- a/src/domain/services/import.service.spec.ts +++ b/src/domain/services/import.service.spec.ts @@ -9,11 +9,7 @@ import { LoadedTag } from '../entities/loaded-tag'; import { ProductTagMap } from '../entities/product-tag-map'; import { ProductSource } from '../enums/product-source'; import { SettingsService } from './settings.service'; -import { createClient } from 'redis'; -import { GenericContainer } from 'testcontainers'; -import { setTimeout } from 'timers/promises'; import { ProductIngredient } from '../entities/product-ingredient'; -import sql from '../../db'; const lastModified = 1692032161; @@ -336,8 +332,6 @@ describe('scheduledImportFromMongo', () => { it('should do a full import if loaded tags arent complete', async () => { await createTestingModule([DomainModule], async (app) => { const importService = app.get(ImportService); - const redisStart = jest.spyOn(importService, 'startRedisConsumer'); - const redisStop = jest.spyOn(importService, 'stopRedisConsumer'); jest .spyOn(app.get(TagService), 'getLoadedTags') .mockImplementation(async () => []); @@ -347,10 +341,6 @@ describe('scheduledImportFromMongo', () => { await importService.scheduledImportFromMongo(); expect(importSpy).toHaveBeenCalledTimes(1); expect(importSpy.mock.calls[0][0]).toBeUndefined(); - - // Should pause redis during import - expect(redisStop).toHaveBeenCalled(); - expect(redisStart).toHaveBeenCalled(); }); }); @@ -402,235 +392,3 @@ describe('importWithFilter', () => { }); }); }); - -describe('receiveMessages', () => { - it('should call importwithfilter when a message is received', async () => { - await createTestingModule([DomainModule], async (app) => { - // GIVEN: Redis is running - const redis = await new GenericContainer('redis') - .withExposedPorts(6379) - .start(); - const redisUrl = `redis://localhost:${redis.getMappedPort(6379)}`; - const settings = app.get(SettingsService); - jest.spyOn(settings, 'getRedisUrl').mockImplementation(() => redisUrl); - - // And lastmessageid is zero - await settings.setLastMessageId('0'); - const importService = app.get(ImportService); - const importSpy = jest - .spyOn(importService, 'importWithFilter') - .mockImplementation(); - await importService.startRedisConsumer(); - - const client = createClient({ url: redisUrl }); - await client.connect(); - try { - const code1 = randomCode(); - const code2 = randomCode(); - - // When: A message is sent - const messageId = await client.xAdd('product_updates_off', '*', { - code: code1, - }); - - // Wait for message to be delivered - await setTimeout(100); - - // Then the import is called - expect(importSpy).toHaveBeenCalledTimes(1); - expect(await settings.getLastMessageId()).toBe(messageId); - - // If a new message is added - importSpy.mockClear(); - await client.xAdd('product_updates_off', '*', { - code: code2, - }); - - // Wait for message to be delivered - await setTimeout(100); - - // Then import is called again but only with the new code - expect(importSpy).toHaveBeenCalledTimes(1); - const codes = importSpy.mock.calls[0][0].code.$in; - expect(codes).toHaveLength(1); - expect(codes[0]).toBe(code2); - - // Update events are created - const events = - await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; - - expect(events).toHaveLength(1); - expect(events[0].id).toBe(messageId); - } finally { - await client.quit(); - await importService.stopRedisConsumer(); - await redis.stop(); - } - }); - }); -}); - -describe('receiveMessages', () => { - it('should call importwithfilter when a message is received', async () => { - await createTestingModule([DomainModule], async (app) => { - // GIVEN: Redis is running - const redis = await new GenericContainer('redis') - .withExposedPorts(6379) - .start(); - const redisUrl = `redis://localhost:${redis.getMappedPort(6379)}`; - const settings = app.get(SettingsService); - jest.spyOn(settings, 'getRedisUrl').mockImplementation(() => redisUrl); - - // And lastmessageid is zero - await settings.setLastMessageId('0'); - const importService = app.get(ImportService); - const importSpy = jest - .spyOn(importService, 'importWithFilter') - .mockImplementation(); - await importService.startRedisConsumer(); - - const client = createClient({ url: redisUrl }); - await client.connect(); - try { - const code1 = randomCode(); - const code2 = randomCode(); - - // When: A message is sent - const messageId = await client.xAdd('product_updates_off', '*', { - code: code1, - }); - - // Wait for message to be delivered - await setTimeout(100); - - // Then the import is called - expect(importSpy).toHaveBeenCalledTimes(1); - expect(await settings.getLastMessageId()).toBe(messageId); - - // If a new message is added - importSpy.mockClear(); - await client.xAdd('product_updates_off', '*', { - code: code2, - }); - - // Wait for message to be delivered - await setTimeout(100); - - // Then import is called again but only with the new code - expect(importSpy).toHaveBeenCalledTimes(1); - const codes = importSpy.mock.calls[0][0].code.$in; - expect(codes).toHaveLength(1); - expect(codes[0]).toBe(code2); - - // Update events are created - const events = - await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; - - expect(events).toHaveLength(1); - expect(events[0].id).toBe(messageId); - } finally { - await client.quit(); - await importService.stopRedisConsumer(); - await redis.stop(); - } - }); - }); -}); - -describe('processMessages', () => { - it('should not call importwithfilter for messages that contain the initial_import diff', async () => { - await createTestingModule([DomainModule], async (app) => { - const importService = app.get(ImportService); - const importSpy = jest - .spyOn(importService, 'importWithFilter') - .mockImplementation(); - - const code1 = randomCode(); - const code2 = randomCode(); - let idCount = 0; - const nextId = () => `${Date.now()}-${idCount++}`; - const messages = [ - { - id: nextId(), - message: { - code: code1, - }, - }, - { - id: nextId(), - message: { - code: code2, - // Note JSON properties in Redis come in as strings - diffs: JSON.stringify({ - initial_import: 1, - }), - }, - }, - ]; - - await importService.processMessages(messages); - // Then the import is called only once for code1 - const codes = importSpy.mock.calls[0][0].code.$in; - expect(codes).toHaveLength(1); - expect(codes[0]).toBe(code1); - - // Update events are created for all codes - const events = - await sql`SELECT * FROM product_update_event WHERE code IN ${sql([ - code1, - code2, - ])}`; - - expect(events).toHaveLength(2); - }); - }); - - it('should not call importwithfilter at all if all messages contain the initial_import diff', async () => { - await createTestingModule([DomainModule], async (app) => { - const importService = app.get(ImportService); - const importSpy = jest - .spyOn(importService, 'importWithFilter') - .mockImplementation(); - - const code1 = randomCode(); - const code2 = randomCode(); - let idCount = 0; - const nextId = () => `${Date.now()}-${idCount++}`; - const messages = [ - { - id: nextId(), - message: { - code: code1, - // Note JSON properties in Redis come in as strings - diffs: JSON.stringify({ - initial_import: 1, - }), - }, - }, - { - id: nextId(), - message: { - code: code2, - // Note JSON properties in Redis come in as strings - diffs: JSON.stringify({ - initial_import: 1, - }), - }, - }, - ]; - - await importService.processMessages(messages); - // Then the import is not called at all - expect(importSpy).toHaveBeenCalledTimes(0); - - // Update events are created for all codes - const events = - await sql`SELECT * FROM product_update_event WHERE code IN ${sql([ - code1, - code2, - ])}`; - - expect(events).toHaveLength(2); - }); - }); -}); diff --git a/src/domain/services/import.service.ts b/src/domain/services/import.service.ts index 5c0228d..77897fe 100644 --- a/src/domain/services/import.service.ts +++ b/src/domain/services/import.service.ts @@ -5,25 +5,21 @@ import { MongoClient } from 'mongodb'; import { EntityManager } from '@mikro-orm/postgresql'; import { TagService } from './tag.service'; import { ProductTagMap } from '../entities/product-tag-map'; -import { createClient, commandOptions } from 'redis'; import { ProductSource } from '../enums/product-source'; import equal from 'fast-deep-equal'; import { SettingsService } from './settings.service'; import sql from '../../db'; import { ReservedSql } from 'postgres'; import { SerializableParameter } from 'postgres'; -import { MessagesService } from './messages.service'; @Injectable() export class ImportService { private logger = new Logger(ImportService.name); - private client: any; // Don't strongly type here is it is really verbose constructor( private readonly em: EntityManager, private readonly tagService: TagService, private readonly settings: SettingsService, - private readonly messages: MessagesService, ) {} // Lowish batch size seems to work best, probably due to the size of the product document @@ -338,107 +334,18 @@ export class ImportService { this.logger.debug(`${deleted.count} Products deleted`); } + // Make sure to pause redis before calling this async scheduledImportFromMongo() { - // Pause redis while doing a scheduled import - await this.stopRedisConsumer(); - - try { - if ( - equal( - Object.keys(ProductTagMap.MAPPED_TAGS).sort(), - (await this.tagService.getLoadedTags()).sort(), - ) - ) { - // Do an incremental load if all tags are already loaded - await this.importFromMongo(''); - } else { - await this.importFromMongo(); - } - } finally { - // Resume redis after import - await this.startRedisConsumer(); - } - } - - async startRedisConsumer() { - const redisUrl = this.settings.getRedisUrl(); - if (!redisUrl) return; - this.client = createClient({ url: redisUrl }); - this.client.on('error', (err) => this.logger.error(err)); - await this.client.connect(); - this.receiveMessages(); - } - - async stopRedisConsumer() { - if (this.client && this.client.isOpen) await this.client.quit(); - } - - async receiveMessages() { - const lastMessageId = await this.settings.getLastMessageId(); - if (!this.client.isOpen) return; - this.client - .xRead( - commandOptions({ - isolated: true, - }), - [ - // XREAD can read from multiple streams, starting at a - // different ID for each... - { - key: 'product_updates_off', - id: lastMessageId, - }, - ], - { - // Read 1000 entry at a time, block for 5 seconds if there are none. - COUNT: 1000, - BLOCK: 5000, - }, + if ( + equal( + Object.keys(ProductTagMap.MAPPED_TAGS).sort(), + (await this.tagService.getLoadedTags()).sort(), ) - .then(async (keys) => { - if (keys?.length) { - const messages = keys[0].messages; - if (messages?.length) { - /** Message looks like this: - { - code: "0850026029062", - flavor: "off", - user_id: "stephane", - action: "updated", - comment: "Modification : Remove changes", - diffs: "{\"fields\":{\"change\":[\"categories\"],\"delete\":[\"product_name\",\"product_name_es\"]}}", - } - */ - await this.processMessages(messages); - } - } - setTimeout(() => { - this.receiveMessages(); - }, 0); - }); - } - - async processMessages(messages: any[]) { - // Fix JSON properties on each message to be objects rather than strings - for (const event of messages) { - if (event.message.diffs) - event.message.diffs = JSON.parse(event.message.diffs); - } - await this.messages.create(messages); - const productCodes = [ - ...new Set( - messages - .filter((m) => !m.message.diffs?.initial_import) // Don't trigger product updates on initial import - .map((m) => m.message.code), - ), - ]; - this.logger.log( - `Received ${messages.length} events with ${productCodes.length} products to import`, - ); - if (productCodes.length) { - const filter = { code: { $in: productCodes } }; - await this.importWithFilter(filter, ProductSource.EVENT); + ) { + // Do an incremental load if all tags are already loaded + await this.importFromMongo(''); + } else { + await this.importFromMongo(); } - await this.settings.setLastMessageId(messages[messages.length - 1].id); } } diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 3562f8d..8a369ad 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -1,36 +1,32 @@ -import { randomCode } from '../../../test/test.helper'; +import { createTestingModule, randomCode } from '../../../test/test.helper'; import sql from '../../db'; +import { DomainModule } from '../domain.module'; import { MessagesService } from './messages.service'; describe('messageTime', () => { it('should return a date from a message id', () => { - const importService = new MessagesService(); const time = Date.now() - 1000; - const date = importService.messageTime({ id: `${time}-0` }); + const date = MessagesService.messageTime({ id: `${time}-0` }); expect(date.getTime()).toBe(time); }); it('should return the current date for an invalid message id', () => { - const importService = new MessagesService(); const now = Date.now(); - const date = importService.messageTime({ id: 'invalid' }); + const date = MessagesService.messageTime({ id: 'invalid' }); expect(date.getTime()).toBeGreaterThanOrEqual(now); }); it('should cope with a null id', async () => { - const importService = new MessagesService(); const now = Date.now(); - const date = importService.messageTime({ id: null }); + const date = MessagesService.messageTime({ id: null }); expect(date.getTime()).toBeGreaterThanOrEqual(now); }); it('should cope with no id', async () => { - const importService = new MessagesService(); const now = Date.now(); - const date = importService.messageTime({}); + const date = MessagesService.messageTime({}); expect(date.getTime()).toBeGreaterThanOrEqual(now); }); it('should use timestamp if provided', async () => { - const importService = new MessagesService(); const time = Math.trunc((Date.now() - 1000) / 1000); - const date = importService.messageTime({ + const date = MessagesService.messageTime({ id: '100-0', message: { timestamp: time }, }); @@ -42,46 +38,88 @@ let idCount = 0; describe('create', () => { it('should ignore duplicate events', async () => { - const code1 = randomCode(); - const messages = new MessagesService(); - const messageId = `${Date.now()}-${idCount++}`; + await createTestingModule([DomainModule], async (app) => { + const messages = app.get(MessagesService); + const code1 = randomCode(); + const messageId = `${Date.now()}-${idCount++}`; - await messages.create([ - { - id: messageId, - message: { - code: code1, + await messages.create([ + { + id: messageId, + message: { + code: code1, + action: 'created', + }, }, - }, - { - id: messageId, - message: { - code: code1, + { + id: messageId, + message: { + code: code1, + action: 'created', + }, }, - }, - ]); + ]); - const result = - await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; - expect(result).toHaveLength(1); + const result = + await sql`SELECT * FROM product_update_event WHERE message->>'code' = ${code1}`; + expect(result).toHaveLength(1); + expect(result[0].message.action).toBe('created'); + }); }); it('should cope with null characters', async () => { - const code1 = randomCode(); - const messages = new MessagesService(); - await messages.create([ - { - id: `${Date.now()}-${idCount++}`, - message: { - code: code1, - comment: 'test \u0000 test2 \u0000 end', + await createTestingModule([DomainModule], async (app) => { + const messages = app.get(MessagesService); + const code1 = randomCode(); + await messages.create([ + { + id: `${Date.now()}-${idCount++}`, + message: { + code: code1, + comment: 'test \u0000 test2 \u0000 end', + }, + }, + ]); + + const result = + await sql`SELECT * FROM product_update_event WHERE message->>'code' = ${code1}`; + expect(result).toHaveLength(1); + expect(result[0].message.comment).toBe('test test2 end'); + }); + }); + + it('should create contributors', async () => { + await createTestingModule([DomainModule], async (app) => { + const messages = app.get(MessagesService); + const code1 = randomCode(); + const user1 = randomCode(); + const user2 = randomCode(); + + // Given and existing contributor record + sql`INSERT INTO contributor (code) VALUES(${user1})`; + + // When events are imported + await messages.create([ + { + id: `${Date.now()}-${idCount++}`, + message: { + code: code1, + user_id: user1, + action: 'created', + }, }, - }, - ]); + { + id: `${Date.now()}-${idCount++}`, + message: { + code: code1, + user_id: user2, + action: 'created', + }, + }, + ]); - const result = - await sql`SELECT * FROM product_update_event WHERE code = ${code1}`; - expect(result).toHaveLength(1); - expect(result[0].message.comment).toBe('test test2 end'); + const result = await sql`SELECT * FROM contributor WHERE code = ${user2}`; + expect(result).toHaveLength(1); + }); }); }); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index 793c5bd..18d41ef 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -1,11 +1,21 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import sql from '../../db'; +import { ImportService } from './import.service'; +import { SettingsService } from './settings.service'; +import { ProductSource } from '../enums/product-source'; const nulRegex = /\\u0000/g; @Injectable() export class MessagesService { - messageTime(message: any) { + private logger = new Logger(MessagesService.name); + + constructor( + private readonly importService: ImportService, + private readonly settings: SettingsService, + ) {} + + static messageTime(message: any) { // First preference is to use timestamp in the message let time = new Date(parseInt(message.message?.timestamp) * 1000); // Otherwise derive from message id @@ -26,10 +36,65 @@ export class MessagesService { await sql`INSERT into product_update_event ${sql( messages.map((m) => ({ id: m.id, - updated_at: this.messageTime(m), - code: m.message.code, + updated_at: MessagesService.messageTime(m), message: m.message, })), )} ON CONFLICT DO NOTHING`; + + const messageIds = messages.map((m) => m.id); + + const results = await sql`insert into contributor (code) + select distinct message->>'user_id' + from product_update_event + where id in ${sql(messageIds)} + on conflict (code) + do nothing`; + /* + const [minTime, maxTime] = messagesForInsert.reduce( + ([prevMin, prevMax], m) => [ + Math.min(prevMin, m.updated_at.getTime()), + Math.max(prevMax, m.updated_at.getTime()), + ], + [Infinity, -Infinity], + ); + + const minDate = new Date(minTime).toISOString().substring(0, 10); + const maxDate = new Date(maxTime + 86400000).toISOString().substring(0, 10); + + // TODO: When we upgrade to PostgreSQL 15 we can us a unique constraint to cover the nullable columns + // so won't need to expressions in the on conflict clause + await sql`INSERT INTO product_updates_by_owner + SELECT + date(pe.updated_at at time zone 'UTC') updated_day, + p.owners_tags owner_tag, + pe.action, + count(*) update_count, + count(DISTINCT pe.code) product_count + FROM product_update_event pe + LEFT JOIN product p on p.code = pe.code + WHERE pe.updated_at >= ${minDate} AND pe.updated_at < ${maxDate} + GROUP BY date(pe.updated_at at time zone 'UTC'), + p.owners_tags, + pe.action + on conflict (updated_date, coalesce(owner_tag, ''), coalesce(action, '')) + do update set + update_count = EXCLUDED.update_count, + product_count = EXCLUDED.product_count`; + */ + const productCodes = [ + ...new Set( + messages + .filter((m) => !m.message.diffs?.initial_import) // Don't trigger product updates on initial import + .map((m) => m.message.code), + ), + ]; + this.logger.log( + `Received ${messages.length} events with ${productCodes.length} products to import`, + ); + if (productCodes.length) { + const filter = { code: { $in: productCodes } }; + await this.importService.importWithFilter(filter, ProductSource.EVENT); + } + await this.settings.setLastMessageId(messages[messages.length - 1].id); } } diff --git a/src/domain/services/redis.listener.spec.ts b/src/domain/services/redis.listener.spec.ts new file mode 100644 index 0000000..d367730 --- /dev/null +++ b/src/domain/services/redis.listener.spec.ts @@ -0,0 +1,180 @@ +import { createClient } from 'redis'; +import { GenericContainer } from 'testcontainers'; +import { createTestingModule, randomCode } from '../../../test/test.helper'; +import sql from '../../db'; +import { DomainModule } from '../domain.module'; +import { ImportService } from './import.service'; +import { SettingsService } from './settings.service'; +import { RedisListener } from './redis.listener'; +import { setTimeout } from 'timers/promises'; + +// Allow a little time for the testcontainer to start +jest.setTimeout(300000); + +describe('receiveMessages', () => { + it('should call importwithfilter when a message is received', async () => { + await createTestingModule([DomainModule], async (app) => { + // GIVEN: Redis is running + const redis = await new GenericContainer('redis') + .withExposedPorts(6379) + .start(); + const redisUrl = `redis://localhost:${redis.getMappedPort(6379)}`; + const settings = app.get(SettingsService); + jest.spyOn(settings, 'getRedisUrl').mockImplementation(() => redisUrl); + + // And lastmessageid is zero + await settings.setLastMessageId('0'); + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + + const redisListener = app.get(RedisListener); + await redisListener.startRedisConsumer(); + + const client = createClient({ url: redisUrl }); + await client.connect(); + try { + const code1 = randomCode(); + const code2 = randomCode(); + + // When: A message is sent + const messageId = await client.xAdd('product_updates_off', '*', { + code: code1, + }); + + // Wait for message to be delivered + await setTimeout(100); + + // Then the import is called + expect(importSpy).toHaveBeenCalledTimes(1); + expect(await settings.getLastMessageId()).toBe(messageId); + + // If a new message is added + importSpy.mockClear(); + await client.xAdd('product_updates_off', '*', { + code: code2, + }); + + // Wait for message to be delivered + await setTimeout(100); + + // Then import is called again but only with the new code + expect(importSpy).toHaveBeenCalledTimes(1); + const codes = importSpy.mock.calls[0][0].code.$in; + expect(codes).toHaveLength(1); + expect(codes[0]).toBe(code2); + + // Update events are created + const events = + await sql`SELECT * FROM product_update_event WHERE message->>'code' = ${code1}`; + + expect(events).toHaveLength(1); + expect(events[0].id).toBe(messageId); + } finally { + await client.quit(); + await redisListener.stopRedisConsumer(); + await redis.stop(); + } + }); + }); +}); + +describe('processMessages', () => { + it('should not call importwithfilter for messages that contain the initial_import diff', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + + const code1 = randomCode(); + const code2 = randomCode(); + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + const messages = [ + { + id: nextId(), + message: { + code: code1, + }, + }, + { + id: nextId(), + message: { + code: code2, + // Note JSON properties in Redis come in as strings + diffs: JSON.stringify({ + initial_import: 1, + }), + }, + }, + ]; + + const redisListener = app.get(RedisListener); + await redisListener.processMessages(messages); + // Then the import is called only once for code1 + const codes = importSpy.mock.calls[0][0].code.$in; + expect(codes).toHaveLength(1); + expect(codes[0]).toBe(code1); + + // Update events are created for all codes + const events = + await sql`SELECT * FROM product_update_event WHERE message->>'code' IN ${sql( + [code1, code2], + )}`; + + expect(events).toHaveLength(2); + }); + }); + + it('should not call importwithfilter at all if all messages contain the initial_import diff', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + + const code1 = randomCode(); + const code2 = randomCode(); + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + const messages = [ + { + id: nextId(), + message: { + code: code1, + // Note JSON properties in Redis come in as strings + diffs: JSON.stringify({ + initial_import: 1, + }), + }, + }, + { + id: nextId(), + message: { + code: code2, + // Note JSON properties in Redis come in as strings + diffs: JSON.stringify({ + initial_import: 1, + }), + }, + }, + ]; + + const redisListener = app.get(RedisListener); + await redisListener.processMessages(messages); + // Then the import is not called at all + expect(importSpy).toHaveBeenCalledTimes(0); + + // Update events are created for all codes + const events = + await sql`SELECT * FROM product_update_event WHERE code IN ${sql([ + code1, + code2, + ])}`; + + expect(events).toHaveLength(2); + }); + }); +}); diff --git a/src/domain/services/redis.listener.ts b/src/domain/services/redis.listener.ts new file mode 100644 index 0000000..5bba1e2 --- /dev/null +++ b/src/domain/services/redis.listener.ts @@ -0,0 +1,94 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { createClient, commandOptions } from 'redis'; +import { MessagesService } from './messages.service'; +import { SettingsService } from './settings.service'; + +@Injectable() +export class RedisListener { + private logger = new Logger(RedisListener.name); + private client: any; // Don't strongly type here is it is really verbose + + constructor( + private readonly settings: SettingsService, + private readonly messages: MessagesService, + ) {} + + async startRedisConsumer() { + const redisUrl = this.settings.getRedisUrl(); + if (!redisUrl) return; + this.client = createClient({ url: redisUrl }); + this.client.on('error', (err: any) => this.logger.error(err)); + await this.client.connect(); + this.receiveMessages(); + } + + async stopRedisConsumer() { + if (this.client && this.client.isOpen) await this.client.quit(); + } + + async receiveMessages() { + const lastMessageId = await this.settings.getLastMessageId(); + if (!this.client.isOpen) return; + this.client + .xRead( + commandOptions({ + isolated: true, + }), + [ + // XREAD can read from multiple streams, starting at a + // different ID for each... + { + key: 'product_updates_off', + id: lastMessageId, + }, + ], + { + // Read 1000 entry at a time, block for 5 seconds if there are none. + COUNT: 1000, + BLOCK: 5000, + }, + ) + .then(async (keys: string | any[]) => { + if (keys?.length) { + const messages = keys[0].messages; + if (messages?.length) { + /** Message looks like this: + { + code: "0850026029062", + flavor: "off", + user_id: "stephane", + action: "updated", + comment: "Modification : Remove changes", + diffs: "{\"fields\":{\"change\":[\"categories\"],\"delete\":[\"product_name\",\"product_name_es\"]}}", + } + */ + await this.processMessages(messages); + } + } + setTimeout(() => { + this.receiveMessages(); + }, 0); + }); + } + + async processMessages(messages: any[]) { + // Fix JSON properties on each message to be objects rather than strings + for (const event of messages) { + if (event.message.diffs) + event.message.diffs = JSON.parse(event.message.diffs); + } + await this.messages.create(messages); + } + + async pauseAndRun(action: () => Promise) { + // Pause redis while doing a scheduled import + await this.stopRedisConsumer(); + + try { + await action(); + } finally { + // Resume redis after import + await this.startRedisConsumer(); + } + } +} diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index 47e1d10..38aa515 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -4,6 +4,7 @@ import sql from '../../db'; import { MessagesService } from './messages.service'; import { VIEW_PASSWORD, VIEW_USER } from '../../constants'; +/* describe('product_updates_view', () => { it('should aggregate events by count and distinct products', async () => { // Create some products @@ -63,10 +64,58 @@ describe('product_updates_view', () => { port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), }); - const results = await viewer`SELECT * from product_update_view`; + const results = await viewer`SELECT * from product_updates_by_owner`; const myResult = results.find((r) => r.owner_tag === owner1); - expect(myResult.update_count).toBe('4'); - expect(myResult.product_count).toBe('2'); + expect(myResult.update_count).toBe(4); + expect(myResult.product_count).toBe(2); + }); + + it('should update existing aggregate counts', async () => { + // Create some products + const code1 = randomCode(); + const action1 = randomCode(); + + // Create an existing message + const messages = new MessagesService(); + + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + await messages.create([ + { + id: nextId(), + message: { + code: code1, + action: action1, + }, + }, + ]); + + // Add another message + await messages.create([ + { + id: nextId(), + message: { + code: code1, + action: action1, + }, + }, + ]); + + // Use viewer user + const viewer = postgres({ + host: process.env.POSTGRES_HOST, + database: process.env.POSTGRES_DB, + user: VIEW_USER, + password: VIEW_PASSWORD, + port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), + }); + + const results = await viewer`SELECT * from product_updates_by_owner`; + + const myResult = results.find((r) => r.action === action1); + expect(myResult.update_count).toBe(2); + expect(myResult.product_count).toBe(1); }); }); +*/ \ No newline at end of file diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index c944bda..186936e 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -4,21 +4,31 @@ import { SCHEMA, VIEW_PASSWORD, VIEW_USER } from '../constants'; export class Migration20240719101700Redis extends Migration { async up(): Promise { - this.addSql(`CREATE TABLE IF NOT EXISTS query.product_update_event (id text NOT NULL PRIMARY KEY, updated_at timestamptz NOT NULL, code text NULL, message jsonb NOT NULL)`); + this.addSql(`CREATE TABLE IF NOT EXISTS product_update_event ( + id text NOT NULL PRIMARY KEY, + updated_at timestamptz NOT NULL, + message jsonb NOT NULL)`); + + this.addSql(`CREATE TABLE IF NOT EXISTS contributor ( + id serial, + code text, + constraint contributor_pkey primary key (id), + constraint contributor_code unique (code))`); + + this.addSql(`CREATE TYPE action AS ENUM ('created', 'updated', 'archived', 'unarchived', 'deleted', 'unknown')`); + + this.addSql(`CREATE TABLE IF NOT EXISTS product_action ( + product_id int, + updated_date date, + action action, + contributor_id int, + update_count int, + constraint product_action_pkey primary key (updated_date, product_id, action, contributor_id))`); + this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); - this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=${SCHEMA},public`, - ); + this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=${SCHEMA},public`); this.addSql(`GRANT USAGE ON SCHEMA ${SCHEMA} TO ${VIEW_USER}`); - - this.addSql(`CREATE OR REPLACE VIEW product_update_view AS SELECT - DATE_TRUNC('day', pe.updated_at) updated_day, - p.owners_tags owner_tag, - count(*) update_count, - count(DISTINCT pe.code) product_count - FROM product_update_event pe - LEFT JOIN product p on p.code = pe.code - GROUP BY DATE_TRUNC('day', pe.updated_at), - p.owners_tags`); - this.addSql(`GRANT SELECT ON product_update_view TO ${VIEW_USER}`); + this.addSql(`GRANT SELECT ON product_action TO ${VIEW_USER}`); + this.addSql(`GRANT SELECT ON product TO ${VIEW_USER}`); } } From 0d8a312387f73f36d089958beab9c9575b18d765 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 26 Jul 2024 18:09:49 +0100 Subject: [PATCH 15/31] Fix tests that were relying on mongo Signed-off-by: John Gomersall --- src/domain/services/messages.service.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 8a369ad..e2cc10c 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -49,6 +49,7 @@ describe('create', () => { message: { code: code1, action: 'created', + diffs: { initial_import: 1 }, }, }, { @@ -56,6 +57,7 @@ describe('create', () => { message: { code: code1, action: 'created', + diffs: { initial_import: 1 }, }, }, ]); @@ -77,6 +79,7 @@ describe('create', () => { message: { code: code1, comment: 'test \u0000 test2 \u0000 end', + diffs: { initial_import: 1 }, }, }, ]); @@ -106,6 +109,7 @@ describe('create', () => { code: code1, user_id: user1, action: 'created', + diffs: { initial_import: 1 }, }, }, { @@ -114,6 +118,7 @@ describe('create', () => { code: code1, user_id: user2, action: 'created', + diffs: { initial_import: 1 }, }, }, ]); From 12ae3afa90b1ecbba43768c11f9e83a90d3018bd Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 26 Jul 2024 18:40:25 +0100 Subject: [PATCH 16/31] Summarise events by product, user and action Signed-off-by: John Gomersall --- src/domain/services/import.service.spec.ts | 1 - src/domain/services/messages.service.spec.ts | 124 ++++++++++++++++++ src/domain/services/messages.service.ts | 60 ++++----- .../Migration20240719101700-redis.ts | 10 +- 4 files changed, 161 insertions(+), 34 deletions(-) diff --git a/src/domain/services/import.service.spec.ts b/src/domain/services/import.service.spec.ts index 5417661..6fc4304 100644 --- a/src/domain/services/import.service.spec.ts +++ b/src/domain/services/import.service.spec.ts @@ -68,7 +68,6 @@ function mockMongoDB(productList) { } // Import tests can sometimes take a little time in GitHub -// Plus Allow a little time for the testcontainer to start jest.setTimeout(300000); describe('importFromMongo', () => { diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index e2cc10c..a2f22a4 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -127,4 +127,128 @@ describe('create', () => { expect(result).toHaveLength(1); }); }); + + it('should aggregate events by count and distinct products', async () => { + await createTestingModule([DomainModule], async (app) => { + const messages = app.get(MessagesService); + // Create some products + const code1 = randomCode(); + const code2 = randomCode(); + const owner1 = randomCode(); + + await sql`INSERT INTO product ${sql([ + { + code: code1, + owners_tags: owner1, + }, + { + code: code2, + owners_tags: owner1, + }, + ])}`; + + // Create some messages + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + await messages.create([ + { + id: nextId(), + message: { + code: code1, + action: 'created', + user_id: 'test', + diffs: { initial_import: 1 }, + }, + }, + { + id: nextId(), + message: { + code: code1, + action: 'created', + user_id: 'test', + diffs: { initial_import: 1 }, + }, + }, + { + id: nextId(), + message: { + code: code1, + action: 'created', + user_id: 'test', + diffs: { initial_import: 1 }, + }, + }, + { + id: nextId(), + message: { + code: code2, + action: 'created', + user_id: 'test', + diffs: { initial_import: 1 }, + }, + }, + ]); + + const results = + await sql`SELECT * from product_action join product on product.id = product_action.product_id`; + + const myResult1 = results.find( + (r) => r.owners_tags === owner1 && r.code === code1, + ); + expect(myResult1.update_count).toBe(3); + + const myResult2 = results.find( + (r) => r.owners_tags === owner1 && r.code === code2, + ); + expect(myResult2.update_count).toBe(1); + }); + }); + + it('should update existing aggregate counts', async () => { + await createTestingModule([DomainModule], async (app) => { + const messages = app.get(MessagesService); + // Create a product + const code1 = randomCode(); + await sql`INSERT INTO product ${sql([ + { + code: code1, + }, + ])}`; + const action1 = randomCode(); + + // Create an existing message + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + await messages.create([ + { + id: nextId(), + message: { + code: code1, + action: action1, + user_id: 'test', + diffs: { initial_import: 1 }, + }, + }, + ]); + + // Add another message + await messages.create([ + { + id: nextId(), + message: { + code: code1, + action: action1, + user_id: 'test', + diffs: { initial_import: 1 }, + }, + }, + ]); + + const results = + await sql`SELECT * from product_action join product on product.id = product_action.product_id`; + + const myResult1 = results.find((r) => r.code === code1); + expect(myResult1.update_count).toBe(2); + }); + }); }); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index 18d41ef..c76730b 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -43,44 +43,20 @@ export class MessagesService { const messageIds = messages.map((m) => m.id); - const results = await sql`insert into contributor (code) + await sql`insert into contributor (code) select distinct message->>'user_id' from product_update_event where id in ${sql(messageIds)} on conflict (code) do nothing`; - /* - const [minTime, maxTime] = messagesForInsert.reduce( - ([prevMin, prevMax], m) => [ - Math.min(prevMin, m.updated_at.getTime()), - Math.max(prevMax, m.updated_at.getTime()), - ], - [Infinity, -Infinity], - ); - const minDate = new Date(minTime).toISOString().substring(0, 10); - const maxDate = new Date(maxTime + 86400000).toISOString().substring(0, 10); + await sql`insert into action (code) + select distinct message->>'action' + from product_update_event + where id in ${sql(messageIds)} + on conflict (code) + do nothing`; - // TODO: When we upgrade to PostgreSQL 15 we can us a unique constraint to cover the nullable columns - // so won't need to expressions in the on conflict clause - await sql`INSERT INTO product_updates_by_owner - SELECT - date(pe.updated_at at time zone 'UTC') updated_day, - p.owners_tags owner_tag, - pe.action, - count(*) update_count, - count(DISTINCT pe.code) product_count - FROM product_update_event pe - LEFT JOIN product p on p.code = pe.code - WHERE pe.updated_at >= ${minDate} AND pe.updated_at < ${maxDate} - GROUP BY date(pe.updated_at at time zone 'UTC'), - p.owners_tags, - pe.action - on conflict (updated_date, coalesce(owner_tag, ''), coalesce(action, '')) - do update set - update_count = EXCLUDED.update_count, - product_count = EXCLUDED.product_count`; - */ const productCodes = [ ...new Set( messages @@ -95,6 +71,28 @@ export class MessagesService { const filter = { code: { $in: productCodes } }; await this.importService.importWithFilter(filter, ProductSource.EVENT); } + + // Update counts on product_action after products have been imported + await sql`INSERT INTO product_action + SELECT + p.id, + date(pe.updated_at at time zone 'UTC') updated_day, + action.id, + contributor.id, + count(*) update_count + FROM product_update_event pe + JOIN product p on p.code = pe.message->>'code' + join contributor on contributor.code = pe.message->>'user_id' + join action on action.code = pe.message->>'action' + where pe.id in ${sql(messageIds)} + GROUP BY p.id, + date(pe.updated_at at time zone 'UTC'), + action.id, + contributor.id + on conflict (updated_date,product_id,action,contributor_id) + do update set + update_count = product_action.update_count + EXCLUDED.update_count`; + await this.settings.setLastMessageId(messages[messages.length - 1].id); } } diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 186936e..5630b99 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -14,13 +14,19 @@ export class Migration20240719101700Redis extends Migration { code text, constraint contributor_pkey primary key (id), constraint contributor_code unique (code))`); + + this.addSql(`CREATE TABLE IF NOT EXISTS action ( + id serial, + code text, + constraint action_pkey primary key (id), + constraint action_code unique (code))`); - this.addSql(`CREATE TYPE action AS ENUM ('created', 'updated', 'archived', 'unarchived', 'deleted', 'unknown')`); + this.addSql(`INSERT INTO action (code) VALUES ('created'), ('updated'), ('archived'), ('unarchived'), ('deleted'), ('unknown')`); this.addSql(`CREATE TABLE IF NOT EXISTS product_action ( product_id int, updated_date date, - action action, + action int, contributor_id int, update_count int, constraint product_action_pkey primary key (updated_date, product_id, action, contributor_id))`); From e63879f5ffc6f7bc85879185c60844e014b3eb4f Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 2 Aug 2024 16:09:20 +0100 Subject: [PATCH 17/31] Rename table to product_update Signed-off-by: John Gomersall --- src/domain/services/messages.service.spec.ts | 4 ++-- src/domain/services/messages.service.ts | 6 +++--- src/domain/services/redis.listener.spec.ts | 2 +- src/migrations/Migration20240719101700-redis.ts | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index a2f22a4..4bb4260 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -190,7 +190,7 @@ describe('create', () => { ]); const results = - await sql`SELECT * from product_action join product on product.id = product_action.product_id`; + await sql`SELECT * from product_update join product on product.id = product_update.product_id`; const myResult1 = results.find( (r) => r.owners_tags === owner1 && r.code === code1, @@ -245,7 +245,7 @@ describe('create', () => { ]); const results = - await sql`SELECT * from product_action join product on product.id = product_action.product_id`; + await sql`SELECT * from product_update join product on product.id = product_update.product_id`; const myResult1 = results.find((r) => r.code === code1); expect(myResult1.update_count).toBe(2); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index c76730b..0da5ea2 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -72,8 +72,8 @@ export class MessagesService { await this.importService.importWithFilter(filter, ProductSource.EVENT); } - // Update counts on product_action after products have been imported - await sql`INSERT INTO product_action + // Update counts on product_update after products have been imported + await sql`INSERT INTO product_update SELECT p.id, date(pe.updated_at at time zone 'UTC') updated_day, @@ -91,7 +91,7 @@ export class MessagesService { contributor.id on conflict (updated_date,product_id,action,contributor_id) do update set - update_count = product_action.update_count + EXCLUDED.update_count`; + update_count = product_update.update_count + EXCLUDED.update_count`; await this.settings.setLastMessageId(messages[messages.length - 1].id); } diff --git a/src/domain/services/redis.listener.spec.ts b/src/domain/services/redis.listener.spec.ts index d367730..810e0e2 100644 --- a/src/domain/services/redis.listener.spec.ts +++ b/src/domain/services/redis.listener.spec.ts @@ -169,7 +169,7 @@ describe('processMessages', () => { // Update events are created for all codes const events = - await sql`SELECT * FROM product_update_event WHERE code IN ${sql([ + await sql`SELECT * FROM product_update_event WHERE message->>'code' IN ${sql([ code1, code2, ])}`; diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 5630b99..fe3c2db 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -23,18 +23,18 @@ export class Migration20240719101700Redis extends Migration { this.addSql(`INSERT INTO action (code) VALUES ('created'), ('updated'), ('archived'), ('unarchived'), ('deleted'), ('unknown')`); - this.addSql(`CREATE TABLE IF NOT EXISTS product_action ( + this.addSql(`CREATE TABLE IF NOT EXISTS product_update ( product_id int, updated_date date, action int, contributor_id int, update_count int, - constraint product_action_pkey primary key (updated_date, product_id, action, contributor_id))`); + constraint product_update_pkey primary key (updated_date, product_id, action, contributor_id))`); this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=${SCHEMA},public`); this.addSql(`GRANT USAGE ON SCHEMA ${SCHEMA} TO ${VIEW_USER}`); - this.addSql(`GRANT SELECT ON product_action TO ${VIEW_USER}`); + this.addSql(`GRANT SELECT ON product_update TO ${VIEW_USER}`); this.addSql(`GRANT SELECT ON product TO ${VIEW_USER}`); } } From 06b533a2d9aa3ea8b389bda11abee486c71fb54b Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 2 Aug 2024 16:43:52 +0100 Subject: [PATCH 18/31] Fix view tests Signed-off-by: John Gomersall --- src/domain/services/views.spec.ts | 221 ++++++++++-------- .../Migration20240719101700-redis.ts | 2 + 2 files changed, 130 insertions(+), 93 deletions(-) diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index 38aa515..5142d1a 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -1,121 +1,156 @@ import postgres from 'postgres'; -import { randomCode } from '../../../test/test.helper'; +import { createTestingModule, randomCode } from '../../../test/test.helper'; import sql from '../../db'; import { MessagesService } from './messages.service'; import { VIEW_PASSWORD, VIEW_USER } from '../../constants'; +import { DomainModule } from '../domain.module'; -/* -describe('product_updates_view', () => { +describe('product_update', () => { it('should aggregate events by count and distinct products', async () => { - // Create some products - const code1 = randomCode(); - const code2 = randomCode(); - const owner1 = randomCode(); + await createTestingModule([DomainModule], async (app) => { + const messages = app.get(MessagesService); + // Create some products + const code1 = randomCode(); + const code2 = randomCode(); + const owner1 = randomCode(); - await sql`INSERT INTO product ${sql([ - { - code: code1, - owners_tags: owner1, - }, - { - code: code2, - owners_tags: owner1, - }, - ])}`; - - // Create some messages - const messages = new MessagesService(); - - let idCount = 0; - const nextId = () => `${Date.now()}-${idCount++}`; - await messages.create([ - { - id: nextId(), - message: { + await sql`INSERT INTO product ${sql([ + { code: code1, + owners_tags: owner1, }, - }, - { - id: nextId(), - message: { - code: code1, + { + code: code2, + owners_tags: owner1, }, - }, - { - id: nextId(), - message: { - code: code1, + ])}`; + + // Create some messages + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + await messages.create([ + { + id: nextId(), + message: { + code: code1, + action: 'updated', + user_id: 'user1', + diffs: { initial_import: 1 }, + }, }, - }, - { - id: nextId(), - message: { - code: code2, + { + id: nextId(), + message: { + code: code1, + action: 'updated', + user_id: 'user1', + diffs: { initial_import: 1 }, + }, + }, + { + id: nextId(), + message: { + code: code1, + action: 'updated', + user_id: 'user1', + diffs: { initial_import: 1 }, + }, + }, + { + id: nextId(), + message: { + code: code2, + action: 'updated', + user_id: 'user1', + diffs: { initial_import: 1 }, + }, }, - }, - ]); + ]); - // Use viewer user - const viewer = postgres({ - host: process.env.POSTGRES_HOST, - database: process.env.POSTGRES_DB, - user: VIEW_USER, - password: VIEW_PASSWORD, - port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), - }); + // Use viewer user + const viewer = postgres({ + host: process.env.POSTGRES_HOST, + database: process.env.POSTGRES_DB, + user: VIEW_USER, + password: VIEW_PASSWORD, + port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), + }); - const results = await viewer`SELECT * from product_updates_by_owner`; + const results = await viewer`SELECT + owners_tags owner_tag, + sum(update_count) update_count, + count(DISTINCT product_id) product_count + from product_update pu + join product p on p.id= pu.product_id + group by owners_tags`; - const myResult = results.find((r) => r.owner_tag === owner1); - expect(myResult.update_count).toBe(4); - expect(myResult.product_count).toBe(2); + const myResult = results.find((r) => r.owner_tag === owner1); + expect(myResult.update_count).toBe('4'); + expect(myResult.product_count).toBe('2'); + }); }); it('should update existing aggregate counts', async () => { - // Create some products - const code1 = randomCode(); - const action1 = randomCode(); + await createTestingModule([DomainModule], async (app) => { + const messages = app.get(MessagesService); + // Create some products + const code1 = randomCode(); + const action1 = randomCode(); - // Create an existing message - const messages = new MessagesService(); - - let idCount = 0; - const nextId = () => `${Date.now()}-${idCount++}`; - await messages.create([ - { - id: nextId(), - message: { + await sql`INSERT INTO product ${sql([ + { code: code1, - action: action1, }, - }, - ]); + ])}`; - // Add another message - await messages.create([ - { - id: nextId(), - message: { - code: code1, - action: action1, + // Create an existing message + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + await messages.create([ + { + id: nextId(), + message: { + code: code1, + action: action1, + user_id: 'user1', + diffs: { initial_import: 1 }, + }, }, - }, - ]); + ]); - // Use viewer user - const viewer = postgres({ - host: process.env.POSTGRES_HOST, - database: process.env.POSTGRES_DB, - user: VIEW_USER, - password: VIEW_PASSWORD, - port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), - }); + // Add another message + await messages.create([ + { + id: nextId(), + message: { + code: code1, + action: action1, + user_id: 'user1', + diffs: { initial_import: 1 }, + }, + }, + ]); - const results = await viewer`SELECT * from product_updates_by_owner`; + // Use viewer user + const viewer = postgres({ + host: process.env.POSTGRES_HOST, + database: process.env.POSTGRES_DB, + user: VIEW_USER, + password: VIEW_PASSWORD, + port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), + }); - const myResult = results.find((r) => r.action === action1); - expect(myResult.update_count).toBe(2); - expect(myResult.product_count).toBe(1); + const results = await viewer`SELECT + a.code, + sum(update_count) update_count, + count(DISTINCT product_id) product_count + from product_update pu + join action a on a.id = pu.action + group by a.code`; + + const myResult = results.find((r) => r.code === action1); + expect(myResult.update_count).toBe('2'); + expect(myResult.product_count).toBe('1'); + }); }); }); -*/ \ No newline at end of file diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index fe3c2db..181ee4f 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -35,6 +35,8 @@ export class Migration20240719101700Redis extends Migration { this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=${SCHEMA},public`); this.addSql(`GRANT USAGE ON SCHEMA ${SCHEMA} TO ${VIEW_USER}`); this.addSql(`GRANT SELECT ON product_update TO ${VIEW_USER}`); + this.addSql(`GRANT SELECT ON action TO ${VIEW_USER}`); + this.addSql(`GRANT SELECT ON contributor TO ${VIEW_USER}`); this.addSql(`GRANT SELECT ON product TO ${VIEW_USER}`); } } From e3bc5f975653b28449d79263307f820264236ecd Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 2 Aug 2024 16:53:55 +0100 Subject: [PATCH 19/31] Rename update_type table as action can be a reserved word Signed-off-by: John Gomersall --- src/domain/services/messages.service.ts | 10 +++++----- src/domain/services/views.spec.ts | 8 ++++---- src/migrations/Migration20240719101700-redis.ts | 10 +++++----- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index 0da5ea2..05bb2ce 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -50,7 +50,7 @@ export class MessagesService { on conflict (code) do nothing`; - await sql`insert into action (code) + await sql`insert into update_type (code) select distinct message->>'action' from product_update_event where id in ${sql(messageIds)} @@ -77,19 +77,19 @@ export class MessagesService { SELECT p.id, date(pe.updated_at at time zone 'UTC') updated_day, - action.id, + update_type.id, contributor.id, count(*) update_count FROM product_update_event pe JOIN product p on p.code = pe.message->>'code' join contributor on contributor.code = pe.message->>'user_id' - join action on action.code = pe.message->>'action' + join update_type on update_type.code = pe.message->>'action' where pe.id in ${sql(messageIds)} GROUP BY p.id, date(pe.updated_at at time zone 'UTC'), - action.id, + update_type.id, contributor.id - on conflict (updated_date,product_id,action,contributor_id) + on conflict (updated_date,product_id,update_type_id,contributor_id) do update set update_count = product_update.update_count + EXCLUDED.update_count`; diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index 5142d1a..20e0ca9 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -141,14 +141,14 @@ describe('product_update', () => { }); const results = await viewer`SELECT - a.code, + ut.code update_type, sum(update_count) update_count, count(DISTINCT product_id) product_count from product_update pu - join action a on a.id = pu.action - group by a.code`; + join update_type ut on ut.id = pu.update_type_id + group by ut.code`; - const myResult = results.find((r) => r.code === action1); + const myResult = results.find((r) => r.update_type === action1); expect(myResult.update_count).toBe('2'); expect(myResult.product_count).toBe('1'); }); diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 181ee4f..50052c8 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -15,27 +15,27 @@ export class Migration20240719101700Redis extends Migration { constraint contributor_pkey primary key (id), constraint contributor_code unique (code))`); - this.addSql(`CREATE TABLE IF NOT EXISTS action ( + this.addSql(`CREATE TABLE IF NOT EXISTS update_type ( id serial, code text, constraint action_pkey primary key (id), constraint action_code unique (code))`); - this.addSql(`INSERT INTO action (code) VALUES ('created'), ('updated'), ('archived'), ('unarchived'), ('deleted'), ('unknown')`); + this.addSql(`INSERT INTO update_type (code) VALUES ('created'), ('updated'), ('archived'), ('unarchived'), ('deleted'), ('unknown')`); this.addSql(`CREATE TABLE IF NOT EXISTS product_update ( product_id int, updated_date date, - action int, + update_type_id int, contributor_id int, update_count int, - constraint product_update_pkey primary key (updated_date, product_id, action, contributor_id))`); + constraint product_update_pkey primary key (updated_date, product_id, update_type_id, contributor_id))`); this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=${SCHEMA},public`); this.addSql(`GRANT USAGE ON SCHEMA ${SCHEMA} TO ${VIEW_USER}`); this.addSql(`GRANT SELECT ON product_update TO ${VIEW_USER}`); - this.addSql(`GRANT SELECT ON action TO ${VIEW_USER}`); + this.addSql(`GRANT SELECT ON update_type TO ${VIEW_USER}`); this.addSql(`GRANT SELECT ON contributor TO ${VIEW_USER}`); this.addSql(`GRANT SELECT ON product TO ${VIEW_USER}`); } From 06a4a9a3193e8d393c73ee106c3d8d00686baa4a Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 2 Aug 2024 18:28:00 +0100 Subject: [PATCH 20/31] Create a view to isolate consumption from implementation Signed-off-by: John Gomersall --- src/domain/services/views.spec.ts | 67 +++++++++---------- .../Migration20240719101700-redis.ts | 24 +++++-- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index 20e0ca9..0248336 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -5,6 +5,25 @@ import { MessagesService } from './messages.service'; import { VIEW_PASSWORD, VIEW_USER } from '../../constants'; import { DomainModule } from '../domain.module'; +async function withViewUser( + action: (viewer: postgres.Sql) => Promise, +) { + // Use viewer user + const viewer = postgres({ + host: process.env.POSTGRES_HOST, + database: process.env.POSTGRES_DB, + user: VIEW_USER, + password: VIEW_PASSWORD, + port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), + }); + + try { + await action(viewer); + } finally { + await viewer.end(); + } +} + describe('product_update', () => { it('should aggregate events by count and distinct products', async () => { await createTestingModule([DomainModule], async (app) => { @@ -68,25 +87,13 @@ describe('product_update', () => { ]); // Use viewer user - const viewer = postgres({ - host: process.env.POSTGRES_HOST, - database: process.env.POSTGRES_DB, - user: VIEW_USER, - password: VIEW_PASSWORD, - port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), - }); - - const results = await viewer`SELECT - owners_tags owner_tag, - sum(update_count) update_count, - count(DISTINCT product_id) product_count - from product_update pu - join product p on p.id= pu.product_id - group by owners_tags`; + await withViewUser(async (viewer) => { + const results = await viewer`SELECT * from product_updates_by_owner`; - const myResult = results.find((r) => r.owner_tag === owner1); - expect(myResult.update_count).toBe('4'); - expect(myResult.product_count).toBe('2'); + const myResult = results.find((r) => r.owner_tag === owner1); + expect(myResult.update_count).toBe('4'); + expect(myResult.product_count).toBe('2'); + }); }); }); @@ -132,25 +139,13 @@ describe('product_update', () => { ]); // Use viewer user - const viewer = postgres({ - host: process.env.POSTGRES_HOST, - database: process.env.POSTGRES_DB, - user: VIEW_USER, - password: VIEW_PASSWORD, - port: parseInt(process.env.POSTGRES_PORT.split(':').pop()), - }); + await withViewUser(async (viewer) => { + const results = await viewer`SELECT * from product_updates_by_owner`; - const results = await viewer`SELECT - ut.code update_type, - sum(update_count) update_count, - count(DISTINCT product_id) product_count - from product_update pu - join update_type ut on ut.id = pu.update_type_id - group by ut.code`; - - const myResult = results.find((r) => r.update_type === action1); - expect(myResult.update_count).toBe('2'); - expect(myResult.product_count).toBe('1'); + const myResult = results.find((r) => r.update_type === action1); + expect(myResult.update_count).toBe('2'); + expect(myResult.product_count).toBe('1'); + }); }); }); }); diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 50052c8..74fb9ed 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -1,6 +1,6 @@ /* eslint-disable prettier/prettier */ import { Migration } from '@mikro-orm/migrations'; -import { SCHEMA, VIEW_PASSWORD, VIEW_USER } from '../constants'; +import { VIEW_PASSWORD, VIEW_USER } from '../constants'; export class Migration20240719101700Redis extends Migration { async up(): Promise { @@ -31,12 +31,22 @@ export class Migration20240719101700Redis extends Migration { update_count int, constraint product_update_pkey primary key (updated_date, product_id, update_type_id, contributor_id))`); + this.addSql('create schema if not exists views;'); this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); - this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=${SCHEMA},public`); - this.addSql(`GRANT USAGE ON SCHEMA ${SCHEMA} TO ${VIEW_USER}`); - this.addSql(`GRANT SELECT ON product_update TO ${VIEW_USER}`); - this.addSql(`GRANT SELECT ON update_type TO ${VIEW_USER}`); - this.addSql(`GRANT SELECT ON contributor TO ${VIEW_USER}`); - this.addSql(`GRANT SELECT ON product TO ${VIEW_USER}`); + this.addSql(`ALTER ROLE ${VIEW_USER} SET search_path=views,public`); + this.addSql(`CREATE OR REPLACE VIEW views.product_updates_by_owner AS + SELECT pu.updated_date, + p.owners_tags owner_tag, + ut.code update_type, + sum(pu.update_count) update_count, + count(DISTINCT pu.product_id) product_count + FROM product_update pu + JOIN product p ON p.id = pu.product_id + JOIN update_type ut ON ut.id = pu.update_type_id + GROUP BY pu.updated_date, + p.owners_tags, + ut.code`); + this.addSql(`GRANT USAGE ON SCHEMA views TO ${VIEW_USER}`); + this.addSql(`GRANT SELECT ON views.product_updates_by_owner TO ${VIEW_USER}`); } } From b0f9be1812379084b74953c8ed0df92d4b3e6686 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Thu, 8 Aug 2024 17:43:23 +0100 Subject: [PATCH 21/31] This not set properly for scheduled import Signed-off-by: John Gomersall --- src/domain/domain.module.spec.ts | 24 ++++++++++++++++++++ src/domain/domain.module.ts | 4 ++-- src/domain/services/messages.service.spec.ts | 1 + 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 src/domain/domain.module.spec.ts diff --git a/src/domain/domain.module.spec.ts b/src/domain/domain.module.spec.ts new file mode 100644 index 0000000..85af9ae --- /dev/null +++ b/src/domain/domain.module.spec.ts @@ -0,0 +1,24 @@ +import { createTestingModule } from '../../test/test.helper'; +import { DomainModule } from './domain.module'; +import { ImportService } from './services/import.service'; +import { RedisListener } from './services/redis.listener'; + +describe('refreshProducts', () => { + it('should pause Redis while doing a scheduled reload', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const redisListener = app.get(RedisListener); + jest.spyOn(importService, 'importFromMongo').mockImplementation(); + const redisStopSpy = jest + .spyOn(redisListener, 'stopRedisConsumer') + .mockImplementation(); + const redisStartSpy = jest + .spyOn(redisListener, 'startRedisConsumer') + .mockImplementation(); + + await app.get(DomainModule).refreshProducts(); + expect(redisStopSpy).toHaveBeenCalledTimes(1); + expect(redisStartSpy).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/src/domain/domain.module.ts b/src/domain/domain.module.ts index a8224da..34d15c9 100644 --- a/src/domain/domain.module.ts +++ b/src/domain/domain.module.ts @@ -52,8 +52,8 @@ export class DomainModule implements OnModuleInit, OnModuleDestroy { // The request context creates a separate entity manager instance // which avoids clashes with other requests await RequestContext.createAsync(this.em, async () => { - await this.redisListener.pauseAndRun( - this.importService.scheduledImportFromMongo, + await this.redisListener.pauseAndRun(() => + this.importService.scheduledImportFromMongo(), ); }); } diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 4bb4260..1d48f50 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -125,6 +125,7 @@ describe('create', () => { const result = await sql`SELECT * FROM contributor WHERE code = ${user2}`; expect(result).toHaveLength(1); + // TODO: test there are no wasted ids }); }); From 5940752fc3689786972482ec3aec8301829ed388 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 13:01:30 +0100 Subject: [PATCH 22/31] Add API for initial load Signed-off-by: John Gomersall --- src/app.controller.spec.ts | 35 +++ src/app.controller.ts | 13 + src/domain/services/messages.service.spec.ts | 276 ++++++++++++------- src/domain/services/messages.service.ts | 17 +- src/domain/services/redis.listener.spec.ts | 94 +------ src/domain/services/views.spec.ts | 90 +++--- 6 files changed, 294 insertions(+), 231 deletions(-) create mode 100644 src/app.controller.spec.ts diff --git a/src/app.controller.spec.ts b/src/app.controller.spec.ts new file mode 100644 index 0000000..0355bd8 --- /dev/null +++ b/src/app.controller.spec.ts @@ -0,0 +1,35 @@ +import { createTestingModule, randomCode } from '../test/test.helper'; +import { AppController } from './app.controller'; +import { AppModule } from './app.module'; +import sql from './db'; +import { ImportService } from './domain/services/import.service'; + +describe('productupdate', () => { + it('should import message but not refresh products', async () => { + await createTestingModule([AppModule], async (app) => { + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + + const code1 = randomCode(); + const updates = [ + { + code: code1, + }, + ]; + + const appController = app.get(AppController); + await appController.addProductUpdates(updates); + + // Then the import is not called + expect(importSpy).not.toHaveBeenCalled(); + + // Update events are created + const events = + await sql`SELECT * FROM product_update_event WHERE message->>'code' = ${code1}`; + + expect(events).toHaveLength(1); + }); + }); +}); diff --git a/src/app.controller.ts b/src/app.controller.ts index 69f9e8f..a056052 100644 --- a/src/app.controller.ts +++ b/src/app.controller.ts @@ -2,6 +2,7 @@ import { Body, Controller, Get, Post, Query, All } from '@nestjs/common'; import { ImportService } from './domain/services/import.service'; import { QueryService } from './domain/services/query.service'; import { RedisListener } from './domain/services/redis.listener'; +import { MessagesService } from './domain/services/messages.service'; @Controller() export class AppController { @@ -9,6 +10,7 @@ export class AppController { private readonly importService: ImportService, private readonly queryService: QueryService, private readonly redisListener: RedisListener, + private readonly messagesService: MessagesService, ) {} @Get('importfrommongo') @@ -45,4 +47,15 @@ export class AppController { async select(@Body() body: any, @Query('obsolete') obsolete) { return await this.queryService.select(body, this.parseBoolean(obsolete)); } + + // Temporary code for initial import + messageId = 0; + @Post('productupdates') + async addProductUpdates(@Body() updates: any[]) { + const messages = []; + for (const update of updates) { + messages.push({ id: `0-${this.messageId++}`, message: update }); + } + await this.messagesService.create(messages, true); + } } diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 1d48f50..56cd1e2 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -1,6 +1,7 @@ import { createTestingModule, randomCode } from '../../../test/test.helper'; import sql from '../../db'; import { DomainModule } from '../domain.module'; +import { ImportService } from './import.service'; import { MessagesService } from './messages.service'; describe('messageTime', () => { @@ -43,24 +44,25 @@ describe('create', () => { const code1 = randomCode(); const messageId = `${Date.now()}-${idCount++}`; - await messages.create([ - { - id: messageId, - message: { - code: code1, - action: 'created', - diffs: { initial_import: 1 }, + await messages.create( + [ + { + id: messageId, + message: { + code: code1, + action: 'created', + }, }, - }, - { - id: messageId, - message: { - code: code1, - action: 'created', - diffs: { initial_import: 1 }, + { + id: messageId, + message: { + code: code1, + action: 'created', + }, }, - }, - ]); + ], + true, + ); const result = await sql`SELECT * FROM product_update_event WHERE message->>'code' = ${code1}`; @@ -73,16 +75,18 @@ describe('create', () => { await createTestingModule([DomainModule], async (app) => { const messages = app.get(MessagesService); const code1 = randomCode(); - await messages.create([ - { - id: `${Date.now()}-${idCount++}`, - message: { - code: code1, - comment: 'test \u0000 test2 \u0000 end', - diffs: { initial_import: 1 }, + await messages.create( + [ + { + id: `${Date.now()}-${idCount++}`, + message: { + code: code1, + comment: 'test \u0000 test2 \u0000 end', + }, }, - }, - ]); + ], + true, + ); const result = await sql`SELECT * FROM product_update_event WHERE message->>'code' = ${code1}`; @@ -102,26 +106,27 @@ describe('create', () => { sql`INSERT INTO contributor (code) VALUES(${user1})`; // When events are imported - await messages.create([ - { - id: `${Date.now()}-${idCount++}`, - message: { - code: code1, - user_id: user1, - action: 'created', - diffs: { initial_import: 1 }, + await messages.create( + [ + { + id: `${Date.now()}-${idCount++}`, + message: { + code: code1, + user_id: user1, + action: 'created', + }, }, - }, - { - id: `${Date.now()}-${idCount++}`, - message: { - code: code1, - user_id: user2, - action: 'created', - diffs: { initial_import: 1 }, + { + id: `${Date.now()}-${idCount++}`, + message: { + code: code1, + user_id: user2, + action: 'created', + }, }, - }, - ]); + ], + true, + ); const result = await sql`SELECT * FROM contributor WHERE code = ${user2}`; expect(result).toHaveLength(1); @@ -151,44 +156,43 @@ describe('create', () => { // Create some messages let idCount = 0; const nextId = () => `${Date.now()}-${idCount++}`; - await messages.create([ - { - id: nextId(), - message: { - code: code1, - action: 'created', - user_id: 'test', - diffs: { initial_import: 1 }, + await messages.create( + [ + { + id: nextId(), + message: { + code: code1, + action: 'created', + user_id: 'test', + }, }, - }, - { - id: nextId(), - message: { - code: code1, - action: 'created', - user_id: 'test', - diffs: { initial_import: 1 }, + { + id: nextId(), + message: { + code: code1, + action: 'created', + user_id: 'test', + }, }, - }, - { - id: nextId(), - message: { - code: code1, - action: 'created', - user_id: 'test', - diffs: { initial_import: 1 }, + { + id: nextId(), + message: { + code: code1, + action: 'created', + user_id: 'test', + }, }, - }, - { - id: nextId(), - message: { - code: code2, - action: 'created', - user_id: 'test', - diffs: { initial_import: 1 }, + { + id: nextId(), + message: { + code: code2, + action: 'created', + user_id: 'test', + }, }, - }, - ]); + ], + true, + ); const results = await sql`SELECT * from product_update join product on product.id = product_update.product_id`; @@ -220,36 +224,124 @@ describe('create', () => { // Create an existing message let idCount = 0; const nextId = () => `${Date.now()}-${idCount++}`; - await messages.create([ + await messages.create( + [ + { + id: nextId(), + message: { + code: code1, + action: action1, + user_id: 'test', + }, + }, + ], + true, + ); + + // Add another message + await messages.create( + [ + { + id: nextId(), + message: { + code: code1, + action: action1, + user_id: 'test', + }, + }, + ], + true, + ); + + const results = + await sql`SELECT * from product_update join product on product.id = product_update.product_id`; + + const myResult1 = results.find((r) => r.code === code1); + expect(myResult1.update_count).toBe(2); + }); + }); + + it('should not call importwithfilter for initialImport', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + + const code1 = randomCode(); + const code2 = randomCode(); + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + const messages = [ { id: nextId(), message: { code: code1, - action: action1, - user_id: 'test', - diffs: { initial_import: 1 }, }, }, - ]); + { + id: nextId(), + message: { + code: code2, + }, + }, + ]; - // Add another message - await messages.create([ + const messagesService = app.get(MessagesService); + await messagesService.create(messages, true); + + // Then the import is not called + expect(importSpy).not.toHaveBeenCalled(); + + // Update events are created for all codes + const events = + await sql`SELECT * FROM product_update_event WHERE message->>'code' IN ${sql( + [code1, code2], + )}`; + + expect(events).toHaveLength(2); + }); + }); + + it('should call importwithfilter for normal imports', async () => { + await createTestingModule([DomainModule], async (app) => { + const importService = app.get(ImportService); + const importSpy = jest + .spyOn(importService, 'importWithFilter') + .mockImplementation(); + + const code1 = randomCode(); + const code2 = randomCode(); + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + const messages = [ { id: nextId(), message: { code: code1, - action: action1, - user_id: 'test', - diffs: { initial_import: 1 }, }, }, - ]); + { + id: nextId(), + message: { + code: code2, + }, + }, + ]; - const results = - await sql`SELECT * from product_update join product on product.id = product_update.product_id`; + const messagesService = app.get(MessagesService); + await messagesService.create(messages); - const myResult1 = results.find((r) => r.code === code1); - expect(myResult1.update_count).toBe(2); + // Then the import is called + expect(importSpy).toHaveBeenCalled(); + + // Update events are created for all codes + const events = + await sql`SELECT * FROM product_update_event WHERE message->>'code' IN ${sql( + [code1, code2], + )}`; + + expect(events).toHaveLength(2); }); }); }); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index 05bb2ce..545c1f7 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -26,7 +26,7 @@ export class MessagesService { return isNaN(time.getTime()) ? new Date() : time; } - async create(messages: any[]) { + async create(messages: any[], initialImport = false) { // Strip out any \u0000 characters as PostgresSQL can't cope with them const messageJson = JSON.stringify(messages); if (messageJson.includes('\\u0000')) { @@ -57,17 +57,8 @@ export class MessagesService { on conflict (code) do nothing`; - const productCodes = [ - ...new Set( - messages - .filter((m) => !m.message.diffs?.initial_import) // Don't trigger product updates on initial import - .map((m) => m.message.code), - ), - ]; - this.logger.log( - `Received ${messages.length} events with ${productCodes.length} products to import`, - ); - if (productCodes.length) { + if (!initialImport) { + const productCodes = [...new Set(messages.map((m) => m.message.code))]; const filter = { code: { $in: productCodes } }; await this.importService.importWithFilter(filter, ProductSource.EVENT); } @@ -94,5 +85,7 @@ export class MessagesService { update_count = product_update.update_count + EXCLUDED.update_count`; await this.settings.setLastMessageId(messages[messages.length - 1].id); + + this.logger.log(`Received ${messages.length} events`); } } diff --git a/src/domain/services/redis.listener.spec.ts b/src/domain/services/redis.listener.spec.ts index 810e0e2..a81ae02 100644 --- a/src/domain/services/redis.listener.spec.ts +++ b/src/domain/services/redis.listener.spec.ts @@ -7,12 +7,13 @@ import { ImportService } from './import.service'; import { SettingsService } from './settings.service'; import { RedisListener } from './redis.listener'; import { setTimeout } from 'timers/promises'; +import { MessagesService } from './messages.service'; // Allow a little time for the testcontainer to start jest.setTimeout(300000); describe('receiveMessages', () => { - it('should call importwithfilter when a message is received', async () => { + it('should call importWithFilter when a message is received', async () => { await createTestingModule([DomainModule], async (app) => { // GIVEN: Redis is running const redis = await new GenericContainer('redis') @@ -81,100 +82,29 @@ describe('receiveMessages', () => { }); describe('processMessages', () => { - it('should not call importwithfilter for messages that contain the initial_import diff', async () => { + it('should convert json properties to objects', async () => { await createTestingModule([DomainModule], async (app) => { - const importService = app.get(ImportService); - const importSpy = jest - .spyOn(importService, 'importWithFilter') - .mockImplementation(); - - const code1 = randomCode(); - const code2 = randomCode(); - let idCount = 0; - const nextId = () => `${Date.now()}-${idCount++}`; - const messages = [ - { - id: nextId(), - message: { - code: code1, - }, - }, - { - id: nextId(), - message: { - code: code2, - // Note JSON properties in Redis come in as strings - diffs: JSON.stringify({ - initial_import: 1, - }), - }, - }, - ]; - - const redisListener = app.get(RedisListener); - await redisListener.processMessages(messages); - // Then the import is called only once for code1 - const codes = importSpy.mock.calls[0][0].code.$in; - expect(codes).toHaveLength(1); - expect(codes[0]).toBe(code1); - - // Update events are created for all codes - const events = - await sql`SELECT * FROM product_update_event WHERE message->>'code' IN ${sql( - [code1, code2], - )}`; - - expect(events).toHaveLength(2); - }); - }); - - it('should not call importwithfilter at all if all messages contain the initial_import diff', async () => { - await createTestingModule([DomainModule], async (app) => { - const importService = app.get(ImportService); - const importSpy = jest - .spyOn(importService, 'importWithFilter') + const messagesService = app.get(MessagesService); + const createSpy = jest + .spyOn(messagesService, 'create') .mockImplementation(); - const code1 = randomCode(); - const code2 = randomCode(); - let idCount = 0; - const nextId = () => `${Date.now()}-${idCount++}`; const messages = [ { - id: nextId(), + id: `0-0`, message: { - code: code1, - // Note JSON properties in Redis come in as strings - diffs: JSON.stringify({ - initial_import: 1, - }), - }, - }, - { - id: nextId(), - message: { - code: code2, - // Note JSON properties in Redis come in as strings - diffs: JSON.stringify({ - initial_import: 1, - }), + code: 'test', + diffs: `{"action":"update"}`, }, }, ]; const redisListener = app.get(RedisListener); await redisListener.processMessages(messages); - // Then the import is not called at all - expect(importSpy).toHaveBeenCalledTimes(0); - - // Update events are created for all codes - const events = - await sql`SELECT * FROM product_update_event WHERE message->>'code' IN ${sql([ - code1, - code2, - ])}`; - expect(events).toHaveLength(2); + // Then create is called with a real object + const diffs = createSpy.mock.calls[0][0][0].message.diffs; + expect(diffs.action).toBe('update'); }); }); }); diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index 0248336..bfc30f2 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -47,44 +47,43 @@ describe('product_update', () => { // Create some messages let idCount = 0; const nextId = () => `${Date.now()}-${idCount++}`; - await messages.create([ - { - id: nextId(), - message: { - code: code1, - action: 'updated', - user_id: 'user1', - diffs: { initial_import: 1 }, + await messages.create( + [ + { + id: nextId(), + message: { + code: code1, + action: 'updated', + user_id: 'user1', + }, }, - }, - { - id: nextId(), - message: { - code: code1, - action: 'updated', - user_id: 'user1', - diffs: { initial_import: 1 }, + { + id: nextId(), + message: { + code: code1, + action: 'updated', + user_id: 'user1', + }, }, - }, - { - id: nextId(), - message: { - code: code1, - action: 'updated', - user_id: 'user1', - diffs: { initial_import: 1 }, + { + id: nextId(), + message: { + code: code1, + action: 'updated', + user_id: 'user1', + }, }, - }, - { - id: nextId(), - message: { - code: code2, - action: 'updated', - user_id: 'user1', - diffs: { initial_import: 1 }, + { + id: nextId(), + message: { + code: code2, + action: 'updated', + user_id: 'user1', + }, }, - }, - ]); + ], + true, + ); // Use viewer user await withViewUser(async (viewer) => { @@ -120,23 +119,24 @@ describe('product_update', () => { code: code1, action: action1, user_id: 'user1', - diffs: { initial_import: 1 }, }, }, ]); // Add another message - await messages.create([ - { - id: nextId(), - message: { - code: code1, - action: action1, - user_id: 'user1', - diffs: { initial_import: 1 }, + await messages.create( + [ + { + id: nextId(), + message: { + code: code1, + action: action1, + user_id: 'user1', + }, }, - }, - ]); + ], + true, + ); // Use viewer user await withViewUser(async (viewer) => { From b248d3616e270e363f2d9cbfd4cf5324352e7640 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 14:28:42 +0100 Subject: [PATCH 23/31] Tidy up Signed-off-by: John Gomersall --- src/domain/services/messages.service.ts | 9 ++-- src/domain/services/redis.listener.ts | 54 +++++++++++-------- src/domain/services/settings.service.ts | 2 +- src/main.ts | 6 +++ .../Migration20240719101700-redis.ts | 1 + 5 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index 545c1f7..f6b1d57 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -10,10 +10,7 @@ const nulRegex = /\\u0000/g; export class MessagesService { private logger = new Logger(MessagesService.name); - constructor( - private readonly importService: ImportService, - private readonly settings: SettingsService, - ) {} + constructor(private readonly importService: ImportService) {} static messageTime(message: any) { // First preference is to use timestamp in the message @@ -33,9 +30,11 @@ export class MessagesService { messages = JSON.parse(messageJson.replace(nulRegex, '')); } + const receivedAt = new Date(); await sql`INSERT into product_update_event ${sql( messages.map((m) => ({ id: m.id, + received_at: receivedAt, updated_at: MessagesService.messageTime(m), message: m.message, })), @@ -84,8 +83,6 @@ export class MessagesService { do update set update_count = product_update.update_count + EXCLUDED.update_count`; - await this.settings.setLastMessageId(messages[messages.length - 1].id); - this.logger.log(`Received ${messages.length} events`); } } diff --git a/src/domain/services/redis.listener.ts b/src/domain/services/redis.listener.ts index 5bba1e2..8d40e97 100644 --- a/src/domain/services/redis.listener.ts +++ b/src/domain/services/redis.listener.ts @@ -29,8 +29,8 @@ export class RedisListener { async receiveMessages() { const lastMessageId = await this.settings.getLastMessageId(); if (!this.client.isOpen) return; - this.client - .xRead( + try { + const keys = await this.client.xRead( commandOptions({ isolated: true, }), @@ -47,28 +47,36 @@ export class RedisListener { COUNT: 1000, BLOCK: 5000, }, - ) - .then(async (keys: string | any[]) => { - if (keys?.length) { - const messages = keys[0].messages; - if (messages?.length) { - /** Message looks like this: - { - code: "0850026029062", - flavor: "off", - user_id: "stephane", - action: "updated", - comment: "Modification : Remove changes", - diffs: "{\"fields\":{\"change\":[\"categories\"],\"delete\":[\"product_name\",\"product_name_es\"]}}", - } - */ - await this.processMessages(messages); - } + ); + if (keys?.length) { + const messages = keys[0].messages; + if (messages?.length) { + /** Message looks like this: + { + code: "0850026029062", + flavor: "off", + user_id: "stephane", + action: "updated", + comment: "Modification : Remove changes", + diffs: "{\"fields\":{\"change\":[\"categories\"],\"delete\":[\"product_name\",\"product_name_es\"]}}", + } + */ + await this.processMessages(messages); + await this.settings.setLastMessageId( + messages[messages.length - 1].id, + ); } - setTimeout(() => { - this.receiveMessages(); - }, 0); - }); + } + setTimeout(() => { + this.receiveMessages(); + }, 0); + } catch (e) { + this.logger.error(e); + // Try again in 10 seconds + setTimeout(() => { + this.receiveMessages(); + }, 10000); + } } async processMessages(messages: any[]) { diff --git a/src/domain/services/settings.service.ts b/src/domain/services/settings.service.ts index 8e7a2be..6a24e83 100644 --- a/src/domain/services/settings.service.ts +++ b/src/domain/services/settings.service.ts @@ -20,7 +20,7 @@ export class SettingsService { async getLastMessageId() { return ( - (await sql`SELECT last_message_id FROM settings`)[0].last_message_id || + (await sql`SELECT last_message_id FROM settings`)[0]?.last_message_id || '$' ); } diff --git a/src/main.ts b/src/main.ts index 6da5b6e..2e46cb4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -5,9 +5,15 @@ import { NestFactory } from '@nestjs/core'; import { AppModule } from './app.module'; import { MikroORM } from '@mikro-orm/core'; import { LogLevel } from '@nestjs/common'; +import { json, urlencoded } from 'express'; async function bootstrap() { const app = await NestFactory.create(AppModule); + + // Accept large payloads for event migration + app.use(json({ limit: '50mb' })); + app.use(urlencoded({ extended: true, limit: '50mb' })); + app.useLogger([(process.env['LOG_LEVEL'] as LogLevel) || 'log']); // Run migrations if needed. Note may need to change this if multiple containers are used for scaling diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index 74fb9ed..e9b2b02 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -6,6 +6,7 @@ export class Migration20240719101700Redis extends Migration { async up(): Promise { this.addSql(`CREATE TABLE IF NOT EXISTS product_update_event ( id text NOT NULL PRIMARY KEY, + received_at timestamptz NOT NULL, updated_at timestamptz NOT NULL, message jsonb NOT NULL)`); From cab7214aed38085eef748204f5fb692b56cf06bd Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 15:31:57 +0100 Subject: [PATCH 24/31] Incorporate revision into table Signed-off-by: John Gomersall --- src/app.controller.spec.ts | 1 + src/domain/services/messages.service.spec.ts | 23 ++++++++------ src/domain/services/messages.service.ts | 30 ++++++++++--------- src/domain/services/redis.listener.spec.ts | 3 +- src/domain/services/views.spec.ts | 6 ++++ .../Migration20240719101700-redis.ts | 23 ++++++++++---- 6 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/app.controller.spec.ts b/src/app.controller.spec.ts index 0355bd8..9572d1b 100644 --- a/src/app.controller.spec.ts +++ b/src/app.controller.spec.ts @@ -16,6 +16,7 @@ describe('productupdate', () => { const updates = [ { code: code1, + rev: 1, }, ]; diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 56cd1e2..ffa726e 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -38,7 +38,7 @@ describe('messageTime', () => { let idCount = 0; describe('create', () => { - it('should ignore duplicate events', async () => { + it('should load duplicate events', async () => { await createTestingModule([DomainModule], async (app) => { const messages = app.get(MessagesService); const code1 = randomCode(); @@ -66,8 +66,7 @@ describe('create', () => { const result = await sql`SELECT * FROM product_update_event WHERE message->>'code' = ${code1}`; - expect(result).toHaveLength(1); - expect(result[0].message.action).toBe('created'); + expect(result).toHaveLength(2); }); }); @@ -164,6 +163,7 @@ describe('create', () => { code: code1, action: 'created', user_id: 'test', + rev: 1, }, }, { @@ -172,6 +172,7 @@ describe('create', () => { code: code1, action: 'created', user_id: 'test', + rev: 2, }, }, { @@ -180,6 +181,7 @@ describe('create', () => { code: code1, action: 'created', user_id: 'test', + rev: 2, // Duplicate }, }, { @@ -188,6 +190,7 @@ describe('create', () => { code: code2, action: 'created', user_id: 'test', + rev: 1, }, }, ], @@ -197,15 +200,15 @@ describe('create', () => { const results = await sql`SELECT * from product_update join product on product.id = product_update.product_id`; - const myResult1 = results.find( + const myResult1 = results.filter( (r) => r.owners_tags === owner1 && r.code === code1, ); - expect(myResult1.update_count).toBe(3); + expect(myResult1).toHaveLength(2); - const myResult2 = results.find( + const myResult2 = results.filter( (r) => r.owners_tags === owner1 && r.code === code2, ); - expect(myResult2.update_count).toBe(1); + expect(myResult2).toHaveLength(1); }); }); @@ -232,6 +235,7 @@ describe('create', () => { code: code1, action: action1, user_id: 'test', + rev: 1, }, }, ], @@ -247,6 +251,7 @@ describe('create', () => { code: code1, action: action1, user_id: 'test', + rev: 2, }, }, ], @@ -256,8 +261,8 @@ describe('create', () => { const results = await sql`SELECT * from product_update join product on product.id = product_update.product_id`; - const myResult1 = results.find((r) => r.code === code1); - expect(myResult1.update_count).toBe(2); + const myResult1 = results.filter((r) => r.code === code1); + expect(myResult1).toHaveLength(2); }); }); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index f6b1d57..db0c416 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -1,7 +1,6 @@ import { Injectable, Logger } from '@nestjs/common'; import sql from '../../db'; import { ImportService } from './import.service'; -import { SettingsService } from './settings.service'; import { ProductSource } from '../enums/product-source'; const nulRegex = /\\u0000/g; @@ -31,21 +30,22 @@ export class MessagesService { } const receivedAt = new Date(); - await sql`INSERT into product_update_event ${sql( + const insertResult = await sql`INSERT into product_update_event ${sql( messages.map((m) => ({ - id: m.id, + message_id: m.id, received_at: receivedAt, updated_at: MessagesService.messageTime(m), message: m.message, })), - )} ON CONFLICT DO NOTHING`; + )} RETURNING (id)`; - const messageIds = messages.map((m) => m.id); + const messageIds = insertResult.map((m) => m.id); await sql`insert into contributor (code) select distinct message->>'user_id' from product_update_event where id in ${sql(messageIds)} + and not exists (select * from contributor where code = message->>'user_id') on conflict (code) do nothing`; @@ -53,6 +53,7 @@ export class MessagesService { select distinct message->>'action' from product_update_event where id in ${sql(messageIds)} + and not exists (select * from update_type where code = message->>'action') on conflict (code) do nothing`; @@ -63,25 +64,26 @@ export class MessagesService { } // Update counts on product_update after products have been imported - await sql`INSERT INTO product_update + await sql`INSERT INTO product_update ( + product_id, + revision, + updated_date, + update_type_id, + contributor_id, + event_id) SELECT p.id, + (pe.message->>'rev')::int, date(pe.updated_at at time zone 'UTC') updated_day, update_type.id, contributor.id, - count(*) update_count + pe.id FROM product_update_event pe JOIN product p on p.code = pe.message->>'code' join contributor on contributor.code = pe.message->>'user_id' join update_type on update_type.code = pe.message->>'action' where pe.id in ${sql(messageIds)} - GROUP BY p.id, - date(pe.updated_at at time zone 'UTC'), - update_type.id, - contributor.id - on conflict (updated_date,product_id,update_type_id,contributor_id) - do update set - update_count = product_update.update_count + EXCLUDED.update_count`; + on conflict (product_id,revision) DO NOTHING`; this.logger.log(`Received ${messages.length} events`); } diff --git a/src/domain/services/redis.listener.spec.ts b/src/domain/services/redis.listener.spec.ts index a81ae02..eb256af 100644 --- a/src/domain/services/redis.listener.spec.ts +++ b/src/domain/services/redis.listener.spec.ts @@ -42,6 +42,7 @@ describe('receiveMessages', () => { // When: A message is sent const messageId = await client.xAdd('product_updates_off', '*', { code: code1, + rev: '1', }); // Wait for message to be delivered @@ -71,7 +72,7 @@ describe('receiveMessages', () => { await sql`SELECT * FROM product_update_event WHERE message->>'code' = ${code1}`; expect(events).toHaveLength(1); - expect(events[0].id).toBe(messageId); + expect(events[0].message_id).toBe(messageId); } finally { await client.quit(); await redisListener.stopRedisConsumer(); diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index bfc30f2..9e54344 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -55,6 +55,7 @@ describe('product_update', () => { code: code1, action: 'updated', user_id: 'user1', + rev: 1, }, }, { @@ -63,6 +64,7 @@ describe('product_update', () => { code: code1, action: 'updated', user_id: 'user1', + rev: 2, }, }, { @@ -71,6 +73,7 @@ describe('product_update', () => { code: code1, action: 'updated', user_id: 'user1', + rev: 3, }, }, { @@ -79,6 +82,7 @@ describe('product_update', () => { code: code2, action: 'updated', user_id: 'user1', + rev: 1, }, }, ], @@ -119,6 +123,7 @@ describe('product_update', () => { code: code1, action: action1, user_id: 'user1', + rev: 1, }, }, ]); @@ -132,6 +137,7 @@ describe('product_update', () => { code: code1, action: action1, user_id: 'user1', + rev: 2, }, }, ], diff --git a/src/migrations/Migration20240719101700-redis.ts b/src/migrations/Migration20240719101700-redis.ts index e9b2b02..a29d7df 100644 --- a/src/migrations/Migration20240719101700-redis.ts +++ b/src/migrations/Migration20240719101700-redis.ts @@ -5,7 +5,8 @@ import { VIEW_PASSWORD, VIEW_USER } from '../constants'; export class Migration20240719101700Redis extends Migration { async up(): Promise { this.addSql(`CREATE TABLE IF NOT EXISTS product_update_event ( - id text NOT NULL PRIMARY KEY, + id bigserial NOT NULL PRIMARY KEY, + message_id text NOT NULL, received_at timestamptz NOT NULL, updated_at timestamptz NOT NULL, message jsonb NOT NULL)`); @@ -22,15 +23,17 @@ export class Migration20240719101700Redis extends Migration { constraint action_pkey primary key (id), constraint action_code unique (code))`); - this.addSql(`INSERT INTO update_type (code) VALUES ('created'), ('updated'), ('archived'), ('unarchived'), ('deleted'), ('unknown')`); + this.addSql(`INSERT INTO update_type (code) VALUES ('created'), ('updated'), ('archived'), ('unarchived'), ('deleted'), ('reprocessed'), ('unknown')`); this.addSql(`CREATE TABLE IF NOT EXISTS product_update ( product_id int, + revision int, updated_date date, update_type_id int, contributor_id int, - update_count int, - constraint product_update_pkey primary key (updated_date, product_id, update_type_id, contributor_id))`); + event_id bigint, + constraint product_update_pkey primary key (product_id, revision))`); + this.addSql('create index product_update_updated_date_index on product_update (updated_date);'); this.addSql('create schema if not exists views;'); this.addSql(`CREATE USER ${VIEW_USER} PASSWORD '${VIEW_PASSWORD}'`); @@ -39,7 +42,7 @@ export class Migration20240719101700Redis extends Migration { SELECT pu.updated_date, p.owners_tags owner_tag, ut.code update_type, - sum(pu.update_count) update_count, + count(*) update_count, count(DISTINCT pu.product_id) product_count FROM product_update pu JOIN product p ON p.id = pu.product_id @@ -50,4 +53,14 @@ export class Migration20240719101700Redis extends Migration { this.addSql(`GRANT USAGE ON SCHEMA views TO ${VIEW_USER}`); this.addSql(`GRANT SELECT ON views.product_updates_by_owner TO ${VIEW_USER}`); } + async down(): Promise { + this.addSql(`drop owned by ${VIEW_USER}`); + this.addSql(`DROP ROLE ${VIEW_USER}`); + this.addSql(`drop view views.product_updates_by_owner`); + this.addSql(`drop schema views`); + this.addSql(`drop table product_update CASCADE`); + this.addSql(`drop table contributor CASCADE`); + this.addSql(`drop table update_type CASCADE`); + this.addSql(`drop table product_update_event CASCADE`); + } } From bbe530bde7bd93f430cc9c6c016f2c8eb8381a0e Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 16:40:34 +0100 Subject: [PATCH 25/31] Fix TODO Signed-off-by: John Gomersall --- src/domain/services/messages.service.spec.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index ffa726e..2549813 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -127,9 +127,12 @@ describe('create', () => { true, ); - const result = await sql`SELECT * FROM contributor WHERE code = ${user2}`; - expect(result).toHaveLength(1); - // TODO: test there are no wasted ids + const result = await sql`SELECT * FROM contributor WHERE code in ${sql([ + user1, + user2, + ])} order by id`; + expect(result).toHaveLength(2); + expect(result[1].id).toBe(result[0].id + 1); }); }); From 90072411633e13c422c4e6a97c3b7d135b75964b Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 17:01:49 +0100 Subject: [PATCH 26/31] Increase test timeout Signed-off-by: John Gomersall --- src/domain/services/views.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index 9e54344..594c23c 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -5,6 +5,9 @@ import { MessagesService } from './messages.service'; import { VIEW_PASSWORD, VIEW_USER } from '../../constants'; import { DomainModule } from '../domain.module'; +// Allow a little time for the testContainers to start +jest.setTimeout(300000); + async function withViewUser( action: (viewer: postgres.Sql) => Promise, ) { From 93e1d807d2c1da07927c5c6f46b68d9adc76c7fd Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 17:13:46 +0100 Subject: [PATCH 27/31] Ensure tests fail if they directly call MongoDB Signed-off-by: John Gomersall --- src/domain/services/views.spec.ts | 26 +++++++++++++------------- test/global-setup.ts | 3 +++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/domain/services/views.spec.ts b/src/domain/services/views.spec.ts index 594c23c..904bb6f 100644 --- a/src/domain/services/views.spec.ts +++ b/src/domain/services/views.spec.ts @@ -5,9 +5,6 @@ import { MessagesService } from './messages.service'; import { VIEW_PASSWORD, VIEW_USER } from '../../constants'; import { DomainModule } from '../domain.module'; -// Allow a little time for the testContainers to start -jest.setTimeout(300000); - async function withViewUser( action: (viewer: postgres.Sql) => Promise, ) { @@ -119,17 +116,20 @@ describe('product_update', () => { // Create an existing message let idCount = 0; const nextId = () => `${Date.now()}-${idCount++}`; - await messages.create([ - { - id: nextId(), - message: { - code: code1, - action: action1, - user_id: 'user1', - rev: 1, + await messages.create( + [ + { + id: nextId(), + message: { + code: code1, + action: action1, + user_id: 'user1', + rev: 1, + }, }, - }, - ]); + ], + true, + ); // Add another message await messages.create( diff --git a/test/global-setup.ts b/test/global-setup.ts index 91ee64b..309f7fa 100644 --- a/test/global-setup.ts +++ b/test/global-setup.ts @@ -16,6 +16,9 @@ export default async function () { // We don't use redis in the tests process.env.REDIS_URL = ''; + // Prevent tests from calling directly to MongoDB + process.env.MONGO_URI = ''; + // Tried running migrations with the API but doesn't work because // of the way Jest mocks things. Even importing MikroORM is enough to break things. // https://github.com/mikro-orm/mikro-orm/discussions/3795 From 4b2775eadbb350d6794e0d28e8cc6893a3b6c733 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 17:29:29 +0100 Subject: [PATCH 28/31] Add index for users and owners Signed-off-by: John Gomersall --- src/domain/entities/product.ts | 4 ++-- src/migrations/.snapshot-query.json | 18 ++++++++++++++++++ src/migrations/Migration20240809162441.ts | 15 +++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 src/migrations/Migration20240809162441.ts diff --git a/src/domain/entities/product.ts b/src/domain/entities/product.ts index e520477..666d3d1 100644 --- a/src/domain/entities/product.ts +++ b/src/domain/entities/product.ts @@ -23,10 +23,10 @@ export class Product { @Property({ columnType: 'timestamptz' }) lastModified?: Date; - @Property() + @Property({ index: true }) creator?: string; - @Property() + @Property({ index: true }) ownersTags?: string; @Property() diff --git a/src/migrations/.snapshot-query.json b/src/migrations/.snapshot-query.json index f1a387c..0f6cf12 100644 --- a/src/migrations/.snapshot-query.json +++ b/src/migrations/.snapshot-query.json @@ -158,6 +158,24 @@ "primary": false, "unique": false }, + { + "columnNames": [ + "creator" + ], + "composite": false, + "keyName": "product_creator_index", + "primary": false, + "unique": false + }, + { + "columnNames": [ + "owners_tags" + ], + "composite": false, + "keyName": "product_owners_tags_index", + "primary": false, + "unique": false + }, { "columnNames": [ "last_update_id" diff --git a/src/migrations/Migration20240809162441.ts b/src/migrations/Migration20240809162441.ts new file mode 100644 index 0000000..9fbeb59 --- /dev/null +++ b/src/migrations/Migration20240809162441.ts @@ -0,0 +1,15 @@ +import { Migration } from '@mikro-orm/migrations'; + +export class Migration20240809162441 extends Migration { + + async up(): Promise { + this.addSql('create index "product_creator_index" on "query"."product" ("creator");'); + this.addSql('create index "product_owners_tags_index" on "query"."product" ("owners_tags");'); + } + + async down(): Promise { + this.addSql('drop index "query"."product_creator_index";'); + this.addSql('drop index "query"."product_owners_tags_index";'); + } + +} From fdb8d01397a4e3045ea72650ef5d267ddc10bb76 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 18:04:30 +0100 Subject: [PATCH 29/31] Get revision from product if not in message Signed-off-by: John Gomersall --- src/domain/entities/product.ts | 4 ++ src/domain/services/import.service.spec.ts | 2 + src/domain/services/import.service.ts | 3 +- src/domain/services/messages.service.spec.ts | 39 ++++++++++++++++++++ src/domain/services/messages.service.ts | 3 +- src/migrations/.snapshot-query.json | 9 +++++ src/migrations/Migration20240809164713.ts | 13 +++++++ 7 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 src/migrations/Migration20240809164713.ts diff --git a/src/domain/entities/product.ts b/src/domain/entities/product.ts index 666d3d1..7b8a298 100644 --- a/src/domain/entities/product.ts +++ b/src/domain/entities/product.ts @@ -47,6 +47,9 @@ export class Product { @Property() source?: ProductSource; + + @Property() + revision?: number; } export const MAPPED_FIELDS = [ @@ -58,4 +61,5 @@ export const MAPPED_FIELDS = [ 'ingredients_n', 'ingredients_without_ciqual_codes_n', 'ingredients', + 'rev', ]; diff --git a/src/domain/services/import.service.spec.ts b/src/domain/services/import.service.spec.ts index 6fc4304..0e380a1 100644 --- a/src/domain/services/import.service.spec.ts +++ b/src/domain/services/import.service.spec.ts @@ -22,6 +22,7 @@ function testProducts() { code: productIdNew, last_modified_t: lastModified, ingredients_tags: ['test'], + rev: 1, }, { // This one will already exist @@ -126,6 +127,7 @@ describe('importFromMongo', () => { }); expect(ingredientsNew).toHaveLength(1); expect(ingredientsNew[0].value).toBe('test'); + expect(productNew.revision).toBe(1); const ingredientsExisting = await em.find(ProductIngredientsTag, { product: productExisting, diff --git a/src/domain/services/import.service.ts b/src/domain/services/import.service.ts index 77897fe..91351aa 100644 --- a/src/domain/services/import.service.ts +++ b/src/domain/services/import.service.ts @@ -221,7 +221,8 @@ export class ImportService { last_modified = tp.last_modified, last_update_id = ${updateId}, last_updated = ${new Date()}, - source = ${source} + source = ${source}, + revision = (tp.data->>'rev')::int FROM product_temp tp WHERE product.id = tp.id`; this.logger.debug(`Updated ${productResults.count} products`); diff --git a/src/domain/services/messages.service.spec.ts b/src/domain/services/messages.service.spec.ts index 2549813..935e578 100644 --- a/src/domain/services/messages.service.spec.ts +++ b/src/domain/services/messages.service.spec.ts @@ -352,4 +352,43 @@ describe('create', () => { expect(events).toHaveLength(2); }); }); + + // This is just needed for backward compatibility with PO versions that don't send rev in the event + it('should get revision from product if not in message', async () => { + await createTestingModule([DomainModule], async (app) => { + const messages = app.get(MessagesService); + // Create a product + const code1 = randomCode(); + + await sql`INSERT INTO product ${sql([ + { + code: code1, + revision: 123, + }, + ])}`; + + // Create a message with no rev + let idCount = 0; + const nextId = () => `${Date.now()}-${idCount++}`; + await messages.create( + [ + { + id: nextId(), + message: { + code: code1, + action: 'created', + user_id: 'test', + }, + }, + ], + true, + ); + + const results = + await sql`SELECT product_update.revision from product_update join product on product.id = product_update.product_id where code = ${code1}`; + + expect(results).toHaveLength(1); + expect(results[0].revision).toBe(123); + }); + }); }); diff --git a/src/domain/services/messages.service.ts b/src/domain/services/messages.service.ts index db0c416..8f38921 100644 --- a/src/domain/services/messages.service.ts +++ b/src/domain/services/messages.service.ts @@ -64,6 +64,7 @@ export class MessagesService { } // Update counts on product_update after products have been imported + // Note coalesce on rev is only needed for transition if an older version of PO is deployed await sql`INSERT INTO product_update ( product_id, revision, @@ -73,7 +74,7 @@ export class MessagesService { event_id) SELECT p.id, - (pe.message->>'rev')::int, + coalesce((pe.message->>'rev')::int, p.revision), date(pe.updated_at at time zone 'UTC') updated_day, update_type.id, contributor.id, diff --git a/src/migrations/.snapshot-query.json b/src/migrations/.snapshot-query.json index 0f6cf12..ca0ca0f 100644 --- a/src/migrations/.snapshot-query.json +++ b/src/migrations/.snapshot-query.json @@ -144,6 +144,15 @@ "primary": false, "nullable": true, "mappedType": "string" + }, + "revision": { + "name": "revision", + "type": "int", + "unsigned": false, + "autoincrement": false, + "primary": false, + "nullable": true, + "mappedType": "integer" } }, "name": "product", diff --git a/src/migrations/Migration20240809164713.ts b/src/migrations/Migration20240809164713.ts new file mode 100644 index 0000000..9e141f7 --- /dev/null +++ b/src/migrations/Migration20240809164713.ts @@ -0,0 +1,13 @@ +import { Migration } from '@mikro-orm/migrations'; + +export class Migration20240809164713 extends Migration { + async up(): Promise { + this.addSql( + 'alter table "query"."product" add column "revision" int null;', + ); + } + + async down(): Promise { + this.addSql('alter table "query"."product" drop column "revision";'); + } +} From 65561b8baa86961d24fc4ca8d9ac0383fb0ebab8 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Fri, 9 Aug 2024 18:31:09 +0100 Subject: [PATCH 30/31] Always expose port for viewer access Signed-off-by: John Gomersall --- docker-compose.yml | 3 +++ docker/dev.yml | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 7f40b1e..dc069b2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -8,6 +8,9 @@ services: - POSTGRES_PASSWORD - POSTGRES_DB shm_size: ${POSTGRES_SHM_SIZE} + # Always expose port for viewer access + ports: + - "${POSTGRES_PORT:-5512}:5432" volumes: - dbdata:/var/lib/postgresql/data networks: diff --git a/docker/dev.yml b/docker/dev.yml index 83cd403..ee4fcb5 100644 --- a/docker/dev.yml +++ b/docker/dev.yml @@ -1,9 +1,4 @@ services: - query_postgres: - # Expose port locally for testing purposes. - ports: - - "${POSTGRES_PORT:-5512}:5432" - query: build: . image: openfoodfacts-query:dev From 99eca4917232584215ff1b21201355618a867847 Mon Sep 17 00:00:00 2001 From: John Gomersall Date: Mon, 12 Aug 2024 17:54:41 +0100 Subject: [PATCH 31/31] Support running as a deps project Signed-off-by: John Gomersall --- .env | 6 ++++-- .github/workflows/container-deploy.yml | 2 +- Makefile | 4 ++++ docker-compose-run.yml | 6 ++++++ 4 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 docker-compose-run.yml diff --git a/.env b/.env index f5c0aba..930894e 100644 --- a/.env +++ b/.env @@ -1,6 +1,7 @@ -COMPOSE_FILE=docker-compose.yml;docker/dev.yml +COMPOSE_FILE_RUN=docker-compose.yml,docker-compose-run.yml +COMPOSE_FILE=${COMPOSE_FILE_RUN},docker/dev.yml COMPOSE_PROJECT_NAME=off-query -COMPOSE_PATH_SEPARATOR=; +COMPOSE_PATH_SEPARATOR=, RESTART_POLICY=no TAG=latest QUERY_PORT=127.0.0.1:5511 @@ -11,6 +12,7 @@ POSTGRES_USER=productopener POSTGRES_PASSWORD=productopener POSTGRES_SHM_SIZE=256m COMMON_NET_NAME=off_shared_network +# Note when running in a container the following settings are changed to use the internal docker network MONGO_URI=mongodb://localhost:27017 REDIS_URL=redis://localhost:6379 # Log levels are: debug, verbose, log (default), warn, error diff --git a/.github/workflows/container-deploy.yml b/.github/workflows/container-deploy.yml index e0b6bb5..27132cd 100644 --- a/.github/workflows/container-deploy.yml +++ b/.github/workflows/container-deploy.yml @@ -133,7 +133,7 @@ jobs: echo "DOCKER_CLIENT_TIMEOUT=120" >> .env echo "COMPOSE_HTTP_TIMEOUT=120" >> .env echo "COMPOSE_PROJECT_NAME=off-query" >> .env - echo "COMPOSE_PATH_SEPARATOR=;" >> .env + echo "COMPOSE_PATH_SEPARATOR=," >> .env echo "RESTART_POLICY=always" >> .env echo "COMPOSE_FILE=docker-compose.yml" >> .env echo "TAG=sha-${{ github.sha }}" >> .env diff --git a/Makefile b/Makefile index c08a4d3..1a50e83 100644 --- a/Makefile +++ b/Makefile @@ -18,6 +18,10 @@ endif up: run_deps docker compose up --build --wait +# Called by other projects to start this project as a dependency +run: run_deps + COMPOSE_FILE=${COMPOSE_FILE_RUN} docker compose up -d + # This task starts a Postgres database in Docker and then prepares the local environment for development dev: run_deps docker compose up --wait query_postgres diff --git a/docker-compose-run.yml b/docker-compose-run.yml new file mode 100644 index 0000000..3c1ef64 --- /dev/null +++ b/docker-compose-run.yml @@ -0,0 +1,6 @@ +services: + query: + environment: + # Use shared-services MongoDB and REDIS + - MONGO_URI=mongodb://mongodb:27017 + - REDIS_URL=redis://redis:6379