From ce984b84fa07543a7acb3fc5d28f2e4cfa27b377 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Wed, 22 Nov 2017 01:02:44 -0300 Subject: [PATCH 1/9] Method to upgrade schemas in Postgres; Adds: - schemaUpgrade method which check the fields to add and delete from old schema; --- .../Postgres/PostgresStorageAdapter.js | 47 ++++++++++++++++--- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 8d830c6bd4..1529d80d54 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -743,6 +743,37 @@ export class PostgresStorageAdapter { }); } + schemaUpgrade(className, schema) { + debug('schemaUpgrade', { className, schema }); + return this._client.tx('schemaUpgrade', t => + t.any(`SELECT column_name FROM information_schema.columns WHERE table_name = '${className}'`) + ) + .then(columns => { + if (!columns) { + return Promise.resolve(); + } + + const currentColumns = columns.map(item => item.column_name); + const newColumns = Object.keys(schema.fields); + + const columnsToCreate = newColumns.filter(item => currentColumns.indexOf(item) === -1); + const columnsToDelete = currentColumns.filter(item => newColumns.indexOf(item) === -1); + + const promise = []; + + columnsToCreate.forEach(fieldName => { + const type = schema.fields[fieldName]; + promise.push(this.addFieldIfNotExists(className, fieldName, type)); + }); + + if (columnsToDelete.length) { + promise.push(this.deleteFields(className, schema, columnsToDelete)); + } + + return Promise.all(promise); + }); + } + addFieldIfNotExists(className, fieldName, type) { // TODO: Must be revised for invalid logic... debug('addFieldIfNotExists', {className, fieldName, type}); @@ -1581,12 +1612,14 @@ export class PostgresStorageAdapter { performInitialization({ VolatileClassesSchemas }) { debug('performInitialization'); const promises = VolatileClassesSchemas.map((schema) => { - return this.createTable(schema.className, schema).catch((err) => { - if (err.code === PostgresDuplicateRelationError || err.code === Parse.Error.INVALID_CLASS_NAME) { - return Promise.resolve(); - } - throw err; - }); + return this.createTable(schema.className, schema) + .catch((err) => { + if (err.code === PostgresDuplicateRelationError || err.code === Parse.Error.INVALID_CLASS_NAME) { + return Promise.resolve(); + } + throw err; + }) + .then(() => this.schemaUpgrade(schema.className, schema)); }); return Promise.all(promises) .then(() => { @@ -1741,4 +1774,4 @@ function literalizeRegexPart(s) { } export default PostgresStorageAdapter; -module.exports = PostgresStorageAdapter; // Required for tests +module.exports = PostgresStorageAdapter; // Required for tests \ No newline at end of file From c87d4d51bcc7f0439c68c75bea3fa129c016b1ff Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 23 Nov 2017 13:22:12 -0300 Subject: [PATCH 2/9] Remove the columns delete in schemaUpgrade method; --- src/Adapters/Storage/Postgres/PostgresStorageAdapter.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 1529d80d54..d490e5e7e8 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -757,7 +757,6 @@ export class PostgresStorageAdapter { const newColumns = Object.keys(schema.fields); const columnsToCreate = newColumns.filter(item => currentColumns.indexOf(item) === -1); - const columnsToDelete = currentColumns.filter(item => newColumns.indexOf(item) === -1); const promise = []; @@ -766,10 +765,6 @@ export class PostgresStorageAdapter { promise.push(this.addFieldIfNotExists(className, fieldName, type)); }); - if (columnsToDelete.length) { - promise.push(this.deleteFields(className, schema, columnsToDelete)); - } - return Promise.all(promise); }); } From 9ed3536ebdc3a9d0356bd802e49a85def96b086a Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Fri, 24 Nov 2017 16:27:37 -0300 Subject: [PATCH 3/9] ESLint fix and PostgresStorageAdapter.schemaUpgrade spec test Adds: - Add PostgresStorageAdapter.schemaUpgrade spec tests: creates a table, simulates the addition of a new field and checks if its present in the database Chores: - ESLint eol-last fix; --- spec/PostgresStorageAdapter.spec.js | 36 +++++++++++++++++++ .../Postgres/PostgresStorageAdapter.js | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index 18e7c83ad7..37d731e835 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -1,6 +1,17 @@ const PostgresStorageAdapter = require('../src/Adapters/Storage/Postgres/PostgresStorageAdapter'); const databaseURI = 'postgres://localhost:5432/parse_server_postgres_adapter_test_database'; +const getColumns = (client, className) => { + return client.any(`SELECT column_name FROM information_schema.columns WHERE table_name = '${className}'`) + .then(columns => { + if (!columns) { + return []; + } + + return columns.map(item => item.column_name); + }); +}; + describe_only_db('postgres')('PostgresStorageAdapter', () => { beforeEach(done => { const adapter = new PostgresStorageAdapter({ uri: databaseURI }) @@ -19,4 +30,29 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { expect(adapter._client.$pool.ending).toEqual(true); done(); }); + + it('schemaUpgrade, upgrade the database schema when schema change', done => { + const adapter = new PostgresStorageAdapter({ uri: databaseURI }); + const client = adapter._client; + const className = '_PushStatus'; + const schema = { + fields: { + "pushTime": { type: 'String' }, + "source": { type: 'String' }, // rest or webui + "query": { type: 'String' }, // the stringified JSON query + }, + }; + + adapter.createTable(className, schema) + .then(() => { + schema.fields.expiration_interval = { type:'Number' }; + return adapter.schemaUpgrade(className, schema); + }) + .then(() => getColumns(client, className)) + .then(columns => { + expect(columns).toContain('expiration_interval'); + done(); + }) + .catch(error => done.fail(error)); + }); }); diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index d490e5e7e8..c7b58d339f 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -1769,4 +1769,4 @@ function literalizeRegexPart(s) { } export default PostgresStorageAdapter; -module.exports = PostgresStorageAdapter; // Required for tests \ No newline at end of file +module.exports = PostgresStorageAdapter; // Required for tests From 248d7c0cf9cc175b0e7f750aa3fb92898636c0f1 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Sat, 25 Nov 2017 00:40:42 -0300 Subject: [PATCH 4/9] Add check columns before and after schema upgrade, and remove the unnecessary piece of code Add: - Check the right columns is present before schema upgrade and the new field is not, then check if the right columns is present and the new field; Remove: - Piece of code unnecessary because it not need to remove diff columns; --- spec/PostgresStorageAdapter.spec.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index 37d731e835..d29087585f 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -44,12 +44,21 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { }; adapter.createTable(className, schema) - .then(() => { + .then(() => getColumns(client, className)) + .then(columns => { + expect(columns).toContain('pushTime'); + expect(columns).toContain('source'); + expect(columns).toContain('query'); + expect(columns).not.toContain('expiration_interval'); + schema.fields.expiration_interval = { type:'Number' }; return adapter.schemaUpgrade(className, schema); }) .then(() => getColumns(client, className)) .then(columns => { + expect(columns).toContain('pushTime'); + expect(columns).toContain('source'); + expect(columns).toContain('query'); expect(columns).toContain('expiration_interval'); done(); }) From 3b9acb4e5edf5a7ec784333d43798a08589434ad Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Mon, 4 Dec 2017 22:29:54 -0300 Subject: [PATCH 5/9] Optimize the schemaUpgrade method following @vitaly-t instructions, and more tests; --- spec/PostgresStorageAdapter.spec.js | 40 +++++++++++++++++-- .../Postgres/PostgresStorageAdapter.js | 39 ++++++++++-------- 2 files changed, 58 insertions(+), 21 deletions(-) diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index d29087585f..84fec68917 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -2,9 +2,9 @@ const PostgresStorageAdapter = require('../src/Adapters/Storage/Postgres/Postgre const databaseURI = 'postgres://localhost:5432/parse_server_postgres_adapter_test_database'; const getColumns = (client, className) => { - return client.any(`SELECT column_name FROM information_schema.columns WHERE table_name = '${className}'`) + return client.any('SELECT column_name FROM information_schema.columns WHERE table_name = $', { className }) .then(columns => { - if (!columns) { + if (!columns.length) { return []; } @@ -38,8 +38,8 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { const schema = { fields: { "pushTime": { type: 'String' }, - "source": { type: 'String' }, // rest or webui - "query": { type: 'String' }, // the stringified JSON query + "source": { type: 'String' }, + "query": { type: 'String' }, }, }; @@ -64,4 +64,36 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { }) .catch(error => done.fail(error)); }); + + it('schemaUpgrade, matain correct schema', done => { + const adapter = new PostgresStorageAdapter({ uri: databaseURI }); + const client = adapter._client; + const className = 'Table'; + const schema = { + fields: { + "columnA": { type: 'String' }, + "columnB": { type: 'String' }, + "columnC": { type: 'String' }, + }, + }; + + adapter.createTable(className, schema) + .then(() => getColumns(client, className)) + .then(columns => { + expect(columns).toContain('columnA'); + expect(columns).toContain('columnB'); + expect(columns).toContain('columnC'); + + return adapter.schemaUpgrade(className, schema); + }) + .then(() => getColumns(client, className)) + .then(columns => { + expect(columns.length).toEqual(3); + expect(columns).toContain('columnA'); + expect(columns).toContain('columnB'); + expect(columns).toContain('columnC'); + done(); + }) + .catch(error => done.fail(error)); + }); }); diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index c7b58d339f..2081a90b2c 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -745,28 +745,33 @@ export class PostgresStorageAdapter { schemaUpgrade(className, schema) { debug('schemaUpgrade', { className, schema }); - return this._client.tx('schemaUpgrade', t => - t.any(`SELECT column_name FROM information_schema.columns WHERE table_name = '${className}'`) - ) - .then(columns => { - if (!columns) { - return Promise.resolve(); - } - const currentColumns = columns.map(item => item.column_name); - const newColumns = Object.keys(schema.fields); + return this._client.tx('schemaUpgrade', t => { + return t.any('SELECT column_name FROM information_schema.columns WHERE table_name = $', { className }) + .then(columns => { + if (!columns.length) { + return Promise.resolve(); + } - const columnsToCreate = newColumns.filter(item => currentColumns.indexOf(item) === -1); + const currentColumns = columns.map(item => item.column_name); + const newColumns = Object.keys(schema.fields); - const promise = []; + const columnsToCreate = newColumns.filter(item => currentColumns.indexOf(item) === -1); - columnsToCreate.forEach(fieldName => { - const type = schema.fields[fieldName]; - promise.push(this.addFieldIfNotExists(className, fieldName, type)); - }); + const addFields = []; - return Promise.all(promise); - }); + columnsToCreate.forEach(fieldName => { + const type = schema.fields[fieldName]; + addFields.push(this.addFieldIfNotExists(className, fieldName, type)); + }); + + if (!addFields.length) { + return Promise.resolve(); + } + + return t.batch(addFields); + }) + }); } addFieldIfNotExists(className, fieldName, type) { From 59dacea1ec1b7a32ba514cd63c8ffcdcdf4568e3 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Wed, 6 Dec 2017 23:13:37 -0300 Subject: [PATCH 6/9] If the class does not have any columns and needs an upgrade the code would return without doing so. Fixed this. Chore: - Allows class with no column to be upgraded; - Test for class with no columns being upgraded; --- spec/PostgresStorageAdapter.spec.js | 30 +++++++++++++++++++ .../Postgres/PostgresStorageAdapter.js | 4 --- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index 84fec68917..4151a7a5f5 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -96,4 +96,34 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { }) .catch(error => done.fail(error)); }); + + it('Create a table without columns and upgrade with columns', done => { + const adapter = new PostgresStorageAdapter({ uri: databaseURI }); + const client = adapter._client; + const className = 'EmptyTable'; + let schema = {}; + + adapter.createTable(className, schema) + .then(() => getColumns(client, className)) + .then(columns => { + expect(columns.length).toBe(0); + + schema = { + fields: { + "columnA": { type: 'String' }, + "columnB": { type: 'String' } + }, + }; + + return adapter.schemaUpgrade(className, schema); + }) + .then(() => getColumns(client, className)) + .then(columns => { + expect(columns.length).toEqual(2); + expect(columns).toContain('columnA'); + expect(columns).toContain('columnB'); + done(); + }) + .catch(error => done.fail(error)); + }) }); diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 2081a90b2c..30c1ec6381 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -749,10 +749,6 @@ export class PostgresStorageAdapter { return this._client.tx('schemaUpgrade', t => { return t.any('SELECT column_name FROM information_schema.columns WHERE table_name = $', { className }) .then(columns => { - if (!columns.length) { - return Promise.resolve(); - } - const currentColumns = columns.map(item => item.column_name); const newColumns = Object.keys(schema.fields); From 6f5db46d414d81fa9d8e694b140786aa8dcde597 Mon Sep 17 00:00:00 2001 From: Vitaly Tomilov Date: Sat, 30 Dec 2017 17:08:18 +0000 Subject: [PATCH 7/9] Update PostgresStorageAdapter.js rewriting method schemaUpgrade --- .../Postgres/PostgresStorageAdapter.js | 40 +++++++------------ 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 30c1ec6381..0c0e55890b 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -743,38 +743,27 @@ export class PostgresStorageAdapter { }); } - schemaUpgrade(className, schema) { + schemaUpgrade(className, schema, conn) { debug('schemaUpgrade', { className, schema }); - - return this._client.tx('schemaUpgrade', t => { - return t.any('SELECT column_name FROM information_schema.columns WHERE table_name = $', { className }) - .then(columns => { - const currentColumns = columns.map(item => item.column_name); - const newColumns = Object.keys(schema.fields); - - const columnsToCreate = newColumns.filter(item => currentColumns.indexOf(item) === -1); - - const addFields = []; - - columnsToCreate.forEach(fieldName => { - const type = schema.fields[fieldName]; - addFields.push(this.addFieldIfNotExists(className, fieldName, type)); - }); - - if (!addFields.length) { - return Promise.resolve(); - } - - return t.batch(addFields); - }) + conn = conn || this._client; + const self = this; + + return conn.tx('schema-upgrade', function * (t) { + const columns = yield t.map('SELECT column_name FROM information_schema.columns WHERE table_name = $', { className }, a => a.column_name); + const newColumns = Object.keys(schema.fields) + .filter(item => columns.indexOf(item) === -1) + .map(fieldName => self.addFieldIfNotExists(className, fieldName, schema.fields[fieldName], t)); + + yield t.batch(newColumns); }); } - addFieldIfNotExists(className, fieldName, type) { + addFieldIfNotExists(className, fieldName, type, conn) { // TODO: Must be revised for invalid logic... debug('addFieldIfNotExists', {className, fieldName, type}); + conn = conn || this._client; const self = this; - return this._client.tx('add-field-if-not-exists', function * (t) { + return conn.tx('add-field-if-not-exists', function * (t) { if (type.type !== 'Relation') { try { yield t.none('ALTER TABLE $ ADD COLUMN $ $', { @@ -1606,6 +1595,7 @@ export class PostgresStorageAdapter { } performInitialization({ VolatileClassesSchemas }) { + // TODO: This method needs to be rewritten to make proper use of connections (@vitaly-t) debug('performInitialization'); const promises = VolatileClassesSchemas.map((schema) => { return this.createTable(schema.className, schema) From fe54648b9d88714ab705ffeaa3252f6abb0c07f6 Mon Sep 17 00:00:00 2001 From: Vitaly Tomilov Date: Sun, 31 Dec 2017 01:59:14 +0000 Subject: [PATCH 8/9] Update PostgresStorageAdapter.spec.js --- spec/PostgresStorageAdapter.spec.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index 4151a7a5f5..6592525549 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -2,14 +2,7 @@ const PostgresStorageAdapter = require('../src/Adapters/Storage/Postgres/Postgre const databaseURI = 'postgres://localhost:5432/parse_server_postgres_adapter_test_database'; const getColumns = (client, className) => { - return client.any('SELECT column_name FROM information_schema.columns WHERE table_name = $', { className }) - .then(columns => { - if (!columns.length) { - return []; - } - - return columns.map(item => item.column_name); - }); + return client.map('SELECT column_name FROM information_schema.columns WHERE table_name = $', { className }, a => a.column_name); }; describe_only_db('postgres')('PostgresStorageAdapter', () => { From 6ceee0701830d7d2e7d1d43d5d873f910c045f17 Mon Sep 17 00:00:00 2001 From: Vitaly Tomilov Date: Sun, 31 Dec 2017 02:04:34 +0000 Subject: [PATCH 9/9] Update PostgresStorageAdapter.spec.js --- spec/PostgresStorageAdapter.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index 6592525549..782271b550 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -24,7 +24,7 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { done(); }); - it('schemaUpgrade, upgrade the database schema when schema change', done => { + it('schemaUpgrade, upgrade the database schema when schema changes', done => { const adapter = new PostgresStorageAdapter({ uri: databaseURI }); const client = adapter._client; const className = '_PushStatus'; @@ -58,7 +58,7 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { .catch(error => done.fail(error)); }); - it('schemaUpgrade, matain correct schema', done => { + it('schemaUpgrade, maintain correct schema', done => { const adapter = new PostgresStorageAdapter({ uri: databaseURI }); const client = adapter._client; const className = 'Table';