Merge pull request #15217 from strapi/fix/duplicate-join-tables

packages/core/database/lib/index.js

@@ -11,6 +11,7 @@ const errors = require('./errors');
 
 // TODO: move back into strapi
 const { transformContentTypes } = require('./utils/content-types');
+const { validateDatabase } = require('./validations');
 
 class Database {
   constructor(config) {
@@ -76,7 +77,11 @@ class Database {
 
 // TODO: move into strapi
 Database.transformContentTypes = transformContentTypes;
-Database.init = async (config) => new Database(config);
+Database.init = async (config) => {
+  const db = new Database(config);
+  await validateDatabase(db);
+  return db;
+};
 
 module.exports = {
   Database,

packages/core/database/lib/metadata/relations.js

@@ -16,6 +16,8 @@ const isAnyToMany = (attribute) => ['oneToMany', 'manyToMany'].includes(attribut
 const isBidirectional = (attribute) => hasInversedBy(attribute) || hasMappedBy(attribute);
 const isOwner = (attribute) => !isBidirectional(attribute) || hasInversedBy(attribute);
 const shouldUseJoinTable = (attribute) => attribute.useJoinTable !== false;
+const getJoinTableName = (tableName, attributeName) =>
+  _.snakeCase(`${tableName}_${attributeName}_links`);
 
 /**
  * Creates a oneToOne relation metadata
@@ -397,7 +399,7 @@ const createJoinTable = (metadata, { attributeName, attribute, meta }) => {
     throw new Error(`Unknown target ${attribute.target}`);
   }
 
-  const joinTableName = _.snakeCase(`${meta.tableName}_${attributeName}_links`);
+  const joinTableName = getJoinTableName(meta.tableName, attributeName);
 
   const joinColumnName = _.snakeCase(`${meta.singularName}_id`);
   let inverseJoinColumnName = _.snakeCase(`${targetMeta.singularName}_id`);
@@ -560,4 +562,5 @@ module.exports = {
   isAnyToMany,
   hasOrderColumn,
   hasInverseOrderColumn,
+  getJoinTableName,
 };

packages/core/database/lib/validations/index.js

@@ -0,0 +1,20 @@
+'use strict';
+
+const { validateRelations } = require('./relations');
+
+/**
+ * Validate if the database is in a valid state before starting the server.
+ *
+ * @param {*} db - Database instance
+ */
+async function validateDatabase(db) {
+  const relationErrors = await validateRelations(db);
+  const errorList = [...relationErrors];
+
+  if (errorList.length > 0) {
+    errorList.forEach((error) => strapi.log.error(error));
+    throw new Error('There are errors in some of your models. Please check the logs above.');
+  }
+}
+
+module.exports = { validateDatabase };

packages/core/database/lib/validations/relations/bidirectional.js

@@ -0,0 +1,89 @@
+'use strict';
+
+const types = require('../../types');
+const { getJoinTableName } = require('../../metadata/relations');
+
+const getLinksWithoutMappedBy = (db) => {
+  const relationsToUpdate = {};
+
+  db.metadata.forEach((contentType) => {
+    const attributes = contentType.attributes;
+
+    // For each relation attribute, add the joinTable name to tablesToUpdate
+    Object.values(attributes).forEach((attribute) => {
+      if (!types.isRelation(attribute.type)) return;
+
+      if (attribute.inversedBy) {
+        const invRelation = db.metadata.get(attribute.target).attributes[attribute.inversedBy];
+
+        // Both relations use inversedBy.
+        if (invRelation.inversedBy) {
+          relationsToUpdate[attribute.joinTable.name] = {
+            relation: attribute,
+            invRelation,
+          };
+        }
+      }
+    });
+  });
+
+  return Object.values(relationsToUpdate);
+};
+
+const isLinkTableEmpty = async (db, linkTableName) => {
+  // If the table doesn't exist, it's empty
+  const exists = await db.getConnection().schema.hasTable(linkTableName);
+  if (!exists) return true;
+
+  const result = await db.getConnection().count('* as count').from(linkTableName);
+  return Number(result[0].count) === 0;
+};
+
+/**
+ * Validates bidirectional relations before starting the server.
+ * - If both sides use inversedBy, one of the sides must switch to mappedBy.
+ *    When this happens, two join tables exist in the database.
+ *    This makes sure you switch the side which does not delete any data.
+ *
+ * @param {*} db
+ * @return {*}
+ */
+const validateBidirectionalRelations = async (db) => {
+  const invalidLinks = getLinksWithoutMappedBy(db);
+  const errorList = [];
+
+  for (const { relation, invRelation } of invalidLinks) {
+    const contentType = db.metadata.get(invRelation.target);
+    const invContentType = db.metadata.get(relation.target);
+
+    // Generate the join table name based on the relation target table and attribute name.
+    const joinTableName = getJoinTableName(contentType.tableName, invRelation.inversedBy);
+    const inverseJoinTableName = getJoinTableName(invContentType.tableName, relation.inversedBy);
+
+    const joinTableEmpty = await isLinkTableEmpty(db, joinTableName);
+    const inverseJoinTableEmpty = await isLinkTableEmpty(db, inverseJoinTableName);
+
+    if (joinTableEmpty) {
+      process.emitWarning(
+        `Error on attribute "${invRelation.inversedBy}" in model "${contentType.singularName}" (${contentType.uid}).` +
+          ` Please modify your ${contentType.singularName} schema by renaming the key "inversedBy" to "mappedBy".` +
+          ` Ex: { "inversedBy": "${relation.inversedBy}" } -> { "mappedBy": "${relation.inversedBy}" }`
+      );
+    } else if (inverseJoinTableEmpty) {
+      // Its safe to delete the inverse join table
+      process.emitWarning(
+        `Error on attribute "${relation.inversedBy}" in model "${invContentType.singularName}" (${invContentType.uid}).` +
+          ` Please modify your ${invContentType.singularName} schema by renaming the key "inversedBy" to "mappedBy".` +
+          ` Ex: { "inversedBy": "${invRelation.inversedBy}" } -> { "mappedBy": "${invRelation.inversedBy}" }`
+      );
+    } else {
+      // Both sides have data in the join table
+    }
+  }
+
+  return errorList;
+};
+
+module.exports = {
+  validateBidirectionalRelations,
+};

packages/core/database/lib/validations/relations/index.js

@@ -0,0 +1,14 @@
+'use strict';
+
+const { validateBidirectionalRelations } = require('./bidirectional');
+
+/**
+ * Validates if relations data and tables are in a valid state before
+ * starting the server.
+ */
+const validateRelations = async (db) => {
+  const bidirectionalRelationsErrors = await validateBidirectionalRelations(db);
+  return [...bidirectionalRelationsErrors];
+};
+
+module.exports = { validateRelations };