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.

Sign up for GitHub

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

Jump to bottom

Cache SLAS callback using request processor #884

Merged
merged 9 commits into from
Jan 12, 2023
Merged

Cache SLAS callback using request processor #884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c604570
cache callback in request processor
kevinxh Jan 6, 2023
20427e5
fix import path
kevinxh Jan 9, 2023
14b16d2
cache callback for a year
kevinxh Jan 10, 2023
219f487
use native URLSearchParams
kevinxh Jan 12, 2023
cb9fdf1
Merge branch 'develop' into feature/callback-rp
kevinxh Jan 12, 2023
1ede940
Merge branch 'develop' into feature/callback-rp
kevinxh Jan 12, 2023
74fa924
revert use native URLSearchParams
kevinxh Jan 12, 2023
cc07e38
Merge branch 'feature/callback-rp' of https://github.com/SalesforceCo…
kevinxh Jan 12, 2023
1fd4369
Merge branch 'develop' into feature/callback-rp
kevinxh Jan 12, 2023
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
29 changes: 17 additions & 12 deletions packages/template-retail-react-app/app/request-processor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// 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 •
edited
Loading

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.

import {QueryParameters} from 'pwa-kit-runtime/utils/ssr-request-processing'

/**
* The processRequest function is called for *every* non-proxy, non-bundle
Expand Down Expand Up @@ -62,25 +62,29 @@ 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)

Expand All @@ -96,6 +100,7 @@ export const processRequest = ({
// Re-generate the querystring
querystring = filteredParameters.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 Down
7 changes: 7 additions & 0 deletions packages/template-retail-react-app/app/request-processor.test.js
View file
Open in desktop
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
View file
Open in desktop
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