Skip to content

Commit 67360f5

Browse files
rachmarirachmari
authored
refactor rest code (github#25879)
1 parent 142ffd0 commit 67360f5

File tree

4 files changed

+275
-271
lines changed

4 files changed

+275
-271
lines changed

‎lib/rest/index.js

Lines changed: 71 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import { fileURLToPath } from 'url'
22
import path from 'path'
3-
import fs from 'fs'
43
import { readCompressedJsonFileFallback } from '../read-json-file.js'
54
import renderContent from '../render-content/index.js'
65
import getMiniTocItems from '../get-mini-toc-items.js'
76
import { allVersions } from '../all-versions.js'
8-
import { get } from 'lodash-es'
7+
import languages from '../languages.js'
98

109
const __dirname = path.dirname(fileURLToPath(import.meta.url))
1110
const schemasPath = path.join(__dirname, 'static/decorated')
1211
const ENABLED_APPS_FILENAME = path.join(__dirname, 'static/apps/enabled-for-apps.json')
13-
/*
12+
/*
1413
Loads the schemas from the static/decorated folder into a single
1514
object organized by version.
1615
@@ -23,23 +22,33 @@ const ENABLED_APPS_FILENAME = path.join(__dirname, 'static/apps/enabled-for-apps
2322
}
2423
}
2524
*/
26-
export default async function getRest() {
27-
const operations = {}
28-
fs.readdirSync(schemasPath).forEach((filename) => {
29-
// In staging deploys, the `.json` files might have been converted to
30-
// to `.json.br`. In that case, we need to also remove the `.json`
31-
// extension from the key
32-
const key = path.parse(filename).name.replace('.json', '')
25+
const restOperationData = new Map()
26+
Object.keys(languages).forEach((language) => {
27+
restOperationData.set(language, new Map())
28+
Object.keys(allVersions).forEach((version) => {
29+
// setting to undefined will allow us to perform checks
30+
// more easily later on
31+
restOperationData.get(language).set(version, new Map())
32+
})
33+
})
34+
35+
const restOperations = new Map()
36+
export default async function getRest(version, category) {
37+
const openApiVersion = getOpenApiVersion(version)
38+
if (!restOperations.has(openApiVersion)) {
39+
const filename = `${openApiVersion}.json`
40+
3341
// The `readCompressedJsonFileFallback()` function
3442
// will check for both a .br and .json extension.
35-
const value = readCompressedJsonFileFallback(path.join(schemasPath, filename))
36-
const docsVersions = getDocsVersions(key)
37-
docsVersions.forEach((docsVersion) => {
38-
operations[docsVersion] = value
39-
})
40-
})
43+
restOperations.set(
44+
openApiVersion,
45+
readCompressedJsonFileFallback(path.join(schemasPath, filename))
46+
)
47+
}
4148

42-
return operations
49+
return category
50+
? restOperations.get(openApiVersion)[category]
51+
: restOperations.get(openApiVersion)
4352
}
4453

4554
// Right now there is not a 1:1 mapping of openapi to docs versions,
@@ -52,48 +61,57 @@ function getDocsVersions(openApiVersion) {
5261
return versions
5362
}
5463

55-
// Used for units tests
56-
export function getFlatListOfOperations(operations) {
57-
const flatList = []
58-
Object.keys(operations).forEach((version) => {
59-
Object.keys(operations[version]).forEach((category) => {
60-
Object.keys(operations[version][category]).forEach((subcategory) => {
61-
flatList.push(...operations[version][category][subcategory])
62-
})
63-
})
64-
})
65-
return flatList
64+
function getOpenApiVersion(version) {
65+
if (!(version in allVersions)) {
66+
throw new Error(`Unrecognized version '${version}'. Not found in ${Object.keys(allVersions)}`)
67+
}
68+
return allVersions[version].openApiVersionName
6669
}
6770

6871
// Generates the rendered Markdown from the data files in the
6972
// data/reusables/rest-reference directory for a given category
7073
// and generates the miniToc for a rest reference page.
71-
export async function getRestOperationData(category, categoryOperations, context) {
72-
const descriptions = {}
73-
let toc = ''
74-
const reusablePath = context.site.data.reusables
75-
const subcategories = Object.keys(categoryOperations)
76-
let introContent = null
77-
for (const subcategory of subcategories) {
78-
const markdown = get(reusablePath['rest-reference'][category], subcategory)
79-
const renderedMarkdown = await renderContent(markdown, context)
80-
descriptions[subcategory] = renderedMarkdown
81-
// only a string with the raw HTML of each heading level 2 and 3
82-
// is needed to generate the toc
83-
const titles = categoryOperations[subcategory]
84-
.map((operation) => `### ${operation.summary}\n`)
85-
.join('')
86-
toc += renderedMarkdown + (await renderContent(titles, context))
87-
}
88-
// This is Markdown content at the path
89-
// data/reusables/rest-reference/<category>/<subcategory>
90-
// that doesn't map directory to a group of operations.
91-
if (get(reusablePath['rest-reference'][category], category) && !categoryOperations[category]) {
92-
const markdown = get(reusablePath['rest-reference'][category], category)
93-
introContent = await renderContent(markdown, context)
74+
export async function getRestOperationData(category, language, version, context) {
75+
if (!restOperationData.get(language).get(version).has(category)) {
76+
const languageTree = restOperationData.get(language)
77+
const descriptions = {}
78+
let toc = ''
79+
const reusablePath = context.site.data.reusables
80+
const categoryOperations = await getRest(version, category)
81+
const subcategories = Object.keys(categoryOperations)
82+
let introContent = null
83+
for (const subcategory of subcategories) {
84+
const markdown = reusablePath['rest-reference']?.[category]?.[subcategory]
85+
const renderedMarkdown = await renderContent(markdown, context)
86+
descriptions[subcategory] = renderedMarkdown
87+
// only a string with the raw HTML of each heading level 2 and 3
88+
// is needed to generate the toc
89+
const titles = categoryOperations[subcategory]
90+
.map((operation) => `### ${operation.summary}\n`)
91+
.join('')
92+
toc += renderedMarkdown + (await renderContent(titles, context))
93+
}
94+
// Usually a Markdown file in
95+
// data/resuables/rest-reference/<category>/<subcategory>
96+
// will always map to a set of operations. But sometimes we have
97+
// introductory text that doesn't map to any operations.
98+
// When that is the case, the category and subcategory are the same.
99+
// Example data/resuables/rest-reference/actions/actions
100+
// The content in this file is called introContent and is displayed
101+
// at the top of the rest reference page.
102+
if (reusablePath['rest-reference']?.[category]?.[category] && !categoryOperations[category]) {
103+
const markdown = reusablePath['rest-reference']?.[category]?.[category]
104+
introContent = await renderContent(markdown, context)
105+
}
106+
const miniTocItems = getMiniTocItems(toc, 3)
107+
languageTree.get(version).set(category, {
108+
descriptions,
109+
miniTocItems,
110+
introContent,
111+
})
112+
restOperationData.set(restOperationData, languageTree)
94113
}
95-
const miniTocItems = getMiniTocItems(toc, 3)
96-
return { descriptions, miniTocItems, introContent }
114+
return restOperationData.get(language).get(version).get(category)
97115
}
98116

99117
export async function getEnabledForApps() {

‎pages/[versionId]/rest/reference/[category].tsx

Lines changed: 10 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,6 @@ import { RestCategoryOperationsT } from 'components/rest/types'
55
import { MiniTocItem } from 'components/context/ArticleContext'
66
import { RestReferencePage } from 'components/rest/RestReferencePage'
77

8-
type RestOperationsT = {
9-
[version: string]: {
10-
[category: string]: RestCategoryOperationsT
11-
}
12-
}
13-
148
type CategoryDataT = {
159
descriptions: {
1610
[subcategory: string]: string
@@ -19,14 +13,6 @@ type CategoryDataT = {
1913
introContent: string
2014
}
2115

22-
type RestDataT = {
23-
[language: string]: {
24-
[version: string]: {
25-
[category: string]: CategoryDataT
26-
}
27-
}
28-
}
29-
3016
type Props = {
3117
mainContext: MainContextT
3218
restOperations: RestCategoryOperationsT
@@ -35,9 +21,6 @@ type Props = {
3521
introContent: string
3622
}
3723

38-
let rest: RestOperationsT | null = null
39-
let restOperationData: RestDataT | null = null
40-
4124
export default function Category({
4225
mainContext,
4326
restOperations,
@@ -65,48 +48,25 @@ export const getServerSideProps: GetServerSideProps<Props> = async (context) =>
6548
const currentVersion = context.params!.versionId as string
6649
const currentLanguage = req.context.currentLanguage as string
6750

68-
// Use a local cache to store all of the REST operations, so
69-
// we only read the directory of static/decorated files once
70-
if (!rest) {
71-
rest = (await getRest()) as RestOperationsT
72-
}
73-
74-
/* This sets up a skeleton object in the format:
75-
{
76-
'en': { free-pro-team@latest: {}, enterprise-cloud@latest: {}},
77-
'ja': { free-pro-team@latest: {}, enterprise-cloud@latest: {}}
78-
}
79-
*/
80-
if (!restOperationData) {
81-
restOperationData = {}
82-
Object.keys(req.context.languages).forEach((language) => {
83-
restOperationData![language] = {}
84-
Object.keys(req.context.allVersions).forEach(
85-
(version) => (restOperationData![language][version] = {})
86-
)
87-
})
88-
}
89-
90-
const restOperations = rest[currentVersion][category]
51+
const restOperations = await getRest(currentVersion, category)
9152

9253
// The context passed will have the Markdown content for the language
9354
// of the page being requested and the Markdown will be rendered
9455
// using the `currentVersion`
95-
if (!(category in restOperationData[currentLanguage][currentVersion])) {
96-
restOperationData[currentLanguage][currentVersion][category] = (await getRestOperationData(
97-
category,
98-
restOperations,
99-
req.context
100-
)) as CategoryDataT
101-
}
56+
const { descriptions, miniTocItems, introContent } = (await getRestOperationData(
57+
category,
58+
currentLanguage,
59+
currentVersion,
60+
req.context
61+
)) as CategoryDataT
10262

10363
return {
10464
props: {
10565
restOperations,
10666
mainContext: getMainContext(req, res),
107-
descriptions: restOperationData[currentLanguage][currentVersion][category].descriptions,
108-
miniTocItems: restOperationData[currentLanguage][currentVersion][category].miniTocItems,
109-
introContent: restOperationData[currentLanguage][currentVersion][category].introContent,
67+
descriptions,
68+
miniTocItems,
69+
introContent,
11070
},
11171
}
11272
}

‎tests/rendering/rest.js

Lines changed: 2 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,19 @@
1-
import { fileURLToPath } from 'url'
2-
import path from 'path'
3-
import fs from 'fs/promises'
4-
import { difference } from 'lodash-es'
51
import { getDOM } from '../helpers/supertest.js'
62
import getRest, { getEnabledForApps } from '../../lib/rest/index.js'
73
import { jest } from '@jest/globals'
84
import { allVersions } from '../../lib/all-versions.js'
95

10-
const __dirname = path.dirname(fileURLToPath(import.meta.url))
11-
// list of REST markdown files that do not correspond to REST API resources
12-
// TODO could we get this list dynamically, say via page frontmatter?
13-
const excludeFromResourceNameCheck = [
14-
'endpoints-available-for-github-apps.md',
15-
'permissions-required-for-github-apps.md',
16-
'index.md',
17-
]
18-
196
describe('REST references docs', () => {
207
jest.setTimeout(3 * 60 * 1000)
218

22-
let operations
23-
24-
beforeAll(async () => {
25-
operations = await getRest()
26-
})
27-
28-
test('markdown file exists for every operationId prefix in the api.github.com schema', async () => {
29-
const categories = [
30-
...new Set(
31-
Object.values(operations)
32-
.map((version) => Object.keys(version))
33-
.flat()
34-
),
35-
]
36-
const referenceDir = path.join(__dirname, '../../content/rest/reference')
37-
const filenames = (await fs.readdir(referenceDir))
38-
.filter(
39-
(filename) =>
40-
!excludeFromResourceNameCheck.find((excludedFile) => filename.endsWith(excludedFile))
41-
)
42-
.map((filename) => filename.replace('.md', ''))
43-
44-
const missingResource =
45-
'Found a markdown file in content/rest/reference that is not represented by an OpenAPI REST operation category.'
46-
expect(difference(filenames, categories), missingResource).toEqual([])
47-
48-
const missingFile =
49-
'Found an OpenAPI REST operation category that is not represented by a markdown file in content/rest/reference.'
50-
expect(difference(categories, filenames), missingFile).toEqual([])
51-
})
52-
539
test('loads schema data for all versions', async () => {
5410
for (const version in allVersions) {
11+
const checksRestOperations = getRest(version, 'checks')
5512
const $ = await getDOM(`/en/${version}/rest/reference/checks`)
5613
const domH3Ids = $('h3')
5714
.map((i, h3) => $(h3).attr('id'))
5815
.get()
59-
const schemaSlugs = Object.values(operations[version].checks)
16+
const schemaSlugs = Object.values(checksRestOperations)
6017
.flat()
6118
.map((operation) => operation.slug)
6219
expect(schemaSlugs.every((slug) => domH3Ids.includes(slug))).toBe(true)
@@ -80,9 +37,4 @@ describe('REST references docs', () => {
8037
expect(schemaSlugs.every((slug) => domH3Ids.includes(slug))).toBe(true)
8138
}
8239
})
83-
84-
test('no wrongly detected AppleScript syntax highlighting in schema data', async () => {
85-
const operations = await getRest()
86-
expect(JSON.stringify(operations).includes('hljs language-applescript')).toBe(false)
87-
})
8840
})

0 commit comments

Comments
 (0)