From a0171028e5ad9a044bcd2efe530afa9c496f8a3e Mon Sep 17 00:00:00 2001 From: Daniel Bachler Date: Fri, 1 Mar 2024 19:05:36 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20clean=20up=20posts=20access=20vi?= =?UTF-8?q?a=20knex?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteServer/apiRouter.ts | 4 +- baker/GrapherBaker.tsx | 2 +- baker/SiteBaker.tsx | 6 ++- baker/algolia/indexToAlgolia.tsx | 2 +- db/DEPRECATEDwpdb.ts | 7 +++- db/migrateWpPostsToArchieMl.ts | 2 +- db/model/Post.ts | 69 +++++++++++++++++++------------- 7 files changed, 55 insertions(+), 37 deletions(-) diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index 402b48434e4..2dffe026e30 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -2204,10 +2204,10 @@ apiRouter.get("/posts.json", async (req) => { return { posts: rows } }) -apiRouter.post("/posts/:postId/setTags", async (req, res) => { +postRouteWithRWTransaction(apiRouter, "/posts/:postId/setTags", async (req, res, trx) => { const postId = expectInt(req.params.postId) - await setTagsForPost(postId, req.body.tagIds) + await setTagsForPost(trx, postId, req.body.tagIds) return { success: true } }) diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index daed2aecd11..128bd1b4f0f 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -336,7 +336,7 @@ const renderGrapherPage = async ( : undefined const relatedCharts = post && isWordpressDBEnabled - ? await getPostRelatedCharts(post.id) + ? await getPostRelatedCharts(knex, post.id) : undefined const relatedArticles = grapher.id && isWordpressAPIEnabled diff --git a/baker/SiteBaker.tsx b/baker/SiteBaker.tsx index b5b46164749..95f1cb57fb1 100644 --- a/baker/SiteBaker.tsx +++ b/baker/SiteBaker.tsx @@ -437,7 +437,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) } @@ -472,7 +472,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 } ) diff --git a/baker/algolia/indexToAlgolia.tsx b/baker/algolia/indexToAlgolia.tsx index b48f5573273..4e841ed6612 100644 --- a/baker/algolia/indexToAlgolia.tsx +++ b/baker/algolia/indexToAlgolia.tsx @@ -98,7 +98,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( diff --git a/db/DEPRECATEDwpdb.ts b/db/DEPRECATEDwpdb.ts index a0dea29f82b..7c2b8fefe9b 100644 --- a/db/DEPRECATEDwpdb.ts +++ b/db/DEPRECATEDwpdb.ts @@ -27,6 +27,7 @@ import { FOR_SYNC_ONLY_graphqlQuery, } from "./wpdb.js" import { getFullPost } from "./model/Post.js" +import { KnexReadonlyTransaction } from "./db.js" const DEPRECATED_ENTRIES_CATEGORY_ID = 44 const DEPRECATED_OWID_API_ENDPOINT = `${WORDPRESS_URL}/wp-json/owid/v1` @@ -88,6 +89,7 @@ export const DEPRECATEDgetPosts = async ( // We might want to cache this as the network of prominent links densifies and // multiple requests to the same posts are happening. export const DEPRECATEDgetPostBySlugFromApi = async ( + trx: KnexReadonlyTransaction, slug: string ): Promise => { if (!isWordpressAPIEnabled) { @@ -96,12 +98,13 @@ export const DEPRECATEDgetPostBySlugFromApi = async ( const postApi = await FOR_SYNC_ONLY_getPostApiBySlugFromApi(slug) - return getFullPost(postApi) + return getFullPost(trx, postApi) } // the /revisions endpoint does not send back all the metadata required for // the proper rendering of the post (e.g. authors), hence the double request. export const DEPRECATEDgetLatestPostRevision = async ( + trx: KnexReadonlyTransaction, id: number ): Promise => { const type = await DEPRECATEDgetPostType(id) @@ -125,7 +128,7 @@ export const DEPRECATEDgetLatestPostRevision = async ( // and could have been modified in the sidebar.) // - authors // ... - return getFullPost({ + return getFullPost(trx, { ...postApi, content: revision.content, title: revision.title, diff --git a/db/migrateWpPostsToArchieMl.ts b/db/migrateWpPostsToArchieMl.ts index 8401c6b3db4..f34dcd034fa 100644 --- a/db/migrateWpPostsToArchieMl.ts +++ b/db/migrateWpPostsToArchieMl.ts @@ -123,7 +123,7 @@ const migrate = async (trx: db.KnexReadWriteTransaction): Promise => { const text = post.content let relatedCharts: RelatedChart[] = [] if (isEntry) { - relatedCharts = await getPostRelatedCharts(post.id) + relatedCharts = await getPostRelatedCharts(trx, post.id) } const shouldIncludeMaxAsAuthor = isPostSlugCitable(post.slug) diff --git a/db/model/Post.ts b/db/model/Post.ts index dee3b427bf1..7de49cc8f8c 100644 --- a/db/model/Post.ts +++ b/db/model/Post.ts @@ -68,32 +68,34 @@ export const getTagsByPostId = async ( } export const setTags = async ( + trx: db.KnexReadWriteTransaction, postId: number, tagIds: number[] -): Promise => - await db.transaction(async (t) => { - const tagRows = tagIds.map((tagId) => [tagId, postId]) - await t.execute(`DELETE FROM post_tags WHERE post_id=?`, [postId]) - if (tagRows.length) - await t.execute( - `INSERT INTO post_tags (tag_id, post_id) VALUES ?`, - [tagRows] - ) - }) +): Promise => { + const tagRows = tagIds.map((tagId) => [tagId, postId]) + await db.knexRaw(trx, `DELETE FROM post_tags WHERE post_id=?`, [postId]) + if (tagRows.length) + await db.knexRaw( + trx, + `INSERT INTO post_tags (tag_id, post_id) VALUES ?`, + [tagRows] + ) +} export const setTagsForPost = async ( + trx: db.KnexReadWriteTransaction, postId: number, tagIds: number[] -): Promise => - await db.transaction(async (t) => { - const tagRows = tagIds.map((tagId) => [tagId, postId]) - await t.execute(`DELETE FROM post_tags WHERE post_id=?`, [postId]) - if (tagRows.length) - await t.execute( - `INSERT INTO post_tags (tag_id, post_id) VALUES ?`, - [tagRows] - ) - }) +): Promise => { + const tagRows = tagIds.map((tagId) => [tagId, postId]) + await db.knexRaw(trx, `DELETE FROM post_tags WHERE post_id=?`, [postId]) + if (tagRows.length) + await db.knexRaw( + trx, + `INSERT INTO post_tags (tag_id, post_id) VALUES ?`, + [tagRows] + ) +} export const getPostRawBySlug = async ( trx: db.KnexReadonlyTransaction, @@ -136,7 +138,7 @@ export const getFullPostBySlugFromSnapshot = async ( ) throw new JsonError(`No page snapshot found by slug ${slug}`, 404) - return getFullPost(postEnriched.wpApiSnapshot) + return getFullPost(trx, postEnriched.wpApiSnapshot) } export const getFullPostByIdFromSnapshot = async ( @@ -150,9 +152,11 @@ export const getFullPostByIdFromSnapshot = async ( ) throw new JsonError(`No page snapshot found by id ${id}`, 404) - return getFullPost(postEnriched.wpApiSnapshot) + return getFullPost(trx, postEnriched.wpApiSnapshot) } +// TODO: I suggest that in the place where we define SiteNavigationStatic we create a Set with all the leaves and +// then this one becomes a simple lookup in the set. Probably nicest to do the set creation as a memoized function. export const isPostSlugCitable = (slug: string): boolean => { const entries = SiteNavigationStatic.categories return entries.some((category) => { @@ -204,9 +208,12 @@ export const getPostsFromSnapshots = async ( } export const getPostRelatedCharts = async ( + trx: db.KnexReadonlyTransaction, postId: number ): Promise => - db.queryMysql(` + db.knexRaw( + trx, + `-- sql SELECT DISTINCT charts.config->>"$.slug" AS slug, charts.config->>"$.title" AS title, @@ -218,9 +225,11 @@ export const getPostRelatedCharts = async ( WHERE post_tags.post_id=${postId} AND charts.config->>"$.isPublished" = "true" ORDER BY title ASC - `) + ` + ) export const getFullPost = async ( + trx: db.KnexReadonlyTransaction, postApi: PostRestApi, excludeContent?: boolean ): Promise => ({ @@ -243,7 +252,8 @@ export const getFullPost = async ( imageId: postApi.featured_media, relatedCharts: postApi.type === "page" - ? await getPostRelatedCharts(postApi.id) + ? // TODO: Instead of the nested await I think we should fetch the related charts via a join and pass them into this function already joined + await getPostRelatedCharts(trx, postApi.id) : undefined, }) @@ -259,7 +269,10 @@ export const getBlogIndex = memoize( knex, [WP_PostType.Post], selectHomepagePosts - ).then((posts) => posts.map((post) => getFullPost(post, true))) + // TODO: consider doing this as a join instead of a 1+N query + ).then((posts) => + posts.map((post) => getFullPost(knex, post, true)) + ) ) const gdocSlugs = new Set(gdocPosts.map(({ slug }) => slug)) @@ -318,7 +331,7 @@ export const getWordpressPostReferencesByChartId = async ( ): Promise => { const relatedWordpressPosts: PostReference[] = await db.knexRaw( knex, - ` + `-- sql SELECT DISTINCT p.title, p.slug, @@ -366,7 +379,7 @@ export const getGdocsPostReferencesByChartId = async ( ): Promise => { const relatedGdocsPosts: PostReference[] = await db.knexRaw( knex, - ` + `-- sql SELECT DISTINCT pg.content ->> '$.title' AS title, pg.slug AS slug,