Skip to content

Fix issue #34 - exception if object field not in fields list #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 47 additions & 13 deletions lib/migration.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,37 @@ function mixinMigration(SQLite3) {
// it might. Will need to disable/check & re-enable foreign key checks
var oldName = escape(model);
var newName = escape(model + '_old');
var field_list;

async.series([
renameOldTable,
self.createTable.bind(self, model, txOptions),
copyRecords,
dropOldTable,
], done);
getExistingFieldNames(function(err, result) {

if (result) {

// Get the list of properties from the model definition (converted to lower case)
var properties = Object.keys(self.getModelDefinition(model).properties).map(function(name) {
return name.toLowerCase();
});

// Eliminate any fields that are not properties of the model from our list of fields to copy
var fields_to_copy = result.filter(function(fieldname) {
return properties.indexOf(fieldname.toLowerCase()) > -1;
});

field_list = fields_to_copy.join(",");

async.series([
enableLegacyTableRename,
renameOldTable,
self.createTable.bind(self, model, txOptions),
copyRecords,
dropOldTable,
], done);
}
})

function enableLegacyTableRename(cb) {
self.executeSQL("PRAGMA legacy_alter_table = ON", [], txOptions, cb)
}

function renameOldTable(cb) {
self.executeSQL(
Expand All @@ -98,14 +122,9 @@ function mixinMigration(SQLite3) {
}

function copyRecords(cb) {
var properties = self.getModelDefinition(model).properties;
properties = Object.keys(properties).map(function(name) {
return escape(name);
}).join(',');

self.executeSQL(
fmt('INSERT INTO %s SELECT %s FROM %s',
oldName, properties, newName
fmt('INSERT INTO %s (%s) SELECT %s FROM %s',
oldName, field_list, field_list, newName
), [], txOptions, cb
);
}
Expand All @@ -117,5 +136,20 @@ function mixinMigration(SQLite3) {
function escape(name) {
return self.escapeName(name.toLowerCase());
}

function getExistingFieldNames(cb) {
self.executeSQL('PRAGMA table_info(' + model.toLowerCase() + ')', [], txOptions, function(err, res) {
if (err) {
cb(err);
} else {
var fields = [];
res.forEach(function(field) {
fields.push (field.name);
});
cb(err, fields);
}
});
}

};
}
29 changes: 20 additions & 9 deletions lib/sqlite3.js
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,9 @@ SQLite3.prototype._columnDataType = function(model, propertyName) {
if (colType && colLength) {
return colType + '(' + colLength + ')';
}
else if (colType) {
return colType;
}
return this._buildColumnType(property);
};

Expand Down Expand Up @@ -481,29 +484,37 @@ SQLite3.prototype._buildColumnIndexes = function(model) {
if (settings && settings.indexes) {
for (var index in settings.indexes) {
if (!settings.indexes.hasOwnProperty(index) || !index) continue;
var indexProps = settings.indexes[index];

var columns;
if (settings.indexes[index].columns)
columns = settings.indexes[index].columns.split(',');
else if (settings.indexes[index].keys) {
if (settings.indexes[index].keys instanceof Array) {
columns = settings.indexes[index].keys;
if (indexProps.columns)
columns = indexProps.columns.split(',');
else if (indexProps.keys) {
if (indexProps.keys instanceof Array) {
columns = indexProps.keys;
} else {
columns = Object.keys(settings.indexes[index].keys);
columns = Object.keys(indexProps.keys);
}
} else {
debug('Unable to locate keys for index in settings: %j',
settings.indexes[index]
indexProps
);
}

var uniqueStr = '';
if (indexProps.unique ||
(indexProps.options && indexProps.options.unique)) {
uniqueStr = 'UNIQUE';
}

if (!columns) continue;
for (var i in columns) {
if (!columns.hasOwnProperty(i)) continue;
columns[i] = self.dbName(columns[i]).trim();
}

indexes.push(fmt('CREATE INDEX IF NOT EXISTS %s ON %s (%s)',
indexes.push(fmt('CREATE %s INDEX IF NOT EXISTS %s ON %s (%s)',
uniqueStr,
escape(index || model + '_' + columns.join('_')),
self.tableEscaped(model), columns.map(escape).join(',')
));
Expand Down Expand Up @@ -558,7 +569,7 @@ SQLite3.prototype.toColumnValue = function(property, value) {
};

SQLite3.prototype.fromColumnValue = function(property, value) {
if (value === null || !property) {
if (value == null || !property) {
return value;
}
switch (property.type.name) {
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "loopback-connector-sqlite3",
"version": "3.0.0-alpha.1",
"version": "3.0.0-alpha.2",
"description": "Loopback SQLite3 Connector",
"engines": {
"node": ">=4"
Expand All @@ -24,7 +24,7 @@
"debug": "^2.1.1",
"loopback-connector": "2.x",
"moment": "^2.10.3",
"sqlite3": "^3.1.0",
"sqlite3": "^4.2.0",
"strong-globalize": "^2.6.2"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion test/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
var DataSource = require('loopback-datasource-juggler').DataSource;
var path = require('path');
var os = require('os');

console.log('tmpdir',os.tmpdir())
var config = require('rc')('loopback', {
test: {
sqlite: {
Expand Down
79 changes: 79 additions & 0 deletions test/sqlite.handleUndefinedObject.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright IBM Corp. 2015. All Rights Reserved.
// Node module: loopback-connector-sqlite3
// This file is licensed under the Artistic License 2.0.
// License text available at https://opensource.org/licenses/Artistic-2.0

var should = require('should');
require('./init');

var Post, db;

/*global describe, before, it, getDataSource*/
describe('handle undefined object', function() {
before(function() {
db = getDataSource();

Post = db.define('HandleUndefinedObject', {
field1: {
type: 'number'
},
field2: {
type: 'object'
}
});
});

it('should run migration', function(done) {
db.automigrate('HandleUndefinedObject', function() {
done();
});
});

it('should handle object field in fields list', function(done) {
var tstPost = new Post();
tstPost.field1 = 1;
tstPost.field2 = {val: 2};
tstPost.save(function(err, p) {
should.not.exist(err);
Post.findOne({where: {id: p.id}}, function(err, p) {
should.not.exist(err);
p.field1.should.equal(1);
p.field2.should.deepEqual({val: 2});
done();
});
});
});

it('should handle object field not in fields list', function(done) {
var tstPost = new Post();
tstPost.field1 = 1;
tstPost.field2 = {val: 2};
tstPost.save(function(err, p) {
should.not.exist(err);
Post.findOne({where: {id: p.id}, fields: ['field1']}, function(err, p) {
should.not.exist(err);
p.field1.should.be.equal(1);
should.not.exist(p.field2);
done();
});
});
});

// it('should insert default value and \'third\' field', function(done) {
// var tstPost = new Post();
// tstPost.third = 3;
// tstPost.save(function(err, p) {
// should.not.exist(err);
// Post.findOne({where: {id: p.id}}, function(err, p) {
// should.not.exist(err);
// p.defaultInt.should.be.equal(5);
// should.not.exist(p.first);
// should.not.exist(p.second);
// should.exist(p.third);
// p.third.should.be.equal(3);
// });
// done();
// });
// });

});
Loading