Skip to content
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

[WIP] Upgrade Knex to latest version and swap unsupported sqlite3 fork #12557

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 57 additions & 15 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2-beta
Expand All @@ -30,7 +30,7 @@ jobs:
CODECOV_TOKEN: ${{ secrets.codecov }}
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2-beta
Expand All @@ -50,7 +50,7 @@ jobs:
CODECOV_TOKEN: ${{ secrets.codecov }}
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2-beta
Expand All @@ -70,7 +70,7 @@ jobs:
name: '[CE] E2E (postgres, node: ${{ matrix.node }})'
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
max-parallel: 3
services:
postgres:
Expand Down Expand Up @@ -106,7 +106,7 @@ jobs:
name: '[CE] E2E (mysql, node: ${{ matrix.node }})'
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
max-parallel: 3
services:
mysql:
Expand Down Expand Up @@ -142,7 +142,7 @@ jobs:
name: '[CE] E2E (mysql:5 , node: ${{ matrix.node }})'
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
max-parallel: 3
services:
mysql:
Expand Down Expand Up @@ -172,13 +172,13 @@ jobs:
with:
dbOptions: '--dbclient=mysql --dbhost=localhost --dbport=3306 --dbname=strapi_test --dbusername=strapi --dbpassword=strapi'

e2e_ce_sqlite:
e2e_ce_sqlite3:
runs-on: ubuntu-latest
needs: [lint, unit_back, unit_front]
name: '[CE] E2E (sqlite, node: ${{ matrix.node }})'
name: '[CE] E2E (sqlite3, node: ${{ matrix.node }})'
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
max-parallel: 3
steps:
- uses: actions/checkout@v2
Expand All @@ -189,7 +189,26 @@ jobs:
- uses: ./.github/actions/install-modules
- uses: ./.github/actions/run-e2e-tests
with:
dbOptions: '--dbclient=sqlite --dbfile=./tmp/data.db'
dbOptions: '--dbclient=sqlite3 --dbfile=./tmp/data.db'

e2e_ce_better_sqlite3:
runs-on: ubuntu-latest
needs: [lint, unit_back, unit_front]
name: '[CE] E2E (better_sqlite3, node: ${{ matrix.node }})'
strategy:
matrix:
node: [14, 16]
max-parallel: 3
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: ${{ matrix.node }}
cache: yarn
- uses: ./.github/actions/install-modules
- uses: ./.github/actions/run-e2e-tests
with:
dbOptions: '--dbclient=better-sqlite3 --dbfile=./tmp/data.db'

# EE
e2e_ee_pg:
Expand All @@ -201,7 +220,7 @@ jobs:
STRAPI_LICENSE: ${{ secrets.strapiLicense }}
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
max-parallel: 3
services:
postgres:
Expand Down Expand Up @@ -242,7 +261,7 @@ jobs:
STRAPI_LICENSE: ${{ secrets.strapiLicense }}
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
max-parallel: 3
services:
mysql:
Expand Down Expand Up @@ -276,13 +295,36 @@ jobs:
e2e_ee_sqlite:
runs-on: ubuntu-latest
needs: [lint, unit_back, unit_front]
name: '[EE] E2E (sqlite, node: ${{ matrix.node }})'
name: '[EE] E2E (sqlite3, node: ${{ matrix.node }})'
if: github.event.pull_request.head.repo.full_name == github.repository && !(github.actor == 'dependabot[bot]' || github.actor == 'dependabot-preview[bot]')
env:
STRAPI_LICENSE: ${{ secrets.strapiLicense }}
strategy:
matrix:
node: [14, 16]
max-parallel: 3
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: ${{ matrix.node }}
cache: yarn
- uses: ./.github/actions/install-modules
- uses: ./.github/actions/run-e2e-tests
with:
dbOptions: '--dbclient=sqlite3 --dbfile=./tmp/data.db'
runEE: true

e2e_ee_better_sqlite:
runs-on: ubuntu-latest
needs: [lint, unit_back, unit_front]
name: '[EE] E2E (better_sqlite3, node: ${{ matrix.node }})'
if: github.event.pull_request.head.repo.full_name == github.repository && !(github.actor == 'dependabot[bot]' || github.actor == 'dependabot-preview[bot]')
env:
STRAPI_LICENSE: ${{ secrets.strapiLicense }}
strategy:
matrix:
node: [12, 14, 16]
node: [14, 16]
max-parallel: 3
steps:
- uses: actions/checkout@v2
Expand All @@ -293,5 +335,5 @@ jobs:
- uses: ./.github/actions/install-modules
- uses: ./.github/actions/run-e2e-tests
with:
dbOptions: '--dbclient=sqlite --dbfile=./tmp/data.db'
dbOptions: '--dbclient=better-sqlite3 --dbfile=./tmp/data.db'
runEE: true
8 changes: 5 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,23 @@ The administration panel should now be available at http://localhost:4000/admin.
The end-to-end tests require a Strapi app to be able to run. You can generate a "test app" using `yarn test:generate-app <database>`:

```bash
$ yarn test:generate-app sqlite
$ yarn test:generate-app sqlite3
$ yarn test:generate-app postgres
$ yarn test:generate-app mysql
$ yarn test:generate-app mysql2
```

A new app is required every time you run the end-to-end tests otherwise, the test suite will fail. A script is available to make this process easier: `node test/e2e.js`. It will delete the current test app, generate a new one and run the test suite.

### Changing the database

By default the script `test/e2e,js` creates an app that uses `sqlite`. But you can run the test suites using different databases:
By default the script `test/e2e,js` creates an app that uses `sqlite3`. But you can run the test suites using different databases:

```bash
$ node test/e2e.js --db=sqlite
$ node test/e2e.js --db=sqlite3
$ node test/e2e.js --db=postgres
$ node test/e2e.js --db=mysql
$ node test/e2e.js --db=mysql2
```

### Running the tests for the Community Editon (CE)\*\*
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"@testing-library/user-event": "13.5.0",
"axios-mock-adapter": "1.20.0",
"babel-eslint": "10.1.0",
"better-sqlite3": "7.5.0",
"chalk": "4.1.2",
"chokidar": "3.5.3",
"cross-env": "7.0.3",
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/create-strapi-app/create-strapi-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ program
.option('--dbusername <dbusername>', 'Database username')
.option('--dbpassword <dbpassword>', 'Database password')
.option('--dbssl <dbssl>', 'Database SSL')
.option('--dbfile <dbfile>', 'Database file path for sqlite')
.option('--dbfile <dbfile>', 'Database file path for sqlite3')
.option('--dbforce', 'Overwrite database content if any')
.option('--template <templateurl>', 'Specify a Strapi template')
.description('create a new application')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ program
.option('--dbusername <dbusername>', 'Database username')
.option('--dbpassword <dbpassword>', 'Database password')
.option('--dbssl <dbssl>', 'Database SSL')
.option('--dbfile <dbfile>', 'Database file path for sqlite')
.option('--dbfile <dbfile>', 'Database file path for sqlite3')
.option('--dbforce', 'Overwrite database content if any')
.description(
'Create a fullstack monorepo application using the strapi backend template specified in the provided starter'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ const GuidedTourHomepage = () => {
const activeSection = enrichedSections.find(section => !section.isDone)?.key;

const handleSkip = () => {
setSkipped(true)
setSkipped(true);
trackUsage('didSkipGuidedtour');
}
};

return (
<Box
Expand Down
6 changes: 4 additions & 2 deletions packages/core/database/lib/dialects/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ const getDialectClass = client => {
case 'postgres':
return require('./postgresql');
case 'mysql':
case 'mysql2':
return require('./mysql');
case 'sqlite':
return require('./sqlite');
case 'sqlite3':
case 'better-sqlite3':
return require('./sqlite3');
default:
throw new Error(`Unknown dialect ${client}`);
}
Expand Down
6 changes: 3 additions & 3 deletions packages/core/database/lib/entity-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,12 @@ const createEntityManager = db => {
.insert(dataToInsert)
.execute();

await this.attachRelations(uid, id, data);
await this.attachRelations(uid, id.id, data);

// TODO: in case there is no select or populate specified return the inserted data ?
// TODO: do not trigger the findOne lifecycles ?
// TODO: do not trigger the findOne lifecycles
const result = await this.findOne(uid, {
where: { id },
where: { ...id },
select: params.select,
populate: params.populate,
});
Expand Down
6 changes: 4 additions & 2 deletions packages/core/database/lib/query/helpers/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ const applySearch = (knex, query, ctx) => {

break;
}
case 'sqlite': {
case 'sqlite3':
case 'better-sqlite3': {
searchColumns.forEach(attr => {
const columnName = toColumnName(meta, attr);
return knex.orWhereRaw(`?? LIKE ? ESCAPE '\\'`, [
Expand All @@ -51,7 +52,8 @@ const applySearch = (knex, query, ctx) => {
});
break;
}
case 'mysql': {
case 'mysql':
case 'mysql2': {
searchColumns.forEach(attr => {
const columnName = toColumnName(meta, attr);
return knex.orWhereRaw(`?? LIKE ?`, [
Expand Down
4 changes: 4 additions & 0 deletions packages/core/database/lib/schema/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ const createTable = meta => {
} else if (types.isScalar(attribute.type)) {
const column = createColumn(attribute.columnName || key, attribute);

if (column.name === 'id') {
column.primary = true;
}

if (column.unique) {
table.indexes.push({
type: 'unique',
Expand Down
2 changes: 1 addition & 1 deletion packages/core/database/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"date-fns": "2.22.1",
"debug": "4.3.1",
"fs-extra": "10.0.0",
"knex": "0.95.6",
"knex": "1.0.4",
"lodash": "4.17.21",
"umzug": "2.3.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/strapi/bin/strapi.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ program
.option('--dbusername <dbusername>', 'Database username')
.option('--dbpassword <dbpassword>', 'Database password')
.option('--dbssl <dbssl>', 'Database SSL')
.option('--dbfile <dbfile>', 'Database file path for sqlite')
.option('--dbfile <dbfile>', 'Database file path for sqlite3')
.option('--dbforce', 'Allow overwriting existing database content')
.description('Create a new application')
.action(require('../lib/commands/new'));
Expand Down
4 changes: 2 additions & 2 deletions packages/core/strapi/tests/filtering.test.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ describe('Filtering API', () => {
});
});

// FIXME: Not working on sqlite due to https://www.sqlite.org/draft/pragma.html#pragma_case_sensitive_like
// FIXME: Not working on sqlite3 due to https://www.sqlite.org/draft/pragma.html#pragma_case_sensitive_like
describe('Filter contains sensitive', () => {
test.skip('Should return empty if the case does not match', async () => {
const res = await rq({
Expand Down Expand Up @@ -421,7 +421,7 @@ describe('Filtering API', () => {
});
});

// FIXME: Not working on sqlite due to https://www.sqlite.org/draft/pragma.html#pragma_case_sensitive_like
// FIXME: Not working on sqlite3 due to https://www.sqlite.org/draft/pragma.html#pragma_case_sensitive_like
describe('Filter not contains sensitive', () => {
test.skip('Should return the entities if the case does not match', async () => {
const res = await rq({
Expand Down
6 changes: 3 additions & 3 deletions packages/generators/app/lib/create-customized-project.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ async function askDbInfosAndTest(scope) {
async function testDatabaseConnection({ scope, configuration }) {
const { client } = configuration;

if (client === 'sqlite') return;
if (client === 'sqlite3' || client === 'better-sqlite3') return;

return;

Expand Down Expand Up @@ -122,8 +122,8 @@ async function askDatabaseInfos(scope) {
type: 'list',
name: 'client',
message: 'Choose your default database client',
choices: ['sqlite', 'postgres', 'mysql'],
default: 'sqlite',
choices: ['better-sqlite3', 'sqlite3', 'postgres', 'mysql', 'mysql2'],
default: 'better-sqlite3',
},
]);

Expand Down
4 changes: 2 additions & 2 deletions packages/generators/app/lib/create-quickstart-project.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ module.exports = async function createQuickStartProject(scope) {
console.log('Creating a quickstart project.');
await trackUsage({ event: 'didChooseQuickstart', scope });

// get default sqlite config
const client = 'sqlite';
// get default sqlite3 config
const client = 'better-sqlite3';
const configuration = {
client,
connection: defaultConfigs[client],
Expand Down
2 changes: 1 addition & 1 deletion packages/generators/app/lib/generate-new.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module.exports = async scope => {
return createCLIDatabaseProject(scope);
}

// if cli quickstart create project with default sqlite options
// if cli quickstart create project with default sqlite3 options
if (scope.quick === true) {
return createQuickStartProject(scope);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const path = require('path');

module.exports = ({ env }) => ({
connection: {
client: 'better-sqlite3',
connection: {
filename: path.join(__dirname, '..', env('DATABASE_FILENAME', '<%= connection.filename %>')),
},
useNullAsDefault: true,
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module.exports = ({ env }) => ({
connection: {
client: 'mysql2',
connection: {
host: env('DATABASE_HOST', '<%= connection.host %>'),
port: env.int('DATABASE_PORT', <%= connection.port %>),
database: env('DATABASE_NAME', '<%= connection.database %>'),
user: env('DATABASE_USERNAME', '<%= connection.username %>'),
password: env('DATABASE_PASSWORD', '<%= connection.password %>'),
ssl: env.bool('DATABASE_SSL', <%= connection.ssl %>),
},
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const path = require('path');

module.exports = ({ env }) => ({
connection: {
client: 'sqlite',
client: 'sqlite3',
connection: {
filename: path.join(__dirname, '..', env('DATABASE_FILENAME', '<%= connection.filename %>')),
},
Expand Down