diff --git a/packages/strapi-admin/controllers/Admin.js b/packages/strapi-admin/controllers/Admin.js index 197699aef1..0b6a84b879 100644 --- a/packages/strapi-admin/controllers/Admin.js +++ b/packages/strapi-admin/controllers/Admin.js @@ -108,25 +108,21 @@ module.exports = { */ async create(ctx) { - const values = ctx.request.body; + const { email, username, password, blocked } = ctx.request.body; - if (!values.email) return ctx.badRequest('Missing email'); - if (!values.username) return ctx.badRequest('Missing username'); - if (!values.password) return ctx.badRequest('Missing password'); + if (!email) return ctx.badRequest('missing.email'); + if (!username) return ctx.badRequest('missing.username'); + if (!password) return ctx.badRequest('missing.password'); const adminsWithSameEmail = await strapi .query('administrator', 'admin') - .find({ - email: values.email, - }); + .findOne({ email }); const adminsWithSameUsername = await strapi .query('administrator', 'admin') - .find({ - username: values.username, - }); + .findOne({ username }); - if (adminsWithSameEmail.length > 0) { + if (adminsWithSameEmail) { return ctx.badRequest( null, ctx.request.admin @@ -137,11 +133,11 @@ module.exports = { ], }, ] - : 'Email is already taken.' + : 'email.alreadyTaken' ); } - if (adminsWithSameUsername.length > 0) { + if (adminsWithSameUsername) { return ctx.badRequest( null, ctx.request.admin @@ -155,15 +151,15 @@ module.exports = { ], }, ] - : 'Username is already taken.' + : 'username.alreadyTaken.' ); } const user = { - email: values.email, - username: values.username, - blocked: values.blocked === true ? true : false, - password: await strapi.admin.services.auth.hashPassword(values.password), + email: email, + username: username, + blocked: blocked === true ? true : false, + password: await strapi.admin.services.auth.hashPassword(password), }; const data = await strapi.query('administrator', 'admin').create(user); @@ -180,11 +176,11 @@ module.exports = { async update(ctx) { const { id } = ctx.params; - const values = ctx.request.body; + const { email, username, password, blocked } = ctx.request.body; - if (!values.email) return ctx.badRequest('Missing email'); - if (!values.username) return ctx.badRequest('Missing username'); - if (!values.password) return ctx.badRequest('Missing password'); + if (!email) return ctx.badRequest('Missing email'); + if (!username) return ctx.badRequest('Missing username'); + if (!password) return ctx.badRequest('Missing password'); const admin = await strapi .query('administrator', 'admin') @@ -194,12 +190,10 @@ module.exports = { if (!admin) return ctx.notFound('Administrator not found'); // check there are not user with requested email - if (values.email !== admin.email) { + if (email !== admin.email) { const adminsWithSameEmail = await strapi .query('administrator', 'admin') - .findOne({ - email: values.email, - }); + .findOne({ email }); if (adminsWithSameEmail && adminsWithSameEmail.id !== admin.id) { return ctx.badRequest( @@ -218,12 +212,10 @@ module.exports = { } // check there are not user with requested username - if (values.username !== admin.username) { + if (username !== admin.username) { const adminsWithSameUsername = await strapi .query('administrator', 'admin') - .findOne({ - username: values.username, - }); + .findOne({ username }); if (adminsWithSameUsername && adminsWithSameUsername.id !== admin.id) { return ctx.badRequest( @@ -245,15 +237,13 @@ module.exports = { } const user = { - email: values.email, - username: values.username, - blocked: values.blocked === true ? true : false, + email: email, + username: username, + blocked: blocked === true ? true : false, }; - if (values.password !== admin.password) { - user.password = await strapi.admin.services.auth.hashPassword( - values.password - ); + if (password !== admin.password) { + user.password = await strapi.admin.services.auth.hashPassword(password); } const data = await strapi diff --git a/packages/strapi-hook-bookshelf/lib/buildDatabaseSchema.js b/packages/strapi-hook-bookshelf/lib/buildDatabaseSchema.js index bf24213b23..bf99f95ff5 100644 --- a/packages/strapi-hook-bookshelf/lib/buildDatabaseSchema.js +++ b/packages/strapi-hook-bookshelf/lib/buildDatabaseSchema.js @@ -16,36 +16,10 @@ module.exports = async ({ [createAtCol, updatedAtCol] = hasTimestamps; } - const quote = definition.client === 'pg' ? '"' : '`'; - // Equilize database tables const createOrUpdateTable = async (table, attributes) => { const tableExists = await ORM.knex.schema.hasTable(table); - // Apply field type of attributes definition - const generateColumns = (attrs, start, tableExists = false) => { - return Object.keys(attrs).reduce((acc, attr) => { - const attribute = attributes[attr]; - - const type = getType({ - definition, - attribute, - name: attr, - tableExists, - }); - - if (type) { - acc.push( - `${quote}${attr}${quote} ${type} ${ - attribute.required ? 'NOT NULL' : '' - }` - ); - } - - return acc; - }, start); - }; - const generateIndexes = async table => { try { const connection = strapi.config.connections[definition.connection]; @@ -104,40 +78,60 @@ module.exports = async ({ } }; - const createTable = async table => { - const defaultAttributeDefinitions = { - mysql: ['id INT AUTO_INCREMENT NOT NULL PRIMARY KEY'], - pg: ['id SERIAL NOT NULL PRIMARY KEY'], - sqlite3: ['id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL'], - }; + const buildColumns = (tbl, columns, opts = {}) => { + const { tableExists, alter = false } = opts; - let idAttributeBuilder = defaultAttributeDefinitions[definition.client]; - - if (definition.primaryKeyType === 'uuid' && definition.client === 'pg') { - idAttributeBuilder = [ - 'id uuid NOT NULL DEFAULT uuid_generate_v4() NOT NULL PRIMARY KEY', - ]; - } else if (definition.primaryKeyType !== 'integer') { + Object.keys(columns).forEach(key => { + const attribute = columns[key]; const type = getType({ definition, - attribute: { - type: definition.primaryKeyType, - }, + attribute, + name: key, + tableExists, }); - idAttributeBuilder = [`id ${type} NOT NULL PRIMARY KEY`]; - } - const columns = generateColumns(attributes, idAttributeBuilder).join( - ',\n\r' - ); + if (type) { + const col = tbl.specificType(key, type); - // Create table - await ORM.knex.raw(`CREATE TABLE ${quote}${table}${quote} (${columns})`); + if (attribute.required === true) { + if (definition.client !== 'sqlite3' || !tableExists) { + col.notNullable(); + } + } else { + col.nullable(); + } + + if (attribute.unique === true) { + if (definition.client !== 'sqlite3' || !tableExists) { + tbl.unique(key, uniqueColName(table, key)); + } + } + + if (alter) { + col.alter(); + } + } + }); + }; + + const createColumns = (tbl, columns, opts = {}) => { + return buildColumns(tbl, columns, opts); + }; + + const alterColumns = (tbl, columns, opts = {}) => { + return createColumns(tbl, columns, { ...opts, alter: true }); + }; + + const createTable = (table, { trx = ORM.knex, ...opts } = {}) => { + return trx.schema.createTable(table, tbl => { + tbl.specificType('id', getIdType(definition)); + createColumns(tbl, attributes, { ...opts, tableExists: false }); + }); }; if (!tableExists) { await createTable(table); - await generateIndexes(table, attributes); + await generateIndexes(table); await storeTable(table, attributes); return; } @@ -160,31 +154,16 @@ module.exports = async ({ } }); - // Generate indexes for new attributes. - await generateIndexes(table, columnsToAdd); - // Generate and execute query to add missing column if (Object.keys(columnsToAdd).length > 0) { await ORM.knex.schema.table(table, tbl => { - Object.keys(columnsToAdd).forEach(key => { - const attribute = columnsToAdd[key]; - const type = getType({ - definition, - attribute, - name: key, - tableExists, - }); - - if (type) { - const col = tbl.specificType(key, type); - if (attribute.required && definition.client !== 'sqlite3') { - col.notNullable(); - } - } - }); + createColumns(tbl, columnsToAdd, { tableExists }); }); } + // Generate indexes for new attributes. + await generateIndexes(table, columnsToAdd); + let previousAttributes; try { previousAttributes = JSON.parse( @@ -201,14 +180,26 @@ module.exports = async ({ ); } - if (JSON.stringify(previousAttributes) === JSON.stringify(attributes)) + if (JSON.stringify(previousAttributes) === JSON.stringify(attributes)) { return; + } if (definition.client === 'sqlite3') { const tmpTable = `tmp_${table}`; - await createTable(tmpTable); - try { + const rebuildTable = async trx => { + await trx.schema.renameTable(table, tmpTable); + + // drop possible conflicting indexes + await Promise.all( + columns.map(key => + trx.raw('DROP INDEX IF EXISTS ??', uniqueColName(table, key)) + ) + ); + + // create the table + await createTable(table, { trx }); + const attrs = Object.keys(attributes).filter(attribute => getType({ definition, @@ -217,51 +208,50 @@ module.exports = async ({ }) ); - await ORM.knex.raw(`INSERT INTO ?? (${attrs.join(', ')}) ??`, [ - tmpTable, - ORM.knex.select(attrs).from(table), + await trx.raw(`INSERT INTO ?? (${attrs.join(', ')}) ??`, [ + table, + trx.select(attrs).from(tmpTable), ]); + + await trx.schema.dropTableIfExists(tmpTable); + }; + + try { + await ORM.knex.transaction(trx => rebuildTable(trx)); + await generateIndexes(table); } catch (err) { strapi.log.error('Migration failed'); strapi.log.error(err); - - await ORM.knex.schema.dropTableIfExists(tmpTable); return false; } - - await ORM.knex.schema.dropTableIfExists(table); - await ORM.knex.schema.renameTable(tmpTable, table); - - await generateIndexes(table, attributes); } else { - await ORM.knex.schema.alterTable(table, tbl => { - columns.forEach(key => { - if ( - JSON.stringify(previousAttributes[key]) === - JSON.stringify(attributes[key]) - ) - return; + const columnsToAlter = columns.filter( + key => + JSON.stringify(previousAttributes[key]) !== + JSON.stringify(attributes[key]) + ); - const attribute = attributes[key]; - const type = getType({ - definition, - attribute, - name: key, + const alterTable = async trx => { + await Promise.all( + columnsToAlter.map(col => { + return ORM.knex.schema + .alterTable(table, tbl => { + tbl.dropUnique(col, uniqueColName(table, col)); + }) + .catch(() => {}); + }) + ); + await trx.schema.alterTable(table, tbl => { + alterColumns(tbl, _.pick(attributes, columnsToAlter), { tableExists, }); - - if (type) { - let col = tbl.specificType(key, type); - if (attribute.required) { - col = col.notNullable(); - } - col.alter(); - } }); - }); + }; - await storeTable(table, attributes); + await ORM.knex.transaction(trx => alterTable(trx)); } + + await storeTable(table, attributes); }; // Add created_at and updated_at field if timestamp option is true @@ -437,3 +427,30 @@ const storeTable = async (table, attributes) => { value: JSON.stringify(attributes), }).save(); }; + +const defaultIdType = { + mysql: 'INT AUTO_INCREMENT NOT NULL PRIMARY KEY', + pg: 'SERIAL NOT NULL PRIMARY KEY', + sqlite3: 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL', +}; + +const getIdType = definition => { + if (definition.primaryKeyType === 'uuid' && definition.client === 'pg') { + return 'uuid NOT NULL DEFAULT uuid_generate_v4() NOT NULL PRIMARY KEY'; + } + + if (definition.primaryKeyType !== 'integer') { + const type = getType({ + definition, + attribute: { + type: definition.primaryKeyType, + }, + }); + + return `${type} NOT NULL PRIMARY KEY`; + } + + return defaultIdType[definition.client]; +}; + +const uniqueColName = (table, key) => `${table}_${key}_unique`; diff --git a/packages/strapi-hook-bookshelf/lib/generate-group-relations.js b/packages/strapi-hook-bookshelf/lib/generate-group-relations.js index 78c0f6f28c..aad3d1ac8e 100644 --- a/packages/strapi-hook-bookshelf/lib/generate-group-relations.js +++ b/packages/strapi-hook-bookshelf/lib/generate-group-relations.js @@ -37,7 +37,9 @@ module.exports = async ({ model, definition, ORM, GLOBALS }) => { }; }); - await ORM.knex.schema.createTableIfNotExists(joinTable, table => { + if (await ORM.knex.schema.hasTable(joinTable)) return; + + await ORM.knex.schema.createTable(joinTable, table => { table.increments(); table.string('field').notNullable(); table @@ -45,14 +47,8 @@ module.exports = async ({ model, definition, ORM, GLOBALS }) => { .unsigned() .notNullable(); table.string('slice_type').notNullable(); - table - .integer('slice_id') - .unsigned() - .notNullable(); - table - .integer(joinColumn) - .unsigned() - .notNullable(); + table.integer('slice_id').notNullable(); + table.integer(joinColumn).notNullable(); table .foreign(joinColumn) diff --git a/packages/strapi-hook-bookshelf/lib/mount-models.js b/packages/strapi-hook-bookshelf/lib/mount-models.js index ae75959cd9..d34e58e77a 100644 --- a/packages/strapi-hook-bookshelf/lib/mount-models.js +++ b/packages/strapi-hook-bookshelf/lib/mount-models.js @@ -114,8 +114,6 @@ module.exports = ({ models, target, plugin = false }, ctx) => { global[definition.globalName] = {}; } - await genGroupRelatons({ model: loadedModel, definition, ORM, GLOBALS }); - // Add every relationships to the loaded model for Bookshelf. // Basic attributes don't need this-- only relations. Object.keys(definition.attributes).forEach(name => { @@ -709,13 +707,15 @@ module.exports = ({ models, target, plugin = false }, ctx) => { target[model]._attributes = definition.attributes; target[model].updateRelations = relations.update; - return buildDatabaseSchema({ + await buildDatabaseSchema({ ORM, definition, loadedModel, connection, model: target[model], }); + + await genGroupRelatons({ model: loadedModel, definition, ORM, GLOBALS }); } catch (err) { strapi.log.error(`Impossible to register the '${model}' model.`); strapi.log.error(err); diff --git a/packages/strapi-plugin-users-permissions/controllers/User.js b/packages/strapi-plugin-users-permissions/controllers/User.js index 2d2b56d0c9..d4d9625395 100644 --- a/packages/strapi-plugin-users-permissions/controllers/User.js +++ b/packages/strapi-plugin-users-permissions/controllers/User.js @@ -8,12 +8,10 @@ const _ = require('lodash'); -const sanitizeUser = user => { - return _.omit(user.toJSON ? user.toJSON() : user, [ - 'password', - 'resetPasswordToken', - ]); -}; +const sanitizeUser = user => _.omit(user, ['password', 'resetPasswordToken']); +const adminError = error => [ + { messages: [{ id: error.message, field: error.field }] }, +]; module.exports = { /** @@ -87,49 +85,72 @@ module.exports = { }) .get(); - if (advanced.unique_email && ctx.request.body.email) { + const { email, username, password, role } = ctx.request.body; + + if (!email) return ctx.badRequest('missing.email'); + if (!username) return ctx.badRequest('missing.username'); + if (!password) return ctx.badRequest('missing.password'); + + const adminsWithSameUsername = await strapi + .query('user', 'users-permissions') + .findOne({ username }); + + if (adminsWithSameUsername) { + return ctx.badRequest( + null, + ctx.request.admin + ? adminError({ + message: 'Auth.form.error.username.taken', + field: ['username'], + }) + : 'username.alreadyTaken.' + ); + } + + if (advanced.unique_email) { const user = await strapi .query('user', 'users-permissions') - .findOne({ email: ctx.request.body.email }); + .findOne({ email }); if (user) { return ctx.badRequest( null, ctx.request.admin - ? [ - { - messages: [ - { id: 'Auth.form.error.email.taken', field: ['email'] }, - ], - }, - ] - : 'Email is already taken.' + ? adminError({ + message: 'Auth.form.error.email.taken', + field: ['email'], + }) + : 'email.alreadyTaken' ); } } - if (!ctx.request.body.role) { + const user = { + email, + username, + password, + role, + provider: 'local', + }; + + if (!role) { const defaultRole = await strapi .query('role', 'users-permissions') .findOne({ type: advanced.default_role }, []); - ctx.request.body.role = defaultRole._id || defaultRole.id; + user.role = defaultRole.id; } - ctx.request.body.provider = 'local'; - try { const data = await strapi.plugins['users-permissions'].services.user.add( - ctx.request.body + user ); ctx.created(data); } catch (error) { ctx.badRequest( null, - ctx.request.admin - ? [{ messages: [{ id: error.message, field: error.field }] }] - : error.message + ctx.request.admin ? adminError(error) : error.message ); } }, @@ -166,13 +187,10 @@ module.exports = { return ctx.badRequest( null, ctx.request.admin - ? [ - { - messages: [ - { id: 'Auth.form.error.email.taken', field: ['email'] }, - ], - }, - ] + ? adminError({ + message: 'Auth.form.error.email.taken', + field: ['email'], + }) : 'Email is already taken.' ); } @@ -206,13 +224,10 @@ module.exports = { return ctx.badRequest( null, ctx.request.admin - ? [ - { - messages: [ - { id: 'Auth.form.error.email.taken', field: ['email'] }, - ], - }, - ] + ? adminError({ + message: 'Auth.form.error.email.taken', + field: ['email'], + }) : 'Email is already taken.' ); } diff --git a/packages/strapi-plugin-users-permissions/services/User.js b/packages/strapi-plugin-users-permissions/services/User.js index af5aca80db..3b875406a6 100644 --- a/packages/strapi-plugin-users-permissions/services/User.js +++ b/packages/strapi-plugin-users-permissions/services/User.js @@ -22,19 +22,6 @@ module.exports = { ].services.user.hashPassword(values); } - // Use Content Manager business logic to handle relation. - if (strapi.plugins['content-manager']) { - return await strapi.plugins['content-manager'].services[ - 'contentmanager' - ].add( - { - model: 'user', - }, - values, - 'users-permissions' - ); - } - return strapi.query('user', 'users-permissions').create(values); },