Skip to content

Commit 38d312c

Browse files
committed
Changed link redirects from middleware to router
ref https://linear.app/ghost/issue/BER-3095 Extracted the link redirects code from the middleware to the router. This avoids route comparison on every requests Ghost receives.
1 parent 532426d commit 38d312c

File tree

4 files changed

+48
-14
lines changed

4 files changed

+48
-14
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const linkRedirects = require('../../../server/services/link-redirection');
2+
3+
module.exports = function handleRedirects(siteApp) {
4+
siteApp.get(linkRedirects.service.redirectPrefix() + '*', linkRedirects.service.handleRequest);
5+
};

ghost/core/core/frontend/web/site.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const themeMiddleware = themeEngine.middleware;
1515
const membersService = require('../../server/services/members');
1616
const offersService = require('../../server/services/offers');
1717
const customRedirects = require('../../server/services/custom-redirects');
18-
const linkRedirects = require('../../server/services/link-redirection');
18+
const linkRedirectsHandler = require('./routers/link-redirects');
1919
const {cardAssets} = require('../services/assets-minification');
2020
const siteRoutes = require('./routes');
2121
const shared = require('../../server/web/shared');
@@ -51,7 +51,7 @@ module.exports = function setupSiteApp(routerConfig) {
5151

5252
siteApp.use(offersService.middleware);
5353

54-
siteApp.use(linkRedirects.service.handleRequest);
54+
linkRedirectsHandler(siteApp);
5555

5656
// you can extend Ghost with a custom redirects file
5757
// see https://github.com/TryGhost/Ghost/issues/7707

ghost/core/core/server/services/link-redirection/LinkRedirectsService.js

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,25 @@ class LinkRedirectsService {
7777
return link;
7878
}
7979

80+
/**
81+
* Returns the full path prefix including subdirectory (e.g., /blog/r/)
82+
* Used for building full URLs
83+
* @return {string}
84+
**/
85+
redirectPrefix() {
86+
const fullURLWithRedirectPrefix = `${this.#baseURL.pathname}${this.#redirectURLPrefix}`;
87+
return fullURLWithRedirectPrefix;
88+
}
89+
90+
/**
91+
* Returns the route path for Express routing (always /r/)
92+
* Express routing uses req.path which excludes subdirectory
93+
* @return {string}
94+
**/
95+
routePath() {
96+
return `/${this.#redirectURLPrefix}`;
97+
}
98+
8099
/**
81100
* @param {import('express').Request} req
82101
* @param {import('express').Response} res
@@ -86,18 +105,6 @@ class LinkRedirectsService {
86105
*/
87106
async handleRequest(req, res, next) {
88107
try {
89-
// skip handling if original url doesn't match the prefix
90-
const fullURLWithRedirectPrefix = `${this.#baseURL.pathname}${this.#redirectURLPrefix}`;
91-
// @NOTE: below is equivalent to doing:
92-
// router.get('/r/'), (req, res) ...
93-
// To make it cleaner we should rework it to:
94-
// linkRedirects.service.handleRequest(router);
95-
// and mount routes on top like for example sitemapHandler does
96-
// Cleanup issue: https://github.com/TryGhost/Toolbox/issues/516
97-
if (!req.originalUrl.startsWith(fullURLWithRedirectPrefix)) {
98-
return next();
99-
}
100-
101108
const url = new URL(req.originalUrl, this.#baseURL);
102109
const link = await this.#linkRedirectRepository.getByURL(url);
103110

ghost/core/test/unit/server/services/link-redirection/LinkRedirectsService.test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,28 @@ describe('LinkRedirectsService', function () {
6565
});
6666
});
6767

68+
describe('redirectPrefix', function () {
69+
it('returns the full path including subdirectory', function () {
70+
const instance = new LinkRedirectsService({
71+
linkRedirectRepository: {},
72+
config: {
73+
baseURL: new URL('https://localhost:2368/blog/')
74+
}
75+
});
76+
assert.equal(instance.redirectPrefix(), '/blog/r/');
77+
});
78+
79+
it('returns path without subdirectory when none configured', function () {
80+
const instance = new LinkRedirectsService({
81+
linkRedirectRepository: {},
82+
config: {
83+
baseURL: new URL('https://localhost:2368/')
84+
}
85+
});
86+
assert.equal(instance.redirectPrefix(), '/r/');
87+
});
88+
});
89+
6890
describe('handleRequest', function () {
6991
it('redirects if found', async function () {
7092
const linkRedirectRepository = {

0 commit comments

Comments
 (0)