fix: respect 'force' and 'conditions' properties on redirects (#38657) (

#38664) * move force redirects changes to main repo * empty commit * fix: conditions and add e2e test case * test: fix country condition assertion --------- Co-authored-by: Michal Piechowiak <[email protected]> (cherry picked from commit 48d311e) Co-authored-by: Jenae Janzen <[email protected]>
gatsbyjs · Oct 26, 2023 · c43080f · c43080f
1 parent 4bf84f0
commit c43080f
Show file tree

Hide file tree

Showing 13 changed files with 373 additions and 38 deletions.
diff --git a/e2e-tests/adapters/cypress/e2e/redirects.cy.ts b/e2e-tests/adapters/cypress/e2e/redirects.cy.ts
@@ -1,6 +1,6 @@
 import { applyTrailingSlashOption } from "../../utils"
 
-Cypress.on("uncaught:exception", (err) => {
+Cypress.on("uncaught:exception", err => {
   if (err.message.includes("Minified React error")) {
     return false
   }
@@ -14,44 +14,149 @@ describe("Redirects", () => {
   it("should redirect from non-existing page to existing", () => {
     cy.visit(applyTrailingSlashOption(`/redirect`, TRAILING_SLASH), {
       failOnStatusCode: false,
-    }).waitForRouteChange()
-      .assertRoute(applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH))
+    })
+      .waitForRouteChange()
+      .assertRoute(
+        applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
+      )
 
     cy.get(`h1`).should(`have.text`, `Hit`)
   })
   it("should respect that pages take precedence over redirects", () => {
-    cy.visit(applyTrailingSlashOption(`/routes/redirect/existing`, TRAILING_SLASH), {
-      failOnStatusCode: false,
-    }).waitForRouteChange()
-      .assertRoute(applyTrailingSlashOption(`/routes/redirect/existing`, TRAILING_SLASH))
+    cy.visit(
+      applyTrailingSlashOption(`/routes/redirect/existing`, TRAILING_SLASH),
+      {
+        failOnStatusCode: false,
+      }
+    )
+      .waitForRouteChange()
+      .assertRoute(
+        applyTrailingSlashOption(`/routes/redirect/existing`, TRAILING_SLASH)
+      )
 
     cy.get(`h1`).should(`have.text`, `Existing`)
   })
+  it("should respect force redirect", () => {
+    cy.visit(
+      applyTrailingSlashOption(
+        `/routes/redirect/existing-force`,
+        TRAILING_SLASH
+      ),
+      {
+        failOnStatusCode: false,
+      }
+    )
+      .waitForRouteChange()
+      .assertRoute(
+        applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
+      )
+
+    cy.get(`h1`).should(`have.text`, `Hit`)
+  })
+  it("should respect country condition on redirect", () => {
+    cy.visit(
+      applyTrailingSlashOption(
+        `/routes/redirect/country-condition`,
+        TRAILING_SLASH
+      ),
+      {
+        failOnStatusCode: false,
+        headers: {
+          "x-nf-country": "us",
+        },
+      }
+    )
+      .waitForRouteChange()
+      .assertRoute(
+        applyTrailingSlashOption(`/routes/redirect/hit-us`, TRAILING_SLASH)
+      )
+
+    cy.get(`h1`).should(`have.text`, `Hit US`)
+
+    cy.visit(
+      applyTrailingSlashOption(
+        `/routes/redirect/country-condition`,
+        TRAILING_SLASH
+      ),
+      {
+        failOnStatusCode: false,
+        headers: {
+          "x-nf-country": "de",
+        },
+      }
+    )
+      .waitForRouteChange()
+      .assertRoute(
+        applyTrailingSlashOption(`/routes/redirect/hit-de`, TRAILING_SLASH)
+      )
+
+    cy.get(`h1`).should(`have.text`, `Hit DE`)
+
+    // testing fallback
+    cy.visit(
+      applyTrailingSlashOption(
+        `/routes/redirect/country-condition`,
+        TRAILING_SLASH
+      ),
+      {
+        failOnStatusCode: false,
+        headers: {
+          "x-nf-country": "fr",
+        },
+      }
+    )
+      .waitForRouteChange()
+      .assertRoute(
+        applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
+      )
+
+    cy.get(`h1`).should(`have.text`, `Hit`)
+  })
   it("should support hash parameter on direct visit", () => {
-    cy.visit(applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) + `#anchor`, {
-      failOnStatusCode: false,
-    }).waitForRouteChange()
+    cy.visit(
+      applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) + `#anchor`,
+      {
+        failOnStatusCode: false,
+      }
+    ).waitForRouteChange()
 
-    cy.location(`pathname`).should(`equal`, applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH))
+    cy.location(`pathname`).should(
+      `equal`,
+      applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
+    )
     cy.location(`hash`).should(`equal`, `#anchor`)
     cy.location(`search`).should(`equal`, ``)
   })
   it("should support search parameter on direct visit", () => {
-    cy.visit(applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) + `?query_param=hello`, {
-      failOnStatusCode: false,
-    }).waitForRouteChange()
+    cy.visit(
+      applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) +
+        `?query_param=hello`,
+      {
+        failOnStatusCode: false,
+      }
+    ).waitForRouteChange()
 
-    cy.location(`pathname`).should(`equal`, applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH))
+    cy.location(`pathname`).should(
+      `equal`,
+      applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
+    )
     cy.location(`hash`).should(`equal`, ``)
     cy.location(`search`).should(`equal`, `?query_param=hello`)
   })
   it("should support search & hash parameter on direct visit", () => {
-    cy.visit(applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) + `?query_param=hello#anchor`, {
-      failOnStatusCode: false,
-    }).waitForRouteChange()
+    cy.visit(
+      applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) +
+        `?query_param=hello#anchor`,
+      {
+        failOnStatusCode: false,
+      }
+    ).waitForRouteChange()
 
-    cy.location(`pathname`).should(`equal`, applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH))
+    cy.location(`pathname`).should(
+      `equal`,
+      applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
+    )
     cy.location(`hash`).should(`equal`, `#anchor`)
     cy.location(`search`).should(`equal`, `?query_param=hello`)
   })
-})
+})
diff --git a/e2e-tests/adapters/gatsby-node.ts b/e2e-tests/adapters/gatsby-node.ts
@@ -2,15 +2,57 @@ import * as path from "path"
 import type { GatsbyNode, GatsbyConfig } from "gatsby"
 import { applyTrailingSlashOption } from "./utils"
 
-const TRAILING_SLASH = (process.env.TRAILING_SLASH || `never`) as GatsbyConfig["trailingSlash"]
+const TRAILING_SLASH = (process.env.TRAILING_SLASH ||
+  `never`) as GatsbyConfig["trailingSlash"]
 
-export const createPages: GatsbyNode["createPages"] = ({ actions: { createRedirect, createSlice } }) => {
+export const createPages: GatsbyNode["createPages"] = ({
+  actions: { createRedirect, createSlice },
+}) => {
   createRedirect({
     fromPath: applyTrailingSlashOption("/redirect", TRAILING_SLASH),
     toPath: applyTrailingSlashOption("/routes/redirect/hit", TRAILING_SLASH),
   })
   createRedirect({
-    fromPath: applyTrailingSlashOption("/routes/redirect/existing", TRAILING_SLASH),
+    fromPath: applyTrailingSlashOption(
+      "/routes/redirect/existing",
+      TRAILING_SLASH
+    ),
+    toPath: applyTrailingSlashOption("/routes/redirect/hit", TRAILING_SLASH),
+  })
+  createRedirect({
+    fromPath: applyTrailingSlashOption(
+      "/routes/redirect/existing-force",
+      TRAILING_SLASH
+    ),
+    toPath: applyTrailingSlashOption("/routes/redirect/hit", TRAILING_SLASH),
+    force: true,
+  })
+  createRedirect({
+    fromPath: applyTrailingSlashOption(
+      "/routes/redirect/country-condition",
+      TRAILING_SLASH
+    ),
+    toPath: applyTrailingSlashOption("/routes/redirect/hit-us", TRAILING_SLASH),
+    conditions: {
+      country: ["us"],
+    },
+  })
+  createRedirect({
+    fromPath: applyTrailingSlashOption(
+      "/routes/redirect/country-condition",
+      TRAILING_SLASH
+    ),
+    toPath: applyTrailingSlashOption("/routes/redirect/hit-de", TRAILING_SLASH),
+    conditions: {
+      country: ["de"],
+    },
+  })
+  // fallback if not matching a country condition
+  createRedirect({
+    fromPath: applyTrailingSlashOption(
+      "/routes/redirect/country-condition",
+      TRAILING_SLASH
+    ),
     toPath: applyTrailingSlashOption("/routes/redirect/hit", TRAILING_SLASH),
   })
 
@@ -19,4 +61,4 @@ export const createPages: GatsbyNode["createPages"] = ({ actions: { createRedire
     component: path.resolve(`./src/components/footer.jsx`),
     context: {},
   })
-}
+}
diff --git a/e2e-tests/adapters/scripts/deploy-and-run/netlify.mjs b/e2e-tests/adapters/scripts/deploy-and-run/netlify.mjs
@@ -32,6 +32,8 @@ const deployInfo = JSON.parse(deployResults.stdout)
 
 process.env.DEPLOY_URL = deployInfo.deploy_url
 
+console.log(`Deployed to ${deployInfo.deploy_url}`)
+
 try {
   await execa(`npm`, [`run`, npmScriptToRun], { stdio: `inherit` })
 } finally {

diff --git a/e2e-tests/adapters/src/pages/routes/redirect/existing-force.jsx b/e2e-tests/adapters/src/pages/routes/redirect/existing-force.jsx
@@ -0,0 +1,14 @@
+import * as React from "react"
+import Layout from "../../../components/layout"
+
+const ExistingForcePage = () => {
+  return (
+    <Layout>
+      <h1>Existing Force</h1>
+    </Layout>
+  )
+}
+
+export default ExistingForcePage
+
+export const Head = () => <title>Existing Force</title>
diff --git a/e2e-tests/adapters/src/pages/routes/redirect/hit-de.jsx b/e2e-tests/adapters/src/pages/routes/redirect/hit-de.jsx
@@ -0,0 +1,14 @@
+import * as React from "react"
+import Layout from "../../../components/layout"
+
+const HitDEPage = () => {
+  return (
+    <Layout>
+      <h1>Hit DE</h1>
+    </Layout>
+  )
+}
+
+export default HitDEPage
+
+export const Head = () => <title>Hit DE</title>
diff --git a/e2e-tests/adapters/src/pages/routes/redirect/hit-us.jsx b/e2e-tests/adapters/src/pages/routes/redirect/hit-us.jsx
@@ -0,0 +1,14 @@
+import * as React from "react"
+import Layout from "../../../components/layout"
+
+const HitUSPage = () => {
+  return (
+    <Layout>
+      <h1>Hit US</h1>
+    </Layout>
+  )
+}
+
+export default HitUSPage
+
+export const Head = () => <title>Hit US</title>
diff --git a/packages/gatsby-adapter-netlify/src/__tests__/route-handler.ts b/packages/gatsby-adapter-netlify/src/__tests__/route-handler.ts
@@ -1,13 +1,15 @@
 import fs from "fs-extra"
 import { tmpdir } from "os"
 import { join } from "path"
+import type { IRedirectRoute, RoutesManifest } from "gatsby"
 import {
   injectEntries,
   ADAPTER_MARKER_START,
   ADAPTER_MARKER_END,
   NETLIFY_PLUGIN_MARKER_START,
   NETLIFY_PLUGIN_MARKER_END,
   GATSBY_PLUGIN_MARKER_START,
+  processRoutesManifest,
 } from "../route-handler"
 
 function generateLotOfContent(placeholderCharacter: string): string {
@@ -142,4 +144,50 @@ describe(`route-handler`, () => {
       })
     })
   })
+
+  describe(`createRedirects`, () => {
+    it(`honors the force parameter`, async () => {
+      const manifest: RoutesManifest = [
+        {
+          path: `/old-url`,
+          type: `redirect`,
+          toPath: `/new-url`,
+          status: 301,
+          headers: [{ key: `string`, value: `string` }],
+          force: true,
+        },
+        {
+          path: `/old-url2`,
+          type: `redirect`,
+          toPath: `/new-url2`,
+          status: 308,
+          headers: [{ key: `string`, value: `string` }],
+          force: false,
+        },
+      ]
+
+      const { redirects } = processRoutesManifest(manifest)
+
+      // `!` is appended to status to mark forced redirect
+      expect(redirects).toMatch(/^\/old-url\s+\/new-url\s+301!$/m)
+      // `!` is not appended to status to mark not forced redirect
+      expect(redirects).toMatch(/^\/old-url2\s+\/new-url2\s+308$/m)
+    })
+
+    it(`honors the conditions parameter`, async () => {
+      const redirect: IRedirectRoute = {
+        path: `/old-url`,
+        type: `redirect`,
+        toPath: `/new-url`,
+        status: 200,
+        headers: [{ key: `string`, value: `string` }],
+        conditions: { language: [`ca`, `us`] },
+      }
+
+      const { redirects } = processRoutesManifest([redirect])
+      expect(redirects).toMatch(
+        /^\/old-url\s+\/new-url\s+200\s+Language=ca,us$/m
+      )
+    })
+  })
 })