Updated according to review

Author: Giovanni Garufi

AbstractSQLCompiler.ts

@@ -59,11 +59,11 @@ export interface SqlModel {
 	dropSchema: string[]
 }
 
-export interface hasDependants {
+export interface HasDependants {
 	[dependant: string]: true
 }
 
-export interface schemaDependencyMap {
+export interface SchemaDependencyMap {
 	[tableName: string]: {
 		resourceName: string
 		primitive: AbstractSqlTable['primitive']
@@ -82,7 +82,7 @@ export interface ModifiedFields {
 	fields?: {}[]
 }
 
-type ReduceFn<T> = (accumulator: any, element: T) => any
+type ReduceFn<T> = (element: T) => string | undefined
 type MatchFn<T> = (haystack:Array<T>, needle:T) => (T | undefined)
 interface Pair<T> { src: T, dst: T }
 interface Split<T> { ins: Array<T>, del: Array<T>, mod: Array<Pair<T>> }
@@ -247,8 +247,8 @@ const mkSchemaDependencyMap = (tables: ResourceMap<AbstractSqlTable>, engine: En
 	} else {
 		ifNotExistsStr = ''
 	}
-	const hasDependants: hasDependants = {}
-	const schemaDependencyMap: schemaDependencyMap = {}
+	const hasDependants: HasDependants = {}
+	const schemaDependencyMap: SchemaDependencyMap = {}
 
 	_.forOwn(tables, (table, resourceName) => {
 		if (_.isString(table)) {
@@ -289,7 +289,7 @@ const mkSchemaDependencyMap = (tables: ResourceMap<AbstractSqlTable>, engine: En
 }
 
 const compileSchema = (abstractSqlModel: AbstractSqlModel, engine: Engines, ifNotExists: boolean): SqlModel => {
-	let { hasDependants, schemaDependencyMap } = mkSchemaDependencyMap(abstractSqlModel.tables, engine, ifNotExists) 
+	const { hasDependants, schemaDependencyMap } = mkSchemaDependencyMap(abstractSqlModel.tables, engine, ifNotExists) 
 
 	const createSchemaStatements = []
 	let dropSchemaStatements = []
@@ -394,10 +394,13 @@ const genSplit = <T>(src:Array<T>, dst: Array<T>, matchFn: MatchFn<T>): Split<T>
 //  The result of the match function will be either the element that should be matched to 
 //  the argument or undefined if no such element is found.
 const genDiff = <T>(insFn: ReduceFn<T>, delFn: ReduceFn<T>, modFn: ReduceFn<Pair<T>>, matchFn: MatchFn<T>, src: Array<T>, dst: Array<T>) => {
-	let split = genSplit(src, dst, matchFn)
-	return  _.flatten(_.reduce(split.ins, insFn, 
-						_.reduce(split.del, delFn,
-							_.reduce(split.mod, modFn, []))))
+	const split = genSplit(src, dst, matchFn)
+
+	const diff = _.map(split.mod, modFn)
+	.concat(_.map(split.del, delFn))
+	.concat(_.map(split.ins, insFn))
+	
+	return _.reject(diff, _.isUndefined)
 }
 
 const diffFields = (src: AbstractSqlField[], dst: AbstractSqlField[], mappings: ResourceMap<string>, engine: Engines, ifNotExists: boolean) => {
@@ -411,12 +414,8 @@ const diffFields = (src: AbstractSqlField[], dst: AbstractSqlField[], mappings:
 		ifNotExistsStr = ''
 		ifExistsStr = ''
 	}
-	//  defaultTypes =
-	//  	'Short Text': '\'\''
-	//  	'Text': '\'\''
-	//  	'Integer': 0
 
-	let matchFn: MatchFn<AbstractSqlField> = (fieldArray, field) => {
+	const matchFn: MatchFn<AbstractSqlField> = (fieldArray, field) => {
 		let match = _.find(fieldArray, { fieldName: field.fieldName })
 		if (_.isUndefined(match)) {
 			if (_.isString(mappings[field.fieldName])) {
@@ -427,26 +426,24 @@ const diffFields = (src: AbstractSqlField[], dst: AbstractSqlField[], mappings:
 		}
 	}
 
-	const insFn: ReduceFn<AbstractSqlField> = (acc, field) => {
+	const insFn: ReduceFn<AbstractSqlField> = (field) => {
 		// Columns are inserted as NULL for the time being, this is because we have no sensible way to automatically assign a default value
-		acc.push('ADD COLUMN ' + ifNotExistsStr + '"' + field.fieldName + '" ' + dataTypeGen(engine, field) + ';\n\t')
-		return acc
+		return 'ADD COLUMN ' + ifNotExistsStr + '"' + field.fieldName + '" ' + dataTypeGen(engine, field) + ';\n\t'
+		
 	}
 
-	const delFn: ReduceFn<AbstractSqlField> = (acc, field) => {
-		acc.push('DROP COLUMN ' + ifExistsStr + '"' + field.fieldName + '";\n\t')
-		return acc
+	const delFn: ReduceFn<AbstractSqlField> = (field) => {
+		return 'DROP COLUMN ' + ifExistsStr + '"' + field.fieldName + '";\n\t'
 	}
 
-	const modFn: ReduceFn<Pair<AbstractSqlField>> = (acc, {src, dst}) => {
+	const modFn: ReduceFn<Pair<AbstractSqlField>> = ({src, dst}) => {
 		if (_.isEqual(src, dst)) {
-			return acc
+			return
 		}
 		if (_.isEqual(_.omit(src, ['fieldName', 'references']), _.omit(dst, ['fieldName', 'references']))) {
-			acc.push('RENAME COLUMN "' + src.fieldName + '" TO "' + dst.fieldName + '";\n\t')
-			return acc
+			return 'RENAME COLUMN "' + src.fieldName + '" TO "' + dst.fieldName + '";\n\t'
 		}
-		throw Error('Can not migrate pre-existing field ' + src.fieldName + ' to ' + dst.fieldName)
+		throw Error(`Can not migrate pre-existing field ${src.fieldName} to ${dst.fieldName}`)
 	}
 
 	return genDiff(insFn, delFn, modFn, matchFn, src, dst)
@@ -471,27 +468,24 @@ const diffSchemas = (src: AbstractSqlModel, dst: AbstractSqlModel, engine: Engin
 		}
 	}
 
-	const insFn: ReduceFn<AbstractSqlTable> = (acc, table ) => {
+	const insFn: ReduceFn<AbstractSqlTable> = (table) => {
 		if (!_.isString(table) && !table.primitive) {
-			acc.push(dstSDM[table.name].createSQL)
+			return dstSDM[table.name].createSQL
 		}
-		return acc
 	}
 
-	const delFn: ReduceFn<AbstractSqlTable> = (acc, table) => {
+	const delFn: ReduceFn<AbstractSqlTable> = (table) => {
 		if (!_.isString(table) && !table.primitive) {
-			acc.push(srcSDM[table.name].dropSQL)
+			return srcSDM[table.name].dropSQL
 		}
-		return acc
 	}
 
-	const modFn: ReduceFn<Pair<AbstractSqlTable>> = (acc, { src: srcTbl, dst: dstTbl }) => {
+	const modFn: ReduceFn<Pair<AbstractSqlTable>> = ({ src: srcTbl, dst: dstTbl }) => {
 		if (_.isEqual(srcTbl, dstTbl)) {
-			return acc
+			return
 		} else if (_.isEqual(_.omit(srcTbl, 'fields'), _.omit(dstTbl, 'fields'))) {
 			const fields = diffFields(srcTbl.fields, dstTbl.fields, _.invert(src.synonyms), engine, ifNotExists)
-			acc.push('ALTER TABLE "' + srcTbl.name + '"\n\t' + fields.join(''))
-			return acc	
+			return 'ALTER TABLE "' + srcTbl.name + '"\n\t' + fields.join('')
 		} else {
 			const [srcResource, srcRest] = extractMappings(srcTbl.name)
 			const [dstResource, dstRest] = extractMappings(dstTbl.name)
@@ -501,8 +495,7 @@ const diffSchemas = (src: AbstractSqlModel, dst: AbstractSqlModel, engine: Engin
 				[srcRest]: dstResource
 			}
 			const fields = diffFields(srcTbl.fields, dstTbl.fields, mappings, engine, ifNotExists)
-			acc.push('ALTER TABLE "' + srcTbl.name + '"\n\t' + 'RENAME TO "' + dstTbl.name + '";\n\t' + fields.join(''))
-			return acc
+			return 'ALTER TABLE "' + srcTbl.name + '"\n\t' + 'RENAME TO "' + dstTbl.name + '";\n\t' + fields.join('')
 		}
 	}
 
@@ -527,4 +520,4 @@ const generateExport = (engine: Engines, ifNotExists: boolean) => {
 }
 export const postgres = generateExport(Engines.postgres, true)
 export const mysql = generateExport(Engines.mysql, true)
-export const websql = generateExport(Engines.websql, false)
+export const websql = generateExport(Engines.websql, false)

test/sbvr/test.coffee

@@ -30,21 +30,17 @@ module.exports = exports = (builtInVocab = false) ->
 
 	runMigration = (describe, src, dst, expectation) ->
 		it 'Migration: \n\nVocabulary: src\n' + src + '\n\nVocabulary: dst\n' + dst, ->
-			try
-				SBVRParser.reset()
-				srcLf = SBVRParser.matchAll(src, 'Process')
-				srcSchema = LF2AbstractSQLTranslator(srcLf, 'Process')
+			SBVRParser.reset()
+			srcLf = SBVRParser.matchAll(src, 'Process')
+			srcSchema = LF2AbstractSQLTranslator(srcLf, 'Process')
 
-				SBVRParser.reset()
-				dstLf = SBVRParser.matchAll(dst, 'Process')
-				dstSchema = LF2AbstractSQLTranslator(dstLf, 'Process')
+			SBVRParser.reset()
+			dstLf = SBVRParser.matchAll(dst, 'Process')
+			dstSchema = LF2AbstractSQLTranslator(dstLf, 'Process')
 
-				migration = AbstractSQLCompiler.postgres.diffSchemas(srcSchema, dstSchema)
-				console.log('GOT MIGRATION', migration)
-			catch e
-				throw e
-			for i in [0...Math.max(migration.length, expectation.length)]
-				expect(migration[i]).to.equal(expectation[i])
+			migration = AbstractSQLCompiler.postgres.diffSchemas(srcSchema, dstSchema)
+			
+			expect(migration).to.deep.equal(expectation)
 
 	runSchema = (describe, input, expectation) ->
 		runExpectation describe, input, (result) ->