WSTEAMA-749: Update service worker tests and URL generation logic #11595

alex-magana · 2024-05-06T14:14:53Z

Resolves JIRA [number]

Overall changes

Add a list of .

Code changes

public/sw.js

Limits .webp URLs pattern to only match images with .jpg and .png
as the fallback image extension.

src/app/lib/utilities/ichefURL/index.js

Add a regex pattern to ensure .webp is only added to

/news/.../cpsprodpb/<image_name>.jpg
/news/.../cpsprodpb/<image_name>.png
/news/.../cpsdevpb/<image_name>.jpg
/news/.../cpsdevpb/<image_name>.png

/ace/ws/.../cpsprodpb/<image_name>.jpg
/ace/ws/.../cpsprodpb/<image_name>.png
/ace/ws/.../cpsdevpb/<image_name>.jpg
/ace/ws/.../cpsdevpb/<image_name>.png

/ace/standard/.../cpsprodpb/<image_name>.jpg
/ace/standard/.../cpsprodpb/<image_name>.png
/ace/standard/.../cpsdevpb/<image_name>.jpg
/ace/standard/.../cpsdevpb/<image_name>.png

/ace/ws/.../amz/worldservice/<image_name>.jpg
/ace/ws/.../amz/worldservice/<image_name>.png
/ace/standard/.../amz/worldservice/<image_name>.jpg
/ace/standard/.../amz/worldservice/<image_name>.png

and excluded from

/news/.../amz/worldservice/<image_name>.jpg
/news/.../amz/worldservice/<image_name>.png

src/app/lib/utilities/ichefURL/index.test.js & src/sw.test.js

Add the pertinent tests for the logic changes above.

Testing

List the steps used to test this PR.

Helpful Links

Add Links to useful resources related to this PR if applicable.

Coding Standards

Repository use guidelines

…morgh' of github.com:bbc/simorgh into WSTEAMA-749-implement-webp-support-for-all-images-in-simorgh

LilyL0u · 2024-05-07T10:28:43Z

src/app/lib/utilities/ichefURL/index.js

@@ -1,5 +1,13 @@
 import { getEnvConfig } from '../getEnvConfig';

+const webpSupportedPatterns = [


Would this mean that according to '//ace/(?:standard|ws)/.+(?:/amz/worldservice/)?.*/', if the url has ace/standard and amz/worldservice then it will always have .webp added to the end, even if it already has. webp on the end? Do some of the ace/standard and amz/worldservice urls ever already have .webp on the end? Just because if we add .webp to one that already has .webp on the end it makes a bad url. Otherwise this looks good! But just checking that scenario. I see the other IRL in this array doesn't have that problem because it specifies .jpg or .png on the end.

Just spoke to @eagerterrier and the URLs won't already have webp on. It is only added in this buildIChefUrl file and the service worker. So this should be ok!

Brill.
I've added this test to ensure we're not appending .webp if it's already in the URL.

As regards this query, since the matching pattern checks that the URL ends with .jpg or .png as per (?:\.jpg|\.png), we wouldn't append .webp to the URL if already exists.
This is because this pattern will return false when we check for a match.

LilyL0u · 2024-05-07T14:22:11Z

public/sw.js

@@ -12,7 +12,9 @@ self.addEventListener('install', event => {

 const fetchEventHandler = async event => {
  if (
-    /^https:\/\/ichef(\.test)?\.bbci\.co\.uk\/.+\.webp$/.test(event.request.url)
+    /^https:\/\/ichef(\.test)?\.bbci\.co\.uk\/(news|ace\/(standard|ws))\/.+(\.jpg|\.png).webp$/.test(


I am thinking about how there are some images with different URL types we may also need to put .webp on in other areas of the code, and then remove it here in cases of it not being supported:

Images like the podcast promo image for example like https://ichef.bbc.co.uk/images/ic/512x512/p0htdh6t.png which we want to be https://ichef.bbc.co.uk/images/ic/512x512/p0htdh6t.png.webp. This one wouldn't fall into this condition, and so we wouldn't remove .webp in the cases webp isn't supported.

To resolve this issues, do we want to use the less specific pattern below
/^https:\/\/ichef(\.test)?\.bbci\.co\.uk\/.+\.webp$/
or expand the URL checks with something along this lines to cover each
scenario we need to support?
The limitation presented by a less specific pattern is the likelihood of matching
.webp URLs that we aren't aware of i.e we don't know where the .webp is being set.

I have looked throught the codebase and I don't see webp being set anywhere else, and it shouldn't be .webp coming from Ares. Maybe this will change when we do some of the work in the BFF to add webp in there, but for now it seems that it will be ok to make the pattern less specific to catch all ichef urls that have webp added.

Sounds good to me.
Thanks for looking looking into this concern. With this knowledge, we know we won't
encounter any surprises.
I'll change the pattern to only check for .webp at the end.

I've changed this to (news|images|ace\/(standard|ws)) to

retain the rationale behind the specificity introduced to the matching pattern here

ensure we're able to keep out these wrong paths in the URL.

to ensure the regex pattern remains performant by being able to weed out unsupported URLs based on the image path.
Do let me know if that's okay.

alex-magana added 8 commits May 3, 2024 20:32

Ensure headers are passed to the Request object

178a0f1

Match .jpg.webp and .png.webp

9c6308e

Merge branch 'WSTEAMA-749-implement-webp-support-for-all-images-in-si…

a448df1

…morgh' of github.com:bbc/simorgh into WSTEAMA-749-implement-webp-support-for-all-images-in-simorgh

Update service worker URL filter and associated tests

cdee302

Add URL checks for adding .webp extension

fb91fcd

Fix amz regex

b220666

Use nono-capture group for performance since only matches are required

6c4dc62

Add tests for ichef URL generation

af64a4b

alex-magana changed the title ~~Wsteama 749 implement webp support url downgrade changes~~ WSTEAMA-749: Update service worker tests and URL generation logic May 7, 2024

Remove redundant tests

219514c

LilyL0u reviewed May 7, 2024

View reviewed changes

alex-magana added 4 commits May 7, 2024 13:40

Ensure amz/worldservice does not apply to the news subdomain

de0b225

Refine web support check to exclude amz/worldservice for news via regex

be69d17

Ensure .webp is not appended to the URL if already included

b4a75b7

Add guide comment

097b2af

LilyL0u reviewed May 7, 2024

View reviewed changes

alex-magana added 3 commits May 8, 2024 17:56

Use the pattern in the service worker to match .webp images

dbc1398

Change service worker to match the images ichef project

5a30985

Set sad path tests in an isolated block

d2157c5

LilyL0u merged commit 01aceaa into WSTEAMA-749-implement-webp-support-for-all-images-in-simorgh May 9, 2024
5 of 9 checks passed

LilyL0u deleted the WSTEAMA-749-implement-webp-support-url-downgrade-changes branch May 9, 2024 16:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WSTEAMA-749: Update service worker tests and URL generation logic #11595

WSTEAMA-749: Update service worker tests and URL generation logic #11595

alex-magana commented May 6, 2024 •

edited

LilyL0u May 7, 2024

LilyL0u May 7, 2024

alex-magana May 7, 2024

alex-magana May 7, 2024

LilyL0u May 7, 2024

alex-magana May 7, 2024 •

edited

LilyL0u May 8, 2024

alex-magana May 8, 2024

alex-magana May 9, 2024

		@@ -1,5 +1,13 @@
		import { getEnvConfig } from '../getEnvConfig';

		const webpSupportedPatterns = [

WSTEAMA-749: Update service worker tests and URL generation logic #11595

WSTEAMA-749: Update service worker tests and URL generation logic #11595

Conversation

alex-magana commented May 6, 2024 • edited

Overall changes

Code changes

Testing

Helpful Links

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-magana May 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-magana commented May 6, 2024 •

edited

alex-magana May 7, 2024 •

edited