Ghost/ghost/core/test/e2e-server/click-tracking.test.js
Chris Raible 5154e8d24f
Fixed race condition when updating member's last_seen_at timestamp (#20389)
ref
https://linear.app/tryghost/issue/ENG-1240/race-condition-when-updating-members-last-seen-at-timestamp
    
When members click a link in an email, Ghost updates the member's
`last_seen_at` timestamp, but it should only update the timestamp if the
member hasn't yet been seen in the current day (based on the
publication's timezone).
    
Currently there is a race condition present where multiple simultaneous
requests from the same member (if e.g. an email link checker is
following all links in an email) can cause the `last_seen_at` timestamp
to be updated multiple times in the same day for the same member. These
additional queries add a significant load on Ghost and its database,
which can contribute to the exhaustion of the connection pool and
eventually requests may time out.
    
The primary motivation for this change is to avoid that race condition
by adding a lock to the member row, checking if `last_seen_at` has
already been updated in the current day, and only updating it if it
hasn't.
    
Another beneficial side-effect of this change is that it avoids locking
the `labels` and `newsletters` tables, which are locked when we update
the `last_seen_at` timestamp in the `members` table currently. This
should improve Ghost's ability to handle a large influx of requests to
redirect endpoints (confirmed with load tests), which tend to happen
immediately after a publisher sends an email.
2024-06-18 20:03:32 -07:00

169 lines
6.7 KiB
JavaScript

const assert = require('assert/strict');
const fetch = require('node-fetch').default;
const {agentProvider, mockManager, fixtureManager, matchers} = require('../utils/e2e-framework');
const urlUtils = require('../../core/shared/url-utils');
const jobService = require('../../core/server/services/jobs/job-service');
const {anyGhostAgent, anyContentVersion, anyNumber, anyISODateTime, anyObjectId} = matchers;
describe('Click Tracking', function () {
let agent;
let webhookMockReceiver;
before(async function () {
const {adminAgent} = await agentProvider.getAgentsWithFrontend();
agent = adminAgent;
await fixtureManager.init('newsletters', 'members:newsletters', 'integrations');
await agent.loginAsOwner();
});
beforeEach(function () {
mockManager.mockMail();
mockManager.mockMailgun();
webhookMockReceiver = mockManager.mockWebhookRequests();
});
afterEach(function () {
mockManager.restore();
});
it('Full test', async function () {
const siteUrl = new URL(urlUtils.urlFor('home', true));
const {body: {posts: [draft]}} = await agent.post('/posts/?source=html', {
body: {
posts: [{
title: 'My Newsletter',
html: `<p>External link <a href="https://example.com/a">https://example.com/a</a>; Internal link <a href=${siteUrl.href}/about">${siteUrl.href}/about</a>;Ghost homepage <a href="https://ghost.org">https://ghost.org</a></p>`
}]
}
});
const newsletterSlug = fixtureManager.get('newsletters', 0).slug;
const {body: {posts: [post]}} = await agent.put(
`/posts/${draft.id}/?newsletter=${newsletterSlug}`,
{
body: {
posts: [{
updated_at: draft.updated_at,
status: 'published'
}]
}
}
);
// Wait for the newsletter to be sent
await jobService.allSettled();
// Setup a webhook listener for member.edited events
const webhookURL = 'https://test-webhook-receiver.com/member-edited/';
await webhookMockReceiver.mock(webhookURL);
await fixtureManager.insertWebhook({
event: 'member.edited',
url: webhookURL
});
const {body: {links}} = await agent.get(
`/links/?filter=${encodeURIComponent(`post_id:'${post.id}'`)}`
);
/** @type {(url: string) => Promise<import('node-fetch').Response>} */
const fetchWithoutFollowingRedirect = url => fetch(url, {redirect: 'manual'});
let internalRedirectHappened = false;
let externalRedirectHappened = false;
let poweredByGhostIgnored = true;
for (const link of links) {
const res = await fetchWithoutFollowingRedirect(link.link.from);
const redirectedToUrl = new URL(res.headers.get('location'));
// startsWith is a little dirty, but we need this because siteUrl
// can have a path when Ghost is hosted on a subdomain.
const isInternal = redirectedToUrl.href.startsWith(siteUrl.href);
if (isInternal) {
internalRedirectHappened = true;
assert(redirectedToUrl.searchParams.get('attribution_id'), 'attribution_id should be present on internal redirects');
assert(redirectedToUrl.searchParams.get('attribution_type'), 'attribution_type should be present on internal redirects');
} else {
externalRedirectHappened = true;
assert(!redirectedToUrl.searchParams.get('attribution_id'), 'attribution_id should not be present on internal redirects');
assert(!redirectedToUrl.searchParams.get('attribution_type'), 'attribution_type should not be present on internal redirects');
}
assert(redirectedToUrl.searchParams.get('ref'), 'ref should be present on all redirects');
// Powered by Ghost link should not be replaced / tracked
if (link.link.to.includes('https://ghost.org/?via=pbg-newsletter')) {
poweredByGhostIgnored = false;
}
}
assert(internalRedirectHappened);
assert(externalRedirectHappened);
assert(poweredByGhostIgnored);
const {body: {members}} = await agent.get(
`/members/`
);
const linkToClick = links[0];
const memberToClickLink = members[0];
assert(memberToClickLink.last_seen_at === null);
const urlOfLinkToClick = new URL(linkToClick.link.from);
urlOfLinkToClick.searchParams.set('m', memberToClickLink.uuid);
const previousClickCount = linkToClick.count.clicks;
await fetchWithoutFollowingRedirect(urlOfLinkToClick.href);
const {body: {links: [clickedLink]}} = await agent.get(
`/links/?filter=${encodeURIComponent(`post_id:'${post.id}'`)}`
);
const clickCount = clickedLink.count.clicks;
const {body: {events: clickEvents}} = await agent.get(
`/members/events/?filter=${encodeURIComponent(`data.member_id:'${memberToClickLink.id}'+type:click_event`)}`
);
const clickEvent = clickEvents.find((/** @type any */ event) => {
return event.data.post.id === post.id && event.data.link.from === urlOfLinkToClick.pathname;
});
assert(clickEvent);
assert(previousClickCount + 1 === clickCount);
// Ensure we updated the member's last_seen_at
const {body: {members: [memberWhoClicked]}} = await agent.get(
`/members/${memberToClickLink.id}`
);
assert(memberWhoClicked.last_seen_at !== null, 'last_seen_at should be set after a click');
assert(new Date(memberWhoClicked.last_seen_at).getTime() > 0, 'last_seen_at should be a valid date');
// Ensure we sent the webhook with the correct payload, including newsletters and labels
await webhookMockReceiver.receivedRequest();
webhookMockReceiver
.matchHeaderSnapshot({
'content-version': anyContentVersion,
'content-length': anyNumber,
'user-agent': anyGhostAgent
})
.matchBodySnapshot({
member: {
current: {
created_at: anyISODateTime,
id: anyObjectId,
last_seen_at: anyISODateTime,
updated_at: anyISODateTime
},
previous: {
last_seen_at: null,
updated_at: anyISODateTime
}
}
});
});
});