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

Cache SLAS callback using request processor #884

Merged
merged 9 commits into from
Jan 12, 2023
44 changes: 22 additions & 22 deletions packages/template-retail-react-app/app/request-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
// 'request-processor.js' and update the processRequest function so that
// it processes requests in whatever way your project requires.

// Uncomment the following line for the example code to work.
// import {QueryParameters} from 'pwa-kit-react-sdk/utils/ssr-request-processing'
Copy link
Collaborator Author

@kevinxh kevinxh Jan 12, 2023

Choose a reason for hiding this comment

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

This QueryParameters class is a custom version of the javascript native API URLSearchParams. I don't see a reason why we have to use QueryParameters?

The request processor should be super lean and fast since it is called on every request at the edge, we should not advocate for importing libraries from this file, especially when we have native alternatives.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the code in the SDK, the reason that we are using this custom QueryParameters class is because the order of queries is important (?a=1&b=2 might be a hit, but ?b=1&a=2 might not). The QueryParameters is here to maintain order when you are manipulating the search string.

Although not using that class doesn't effect this particular situation, the removal of the example boilerplate code might mean that partners that do decide to make changes to the request processor will do so incorrectly not knowing about this restriction.

Copy link
Contributor

Choose a reason for hiding this comment

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

URLSearchParams also preserves the order; the key differences seem to be how null is handled and how spaces are encoded.


/**
* The processRequest function is called for *every* non-proxy, non-bundle
* request received. That is, all requests that will result in pages being
Expand Down Expand Up @@ -62,40 +59,42 @@ export const processRequest = ({
path,
querystring
}) => {
// Uncomment the snippet below for the example code to work.
/***************************************************************************
// Example code for filtering query parameters and detecting bots user.

console.assert(parameters, 'Missing parameters')

// This is an EXAMPLE processRequest implementation. You should
// replace it with code that processes your requests as needed.

// This example code will remove any of the parameters whose keys appear
// in the 'exclusions' array.
const exclusions = [
'gclid',
'utm_campaign',
'utm_content',
'utm_medium',
'utm_source'
// 'gclid',
// 'utm_campaign',
// 'utm_content',
// 'utm_medium',
// 'utm_source'
]

// This is a performance optimization for SLAS.
// On client side, browser always follow the redirect
// to /callback but the response is always the same.
// We strip out the unique query parameters so this
// endpoint is cached at the CDN level
if (path === '/callback') {
exclusions.push('usid')
exclusions.push('code')
}

// Build a first QueryParameters object from the given querystring
const incomingParameters = new QueryParameters(querystring)
const incomingParameters = new URLSearchParams(querystring)

// Build a second QueryParameters from the first, with all
// excluded parameters removed
const filteredParameters = QueryParameters.from(
incomingParameters.parameters.filter(
// parameter.key is always lower-case
(parameter) => !exclusions.includes(parameter.key)
)
)
exclusions.forEach((key) => {
incomingParameters.delete(key)
})

// Re-generate the querystring
querystring = filteredParameters.toString()
querystring = incomingParameters.toString()

/***************************************************************************
// This example code will detect bots by examining the user-agent,
// and will set the request class to 'bot' for all such requests.
const ua = headers.getHeader('user-agent')
Expand All @@ -105,6 +104,7 @@ export const processRequest = ({
setRequestClass('bot')
}
***************************************************************************/

// Return the path unchanged, and the updated query string
return {
path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,11 @@ describe('processRequest', () => {
expect(result.path).toEqual(expect.any(String))
expect(result.querystring).toEqual(expect.any(String))
})

test('SLAS callback parameters are removed', () => {
const result = processRequest({path: '/callback', querystring: 'usid=1&code=2&test=3'})

expect(result.path).toEqual('/callback')
expect(result.querystring).toEqual('test=3')
})
})
3 changes: 3 additions & 0 deletions packages/template-retail-react-app/app/ssr.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ const {handler} = runtime.createHandler(options, (app) => {

// Handle the redirect from SLAS as to avoid error
app.get('/callback?*', (req, res) => {
// This endpoint does nothing and is not expected to change
// Thus we cache it for a year to maximize performance
res.set('Cache-Control', `max-age=31536000`)
res.send()
})
app.get('/robots.txt', runtime.serveStaticFile('static/robots.txt'))
Expand Down