From 8a389c2943c457ae940b953e68d3db7d9016dd2e Mon Sep 17 00:00:00 2001 From: Daniel Bachler Date: Mon, 29 Jan 2024 15:08:08 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20switch=20Variable=20and=20User?= =?UTF-8?q?=20models=20to=20support=20knex?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also make sure we pass down knex in the related places and use transaction scopes where appropriate. --- adminSiteServer/adminRouter.tsx | 24 +++--- adminSiteServer/apiRouter.ts | 55 +++++++------- adminSiteServer/gitDataExport.ts | 2 +- adminSiteServer/mockSiteRouter.tsx | 29 ++++--- baker/DeployUtils.ts | 7 +- baker/GrapherBaker.tsx | 98 +++++++++++++++--------- baker/SiteBaker.tsx | 40 ++++++---- baker/algolia/indexToAlgolia.tsx | 20 +++-- baker/buildLocalBake.ts | 3 +- baker/countryProfiles.tsx | 10 +-- baker/formatWordpressPost.tsx | 12 ++- baker/runBakeGraphers.ts | 4 +- baker/siteRenderers.tsx | 32 ++++++-- baker/startDeployQueueServer.ts | 2 +- db/Variable.test.ts | 5 +- db/model/Dataset.ts | 44 ++++++----- db/model/User.ts | 12 +-- db/model/Variable.ts | 118 +++++++++++++++++------------ db/tests/basic.test.ts | 63 +++++++++++++-- 19 files changed, 372 insertions(+), 208 deletions(-) diff --git a/adminSiteServer/adminRouter.tsx b/adminSiteServer/adminRouter.tsx index b144559abd8..c2b5941f3fa 100644 --- a/adminSiteServer/adminRouter.tsx +++ b/adminSiteServer/adminRouter.tsx @@ -148,7 +148,7 @@ adminRouter.get("/datasets/:datasetId.csv", async (req, res) => { callback(null) }, }) - await Dataset.writeCSV(datasetId, writeStream) + await Dataset.writeCSV(datasetId, writeStream, db.knexInstance()) res.end() }) @@ -167,13 +167,13 @@ adminRouter.get("/datasets/:datasetId/downloadZip", async (req, res) => { adminRouter.get("/posts/preview/:postId", async (req, res) => { const postId = expectInt(req.params.postId) - res.send(await renderPreview(postId)) + res.send(await renderPreview(postId, db.knexInstance())) }) adminRouter.get("/posts/compare/:postId", async (req, res) => { const postId = expectInt(req.params.postId) - const wpPage = await renderPreview(postId) + const wpPage = await renderPreview(postId, db.knexInstance()) const archieMlText = await Post.select( "archieml", "archieml_update_statistics" @@ -279,13 +279,16 @@ adminRouter.get("/datapage-preview/:id", async (req, res) => { await explorerAdminServer.getAllPublishedExplorersBySlugCached() res.send( - await renderDataPageV2({ - variableId, - variableMetadata, - isPreviewing: true, - useIndicatorGrapherConfigs: true, - publishedExplorersBySlug, - }) + await renderDataPageV2( + { + variableId, + variableMetadata, + isPreviewing: true, + useIndicatorGrapherConfigs: true, + publishedExplorersBySlug, + }, + db.knexInstance() + ) ) }) @@ -300,6 +303,7 @@ adminRouter.get("/grapher/:slug", async (req, res) => { res.send( await renderPreviewDataPageOrGrapherPage( entity.config, + db.knexInstance(), publishedExplorersBySlug ) ) diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index c261eeb14a1..918560081a8 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -54,7 +54,7 @@ import { import { GrapherInterface, OwidGdocLinkType, - UsersRow, + DbPlainUser, UsersTableName, grapherKeysToSerialize, } from "@ourworldindata/types" @@ -1415,20 +1415,20 @@ apiRouter.post( ) apiRouter.get("/users.json", async (req: Request, res: Response) => ({ - users: db + users: await db .knexInstance() .select( - "id" satisfies keyof UsersRow, - "email" satisfies keyof UsersRow, - "fullName" satisfies keyof UsersRow, - "isActive" satisfies keyof UsersRow, - "isSuperuser" satisfies keyof UsersRow, - "createdAt" satisfies keyof UsersRow, - "updatedAt" satisfies keyof UsersRow, - "lastLogin" satisfies keyof UsersRow, - "lastSeen" satisfies keyof UsersRow + "id" satisfies keyof DbPlainUser, + "email" satisfies keyof DbPlainUser, + "fullName" satisfies keyof DbPlainUser, + "isActive" satisfies keyof DbPlainUser, + "isSuperuser" satisfies keyof DbPlainUser, + "createdAt" satisfies keyof DbPlainUser, + "updatedAt" satisfies keyof DbPlainUser, + "lastLogin" satisfies keyof DbPlainUser, + "lastSeen" satisfies keyof DbPlainUser ) - .from(UsersTableName) + .from(UsersTableName) .orderBy("lastSeen", "desc"), })) @@ -1455,23 +1455,18 @@ apiRouter.put("/users/:userId", async (req: Request, res: Response) => { if (!res.locals.user.isSuperuser) throw new JsonError("Permission denied", 403) - const userId = parseIntOrUndefined(req.params.userId) - const user = - userId !== undefined - ? await getUserById(db.knexInstance(), userId) - : null - if (!user) throw new JsonError("No such user", 404) + return db.knexInstance().transaction(async (t) => { + const userId = parseIntOrUndefined(req.params.userId) + const user = userId !== undefined ? await getUserById(t, userId) : null + if (!user) throw new JsonError("No such user", 404) - user.fullName = req.body.fullName - user.isActive = req.body.isActive + user.fullName = req.body.fullName + user.isActive = req.body.isActive - await updateUser( - db.knexInstance(), - userId!, - pick(user, ["fullName", "isActive"]) - ) + await updateUser(t, userId!, pick(user, ["fullName", "isActive"])) - return { success: true } + return { success: true } + }) }) apiRouter.post("/users/add", async (req: Request, res: Response) => { @@ -1491,7 +1486,7 @@ apiRouter.post("/users/add", async (req: Request, res: Response) => { apiRouter.get("/variables.json", async (req) => { const limit = parseIntOrUndefined(req.query.limit as string) ?? 50 const query = req.query.search as string - return await searchVariables(query, limit) + return await searchVariables(query, limit, db.knexInstance()) }) apiRouter.get( @@ -1709,8 +1704,10 @@ apiRouter.get( await Chart.assignTagsForCharts(charts) - const grapherConfig = - await getMergedGrapherConfigForVariable(variableId) + const grapherConfig = await getMergedGrapherConfigForVariable( + variableId, + db.knexInstance() + ) if ( grapherConfig && (!grapherConfig.dimensions || grapherConfig.dimensions.length === 0) diff --git a/adminSiteServer/gitDataExport.ts b/adminSiteServer/gitDataExport.ts index 1d4a8a2f687..fb906f672b5 100644 --- a/adminSiteServer/gitDataExport.ts +++ b/adminSiteServer/gitDataExport.ts @@ -104,7 +104,7 @@ export async function syncDatasetToGitRepo( await Promise.all([ fs.writeFile( path.join(tmpDatasetDir, `${dataset.filename}.csv`), - await dataset.toCSV() + await dataset.toCSV(db.knexInstance()) ), fs.writeFile( path.join(tmpDatasetDir, `datapackage.json`), diff --git a/adminSiteServer/mockSiteRouter.tsx b/adminSiteServer/mockSiteRouter.tsx index 7dd809be0d6..326bc7fcd78 100644 --- a/adminSiteServer/mockSiteRouter.tsx +++ b/adminSiteServer/mockSiteRouter.tsx @@ -53,6 +53,7 @@ import { } from "../baker/GrapherBaker.js" import { GdocPost } from "../db/model/Gdoc/GdocPost.js" import { GdocDataInsight } from "../db/model/Gdoc/GdocDataInsight.js" +import * as db from "../db/db.js" require("express-async-errors") @@ -160,6 +161,7 @@ mockSiteRouter.get("/grapher/:slug", async (req, res) => { res.send( await renderPreviewDataPageOrGrapherPage( entity.config, + db.knexInstance(), publishedExplorersBySlug ) ) @@ -227,19 +229,28 @@ mockSiteRouter.get("/datapage-preview/:id", async (req, res) => { await explorerAdminServer.getAllPublishedExplorersBySlugCached() res.send( - await renderDataPageV2({ - variableId, - variableMetadata, - isPreviewing: true, - useIndicatorGrapherConfigs: true, - publishedExplorersBySlug, - }) + await renderDataPageV2( + { + variableId, + variableMetadata, + isPreviewing: true, + useIndicatorGrapherConfigs: true, + publishedExplorersBySlug, + }, + db.knexInstance() + ) ) }) countryProfileSpecs.forEach((spec) => mockSiteRouter.get(`/${spec.rootPath}/:countrySlug`, async (req, res) => - res.send(await countryProfileCountryPage(spec, req.params.countrySlug)) + res.send( + await countryProfileCountryPage( + spec, + req.params.countrySlug, + db.knexInstance() + ) + ) ) ) @@ -345,7 +356,7 @@ mockSiteRouter.get("/*", async (req, res) => { } try { - res.send(await renderPageBySlug(slug)) + res.send(await renderPageBySlug(slug, db.knexInstance())) } catch (e) { console.error(e) res.status(404).send(await renderNotFoundPage()) diff --git a/baker/DeployUtils.ts b/baker/DeployUtils.ts index b3203874ee8..88b2ea794bb 100644 --- a/baker/DeployUtils.ts +++ b/baker/DeployUtils.ts @@ -11,6 +11,7 @@ import { import { SiteBaker } from "../baker/SiteBaker.js" import { WebClient } from "@slack/web-api" import { DeployChange, DeployMetadata } from "@ourworldindata/utils" +import { Knex } from "knex" const deployQueueServer = new DeployQueueServer() @@ -34,6 +35,7 @@ export const defaultCommitMessage = async (): Promise => { */ const triggerBakeAndDeploy = async ( deployMetadata: DeployMetadata, + knex: Knex, lightningQueue?: DeployChange[] ) => { // deploy to Buildkite if we're on master and BUILDKITE_API_ACCESS_TOKEN is set @@ -60,7 +62,7 @@ const triggerBakeAndDeploy = async ( await baker.bakeGDocPosts(lightningQueue.map((c) => c.slug!)) } else { - await baker.bakeAll() + await baker.bakeAll(knex) } } } @@ -151,7 +153,7 @@ let deploying = false * the end of the current one, as long as there are changes in the queue. * If there are no changes in the queue, a deploy won't be initiated. */ -export const deployIfQueueIsNotEmpty = async () => { +export const deployIfQueueIsNotEmpty = async (knex: Knex) => { if (deploying) return deploying = true let failures = 0 @@ -184,6 +186,7 @@ export const deployIfQueueIsNotEmpty = async () => { await getChangesSlackMentions(parsedQueue) await triggerBakeAndDeploy( { title: changesAuthorNames[0], changesSlackMentions }, + knex, // If every DeployChange is a lightning change, then we can do a // lightning deploy. In the future, we might want to separate // lightning updates from regular deploys so we could prioritize diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index 37d52ff0ecf..f1de4417a69 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -63,10 +63,13 @@ import { GdocPost } from "../db/model/Gdoc/GdocPost.js" import { getShortPageCitation } from "../site/gdocs/utils.js" import { isEmpty } from "lodash" import { getSlugForTopicTag, getTagToSlugMap } from "./GrapherBakingUtils.js" +import { Knex } from "knex" +import { knexRaw } from "../db/db.js" const renderDatapageIfApplicable = async ( grapher: GrapherInterface, isPreviewing: boolean, + knex: Knex, publishedExplorersBySlug?: Record, imageMetadataDictionary?: Record ) => { @@ -102,15 +105,18 @@ const renderDatapageIfApplicable = async ( !isEmpty(variableMetadata.descriptionFromProducer) || !isEmpty(variableMetadata.presentation?.titlePublic)) ) { - return await renderDataPageV2({ - variableId, - variableMetadata, - isPreviewing: isPreviewing, - useIndicatorGrapherConfigs: false, - pageGrapher: grapher, - publishedExplorersBySlug, - imageMetadataDictionary, - }) + return await renderDataPageV2( + { + variableId, + variableMetadata, + isPreviewing: isPreviewing, + useIndicatorGrapherConfigs: false, + pageGrapher: grapher, + publishedExplorersBySlug, + imageMetadataDictionary, + }, + knex + ) } } return undefined @@ -122,12 +128,14 @@ const renderDatapageIfApplicable = async ( */ export const renderDataPageOrGrapherPage = async ( grapher: GrapherInterface, + knex: Knex, publishedExplorersBySlug?: Record, imageMetadataDictionary?: Record ): Promise => { const datapage = await renderDatapageIfApplicable( grapher, false, + knex, publishedExplorersBySlug, imageMetadataDictionary ) @@ -147,25 +155,30 @@ type EnrichedFaqLookupSuccess = { type EnrichedFaqLookupResult = EnrichedFaqLookupError | EnrichedFaqLookupSuccess -export async function renderDataPageV2({ - variableId, - variableMetadata, - isPreviewing, - useIndicatorGrapherConfigs, - pageGrapher, - publishedExplorersBySlug, - imageMetadataDictionary = {}, -}: { - variableId: number - variableMetadata: OwidVariableWithSource - isPreviewing: boolean - useIndicatorGrapherConfigs: boolean - pageGrapher?: GrapherInterface - publishedExplorersBySlug?: Record - imageMetadataDictionary?: Record -}) { - const grapherConfigForVariable = - await getMergedGrapherConfigForVariable(variableId) +export async function renderDataPageV2( + { + variableId, + variableMetadata, + isPreviewing, + useIndicatorGrapherConfigs, + pageGrapher, + publishedExplorersBySlug, + imageMetadataDictionary = {}, + }: { + variableId: number + variableMetadata: OwidVariableWithSource + isPreviewing: boolean + useIndicatorGrapherConfigs: boolean + pageGrapher?: GrapherInterface + publishedExplorersBySlug?: Record + imageMetadataDictionary?: Record + }, + knex: Knex +) { + const grapherConfigForVariable = await getMergedGrapherConfigForVariable( + variableId, + knex + ) // Only merge the grapher config on the indicator if the caller tells us to do so - // this is true for preview pages for datapages on the indicator level but false // if we are on Grapher pages. Once we have a good way in the grapher admin for how @@ -345,11 +358,13 @@ export async function renderDataPageV2({ */ export const renderPreviewDataPageOrGrapherPage = async ( grapher: GrapherInterface, + knex: Knex, publishedExplorersBySlug?: Record ) => { const datapage = await renderDatapageIfApplicable( grapher, true, + knex, publishedExplorersBySlug ) if (datapage) return datapage @@ -398,7 +413,8 @@ const chartIsSameVersion = async ( const bakeGrapherPageAndVariablesPngAndSVGIfChanged = async ( bakedSiteDir: string, imageMetadataDictionary: Record, - grapher: GrapherInterface + grapher: GrapherInterface, + knex: Knex ) => { const htmlPath = `${bakedSiteDir}/grapher/${grapher.slug}.html` const isSameVersion = await chartIsSameVersion(htmlPath, grapher.version) @@ -415,7 +431,12 @@ const bakeGrapherPageAndVariablesPngAndSVGIfChanged = async ( const outPath = `${bakedSiteDir}/grapher/${grapher.slug}.html` await fs.writeFile( outPath, - await renderDataPageOrGrapherPage(grapher, {}, imageMetadataDictionary) + await renderDataPageOrGrapherPage( + grapher, + knex, + {}, + imageMetadataDictionary + ) ) console.log(outPath) @@ -477,7 +498,8 @@ export interface BakeSingleGrapherChartArguments { } export const bakeSingleGrapherChart = async ( - args: BakeSingleGrapherChartArguments + args: BakeSingleGrapherChartArguments, + knex: Knex ) => { const grapher: GrapherInterface = JSON.parse(args.config) grapher.id = args.id @@ -492,20 +514,24 @@ export const bakeSingleGrapherChart = async ( await bakeGrapherPageAndVariablesPngAndSVGIfChanged( args.bakedSiteDir, args.imageMetadataDictionary, - grapher + grapher, + knex ) return args } export const bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers = - async (bakedSiteDir: string) => { + async (bakedSiteDir: string, knex: Knex) => { const chartsToBake: { id: number; config: string; slug: string }[] = - await db.queryMysql(` + await knexRaw( + ` SELECT id, config, config->>'$.slug' as slug FROM charts WHERE JSON_EXTRACT(config, "$.isPublished")=true ORDER BY JSON_EXTRACT(config, "$.slug") ASC - `) + `, + knex + ) const newSlugs = chartsToBake.map((row) => row.slug) await fs.mkdirp(bakedSiteDir + "/grapher") @@ -538,7 +564,7 @@ export const bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers = if (MAX_NUM_BAKE_PROCESSES === 1) { await Promise.all( jobs.map(async (job) => { - await bakeSingleGrapherChart(job) + await bakeSingleGrapherChart(job, knex) progressBar.tick({ name: `slug ${job.slug}` }) }) ) diff --git a/baker/SiteBaker.tsx b/baker/SiteBaker.tsx index 60c6a3f5086..994937b9d5f 100644 --- a/baker/SiteBaker.tsx +++ b/baker/SiteBaker.tsx @@ -84,6 +84,7 @@ import { import pMap from "p-map" import { GdocDataInsight } from "../db/model/Gdoc/GdocDataInsight.js" import { fullGdocToMinimalGdoc } from "../db/model/Gdoc/gdocUtils.js" +import { Knex } from "knex" type PrefetchedAttachments = { linkedDocuments: Record @@ -194,7 +195,7 @@ export class SiteBaker { this.progressBar.tick({ name: "✅ baked embeds" }) } - private async bakeCountryProfiles() { + private async bakeCountryProfiles(knex: Knex) { if (!this.bakeSteps.has("countryProfiles")) return await Promise.all( countryProfileSpecs.map(async (spec) => { @@ -207,6 +208,7 @@ export class SiteBaker { const html = await renderCountryProfile( spec, country, + knex, this.grapherExports ).catch(() => console.error( @@ -240,8 +242,13 @@ export class SiteBaker { } // Bake an individual post/page - private async bakePost(post: FullPost) { - const html = await renderPost(post, this.baseUrl, this.grapherExports) + private async bakePost(post: FullPost, knex: Knex) { + const html = await renderPost( + post, + knex, + this.baseUrl, + this.grapherExports + ) const outPath = path.join(this.bakedSiteDir, `${post.slug}.html`) await fs.mkdirp(path.dirname(outPath)) @@ -409,7 +416,7 @@ export class SiteBaker { this.progressBar.tick({ name: "✅ removed deleted posts" }) } - private async bakePosts() { + private async bakePosts(knex: Knex) { if (!this.bakeSteps.has("wordpressPosts")) return // TODO: the knex instance should be handed down as a parameter const alreadyPublishedViaGdocsSlugsSet = @@ -423,7 +430,9 @@ export class SiteBaker { await pMap( postsApi, async (postApi) => - wpdb.getFullPost(postApi).then((post) => this.bakePost(post)), + wpdb + .getFullPost(postApi) + .then((post) => this.bakePost(post, knex)), { concurrency: 10 } ) @@ -824,26 +833,27 @@ export class SiteBaker { this.progressBar.tick({ name: "✅ baked redirects" }) } - async bakeWordpressPages() { + async bakeWordpressPages(knex: Knex) { await this.bakeRedirects() await this.bakeEmbeds() await this.bakeBlogIndex() await this.bakeRSS() await this.bakeAssets() await this.bakeGoogleScholar() - await this.bakePosts() + await this.bakePosts(knex) } - private async _bakeNonWordpressPages() { + private async _bakeNonWordpressPages(knex: Knex) { if (this.bakeSteps.has("countries")) { await bakeCountries(this) } await this.bakeSpecialPages() - await this.bakeCountryProfiles() + await this.bakeCountryProfiles(knex) await this.bakeExplorers() if (this.bakeSteps.has("charts")) { await bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers( - this.bakedSiteDir + this.bakedSiteDir, + knex ) this.progressBar.tick({ name: "✅ bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers", @@ -856,7 +866,7 @@ export class SiteBaker { await this.bakeDriveImages() } - async bakeNonWordpressPages() { + async bakeNonWordpressPages(knex: Knex) { await db.getConnection() const progressBarTotal = nonWordpressSteps .map((step) => this.bakeSteps.has(step)) @@ -867,15 +877,15 @@ export class SiteBaker { total: progressBarTotal, } ) - await this._bakeNonWordpressPages() + await this._bakeNonWordpressPages(knex) } - async bakeAll() { + async bakeAll(knex: Knex) { // Ensure caches are correctly initialized this.flushCache() await this.removeDeletedPosts() - await this.bakeWordpressPages() - await this._bakeNonWordpressPages() + await this.bakeWordpressPages(knex) + await this._bakeNonWordpressPages(knex) this.flushCache() } diff --git a/baker/algolia/indexToAlgolia.tsx b/baker/algolia/indexToAlgolia.tsx index 402490c36e0..7c3eaf97103 100644 --- a/baker/algolia/indexToAlgolia.tsx +++ b/baker/algolia/indexToAlgolia.tsx @@ -26,6 +26,7 @@ import { Pageview } from "../../db/model/Pageview.js" import { GdocPost } from "../../db/model/Gdoc/GdocPost.js" import { ArticleBlocks } from "../../site/gdocs/components/ArticleBlocks.js" import React from "react" +import { Knex } from "knex" interface TypeAndImportance { type: PageType @@ -73,7 +74,8 @@ function generateChunksFromHtmlText(htmlString: string) { async function generateWordpressRecords( postsApi: PostRestApi[], - pageviews: Record + pageviews: Record, + knex: Knex ): Promise { const getPostTypeAndImportance = ( post: FormattedPost, @@ -101,7 +103,7 @@ async function generateWordpressRecords( continue } - const post = await formatPost(rawPost, { footnotes: false }) + const post = await formatPost(rawPost, { footnotes: false }, knex) const chunks = generateChunksFromHtmlText(post.html) const tags = await wpdb.getPostTags(post.id) const postTypeAndImportance = getPostTypeAndImportance(post, tags) @@ -186,7 +188,7 @@ function generateGdocRecords( } // Generate records for countries, WP posts (not including posts that have been succeeded by Gdocs equivalents), and Gdocs -const getPagesRecords = async () => { +const getPagesRecords = async (knex: Knex) => { const pageviews = await Pageview.getViewsByUrlObj() const gdocs = await GdocPost.getPublishedGdocs() const publishedGdocsBySlug = keyBy(gdocs, "slug") @@ -205,13 +207,17 @@ const getPagesRecords = async () => { }) const countryRecords = generateCountryRecords(countries, pageviews) - const wordpressRecords = await generateWordpressRecords(postsApi, pageviews) + const wordpressRecords = await generateWordpressRecords( + postsApi, + pageviews, + knex + ) const gdocsRecords = generateGdocRecords(gdocs, pageviews) return [...countryRecords, ...wordpressRecords, ...gdocsRecords] } -const indexToAlgolia = async () => { +const indexToAlgolia = async (knex: Knex) => { if (!ALGOLIA_INDEXING) return const client = getAlgoliaClient() @@ -222,7 +228,7 @@ const indexToAlgolia = async () => { const index = client.initIndex(SearchIndexName.Pages) await db.getConnection() - const records = await getPagesRecords() + const records = await getPagesRecords(knex) index.replaceAllObjects(records) @@ -235,4 +241,4 @@ process.on("unhandledRejection", (e) => { process.exit(1) }) -indexToAlgolia() +indexToAlgolia(db.knexInstance()) diff --git a/baker/buildLocalBake.ts b/baker/buildLocalBake.ts index 930f46a0750..1d7ed914854 100644 --- a/baker/buildLocalBake.ts +++ b/baker/buildLocalBake.ts @@ -5,6 +5,7 @@ import { hideBin } from "yargs/helpers" import { BakeStep, BakeStepConfig, bakeSteps, SiteBaker } from "./SiteBaker.js" import fs from "fs-extra" import { normalize } from "path" +import * as db from "../db/db.js" const bakeDomainToFolder = async ( baseUrl = "http://localhost:3000/", @@ -15,7 +16,7 @@ const bakeDomainToFolder = async ( fs.mkdirp(dir) const baker = new SiteBaker(dir, baseUrl, bakeSteps) console.log(`Baking site locally with baseUrl '${baseUrl}' to dir '${dir}'`) - await baker.bakeAll() + await baker.bakeAll(db.knexInstance()) } yargs(hideBin(process.argv)) diff --git a/baker/countryProfiles.tsx b/baker/countryProfiles.tsx index b5bad505cc6..4f77261e142 100644 --- a/baker/countryProfiles.tsx +++ b/baker/countryProfiles.tsx @@ -3,8 +3,8 @@ import * as db from "../db/db.js" import { CountriesIndexPage } from "../site/CountriesIndexPage.js" import { GrapherInterface, - VariablesRowEnriched, - VariablesRowRaw, + DbEnrichedVariable, + DbRawVariable, VariablesTableName, parseVariablesRow, } from "@ourworldindata/types" @@ -53,13 +53,13 @@ const countryIndicatorGraphers = async (): Promise => }) export const countryIndicatorVariables = async (): Promise< - VariablesRowEnriched[] + DbEnrichedVariable[] > => bakeCache(countryIndicatorVariables, async () => { const variableIds = (await countryIndicatorGraphers()).map( (c) => c.dimensions![0]!.variableId ) - const rows: VariablesRowRaw[] = await db + const rows: DbRawVariable[] = await db .knexTable(VariablesTableName) .whereIn("id", variableIds) return rows.map(parseVariablesRow) @@ -96,7 +96,7 @@ export const denormalizeLatestCountryData = async (variableIds?: number[]) => { const currentYear = new Date().getUTCFullYear() - const df = (await dataAsDF(variableIds)) + const df = (await dataAsDF(variableIds, db.knexInstance())) .filter( pl .col("entityId") diff --git a/baker/formatWordpressPost.tsx b/baker/formatWordpressPost.tsx index eeebf32b6e0..002d4578fe0 100644 --- a/baker/formatWordpressPost.tsx +++ b/baker/formatWordpressPost.tsx @@ -61,6 +61,7 @@ import { renderKeyInsights, renderProminentLinks } from "./siteRenderers.js" import { KEY_INSIGHTS_CLASS_NAME } from "../site/blocks/KeyInsights.js" import { RELATED_CHARTS_CLASS_NAME } from "../site/blocks/RelatedCharts.js" import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js" +import { Knex } from "knex" const initMathJax = () => { const adaptor = liteAdaptor() @@ -130,6 +131,7 @@ const formatLatex = async ( export const formatWordpressPost = async ( post: FullPost, formattingOptions: FormattingOptions, + knex: Knex, grapherExports?: GrapherExports ): Promise => { let html = post.content @@ -183,18 +185,19 @@ export const formatWordpressPost = async ( const { queryArgs, template } = dataValueConfiguration const { variableId, chartId } = queryArgs const { value, year, unit, entityName } = - (await getDataValue(queryArgs)) || {} + (await getDataValue(queryArgs, knex)) || {} if (!value || !year || !entityName || !template) continue let formattedValue if (variableId && chartId) { const legacyVariableDisplayConfig = - await getOwidVariableDisplayConfig(variableId) + await getOwidVariableDisplayConfig(variableId, knex) const legacyChartDimension = await getOwidChartDimensionConfigForVariable( variableId, - chartId + chartId, + knex ) formattedValue = formatDataValue( value, @@ -606,6 +609,7 @@ export const formatWordpressPost = async ( export const formatPost = async ( post: FullPost, formattingOptions: FormattingOptions, + knex: Knex, grapherExports?: GrapherExports ): Promise => { // No formatting applied, plain source HTML returned @@ -625,5 +629,5 @@ export const formatPost = async ( ...formattingOptions, } - return formatWordpressPost(post, options, grapherExports) + return formatWordpressPost(post, options, knex, grapherExports) } diff --git a/baker/runBakeGraphers.ts b/baker/runBakeGraphers.ts index 4136be0ba16..d034bea8682 100755 --- a/baker/runBakeGraphers.ts +++ b/baker/runBakeGraphers.ts @@ -1,5 +1,6 @@ #! /usr/bin/env node import { bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers } from "./GrapherBaker.js" +import * as db from "../db/db.js" /** * This bakes all the Graphers to a folder on your computer, running the same baking code as the SiteBaker. @@ -9,7 +10,8 @@ import { bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers } fro const main = async (folder: string) => { await bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers( - folder + folder, + db.knexInstance() ) } diff --git a/baker/siteRenderers.tsx b/baker/siteRenderers.tsx index e805c32d833..dd022517a92 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -92,6 +92,7 @@ import { GdocPost } from "../db/model/Gdoc/GdocPost.js" import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js" import { GdocFactory } from "../db/model/Gdoc/GdocFactory.js" import { SiteNavigationStatic } from "../site/SiteNavigation.js" +import { Knex } from "knex" export const renderToHtmlPage = (element: any) => `${ReactDOMServer.renderToStaticMarkup(element)}` @@ -193,14 +194,20 @@ export const renderGdoc = (gdoc: OwidGdoc, isPreviewing: boolean = false) => { ) } -export const renderPageBySlug = async (slug: string) => { +export const renderPageBySlug = async ( + slug: string, + knex: Knex +) => { const post = await getPostBySlug(slug) - return renderPost(post) + return renderPost(post, knex) } -export const renderPreview = async (postId: number): Promise => { +export const renderPreview = async ( + postId: number, + knex: Knex +): Promise => { const postApi = await getLatestPostRevision(postId) - return renderPost(postApi) + return renderPost(postApi, knex) } export const renderMenuJson = async () => { @@ -209,6 +216,7 @@ export const renderMenuJson = async () => { export const renderPost = async ( post: FullPost, + knex: Knex, baseUrl: string = BAKED_BASE_URL, grapherExports?: GrapherExports ) => { @@ -229,7 +237,12 @@ export const renderPost = async ( // Extract formatting options from post HTML comment (if any) const formattingOptions = extractFormattingOptions(post.content) - const formatted = await formatPost(post, formattingOptions, grapherExports) + const formatted = await formatPost( + post, + formattingOptions, + knex, + grapherExports + ) const pageOverrides = await getPageOverrides(post, formattingOptions) const citationStatus = @@ -482,6 +495,7 @@ export const feedbackPage = () => const getCountryProfilePost = memoize( async ( profileSpec: CountryProfileSpec, + knex: Knex, grapherExports?: GrapherExports ): Promise<[FormattedPost, FormattingOptions]> => { // Get formatted content from generic covid country profile page. @@ -495,6 +509,7 @@ const getCountryProfilePost = memoize( const formattedPost = await formatPost( genericCountryProfilePost, profileFormattingOptions, + knex, grapherExports ) @@ -512,10 +527,12 @@ const getCountryProfileLandingPost = memoize( export const renderCountryProfile = async ( profileSpec: CountryProfileSpec, country: Country, + knex: Knex, grapherExports?: GrapherExports ) => { const [formatted, formattingOptions] = await getCountryProfilePost( profileSpec, + knex, grapherExports ) @@ -546,13 +563,14 @@ export const renderCountryProfile = async ( export const countryProfileCountryPage = async ( profileSpec: CountryProfileSpec, - countrySlug: string + countrySlug: string, + knex: Knex ) => { const country = getCountryBySlug(countrySlug) if (!country) throw new JsonError(`No such country ${countrySlug}`, 404) // Voluntarily not dealing with grapherExports on devServer for simplicity - return renderCountryProfile(profileSpec, country) + return renderCountryProfile(profileSpec, country, knex) } export const flushCache = () => getCountryProfilePost.cache.clear?.() diff --git a/baker/startDeployQueueServer.ts b/baker/startDeployQueueServer.ts index 0f7ae181a46..2e82864cbe5 100644 --- a/baker/startDeployQueueServer.ts +++ b/baker/startDeployQueueServer.ts @@ -35,7 +35,7 @@ const main = async () => { setTimeout(deployIfQueueIsNotEmpty, 10 * 1000) }) - deployIfQueueIsNotEmpty() + deployIfQueueIsNotEmpty(db.knexInstance()) } main() diff --git a/db/Variable.test.ts b/db/Variable.test.ts index 537040f4738..30cf95002f9 100644 --- a/db/Variable.test.ts +++ b/db/Variable.test.ts @@ -5,6 +5,7 @@ import * as Variable from "./model/Variable.js" import pl from "nodejs-polars" import { Writable } from "stream" import { OwidVariableId } from "@ourworldindata/utils" +import * as db from "./db.js" import { jest } from "@jest/globals" @@ -45,7 +46,7 @@ describe("writeVariableCSV", () => { callback(null) }, }) - await writeVariableCSV(variableIds, writeStream) + await writeVariableCSV(variableIds, writeStream, db.knexInstance()) return out } @@ -164,7 +165,7 @@ describe("_dataAsDFfromS3", () => { }, } mockS3data(s3data) - const df = await _dataAsDFfromS3([1]) + const df = await _dataAsDFfromS3([1], db.knexInstance()) expect(df.toObject()).toEqual({ entityCode: ["code", "code"], entityId: [1, 1], diff --git a/db/model/Dataset.ts b/db/model/Dataset.ts index ef943d23eb1..fb243ea1a6c 100644 --- a/db/model/Dataset.ts +++ b/db/model/Dataset.ts @@ -14,14 +14,16 @@ import { Source } from "./Source.js" import * as db from "../db.js" import { - TagsRow, - VariablesRowRaw, + DbPlainTag, + DbRawVariable, VariablesTableName, slugify, } from "@ourworldindata/utils" import filenamify from "filenamify" import { writeVariableCSV } from "./Variable.js" import _ from "lodash" +import { Knex } from "knex" +import { knexRaw } from "../db.js" @Entity("datasets") @Unique(["name", "namespace"]) @@ -43,20 +45,25 @@ export class Dataset extends BaseEntity { createdByUser!: Relation // Export dataset variables to CSV (not including metadata) - static async writeCSV(datasetId: number, stream: Writable): Promise { + static async writeCSV( + datasetId: number, + stream: Writable, + knex: Knex + ): Promise { // get variables of a dataset const variableIds = ( - await db.queryMysql( + await knexRaw( ` SELECT id as variableId FROM variables v WHERE datasetId=?`, + knex, [datasetId] ) ).map((row: any) => row.variableId) - await writeVariableCSV(variableIds, stream) + await writeVariableCSV(variableIds, stream, knex) } static async setTags(datasetId: number, tagIds: number[]): Promise { @@ -73,12 +80,16 @@ export class Dataset extends BaseEntity { }) } - async toCSV(): Promise { + async toCSV(knex: Knex): Promise { let csv = "" - await Dataset.writeCSV(this.id, { - write: (s: string) => (csv += s), - end: () => null, - } as any) + await Dataset.writeCSV( + this.id, + { + write: (s: string) => (csv += s), + end: () => null, + } as any, + knex + ) return csv } @@ -96,13 +107,12 @@ export class Dataset extends BaseEntity { const sources = await Source.findBy({ datasetId: this.id }) const variables = (await db .knexTable(VariablesTableName) - .where({ datasetId: this.id })) as VariablesRowRaw[] - const tags: Pick[] = await db - .knexInstance() - .raw( - `SELECT t.id, t.name FROM dataset_tags dt JOIN tags t ON t.id=dt.tagId WHERE dt.datasetId=?`, - [this.id] - ) + .where({ datasetId: this.id })) as DbRawVariable[] + const tags: Pick[] = await knexRaw( + `SELECT t.id, t.name FROM dataset_tags dt JOIN tags t ON t.id=dt.tagId WHERE dt.datasetId=?`, + db.knexInstance(), // TODO: pass down the knex instance instead of using the global one + [this.id] + ) const initialFields = [ { name: "Entity", type: "string" }, diff --git a/db/model/User.ts b/db/model/User.ts index f9b0d1c416d..4964b8198e4 100644 --- a/db/model/User.ts +++ b/db/model/User.ts @@ -11,8 +11,8 @@ import { Dataset } from "./Dataset.js" import { ChartRevision } from "./ChartRevision.js" import { BCryptHasher } from "../hashers.js" import { - UsersRow, - UsersRowForInsert, + DbPlainUser, + DbInsertUser, UsersTableName, } from "@ourworldindata/types" import { Knex } from "knex" @@ -56,13 +56,13 @@ export async function setPassword( export async function getUserById( knex: Knex, id: number -): Promise { - return knex(UsersTableName).where({ id }).first() +): Promise { + return knex(UsersTableName).where({ id }).first() } export async function insertUser( knex: Knex, - user: UsersRowForInsert + user: DbInsertUser ): Promise<{ id: number }> { return knex(UsersTableName).returning("id").insert(user) } @@ -70,7 +70,7 @@ export async function insertUser( export async function updateUser( knex: Knex, id: number, - user: Partial + user: Partial ): Promise { return knex(UsersTableName).where({ id }).update(user) } diff --git a/db/model/Variable.ts b/db/model/Variable.ts index 36e5705c7cd..c266b58e5c4 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -13,7 +13,7 @@ import { OwidVariableId, retryPromise, GrapherInterface, - VariablesRowRaw, + DbRawVariable, VariablesTableName, ChartsTableName, } from "@ourworldindata/utils" @@ -24,19 +24,21 @@ import { import pl from "nodejs-polars" import { DATA_API_URL } from "../../settings/serverSettings.js" import { escape } from "mysql" +import { Knex } from "knex/types" +import { knexRaw, knexRawFirst } from "../db.js" export async function getMergedGrapherConfigForVariable( - variableId: number + variableId: number, + knex: Knex ): Promise { const rows: Pick< - VariablesRowRaw, + DbRawVariable, "grapherConfigAdmin" | "grapherConfigETL" - >[] = await db - .knexInstance() - .raw( - `SELECT grapherConfigAdmin, grapherConfigETL FROM variables WHERE id = ?`, - [variableId] - ) + >[] = await knexRaw( + `SELECT grapherConfigAdmin, grapherConfigETL FROM variables WHERE id = ?`, + knex, + [variableId] + ) if (!rows.length) return const row = rows[0] const grapherConfigAdmin = row.grapherConfigAdmin @@ -89,11 +91,12 @@ export async function getDataForMultipleVariables( export async function writeVariableCSV( variableIds: number[], - stream: Writable + stream: Writable, + knex: Knex ): Promise { // get variables as dataframe const variablesDF = ( - await db.knexInstance().raw( + await knex.raw( `-- sql SELECT id as variableId, @@ -114,7 +117,8 @@ export async function writeVariableCSV( // get data values as dataframe const dataValuesDF = await dataAsDF( - variablesDF.getColumn("variableId").toArray() + variablesDF.getColumn("variableId").toArray(), + knex ) dataValuesDF @@ -130,26 +134,26 @@ export async function writeVariableCSV( .writeCSV(stream) } -export const getDataValue = async ({ - variableId, - entityId, - year, -}: DataValueQueryArgs): Promise => { +export const getDataValue = async ( + { variableId, entityId, year }: DataValueQueryArgs, + knex: Knex +): Promise => { if (!variableId || !entityId) return - let df = (await dataAsDF([variableId])).filter( + let df = (await dataAsDF([variableId], knex)).filter( pl.col("entityId").eq(entityId) ) const unit = ( - await db.knexInstance().raw( + await knexRawFirst>( `-- sql SELECT unit FROM ?? WHERE id = ? `, + knex, [VariablesTableName, variableId] ) - ).unit + )?.unit if (year) { df = df.filter(pl.col("year").eq(year)) @@ -176,17 +180,19 @@ export const getDataValue = async ({ export const getOwidChartDimensionConfigForVariable = async ( variableId: OwidVariableId, - chartId: number + chartId: number, + knex: Knex ): Promise => { - const row = await db.knexInstance().raw( + const row = await db.knexRawFirst<{ dimensions: string }>( ` SELECT config->"$.dimensions" AS dimensions FROM ?? WHERE id = ? `, + knex, [ChartsTableName, chartId] ) - if (!row.dimensions) return + if (!row?.dimensions) return const dimensions = JSON.parse(row.dimensions) return dimensions.find( (dimension: OwidChartDimensionInterface) => @@ -195,18 +201,21 @@ export const getOwidChartDimensionConfigForVariable = async ( } export const getOwidVariableDisplayConfig = async ( - variableId: OwidVariableId + variableId: OwidVariableId, + knex: Knex ): Promise => { - const row = await db.mysqlFirst( + const row = await knexRawFirst>( `SELECT display FROM variables WHERE id = ?`, + knex, [variableId] ) - if (!row.display) return + if (!row?.display) return return JSON.parse(row.display) } export const entitiesAsDF = async ( - entityIds: number[] + entityIds: number[], + knex: Knex ): Promise => { return ( await readSQLasDF( @@ -217,7 +226,8 @@ export const entitiesAsDF = async ( code AS entityCode FROM entities WHERE id in (?) `, - [_.uniq(entityIds)] + [_.uniq(entityIds)], + knex ) ).select( pl.col("entityId").cast(pl.Int32), @@ -251,7 +261,8 @@ const emptyDataDF = (): pl.DataFrame => { } export const _dataAsDFfromS3 = async ( - variableIds: OwidVariableId[] + variableIds: OwidVariableId[], + knex: Knex ): Promise => { if (variableIds.length === 0) { return emptyDataDF() @@ -288,15 +299,19 @@ export const _dataAsDFfromS3 = async ( return emptyDataDF() } - const entityDF = await entitiesAsDF(df.getColumn("entityId").toArray()) + const entityDF = await entitiesAsDF( + df.getColumn("entityId").toArray(), + knex + ) return _castDataDF(df.join(entityDF, { on: "entityId" })) } export const dataAsDF = async ( - variableIds: OwidVariableId[] + variableIds: OwidVariableId[], + knex: Knex ): Promise => { - return _dataAsDFfromS3(variableIds) + return _dataAsDFfromS3(variableIds, knex) } export const fetchS3Values = async ( @@ -365,9 +380,10 @@ export const createDataFrame = (data: unknown): pl.DataFrame => { export const readSQLasDF = async ( sql: string, - params: any[] + params: any[], + knex: Knex ): Promise => { - return createDataFrame(await db.queryMysql(sql, params)) + return createDataFrame(await knexRaw(sql, knex, params)) } /** @@ -375,7 +391,8 @@ export const readSQLasDF = async ( */ export const searchVariables = async ( query: string, - limit: number + limit: number, + knex: Knex ): Promise => { const whereClauses = buildWhereClauses(query) @@ -405,9 +422,9 @@ export const searchVariables = async ( ORDER BY d.dataEditedAt DESC LIMIT ${escape(limit)} ` - const rows = await queryRegexSafe(sqlResults) + const rows = await queryRegexSafe(sqlResults, knex) - const numTotalRows = await queryRegexCount(sqlCount) + const numTotalRows = await queryRegexCount(sqlCount, knex) rows.forEach((row: any) => { if (row.catalogPath) { @@ -527,21 +544,24 @@ const buildWhereClauses = (query: string): string[] => { * This is useful if the regex is user-supplied and we want them to be able * to construct it incrementally. */ -const queryRegexSafe = async (query: string): Promise => { +const queryRegexSafe = async ( + query: string, + knex: Knex +): Promise => { // catch regular expression failures in MySQL and return empty result - return await db - .knexInstance() - .raw(query) - .catch((err) => { - if (err.message.includes("regular expression")) { - return [] - } - throw err - }) + return await knexRaw(query, knex).catch((err) => { + if (err.message.includes("regular expression")) { + return [] + } + throw err + }) } -const queryRegexCount = async (query: string): Promise => { - const results = await queryRegexSafe(query) +const queryRegexCount = async ( + query: string, + knex: Knex +): Promise => { + const results = await queryRegexSafe(query, knex) if (!results.length) { return 0 } diff --git a/db/tests/basic.test.ts b/db/tests/basic.test.ts index c3a29d32a2c..1b8465e252b 100644 --- a/db/tests/basic.test.ts +++ b/db/tests/basic.test.ts @@ -3,11 +3,11 @@ import sqlFixtures from "sql-fixtures" import { dbTestConfig } from "./dbTestConfig.js" import { dataSource } from "./dataSource.dbtests.js" import { knex, Knex } from "knex" -import { getConnection } from "../db.js" +import { getConnection, knexRaw } from "../db.js" import { DataSource } from "typeorm" import { insertUser, updateUser, User } from "../model/User.js" import { Chart } from "../model/Chart.js" -import { UsersRow, UsersTableName } from "@ourworldindata/types" +import { DbPlainUser, UsersTableName } from "@ourworldindata/types" let knexInstance: Knex | undefined = undefined let typeOrmConnection: DataSource | undefined = undefined @@ -97,13 +97,13 @@ test("knex interface", async () => { knexInstance.transaction(async (trx) => { // Fetch all users into memory const users = await trx - .from(UsersTableName) + .from(UsersTableName) .select("isSuperuser", "email") expect(users.length).toBe(1) // Fetch all users in a streaming fashion, iterate over them async to avoid having to load everything into memory const usersStream = trx - .from(UsersTableName) + .from(UsersTableName) .select("isSuperuser", "email") .stream() @@ -123,7 +123,7 @@ test("knex interface", async () => { // Check results after update and insert const afterUpdate = await trx - .from(UsersTableName) + .from(UsersTableName) .select("isSuperuser", "email") .orderBy("id") expect(afterUpdate.length).toBe(2) @@ -131,8 +131,59 @@ test("knex interface", async () => { // Use raw queries, using ?? to specify the table name using the shared const value // The pick type is used to type the result row - const usersFromRawQuery: Pick[] = await trx.raw( + const usersFromRawQuery: Pick[] = await knexRaw( "select email from ??", + trx, + [UsersTableName] + ) + expect(usersFromRawQuery.length).toBe(2) + }) +}) + +test("knex interface raw", async () => { + if (!knexInstance) throw new Error("Knex connection not initialized") + + // Create a transaction and run all tests inside it + knexInstance.transaction(async (trx) => { + // Fetch all users into memory + const users = await trx + .from(UsersTableName) + .select("isSuperuser", "email") + expect(users.length).toBe(1) + + // Fetch all users in a streaming fashion, iterate over them async to avoid having to load everything into memory + const usersStream = trx + .from(UsersTableName) + .select("isSuperuser", "email") + .stream() + + for await (const user of usersStream) { + expect(user.isSuperuser).toBe(0) + expect(user.email).toBe("admin@example.com") + } + + // Use the insert helper method + await insertUser(trx, { + email: "test@example.com", + fullName: "Test User", + }) + + // Use the update helper method + await updateUser(trx, 2, { isSuperuser: 1 }) + + // Check results after update and insert + const afterUpdate = await trx + .from(UsersTableName) + .select("isSuperuser", "email") + .orderBy("id") + expect(afterUpdate.length).toBe(2) + expect(afterUpdate[1].isSuperuser).toBe(1) + + // Use raw queries, using ?? to specify the table name using the shared const value + // The pick type is used to type the result row + const usersFromRawQuery: Pick[] = await knexRaw( + "select email from ??", + trx, [UsersTableName] ) expect(usersFromRawQuery.length).toBe(2)