From d236c9d9425f51a6efd713333c2085a38835a7ee Mon Sep 17 00:00:00 2001 From: Rebecca Ransome Date: Tue, 16 May 2023 16:35:17 +0100 Subject: [PATCH 1/2] Adding logging to the db export service https://eaflood.atlassian.net/browse/WATER-3968 This pull request is just a small part of a larger project that involves exporting all our database schemas, converting them into CSV files, and uploading them to our Amazon S3 bucket. This PR's primary focus is to add logging to the db-export service. To expedite the export process and see the output sooner, we are using a vertical slicing approach, rather than a horizontal one, which means exporting a single table at a time from each schema. From 859fb58226056a8b458a043f8706f18d75948dcf Mon Sep 17 00:00:00 2001 From: Rebecca Ransome Date: Wed, 17 May 2023 14:30:33 +0100 Subject: [PATCH 2/2] Remove successful logging --- app/controllers/data/data.controller.js | 10 ++---- .../db-export/compress-files.service.js | 3 -- .../db-export/export-data-files.service.js | 3 -- .../db-export/send-to-s3-bucket.service.js | 3 +- test/controllers/data/data.controller.test.js | 8 ++--- .../db-export/compress-files.service.test.js | 14 --------- .../export-data-files.service.test.js | 14 --------- .../send-to-s3-bucket.service.test.js | 31 +++---------------- 8 files changed, 10 insertions(+), 76 deletions(-) diff --git a/app/controllers/data/data.controller.js b/app/controllers/data/data.controller.js index 9f1c7760ff..3b177aa5e3 100644 --- a/app/controllers/data/data.controller.js +++ b/app/controllers/data/data.controller.js @@ -23,17 +23,13 @@ async function tearDown (_request, h) { /** * Triggers export of all relevant tables to CSV and then uploads them to S3 */ -async function dbExport (_request, _h) { - global.GlobalNotifier.omg('Starting db export service ') - +async function dbExport (_request, h) { try { await DbExportService.go() - global.GlobalNotifier.omg('Finished export service') - return { status: 'successful' } + return h.response().code(204) } catch (error) { - global.GlobalNotifier.omfg(`Error: ${error.message}`) - return { status: `Error: ${error.message}` } + return Boom.badImplementation(error.message) } } diff --git a/app/services/db-export/compress-files.service.js b/app/services/db-export/compress-files.service.js index a0bbcb992e..80d4e5675c 100644 --- a/app/services/db-export/compress-files.service.js +++ b/app/services/db-export/compress-files.service.js @@ -19,13 +19,10 @@ const zlib = require('node:zlib') */ async function go (filePath) { if (!fs.existsSync(filePath)) { - global.GlobalNotifier.omfg(`ERROR: ${filePath} did not successfully compress to gzip.`) - return false } await _compressFile(filePath) - global.GlobalNotifier.omg(`${filePath} successfully compressed to gzip.`) return `${filePath}.gz` } diff --git a/app/services/db-export/export-data-files.service.js b/app/services/db-export/export-data-files.service.js index 95dd04e935..46a0863aa6 100644 --- a/app/services/db-export/export-data-files.service.js +++ b/app/services/db-export/export-data-files.service.js @@ -23,12 +23,9 @@ async function go (tableConvertedToCsv, tableName) { try { await fs.writeFile(filePath, tableConvertedToCsv) - global.GlobalNotifier.omg(`${tableName} exported successfully`) return filePath } catch (error) { - global.GlobalNotifier.omfg(`${tableName} Export request errored`, error) - return false } } diff --git a/app/services/db-export/send-to-s3-bucket.service.js b/app/services/db-export/send-to-s3-bucket.service.js index 34e8447f1c..817cb4256b 100644 --- a/app/services/db-export/send-to-s3-bucket.service.js +++ b/app/services/db-export/send-to-s3-bucket.service.js @@ -44,10 +44,9 @@ async function _uploadToBucket (params, fileName) { try { await s3Client.send(command) - global.GlobalNotifier.omg(`The file ${fileName} was uploaded successfully`) + return true } catch (error) { - global.GlobalNotifier.omfg(`ERROR uploading file: ${error.message}`) return false } } diff --git a/test/controllers/data/data.controller.test.js b/test/controllers/data/data.controller.test.js index ce03acc382..dff6712afb 100644 --- a/test/controllers/data/data.controller.test.js +++ b/test/controllers/data/data.controller.test.js @@ -74,10 +74,8 @@ describe('Data controller', () => { it('displays the correct message', async () => { const response = await server.inject(options) - const payload = JSON.parse(response.payload) - expect(response.statusCode).to.equal(200) - expect(payload.status).to.equal('successful') + expect(response.statusCode).to.equal(204) }) }) @@ -89,10 +87,8 @@ describe('Data controller', () => { it('displays the error message', async () => { const response = await server.inject(options) - const payload = JSON.parse(response.payload) - expect(payload.status).to.equal('Error: Error') - expect(response.statusCode).to.equal(200) + expect(response.statusCode).to.equal(500) }) }) }) diff --git a/test/services/db-export/compress-files.service.test.js b/test/services/db-export/compress-files.service.test.js index 5906e109da..9a1ba2de24 100644 --- a/test/services/db-export/compress-files.service.test.js +++ b/test/services/db-export/compress-files.service.test.js @@ -3,7 +3,6 @@ // Test framework dependencies const Lab = require('@hapi/lab') const Code = require('@hapi/code') -const Sinon = require('sinon') const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() const { expect } = Code @@ -15,19 +14,8 @@ const fs = require('fs') const CompressFilesService = require('../../../app/services/db-export/compress-files.service.js') describe('Compress files service', () => { - let notifierStub let filePath - beforeEach(() => { - notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } - global.GlobalNotifier = notifierStub - }) - - afterEach(() => { - Sinon.restore() - delete global.GlobalNotifier - }) - describe('when successful', () => { beforeEach(() => { filePath = 'test/fixtures/compress-files.service.csv' @@ -43,7 +31,6 @@ describe('Compress files service', () => { expect(result).to.equal(`${filePath}.gz`) expect(fs.existsSync(`${filePath}.gz`)).to.equal(true) - expect(notifierStub.omg.calledWith(`${filePath} successfully compressed to gzip.`)).to.be.true() }) }) @@ -57,7 +44,6 @@ describe('Compress files service', () => { expect(result).to.equal(false) expect(fs.existsSync((`${filePath}.gz`))).to.equal(false) - expect(notifierStub.omfg.calledWith(`ERROR: ${filePath} did not successfully compress to gzip.`)).to.be.true() }) }) }) diff --git a/test/services/db-export/export-data-files.service.test.js b/test/services/db-export/export-data-files.service.test.js index 0936c95f82..159a77632f 100644 --- a/test/services/db-export/export-data-files.service.test.js +++ b/test/services/db-export/export-data-files.service.test.js @@ -3,7 +3,6 @@ // Test framework dependencies const Lab = require('@hapi/lab') const Code = require('@hapi/code') -const Sinon = require('sinon') const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() const { expect } = Code @@ -49,17 +48,6 @@ const csvValues = [ describe('Export data files service', () => { const tableName = 'billing_charge_categories' - let notifierStub - - beforeEach(() => { - notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } - global.GlobalNotifier = notifierStub - }) - - afterEach(() => { - Sinon.restore() - delete global.GlobalNotifier - }) let filePath @@ -82,7 +70,6 @@ describe('Export data files service', () => { expect(fs.existsSync(filePath)).to.equal(true) expect(returnedResult).to.equal('/tmp/billing_charge_categories.csv') - expect(notifierStub.omg.calledWith('billing_charge_categories exported successfully')).to.be.true() }) }) @@ -92,7 +79,6 @@ describe('Export data files service', () => { const result = await ExportDataFilesService.go(data, tableName) expect(result).to.equal(false) - expect(notifierStub.omfg.calledWith('billing_charge_categories Export request errored')).to.be.true() }) }) }) diff --git a/test/services/db-export/send-to-s3-bucket.service.test.js b/test/services/db-export/send-to-s3-bucket.service.test.js index 3e8c67b259..dfa456e482 100644 --- a/test/services/db-export/send-to-s3-bucket.service.test.js +++ b/test/services/db-export/send-to-s3-bucket.service.test.js @@ -8,9 +8,6 @@ const Sinon = require('sinon') const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() const { expect } = Code -// Test helpers -const path = require('path') - // Things we need to stub const { S3Client, PutObjectCommand } = require('@aws-sdk/client-s3') @@ -19,17 +16,6 @@ const SendToS3BucketService = require('../../../app/services/db-export/send-to-s describe('Send to S3 bucket service', () => { let s3Stub - let notifierStub - - beforeEach(() => { - notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } - global.GlobalNotifier = notifierStub - }) - - afterEach(() => { - Sinon.restore() - delete global.GlobalNotifier - }) describe('when successful', () => { beforeEach(() => { @@ -37,8 +23,11 @@ describe('Send to S3 bucket service', () => { s3Stub = Sinon.stub(S3Client.prototype, 'send') }) + afterEach(() => { + Sinon.restore() + }) + const filePath = 'test/fixtures/compress-files.service.csv' - const fileName = path.basename(filePath) it('uploads a file to the S3 bucket', async () => { await SendToS3BucketService.go(filePath) @@ -56,12 +45,6 @@ describe('Send to S3 bucket service', () => { expect(result).to.equal(true) }) - - it('logs a success message', async () => { - await SendToS3BucketService.go(filePath) - - expect(notifierStub.omg.calledWith(`The file ${fileName} was uploaded successfully`)).to.be.true() - }) }) describe('when unsuccessful', () => { @@ -89,12 +72,6 @@ describe('Send to S3 bucket service', () => { expect(result).to.equal(false) }) - - it('logs the error', async () => { - await SendToS3BucketService.go(filePath) - - expect(notifierStub.omfg.calledWith('ERROR uploading file: Error')).to.be.true() - }) }) }) })