#4338 pg schema upgrade (#4375)

* Method to upgrade schemas in Postgres; Adds: - schemaUpgrade method which check the fields to add and delete from old schema; * Remove the columns delete in schemaUpgrade method; * 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; * 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; * Optimize the schemaUpgrade method following @vitaly-t instructions, and more tests; * 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; * Update PostgresStorageAdapter.js rewriting method schemaUpgrade * Update PostgresStorageAdapter.spec.js * Update PostgresStorageAdapter.spec.js
parse-community · Jan 3, 2018 · cb8f038 · cb8f038
1 parent fc6a2fd
commit cb8f038
Show file tree

Hide file tree

Showing 2 changed files with 127 additions and 8 deletions.
diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js
@@ -1,6 +1,10 @@
 import PostgresStorageAdapter from '../src/Adapters/Storage/Postgres/PostgresStorageAdapter';
 const databaseURI = 'postgres://localhost:5432/parse_server_postgres_adapter_test_database';
 
+const getColumns = (client, className) => {
+  return client.map('SELECT column_name FROM information_schema.columns WHERE table_name = $<className>', { className }, a => a.column_name);
+};
+
 describe_only_db('postgres')('PostgresStorageAdapter', () => {
   beforeEach(done => {
     const adapter = new PostgresStorageAdapter({ uri: databaseURI })
@@ -19,4 +23,100 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
     expect(adapter._client.$pool.ending).toEqual(true);
     done();
   });
+
+  it('schemaUpgrade, upgrade the database schema when schema changes', done => {
+    const adapter = new PostgresStorageAdapter({ uri: databaseURI });
+    const client = adapter._client;
+    const className = '_PushStatus';
+    const schema = {
+      fields: {
+        "pushTime": { type: 'String' },
+        "source": { type: 'String' },
+        "query": { type: 'String' },
+      },
+    };
+
+    adapter.createTable(className, schema)
+      .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();
+      })
+      .catch(error => done.fail(error));
+  });
+
+  it('schemaUpgrade, maintain 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));
+  });
+
+  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
@@ -750,11 +750,27 @@ export class PostgresStorageAdapter implements StorageAdapter {
     });
   }
 
-  addFieldIfNotExists(className: string, fieldName: string, type: any) {
+  schemaUpgrade(className: string, schema: SchemaType, conn: any) {
+    debug('schemaUpgrade', { className, schema });
+    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>', { 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: string, fieldName: string, type: any, conn: any) {
     // 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 $<className:name> ADD COLUMN $<fieldName:name> $<postgresType:raw>', {
@@ -1591,14 +1607,17 @@ export class PostgresStorageAdapter implements StorageAdapter {
   }
 
   performInitialization({ VolatileClassesSchemas }: any) {
+    // 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).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(() => {