Skip to content

Commit 2096d40

Browse files
elv1selv1s
committed
bug fix for multiple fk
1 parent 4afc562 commit 2096d40

File tree

2 files changed

+148
-39
lines changed

2 files changed

+148
-39
lines changed

‎lib/migration.js

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,6 @@ function mixinMigration(PostgreSQL) {
108108
return;
109109
}
110110

111-
// actualFks is a list of EXISTING fkeys here,
112-
// so you don't need to recreate them again
113-
// prepare fkSQL for new foreign keys
114-
var fkSQL = self.getForeignKeySQL(model,
115-
self.getModelDefinition(model).settings.foreignKeys,
116-
actualFks);
117-
118111
async.series([
119112
function(cb) {
120113
applyPending([
@@ -127,6 +120,13 @@ function mixinMigration(PostgreSQL) {
127120
self.addIndexes(model, actualIndexes, cb);
128121
},
129122
function(cb) {
123+
// actualFks is a list of EXISTING fkeys here,
124+
// so you don't need to recreate them again
125+
// prepare fkSQL for new foreign keys
126+
var fkSQL = self.getForeignKeySQL(model,
127+
self.getModelDefinition(model).settings.foreignKeys,
128+
actualFks);
129+
130130
self.addForeignKeys(model, fkSQL, cb);
131131
},
132132
], function(err, result) {
@@ -330,7 +330,9 @@ function mixinMigration(PostgreSQL) {
330330
if (err) {
331331
return cb(err);
332332
}
333-
self.addForeignKeys(model, function(err, result) {
333+
var fkSQL = self.getForeignKeySQL(model,
334+
self.getModelDefinition(model).settings.foreignKeys);
335+
self.addForeignKeys(model, fkSQL, function(err, result) {
334336
cb(err);
335337
});
336338
});
@@ -498,18 +500,6 @@ function mixinMigration(PostgreSQL) {
498500

499501
PostgreSQL.prototype.addForeignKeys = function(model, fkSQL, cb) {
500502
var self = this;
501-
var m = this.getModelDefinition(model);
502-
503-
if ((!cb) && ('function' === typeof fkSQL)) {
504-
cb = fkSQL;
505-
fkSQL = undefined;
506-
}
507-
508-
if (!fkSQL || fkSQL.length === 0) {
509-
var newFks = m.settings.foreignKeys;
510-
if (newFks)
511-
fkSQL = self.getForeignKeySQL(model, newFks);
512-
}
513503

514504
if (fkSQL && fkSQL.length) {
515505
self.applySqlChanges(model, [fkSQL.toString()], function(err, result) {
@@ -539,17 +529,24 @@ function mixinMigration(PostgreSQL) {
539529
var fkRefKey = newFk.entityKey;
540530
var fkEntityName = (typeof newFk.entity === 'object') ? newFk.entity.name : newFk.entity;
541531
var fkRefTable = self.table(fkEntityName);
542-
needsToDrop = fkCol != fk.fkColumnName ||
543-
fkRefKey != fk.pkColumnName ||
544-
fkRefTable != fk.pkTableName;
532+
needsToDrop = !isCaseInsensitiveEqual(fkCol, fk.fkColumnName) ||
533+
!isCaseInsensitiveEqual(fkRefKey, fk.pkColumnName) ||
534+
!isCaseInsensitiveEqual(fkRefTable, fk.pkTableName);
545535
} else {
546-
needsToDrop = true;
536+
// only if FK is in model properties then need to drop
537+
if (hasColumnProperty(m.properties, fk.fkColumnName)) {
538+
needsToDrop = true;
539+
}
547540
}
548541

549542
if (needsToDrop) {
550543
sql.push('DROP CONSTRAINT ' + self.escapeName(fk.fkName));
551544
removedFks.push(fk); // keep track that we removed these
552545
}
546+
547+
if (sql.length > 0) {
548+
sql = [sql.join(', ')];
549+
}
553550
});
554551

555552
// update out list of existing keys by removing dropped keys
@@ -602,6 +599,29 @@ function mixinMigration(PostgreSQL) {
602599
return '';
603600
};
604601

602+
/*!
603+
* Case insensitive comparison of two strings
604+
* @param {String} val1
605+
* @param {String} val2
606+
*/
607+
function isCaseInsensitiveEqual(val1, val2) {
608+
return val1.toLowerCase() === val2.toLowerCase();
609+
}
610+
/*!
611+
* Case insensitive comparison of object properties
612+
* @param {Object} properties
613+
* @param {String} name
614+
*/
615+
function hasColumnProperty(properties, name) {
616+
if (!name) { return false; }
617+
618+
return (Object.keys(properties)
619+
.map(function(k) {
620+
return k.toLowerCase();
621+
})
622+
.indexOf(name.toLowerCase()) > -1);
623+
}
624+
605625
/*!
606626
* Map postgresql data types to json types
607627
* @param {String} postgresqlType

‎test/postgresql.autoupdate.test.js

Lines changed: 104 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,29 @@ describe('autoupdate', function() {
364364

365365
describe('foreign key constraint', function() {
366366
it('should create, update, and delete foreign keys', function(done) {
367+
var product_schema = {
368+
'name': 'Product',
369+
'options': {
370+
'idInjection': false,
371+
'postgresql': {
372+
'schema': 'myapp_test',
373+
'table': 'product_test',
374+
},
375+
},
376+
'properties': {
377+
'id': {
378+
'type': 'String',
379+
'length': 20,
380+
'id': 1,
381+
},
382+
'name': {
383+
'type': 'String',
384+
'required': false,
385+
'length': 40,
386+
},
387+
},
388+
};
389+
367390
var customer2_schema = {
368391
'name': 'CustomerTest2',
369392
'options': {
@@ -495,6 +518,10 @@ describe('autoupdate', function() {
495518
'required': false,
496519
'length': 40,
497520
},
521+
'productId': {
522+
'type': 'String',
523+
'length': 20,
524+
},
498525
},
499526
};
500527

@@ -506,6 +533,20 @@ describe('autoupdate', function() {
506533
'schema': 'myapp_test',
507534
'table': 'order_test',
508535
},
536+
'foreignKeys': {
537+
'fk_ordertest_customerId': {
538+
'name': 'fk_ordertest_customerId',
539+
'entity': 'CustomerTest2',
540+
'entityKey': 'id',
541+
'foreignKey': 'customerId',
542+
},
543+
'fk_ordertest_productId': {
544+
'name': 'fk_ordertest_productId',
545+
'entity': 'Product',
546+
'entityKey': 'id',
547+
'foreignKey': 'productId',
548+
},
549+
},
509550
},
510551
'properties': {
511552
'id': {
@@ -522,9 +563,45 @@ describe('autoupdate', function() {
522563
'required': false,
523564
'length': 40,
524565
},
566+
'productId': {
567+
'type': 'String',
568+
'length': 20,
569+
},
570+
},
571+
};
572+
573+
var orderTest_schema_v4 = {
574+
'name': 'OrderTest',
575+
'options': {
576+
'idInjection': false,
577+
'postgresql': {
578+
'schema': 'myapp_test',
579+
'table': 'order_test',
580+
},
581+
},
582+
'properties': {
583+
'id': {
584+
'type': 'String',
585+
'length': 20,
586+
'id': 1,
587+
},
588+
'customerId': {
589+
'type': 'String',
590+
'length': 20,
591+
},
592+
'productId': {
593+
'type': 'String',
594+
'length': 20,
595+
},
596+
'description': {
597+
'type': 'String',
598+
'required': false,
599+
'length': 40,
600+
},
525601
},
526602
};
527603

604+
ds.createModel(product_schema.name, product_schema.properties, product_schema.options);
528605
ds.createModel(customer2_schema.name, customer2_schema.properties, customer2_schema.options);
529606
ds.createModel(customer3_schema.name, customer3_schema.properties, customer3_schema.options);
530607

@@ -553,18 +630,18 @@ describe('autoupdate', function() {
553630
assert.equal(foreignKeys[0].fkColumnName, 'customerid');
554631
assert.equal(foreignKeys[0].fkName, 'fk_ordertest_customerId');
555632

556-
// update fk
633+
// update and add another fk
557634
ds.createModel(orderTest_schema_v2.name, orderTest_schema_v2.properties, orderTest_schema_v2.options);
558635
ds.autoupdate(function(err) {
559636
assert(!err, err);
560637
ds.discoverModelProperties('order_test', function(err, props) {
561638
// validate that we have the correct number of properties
562-
assert.equal(props.length, 3);
639+
assert.equal(props.length, 4);
563640

564641
// get the foreign keys for order_test
565642
ds.connector.discoverForeignKeys('order_test', {}, function(err, foreignKeysUpdated) {
566643
assert(!err, err);
567-
// validate that the foreign key exists and points to the new column
644+
// validate that the foreign keys exist and point to the new column
568645
assert(foreignKeysUpdated);
569646
assert.equal(foreignKeysUpdated.length, 1);
570647
assert.equal(foreignKeysUpdated[0].pkColumnName, 'id');
@@ -573,21 +650,33 @@ describe('autoupdate', function() {
573650
assert.equal(foreignKeysUpdated[0].fkColumnName, 'customerid');
574651
assert.equal(foreignKeysUpdated[0].fkName, 'fk_ordertest_customerId');
575652

576-
// remove fk
653+
// create multiple fks on object
577654
ds.createModel(orderTest_schema_v3.name, orderTest_schema_v3.properties,
578655
orderTest_schema_v3.options);
579656
ds.autoupdate(function(err) {
580-
assert(!err, err);
581-
ds.discoverModelProperties('order_test', function(err, props) {
582-
// validate that we have the correct number of properties
583-
assert.equal(props.length, 3);
584-
585-
// get the foreign keys for order_test
586-
ds.connector.discoverForeignKeys('order_test', {}, function(err, foreignKeysEmpty) {
587-
if (err) return done(err);
588-
assert(foreignKeysEmpty);
589-
assert.equal(foreignKeysEmpty.length, 0);
590-
done();
657+
// get the foreign keys for order_test
658+
ds.connector.discoverForeignKeys('order_test', {}, function(err, foreignKeysMulti) {
659+
assert(!err, err);
660+
assert(foreignKeysMulti);
661+
assert.equal(foreignKeysMulti.length, 2);
662+
663+
// remove fk
664+
ds.createModel(orderTest_schema_v4.name, orderTest_schema_v4.properties,
665+
orderTest_schema_v4.options);
666+
ds.autoupdate(function(err) {
667+
assert(!err, err);
668+
ds.discoverModelProperties('order_test', function(err, props) {
669+
// validate that we have the correct number of properties
670+
assert.equal(props.length, 4);
671+
672+
// get the foreign keys for order_test
673+
ds.connector.discoverForeignKeys('order_test', {}, function(err, foreignKeysEmpty) {
674+
if (err) return done(err);
675+
assert(foreignKeysEmpty);
676+
assert.equal(foreignKeysEmpty.length, 0);
677+
done();
678+
});
679+
});
591680
});
592681
});
593682
});

0 commit comments

Comments
 (0)