diff --git a/adminSiteServer/adminRouter.tsx b/adminSiteServer/adminRouter.tsx index 7f362e2402c..2e738c12954 100644 --- a/adminSiteServer/adminRouter.tsx +++ b/adminSiteServer/adminRouter.tsx @@ -45,6 +45,7 @@ import { import { getChartConfigBySlug } from "../db/model/Chart.js" import { getVariableMetadata } from "../db/model/Variable.js" import { DbPlainDatasetFile, DbPlainDataset } from "@ourworldindata/types" +import { getPlainRouteWithROTransaction } from "./plainRouterHelpers.js" // Used for rate-limiting important endpoints (login, register) to prevent brute force attacks const limiterMiddleware = ( @@ -136,13 +137,15 @@ adminRouter.post( adminRouter.get("/logout", logOut) -adminRouter.get("/datasets/:datasetId.csv", async (req, res) => { - const datasetId = expectInt(req.params.datasetId) +getPlainRouteWithROTransaction( + adminRouter, + "/datasets/:datasetId.csv", + async (req, res, trx) => { + const datasetId = expectInt(req.params.datasetId) - await db.knexReadonlyTransaction(async (t) => { const datasetName = ( await db.knexRawFirst>( - t, + trx, `SELECT name FROM datasets WHERE id=?`, [datasetId] ) @@ -156,70 +159,74 @@ adminRouter.get("/datasets/:datasetId.csv", async (req, res) => { callback(null) }, }) - await writeDatasetCSV(t, datasetId, writeStream) + await writeDatasetCSV(trx, datasetId, writeStream) res.end() - }) -}) + } +) -adminRouter.get("/datasets/:datasetId/downloadZip", async (req, res) => { - const datasetId = expectInt(req.params.datasetId) +getPlainRouteWithROTransaction( + adminRouter, + "/datasets/:datasetId/downloadZip", + async (req, res, trx) => { + const datasetId = expectInt(req.params.datasetId) - res.attachment("additional-material.zip") + res.attachment("additional-material.zip") - const file = await await db.knexReadonlyTransaction((knex) => - db.knexRawFirst>( - knex, - `SELECT filename, file FROM dataset_files WHERE datasetId=?`, - [datasetId] - ) - ) - res.send(file?.file) -}) + const file = await db.knexRawFirst< + Pick + >(trx, `SELECT filename, file FROM dataset_files WHERE datasetId=?`, [ + datasetId, + ]) + res.send(file?.file) + } +) -adminRouter.get("/posts/preview/:postId", async (req, res) => { - const postId = expectInt(req.params.postId) - const preview = await db.knexReadonlyTransaction(async (knex) => { - return renderPreview(postId, knex) - }) - res.send(preview) -}) +getPlainRouteWithROTransaction( + adminRouter, + "/posts/preview/:postId", + async (req, res, trx) => { + const postId = expectInt(req.params.postId) + const preview = await renderPreview(postId, trx) + res.send(preview) + } +) -adminRouter.get("/posts/compare/:postId", async (req, res) => { - const postId = expectInt(req.params.postId) - - const [wpPage, archieMlText] = await db.knexReadonlyTransaction( - async (t) => { - const wpPage = await renderPreview(postId, t) - const archieMlText = await Post.select( - "archieml", - "archieml_update_statistics" - ).from(t(Post.postsTable).where({ id: postId })) - return [wpPage, archieMlText] - } - ) - - if ( - archieMlText.length === 0 || - archieMlText[0].archieml === null || - archieMlText[0].archieml_update_statistics === null - ) - throw new Error( - `Could not compare posts because archieml was not present in the database for ${postId}` - ) - const archieMlJson = JSON.parse(archieMlText[0].archieml) as OwidGdocJSON - const updateStatsJson = JSON.parse( - archieMlText[0].archieml_update_statistics - ) as OwidArticleBackportingStatistics +getPlainRouteWithROTransaction( + adminRouter, + "/posts/compare/:postId", + async (req, res, trx) => { + const postId = expectInt(req.params.postId) + + const wpPage = await renderPreview(postId, trx) + const archieMlText = await Post.select( + "archieml", + "archieml_update_statistics" + ).from(trx(Post.postsTable).where({ id: postId })) - const errorItems = updateStatsJson.errors.map( - (error) => `
  • ${error.details}
    • ` - ) - const errorList = `
      ${errorItems.join("")}
    ` + if ( + archieMlText.length === 0 || + archieMlText[0].archieml === null || + archieMlText[0].archieml_update_statistics === null + ) + throw new Error( + `Could not compare posts because archieml was not present in the database for ${postId}` + ) + const archieMlJson = JSON.parse( + archieMlText[0].archieml + ) as OwidGdocJSON + const updateStatsJson = JSON.parse( + archieMlText[0].archieml_update_statistics + ) as OwidArticleBackportingStatistics + + const errorItems = updateStatsJson.errors.map( + (error) => `
  • ${error.details}
    • ` + ) + const errorList = `
      ${errorItems.join("")}
    ` - const archieMl = getOwidGdocFromJSON(archieMlJson) - const archieMlPage = renderGdoc(archieMl) + const archieMl = getOwidGdocFromJSON(archieMlJson) + const archieMlPage = renderGdoc(archieMl) - res.send(` + res.send(` @@ -252,7 +259,8 @@ adminRouter.get("/posts/compare/:postId", async (req, res) => { `) -}) + } +) adminRouter.get("/errorTest.csv", async (req, res) => { // Add `table /admin/errorTest.csv?code=404` to test fetch download failures @@ -273,19 +281,23 @@ adminRouter.get(`/${GetAllExplorersRoute}`, async (req, res) => { res.send(await explorerAdminServer.getAllExplorersCommand()) }) -adminRouter.get(`/${GetAllExplorersTagsRoute}`, async (_, res) => { - return res.send({ - explorers: await db.knexReadonlyTransaction((trx) => - db.getExplorerTags(trx) - ), - }) -}) +getPlainRouteWithROTransaction( + adminRouter, + `/${GetAllExplorersTagsRoute}`, + async (_, res, trx) => { + return res.send({ + explorers: await db.getExplorerTags(trx), + }) + } +) -adminRouter.get(`/${EXPLORERS_PREVIEW_ROUTE}/:slug`, async (req, res) => { - const slug = slugify(req.params.slug) - const filename = slug + EXPLORER_FILE_SUFFIX +getPlainRouteWithROTransaction( + adminRouter, + `/${EXPLORERS_PREVIEW_ROUTE}/:slug`, + async (req, res, knex) => { + const slug = slugify(req.params.slug) + const filename = slug + EXPLORER_FILE_SUFFIX - const explorerPage = await db.knexReadonlyTransaction(async (knex) => { if (slug === DefaultNewExplorerSlug) return renderExplorerPage( new ExplorerProgram(DefaultNewExplorerSlug, ""), @@ -297,19 +309,21 @@ adminRouter.get(`/${EXPLORERS_PREVIEW_ROUTE}/:slug`, async (req, res) => { ) return `File not found` const explorer = await explorerAdminServer.getExplorerFromFile(filename) - return renderExplorerPage(explorer, knex) - }) + const explorerPage = renderExplorerPage(explorer, knex) - res.send(explorerPage) -}) + return res.send(explorerPage) + } +) -adminRouter.get("/datapage-preview/:id", async (req, res) => { - const variableId = expectInt(req.params.id) - const variableMetadata = await getVariableMetadata(variableId) - if (!variableMetadata) throw new JsonError("No such variable", 404) +getPlainRouteWithROTransaction( + adminRouter, + "/datapage-preview/:id", + async (req, res, trx) => { + const variableId = expectInt(req.params.id) + const variableMetadata = await getVariableMetadata(variableId) + if (!variableMetadata) throw new JsonError("No such variable", 404) - res.send( - await db.knexReadonlyTransaction((trx) => + res.send( renderDataPageV2( { variableId, @@ -320,19 +334,20 @@ adminRouter.get("/datapage-preview/:id", async (req, res) => { trx ) ) - ) -}) + } +) -adminRouter.get("/grapher/:slug", async (req, res) => { - const previewDataPageOrGrapherPage = db.knexReadonlyTransaction( - async (knex) => { - const entity = await getChartConfigBySlug(knex, req.params.slug) - if (!entity) throw new JsonError("No such chart", 404) - return renderPreviewDataPageOrGrapherPage(entity.config, knex) - } - ) - res.send(previewDataPageOrGrapherPage) -}) +getPlainRouteWithROTransaction( + adminRouter, + "/grapher/:slug", + async (req, res, trx) => { + const entity = await getChartConfigBySlug(trx, req.params.slug) + if (!entity) throw new JsonError("No such chart", 404) + const previewDataPageOrGrapherPage = + await renderPreviewDataPageOrGrapherPage(entity.config, trx) + res.send(previewDataPageOrGrapherPage) + } +) const gitCmsServer = new GitCmsServer({ baseDir: GIT_CMS_DIR, diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index 5edd0b6b679..0ef632c08ef 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -121,7 +121,7 @@ import { postRouteWithRWTransaction, patchRouteWithRWTransaction, getRouteNonIdempotentWithRWTransaction, -} from "./routerHelpers.js" +} from "./functionalRouterHelpers.js" import { getPublishedLinksTo } from "../db/model/Link.js" import { createGdocAndInsertIntoDb, @@ -214,7 +214,8 @@ const getReferencesByChartId = async ( knex ) const postGdocsPromise = getGdocsPostReferencesByChartId(chartId, knex) - const explorerSlugsPromise = db.queryMysql( + const explorerSlugsPromise = db.knexRaw<{ explorerSlug: string }>( + knex, `select distinct explorerSlug from explorer_charts where chartId = ?`, [chartId] ) @@ -426,12 +427,13 @@ getRouteWithROTransaction(apiRouter, "/charts.json", async (req, res, trx) => { return { charts } }) -apiRouter.get("/charts.csv", async (req, res) => { +getRouteWithROTransaction(apiRouter, "/charts.csv", async (req, res, trx) => { const limit = parseIntOrUndefined(req.query.limit as string) ?? 10000 // note: this query is extended from OldChart.listFields. - const charts = await db.queryMysql( - ` + const charts = await db.knexRaw( + trx, + `-- sql SELECT charts.id, charts.config->>"$.version" AS version, @@ -486,25 +488,34 @@ getRouteWithROTransaction( async (req, res, trx) => expectChartById(trx, req.params.chartId) ) -apiRouter.get("/editorData/namespaces.json", async (req, res) => { - const rows = (await db.queryMysql( - `SELECT DISTINCT +getRouteWithROTransaction( + apiRouter, + "/editorData/namespaces.json", + async (req, res, trx) => { + const rows = await db.knexRaw<{ + name: string + description?: string + isArchived: boolean + }>( + trx, + `SELECT DISTINCT namespace AS name, namespaces.description AS description, namespaces.isArchived AS isArchived FROM active_datasets JOIN namespaces ON namespaces.name = active_datasets.namespace` - )) as { name: string; description?: string; isArchived: boolean }[] + ) - return { - namespaces: lodash - .sortBy(rows, (row) => row.description) - .map((namespace) => ({ - ...namespace, - isArchived: !!namespace.isArchived, - })), + return { + namespaces: lodash + .sortBy(rows, (row) => row.description) + .map((namespace) => ({ + ...namespace, + isArchived: !!namespace.isArchived, + })), + } } -}) +) getRouteWithROTransaction( apiRouter, @@ -1375,17 +1386,19 @@ getRouteWithROTransaction( } ) -apiRouter.delete("/users/:userId", async (req, res) => { - if (!res.locals.user.isSuperuser) - throw new JsonError("Permission denied", 403) +deleteRouteWithRWTransaction( + apiRouter, + "/users/:userId", + async (req, res, trx) => { + if (!res.locals.user.isSuperuser) + throw new JsonError("Permission denied", 403) - const userId = expectInt(req.params.userId) - await db.transaction(async (t) => { - await t.execute(`DELETE FROM users WHERE id=?`, [userId]) - }) + const userId = expectInt(req.params.userId) + await db.knexRaw(trx, `DELETE FROM users WHERE id=?`, [userId]) - return { success: true } -}) + return { success: true } + } +) putRouteWithRWTransaction( apiRouter, @@ -1436,10 +1449,13 @@ getRouteWithROTransaction( } ) -apiRouter.get( +getRouteWithROTransaction( + apiRouter, "/chart-bulk-update", async ( - req + req, + res, + trx ): Promise> => { const context: OperationContext = { grapherConfigFieldName: "config", @@ -1458,8 +1474,9 @@ apiRouter.get( // careful there to only allow carefully guarded vocabularies from being used, not // arbitrary user input const whereClause = filterSExpr?.toSql() ?? "true" - const resultsWithStringGrapherConfigs = - await db.queryMysql(`SELECT charts.id as id, + const resultsWithStringGrapherConfigs = await db.knexRaw( + trx, + `SELECT charts.id as id, charts.config as config, charts.createdAt as createdAt, charts.updatedAt as updatedAt, @@ -1473,15 +1490,19 @@ LEFT JOIN users publishedByUser ON publishedByUser.id=charts.publishedByUserId WHERE ${whereClause} ORDER BY charts.id DESC LIMIT 50 -OFFSET ${offset.toString()}`) +OFFSET ${offset.toString()}` + ) const results = resultsWithStringGrapherConfigs.map((row: any) => ({ ...row, config: lodash.isNil(row.config) ? null : JSON.parse(row.config), })) - const resultCount = await db.queryMysql(`SELECT count(*) as count + const resultCount = await db.knexRaw<{ count: number }>( + trx, + `SELECT count(*) as count FROM charts -WHERE ${whereClause}`) +WHERE ${whereClause}` + ) return { rows: results, numTotalRows: resultCount[0].count } } ) @@ -1527,10 +1548,13 @@ patchRouteWithRWTransaction( } ) -apiRouter.get( +getRouteWithROTransaction( + apiRouter, "/variable-annotations", async ( - req + req, + res, + trx ): Promise> => { const context: OperationContext = { grapherConfigFieldName: "grapherConfigAdmin", @@ -1549,8 +1573,9 @@ apiRouter.get( // careful there to only allow carefully guarded vocabularies from being used, not // arbitrary user input const whereClause = filterSExpr?.toSql() ?? "true" - const resultsWithStringGrapherConfigs = - await db.queryMysql(`SELECT variables.id as id, + const resultsWithStringGrapherConfigs = await db.knexRaw( + trx, + `SELECT variables.id as id, variables.name as name, variables.grapherConfigAdmin as config, d.name as datasetname, @@ -1564,30 +1589,37 @@ LEFT JOIN namespaces on d.namespace = namespaces.name WHERE ${whereClause} ORDER BY variables.id DESC LIMIT 50 -OFFSET ${offset.toString()}`) +OFFSET ${offset.toString()}` + ) const results = resultsWithStringGrapherConfigs.map((row: any) => ({ ...row, config: lodash.isNil(row.config) ? null : JSON.parse(row.config), })) - const resultCount = await db.queryMysql(`SELECT count(*) as count + const resultCount = await db.knexRaw<{ count: number }>( + trx, + `SELECT count(*) as count FROM variables LEFT JOIN active_datasets as d on variables.datasetId = d.id LEFT JOIN namespaces on d.namespace = namespaces.name -WHERE ${whereClause}`) +WHERE ${whereClause}` + ) return { rows: results, numTotalRows: resultCount[0].count } } ) -apiRouter.patch("/variable-annotations", async (req) => { - const patchesList = req.body as GrapherConfigPatch[] - const variableIds = new Set(patchesList.map((patch) => patch.id)) +patchRouteWithRWTransaction( + apiRouter, + "/variable-annotations", + async (req, res, trx) => { + const patchesList = req.body as GrapherConfigPatch[] + const variableIds = new Set(patchesList.map((patch) => patch.id)) - await db.transaction(async (manager) => { - const configsAndIds = await manager.query( - `SELECT id, grapherConfigAdmin FROM variables where id IN (?)`, - [[...variableIds.values()]] - ) + const configsAndIds = await db.knexRaw< + Pick + >(trx, `SELECT id, grapherConfigAdmin FROM variables where id IN (?)`, [ + [...variableIds.values()], + ]) const configMap = new Map( configsAndIds.map((item: any) => [ item.id, @@ -1601,26 +1633,31 @@ apiRouter.patch("/variable-annotations", async (req) => { } for (const [variableId, newConfig] of configMap.entries()) { - await manager.execute( + await db.knexRaw( + trx, `UPDATE variables SET grapherConfigAdmin = ? where id = ?`, [JSON.stringify(newConfig), variableId] ) } - }) - return { success: true } -}) + return { success: true } + } +) -apiRouter.get("/variables.usages.json", async (req) => { - const query = `SELECT variableId, COUNT(DISTINCT chartId) AS usageCount +getRouteWithROTransaction( + apiRouter, + "/variables.usages.json", + async (req, res, trx) => { + const query = `SELECT variableId, COUNT(DISTINCT chartId) AS usageCount FROM chart_dimensions GROUP BY variableId ORDER BY usageCount DESC` - const rows = await db.queryMysql(query) + const rows = await db.knexRaw(trx, query) - return rows -}) + return rows + } +) // Used in VariableEditPage getRouteWithROTransaction( @@ -2064,12 +2101,19 @@ postRouteWithRWTransaction( ) // Get a list of redirects that map old slugs to charts -apiRouter.get("/redirects.json", async (req, res) => ({ - redirects: await db.queryMysql(` +getRouteWithROTransaction( + apiRouter, + "/redirects.json", + async (req, res, trx) => ({ + redirects: await db.knexRaw( + trx, + `-- sql SELECT r.id, r.slug, r.chart_id as chartId, JSON_UNQUOTE(JSON_EXTRACT(charts.config, "$.slug")) AS chartSlug FROM chart_slug_redirects AS r JOIN charts ON charts.id = r.chart_id - ORDER BY r.id DESC`), -})) + ORDER BY r.id DESC` + ), + }) +) getRouteWithROTransaction( apiRouter, @@ -2346,8 +2390,9 @@ deleteRouteWithRWTransaction( } ) -apiRouter.get("/posts.json", async (req) => { - const raw_rows = await db.queryMysql( +getRouteWithROTransaction(apiRouter, "/posts.json", async (req, res, trx) => { + const raw_rows = await db.knexRaw( + trx, `-- sql with posts_tags_aggregated as ( select post_id, if(count(tags.id) = 0, json_array(), json_arrayagg(json_object("id", tags.id, "name", tags.name))) as tags diff --git a/adminSiteServer/authentication.tsx b/adminSiteServer/authentication.tsx index 285750176d1..31e6317bf41 100644 --- a/adminSiteServer/authentication.tsx +++ b/adminSiteServer/authentication.tsx @@ -112,9 +112,11 @@ export async function authCloudflareSSOMiddleware( export async function logOut(req: express.Request, res: express.Response) { if (res.locals.user) - await db.queryMysql(`DELETE FROM sessions WHERE session_key = ?`, [ - res.locals.session.id, - ]) + await db.knexReadWriteTransaction((trx) => + db.knexRaw(trx, `DELETE FROM sessions WHERE session_key = ?`, [ + res.locals.session.id, + ]) + ) res.clearCookie("sessionid") res.clearCookie(CLOUDFLARE_COOKIE_NAME) @@ -216,16 +218,18 @@ async function logInAsUser(user: DbPlainUser) { const now = new Date() const expiryDate = new Date(now.getTime() + 1000 * SESSION_COOKIE_AGE) - await db.execute( - `INSERT INTO sessions (session_key, session_data, expire_date) VALUES (?, ?, ?)`, - [sessionId, sessionData, expiryDate] - ) + await db.knexReadWriteTransaction(async (trx) => { + await db.knexRaw( + trx, + `INSERT INTO sessions (session_key, session_data, expire_date) VALUES (?, ?, ?)`, + [sessionId, sessionData, expiryDate] + ) - await db - .knexInstance() - .table("users") - .where({ id: user.id }) - .update({ lastLogin: now }) + await trx + .table("users") + .where({ id: user.id }) + .update({ lastLogin: now }) + }) return { id: sessionId, expiryDate: expiryDate } } diff --git a/adminSiteServer/routerHelpers.tsx b/adminSiteServer/functionalRouterHelpers.tsx similarity index 100% rename from adminSiteServer/routerHelpers.tsx rename to adminSiteServer/functionalRouterHelpers.tsx diff --git a/adminSiteServer/gitDataExport.ts b/adminSiteServer/gitDataExport.ts index f14fcd6643d..cdb42c387f0 100644 --- a/adminSiteServer/gitDataExport.ts +++ b/adminSiteServer/gitDataExport.ts @@ -64,7 +64,6 @@ export async function syncDatasetToGitRepo( knex: db.KnexReadonlyTransaction, datasetId: number, options: { - transaction?: db.TransactionContext oldDatasetName?: string commitName?: string commitEmail?: string diff --git a/adminSiteServer/mockSiteRouter.tsx b/adminSiteServer/mockSiteRouter.tsx index c8ba6140331..ac87e29c733 100644 --- a/adminSiteServer/mockSiteRouter.tsx +++ b/adminSiteServer/mockSiteRouter.tsx @@ -57,6 +57,7 @@ import { GdocPost } from "../db/model/Gdoc/GdocPost.js" import { GdocDataInsight } from "../db/model/Gdoc/GdocDataInsight.js" import * as db from "../db/db.js" import { calculateDataInsightIndexPageCount } from "../db/model/Gdoc/gdocUtils.js" +import { getPlainRouteWithROTransaction } from "./plainRouterHelpers.js" require("express-async-errors") @@ -66,40 +67,47 @@ const mockSiteRouter = Router() mockSiteRouter.use(express.urlencoded({ extended: true })) mockSiteRouter.use(express.json()) -mockSiteRouter.get("/sitemap.xml", async (req, res) => { - res.set("Content-Type", "application/xml") - const sitemap = await db.knexReadonlyTransaction(async (knex) => - makeSitemap(explorerAdminServer, knex) - ) - res.send(sitemap) -}) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/sitemap.xml", + async (req, res, trx) => { + res.set("Content-Type", "application/xml") + const sitemap = await makeSitemap(explorerAdminServer, trx) + res.send(sitemap) + } +) -mockSiteRouter.get("/atom.xml", async (req, res) => { - res.set("Content-Type", "application/xml") - const atomFeed = await db.knexReadonlyTransaction(async (knex) => - makeAtomFeed(knex) - ) - res.send(atomFeed) -}) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/atom.xml", + async (req, res, trx) => { + res.set("Content-Type", "application/xml") + const atomFeed = await makeAtomFeed(trx) + res.send(atomFeed) + } +) -mockSiteRouter.get("/atom-no-topic-pages.xml", async (req, res) => { - res.set("Content-Type", "application/xml") - const atomFeedNoTopicPages = await db.knexReadonlyTransaction( - async (knex) => makeAtomFeedNoTopicPages(knex) - ) - res.send(atomFeedNoTopicPages) -}) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/atom-no-topic-pages.xml", + async (req, res, trx) => { + res.set("Content-Type", "application/xml") + const atomFeedNoTopicPages = await makeAtomFeedNoTopicPages(trx) + res.send(atomFeedNoTopicPages) + } +) -mockSiteRouter.get("/entries-by-year", async (req, res) => - res.send(await db.knexReadonlyTransaction((trx) => entriesByYearPage(trx))) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/entries-by-year", + async (req, res, trx) => res.send(await entriesByYearPage(trx)) ) -mockSiteRouter.get(`/entries-by-year/:year`, async (req, res) => - res.send( - await db.knexReadonlyTransaction((trx) => - entriesByYearPage(trx, parseInt(req.params.year)) - ) - ) +getPlainRouteWithROTransaction( + mockSiteRouter, + `/entries-by-year/:year`, + async (req, res, trx) => + res.send(await entriesByYearPage(trx, parseInt(req.params.year))) ) mockSiteRouter.get( @@ -127,98 +135,106 @@ mockSiteRouter.get("/assets/embedCharts.js", async (req, res) => { const explorerAdminServer = new ExplorerAdminServer(GIT_CMS_DIR) -mockSiteRouter.get(`/${EXPLORERS_ROUTE_FOLDER}/:slug`, async (req, res) => { - res.set("Access-Control-Allow-Origin", "*") - const explorers = await explorerAdminServer.getAllPublishedExplorers() - const explorerProgram = explorers.find( - (program) => program.slug === req.params.slug - ) - if (explorerProgram) { - const explorerPage = await db.knexReadonlyTransaction(async (knex) => { - return renderExplorerPage(explorerProgram, knex) - }) - - res.send(explorerPage) - } else - throw new JsonError( - "A published explorer with that slug was not found", - 404 +getPlainRouteWithROTransaction( + mockSiteRouter, + `/${EXPLORERS_ROUTE_FOLDER}/:slug`, + async (req, res, trx) => { + res.set("Access-Control-Allow-Origin", "*") + const explorers = await explorerAdminServer.getAllPublishedExplorers() + const explorerProgram = explorers.find( + (program) => program.slug === req.params.slug ) -}) -mockSiteRouter.get("/*", async (req, res, next) => { - const explorerRedirect = getExplorerRedirectForPath(req.path) - // If no explorer redirect exists, continue to next express handler - if (!explorerRedirect) return next() - - const { migrationId, baseQueryStr } = explorerRedirect - const { explorerSlug } = explorerUrlMigrationsById[migrationId] - const program = await explorerAdminServer.getExplorerFromSlug(explorerSlug) - const explorerPage = await db.knexReadonlyTransaction(async (knex) => { - return renderExplorerPage(program, knex, { + if (explorerProgram) { + const explorerPage = await renderExplorerPage(explorerProgram, trx) + + res.send(explorerPage) + } else + throw new JsonError( + "A published explorer with that slug was not found", + 404 + ) + } +) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/*", + async (req, res, trx, next) => { + const explorerRedirect = getExplorerRedirectForPath(req.path) + // If no explorer redirect exists, continue to next express handler + if (!explorerRedirect) return next!() + + const { migrationId, baseQueryStr } = explorerRedirect + const { explorerSlug } = explorerUrlMigrationsById[migrationId] + const program = + await explorerAdminServer.getExplorerFromSlug(explorerSlug) + const explorerPage = await renderExplorerPage(program, trx, { explorerUrlMigrationId: migrationId, baseQueryStr, }) - }) - res.send(explorerPage) -}) + res.send(explorerPage) + } +) -mockSiteRouter.get("/collection/top-charts", async (_, res) => { - return res.send(await renderTopChartsCollectionPage()) -}) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/collection/top-charts", + async (_, res, trx) => { + return res.send(await renderTopChartsCollectionPage(trx)) + } +) mockSiteRouter.get("/collection/custom", async (_, res) => { return res.send(await renderDynamicCollectionPage()) }) -mockSiteRouter.get("/grapher/:slug", async (req, res) => { - const previewDataPageOrGrapherPage = await db.knexReadonlyTransaction( - async (knex) => { - const entity = await getChartConfigBySlug(knex, req.params.slug) - if (!entity) throw new JsonError("No such chart", 404) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/grapher/:slug", + async (req, res, trx) => { + const entity = await getChartConfigBySlug(trx, req.params.slug) + if (!entity) throw new JsonError("No such chart", 404) - // XXX add dev-prod parity for this - res.set("Access-Control-Allow-Origin", "*") + // XXX add dev-prod parity for this + res.set("Access-Control-Allow-Origin", "*") - return renderPreviewDataPageOrGrapherPage(entity.config, knex) - } - ) - res.send(previewDataPageOrGrapherPage) -}) + const previewDataPageOrGrapherPage = + await renderPreviewDataPageOrGrapherPage(entity.config, trx) + res.send(previewDataPageOrGrapherPage) + } +) -mockSiteRouter.get("/", async (req, res) => { - const frontPage = await db.knexReadonlyTransaction(async (knex) => - renderFrontPage(knex) - ) +getPlainRouteWithROTransaction(mockSiteRouter, "/", async (req, res, trx) => { + const frontPage = await renderFrontPage(trx) res.send(frontPage) }) -mockSiteRouter.get( +getPlainRouteWithROTransaction( + mockSiteRouter, "/donate", - async (req, res) => - await db.knexReadonlyTransaction(async (knex) => - res.send(await renderDonatePage(knex)) - ) + async (req, res, trx) => res.send(await renderDonatePage(trx)) ) mockSiteRouter.get("/thank-you", async (req, res) => res.send(await renderThankYouPage()) ) -mockSiteRouter.get("/data-insights/:pageNumberOrSlug?", async (req, res) => { - return await db.knexReadonlyTransaction(async (knex) => { +getPlainRouteWithROTransaction( + mockSiteRouter, + "/data-insights/:pageNumberOrSlug?", + async (req, res, trx) => { const totalPageCount = calculateDataInsightIndexPageCount( await db - .getPublishedDataInsights(knex) + .getPublishedDataInsights(trx) .then((insights) => insights.length) ) async function renderIndexPage(pageNumber: number) { const dataInsights = await GdocDataInsight.getPublishedDataInsights( - knex, + trx, pageNumber ) // calling fetchImageMetadata 20 times makes me sad, would be nice if we could cache this await Promise.all( - dataInsights.map((insight) => insight.loadState(knex)) + dataInsights.map((insight) => insight.loadState(trx)) ) return renderDataInsightsIndexPage( dataInsights, @@ -243,27 +259,33 @@ mockSiteRouter.get("/data-insights/:pageNumberOrSlug?", async (req, res) => { const slug = pageNumberOrSlug try { - return res.send(await renderGdocsPageBySlug(knex, slug, true)) + return res.send(await renderGdocsPageBySlug(trx, slug, true)) } catch (e) { console.error(e) } return new JsonError(`Data insight with slug "${slug}" not found`, 404) - }) -}) + } +) -mockSiteRouter.get("/charts", async (req, res) => { - const explorerAdminServer = new ExplorerAdminServer(GIT_CMS_DIR) - res.send(await renderChartsPage(explorerAdminServer)) -}) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/charts", + async (req, res, trx) => { + const explorerAdminServer = new ExplorerAdminServer(GIT_CMS_DIR) + res.send(await renderChartsPage(trx, explorerAdminServer)) + } +) -mockSiteRouter.get("/datapage-preview/:id", async (req, res) => { - const variableId = expectInt(req.params.id) - const variableMetadata = await getVariableMetadata(variableId) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/datapage-preview/:id", + async (req, res, trx) => { + const variableId = expectInt(req.params.id) + const variableMetadata = await getVariableMetadata(variableId) - res.send( - await db.knexReadonlyTransaction((trx) => - renderDataPageV2( + res.send( + await renderDataPageV2( { variableId, variableMetadata, @@ -273,38 +295,51 @@ mockSiteRouter.get("/datapage-preview/:id", async (req, res) => { trx ) ) - ) -}) + } +) countryProfileSpecs.forEach((spec) => - mockSiteRouter.get(`/${spec.rootPath}/:countrySlug`, async (req, res) => { - const countryPage = await db.knexReadonlyTransaction(async (knex) => - countryProfileCountryPage(spec, req.params.countrySlug, knex) - ) - res.send(countryPage) - }) + getPlainRouteWithROTransaction( + mockSiteRouter, + `/${spec.rootPath}/:countrySlug`, + async (req, res, trx) => { + const countryPage = await countryProfileCountryPage( + spec, + req.params.countrySlug, + trx + ) + res.send(countryPage) + } + ) ) mockSiteRouter.get("/search", async (req, res) => res.send(await renderSearchPage()) ) -mockSiteRouter.get("/latest", async (req, res) => { - const latest = await db.knexReadonlyTransaction(async (knex) => - renderBlogByPageNum(1, knex) - ) - res.send(latest) -}) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/latest", + async (req, res, trx) => { + const latest = await renderBlogByPageNum(1, trx) + res.send(latest) + } +) -mockSiteRouter.get("/latest/page/:pageno", async (req, res) => { - const pagenum = parseInt(req.params.pageno, 10) - if (!isNaN(pagenum)) { - const latestPageNum = await db.knexReadonlyTransaction(async (knex) => - renderBlogByPageNum(isNaN(pagenum) ? 1 : pagenum, knex) - ) - res.send(latestPageNum) - } else throw new Error("invalid page number") -}) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/latest/page/:pageno", + async (req, res, trx) => { + const pagenum = parseInt(req.params.pageno, 10) + if (!isNaN(pagenum)) { + const latestPageNum = await renderBlogByPageNum( + isNaN(pagenum) ? 1 : pagenum, + trx + ) + res.send(latestPageNum) + } else throw new Error("invalid page number") + } +) mockSiteRouter.get("/headerMenu.json", async (req, res) => { res.contentType("application/json") @@ -328,14 +363,19 @@ mockSiteRouter.use( mockSiteRouter.use("/assets", express.static("dist/assets")) -mockSiteRouter.use("/grapher/exports/:slug.svg", async (req, res) => { - await db.knexReadonlyTransaction(async (knex) => { - const grapher = await getChartConfigBySlug(knex, req.params.slug) +// TODO: this used to be a mockSiteRouter.use call but otherwise it looked like a route and +// it didn't look like it was making use of any middleware - if this causese issues then +// this has to be reverted to a use call +getPlainRouteWithROTransaction( + mockSiteRouter, + "/grapher/exports/:slug.svg", + async (req, res, trx) => { + const grapher = await getChartConfigBySlug(trx, req.params.slug) const vardata = await getChartVariableData(grapher.config) res.setHeader("Content-Type", "image/svg+xml") res.send(await grapherToSVG(grapher.config, vardata)) - }) -}) + } +) mockSiteRouter.use( "/fonts", @@ -352,12 +392,17 @@ mockSiteRouter.get("/countries", async (req, res) => res.send(await countriesIndexPage(BAKED_BASE_URL)) ) -mockSiteRouter.get("/country/:countrySlug", async (req, res) => - res.send( - await db.knexReadonlyTransaction((trx) => - countryProfilePage(trx, req.params.countrySlug, BAKED_BASE_URL) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/country/:countrySlug", + async (req, res, trx) => + res.send( + await countryProfilePage( + trx, + req.params.countrySlug, + BAKED_BASE_URL + ) ) - ) ) mockSiteRouter.get("/feedback", async (req, res) => @@ -372,40 +417,42 @@ mockSiteRouter.get("/multiEmbedderTest", async (req, res) => ) ) -mockSiteRouter.get("/dods.json", async (_, res) => { - res.set("Access-Control-Allow-Origin", "*") - const { details, parseErrors } = await db.knexReadonlyTransaction((trx) => - GdocPost.getDetailsOnDemandGdoc(trx) - ) - if (parseErrors.length) { - console.error( - `Error(s) parsing details: ${parseErrors - .map((e) => e.message) - .join(", ")}` - ) +getPlainRouteWithROTransaction( + mockSiteRouter, + "/dods.json", + async (_, res, trx) => { + res.set("Access-Control-Allow-Origin", "*") + const { details, parseErrors } = + await GdocPost.getDetailsOnDemandGdoc(trx) + + if (parseErrors.length) { + console.error( + `Error(s) parsing details: ${parseErrors + .map((e) => e.message) + .join(", ")}` + ) + } + res.send(details) } - res.send(details) -}) +) -mockSiteRouter.get("/*", async (req, res) => { +getPlainRouteWithROTransaction(mockSiteRouter, "/*", async (req, res, trx) => { const slug = req.path.replace(/^\//, "") - await db.knexReadonlyTransaction(async (knex) => { - try { - const page = await renderGdocsPageBySlug(knex, slug) - res.send(page) - } catch (e) { - console.error(e) - } + try { + const page = await renderGdocsPageBySlug(trx, slug) + res.send(page) + } catch (e) { + console.error(e) + } - try { - const page = await renderPageBySlug(slug, knex) - res.send(page) - } catch (e) { - console.error(e) - res.status(404).send(await renderNotFoundPage()) - } - }) + try { + const page = await renderPageBySlug(slug, trx) + res.send(page) + } catch (e) { + console.error(e) + res.status(404).send(await renderNotFoundPage()) + } }) export { mockSiteRouter } diff --git a/adminSiteServer/plainRouterHelpers.tsx b/adminSiteServer/plainRouterHelpers.tsx new file mode 100644 index 00000000000..3e3581fb4d7 --- /dev/null +++ b/adminSiteServer/plainRouterHelpers.tsx @@ -0,0 +1,105 @@ +import { NextFunction, Request, Response, Router } from "express" +import * as db from "../db/db.js" +export function getPlainRouteWithROTransaction( + router: Router, + targetPath: string, + handler: ( + req: Request, + res: Response, + trx: db.KnexReadonlyTransaction, + next?: NextFunction + ) => Promise +) { + return router.get( + targetPath, + (req: Request, res: Response, nxt: NextFunction) => { + return db.knexReadonlyTransaction((transaction) => + handler(req, res, transaction, nxt) + ) + } + ) +} + +/** Usually get routes should be idempotent (caching and retry reasons among others), + but for example the gdoc preview route is not because it updates the gdoc in the DB after + fetching it from the google API. + */ +export function getPlainRouteNonIdempotentWithRWTransaction( + router: Router, + targetPath: string, + handler: ( + req: Request, + res: Response, + trx: db.KnexReadWriteTransaction + ) => Promise +) { + return router.get(targetPath, (req: Request, res: Response) => { + return db.knexReadWriteTransaction((transaction) => + handler(req, res, transaction) + ) + }) +} + +export function postPlainRouteWithRWTransaction( + router: Router, + targetPath: string, + handler: ( + req: Request, + res: Response, + trx: db.KnexReadWriteTransaction + ) => Promise +) { + return router.post(targetPath, (req: Request, res: Response) => { + return db.knexReadWriteTransaction((transaction) => + handler(req, res, transaction) + ) + }) +} + +export function putPlainRouteWithRWTransaction( + router: Router, + targetPath: string, + handler: ( + req: Request, + res: Response, + trx: db.KnexReadWriteTransaction + ) => Promise +) { + return router.put(targetPath, (req: Request, res: Response) => { + return db.knexReadWriteTransaction((transaction) => + handler(req, res, transaction) + ) + }) +} + +export function patchPlainRouteWithRWTransaction( + router: Router, + targetPath: string, + handler: ( + req: Request, + res: Response, + trx: db.KnexReadWriteTransaction + ) => Promise +) { + return router.patch(targetPath, (req: Request, res: Response) => { + return db.knexReadWriteTransaction((transaction) => + handler(req, res, transaction) + ) + }) +} + +export function deletePlainRouteWithRWTransaction( + router: Router, + targetPath: string, + handler: ( + req: Request, + res: Response, + trx: db.KnexReadWriteTransaction + ) => Promise +) { + return router.delete(targetPath, (req: Request, res: Response) => { + return db.knexReadWriteTransaction((transaction) => + handler(req, res, transaction) + ) + }) +} diff --git a/adminSiteServer/publicApiRouter.ts b/adminSiteServer/publicApiRouter.ts index 15a87b65d96..946c8859ced 100644 --- a/adminSiteServer/publicApiRouter.ts +++ b/adminSiteServer/publicApiRouter.ts @@ -9,9 +9,12 @@ function rejectAfterDelay(ms: number) { } publicApiRouter.router.get("/health", async (req: Request, res: Response) => { - const sqlPromise = db.mysqlFirst(`SELECT id FROM charts LIMIT 1`) - const timeoutPromise = rejectAfterDelay(1500) // Wait 1.5 seconds at most try { + const sqlPromise = db.knexRaw( + db.knexInstance() as db.KnexReadonlyTransaction, + `SELECT id FROM charts LIMIT 1` + ) + const timeoutPromise = rejectAfterDelay(1500) // Wait 1.5 seconds at most await Promise.race([sqlPromise, timeoutPromise]) res.status(200).end("OK") } catch (e) { diff --git a/adminSiteServer/testPageRouter.tsx b/adminSiteServer/testPageRouter.tsx index 855d3d55eb6..3f197d28fc9 100644 --- a/adminSiteServer/testPageRouter.tsx +++ b/adminSiteServer/testPageRouter.tsx @@ -34,6 +34,7 @@ import { import { ExplorerAdminServer } from "../explorerAdminServer/ExplorerAdminServer.js" import { GIT_CMS_DIR } from "../gitCms/GitCmsConstants.js" import { ExplorerChartCreationMode } from "../explorer/ExplorerConstants.js" +import { getPlainRouteWithROTransaction } from "./plainRouterHelpers.js" const IS_LIVE = ADMIN_BASE_URL === "https://owid.cloud" const DEFAULT_COMPARISON_URL = "https://ourworldindata.org" @@ -437,19 +438,23 @@ function EmbedTestPage(props: EmbedTestPageProps) { ) } -testPageRouter.get("/embeds", async (req, res) => { - const props = await db.knexReadonlyTransaction((trx) => - propsFromQueryParams(trx, { +getPlainRouteWithROTransaction( + testPageRouter, + "/embeds", + async (req, res, trx) => { + const props = await propsFromQueryParams(trx, { ...req.query, originalUrl: req.originalUrl, }) - ) - res.send(renderToHtmlPage()) -}) + res.send(renderToHtmlPage()) + } +) -testPageRouter.get("/embeds/:id", async (req, res) => { - const id = req.params.id - await db.knexReadonlyTransaction(async (trx) => { +getPlainRouteWithROTransaction( + testPageRouter, + "/embeds/:id", + async (req, res, trx) => { + const id = req.params.id const chartRaw: DbRawChart = await trx .table(ChartsTableName) .where({ id: id }) @@ -477,8 +482,8 @@ testPageRouter.get("/embeds/:id", async (req, res) => { } else { res.send("Could not find chart ID") } - }) -}) + } +) function PreviewTestPage(props: { charts: any[] }) { const style = ` @@ -618,36 +623,52 @@ function EmbedVariantsTestPage( ) } -testPageRouter.get("/previews", async (req, res) => { - const rows = await db.queryMysql(`SELECT config FROM charts LIMIT 200`) - const charts = rows.map((row: any) => JSON.parse(row.config)) - - res.send(renderToHtmlPage()) -}) - -testPageRouter.get("/embedVariants", async (req, res) => { - const rows = await db.queryMysql(`SELECT config FROM charts WHERE id=64`) - const charts = rows.map((row: any) => JSON.parse(row.config)) - const viewProps = getViewPropsFromQueryParams(req.query) +getPlainRouteWithROTransaction( + testPageRouter, + "/previews", + async (req, res, trx) => { + const rows = await db.knexRaw( + trx, + `SELECT config FROM charts LIMIT 200` + ) + const charts = rows.map((row: any) => JSON.parse(row.config)) - res.send( - renderToHtmlPage( - + res.send(renderToHtmlPage()) + } +) + +getPlainRouteWithROTransaction( + testPageRouter, + "/embedVariants", + async (req, res, trx) => { + const rows = await db.knexRaw( + trx, + `SELECT config FROM charts WHERE id=64` ) - ) -}) + const charts = rows.map((row: any) => JSON.parse(row.config)) + const viewProps = getViewPropsFromQueryParams(req.query) + + res.send( + renderToHtmlPage( + + ) + ) + } +) -testPageRouter.get("/:slug.svg", async (req, res) => { - await db.knexReadonlyTransaction(async (trx) => { +getPlainRouteWithROTransaction( + testPageRouter, + "/:slug.svg", + async (req, res, trx) => { const grapher = await getChartConfigBySlug(trx, req.params.slug) const vardata = await getChartVariableData(grapher.config) const svg = await grapherToSVG(grapher.config, vardata) res.send(svg) - }) -}) + } +) testPageRouter.get("/explorers", async (req, res) => { let explorers = await explorerAdminServer.getAllPublishedExplorers() diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index e862a6e32cd..d5310a9263f 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -245,7 +245,7 @@ export async function renderDataPageV2( let slug = "" if (firstTopicTag) { try { - slug = await getSlugForTopicTag(firstTopicTag) + slug = await getSlugForTopicTag(knex, firstTopicTag) } catch (error) { await logErrorAndMaybeSendToBugsnag( `Datapage with variableId "${variableId}" and title "${datapageData.title.title}" is using "${firstTopicTag}" as its primary tag, which we are unable to resolve to a tag in the grapher DB` @@ -293,7 +293,7 @@ export async function renderDataPageV2( datapageData.relatedResearch = await getRelatedResearchAndWritingForVariable(knex, variableId) - const tagToSlugMap = await getTagToSlugMap() + const tagToSlugMap = await getTagToSlugMap(knex) return renderToHtmlPage( { - await db.knexReadonlyTransaction((trx) => - bakeSingleGrapherChart(job, trx) - ) + // TODO: not sure if the shared transaction will be an issue - I think it should be fine but just to put a flag here + // that this could be causing issues + await bakeSingleGrapherChart(job, knex) progressBar.tick({ name: `slug ${job.slug}` }) }, { concurrency: 10 } diff --git a/baker/GrapherBakingUtils.ts b/baker/GrapherBakingUtils.ts index 68061371abb..f44c67083c0 100644 --- a/baker/GrapherBakingUtils.ts +++ b/baker/GrapherBakingUtils.ts @@ -95,6 +95,7 @@ export const bakeGrapherUrls = async ( if (toBake.length > 0) { await bakeGraphersToSvgs( + knex, toBake, `${BAKED_SITE_DIR}/exports`, OPTIMIZE_SVG_EXPORTS @@ -141,12 +142,13 @@ export const getGrapherExportsByUrl = async (): Promise => { * "Women's Rights" -> "womens-rights" * 123 -> "womens-rights" */ -export async function getTagToSlugMap(): Promise< - Record -> { - const tags = (await db.queryMysql( +export async function getTagToSlugMap( + knex: db.KnexReadonlyTransaction +): Promise> { + const tags = await db.knexRaw>( + knex, `SELECT slug, name, id FROM tags WHERE slug IS NOT NULL` - )) as Pick[] + ) const tagsByIdAndName: Record = {} for (const tag of tags) { if (tag.slug) { @@ -163,10 +165,11 @@ export async function getTagToSlugMap(): Promise< * Throws an error if no slug is found so we can log it in Bugsnag */ export async function getSlugForTopicTag( + knex: db.KnexReadonlyTransaction, identifier: string | number ): Promise { const propertyToMatch = typeof identifier === "string" ? "slug" : "id" - const tagsByIdAndName = await getTagToSlugMap() + const tagsByIdAndName = await getTagToSlugMap(knex) const slug = tagsByIdAndName[identifier] if (!slug) { diff --git a/baker/GrapherImageBaker.tsx b/baker/GrapherImageBaker.tsx index 03cc579d3c8..a8232510b88 100644 --- a/baker/GrapherImageBaker.tsx +++ b/baker/GrapherImageBaker.tsx @@ -1,4 +1,8 @@ -import { GrapherInterface } from "@ourworldindata/types" +import { + DbPlainChartSlugRedirect, + DbRawChart, + GrapherInterface, +} from "@ourworldindata/types" import { Grapher, GrapherProgrammaticInterface } from "@ourworldindata/grapher" import { MultipleOwidVariableDataDimensionsMap } from "@ourworldindata/utils" import fs from "fs-extra" @@ -48,14 +52,17 @@ export async function bakeGraphersToPngs( ]) } -export async function getGraphersAndRedirectsBySlug() { - const { graphersBySlug, graphersById } = await getPublishedGraphersBySlug() +export async function getGraphersAndRedirectsBySlug( + knex: db.KnexReadonlyTransaction +) { + const { graphersBySlug, graphersById } = + await getPublishedGraphersBySlug(knex) - const redirectQuery = db.queryMysql( - `SELECT slug, chart_id FROM chart_slug_redirects` - ) + const redirectQuery = await db.knexRaw< + Pick + >(knex, `SELECT slug, chart_id FROM chart_slug_redirects`) - for (const row of await redirectQuery) { + for (const row of redirectQuery) { const grapher = graphersById.get(row.chart_id) if (grapher) { graphersBySlug.set(row.slug, grapher) @@ -65,14 +72,16 @@ export async function getGraphersAndRedirectsBySlug() { return graphersBySlug } -export async function getPublishedGraphersBySlug() { +export async function getPublishedGraphersBySlug( + knex: db.KnexReadonlyTransaction +) { const graphersBySlug: Map = new Map() const graphersById: Map = new Map() // Select all graphers that are published const sql = `SELECT id, config FROM charts WHERE config->>"$.isPublished" = "true"` - const query = db.queryMysql(sql) + const query = db.knexRaw>(knex, sql) for (const row of await query) { const grapher = JSON.parse(row.config) @@ -160,12 +169,13 @@ export function buildSvgOutFilepath( } export async function bakeGraphersToSvgs( + knex: db.KnexReadonlyTransaction, grapherUrls: string[], outDir: string, optimizeSvgs = false ) { await fs.mkdirp(outDir) - const graphersBySlug = await getGraphersAndRedirectsBySlug() + const graphersBySlug = await getGraphersAndRedirectsBySlug(knex) return pMap( grapherUrls, diff --git a/baker/SiteBaker.tsx b/baker/SiteBaker.tsx index 87f211b3044..09f9eeebfc8 100644 --- a/baker/SiteBaker.tsx +++ b/baker/SiteBaker.tsx @@ -431,8 +431,6 @@ export class SiteBaker { private async removeDeletedPosts(knex: db.KnexReadonlyTransaction) { if (!this.bakeSteps.has("removeDeletedPosts")) return - await db.getConnection() - const postsApi = await getPostsFromSnapshots(knex) const postSlugs = [] @@ -574,7 +572,7 @@ export class SiteBaker { ) await this.stageWrite( `${this.bakedSiteDir}/collection/top-charts.html`, - await renderTopChartsCollectionPage() + await renderTopChartsCollectionPage(knex) ) await this.stageWrite( `${this.bakedSiteDir}/404.html`, @@ -592,7 +590,7 @@ export class SiteBaker { await this.stageWrite( `${this.bakedSiteDir}/charts.html`, - await renderChartsPage(this.explorerAdminServer) + await renderChartsPage(knex, this.explorerAdminServer) ) this.progressBar.tick({ name: "✅ baked special pages" }) } @@ -947,7 +945,7 @@ export class SiteBaker { redirects.join("\n") ) - const grapherRedirects = await getGrapherRedirectsMap("") + const grapherRedirects = await getGrapherRedirectsMap(knex, "") await this.stageWrite( path.join(this.bakedSiteDir, `grapher/_grapherRedirects.json`), JSON.stringify(Object.fromEntries(grapherRedirects), null, 2) @@ -991,7 +989,6 @@ export class SiteBaker { } async bakeNonWordpressPages(knex: db.KnexReadonlyTransaction) { - await db.getConnection() const progressBarTotal = nonWordpressSteps .map((step) => this.bakeSteps.has(step)) .filter((hasStep) => hasStep).length diff --git a/baker/algolia/indexChartsToAlgolia.ts b/baker/algolia/indexChartsToAlgolia.ts index 65b2942dbfe..d36aeb661d7 100644 --- a/baker/algolia/indexChartsToAlgolia.ts +++ b/baker/algolia/indexChartsToAlgolia.ts @@ -132,7 +132,6 @@ const indexChartsToAlgolia = async () => { const index = client.initIndex(getIndexName(SearchIndexName.Charts)) - await db.getConnection() const records = await db.knexReadonlyTransaction(getChartsRecords) await index.replaceAllObjects(records) diff --git a/baker/algolia/indexToAlgolia.tsx b/baker/algolia/indexToAlgolia.tsx index 019e984d71e..d3eedbda1de 100644 --- a/baker/algolia/indexToAlgolia.tsx +++ b/baker/algolia/indexToAlgolia.tsx @@ -234,7 +234,6 @@ const indexToAlgolia = async () => { } const index = client.initIndex(getIndexName(SearchIndexName.Pages)) - await db.getConnection() const records = await db.knexReadonlyTransaction(getPagesRecords) await index.replaceAllObjects(records) diff --git a/baker/bakeGdocPost.ts b/baker/bakeGdocPost.ts index 108d116f7c4..3defc7afbf3 100644 --- a/baker/bakeGdocPost.ts +++ b/baker/bakeGdocPost.ts @@ -19,7 +19,6 @@ void yargs(hideBin(process.argv)) async ({ slug }) => { const baker = new SiteBaker(BAKED_SITE_DIR, BAKED_BASE_URL) - await db.getConnection() await db.knexReadonlyTransaction((trx) => baker.bakeGDocPosts(trx, [slug]) ) diff --git a/baker/bakeGdocPosts.ts b/baker/bakeGdocPosts.ts index 33e822fa0a9..64de00aa80e 100644 --- a/baker/bakeGdocPosts.ts +++ b/baker/bakeGdocPosts.ts @@ -24,7 +24,6 @@ void yargs(hideBin(process.argv)) async ({ slugs }) => { const baker = new SiteBaker(BAKED_SITE_DIR, BAKED_BASE_URL) - await db.getConnection() await db.knexReadonlyTransaction((trx) => baker.bakeGDocPosts(trx, slugs) ) diff --git a/baker/batchTagWithGpt.ts b/baker/batchTagWithGpt.ts index c40f738fb12..d7f80a49aec 100644 --- a/baker/batchTagWithGpt.ts +++ b/baker/batchTagWithGpt.ts @@ -18,14 +18,14 @@ Example: yarn batchTagWithGpt --debug --limit 1 Note: this script is not called automatically yet, and needs to be run manually. */ -export const batchTagWithGpt = async ({ - debug, - limit, -}: BatchTagWithGptArgs = {}) => { - await db.knexReadonlyTransaction((trx) => - batchTagChartsWithGpt(trx, { debug, limit }) - ) -} +// export const batchTagWithGpt = async ({ +// debug, +// limit, +// }: BatchTagWithGptArgs = {}) => { +// await db.knexReadonlyTransaction((trx) => +// batchTagChartsWithGpt(trx, { debug, limit }) +// ) +// } const batchTagChartsWithGpt = async ( knex: db.KnexReadonlyTransaction, diff --git a/baker/redirects.ts b/baker/redirects.ts index d55be1acc6f..c48f3833ba2 100644 --- a/baker/redirects.ts +++ b/baker/redirects.ts @@ -74,12 +74,19 @@ export const getRedirects = async (knex: db.KnexReadonlyTransaction) => { } export const getGrapherRedirectsMap = async ( + knex: db.KnexReadonlyTransaction, urlPrefix: string = "/grapher/" ) => { - const chartRedirectRows = (await db.queryMysql(`-- sql + const chartRedirectRows = (await db.knexRaw<{ + oldSlug: string + newSlug: string + }>( + knex, + `-- sql SELECT chart_slug_redirects.slug as oldSlug, charts.config ->> "$.slug" as newSlug FROM chart_slug_redirects INNER JOIN charts ON charts.id=chart_id - `)) as Array<{ oldSlug: string; newSlug: string }> + ` + )) as Array<{ oldSlug: string; newSlug: string }> return new Map( chartRedirectRows @@ -104,7 +111,7 @@ export const getGrapherAndWordpressRedirectsMap = memoize( // source: pathnames only (e.g. /transport) // target: pathnames with or without origins (e.g. /transport-new or https://ourworldindata.org/transport-new) - const grapherRedirects = await getGrapherRedirectsMap() + const grapherRedirects = await getGrapherRedirectsMap(knex) const wordpressRedirects = await getWordpressRedirectsMap(knex) // The order the redirects are added to the map is important. Adding the diff --git a/baker/siteRenderers.tsx b/baker/siteRenderers.tsx index 0d1ee4aa1ea..8b7109ce8e8 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -58,7 +58,7 @@ import { import { FormattingOptions, GrapherInterface } from "@ourworldindata/types" import { CountryProfileSpec } from "../site/countryProfileProjects.js" import { formatPost } from "./formatWordpressPost.js" -import { queryMysql, getHomepageId, KnexReadonlyTransaction } from "../db/db.js" +import { getHomepageId, knexRaw, KnexReadonlyTransaction } from "../db/db.js" import { getPageOverrides, isPageOverridesCitable } from "./pageOverrides.js" import { ProminentLink } from "../site/blocks/ProminentLink.js" import { @@ -101,11 +101,14 @@ export const renderToHtmlPage = (element: any) => `${ReactDOMServer.renderToStaticMarkup(element)}` export const renderChartsPage = async ( + knex: KnexReadonlyTransaction, explorerAdminServer: ExplorerAdminServer ) => { const explorers = await explorerAdminServer.getAllPublishedExplorers() - const chartItems = (await queryMysql(` + const chartItems = await knexRaw( + knex, + `-- sql SELECT id, config->>"$.slug" AS slug, @@ -116,13 +119,22 @@ export const renderChartsPage = async ( is_indexable IS TRUE AND publishedAt IS NOT NULL AND config->>"$.isPublished" = "true" - `)) as ChartIndexItem[] + ` + ) - const chartTags = await queryMysql(` + const chartTags = await knexRaw<{ + chartId: number + tagId: number + tagName: string + tagParentId: number + }>( + knex, + `-- sql SELECT ct.chartId, ct.tagId, t.name as tagName, t.parentId as tagParentId FROM chart_tags ct JOIN charts c ON c.id=ct.chartId JOIN tags t ON t.id=ct.tagId - `) + ` + ) for (const c of chartItems) { c.tags = [] @@ -144,9 +156,12 @@ export const renderChartsPage = async ( ) } -export async function renderTopChartsCollectionPage() { - const charts: string[] = await queryMysql( - ` +export async function renderTopChartsCollectionPage( + knex: KnexReadonlyTransaction +) { + const charts: string[] = await knexRaw<{ slug: string }>( + knex, + `-- sql SELECT SUBSTRING_INDEX(url, '/', -1) AS slug FROM analytics_pageviews WHERE url LIKE "%https://ourworldindata.org/grapher/%" @@ -642,7 +657,8 @@ export const renderExplorerPage = async ( type ChartRow = { id: number; config: string } let grapherConfigRows: ChartRow[] = [] if (requiredGrapherIds.length) - grapherConfigRows = await queryMysql( + grapherConfigRows = await knexRaw( + knex, `SELECT id, config FROM charts WHERE id IN (?)`, [requiredGrapherIds] ) @@ -653,7 +669,8 @@ export const renderExplorerPage = async ( grapherConfigETL: string | null }[] = [] if (requiredVariableIds.length) { - partialGrapherConfigRows = await queryMysql( + partialGrapherConfigRows = await knexRaw( + knex, `SELECT id, grapherConfigETL, grapherConfigAdmin FROM variables WHERE id IN (?)`, [requiredVariableIds] ) diff --git a/baker/startDeployQueueServer.ts b/baker/startDeployQueueServer.ts index 6b6a87c902e..775f204ab4b 100644 --- a/baker/startDeployQueueServer.ts +++ b/baker/startDeployQueueServer.ts @@ -26,8 +26,6 @@ const main = async () => { }) } - await db.getConnection() - // Listen for file changes fs.watchFile(DEPLOY_QUEUE_FILE_PATH, () => { // Start deploy after 10 seconds in order to avoid the quick successive diff --git a/baker/syncRedirectsToGrapher.ts b/baker/syncRedirectsToGrapher.ts index 1eea3449816..f27e97bb5ea 100644 --- a/baker/syncRedirectsToGrapher.ts +++ b/baker/syncRedirectsToGrapher.ts @@ -67,7 +67,6 @@ export const syncRedirectsToGrapher = async ( const main = async (): Promise => { try { - await db.getConnection() await db.knexReadWriteTransaction((trx) => syncRedirectsToGrapher(trx)) } finally { await wpdb.singleton.end() diff --git a/db/analyzeWpPosts.ts b/db/analyzeWpPosts.ts index eb0949b3d60..6a33348476b 100644 --- a/db/analyzeWpPosts.ts +++ b/db/analyzeWpPosts.ts @@ -23,46 +23,49 @@ export function traverseNode( } const analyze = async (): Promise => { - await db.getConnection() - - const posts: { id: number; content: string }[] = await db.queryMysql(` + await db.knexReadonlyTransaction(async (trx): Promise => { + const posts: { id: number; content: string }[] = await db.knexRaw( + trx, + ` SELECT id, content from posts where type<>'wp_block' - `) + ` + ) - const tagCounts = new Map() - const decideFilter = (node: CheerioElement): boolean => - node.type === "tag" && node.tagName === "iframe" + const tagCounts = new Map() + const decideFilter = (node: CheerioElement): boolean => + node.type === "tag" && node.tagName === "iframe" - for (const post of posts) { - // temp workaround for load with 3 params not showing up in TS type - const $: CheerioStatic = cheerio.load(post.content) - $("body").each((i, node) => { - traverseNode( - node, - 1, - false, - decideFilter, - (elem, depth, isFilterActive) => { - if (isFilterActive) { - const tagName = - elem.type !== "tag" - ? `${elem.type} - depth ${depth}` - : elem.tagName - const currentCount = tagCounts.get(tagName) ?? 0 - tagCounts.set(tagName, currentCount + 1) + for (const post of posts) { + // temp workaround for load with 3 params not showing up in TS type + const $: CheerioStatic = cheerio.load(post.content) + $("body").each((i, node) => { + traverseNode( + node, + 1, + false, + decideFilter, + (elem, depth, isFilterActive) => { + if (isFilterActive) { + const tagName = + elem.type !== "tag" + ? `${elem.type} - depth ${depth}` + : elem.tagName + const currentCount = tagCounts.get(tagName) ?? 0 + tagCounts.set(tagName, currentCount + 1) + } } - } - ) - }) - } + ) + }) + } - const sortedTagCount = _.sortBy( - Array.from(tagCounts.entries()), - ([tag, _]) => tag - ) - for (const [tag, count] of sortedTagCount) { - console.log(`${tag}: ${count}`) - } + const sortedTagCount = _.sortBy( + Array.from(tagCounts.entries()), + ([tag, _]) => tag + ) + for (const [tag, count] of sortedTagCount) { + console.log(`${tag}: ${count}`) + } + }) await db.closeTypeOrmAndKnexConnections() } diff --git a/db/db.ts b/db/db.ts index cb28560df5e..dc06fdd50a8 100644 --- a/db/db.ts +++ b/db/db.ts @@ -66,9 +66,6 @@ export const queryMysql = async ( return conn.query(params ? mysql.format(queryStr, params) : queryStr) } -// For operations that modify data (TODO: handling to check query isn't used for this) -export const execute = queryMysql - // Return the first match from a mysql query export const mysqlFirst = async ( queryStr: string, diff --git a/db/exportMetadata.ts b/db/exportMetadata.ts index 12ea97d8953..3b7cd2958f2 100644 --- a/db/exportMetadata.ts +++ b/db/exportMetadata.ts @@ -25,8 +25,6 @@ const filePath = const excludeTables = ["sessions", "dataset_files", "analytics_pageviews"] async function dataExport(): Promise { - await db.getConnection() - console.log(`Exporting database structure and metadata to ${filePath}...`) // Expose password to mysqldump diff --git a/db/model/Post.ts b/db/model/Post.ts index bd21c3ff60e..693b888584e 100644 --- a/db/model/Post.ts +++ b/db/model/Post.ts @@ -264,7 +264,6 @@ const selectHomepagePosts: FilterFnPostRestApi = (post) => export const getBlogIndex = memoize( async (knex: db.KnexReadonlyTransaction): Promise => { - await db.getConnection() // side effect: ensure connection is established const gdocPosts = await GdocPost.getListedGdocPosts(knex) const wpPosts = await Promise.all( await getPostsFromSnapshots( diff --git a/db/syncPostsToGrapher.ts b/db/syncPostsToGrapher.ts index b59b28c8f2c..2ae32e68c6d 100644 --- a/db/syncPostsToGrapher.ts +++ b/db/syncPostsToGrapher.ts @@ -435,7 +435,6 @@ const syncPostsToGrapher = async ( const main = async (): Promise => { try { - await db.getConnection() await db.knexReadWriteTransaction((trx) => syncPostsToGrapher(trx)) } finally { await wpdb.singleton.end() diff --git a/db/tests/basic.test.ts b/db/tests/basic.test.ts index 7639b253bbe..d9f85680e7c 100644 --- a/db/tests/basic.test.ts +++ b/db/tests/basic.test.ts @@ -1,10 +1,8 @@ #! /usr/bin/env jest import sqlFixtures from "sql-fixtures" import { dbTestConfig } from "./dbTestConfig.js" -import { dataSource } from "./dataSource.dbtests.js" import { knex, Knex } from "knex" import { - getConnection, knexRaw, knexReadWriteTransaction, KnexReadonlyTransaction, @@ -12,7 +10,6 @@ import { knexRawFirst, knexReadonlyTransaction, } from "../db.js" -import { DataSource } from "typeorm" import { deleteUser, insertUser, updateUser } from "../model/User.js" import { ChartsTableName, @@ -23,7 +20,6 @@ import { } from "@ourworldindata/types" let knexInstance: Knex | undefined = undefined -let typeOrmConnection: DataSource | undefined = undefined beforeAll(async () => { const dataSpec = { @@ -40,21 +36,13 @@ beforeAll(async () => { knexInstance = knex(dbTestConfig) const fixturesCreator = new sqlFixtures(knexInstance) - fixturesCreator.create(dataSpec, function (err: any, _: any) { - if (err) console.error(err) - // In case you want to see results of fixture creation you can do it like below - // console.log(_.users[0].email) - }) - typeOrmConnection = await getConnection(dataSource) + await fixturesCreator.create(dataSpec) }) afterAll((done: any) => { // We leave the user in the database for other tests to use // For other cases it is good to drop any rows created in the test - void Promise.allSettled([ - typeOrmConnection?.destroy(), - knexInstance?.destroy(), - ]).then(() => done()) + void Promise.allSettled([knexInstance?.destroy()]).then(() => done()) }) test("it can query a user created in fixture via TypeORM", async () => { @@ -63,6 +51,7 @@ test("it can query a user created in fixture via TypeORM", async () => { .table(UsersTableName) .where({ email: "admin@example.com" }) .first() + expect(user).toBeTruthy() expect(user.id).toBe(1) expect(user.email).toBe("admin@example.com") }) @@ -166,14 +155,22 @@ test("knex interface", async () => { expect(usersFromRawQuery.length).toBe(2) // Check if in queries work as expected - const usersFromRawQueryWithInClause: Pick[] = + const usersFromRawQueryWithInClauseAsObj: Pick[] = await knexRaw(trx, "select * from users where email in (:emails)", { emails: [ usersFromRawQuery[0].email, usersFromRawQuery[1].email, ], }) - expect(usersFromRawQueryWithInClause.length).toBe(2) + expect(usersFromRawQueryWithInClauseAsObj.length).toBe(2) + + const usersFromRawQueryWithInClauseAsArray: Pick< + DbPlainUser, + "email" + >[] = await knexRaw(trx, "select * from users where email in (?)", [ + [usersFromRawQuery[0].email, usersFromRawQuery[1].email], + ]) + expect(usersFromRawQueryWithInClauseAsArray.length).toBe(2) await deleteUser(trx, 2) }, knexInstance) diff --git a/devTools/markdownTest/markdown.ts b/devTools/markdownTest/markdown.ts index ba3658f8f13..95f5a1f1ac2 100644 --- a/devTools/markdownTest/markdown.ts +++ b/devTools/markdownTest/markdown.ts @@ -1,12 +1,9 @@ import { closeTypeOrmAndKnexConnections, - getConnection, knexReadonlyTransaction, } from "../../db/db.js" import { getPostRawBySlug } from "../../db/model/Post.js" import { enrichedBlocksToMarkdown } from "../../db/model/Gdoc/enrichedToMarkdown.js" -import { GdocBase } from "../../db/model/Gdoc/GdocBase.js" -import { GdocPost } from "../../db/model/Gdoc/GdocPost.js" import fs from "fs-extra" diff --git a/devTools/svgTester/dump-data.ts b/devTools/svgTester/dump-data.ts index 37375152ce1..069b014d2ba 100644 --- a/devTools/svgTester/dump-data.ts +++ b/devTools/svgTester/dump-data.ts @@ -2,7 +2,10 @@ import { getPublishedGraphersBySlug } from "../../baker/GrapherImageBaker.js" -import { closeTypeOrmAndKnexConnections } from "../../db/db.js" +import { + closeTypeOrmAndKnexConnections, + knexReadonlyTransaction, +} from "../../db/db.js" import fs from "fs-extra" @@ -15,7 +18,11 @@ async function main(parsedArgs: parseArgs.ParsedArgs) { const outDir = parsedArgs["o"] ?? utils.DEFAULT_CONFIGS_DIR if (!fs.existsSync(outDir)) fs.mkdirSync(outDir) - const { graphersBySlug } = await getPublishedGraphersBySlug() + const { graphersBySlug } = await knexReadonlyTransaction( + async (trx) => { + return getPublishedGraphersBySlug(trx) + } + ) const allGraphers = [...graphersBySlug.values()] const saveJobs: utils.SaveGrapherSchemaAndDataJob[] = allGraphers.map( (grapher) => ({ config: grapher, outDir })