Skip to content

Commit

Permalink
Move redirect handling from Middleware to [[...path]]/page and resolv…
Browse files Browse the repository at this point in the history
…e them on demand (#2638)

Major advantage is that we don't have to load all redirects into memory and get independent of the number of redirects.

Theoretically incompatible: if a redirect and page with the same path exist, previously the redirect was used, now the page is used.

One tricky part is that redirects are not language scoped, but pages are:
- for redirects I take the language param and put append it to the source path
- to support redirects with "invalid" languages (from the page tree perspective) - for example `/redirect-10` I had to avoid sending an invalid `scope.language` (would be `redirect-10`) by adding the `@skip` directive - else the api responds with an Access Denied error as the scope is not allowed

Co-authored-by: Johannes Obermair <[email protected]>
  • Loading branch information
nsams and johnnyomair authored Oct 22, 2024
1 parent da49d05 commit 44ec9eb
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 154 deletions.
5 changes: 5 additions & 0 deletions .changeset/happy-pumpkins-agree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@comet/cms-api": minor
---

Redirects: Add `redirectBySource` query that can be used to query for a single redirect by source
1 change: 1 addition & 0 deletions demo/api/schema.gql
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ type Query {
redirects(scope: RedirectScopeInput!, query: String, type: RedirectGenerationType, active: Boolean, sortColumnName: String, sortDirection: SortDirection! = ASC): [Redirect!]! @deprecated(reason: "Use paginatedRedirects instead. Will be removed in the next version.")
paginatedRedirects(scope: RedirectScopeInput!, search: String, filter: RedirectFilter, sort: [RedirectSort!], offset: Int! = 0, limit: Int! = 25): PaginatedRedirects!
redirect(id: ID!): Redirect!
redirectBySource(scope: RedirectScopeInput!, source: String!, sourceType: RedirectSourceTypeValues!): Redirect
redirectSourceAvailable(scope: RedirectScopeInput!, source: String!): Boolean!
damItemsList(offset: Int! = 0, limit: Int! = 25, sortColumnName: String, sortDirection: SortDirection! = ASC, scope: DamScopeInput!, folderId: ID, includeArchived: Boolean, filter: DamItemFilterInput): PaginatedDamItems!
damItemListPosition(sortColumnName: String, sortDirection: SortDirection! = ASC, scope: DamScopeInput!, id: ID!, type: DamItemType!, folderId: ID, includeArchived: Boolean, filter: DamItemFilterInput): Int!
Expand Down
14 changes: 14 additions & 0 deletions demo/site/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,20 @@ const nextConfig = {
},
];
},
async redirects() {
const adminUrl = process.env.ADMIN_URL;

if (!adminUrl) {
throw Error("ADMIN_URL is not defined");
}
return [
{
source: "/admin",
destination: adminUrl,
permanent: false,
},
];
},
images: {
deviceSizes: cometConfig.dam.allowedImageSizes,
},
Expand Down
50 changes: 41 additions & 9 deletions demo/site/src/app/[lang]/[[...path]]/page.tsx
Original file line number Diff line number Diff line change
@@ -1,30 +1,45 @@
import { gql, previewParams } from "@comet/cms-site";
import { ExternalLinkBlockData, InternalLinkBlockData, RedirectsLinkBlockData } from "@src/blocks.generated";
import { domain, languages } from "@src/config";
import { documentTypes } from "@src/documentTypes";
import { GQLPageTreeNodeScopeInput } from "@src/graphql.generated";
import { GQLPageTreeNodeScope, GQLPageTreeNodeScopeInput } from "@src/graphql.generated";
import { createGraphQLFetch } from "@src/util/graphQLClient";
import type { Metadata, ResolvingMetadata } from "next";
import { notFound } from "next/navigation";
import { notFound, redirect } from "next/navigation";

import { GQLDocumentTypeQuery, GQLDocumentTypeQueryVariables } from "./page.generated";

const documentTypeQuery = gql`
query DocumentType($path: String!, $scope: PageTreeNodeScopeInput!) {
pageTreeNodeByPath(path: $path, scope: $scope) {
query DocumentType(
$skipPage: Boolean!
$path: String!
$scope: PageTreeNodeScopeInput!
$redirectSource: String!
$redirectScope: RedirectScopeInput!
) {
pageTreeNodeByPath(path: $path, scope: $scope) @skip(if: $skipPage) {
id
documentType
}
redirectBySource(source: $redirectSource, sourceType: path, scope: $redirectScope) {
target
}
}
`;

async function fetchPageTreeNode(params: { path: string[]; lang: string }) {
const skipPage = !languages.includes(params.lang);
const { scope, previewData } = (await previewParams()) || { scope: { domain, language: params.lang }, previewData: undefined };
const graphQLFetch = createGraphQLFetch(previewData);
const path = `/${(params.path ?? []).join("/")}`;
return graphQLFetch<GQLDocumentTypeQuery, GQLDocumentTypeQueryVariables>(
documentTypeQuery,
{
path: `/${(params.path ?? []).join("/")}`,
skipPage,
path,
scope: scope as GQLPageTreeNodeScopeInput, //TODO fix type, the scope from previewParams() is not compatible with GQLPageTreeNodeScopeInput
redirectSource: `/${params.lang}${path !== "/" ? path : ""}`,
redirectScope: { domain: scope.domain },
},
{ method: "GET" }, //for request memoization
);
Expand Down Expand Up @@ -60,13 +75,30 @@ export default async function Page({ params }: Props) {
// TODO support multiple domains, get domain by Host header
const { scope } = (await previewParams()) || { scope: { domain, language: params.lang } };

if (!languages.includes(params.lang)) {
notFound();
}

const data = await fetchPageTreeNode(params);

if (!data.pageTreeNodeByPath?.documentType) {
if (data.redirectBySource?.target) {
const target = data.redirectBySource?.target as RedirectsLinkBlockData;
let destination: string | undefined;
if (target.block !== undefined) {
switch (target.block.type) {
case "internal": {
const internalLink = target.block.props as InternalLinkBlockData;
if (internalLink.targetPage) {
destination = `${(internalLink.targetPage.scope as GQLPageTreeNodeScope).language}/${internalLink.targetPage.path}`;
}
break;
}
case "external":
destination = (target.block.props as ExternalLinkBlockData).targetUrl;
break;
}
}
if (destination) {
redirect(destination);
}
}
notFound();
}

Expand Down
9 changes: 7 additions & 2 deletions demo/site/src/app/[lang]/layout.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { languages } from "@src/config";
import { readFile } from "fs/promises";
import { PropsWithChildren } from "react";

Expand All @@ -13,9 +14,13 @@ async function loadMessages(lang: string) {
}

export default async function Page({ children, params }: PropsWithChildren<{ params: { lang: string } }>) {
const messages = await loadMessages(params.lang);
let language = params.lang;
if (!languages.includes(language)) {
language = "en";
}
const messages = await loadMessages(language);
return (
<IntlProvider locale={params.lang} messages={messages}>
<IntlProvider locale={language} messages={messages}>
{children}
</IntlProvider>
);
Expand Down
9 changes: 0 additions & 9 deletions demo/site/src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,12 @@ import { NextResponse } from "next/server";

import { domain } from "./config";
import { getPredefinedPageRedirect, getPredefinedPageRewrite } from "./middleware/predefinedPages";
import { createRedirects } from "./middleware/redirects";

export async function middleware(request: NextRequest) {
const { pathname } = new URL(request.url);

const scope = { domain };

const redirects = await createRedirects(scope);

const redirect = redirects.get(pathname);
if (redirect) {
const destination: string = redirect.destination;
return NextResponse.redirect(new URL(destination, request.url), redirect.permanent ? 308 : 307);
}

const predefinedPageRedirect = await getPredefinedPageRedirect(scope, pathname);

if (predefinedPageRedirect) {
Expand Down
131 changes: 0 additions & 131 deletions demo/site/src/middleware/redirects.ts

This file was deleted.

1 change: 1 addition & 0 deletions packages/api/cms-api/schema.gql
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ type Query {
redirects(scope: RedirectScopeInput! = {}, query: String, type: RedirectGenerationType, active: Boolean, sortColumnName: String, sortDirection: SortDirection! = ASC): [Redirect!]! @deprecated(reason: "Use paginatedRedirects instead. Will be removed in the next version.")
paginatedRedirects(scope: RedirectScopeInput! = {}, search: String, filter: RedirectFilter, sort: [RedirectSort!], offset: Int! = 0, limit: Int! = 25): PaginatedRedirects!
redirect(id: ID!): Redirect!
redirectBySource(scope: RedirectScopeInput! = {}, source: String!, sourceType: RedirectSourceTypeValues!): Redirect
redirectSourceAvailable(scope: RedirectScopeInput! = {}, source: String!): Boolean!
damItemsList(offset: Int! = 0, limit: Int! = 25, sortColumnName: String, sortDirection: SortDirection! = ASC, scope: DamScopeInput! = {}, folderId: ID, includeArchived: Boolean, filter: DamItemFilterInput): PaginatedDamItems!
damItemListPosition(sortColumnName: String, sortDirection: SortDirection! = ASC, scope: DamScopeInput! = {}, id: ID!, type: DamItemType!, folderId: ID, includeArchived: Boolean, filter: DamItemFilterInput): Int!
Expand Down
21 changes: 18 additions & 3 deletions packages/api/cms-api/src/redirects/redirects.resolver.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FindOptions, wrap } from "@mikro-orm/core";
import { FilterQuery, FindOptions, wrap } from "@mikro-orm/core";
import { InjectRepository } from "@mikro-orm/nestjs";
import { EntityRepository } from "@mikro-orm/postgresql";
import { Type } from "@nestjs/common";
Expand All @@ -17,6 +17,7 @@ import { RedirectInputInterface } from "./dto/redirect-input.factory";
import { RedirectUpdateActivenessInput } from "./dto/redirect-update-activeness.input";
import { RedirectsArgsFactory } from "./dto/redirects-args.factory";
import { RedirectInterface } from "./entities/redirect-entity.factory";
import { RedirectSourceTypeValues } from "./redirects.enum";
import { RedirectsService } from "./redirects.service";
import { RedirectScopeInterface } from "./types";

Expand Down Expand Up @@ -103,8 +104,22 @@ export function createRedirectsResolver({

@Query(() => Redirect)
@AffectedEntity(Redirect)
async redirect(@Args("id", { type: () => ID }) id: string): Promise<RedirectInterface | null> {
const redirect = await this.repository.findOne(id);
async redirect(@Args("id", { type: () => ID }) id: string): Promise<RedirectInterface> {
const redirect = await this.repository.findOneOrFail(id);
return redirect;
}

@Query(() => Redirect, { nullable: true })
async redirectBySource(
@Args("scope", { type: () => Scope, defaultValue: hasNonEmptyScope ? undefined : {} }) scope: typeof Scope,
@Args("source", { type: () => String }) source: string,
@Args("sourceType", { type: () => RedirectSourceTypeValues }) sourceType: RedirectSourceTypeValues,
): Promise<RedirectInterface | null> {
const where: FilterQuery<RedirectInterface> = { source, sourceType };
if (hasNonEmptyScope) {
where.scope = scope;
}
const redirect = await this.repository.findOne(where);
return redirect ?? null;
}

Expand Down

0 comments on commit 44ec9eb

Please sign in to comment.