fix(cdn): MERC-9364 Use CDN Original images for webdav - cache control

[christensenep]

What? Why?

! DO NOT MERGE !

Use CDN Original images for webdav - cache control. Take 2. See bigcommerce/paper#343

If a developer/merchant uses the cdn handlebars helper to inject images from webdav into the theme using e.g.({{cdn "webdav:/img/image.jpg"}}), there is no cache-control header when the image is fetched on the storefront.

Similar to our Image Manager, we should be using the cdn original images (Similar to Jinsoo's PR here MERC-7127)

This PR differs from the previous as it only does this for file suffixes that image manager supports (jpg/jpeg/png/gif). This avoids the issue where the url is changed for e.g. a pdf, and thus creates a broken link.

How was it tested?

Tested locally and in integration, and with unit tests. Have verified that images (png/gif/jpeg/jpg) uploaded to the content directory via webdav can be accessed at their cache-controlled url using the cdn helper (/images/stencil/original/content/<whatever>), while all other file extensions still return the /content/<whatever> path when using the helper.

Behold, the results of outputting some of these urls directly to the DOM:

cc @bigcommerce/storefront-team

[christensenep]

♻️ I have tested this with gifs, jpeg/jpgs and pngs, and verified that the new urls are valid and have cache control headers. I have also verified that other file extensions such as pdfs and, importantly, bmps do not change the url from its current form and are thus still valid. I also verified that bmps do not have the images/stencil/content form of the url available, so I believe this is the correct and exhaustive of extensions we should be doing this for.

[christensenep]

@bigcommerce/team-merchandising @bigcommerce/team-storefront

[jmwiese]

LGTM

[bcsnyk]

🎉 Snyk checks have passed. No issues have been found so far.

✅ code/snyk check is complete. No issues have been found. (View Details)

[bc-alexsaiannyi]

FYI. currently 2.8.1 is broken and temporary downgraded to 2.8.0

[jkanive]

Do we need to upgrade this as part of this PR? I am asking because if we need to revert it might not be straightforward PR revert.

[bc-alexsaiannyi]

yes, since we have to update storefront-renderer as well based on these changes , it will break it also. I guess since we won't merge it right now, later the version will be updated anyway. And we also have experiment for now, so we should be safe.

[rtalvarez]

i guess im also not following, why do we need the bump? your changes dont look to require this update. maybe im missing something

also i dont see how the experiment would protect you from this version bump (also missing package-lock changes either way ?)

[bc-alexsaiannyi]

@rtalvarez, it' was a blocker for running jobs. Most of them were failed due to Error: Cannot find module '../../../../src/common/abstractRefiners' that eventually was cased by bad release of chrono-node dependency. Similar issue can be found here. So I went to Jairo asking him how much it's critical and since we're not going to merge this branch, if we can skip it. My understanding - since we need to update SFR based on paper-handlebars changes, probably dependencies will be reinstalled under the hood with broken version of chrono-node anyway. cc @jairo-bc

[bc-alexsaiannyi]

in terms of experiment I only mean that we simply can turn it off, if something is broken and SFR would use original version of handlebars.
package-lock is under gitignore and i don't see it in the project structure as well.

[jairo-bc]

@bc-alexsaiannyi feel free to merge master into your branch, this change is already there

[bc-alexsaiannyi]

great, thanks!