diff --git a/adminSiteServer/adminRouter.tsx b/adminSiteServer/adminRouter.tsx index 7f362e2402c..535bb1318c2 100644 --- a/adminSiteServer/adminRouter.tsx +++ b/adminSiteServer/adminRouter.tsx @@ -45,6 +45,8 @@ import { import { getChartConfigBySlug } from "../db/model/Chart.js" import { getVariableMetadata } from "../db/model/Variable.js" import { DbPlainDatasetFile, DbPlainDataset } from "@ourworldindata/types" +import { getRouteWithROTransaction } from "./functionalRouterHelpers.js" +import { getPlainRouteWithROTransaction } from "./plainRouterHelpers.js" // Used for rate-limiting important endpoints (login, register) to prevent brute force attacks const limiterMiddleware = ( @@ -136,13 +138,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 +160,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 +260,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 +282,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 +310,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 +335,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 22305ccc2a9..9005c61c6f5 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, 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/mockSiteRouter.tsx b/adminSiteServer/mockSiteRouter.tsx index e7302372d79..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,100 +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) => { - await db.knexReadonlyTransaction(async (knex) => { - return res.send(await renderTopChartsCollectionPage(knex)) - }) -}) +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, @@ -245,29 +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) - await db.knexReadonlyTransaction(async (trx): Promise => { +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, @@ -277,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") @@ -332,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", @@ -356,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) => @@ -376,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..01b838f8e1a --- /dev/null +++ b/adminSiteServer/plainRouterHelpers.tsx @@ -0,0 +1,106 @@ +import { NextFunction, Request, Response, Router } from "express" +import * as db from "../db/db.js" +import { next } from "@ourworldindata/utils" +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/testPageRouter.tsx b/adminSiteServer/testPageRouter.tsx index 1e5752e56d7..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,8 +623,10 @@ function EmbedVariantsTestPage( ) } -testPageRouter.get("/previews", async (req, res) => { - await db.knexReadonlyTransaction(async (trx): Promise => { +getPlainRouteWithROTransaction( + testPageRouter, + "/previews", + async (req, res, trx) => { const rows = await db.knexRaw( trx, `SELECT config FROM charts LIMIT 200` @@ -627,11 +634,13 @@ testPageRouter.get("/previews", async (req, res) => { const charts = rows.map((row: any) => JSON.parse(row.config)) res.send(renderToHtmlPage()) - }) -}) + } +) -testPageRouter.get("/embedVariants", async (req, res) => { - await db.knexReadonlyTransaction(async (trx): Promise => { +getPlainRouteWithROTransaction( + testPageRouter, + "/embedVariants", + async (req, res, trx) => { const rows = await db.knexRaw( trx, `SELECT config FROM charts WHERE id=64` @@ -647,17 +656,19 @@ testPageRouter.get("/embedVariants", async (req, res) => { /> ) ) - }) -}) + } +) -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 16f34127633..d5310a9263f 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -515,9 +515,9 @@ export const bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers = await pMap( jobs, async (job) => { - 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/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,