Skip to content

Commit f95ab7c

Browse files
vershwalcursoragent
andcommitted
Added eager/lazy URL service parity coverage
ref https://linear.app/ghost/issue/HKG-1817 - the whole point of this milestone is to swap the eager service for the lazy one without changing any URL, so the lazy service is only trustworthy if it provably returns the same answer as the eager service for the same inputs; this integration test is that proof, run against real fixtures and a real findResource over the model layer - drove both services from one identical routing set per scenario and fed the lazy service the exact records the eager service cached, so any difference the test reports is a routing-logic divergence and not a data-shape artefact - compared all three directions a caller depends on: forward (relative and absolute URL for every cached resource, including the resources that resolve to /404/), ownership (every router against every resource), and reverse (every eager-generated URL resolves back to the same resource and type) - asserted the reverse lookup returns the same record shape as the eager service, not just the same id and type: exact field-set parity, matching slug/name, no leaked title, and relations trimmed to {id, slug}, so a divergence like lazy returning the post body or untrimmed author rows can't slip through - added a date-based permalink scenario and asserted both services 404 a wrong-date variant of a generated URL, locking the canonical re-check so lazy can't resolve a non-canonical URL the eager service rejects - added a featured-collection scenario alongside the default set specifically to exercise priority fallthrough; this is what caught the ownership exclusivity bug now fixed in the service, where a catch-all router claimed resources already owned by a higher-priority collection - guarded against a vacuous pass by asserting the fixtures actually produce cached resources and generated URLs, so the parity assertions can never silently iterate over nothing Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 01c2f52 commit f95ab7c

1 file changed

Lines changed: 229 additions & 0 deletions

File tree

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
const assert = require('node:assert/strict');
2+
const sinon = require('sinon');
3+
const testUtils = require('../utils');
4+
const models = require('../../core/server/models');
5+
const UrlService = require('../../core/server/services/url/url-service');
6+
const LazyUrlService = require('../../core/server/services/url/lazy-url-service');
7+
const {createFindResource} = require('../../core/server/services/url/lazy-find-resource');
8+
9+
const RESOURCE_TYPES = ['posts', 'pages', 'tags', 'authors'];
10+
11+
// Routing sets registered identically on both services. Slug-backed permalinks
12+
// only, since that is Ghost's default shape and the one resolveUrl queries by.
13+
const SCENARIOS = [
14+
{
15+
name: 'default routing set',
16+
routes: [
17+
{identifier: 'posts-router', filter: 'featured:false', resourceType: 'posts', permalink: '/:slug/'},
18+
{identifier: 'authors-router', filter: null, resourceType: 'authors', permalink: '/author/:slug/'},
19+
{identifier: 'tags-router', filter: null, resourceType: 'tags', permalink: '/tag/:slug/'},
20+
{identifier: 'pages-router', filter: null, resourceType: 'pages', permalink: '/:slug/'}
21+
]
22+
},
23+
{
24+
name: 'featured collection with priority fallthrough',
25+
routes: [
26+
{identifier: 'featured-router', filter: 'featured:true', resourceType: 'posts', permalink: '/featured/:slug/'},
27+
{identifier: 'posts-router', filter: null, resourceType: 'posts', permalink: '/:slug/'},
28+
{identifier: 'tags-router', filter: null, resourceType: 'tags', permalink: '/tag/:slug/'},
29+
{identifier: 'authors-router', filter: null, resourceType: 'authors', permalink: '/author/:slug/'}
30+
]
31+
},
32+
{
33+
name: 'date-based post permalinks',
34+
routes: [
35+
{identifier: 'posts-router', filter: null, resourceType: 'posts', permalink: '/:year/:month/:slug/'},
36+
{identifier: 'tags-router', filter: null, resourceType: 'tags', permalink: '/tag/:slug/'},
37+
{identifier: 'authors-router', filter: null, resourceType: 'authors', permalink: '/author/:slug/'}
38+
]
39+
}
40+
];
41+
42+
function waitUntilFinished(urlService, timeout = 5000) {
43+
return new Promise((resolve, reject) => {
44+
const start = Date.now();
45+
(function retry() {
46+
if (urlService.hasFinished()) {
47+
return resolve();
48+
}
49+
if (Date.now() - start > timeout) {
50+
return reject(new Error('Eager UrlService did not finish in time'));
51+
}
52+
setTimeout(retry, 50);
53+
})();
54+
});
55+
}
56+
57+
describe('Integration: eager/lazy URL service parity', function () {
58+
before(testUtils.teardownDb);
59+
before(testUtils.setup('users:roles', 'posts'));
60+
after(testUtils.teardownDb);
61+
62+
after(function () {
63+
sinon.restore();
64+
});
65+
66+
SCENARIOS.forEach(function (scenario) {
67+
describe(scenario.name, function () {
68+
let eager;
69+
let lazy;
70+
71+
before(async function () {
72+
eager = new UrlService();
73+
scenario.routes.forEach(r => eager.onRouterAddedType(r.identifier, r.filter, r.resourceType, r.permalink));
74+
eager.init();
75+
await waitUntilFinished(eager);
76+
77+
lazy = new LazyUrlService({findResource: createFindResource(models)});
78+
scenario.routes.forEach(r => lazy.onRouterAddedType(r.identifier, r.filter, r.resourceType, r.permalink));
79+
});
80+
81+
after(function () {
82+
eager.reset();
83+
});
84+
85+
function cachedResourcesByType() {
86+
return RESOURCE_TYPES.map(type => ({
87+
type,
88+
resources: eager.resources.getAllByType(type) || []
89+
}));
90+
}
91+
92+
function allGeneratedUrls() {
93+
const urls = [];
94+
eager.urlGenerators.forEach((generator) => {
95+
generator.getUrls().forEach((entry) => {
96+
urls.push({url: entry.url, id: entry.resource.data.id});
97+
});
98+
});
99+
return urls;
100+
}
101+
102+
it('loaded a non-trivial fixture set so the comparison is not vacuous', function () {
103+
const total = cachedResourcesByType().reduce((sum, group) => sum + group.resources.length, 0);
104+
assert.ok(total > 0, 'expected the eager service to cache at least one resource');
105+
assert.ok(allGeneratedUrls().length > 0, 'expected the eager service to generate at least one url');
106+
});
107+
108+
it('forward lookup returns the same relative URL for every cached resource', function () {
109+
for (const {type, resources} of cachedResourcesByType()) {
110+
for (const resource of resources) {
111+
const id = resource.data.id;
112+
const lazyResource = Object.assign({}, resource.data, {type});
113+
114+
assert.equal(
115+
lazy.getUrlForResource(lazyResource),
116+
eager.getUrlByResourceId(id),
117+
`relative URL mismatch for ${type} ${id}`
118+
);
119+
}
120+
}
121+
});
122+
123+
it('forward lookup returns the same absolute URL for every cached resource', function () {
124+
for (const {type, resources} of cachedResourcesByType()) {
125+
for (const resource of resources) {
126+
const id = resource.data.id;
127+
const lazyResource = Object.assign({}, resource.data, {type});
128+
129+
assert.equal(
130+
lazy.getUrlForResource(lazyResource, {absolute: true}),
131+
eager.getUrlByResourceId(id, {absolute: true}),
132+
`absolute URL mismatch for ${type} ${id}`
133+
);
134+
}
135+
}
136+
});
137+
138+
it('agrees with the eager service on which router owns each resource', function () {
139+
for (const {type, resources} of cachedResourcesByType()) {
140+
for (const resource of resources) {
141+
const id = resource.data.id;
142+
const lazyResource = Object.assign({}, resource.data, {type});
143+
144+
for (const route of scenario.routes) {
145+
assert.equal(
146+
lazy.ownsResource(route.identifier, lazyResource),
147+
eager.owns(route.identifier, id),
148+
`ownership mismatch for router ${route.identifier} and ${type} ${id}`
149+
);
150+
}
151+
}
152+
}
153+
});
154+
155+
it('reverse lookup resolves every eager-generated URL to the same resource', async function () {
156+
for (const {url, id} of allGeneratedUrls()) {
157+
const eagerEnvelope = eager.getResource(url);
158+
const resolved = await lazy.resolveUrl(url);
159+
160+
assert.ok(resolved, `lazy resolveUrl returned null for ${url}`);
161+
assert.equal(resolved.id, id, `resolved id mismatch for ${url}`);
162+
assert.equal(resolved.type, eagerEnvelope.config.type, `resolved type mismatch for ${url}`);
163+
}
164+
});
165+
166+
it('reverse lookup returns the same record shape as the eager service', async function () {
167+
for (const {url} of allGeneratedUrls()) {
168+
const eagerEnvelope = eager.getResource(url);
169+
const eagerFlat = Object.assign({}, eagerEnvelope.data, {type: eagerEnvelope.config.type});
170+
const resolved = await lazy.resolveUrl(url);
171+
172+
// Exact field-set parity guards against lazy leaking fields the
173+
// eager backend strips (e.g. title/html/mobiledoc on posts,
174+
// email on authors), which would change downstream behaviour.
175+
assert.deepEqual(
176+
Object.keys(resolved).sort(),
177+
Object.keys(eagerFlat).sort(),
178+
`field-set mismatch for ${url}`
179+
);
180+
181+
assert.equal(resolved.slug, eagerFlat.slug, `slug mismatch for ${url}`);
182+
if ('name' in eagerFlat) {
183+
assert.equal(resolved.name, eagerFlat.name, `name mismatch for ${url}`);
184+
}
185+
assert.ok(!('title' in resolved), `lazy leaked title for ${url}`);
186+
187+
// Relations stay trimmed to {id, slug} like eager's withRelatedFields.
188+
for (const key of ['tags', 'authors']) {
189+
if (Array.isArray(resolved[key])) {
190+
resolved[key].forEach((relation) => {
191+
assert.deepEqual(
192+
Object.keys(relation).sort(),
193+
['id', 'slug'],
194+
`${key} relation not trimmed for ${url}`
195+
);
196+
});
197+
}
198+
}
199+
}
200+
});
201+
202+
it('reverse lookup returns null for an unknown URL, like the eager service', async function () {
203+
assert.equal(eager.getResource('/this-does-not-exist/'), null);
204+
assert.equal(await lazy.resolveUrl('/this-does-not-exist/'), null);
205+
});
206+
207+
if (scenario.routes.some(r => r.permalink.includes(':year'))) {
208+
it('agrees with the eager service that a non-canonical (wrong-date) URL 404s', async function () {
209+
let checked = 0;
210+
for (const {url} of allGeneratedUrls()) {
211+
const yearMatch = url.match(/^\/(\d{4})\//);
212+
if (!yearMatch) {
213+
continue;
214+
}
215+
// Swap the year for one the post wasn't published in: a
216+
// valid permalink shape that is not this post's canonical
217+
// URL, so both services must reject it.
218+
const variant = url.replace(/^\/\d{4}\//, `/${Number(yearMatch[1]) - 1}/`);
219+
220+
assert.equal(eager.getResource(variant), null, `eager resolved non-canonical ${variant}`);
221+
assert.equal(await lazy.resolveUrl(variant), null, `lazy resolved non-canonical ${variant}`);
222+
checked += 1;
223+
}
224+
assert.ok(checked > 0, 'expected at least one date-based URL to mutate');
225+
});
226+
}
227+
});
228+
});
229+
});

0 commit comments

Comments
 (0)