From cb41e4d5a18354ec797beff9a5cbde7bd1ba0b63 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Jul 2024 12:50:33 +0100 Subject: [PATCH 1/3] Fix for CouchDB integration, the typing in it was very poor and wise hiding issues, I've updated how this is implemented so that static typing can catch issues with it. --- .../backend-core/src/db/couch/DatabaseImpl.ts | 5 ++ packages/server/src/integrations/couchdb.ts | 57 ++++++++++--------- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/packages/backend-core/src/db/couch/DatabaseImpl.ts b/packages/backend-core/src/db/couch/DatabaseImpl.ts index 4db63ad695..274c09438d 100644 --- a/packages/backend-core/src/db/couch/DatabaseImpl.ts +++ b/packages/backend-core/src/db/couch/DatabaseImpl.ts @@ -80,6 +80,11 @@ export function DatabaseWithConnection( connection: string, opts?: DatabaseOpts ) { + if (!dbName || !connection) { + throw new Error( + "Unable to create database without database name or connection" + ) + } const db = new DatabaseImpl(dbName, opts, connection) return new DDInstrumentedDatabase(db) } diff --git a/packages/server/src/integrations/couchdb.ts b/packages/server/src/integrations/couchdb.ts index c271fb12b2..823e5023ea 100644 --- a/packages/server/src/integrations/couchdb.ts +++ b/packages/server/src/integrations/couchdb.ts @@ -71,6 +71,9 @@ class CouchDBIntegration implements IntegrationBase { private readonly client: Database constructor(config: CouchDBConfig) { + if (!config.url || !config.database) { + throw new Error("Unable to connect without URL or database") + } this.client = dbCore.DatabaseWithConnection(config.database, config.url) } @@ -79,21 +82,19 @@ class CouchDBIntegration implements IntegrationBase { connected: false, } try { - const result = await this.query("exists", "validation error", {}) - response.connected = result === true + response.connected = await this.query( + () => this.client.exists(), + "validation error" + ) } catch (e: any) { response.error = e.message as string } return response } - async query( - command: string, - errorMsg: string, - query: { json?: object; id?: string } - ) { + async query(operation: () => Promise, errorMsg: string) { try { - return await (this.client as any)[command](query.id || query.json) + return await operation() } catch (err) { console.error(errorMsg, err) throw err @@ -106,18 +107,20 @@ class CouchDBIntegration implements IntegrationBase { async create(query: { json: string | object }) { const parsed = this.parse(query) - return this.query("post", "Error writing to couchDB", { json: parsed }) + return this.query(() => this.client.put(parsed), "Error writing to couchDB") } async read(query: { json: string | object }) { const parsed = this.parse(query) - const result = await this.query("allDocs", "Error querying couchDB", { - json: { - include_docs: true, - ...parsed, - }, - }) - return result.rows.map((row: { doc: object }) => row.doc) + const params = { + include_docs: true, + ...parsed, + } + const result = await this.query( + () => this.client.allDocs(params), + "Error querying couchDB" + ) + return result.rows.map(row => row.doc) } async update(query: { json: string | object }) { @@ -126,22 +129,24 @@ class CouchDBIntegration implements IntegrationBase { const oldDoc = await this.get({ id: parsed._id }) parsed._rev = oldDoc._rev } - return this.query("put", "Error updating couchDB document", { - json: parsed, - }) + return this.query( + () => this.client.put(parsed), + "Error updating couchDB document" + ) } async get(query: { id: string }) { - return this.query("get", "Error retrieving couchDB document by ID", { - id: query.id, - }) + return this.query( + () => this.client.get(query.id), + "Error retrieving couchDB document by ID" + ) } async delete(query: { id: string }) { - const doc = await this.query("get", "Cannot find doc to be deleted", query) - return this.query("remove", "Error deleting couchDB document", { - json: doc, - }) + return this.query( + () => this.client.remove(query.id), + "Error deleting couchDB document" + ) } } From 378bf6d42f6416c4565fb1f580f1b466a4e4cbc7 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Jul 2024 13:40:57 +0100 Subject: [PATCH 2/3] Updating tests. --- packages/server/src/integrations/tests/couchdb.spec.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/server/src/integrations/tests/couchdb.spec.ts b/packages/server/src/integrations/tests/couchdb.spec.ts index f27713a81f..6cb0c438c0 100644 --- a/packages/server/src/integrations/tests/couchdb.spec.ts +++ b/packages/server/src/integrations/tests/couchdb.spec.ts @@ -6,7 +6,6 @@ jest.mock("@budibase/backend-core", () => { ...core.db, DatabaseWithConnection: function () { return { - post: jest.fn(), allDocs: jest.fn().mockReturnValue({ rows: [] }), put: jest.fn(), get: jest.fn().mockReturnValue({ _rev: "a" }), @@ -43,7 +42,7 @@ describe("CouchDB Integration", () => { await config.integration.create({ json: JSON.stringify(doc), }) - expect(config.integration.client.post).toHaveBeenCalledWith(doc) + expect(config.integration.client.put).toHaveBeenCalledWith(doc) }) it("calls the read method with the correct params", async () => { @@ -80,7 +79,6 @@ describe("CouchDB Integration", () => { it("calls the delete method with the correct params", async () => { const id = "1234" await config.integration.delete({ id }) - expect(config.integration.client.get).toHaveBeenCalledWith(id) - expect(config.integration.client.remove).toHaveBeenCalled() + expect(config.integration.client.remove).toHaveBeenCalledWith(id) }) }) From e83c37263d0f828f7dd94927ba1e1aca0270ac5c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Jul 2024 16:59:31 +0100 Subject: [PATCH 3/3] Further simplification. --- packages/server/src/integrations/couchdb.ts | 36 ++++----------------- 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/packages/server/src/integrations/couchdb.ts b/packages/server/src/integrations/couchdb.ts index 823e5023ea..4004bc47c6 100644 --- a/packages/server/src/integrations/couchdb.ts +++ b/packages/server/src/integrations/couchdb.ts @@ -82,32 +82,20 @@ class CouchDBIntegration implements IntegrationBase { connected: false, } try { - response.connected = await this.query( - () => this.client.exists(), - "validation error" - ) + response.connected = await this.client.exists() } catch (e: any) { response.error = e.message as string } return response } - async query(operation: () => Promise, errorMsg: string) { - try { - return await operation() - } catch (err) { - console.error(errorMsg, err) - throw err - } - } - private parse(query: { json: string | object }) { return typeof query.json === "string" ? JSON.parse(query.json) : query.json } async create(query: { json: string | object }) { const parsed = this.parse(query) - return this.query(() => this.client.put(parsed), "Error writing to couchDB") + return await this.client.put(parsed) } async read(query: { json: string | object }) { @@ -116,10 +104,7 @@ class CouchDBIntegration implements IntegrationBase { include_docs: true, ...parsed, } - const result = await this.query( - () => this.client.allDocs(params), - "Error querying couchDB" - ) + const result = await this.client.allDocs(params) return result.rows.map(row => row.doc) } @@ -129,24 +114,15 @@ class CouchDBIntegration implements IntegrationBase { const oldDoc = await this.get({ id: parsed._id }) parsed._rev = oldDoc._rev } - return this.query( - () => this.client.put(parsed), - "Error updating couchDB document" - ) + return await this.client.put(parsed) } async get(query: { id: string }) { - return this.query( - () => this.client.get(query.id), - "Error retrieving couchDB document by ID" - ) + return await this.client.get(query.id) } async delete(query: { id: string }) { - return this.query( - () => this.client.remove(query.id), - "Error deleting couchDB document" - ) + return await this.client.remove(query.id) } }