diff --git a/.github/actions/install-modules/action.yml b/.github/actions/install-modules/action.yml deleted file mode 100644 index 73e6826b46..0000000000 --- a/.github/actions/install-modules/action.yml +++ /dev/null @@ -1,7 +0,0 @@ -name: 'Install modules' -description: 'Install yarn dependencies' -runs: - using: 'composite' - steps: - - run: $GITHUB_ACTION_PATH/script.sh - shell: bash diff --git a/.github/actions/install-modules/script.sh b/.github/actions/install-modules/script.sh deleted file mode 100755 index f4a3806144..0000000000 --- a/.github/actions/install-modules/script.sh +++ /dev/null @@ -1,2 +0,0 @@ -# run yarn -yarn diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0ab790d4a1..35b4b38363 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -26,7 +26,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - name: Run lint run: yarn run -s lint @@ -43,7 +43,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - name: Run tests run: yarn run -s test:unit --coverage - name: Upload coverage to Codecov @@ -66,7 +66,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - name: Build run: yarn build - name: Run test @@ -110,7 +110,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - uses: ./.github/actions/run-api-tests with: dbOptions: '--dbclient=postgres --dbhost=localhost --dbport=5432 --dbname=strapi_test --dbusername=strapi --dbpassword=strapi' @@ -145,7 +145,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - uses: ./.github/actions/run-api-tests with: dbOptions: '--dbclient=mysql --dbhost=localhost --dbport=3306 --dbname=strapi_test --dbusername=strapi --dbpassword=strapi' @@ -180,7 +180,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - uses: ./.github/actions/run-api-tests with: dbOptions: '--dbclient=mysql --dbhost=localhost --dbport=3306 --dbname=strapi_test --dbusername=strapi --dbpassword=strapi' @@ -199,7 +199,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - uses: ./.github/actions/run-api-tests env: SQLITE_PKG: ${{ matrix.sqlite_pkg }} @@ -241,12 +241,11 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - uses: ./.github/actions/run-api-tests with: dbOptions: '--dbclient=postgres --dbhost=localhost --dbport=5432 --dbname=strapi_test --dbusername=strapi --dbpassword=strapi' runEE: true - api_ee_mysql: runs-on: ubuntu-latest needs: [lint, unit_back, unit_front] @@ -280,7 +279,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - uses: ./.github/actions/run-api-tests with: dbOptions: '--dbclient=mysql --dbhost=localhost --dbport=3306 --dbname=strapi_test --dbusername=strapi --dbpassword=strapi' @@ -303,7 +302,7 @@ jobs: with: node-version: ${{ matrix.node }} cache: yarn - - uses: ./.github/actions/install-modules + - run: yarn install --frozen-lockfile - uses: ./.github/actions/run-api-tests env: SQLITE_PKG: ${{ matrix.sqlite_pkg }} diff --git a/docs/docs/core/content-manager/relations.mdx b/docs/docs/core/content-manager/relations.mdx new file mode 100644 index 0000000000..8a080eacda --- /dev/null +++ b/docs/docs/core/content-manager/relations.mdx @@ -0,0 +1,144 @@ +--- +title: Relations +slug: /content-manager/relations +description: Conceptual guide to relations in the Content Manager focussing on the technical decisions taken. +tags: + - content-manager + - relations + - redux-store +--- + +## Summary + +Relations are a term used to describe how two or more entities are connected. Previously in the sidebar of an entity, +in Nov2020 we released a refactor that moved these fields into the main editing flow for a better editor experience +and to improve performance of the CMS application when many relations were used. + +An example of the relations input in the CMS edit view + +_above: An example of the relations input in the CMS edit view_ + +## Data management in frontend + +a diagram overview explaining how state management works in relations + +_above: A high-level diagram of how relations state management works_ + +### Preparing relation fields in the store + +When you first open an existing entity, we call the admin API and put the data into the store to pre-populate fields +with existing values. However, its important to know when you have fields with `type === 'relation'` in your schema +that the data you receive will not be an array, but rather an object with the count of how many relations in that +field exist. For example, a section of the response may look like this: + +```json +{ + "my_relations": { + "count": 6 + } +} +``` + +So without intervention, your inputs would try to append new relations to the `my_relations` object, which would not +work. Instead of this, before calling the redux action `INIT_FORM` we recursively find the paths fields based on the +following conditions: + +- The field is a relation +- The field is a component +- The field is a repeatable component +- The field is a dynamic zone + +These paths _do not_ take into account index values. So if you have a repetable component field where the schema looks like: + +```json +{ + "repeatable_single_component_relation": { + "type": "component", + "repeatable": true, + "component": "basic.relation" + } +} +``` + +and the components looks like: + +```json +{ + "basic.relation": { + "attributes": { + "id": { + "type": "integer" + }, + "categories": { + "type": "relation", + "relation": "oneToMany", + "target": "api::category.category", + "targetModel": "api::category.category", + "relationType": "oneToMany" + }, + "my_name": { + "type": "string" + } + } + } +} +``` + +Then the path to the relation field would be `repeatable_single_component_relation.categories`. Even though when +relations are added the path to the field in the redux store would be `repeatable_single_component_relation.0.categories`. + +Inside the reducer we reduce the array of `relationalFieldPaths` to an object with the `initialValues` clone as +as the base. If there is `modifiedData` in the browser i.e. you've made changes to the entity and saved those changes, +we just replace the first level of the field with the `modifiedData` so the data structure is preserved and we're not +loosing the relations we had already loaded in the component. If the first part of the path is highlighted as the +`relationalField` then we simply replace that intial object with an empty array. + +However, if the first part of the path is either a repeatable component, a dynamic zone or a regular component then we +recursively find the relation fields and replace the object with an array. This is handled by the `findLeafByPathAndReplace` +utility function. This function in short, takes an end path (in this case the relational field) and a primitive to replace +when it finds the endpath (an empty array in this case). It then recursively reduces the paths to the relational field mapping +through arrays if necessary (in the instance of repetable components for example) replacing the endpath with the primitive. + +When this is done, we have sucessfully prepared our initial data for usage with relations. + +### Handling updates to relation fields + +Because we've prepared the fields prior to the component loading, adding & removing relations, it's relatively easy to do so. +When a relation is added, we simply push the new relation to the array of relations. When a relation is removed, we simply +filter out the relation from the array of relations. This is handled inside the reducer actions `CONNECT_RELATION` & +`DISCONNECT_RELATION` respectively. + +:::note +Connecting relations adds the item to the end of the list, whilst loading more relations prepends to +the beginning of the list. This is the expected behaviour. +::: + +The `RelationInput` component takes the field in `modifiedData` as its source of truth. You could therefore consider this to +be the `browserState` and `initialData` to be the `serverState`. When relations are loaded they're added to both the `intialData` +and `modifiedData` objects, but when you connect/disconnect only the `modifiedData` is updated. This is useful when we're preparing +data for the api. + +### Cleaning data to be posted to the API + +The API to update the enttiy expects relations to be categorised into two groups, a `connect` array and `disconnect` array. +You could do this as the user interacts with the input but we found this to be confusing and then involved us managing three +different arrays which makes the code more complex. Instead, because the browser doesn't really care about whats new and removed +and we have a copy of the slice of data we're mutating from the server we can run a small diff algorithm to determine which +relations have been connected and which have been disconnected. Returning an object like so: + +```json +{ + "my_relations": { + "connect": [{ "id": 1 }, { "id": 2 }], + "disconnect": [] + } +} +``` + +## Frontend component architecture diff --git a/docs/sidebars.js b/docs/sidebars.js index 24047f984b..c64e6f077a 100644 --- a/docs/sidebars.js +++ b/docs/sidebars.js @@ -31,7 +31,13 @@ const sidebars = { type: 'doc', id: 'core/content-manager/intro', }, - items: ['example'], + items: [ + { + type: 'doc', + label: 'Relations', + id: 'core/content-manager/relations', + }, + ], }, { type: 'category', diff --git a/docs/static/img/content-manager/relations/component-example.png b/docs/static/img/content-manager/relations/component-example.png new file mode 100644 index 0000000000..71630c2142 Binary files /dev/null and b/docs/static/img/content-manager/relations/component-example.png differ diff --git a/docs/static/img/content-manager/relations/relations-statemanagemen-diagram.png b/docs/static/img/content-manager/relations/relations-statemanagemen-diagram.png new file mode 100644 index 0000000000..7092b517f4 Binary files /dev/null and b/docs/static/img/content-manager/relations/relations-statemanagemen-diagram.png differ diff --git a/examples/getstarted/config/database.js b/examples/getstarted/config/database.js index d78eb0ebc8..88aa65cd8f 100644 --- a/examples/getstarted/config/database.js +++ b/examples/getstarted/config/database.js @@ -28,10 +28,22 @@ const mysql = { }, }; +const mariadb = { + client: 'mysql', + connection: { + database: 'strapi', + user: 'strapi', + password: 'strapi', + port: 3307, + host: 'localhost', + }, +}; + const db = { mysql, sqlite, postgres, + mariadb, }; module.exports = { diff --git a/packages/core/admin/package.json b/packages/core/admin/package.json index 67b9c09203..78d2ba0004 100644 --- a/packages/core/admin/package.json +++ b/packages/core/admin/package.json @@ -128,7 +128,7 @@ "reselect": "^4.0.0", "rimraf": "3.0.2", "sanitize-html": "2.7.1", - "semver": "7.3.7", + "semver": "7.3.8", "sift": "16.0.0", "style-loader": "3.3.1", "styled-components": "5.3.3", diff --git a/packages/core/database/lib/dialects/dialect.js b/packages/core/database/lib/dialects/dialect.js index 4ead7f9200..3d05b56c27 100644 --- a/packages/core/database/lib/dialects/dialect.js +++ b/packages/core/database/lib/dialects/dialect.js @@ -29,6 +29,10 @@ class Dialect { return false; } + supportsWindowFunctions() { + return true; + } + async startSchemaUpdate() { // noop } diff --git a/packages/core/database/lib/dialects/mysql/constants.js b/packages/core/database/lib/dialects/mysql/constants.js new file mode 100644 index 0000000000..1dcff942f6 --- /dev/null +++ b/packages/core/database/lib/dialects/mysql/constants.js @@ -0,0 +1,6 @@ +'use strict'; + +module.exports = { + MYSQL: 'MYSQL', + MARIADB: 'MARIADB', +}; diff --git a/packages/core/database/lib/dialects/mysql/database-inspector.js b/packages/core/database/lib/dialects/mysql/database-inspector.js new file mode 100644 index 0000000000..229a9cd109 --- /dev/null +++ b/packages/core/database/lib/dialects/mysql/database-inspector.js @@ -0,0 +1,37 @@ +'use strict'; + +const { MARIADB, MYSQL } = require('./constants'); + +const SQL_QUERIES = { + VERSION: `SELECT version() as version`, +}; + +class MysqlDatabaseInspector { + constructor(db) { + this.db = db; + } + + async getInformation() { + let database; + let versionNumber; + try { + const [results] = await this.db.connection.raw(SQL_QUERIES.VERSION); + const versionSplit = results[0].version.split('-'); + const databaseName = versionSplit[1]; + versionNumber = versionSplit[0]; + database = databaseName && databaseName.toLowerCase() === 'mariadb' ? MARIADB : MYSQL; + } catch (e) { + return { + database: null, + version: null, + }; + } + + return { + database, + version: versionNumber, + }; + } +} + +module.exports = MysqlDatabaseInspector; diff --git a/packages/core/database/lib/dialects/mysql/index.js b/packages/core/database/lib/dialects/mysql/index.js index 9e01d5e0b1..668a78a4b1 100644 --- a/packages/core/database/lib/dialects/mysql/index.js +++ b/packages/core/database/lib/dialects/mysql/index.js @@ -1,13 +1,19 @@ 'use strict'; +const semver = require('semver'); + const { Dialect } = require('../dialect'); const MysqlSchemaInspector = require('./schema-inspector'); +const MysqlDatabaseInspector = require('./database-inspector'); +const { MYSQL } = require('./constants'); class MysqlDialect extends Dialect { constructor(db) { super(db); this.schemaInspector = new MysqlSchemaInspector(db); + this.databaseInspector = new MysqlDatabaseInspector(db); + this.info = null; } configure() { @@ -38,6 +44,8 @@ class MysqlDialect extends Dialect { } catch (err) { // Ignore error due to lack of session permissions } + + this.info = await this.databaseInspector.getInformation(); } async startSchemaUpdate() { @@ -57,6 +65,17 @@ class MysqlDialect extends Dialect { return true; } + supportsWindowFunctions() { + const isMysqlDB = !this.info.database || this.info.database === MYSQL; + const isBeforeV8 = !semver.valid(this.info.version) || semver.lt(this.info.version, '8.0.0'); + + if (isMysqlDB && isBeforeV8) { + return false; + } + + return true; + } + usesForeignKeys() { return true; } diff --git a/packages/core/database/lib/dialects/postgresql/index.js b/packages/core/database/lib/dialects/postgresql/index.js index 8cebf7be17..b865534f4f 100644 --- a/packages/core/database/lib/dialects/postgresql/index.js +++ b/packages/core/database/lib/dialects/postgresql/index.js @@ -15,7 +15,7 @@ class PostgresDialect extends Dialect { return true; } - initialize() { + async initialize() { this.db.connection.client.driver.types.setTypeParser(1082, 'text', (v) => v); // Don't cast DATE string to Date() this.db.connection.client.driver.types.setTypeParser(1700, 'text', parseFloat); } diff --git a/packages/core/database/lib/dialects/sqlite/index.js b/packages/core/database/lib/dialects/sqlite/index.js index 1b91a6c214..cc4ee9e2ac 100644 --- a/packages/core/database/lib/dialects/sqlite/index.js +++ b/packages/core/database/lib/dialects/sqlite/index.js @@ -5,13 +5,13 @@ const fse = require('fs-extra'); const errors = require('../../errors'); const { Dialect } = require('../dialect'); -const SqliteSchmeaInspector = require('./schema-inspector'); +const SqliteSchemaInspector = require('./schema-inspector'); class SqliteDialect extends Dialect { constructor(db) { super(db); - this.schemaInspector = new SqliteSchmeaInspector(db); + this.schemaInspector = new SqliteSchemaInspector(db); } configure() { diff --git a/packages/core/database/lib/entity-manager/regular-relations.js b/packages/core/database/lib/entity-manager/regular-relations.js index 0174cae83b..86e652ed72 100644 --- a/packages/core/database/lib/entity-manager/regular-relations.js +++ b/packages/core/database/lib/entity-manager/regular-relations.js @@ -1,6 +1,7 @@ 'use strict'; const { map, isEmpty } = require('lodash/fp'); + const { isBidirectional, isOneToAny, @@ -196,6 +197,12 @@ const cleanOrderColumns = async ({ id, attribute, db, inverseRelIds, transaction return; } + // Handle databases that don't support window function ROW_NUMBER + if (!strapi.db.dialect.supportsWindowFunctions()) { + await cleanOrderColumnsForOldDatabases({ id, attribute, db, inverseRelIds, transaction: trx }); + return; + } + const { joinTable } = attribute; const { joinColumn, inverseJoinColumn, orderColumnName, inverseOrderColumnName } = joinTable; const update = []; @@ -274,6 +281,103 @@ const cleanOrderColumns = async ({ id, attribute, db, inverseRelIds, transaction } }; +const cleanOrderColumnsForOldDatabases = async ({ + id, + attribute, + db, + inverseRelIds, + transaction: trx, +}) => { + const { joinTable } = attribute; + const { joinColumn, inverseJoinColumn, orderColumnName, inverseOrderColumnName } = joinTable; + + const now = new Date().valueOf(); + + if (hasOrderColumn(attribute) && id) { + const tempOrderTableName = `tempOrderTableName_${now}`; + try { + await db.connection + .raw( + ` + CREATE TEMPORARY TABLE :tempOrderTableName: + SELECT + id, + ( + SELECT count(*) + FROM :joinTableName: b + WHERE a.:orderColumnName: >= b.:orderColumnName: AND a.:joinColumnName: = b.:joinColumnName: AND a.:joinColumnName: = :id + ) AS src_order + FROM :joinTableName: a`, + { + tempOrderTableName, + joinTableName: joinTable.name, + orderColumnName, + joinColumnName: joinColumn.name, + id, + } + ) + .transacting(trx); + await db.connection + .raw( + `UPDATE ?? as a, (SELECT * FROM ??) AS b + SET ?? = b.src_order + WHERE a.id = b.id`, + [joinTable.name, tempOrderTableName, orderColumnName] + ) + .transacting(trx); + } finally { + await db.connection + .raw(`DROP TEMPORARY TABLE IF EXISTS ??`, [tempOrderTableName]) + .transacting(trx); + } + } + + if (hasInverseOrderColumn(attribute) && !isEmpty(inverseRelIds)) { + const tempInvOrderTableName = `tempInvOrderTableName_${now}`; + try { + await db.connection + .raw( + ` + CREATE TEMPORARY TABLE ?? + SELECT + id, + ( + SELECT count(*) + FROM ?? b + WHERE a.?? >= b.?? AND a.?? = b.?? AND a.?? IN (${inverseRelIds + .map(() => '?') + .join(', ')}) + ) AS inv_order + FROM ?? a`, + [ + tempInvOrderTableName, + joinTable.name, + inverseOrderColumnName, + inverseOrderColumnName, + inverseJoinColumn.name, + inverseJoinColumn.name, + inverseJoinColumn.name, + ...inverseRelIds, + joinTable.name, + ] + ) + .transacting(trx); + await db.connection + .raw( + `UPDATE ?? as a, (SELECT * FROM ??) AS b + SET ?? = b.inv_order + WHERE a.id = b.id`, + [joinTable.name, tempInvOrderTableName, inverseOrderColumnName] + ) + .transacting(trx); + } finally { + await db.connection + .raw(`DROP TEMPORARY TABLE IF EXISTS ??`, [tempInvOrderTableName]) + .transacting(trx); + } + } +}; + module.exports = { deletePreviousOneToAnyRelations, deletePreviousAnyToOneRelations, diff --git a/packages/core/database/package.json b/packages/core/database/package.json index a1219cc38c..8014111cb4 100644 --- a/packages/core/database/package.json +++ b/packages/core/database/package.json @@ -36,6 +36,7 @@ "fs-extra": "10.0.0", "knex": "1.0.7", "lodash": "4.17.21", + "semver": "7.3.8", "umzug": "3.1.1" }, "engines": { diff --git a/packages/core/strapi/package.json b/packages/core/strapi/package.json index 34f46fb36c..68a50a52e2 100644 --- a/packages/core/strapi/package.json +++ b/packages/core/strapi/package.json @@ -128,7 +128,7 @@ "package-json": "7.0.0", "qs": "6.10.1", "resolve-cwd": "3.0.0", - "semver": "7.3.7", + "semver": "7.3.8", "statuses": "2.0.1", "uuid": "^8.3.2" }, diff --git a/packages/generators/app/package.json b/packages/generators/app/package.json index e728a25622..29d6eeb01c 100644 --- a/packages/generators/app/package.json +++ b/packages/generators/app/package.json @@ -45,7 +45,7 @@ "node-fetch": "^2.6.1", "node-machine-id": "^1.1.10", "ora": "^5.4.1", - "semver": "^7.3.4", + "semver": "7.3.8", "tar": "6.1.11", "uuid": "^8.3.2" }, diff --git a/test/api.js b/test/api.js index e3341236e7..410bd5c575 100644 --- a/test/api.js +++ b/test/api.js @@ -69,17 +69,11 @@ const main = async ({ database, generateApp }, args) => { } await runAllTests(args).catch(() => { - process.stdout.write('Tests failed\n', () => { - process.exit(1); - }); - }); - - process.exit(0); - } catch (error) { - console.error(error); - process.stdout.write('Tests failed\n', () => { process.exit(1); }); + } catch (error) { + console.error(error); + process.exit(1); } }; diff --git a/yarn.lock b/yarn.lock index c2f0cd66fc..b463fff030 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20409,10 +20409,10 @@ semver@7.3.4: dependencies: lru-cache "^6.0.0" -semver@7.3.7: - version "7.3.7" - resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.7.tgz#12c5b649afdbf9049707796e22a4028814ce523f" - integrity sha512-QlYTucUYOews+WeEujDoEGziz4K6c47V/Bd+LjSSYcA94p+DmINdf7ncaUinThfvZyu13lN9OY1XDxt8C0Tw0g== +semver@7.3.8, semver@^7.3.8: + version "7.3.8" + resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.8.tgz#07a78feafb3f7b32347d725e33de7e2a2df67798" + integrity sha512-NB1ctGL5rlHrPJtFDVIVzTyQylMLu9N9VICA6HSFJo8MCGVTMW6gfpicwKmmK/dAjTOrqu5l63JJOpDSrAis3A== dependencies: lru-cache "^6.0.0" @@ -20421,10 +20421,10 @@ semver@^6.0.0, semver@^6.1.0, semver@^6.1.1, semver@^6.1.2, semver@^6.3.0: resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.0.tgz#ee0a64c8af5e8ceea67687b133761e1becbd1d3d" integrity sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw== -semver@^7.0.0, semver@^7.1.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5, semver@^7.3.7, semver@^7.3.8: - version "7.3.8" - resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.8.tgz#07a78feafb3f7b32347d725e33de7e2a2df67798" - integrity sha512-NB1ctGL5rlHrPJtFDVIVzTyQylMLu9N9VICA6HSFJo8MCGVTMW6gfpicwKmmK/dAjTOrqu5l63JJOpDSrAis3A== +semver@^7.0.0, semver@^7.1.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5, semver@^7.3.7: + version "7.3.7" + resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.7.tgz#12c5b649afdbf9049707796e22a4028814ce523f" + integrity sha512-QlYTucUYOews+WeEujDoEGziz4K6c47V/Bd+LjSSYcA94p+DmINdf7ncaUinThfvZyu13lN9OY1XDxt8C0Tw0g== dependencies: lru-cache "^6.0.0"