Merge pull request #23951 from github/repo-sync

repo sync

Author: Octomerger

lib/schema-event.js

@@ -401,29 +401,6 @@ const experimentSchema = {
   },
 }
 
-const redirectSchema = {
-  type: 'object',
-  additionalProperties: false,
-  required: ['type', 'context', 'redirect_from', 'redirect_to'],
-  properties: {
-    context,
-    type: {
-      type: 'string',
-      pattern: '^redirect$',
-    },
-    redirect_from: {
-      type: 'string',
-      description: 'The requested href.',
-      format: 'uri-reference',
-    },
-    redirect_to: {
-      type: 'string',
-      description: 'The destination href of the redirect.',
-      format: 'uri-reference',
-    },
-  },
-}
-
 const clipboardSchema = {
   type: 'object',
   additionalProperties: false,
@@ -498,7 +475,6 @@ export const eventSchema = {
     navigateSchema,
     surveySchema,
     experimentSchema,
-    redirectSchema,
     clipboardSchema,
     printSchema,
     preferenceSchema,
@@ -514,7 +490,6 @@ export const hydroNames = {
   navigate: 'docs.v0.NavigateEvent',
   survey: 'docs.v0.SurveyEvent',
   experiment: 'docs.v0.ExperimentEvent',
-  redirect: 'docs.v0.RedirectEvent',
   clipboard: 'docs.v0.ClipboardEvent',
   print: 'docs.v0.PrintEvent',
   preference: 'docs.v0.PreferenceEvent',

middleware/api/es-search.js

@@ -41,10 +41,19 @@ export async function getSearchResults({
   includeTopics,
   usePrefixSearch,
   highlights,
+  include,
 }) {
   if (topics && !Array.isArray(topics)) {
     throw new Error("'topics' has to be an array")
   }
+  if (include) {
+    if (!Array.isArray(include)) {
+      throw new Error("'include' has to be an array")
+    }
+    if (!include.every((value) => typeof value === 'string')) {
+      throw new Error("Every entry in the 'include' must be a string")
+    }
+  }
   const t0 = new Date()
   const client = getClient()
   const from = size * (page - 1)
@@ -146,7 +155,13 @@ export async function getSearchResults({
 
   // const hitsAll = result.hits  // ES >7.11
   const hitsAll = result.body // ES <=7.11
-  const hits = getHits(hitsAll.hits.hits, { indexName, debug, includeTopics, highlightFields })
+  const hits = getHits(hitsAll.hits.hits, {
+    indexName,
+    debug,
+    includeTopics,
+    highlightFields,
+    include,
+  })
   const t1 = new Date()
 
   const meta = {
@@ -350,7 +365,7 @@ function getMatchQueries(query, { usePrefixSearch, fuzzy }) {
   return matchQueries
 }
 
-function getHits(hits, { indexName, debug, includeTopics, highlightFields }) {
+function getHits(hits, { indexName, debug, includeTopics, highlightFields, include }) {
   return hits.map((hit) => {
     // Return `hit.highlights[...]` based on the highlight fields requested.
     // So if you searched with `&highlights=headings&highlights=content`
@@ -381,7 +396,9 @@ function getHits(hits, { indexName, debug, includeTopics, highlightFields }) {
         result.es_url = `http://localhost:9200/${indexName}/_doc/${hit._id}`
       }
     }
-
+    for (const field of include || []) {
+      result[field] = hit._source[field]
+    }
     return result
   })
 }

middleware/api/search.js

@@ -27,6 +27,13 @@ const POSSIBLE_SORTS = ['best', 'relevance']
 const DEFAULT_SORT = POSSIBLE_SORTS[0]
 const MAX_PAGE = 10
 
+// There are some fields you can optionally include in the output.
+// These are fields available in Elasticsearch that we don't include in
+// the output by default. E.g. `...&include=intro`
+// Requesting anything that is not in this list will result in
+// a 400 Bad Request.
+const V1_ADDITIONAL_INCLUDES = ['intro', 'headings']
+
 // If someone searches for `...&version=3.5` what they actually mean
 // is `ghes-3.5`. This is because of legacy formatting with the old search.
 // In some distant future we can clean up any client enough that this
@@ -204,6 +211,16 @@ const validationMiddleware = (req, res, next) => {
     },
     { key: 'autocomplete', default_: false, cast: toBoolean },
     { key: 'debug', default_: process.env.NODE_ENV === 'development', cast: toBoolean },
+    {
+      key: 'include',
+      default_: [],
+      cast: toArray,
+      // Note: At the time of writing this general validator middleware
+      // doesn't yet know it's being used by the v1 version.
+      // But we don't have any other versions yet so no need to
+      // over-engineer this more.
+      validate: (values) => values.every((value) => V1_ADDITIONAL_INCLUDES.includes(value)),
+    },
   ]
 
   const search = {}
@@ -247,12 +264,26 @@ function toBoolean(value) {
   return false
 }
 
+function toArray(value) {
+  return Array.isArray(value) ? value : [value]
+}
+
 router.get(
   '/v1',
   validationMiddleware,
   catchMiddlewareError(async function search(req, res) {
-    const { indexName, language, query, autocomplete, page, size, debug, sort, highlights } =
-      req.search
+    const {
+      indexName,
+      language,
+      query,
+      autocomplete,
+      page,
+      size,
+      debug,
+      sort,
+      highlights,
+      include,
+    } = req.search
 
     // The getSearchResults() function is a mix of preparing the search,
     // sending & receiving it, and post-processing the response from the
@@ -272,6 +303,7 @@ router.get(
       sort,
       highlights,
       usePrefixSearch: autocomplete,
+      include,
     }
     try {
       const { meta, hits } = await timed(options)

middleware/index.js

@@ -16,7 +16,6 @@ import {
   setLanguageFastlySurrogateKey,
 } from './set-fastly-surrogate-key.js'
 import reqUtils from './req-utils.js'
-import recordRedirect from './record-redirect.js'
 import handleErrors from './handle-errors.js'
 import handleInvalidPaths from './handle-invalid-paths.js'
 import handleNextDataPath from './handle-next-data-path.js'
@@ -210,8 +209,7 @@ export default function (app) {
   app.set('etag', false) // We will manage our own ETags if desired
 
   // *** Config and context for redirects ***
-  app.use(reqUtils) // Must come before record-redirect and events
-  app.use(recordRedirect)
+  app.use(reqUtils) // Must come before events
   app.use(instrument(detectLanguage, './detect-language')) // Must come before context, breadcrumbs, find-page, handle-errors, homepages
   app.use(asyncMiddleware(instrument(context, './context'))) // Must come before early-access-*, handle-redirects
   app.use(instrument(shortVersions, './contextualizers/short-versions')) // Support version shorthands

middleware/record-redirect.js

@@ -285,3 +285,53 @@ describeIfElasticsearchURL('search legacy middleware', () => {
     expect(results.length).toBe(0)
   })
 })
+
+describeIfElasticsearchURL("additional fields with 'include'", () => {
+  jest.setTimeout(60 * 1000)
+
+  test("'intro' and 'headings' are omitted by default", async () => {
+    const sp = new URLSearchParams()
+    sp.set('query', 'foo')
+    const res = await get('/api/search/v1?' + sp)
+    expect(res.statusCode).toBe(200)
+    const results = JSON.parse(res.text)
+    const firstKeys = Object.keys(results.hits[0])
+    expect(firstKeys.includes('intro')).toBeFalsy()
+    expect(firstKeys.includes('headings')).toBeFalsy()
+  })
+
+  test("additionally include 'intro'", async () => {
+    const sp = new URLSearchParams()
+    sp.set('query', 'foo')
+    sp.set('include', 'intro')
+    const res = await get('/api/search/v1?' + sp)
+    expect(res.statusCode).toBe(200)
+    const results = JSON.parse(res.text)
+    const firstKeys = Object.keys(results.hits[0])
+    expect(firstKeys.includes('intro')).toBeTruthy()
+    expect(firstKeys.includes('headings')).toBeFalsy()
+  })
+
+  test("additionally include 'intro' and 'headings'", async () => {
+    const sp = new URLSearchParams()
+    sp.set('query', 'foo')
+    sp.append('include', 'intro')
+    sp.append('include', 'headings')
+    const res = await get('/api/search/v1?' + sp)
+    expect(res.statusCode).toBe(200)
+    const results = JSON.parse(res.text)
+    const firstKeys = Object.keys(results.hits[0])
+    expect(firstKeys.includes('intro')).toBeTruthy()
+    expect(firstKeys.includes('headings')).toBeTruthy()
+  })
+
+  test("unknown 'include' is 400 bad request", async () => {
+    const sp = new URLSearchParams()
+    sp.set('query', 'foo')
+    sp.set('include', 'xxxxx')
+    const res = await get('/api/search/v1?' + sp)
+    expect(res.statusCode).toBe(400)
+    const results = JSON.parse(res.text)
+    expect(results.error).toMatch(`Not a valid value (["xxxxx"]) for key 'include'`)
+  })
+})

tests/content/api-search.js

@@ -416,23 +416,6 @@ describe('POST /events', () => {
       checkEvent({ ...experimentExample, experiment_success: undefined }, 200))
   })
 
-  describe('redirect', () => {
-    const redirectExample = {
-      ...baseExample,
-      type: 'redirect',
-      redirect_from: 'http://example.com/a',
-      redirect_to: 'http://example.com/b',
-    }
-
-    it('should record an redirect event', () => checkEvent(redirectExample, 200))
-
-    it('redirect_from is required url', () =>
-      checkEvent({ ...redirectExample, redirect_from: ' ' }, 400))
-
-    it('redirect_to is required url', () =>
-      checkEvent({ ...redirectExample, redirect_to: undefined }, 400))
-  })
-
   describe('clipboard', () => {
     const clipboardExample = {
       ...baseExample,