-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
[WIP] Switch to using native SQLite bindings #24414
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
Conversation
ref #23924 closes https://linear.app/ghost/issue/PROD-1763/breaking-change-drop-support-for-node-18 - Node.js v18 was EOL on April 30, 2025 - This officially removes support for Node.js v18 in Ghost 6.0 - I've also removed some caveats from the dev tooling which applied to versions less than 18 and so is really not relevant anymore
ref #23924 ref https://linear.app/ghost/issue/PROD-1598/breaking-change-drop-support-for-node-v20 - Dropping support for Node.js v20 early as a breaking change - Node.js v22 has a couple of new features we want to use: - support for typescript syntax: https://nodejs.org/en/learn/typescript/run-natively - support for loading esm modules https://nodejs.org/en/learn/modules/publishing-a-package - These will allow us to ugprade tooling and move faster - Although we usually support all LTS versions of Node.js, there are significant benefits to being Node v22+ - Therefore we feel dropping Node.js v20 early is a reasonable course of action for a major release
ref #23924 closes https://linear.app/ghost/issue/PROD-2115/unload-amp-app-from-express - First step for removing AMP support in Ghost 6.0 is no longer loading the AMP app - The AMP app was responsible for redirecting /amp routes, so redirectAmpUrls was added as a middleware to take over that task - this was added as part of the pretty-urls middleware sequence
closes #22077 closes https://linear.app/ghost/issue/PROD-1611/breaking-change-schema-oss-issue-membersuuid-is-marked-as-nullable-in ref 4095178 ref 12f569e - The migration to add a UUID was originally added in Ghost 3.1 - Therefore, we're going to rerun it first, although we expect this to be a no-op in everything other than extreme edge cases - I have verified that there are zero instances of null member uuids on Pro - This then sets nullable to false in the DB after - The net result of this change is that the schema matches the requirements / expectations about the world
ref https://linear.app/ghost/issue/PROD-1597 Removed `updated_by` & `created_by` fields as they are deprecated and should not be being used anywhere meaningful in the codebase. Removal of these fields is a prerequisite for the work needed for https://linear.app/ghost/issue/PROD-1594 (removal of hardocded owner user id `1`) --------- Co-authored-by: Hannah Wolfe <[email protected]>
ref #23924 closes https://linear.app/ghost/issue/PROD-2116/completely-remove-amp-app-from-core - deletes the entire AMP app and its associated tests - no longer includes AMP as a possible context type and removes any checks for it - removes other tests related to AMP posts - ignores AMP as a valid setting in SettingsImporter - removes `amperize` from `package.json`
ref #23924 closes https://linear.app/ghost/issue/PROD-2117/completely-remove-amp-from-admin - removes amp from the group filter on the settings API, so we no longer return amp or amp_gtag_id
ref https://linear.app/ghost/issue/PROD-1597 ref #24053 Removed `updatedBy` & `createdBy` fields in the admin app as they are deprecated and should not be being used anywhere meaningful in the codebase
no ref Fixed `MentionSendingService.sendForPost` failing browser tests that was caused by the removal of the `x_by` fields
ref https://linear.app/ghost/issue/PROD-1605/ - adds `max-limit-cap` middleware that rewrites `?limit` parameters to a capped limit, including `?limit=all` - has an exceptions list for endpoints that still require a larger limit param - has config escape valves for exceptional use-cases after 6.0 upgrade - by rewriting the `query.limit` on the request it allows everything lower in the stack to behave as if the user requested the capped limit, avoiding need to adjust code anywhere else - sets default capped limit to 100 - applies the middleware to Admin API, Content API, and Comments API
ref https://linear.app/ghost/issue/PROD-1594 This changeset updates the system to ensure that the owner user is created with an `ObjectID` instead of being hardcoded to `1`, and there are no assumptions made that the owner user is `1`: - A simple placeholder replacement system has been introduced into the fixtures JSON to allow dynamic values to be used at runtime (for the generation of the owner user id) - I did toy with a few different ideas (swapping the format to a `js` func instead etc). but this snowballed into much bigger changes that did not warrant the complexity involved - i.e we don't want to be maintaining 2 different formats for the fixtures - The lazy initialisation of the fixtures may seem a bit strange at first, but this is to get around the fact that `models` may not always be initialised at the time they are used, depending on the environment and order of loading (i.e in the test environment, the fixture manager may be loaded before the model initialisation / the Knex migrator hooks do not get executed). This lazy initialisation means we can keep the ID generation logic for the user with the appropriate model, and we do not have to do `models.init` from within the fixture manager - The `contextUser` property provided by the `user-type` bookshelf plugin has been updated to be async, and instead of returning hardcoded `1`, it now resolves the owner user ID from the db - The resolved value is cached on first retrieval and cleared when an ownership transfer occurs - All the call sites of this property have also been updated to be async (`post` model, `author` relation model) - The `UpdateCheckService` has been updated to not assume that the first retrieved user is the owner user, and instead analyse the roles of those users to determine which is the owner user - The test suite has been updated to use a fixed owner user ID that is an `ObjectID` (`5951f5fc0000000000000000`) - The data generation tooling has been updated to retrieve the owner user ID from the db rather than assuming that it is `1`
…jectID` (#24033) ref https://linear.app/ghost/issue/PROD-2127 Added migration for migrating user with hardcoded ID of `1` to an `ObjectID`. We want to perform this migration on existing installations so we that we do not need to add codebase checks in the future for if a user id is `1` or an `ObjectID`
ref https://linear.app/ghost/issue/PROD-1605/ ref #23924 - extracted core `limit` value capping logic into a shared util - updated `max-limit-cap` middleware to use shared util - replaced old/unused `getHelperLimitAllMax` config in `{{#get}}` helper's options parser with a call to the new limit cap util - rewrites any `limit` option used in `{{#get}}` (including `'all'`) so it matches Ghost's configured max API limit
ref https://linear.app/ghost/issue/PROD-2113 Removed the global `MIGRATION_USER` constant to prevent further usage. It's usage has been inlined, mainly in existing migrations, and replaced with a local constant, which was already a pattern being used in some migrations. We do not want to change any existing logic within the migration files as they are already in use
ref #23924 closes https://linear.app/ghost/issue/PROD-2191/breaking-change-drop-get-session-endpoint - This endpoint was originally created with an intent to return some other data - It was never needed or updated, and has sat unused - Meanwhile, we have a /users/me/ endpoint which correctly returns the logged in user data and can be used in place of this endpoint - Therefore we're deleting the endpoint for cleanliness
ref 145dc46 ref b788604 ref https://ghost.org/docs/update-major-version/. - In order to upgrade to 6.x you need to be on: a) the very latest 4.x b) any 5.x version - We clean out migrations from the previous migration as it allows us to keep the codebase a little cleaner - And means that we don't have to maintain unmanageable levels of backwards compatibility across migration utils etc - We add a "final migration" to the very last minor of 4.x with migrations (4.47) - This ensures that attempting to update to 6.0 from a too-early version will error with a clear message telling you what steps to take instead. - Note: we also have to noop the additional migration in 4.47 so that knex-migrator doesn't get confused.
…24152) ref https://linear.app/ghost/issue/PROD-1597 Added eslint rule to prevent `created_by` / `updated_by` from being added to a database schema - User actions should be logged using the action log system instead
ref #23924 closes https://linear.app/ghost/issue/PROD-2118/run-a-migration-to-remove-amp-from-the-settings-table removes amp and amp_gtag_id from settings table
ref https://linear.app/ghost/issue/PROD-2220 Removed the mail events package and the associated `temp_mail_events` table as this is no longer used
closes https://linear.app/ghost/issue/PROD-2219/breaking-change-change-notimplemented-middleware-to-throw-403 - The intention of this middleware is to provide an allowlist of API endpoints we definitely trust to allow integrations to access - It was meant to be a short term thing whilst we audited all the endpoints but it's ended up living a lot longer - Given that we're blocking access to an endpoint that does exist and IS implemented, the 501 error makes no sense - This changes the middleware to throw a 403 NoPermissionError instead so at least it makes sense - Ideally we should move this code into the API permission layer or implement it with real permissions BUT for now this change at least makes the weirdest change (not really breaking, but different) in 6.x so it's done
closes https://linear.app/ghost/issue/PROD-2203/increase-column-length-on-posts-metafeature-image-alt-and-post - increases the length of feature_image_alt columns in posts_meta and post_revisions - this increases the underlying column width as column changes like this are often long-running migrations we only run during major updates - The soft limit (validation) on the field has not yet changed we can change this outside a major, when we've got time to think about what the validation needs to be
- We have lots of tidying to do but for now we will just promote all the features, so that the labs flags all return true in 6.x - This makes it seem as if we didn't have flags at all
- The browser tests are failing. - assuming because the labs flags being set to GA caused issues with the UI - We'll fix this later, for now we want the 6.x branch to build
no ref Post 6.0 upgrade, any users who were previously logged in and had an ID of `1` would be logged out due the upgrade process changing any users with an ID of `1` to be an `ObjectID` but not updating the `user_id` field in the `session_data` field of the `sessions` table. This change ensures that this field gets updated as part of the original migration
no issue - in `home` we need to use the same conditional for the stats-x redirect as we use to redirect away from `stats-x` otherwise we can end up in an infinite loop - in `setup.done` there were two different redirects to `stats-x`, removing the old `ui60` flag conditional means we don't have unexpected redirects to `stats-x` when we don't know if it's actually accessible
ref https://linear.app/ghost/issue/PROD-2317/remove-settings-related-ui60-flag - Depending on the state of the `ui60` flag we redirected users to different pages when quitting Settings. Also we had different main navigation menus in Admin depending on the `ui60` flag. This PR removes the flag related to these behaviors and defaults to correct behavior in 6.0.
ref https://linear.app/ghost/issue/PROD-2317/remove-settings-related-ui60-flag - Removed `ui60` flag for settings sidebar navigation and search, content area and related tests. This PR doesn't remove the flag itself as it's still used in the Admin.
no ref This is removing the tests that no longer apply and are preventing us from being green on CI. We will follow up with changes that remove the flags/Dashboard routing and properly fix the state of things.
There are a few places on the 6.x branch where we redirect to a different location than before, or redirect when we didn't previously. No functional changes here, just a couple fixes to browser tests. - password reset tests: we previously checked for a redirect to `/dashboard`, but we now redirect to `/analytics`. This just updates the assertion in the browser test to `/analytics` - publishing tests: when publishing a post with email (publish + email or email only), Ghost redirects to the post analytics for the post right away. The browser tests were timing out trying to close the publishing flow, but the app had already redirected away from it. This just removes this step from the tests so they can continue to check that the post is/is not published on the site.
WalkthroughThe changes introduce support for the 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
🧰 Additional context used🧠 Learnings (5)📓 Common learningsghost/core/core/shared/config/env/config.testing.json (1)ghost/core/core/shared/config/env/config.testing-browser.json (1)ghost/core/test/e2e-api/admin/config.test.js (2)ghost/core/package.json (6)🧬 Code Graph Analysis (3)ghost/core/core/shared/config/utils.js (1)
ghost/core/core/server/data/db/connection.js (1)
ghost/core/core/server/services/url/Resources.js (2)
🔇 Additional comments (12)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
It looks like this PR contains a migration 👀 General requirements
Schema changes
Data changes
|
These bindings also rely on a forked version of knex, as well as updates to @tryghost/database-info and knex-migrator
3cbbb3f to
e5d570e
Compare
7b135d2 to
336f1e7
Compare
|
Closing this for now. |
This change allows both sqlite3 and node-sqlite bindings to work in Ghost.
It's a WIP, and a couple of tests are failing in the E2E suite. The main changes that aren't represented in the current commit are a switch to forked version of knex + other package updates that would be required.
Currently there a 3 tests failing in the e2e suite, and 3 failing in the legacy suite. All other tests pass ✅
yarn link