✨ automatically migrate outdated configs

owid · Sep 20, 2024 · fcccc33 · fcccc33
1 parent 496aeba
commit fcccc33
Show file tree

Hide file tree

Showing 12 changed files with 378 additions and 33 deletions.
diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts
@@ -106,6 +106,7 @@ import {
 import { uuidv7 } from "uuidv7"
 import {
     defaultGrapherConfig,
+    migrateGrapherConfigToLatestVersion,
     getVariableDataRoute,
     getVariableMetadataRoute,
 } from "@ourworldindata/grapher"
@@ -493,7 +494,7 @@ const saveGrapher = async (
         referencedVariablesMightChange = true,
     }: {
         user: DbPlainUser
-        newConfig: GrapherInterface // Note that it is valid for newConfig to be of an older schema version which means that GrapherInterface as a type is slightly misleading
+        newConfig: GrapherInterface
         existingConfig?: GrapherInterface
         // if undefined, keep inheritance as is.
         // if true or false, enable or disable inheritance
@@ -579,22 +580,6 @@ const saveGrapher = async (
         newConfig.version += 1
     else newConfig.version = 1
 
-    // if the schema version is missing, assume it's the latest
-    if (newConfig.$schema === undefined) {
-        newConfig.$schema = defaultGrapherConfig.$schema
-    } else if (
-        newConfig.$schema ===
-        "https://files.ourworldindata.org/schemas/grapher-schema.004.json"
-    ) {
-        // TODO: find a more principled way to do schema upgrades
-
-        // grapher-schema.004 -> grapher-schema.005 removed the obsolete hideLinesOutsideTolerance field
-        const configForMigration = newConfig as any
-        delete configForMigration.hideLinesOutsideTolerance
-        configForMigration.$schema = defaultGrapherConfig.$schema
-        newConfig = configForMigration
-    }
-
     // add the isPublished field if is missing
     if (newConfig.isPublished === undefined) {
         newConfig.isPublished = false
@@ -1034,9 +1019,19 @@ postRouteWithRWTransaction(apiRouter, "/charts", async (req, res, trx) => {
         shouldInherit = req.query.inheritance === "enable"
     }
 
+    let validConfig: GrapherInterface
+    try {
+        validConfig = migrateGrapherConfigToLatestVersion(req.body)
+    } catch (err) {
+        return {
+            success: false,
+            error: String(err),
+        }
+    }
+
     const { chartId } = await saveGrapher(trx, {
         user: res.locals.user,
-        newConfig: req.body,
+        newConfig: validConfig,
         shouldInherit,
     })
 
@@ -1064,11 +1059,21 @@ putRouteWithRWTransaction(
             shouldInherit = req.query.inheritance === "enable"
         }
 
+        let validConfig: GrapherInterface
+        try {
+            validConfig = migrateGrapherConfigToLatestVersion(req.body)
+        } catch (err) {
+            return {
+                success: false,
+                error: String(err),
+            }
+        }
+
         const existingConfig = await expectChartById(trx, req.params.chartId)
 
         const { chartId, savedPatch } = await saveGrapher(trx, {
             user: res.locals.user,
-            newConfig: req.body,
+            newConfig: validConfig,
             existingConfig,
             shouldInherit,
         })
@@ -1609,13 +1614,23 @@ putRouteWithRWTransaction(
     async (req, res, trx) => {
         const variableId = expectInt(req.params.variableId)
 
+        let validConfig: GrapherInterface
+        try {
+            validConfig = migrateGrapherConfigToLatestVersion(req.body)
+        } catch (err) {
+            return {
+                success: false,
+                error: String(err),
+            }
+        }
+
         const variable = await getGrapherConfigsForVariable(trx, variableId)
         if (!variable) {
             throw new JsonError(`Variable with id ${variableId} not found`, 500)
         }
 
         const { savedPatch, updatedCharts } =
-            await updateGrapherConfigETLOfVariable(trx, variable, req.body)
+            await updateGrapherConfigETLOfVariable(trx, variable, validConfig)
 
         // trigger build if any published chart has been updated
         if (updatedCharts.some((chart) => chart.isPublished)) {
@@ -1700,13 +1715,23 @@ putRouteWithRWTransaction(
     async (req, res, trx) => {
         const variableId = expectInt(req.params.variableId)
 
+        let validConfig: GrapherInterface
+        try {
+            validConfig = migrateGrapherConfigToLatestVersion(req.body)
+        } catch (err) {
+            return {
+                success: false,
+                error: String(err),
+            }
+        }
+
         const variable = await getGrapherConfigsForVariable(trx, variableId)
         if (!variable) {
             throw new JsonError(`Variable with id ${variableId} not found`, 500)
         }
 
         const { savedPatch, updatedCharts } =
-            await updateGrapherConfigAdminOfVariable(trx, variable, req.body)
+            await updateGrapherConfigAdminOfVariable(trx, variable, validConfig)
 
         // trigger build if any published chart has been updated
         if (updatedCharts.some((chart) => chart.isPublished)) {

diff --git a/adminSiteServer/app.test.tsx b/adminSiteServer/app.test.tsx
@@ -53,7 +53,6 @@ import {
     DatasetsTableName,
     VariablesTableName,
 } from "@ourworldindata/types"
-import { defaultGrapherConfig } from "@ourworldindata/grapher"
 import path from "path"
 import fs from "fs"
 import { omitUndefinedValues } from "@ourworldindata/utils"
@@ -191,6 +190,8 @@ async function makeRequestAgainstAdminApi(
 
 describe("OwidAdminApp", () => {
     const testChartConfig = {
+        $schema:
+            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
         slug: "test-chart",
         title: "Test chart",
         type: "LineChart",
@@ -252,7 +253,7 @@ describe("OwidAdminApp", () => {
         )
         expect(fullConfig).toHaveProperty(
             "$schema",
-            defaultGrapherConfig.$schema
+            "https://files.ourworldindata.org/schemas/grapher-schema.005.json"
         )
         expect(fullConfig).toHaveProperty("id", chartId) // must match the db id
         expect(fullConfig).toHaveProperty("version", 1) // automatically added
@@ -267,7 +268,8 @@ describe("OwidAdminApp", () => {
             `/charts/${chartId}.patchConfig.json`
         )
         expect(patchConfig).toEqual({
-            $schema: defaultGrapherConfig["$schema"],
+            $schema:
+                "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
             id: chartId,
             version: 1,
             isPublished: false,
@@ -321,12 +323,16 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         display: '{ "unit": "kg", "shortUnit": "kg" }',
     }
     const testVariableConfigETL = {
+        $schema:
+            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
         hasMapTab: true,
         note: "Indicator note",
         selectedEntityNames: ["France", "Italy", "Spain"],
         hideRelativeToggle: false,
     }
     const testVariableConfigAdmin = {
+        $schema:
+            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
         title: "Admin title",
         subtitle: "Admin subtitle",
     }
@@ -337,10 +343,14 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         id: otherVariableId,
     }
     const otherTestVariableConfig = {
+        $schema:
+            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
         note: "Other indicator note",
     }
 
     const testChartConfig = {
+        $schema:
+            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
         slug: "test-chart",
         title: "Test chart",
         type: "Marimekko",
@@ -382,12 +392,11 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         // for ETL configs, patch and full configs should be the same
         expect(patchConfigETL).toEqual(fullConfigETL)
 
-        // check that $schema and dimensions field were added to the config
+        // check that the dimensions field were added to the config
         const processedTestVariableConfigETL = {
             ...testVariableConfigETL,
 
             // automatically added
-            $schema: defaultGrapherConfig.$schema,
             dimensions: [
                 {
                     property: "y",
@@ -503,7 +512,8 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
             `/charts/${chartId}.patchConfig.json`
         )
         expect(patchConfig).toEqual({
-            $schema: defaultGrapherConfig["$schema"],
+            $schema:
+                "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
             id: chartId,
             version: 1,
             isPublished: false,
@@ -558,7 +568,8 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
             `/charts/${chartId}.patchConfig.json`
         )
         expect(patchConfigAfterDelete).toEqual({
-            $schema: defaultGrapherConfig["$schema"],
+            $schema:
+                "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
             id: chartId,
             version: 1,
             isPublished: false,

diff --git a/db/migration/1726588731621-MigrateOutdatedConfigsToLatestVersion.ts b/db/migration/1726588731621-MigrateOutdatedConfigsToLatestVersion.ts
@@ -0,0 +1,33 @@
+import { MigrationInterface, QueryRunner } from "typeorm"
+
+export class MigrateOutdatedConfigsToLatestVersion1726588731621
+    implements MigrationInterface
+{
+    public async up(queryRunner: QueryRunner): Promise<void> {
+        // we have v3 configs in the database; turn these into v5 configs
+        // by removing the `data` and `hideLinesOutsideTolerance` properties
+        await queryRunner.query(
+            `-- sql
+              UPDATE chart_configs
+              SET
+                  patch = JSON_SET(
+                      JSON_REMOVE(patch, '$.data', '$.hideLinesOutsideTolerance'),
+                      '$.$schema',
+                      'https://files.ourworldindata.org/schemas/grapher-schema.005.json'
+                  ),
+                  full = JSON_SET(
+                      JSON_REMOVE(full, '$.data', '$.hideLinesOutsideTolerance'),
+                      '$.$schema',
+                      'https://files.ourworldindata.org/schemas/grapher-schema.005.json'
+                  )
+              WHERE patch ->> '$.$schema' = 'https://files.ourworldindata.org/schemas/grapher-schema.003.json'
+            `
+        )
+    }
+
+    public async down(): Promise<void> {
+        throw new Error(
+            "Can't revert migration MigrateOutdatedConfigsToLatestVersion1726588731621"
+        )
+    }
+}
diff --git a/db/model/Variable.ts b/db/model/Variable.ts
@@ -189,11 +189,6 @@ function makeConfigValidForIndicator({
 }): GrapherInterface {
     const updatedConfig = { ...config }
 
-    // if no schema is given, assume it's the latest
-    if (!updatedConfig.$schema) {
-        updatedConfig.$schema = defaultGrapherConfig.$schema
-    }
-
     // validate the given y-dimensions
     const defaultDimension = { property: DimensionProperty.y, variableId }
     const [yDimensions, otherDimensions] = _.partition(

diff --git a/devTools/schema/generate-default-object-from-schema.ts b/devTools/schema/generate-default-object-from-schema.ts
@@ -2,6 +2,15 @@
 
 import parseArgs from "minimist"
 import fs from "fs-extra"
+import { range } from "lodash"
+
+const schemaVersionRegex =
+    /https:\/\/files\.ourworldindata\.org\/schemas\/grapher-schema\.(?<version>\d{3})\.json/m
+const getSchemaVersion = (config: Record<string, any>): string =>
+    config.$schema?.match(schemaVersionRegex)?.groups?.version ?? "000"
+
+const toArrayString = (arr: string[]) =>
+    `[${arr.map((v) => `"${v}"`).join(", ")}]`
 
 function generateDefaultObjectFromSchema(
     schema: Record<string, any>,
@@ -49,13 +58,22 @@ async function main(parsedArgs: parseArgs.ParsedArgs) {
 
     // save as ts file if requested
     if (parsedArgs["save-ts"]) {
+        const latestVersion = getSchemaVersion(defaultConfig)
+        const outdatedVersionsAsInts = range(1, parseInt(latestVersion))
+        const outdatedVersions = outdatedVersionsAsInts.map((versionNumber) =>
+            versionNumber.toString().padStart(3, "0")
+        )
+
         const out = parsedArgs["save-ts"]
         const content = `// THIS IS A GENERATED FILE, DO NOT EDIT DIRECTLY
 
 // GENERATED BY devTools/schema/generate-default-object-from-schema.ts
 
 import { GrapherInterface } from "@ourworldindata/types"
 
+export const latestSchemaVersion = "${latestVersion}" as const
+export const outdatedSchemaVersions = ${toArrayString(outdatedVersions)} as const
+
 export const defaultGrapherConfig = ${defaultConfigJSON} as GrapherInterface`
         fs.outputFileSync(out, content)
     }

diff --git a/packages/@ourworldindata/grapher/src/index.ts b/packages/@ourworldindata/grapher/src/index.ts
@@ -80,3 +80,4 @@ export {
     SlideShowController,
 } from "./slideshowController/SlideShowController"
 export { defaultGrapherConfig } from "./schema/defaultGrapherConfig"
+export { migrateGrapherConfigToLatestVersion } from "./schema/migrations/migrate"
diff --git a/packages/@ourworldindata/grapher/src/schema/README.md b/packages/@ourworldindata/grapher/src/schema/README.md
@@ -19,6 +19,7 @@ Checklist for breaking changes:
 -   Write the migrations from the last to the current version of the schema, including the migration of pointing to the URL of the new schema version
 -   Regenerate the default object from the schema and save it to `defaultGrapherConfig.ts` (see below)
 -   Write a migration to update the `chart_configs.full` column in the database for all stand-alone charts
+-   Write a migration to update configs in code (see `migrations/migrations.ts`)
 
 To regenerate `defaultGrapherConfig.ts` from the schema, replace `XXX` with the current schema version number and run:
 

diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts
@@ -4,6 +4,9 @@
 
 import { GrapherInterface } from "@ourworldindata/types"
 
+export const latestSchemaVersion = "005" as const
+export const outdatedSchemaVersions = ["001", "002", "003", "004"] as const
+
 export const defaultGrapherConfig = {
     $schema: "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
     map: {