Skip to content

Commit

Permalink
🔨 further changes to use custom transaction types, fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
danyx23 committed Mar 6, 2024
1 parent 2b1b0b9 commit 1fa4803
Show file tree
Hide file tree
Showing 20 changed files with 259 additions and 196 deletions.
8 changes: 6 additions & 2 deletions adminSiteServer/mockSiteRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,15 @@ mockSiteRouter.get("/atom-no-topic-pages.xml", async (req, res) => {
})

mockSiteRouter.get("/entries-by-year", async (req, res) =>
res.send(await entriesByYearPage())
res.send(await db.knexReadonlyTransaction((trx) => entriesByYearPage(trx)))
)

mockSiteRouter.get(`/entries-by-year/:year`, async (req, res) =>
res.send(await entriesByYearPage(parseInt(req.params.year)))
res.send(
await db.knexReadonlyTransaction((trx) =>
entriesByYearPage(trx, parseInt(req.params.year))
)
)
)

mockSiteRouter.get(
Expand Down
12 changes: 8 additions & 4 deletions baker/GrapherBaker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ export async function renderDataPageV2(
citation,
}
} else {
const post = await getPostEnrichedBySlug(slug)
const post = await getPostEnrichedBySlug(knex, slug)
if (post) {
const authors = post.authors
const citation = getShortPageCitation(
Expand Down Expand Up @@ -331,10 +331,14 @@ const renderGrapherPage = async (
knex: db.KnexReadonlyTransaction
) => {
const postSlug = urlToSlug(grapher.originUrl || "")
const post = postSlug ? await getPostEnrichedBySlug(postSlug) : undefined
const relatedCharts = post ? await getPostRelatedCharts(post.id) : undefined
const post = postSlug
? await getPostEnrichedBySlug(knex, postSlug)
: undefined
const relatedCharts = post
? await getPostRelatedCharts(knex, post.id)
: undefined
const relatedArticles = grapher.id
? await getRelatedArticles(grapher.id, knex)
? await getRelatedArticles(knex, grapher.id)
: undefined

return renderToHtmlPage(
Expand Down
22 changes: 12 additions & 10 deletions baker/SiteBaker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ export class SiteBaker {

const postSlugs = []
for (const postApi of postsApi) {
const post = await getFullPost(postApi)
const post = await getFullPost(knex, postApi)
postSlugs.push(post.slug)
}

Expand Down Expand Up @@ -476,7 +476,9 @@ export class SiteBaker {
await pMap(
postsApi,
async (postApi) =>
getFullPost(postApi).then((post) => this.bakePost(post, knex)),
getFullPost(knex, postApi).then((post) =>
this.bakePost(post, knex)
),
{ concurrency: 10 }
)

Expand Down Expand Up @@ -774,7 +776,7 @@ export class SiteBaker {
if (!this.bakeSteps.has("googleScholar")) return
await this.stageWrite(
`${this.bakedSiteDir}/entries-by-year/index.html`,
await entriesByYearPage()
await entriesByYearPage(trx)
)

const rows = (await trx
Expand All @@ -792,7 +794,7 @@ export class SiteBaker {
for (const year of years) {
await this.stageWrite(
`${this.bakedSiteDir}/entries-by-year/${year}.html`,
await entriesByYearPage(year)
await entriesByYearPage(trx, year)
)
}

Expand Down Expand Up @@ -836,9 +838,9 @@ export class SiteBaker {

// We don't have an icon for every single tag (yet), but for the icons that we *do* have,
// we want to make sure that we have a corresponding tag in the database.
private async validateTagIcons() {
const allTags = await db
.knexTable("tags")
private async validateTagIcons(trx: db.KnexReadonlyTransaction) {
const allTags = await trx
.table("tags")
.select<{ name: string }[]>("name")
const tagNames = new Set(allTags.map((tag) => tag.name))
const tagIcons = await fs.readdir("public/images/tag-icons")
Expand All @@ -853,7 +855,7 @@ export class SiteBaker {
}

// Bake the static assets
private async bakeAssets() {
private async bakeAssets(trx: db.KnexReadonlyTransaction) {
if (!this.bakeSteps.has("assets")) return

// do not delete images/published folder so that we don't have to sync gdrive images again
Expand All @@ -866,7 +868,7 @@ export class SiteBaker {
await execWrapper(
`rm -rf ${this.bakedSiteDir}/assets && cp -r ${BASE_DIR}/dist/assets ${this.bakedSiteDir}/assets`
)
await this.validateTagIcons()
await this.validateTagIcons(trx)
await execWrapper(
`rsync -hav --delete ${BASE_DIR}/public/* ${this.bakedSiteDir}/ ${excludes}`
)
Expand Down Expand Up @@ -903,7 +905,7 @@ export class SiteBaker {
await this.bakeEmbeds(knex)
await this.bakeBlogIndex(knex)
await this.bakeRSS(knex)
await this.bakeAssets()
await this.bakeAssets(knex)
await this.bakeGoogleScholar(knex)
await this.bakePosts(knex)
}
Expand Down
2 changes: 1 addition & 1 deletion baker/algolia/indexChartsToAlgolia.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const getChartsRecords = async (
// otherwise they will fail when rendered in the search results
if (isPathRedirectedToExplorer(`/grapher/${c.slug}`)) continue

const relatedArticles = (await getRelatedArticles(c.id, knex)) ?? []
const relatedArticles = (await getRelatedArticles(knex, c.id)) ?? []
const linksFromGdocs = await Link.getPublishedLinksTo(
c.slug,
OwidGdocLinkType.Grapher
Expand Down
4 changes: 2 additions & 2 deletions baker/algolia/indexToAlgolia.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ async function generateWordpressRecords(
const records: PageRecord[] = []

for (const postApi of postsApi) {
const rawPost = await getFullPost(postApi)
const rawPost = await getFullPost(knex, postApi)
if (isEmpty(rawPost.content)) {
// we have some posts that are only placeholders (e.g. for a redirect); don't index these
console.log(
Expand All @@ -110,7 +110,7 @@ async function generateWordpressRecords(

const post = await formatPost(rawPost, { footnotes: false }, knex)
const chunks = generateChunksFromHtmlText(post.html)
const tags = await getPostTags(post.id)
const tags = await getPostTags(knex, post.id)
const postTypeAndImportance = getPostTypeAndImportance(post, tags)

let i = 0
Expand Down
25 changes: 14 additions & 11 deletions baker/countryProfiles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { countries, getCountryBySlug, JsonError } from "@ourworldindata/utils"
import { renderToHtmlPage } from "./siteRenderers.js"
import { dataAsDF } from "../db/model/Variable.js"
import pl from "nodejs-polars"
import knex from "knex"

export const countriesIndexPage = (baseUrl: string) =>
renderToHtmlPage(
Expand All @@ -41,26 +42,28 @@ const checkShouldShowIndicator = (grapher: GrapherInterface) =>

// Find the charts that will be shown on the country profile page (if they have that country)
// TODO: make this page per variable instead
const countryIndicatorGraphers = async (): Promise<GrapherInterface[]> =>
const countryIndicatorGraphers = async (
trx: db.KnexReadonlyTransaction
): Promise<GrapherInterface[]> =>
bakeCache(countryIndicatorGraphers, async () => {
const graphers = (
await db
.knexTable("charts")
await trx
.table("charts")
.whereRaw("publishedAt is not null and is_indexable is true")
).map((c: any) => JSON.parse(c.config)) as GrapherInterface[]

return graphers.filter(checkShouldShowIndicator)
})

export const countryIndicatorVariables = async (): Promise<
DbEnrichedVariable[]
> =>
export const countryIndicatorVariables = async (
trx: db.KnexReadonlyTransaction
): Promise<DbEnrichedVariable[]> =>
bakeCache(countryIndicatorVariables, async () => {
const variableIds = (await countryIndicatorGraphers()).map(
const variableIds = (await countryIndicatorGraphers(trx)).map(
(c) => c.dimensions![0]!.variableId
)
const rows: DbRawVariable[] = await db
.knexTable(VariablesTableName)
const rows: DbRawVariable[] = await trx
.table(VariablesTableName)
.whereIn("id", variableIds)
return rows.map(parseVariablesRow)
})
Expand All @@ -81,7 +84,7 @@ export const denormalizeLatestCountryData = async (
const entityIds = countries.map((c) => entitiesByCode[c.code].id)

if (!variableIds) {
variableIds = (await countryIndicatorVariables()).map((v) => v.id)
variableIds = (await countryIndicatorVariables(trx)).map((v) => v.id)

// exclude variables that are already in country_latest_data
// NOTE: we always fetch all variables that don't have country entities because they never
Expand Down Expand Up @@ -163,7 +166,7 @@ export const countryProfilePage = async (
const country = getCountryBySlug(countrySlug)
if (!country) throw new JsonError(`No such country ${countrySlug}`, 404)

const graphers = await countryIndicatorGraphers()
const graphers = await countryIndicatorGraphers(trx)
const dataValues = await countryIndicatorLatestData(trx, country.code)

const valuesByVariableId = lodash.groupBy(dataValues, (v) => v.variableId)
Expand Down
7 changes: 6 additions & 1 deletion baker/pageOverrides.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
import * as pageOverrides from "./pageOverrides.js"

import { jest } from "@jest/globals"
import { KnexReadonlyTransaction } from "../db/db.js"

const mockCreatePost = (slug: string): FullPost => {
return {
Expand All @@ -27,7 +28,7 @@ const getPostBySlugLogToSlackNoThrow = jest.spyOn(
pageOverrides,
"getPostBySlugLogToSlackNoThrow"
)
getPostBySlugLogToSlackNoThrow.mockImplementation((landingSlug) =>
getPostBySlugLogToSlackNoThrow.mockImplementation((knex, landingSlug) =>
Promise.resolve(mockCreatePost(landingSlug))
)

Expand All @@ -38,6 +39,7 @@ it("gets parent landing", async () => {

await expect(
pageOverrides.getLandingOnlyIfParent(
{} as KnexReadonlyTransaction,
mockCreatePost("forest-area"),
formattingOptions
)
Expand All @@ -51,6 +53,7 @@ it("does not get parent landing (subnavId invalid)", async () => {

await expect(
pageOverrides.getLandingOnlyIfParent(
{} as KnexReadonlyTransaction,
mockCreatePost("forest-area"),
formattingOptions
)
Expand All @@ -64,6 +67,7 @@ it("does not get parent landing (post is already a landing)", async () => {

await expect(
pageOverrides.getLandingOnlyIfParent(
{} as KnexReadonlyTransaction,
mockCreatePost(forestLandingSlug),
formattingOptions
)
Expand All @@ -81,6 +85,7 @@ it("does not get parent landing and logs (landing post not found)", async () =>

await expect(
pageOverrides.getLandingOnlyIfParent(
{} as KnexReadonlyTransaction,
mockCreatePost("forest-area"),
formattingOptions
)
Expand Down
14 changes: 10 additions & 4 deletions baker/pageOverrides.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,15 @@ import {
getFullPostBySlugFromSnapshot,
isPostSlugCitable,
} from "../db/model/Post.js"
import { KnexReadonlyTransaction } from "../db/db.js"

export const getPostBySlugLogToSlackNoThrow = async (slug: string) => {
export const getPostBySlugLogToSlackNoThrow = async (
knex: KnexReadonlyTransaction,
slug: string
) => {
let post
try {
post = await getFullPostBySlugFromSnapshot(slug)
post = await getFullPostBySlugFromSnapshot(knex, slug)
} catch (err) {
logErrorAndMaybeSendToBugsnag(err)
} finally {
Expand All @@ -21,6 +25,7 @@ export const getPostBySlugLogToSlackNoThrow = async (slug: string) => {
}

export const getLandingOnlyIfParent = async (
knex: KnexReadonlyTransaction,
post: FullPost,
formattingOptions: FormattingOptions
) => {
Expand All @@ -37,7 +42,7 @@ export const getLandingOnlyIfParent = async (
// Using no-throw version to prevent throwing and stopping baking mid-way.
// It is more desirable to temporarily deploy with citation overrides
// absent, while fixing the issue.
const landing = await getPostBySlugLogToSlackNoThrow(landingSlug)
const landing = await getPostBySlugLogToSlackNoThrow(knex, landingSlug)
if (!landing) {
// todo: the concept of "citation overrides" does not belong to that
// generic function. Logging this message should be the responsibility
Expand All @@ -58,10 +63,11 @@ export const getLandingOnlyIfParent = async (
}

export const getPageOverrides = async (
knex: KnexReadonlyTransaction,
post: FullPost,
formattingOptions: FormattingOptions
): Promise<PageOverrides | undefined> => {
const landing = await getLandingOnlyIfParent(post, formattingOptions)
const landing = await getLandingOnlyIfParent(knex, post, formattingOptions)
if (!landing) return

const isParentLandingCitable = isPostSlugCitable(landing.slug)
Expand Down
2 changes: 1 addition & 1 deletion baker/postUpdatedHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const syncPostToGrapher = async (
const dereferenceReusableBlocksFn = await buildReusableBlocksResolver()
const dereferenceTablePressFn = await buildTablePressResolver()

const matchingRows = await db.knexTable(postsTable).where({ id: postId })
const matchingRows = await knex.table(postsTable).where({ id: postId })
const existsInGrapher = !!matchingRows.length

const wpPost = rows[0]
Expand Down
Loading

0 comments on commit 1fa4803

Please sign in to comment.