Skip to content

Commit

Permalink
refactor: create helpers to save chart configs to DB and R2 simultane…
Browse files Browse the repository at this point in the history
…ously (#4189)

* refactor: create helper file for common chart config writes

* refactor: adapt multidim to use helper methods

* refactor: use helpers in `apiRouter`

* refactor: use better types in multidim

* enhance: address review comments
  • Loading branch information
marcelgerber authored Nov 20, 2024
1 parent 53e4191 commit 844a20b
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 170 deletions.
124 changes: 37 additions & 87 deletions adminSiteServer/apiRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ import {
VariableAnnotationsResponseRow,
} from "../adminShared/AdminSessionTypes.js"
import {
Base64String,
DbPlainDatasetTag,
GrapherInterface,
OwidGdocType,
Expand Down Expand Up @@ -106,6 +105,7 @@ import {
MultiDimDataPageConfigRaw,
R2GrapherConfigDirectory,
ChartConfigsTableName,
Base64String,
} from "@ourworldindata/types"
import { uuidv7 } from "uuidv7"
import {
Expand Down Expand Up @@ -180,12 +180,15 @@ import path from "path"
import {
deleteGrapherConfigFromR2,
deleteGrapherConfigFromR2ByUUID,
saveGrapherConfigToR2,
saveGrapherConfigToR2ByUUID,
} from "./chartConfigR2Helpers.js"
import { fetchImagesFromDriveAndSyncToS3 } from "../db/model/Image.js"
import { createMultiDimConfig } from "./multiDim.js"
import { isMultiDimDataPagePublished } from "../db/model/MultiDimDataPage.js"
import {
retrieveChartConfigFromDbAndSaveToR2,
updateChartConfigInDbAndR2,
} from "./chartConfigHelpers.js"

const apiRouter = new FunctionalRouter()

Expand Down Expand Up @@ -322,7 +325,11 @@ const saveNewChart = async (
// new charts inherit by default
shouldInherit = true,
}: { config: GrapherInterface; user: DbPlainUser; shouldInherit?: boolean }
): Promise<{ patchConfig: GrapherInterface; fullConfig: GrapherInterface }> => {
): Promise<{
chartConfigId: Base64String
patchConfig: GrapherInterface
fullConfig: GrapherInterface
}> => {
// grab the parent of the chart if inheritance should be enabled
const parent = shouldInherit
? await getParentByChartConfig(knex, config)
Expand All @@ -331,10 +338,11 @@ const saveNewChart = async (
// compute patch and full configs
const patchConfig = diffGrapherConfigs(config, parent?.config ?? {})
const fullConfig = mergeGrapherConfigs(parent?.config ?? {}, patchConfig)
const fullConfigStringified = serializeChartConfig(fullConfig)

// insert patch & full configs into the chart_configs table
const chartConfigId = uuidv7()
// We can't quite use `saveNewChartConfigInDbAndR2` here, because
// we need to update the chart id in the config after inserting it.
const chartConfigId = uuidv7() as Base64String
await db.knexRaw(
knex,
`-- sql
Expand All @@ -344,7 +352,7 @@ const saveNewChart = async (
[
chartConfigId,
serializeChartConfig(patchConfig),
fullConfigStringified,
serializeChartConfig(fullConfig),
]
)

Expand Down Expand Up @@ -375,25 +383,9 @@ const saveNewChart = async (
[chartId, chartId, chartId]
)

// We need to get the full config and the md5 hash from the database instead of
// computing our own md5 hash because MySQL normalizes JSON and our
// client computed md5 would be different from the ones computed by and stored in R2
const fullConfigMd5 = await db.knexRawFirst<
Pick<DbRawChartConfig, "full" | "fullMd5">
>(
knex,
`-- sql
select full, fullMd5 from chart_configs where id = ?`,
[chartConfigId]
)

await saveGrapherConfigToR2ByUUID(
chartConfigId,
fullConfigMd5!.full,
fullConfigMd5!.fullMd5 as Base64String
)
await retrieveChartConfigFromDbAndSaveToR2(knex, chartConfigId)

return { patchConfig, fullConfig }
return { chartConfigId, patchConfig, fullConfig }
}

const updateExistingChart = async (
Expand All @@ -406,7 +398,11 @@ const updateExistingChart = async (
// if true or false, enable or disable inheritance
shouldInherit?: boolean
}
): Promise<{ patchConfig: GrapherInterface; fullConfig: GrapherInterface }> => {
): Promise<{
chartConfigId: Base64String
patchConfig: GrapherInterface
fullConfig: GrapherInterface
}> => {
const { config, user, chartId } = params

// make sure that the id of the incoming config matches the chart id
Expand All @@ -423,36 +419,21 @@ const updateExistingChart = async (
// compute patch and full configs
const patchConfig = diffGrapherConfigs(config, parent?.config ?? {})
const fullConfig = mergeGrapherConfigs(parent?.config ?? {}, patchConfig)
const fullConfigStringified = serializeChartConfig(fullConfig)

const chartConfigId = await db.knexRawFirst<Pick<DbPlainChart, "configId">>(
knex,
`SELECT configId FROM charts WHERE id = ?`,
[chartId]
)
const chartConfigIdRow = await db.knexRawFirst<
Pick<DbPlainChart, "configId">
>(knex, `SELECT configId FROM charts WHERE id = ?`, [chartId])

if (!chartConfigId)
if (!chartConfigIdRow)
throw new JsonError(`No chart config found for id ${chartId}`, 404)

const now = new Date()

// update configs
await db.knexRaw(
const { chartConfigId } = await updateChartConfigInDbAndR2(
knex,
`-- sql
UPDATE chart_configs
SET
patch=?,
full=?,
updatedAt=?
WHERE id = ?
`,
[
serializeChartConfig(patchConfig),
fullConfigStringified,
now,
chartConfigId.configId,
]
chartConfigIdRow.configId as Base64String,
patchConfig,
fullConfig
)

// update charts row
Expand All @@ -466,25 +447,7 @@ const updateExistingChart = async (
[shouldInherit, now, now, user.id, chartId]
)

// We need to get the full config and the md5 hash from the database instead of
// computing our own md5 hash because MySQL normalizes JSON and our
// client computed md5 would be different from the ones computed by and stored in R2
const fullConfigMd5 = await db.knexRawFirst<
Pick<DbRawChartConfig, "full" | "fullMd5">
>(
knex,
`-- sql
select full, fullMd5 from chart_configs where id = ?`,
[chartConfigId.configId]
)

await saveGrapherConfigToR2ByUUID(
chartConfigId.configId,
fullConfigMd5!.full,
fullConfigMd5!.fullMd5 as Base64String
)

return { patchConfig, fullConfig }
return { chartConfigId, patchConfig, fullConfig }
}

const saveGrapher = async (
Expand Down Expand Up @@ -593,6 +556,7 @@ const saveGrapher = async (

// Execute the actual database update or creation
let chartId: number
let chartConfigId: Base64String
let patchConfig: GrapherInterface
let fullConfig: GrapherInterface
if (existingConfig) {
Expand All @@ -603,6 +567,7 @@ const saveGrapher = async (
chartId,
shouldInherit,
})
chartConfigId = configs.chartConfigId
patchConfig = configs.patchConfig
fullConfig = configs.fullConfig
} else {
Expand All @@ -611,6 +576,7 @@ const saveGrapher = async (
user,
shouldInherit,
})
chartConfigId = configs.chartConfigId
patchConfig = configs.patchConfig
fullConfig = configs.fullConfig
chartId = fullConfig.id!
Expand Down Expand Up @@ -660,26 +626,10 @@ const saveGrapher = async (
)

if (fullConfig.isPublished) {
// We need to get the full config and the md5 hash from the database instead of
// computing our own md5 hash because MySQL normalizes JSON and our
// client computed md5 would be different from the ones computed by and stored in R2
const fullConfigMd5 = await db.knexRawFirst<
Pick<DbRawChartConfig, "full" | "fullMd5">
>(
knex,
`-- sql
select cc.full, cc.fullMd5 from chart_configs cc
join charts c on c.configId = cc.id
where c.id = ?`,
[chartId]
)

await saveGrapherConfigToR2(
fullConfigMd5!.full,
R2GrapherConfigDirectory.publishedGrapherBySlug,
`${fullConfig.slug}.json`,
fullConfigMd5!.fullMd5 as Base64String
)
await retrieveChartConfigFromDbAndSaveToR2(knex, chartConfigId, {
directory: R2GrapherConfigDirectory.publishedGrapherBySlug,
filename: `${fullConfig.slug}.json`,
})
}

if (
Expand Down
102 changes: 102 additions & 0 deletions adminSiteServer/chartConfigHelpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import {
Base64String,
ChartConfigsTableName,
DbInsertChartConfig,
DbRawChartConfig,
GrapherInterface,
R2GrapherConfigDirectory,
serializeChartConfig,
} from "@ourworldindata/types"
import { uuidv7 } from "uuidv7"
import * as db from "../db/db.js"
import {
saveGrapherConfigToR2,
saveGrapherConfigToR2ByUUID,
} from "./chartConfigR2Helpers.js"

/**
* One particular detail of of MySQL's JSON support is that MySQL _normalizes_ JSON when storing it.
* This means that the JSON string representation of a JSON object stored in MySQL is not equivalent
* to the input of an INSERT statement: it may have different whitespace and key order.
* This is a problem when we compute MD5 hashes of JSON objects using computed MySQL columns - in
* order to get the correct hash, we need to first store the JSON object in MySQL and then retrieve
* it and its hash again from MySQL immediately afterwards, such that we can store the exact same
* JSON representation and hash in R2 also.
* The below is a helper function that does just this.
* - @marcelgerber, 2024-11-20
*/

export const retrieveChartConfigFromDbAndSaveToR2 = async (
knex: db.KnexReadonlyTransaction,
chartConfigId: Base64String,
r2Path?: { directory: R2GrapherConfigDirectory; filename: string }
) => {
// We need to get the full config and the md5 hash from the database instead of
// computing our own md5 hash because MySQL normalizes JSON and our
// client computed md5 would be different from the ones computed by and stored in R2
const fullConfigMd5: Pick<DbRawChartConfig, "full" | "fullMd5"> =
await knex(ChartConfigsTableName)
.select("full", "fullMd5")
.where({ id: chartConfigId })
.first()

if (!fullConfigMd5)
throw new Error(
`Chart config not found in the database! id=${chartConfigId}`
)

if (!r2Path) {
await saveGrapherConfigToR2ByUUID(
chartConfigId,
fullConfigMd5.full,
fullConfigMd5.fullMd5 as Base64String
)
} else {
await saveGrapherConfigToR2(
fullConfigMd5.full,
r2Path.directory,
r2Path.filename,
fullConfigMd5.fullMd5 as Base64String
)
}

return {
chartConfigId,
fullConfig: fullConfigMd5.full,
fullConfigMd5: fullConfigMd5.fullMd5,
}
}

export const updateChartConfigInDbAndR2 = async (
knex: db.KnexReadWriteTransaction,
chartConfigId: Base64String,
patchConfig: GrapherInterface,
fullConfig: GrapherInterface
) => {
await knex<DbInsertChartConfig>(ChartConfigsTableName)
.update({
patch: serializeChartConfig(patchConfig),
full: serializeChartConfig(fullConfig),
updatedAt: new Date(), // It's not updated automatically in the DB.
})
.where({ id: chartConfigId })

return retrieveChartConfigFromDbAndSaveToR2(knex, chartConfigId)
}

export const saveNewChartConfigInDbAndR2 = async (
knex: db.KnexReadWriteTransaction,
chartConfigId: Base64String | undefined,
patchConfig: GrapherInterface,
fullConfig: GrapherInterface
) => {
chartConfigId = chartConfigId ?? (uuidv7() as Base64String)

await knex<DbInsertChartConfig>(ChartConfigsTableName).insert({
id: chartConfigId,
patch: serializeChartConfig(patchConfig),
full: serializeChartConfig(fullConfig),
})

return retrieveChartConfigFromDbAndSaveToR2(knex, chartConfigId)
}
Loading

0 comments on commit 844a20b

Please sign in to comment.