Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WORLDSERVICE-464] Remove sw.js for Public Services #12566

Merged
merged 13 commits into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,6 @@ describe('Application', () => {
});
});

it(`should return a 200 status code for ${service}'s article service worker`, () => {
cy.testResponseCodeAndType({
path: `/${config[service].name}/articles/sw.js`,
responseCode: 200,
type: 'application/javascript',
});
});

it(`should return a 200 status code for ${service} manifest file`, () => {
cy.testResponseCodeAndType({
path: `/${config[service].name}/manifest.json`,
Expand All @@ -39,14 +31,6 @@ describe('Application', () => {
});
});

it(`should return a 200 status code for ${service} article manifest file`, () => {
cy.testResponseCodeAndType({
path: `/${config[service].name}/articles/manifest.json`,
responseCode: 200,
type: 'application/json',
});
});

it(`should awaken fresh data for pages for later tests`, () => {
// Add more here if you want to awaken fresh data for other page types
if (serviceHasPageType(service, 'topicPage')) {
Expand Down
1 change: 0 additions & 1 deletion src/app/lib/config/services/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export const service: DefaultServiceConfig = {
publishingPrinciples: null,
isTrustProjectParticipant: false,
script: latin,
swPath: '/articles/sw.js',
homePageTitle: 'Home',
showAdPlaceholder: false,
showRelatedTopics: true,
Expand Down
1 change: 0 additions & 1 deletion src/app/lib/config/services/scotland.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export const service: DefaultServiceConfig = {
publishingPrinciples: null,
isTrustProjectParticipant: false,
script: latin,
swPath: '/articles/sw.js',
homePageTitle: 'Home',
passportHomes: ['BBCScotland'],
showAdPlaceholder: false,
Expand Down
5 changes: 0 additions & 5 deletions src/app/routes/utils/regex/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import services from '../../../lib/config/services/loadableConfig';
import {
getArticleRegex,
getArticleSwRegex,
getArticleManifestRegex,
getHomePageRegex,
getSwRegex,
getManifestRegex,
Expand All @@ -27,9 +25,6 @@ const allServices = Object.keys(services);
export const articlePath = getArticleRegex(allServices);
export const articleDataPath = `${articlePath}.json`;

export const articleSwPath = getArticleSwRegex(allServices);
export const articleManifestPath = getArticleManifestRegex(allServices);

export const homePageSwPath = getSwRegex(allServices);
export const homePageManifestPath = getManifestRegex(allServices);
export const homePagePath = getHomePageRegex(allServices);
Expand Down
49 changes: 5 additions & 44 deletions src/app/routes/utils/regex/index.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { matchPath } from 'react-router-dom';
import {
articleDataPath,
articleManifestPath,
articlePath,
articleSwPath,
cpsAssetPageDataPath,
cpsAssetPagePath,
homePagePath,
Expand Down Expand Up @@ -124,55 +122,16 @@ describe('homePageDataPath', () => {
shouldNotMatchInvalidRoutes(invalidRoutes, homePageDataPath);
});

describe('articleSwPath', () => {
const validRoutes = [
'/news/articles/sw.js',
'/persian/articles/sw.js',
'/cymrufyw/erthyglau/sw.js',
];
shouldMatchValidRoutes(validRoutes, articleSwPath);

const invalidRoutes = [
'/news/sw.js',
'/persian/articles/sw',
'/news/trad/sw.js',
'/cymrufyw/sw.js',
];
shouldNotMatchInvalidRoutes(invalidRoutes, articleSwPath);
});

describe('articleManifestPath', () => {
const validRoutes = [
'/persian/articles/manifest.json',
'/serbian/articles/manifest.json',
];
shouldMatchValidRoutes(validRoutes, articleManifestPath);

const invalidRoutes = [
'/news/articles/manifest.json',
'/sport/articles/manifest.json',
'/naidheachdan/sgeulachdan/manifest.json',
'/cymrufyw/erthyglau/manifest.json',
'/newsround/articles/manifest.json',
'/news/manifest.json',
'/sport/manifest.json',
'/naidheachdan/manifest.json',
'/cymrufyw/manifest.json',
'/newsround/manifest.json',
'/persian/articles/manifest',
'/news/simp/sw.js',
];
shouldNotMatchInvalidRoutes(invalidRoutes, articleManifestPath);
});

describe('homePageSwPath', () => {
const validRoutes = ['/news/sw.js', '/persian/sw.js'];
const validRoutes = ['/gahuza/sw.js', '/persian/sw.js'];
shouldMatchValidRoutes(validRoutes, homePageSwPath);

const invalidRoutes = [
'/news/sw.js',
'/news/articles/sw.js',
'/persian/sw',
'/persian/simp/sw.js',
'/gahuza/articles/sw.js',
];
shouldNotMatchInvalidRoutes(invalidRoutes, homePageSwPath);
});
Expand All @@ -190,6 +149,8 @@ describe('homePageManifestPath', () => {
'/foobar/manifest.json',
'/foobar/manifest',
'/news/trad/sw.js',
'/persian/articles/manifest.json',
'/serbian/articles/manifest.json',
];
shouldNotMatchInvalidRoutes(invalidRoutes, homePageManifestPath);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@

exports[`regex utils snapshots should create expected regex from getAfricaEyeTVPageRegex 1`] = `"/worldservice/tv/africa_eye/:episodeId([a-z0-9]+)?:lite(.lite)?"`;

exports[`regex utils snapshots should create expected regex from getArticleManifestRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:local(articles|erthyglau|sgeulachdan)/manifest.json"`;

exports[`regex utils snapshots should create expected regex from getArticleRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)?:discipline(/[a-z0-9-_]{1,})?/:local(articles|erthyglau|sgeulachdan)/:id(c[a-zA-Z0-9]{10}o):variant(/simp|/trad|/cyr|/lat)?:nonCanonicalArticleRenderPlatform(.amp|.app|.lite)?"`;

exports[`regex utils snapshots should create expected regex from getArticleSwRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:local(articles|erthyglau|sgeulachdan)/sw.js"`;

exports[`regex utils snapshots should create expected regex from getCpsAssetRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen):variant(/simp|/trad|/cyr|/lat)?/:assetUri([a-z0-9-_+]{0,}[0-9]{8,}):amp(.amp)?:lite(.lite)?"`;

exports[`regex utils snapshots should create expected regex from getErrorPageRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:errorCode(404|500):variant(/simp|/trad|/cyr|/lat)?:lite(.lite)?"`;
Expand All @@ -18,7 +14,7 @@ exports[`regex utils snapshots should create expected regex from getLegacyAssetR

exports[`regex utils snapshots should create expected regex from getLiveRadioRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:masterBrand(bbc_[a-z]+_radio)/:mediaId(liveRadio):lite(.lite)?"`;

exports[`regex utils snapshots should create expected regex from getManifestRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/manifest.json"`;
exports[`regex utils snapshots should create expected regex from getManifestRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/manifest.json"`;

exports[`regex utils snapshots should create expected regex from getMostReadDataRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/mostread:variant(/simp|/trad|/cyr|/lat)?.json"`;

Expand All @@ -36,6 +32,6 @@ exports[`regex utils snapshots should create expected regex from getRecommendati

exports[`regex utils snapshots should create expected regex from getSecondaryColumnDataRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/sty-secondary-column:variant(/simp|/trad|/cyr|/lat)?.json"`;

exports[`regex utils snapshots should create expected regex from getSwRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/sw.js"`;
exports[`regex utils snapshots should create expected regex from getSwRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/sw.js"`;

exports[`regex utils snapshots should create expected regex from getTopicPageRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/topics/:id([a-z0-9]+)?:variant(/simp|/trad|/cyr|/lat)?:lite(.lite)?"`;
13 changes: 2 additions & 11 deletions src/app/routes/utils/regex/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ export const getArticleRegex = services => {
return `/:service(${serviceRegex})?:discipline(${sportDisciplineRegex})?/:local(${articleLocalRegex})/:id(${idRegex}):variant(${variantRegex})?:nonCanonicalArticleRenderPlatform(${nonCanonicalArticleRenderPlatform})?`;
};

export const getArticleSwRegex = services => {
const serviceRegex = getServiceRegex(services);
return `/:service(${serviceRegex})/:local(${articleLocalRegex})/sw.js`;
};

const getWorldServices = services => {
const publicServices = [
'news',
Expand All @@ -35,23 +30,19 @@ const getWorldServices = services => {
'cymrufyw',
'naidheachdan',
'archive',
'scotland',
];

return services.filter(service => !publicServices.includes(service));
};

export const getArticleManifestRegex = services => {
const serviceRegex = getServiceRegex(getWorldServices(services));
return `/:service(${serviceRegex})/:local(${articleLocalRegex})/manifest.json`;
};

export const getHomePageRegex = services => {
const homePageServiceRegex = getServiceRegex(services);
return `/:service(${homePageServiceRegex}):variant(${variantRegex})?:lite(${liteRegex})?`;
};

export const getSwRegex = services => {
const serviceRegex = getServiceRegex(services);
const serviceRegex = getServiceRegex(getWorldServices(services));
return `/:service(${serviceRegex})/sw.js`;
};

Expand Down
6 changes: 2 additions & 4 deletions src/server/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import injectCspHeader from './utilities/cspHeader';
import logResponseTime from './utilities/logResponseTime';
import renderDocument from './Document';
import {
articleManifestPath,
articleSwPath,
homePageManifestPath,
homePageSwPath,
} from '../app/routes/utils/regex';
Expand Down Expand Up @@ -112,7 +110,7 @@ server
* Application env routes
*/
server
.get([articleSwPath, homePageSwPath], (req, res) => {
.get(homePageSwPath, (req, res) => {
const swPath = `${__dirname}/public/sw.js`;
res.set(
`Cache-Control`,
Expand All @@ -125,7 +123,7 @@ server
}
});
})
.get([articleManifestPath, homePageManifestPath], async ({ params }, res) => {
.get(homePageManifestPath, async ({ params }, res) => {
const { service } = params;
const variant = defaultServiceVariants[service] || 'default';
const manifestPath = `${__dirname}/public${serviceConfigs[service][variant].manifestPath}`;
Expand Down
14 changes: 6 additions & 8 deletions src/server/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -953,14 +953,14 @@ describe('Server', () => {

describe('Service workers', () => {
it('should serve a file for existing service workers', async () => {
Copy link
Contributor

@karinathomasbbc karinathomasbbc Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make sure there's still a test for existence of /world-service/sw.js? (choose one of the world services as an example)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, we should def keep it. Updated

await makeRequest('/news/articles/sw.js');
await makeRequest('/gahuza/sw.js');
expect(sendFileSpy.mock.calls[0][0]).toEqual(
path.join(__dirname, '/public/sw.js'),
);
});

it('should not serve a file for non-existing service workers', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we update this test to use one of the public services as the test (instead of some-service)

const { statusCode } = await makeRequest('/some-service/articles/sw.js');
const { statusCode } = await makeRequest('/news/sw.js');
expect(sendFileSpy.mock.calls.length).toEqual(0);
expect(statusCode).toEqual(500);
});
Expand All @@ -975,11 +975,9 @@ describe('Server', () => {

describe('Manifest json', () => {
it.each`
manifestPath | expectedManifestFile
${'/pidgin/articles/manifest.json'} | ${'/pidgin/manifest.json'}
${'/pidgin/manifest.json'} | ${'/pidgin/manifest.json'}
${'/serbian/articles/manifest.json'} | ${'/serbian/manifest.json'}
${'/serbian/manifest.json'} | ${'/serbian/manifest.json'}
manifestPath | expectedManifestFile
${'/pidgin/manifest.json'} | ${'/pidgin/manifest.json'}
${'/serbian/manifest.json'} | ${'/serbian/manifest.json'}
`(
'should serve a file for $manifestPath',
async ({ manifestPath, expectedManifestFile }) => {
Expand All @@ -997,7 +995,7 @@ describe('Server', () => {
});

it('should serve a response cache control of 1 day', async () => {
const { header } = await makeRequest('/pidgin/articles/manifest.json');
const { header } = await makeRequest('/pidgin/manifest.json');
expect(header['cache-control']).toBe(
'public, stale-if-error=172800, stale-while-revalidate=172800, max-age=86400',
);
Expand Down
Loading