Skip to content

Commit

Permalink
🔨 incoprorate PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
danyx23 committed Sep 3, 2024
1 parent e52e23f commit 38489b5
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 29 deletions.
12 changes: 12 additions & 0 deletions functions/_common/env.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export interface Env {
ASSETS: {
fetch: typeof fetch
}
url: URL
GRAPHER_CONFIG_R2_BUCKET_URL: string
GRAPHER_CONFIG_R2_BUCKET_FALLBACK_URL: string
GRAPHER_CONFIG_R2_BUCKET_PATH: string
GRAPHER_CONFIG_R2_BUCKET_FALLBACK_PATH: string
CF_PAGES_BRANCH: string
ENV: string
}
26 changes: 19 additions & 7 deletions functions/_common/grapherRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import LatoRegular from "../_common/fonts/LatoLatin-Regular.ttf.bin"
import LatoMedium from "../_common/fonts/LatoLatin-Medium.ttf.bin"
import LatoBold from "../_common/fonts/LatoLatin-Bold.ttf.bin"
import PlayfairSemiBold from "../_common/fonts/PlayfairDisplayLatin-SemiBold.ttf.bin"
import { Env } from "../grapher/thumbnail/[slug].js"
import { Env } from "./env.js"

declare global {
// eslint-disable-next-line no-var
Expand Down Expand Up @@ -164,11 +164,11 @@ interface FetchGrapherConfigResult {
etag: string | undefined
}

export async function fetchGrapherConfig(
export async function fetchUnparsedGrapherConfig(
slug: string,
env: Env,
etag: string
): Promise<FetchGrapherConfigResult> {
etag?: string
) {
// The top level directory is either the bucket path (should be set in dev environments and production)
// or the branch name on preview staging environments
console.log("branch", env.CF_PAGES_BRANCH)
Expand Down Expand Up @@ -205,16 +205,28 @@ export async function fetchGrapherConfig(
}

// Fetch grapher config
const fetchResponse = await fetchFromR2(requestUrl, undefined, fallbackUrl)
return fetchFromR2(requestUrl, etag, fallbackUrl)
}

export async function fetchGrapherConfig(
slug: string,
env: Env,
etag?: string
): Promise<FetchGrapherConfigResult> {
const fetchResponse = await fetchUnparsedGrapherConfig(slug, env, etag)

if (fetchResponse.status !== 200) {
console.log("Failed to fetch grapher config", fetchResponse.status)
console.log(
"Status code is not 200, returning empty response with status code",
fetchResponse.status
)
return {
grapherConfig: null,
status: fetchResponse.status,
etag: fetchResponse.headers.get("etag"),
}
}

const grapherConfig: GrapherInterface = await fetchResponse.json()
console.log("grapher title", grapherConfig.title)
return {
Expand All @@ -232,7 +244,7 @@ async function fetchAndRenderGrapherToSvg(
) {
const grapherLogger = new TimeLogger("grapher")

const grapherConfigResponse = await fetchGrapherConfig(slug, env, etag)
const grapherConfigResponse = await fetchGrapherConfig(slug, env)

if (grapherConfigResponse.status !== 200) {
return null
Expand Down
23 changes: 14 additions & 9 deletions functions/grapher/[slug].ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Env } from "./thumbnail/[slug].js"
import { Env } from "../_common/env.js"
import {
fetchGrapherConfig,
getOptionalRedirectForSlug,
createRedirectResponse,
fetchUnparsedGrapherConfig,
} from "../_common/grapherRenderer.js"
import { IRequestStrict, Router, error, cors } from "itty-router"

Expand Down Expand Up @@ -145,7 +145,7 @@ async function handleConfigRequest(
etag: string | undefined
) {
const shouldCache = searchParams.get("nocache") === null
console.log("Preapring json response for ", slug)
console.log("Preparing json response for ", slug)
// All our grapher slugs are lowercase by convention.
// To allow incoming links that may contain uppercase characters to work, we redirect to the lowercase version.
const lowerCaseSlug = slug.toLowerCase()
Expand All @@ -157,14 +157,15 @@ async function handleConfigRequest(
)

return createRedirectResponse(
redirectSlug ? `${redirectSlug}.config.json` : lowerCaseSlug,
`${redirectSlug ?? lowerCaseSlug}.config.json`,
env.url
)
}

const grapherPageResp = await fetchGrapherConfig(slug, env, etag)
const grapherPageResp = await fetchUnparsedGrapherConfig(slug, env, etag)

if (grapherPageResp.status === 304) {
console.log("Returning 304 for ", slug)
return new Response(null, { status: 304 })
}

Expand All @@ -177,27 +178,31 @@ async function handleConfigRequest(
env
)
if (redirectSlug && redirectSlug !== slug) {
console.log("Redirecting to ", redirectSlug)
return createRedirectResponse(
`${redirectSlug}.config.json`,
env.url
)
} else {
console.log("Returning 404 for ", slug)
// Otherwise we just return the status code.
return new Response(null, { status: grapherPageResp.status })
}
}

console.log("Grapher page response", grapherPageResp.grapherConfig.title)
console.log("Returning 200 for ", slug)

const cacheControl = shouldCache
? "public, s-maxage=3600, max-age=0, must-revalidate"
: "public, s-maxage=0, max-age=0, must-revalidate"

return new Response(JSON.stringify(grapherPageResp.grapherConfig), {
//grapherPageResp.headers.set("Cache-Control", cacheControl)
return new Response(grapherPageResp.body as any, {
status: 200,
headers: {
"content-type": "application/json",
"Content-Type": "application/json",
"Cache-Control": cacheControl,
ETag: grapherPageResp.etag,
ETag: grapherPageResp.headers.get("ETag") ?? "",
},
})
}
14 changes: 1 addition & 13 deletions functions/grapher/thumbnail/[slug].ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,7 @@
import { Env } from "../../_common/env.js"
import { fetchAndRenderGrapher } from "../../_common/grapherRenderer.js"
import { IRequestStrict, Router, error } from "itty-router"

export interface Env {
ASSETS: {
fetch: typeof fetch
}
url: URL
GRAPHER_CONFIG_R2_BUCKET_URL: string
GRAPHER_CONFIG_R2_BUCKET_FALLBACK_URL: string
GRAPHER_CONFIG_R2_BUCKET_PATH: string
GRAPHER_CONFIG_R2_BUCKET_FALLBACK_PATH: string
CF_PAGES_BRANCH: string
ENV: string
}

const router = Router<IRequestStrict, [URL, Env, ExecutionContext]>()
router
.get(
Expand Down

0 comments on commit 38489b5

Please sign in to comment.