feat: Replace url parsing with URL class (#781)

newrelic · Oct 23, 2023 · 4206263 · 4206263
1 parent 5f56118
commit 4206263
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 164 deletions.
diff --git a/jest.config.js b/jest.config.js
@@ -1,16 +1,5 @@
 const commonConfig = {
   clearMocks: true,
-  coverageDirectory: 'coverage',
-  collectCoverageFrom: [
-    'src/**/*.js',
-    '!src/**/*.test.js',
-    '!src/**/*.component-test.js',
-    '!src/index.js',
-    '!src/cdn/**/*.js',
-    '!src/features/*/index.js',
-    '!src/features/*/constants.js',
-    '!src/loaders/features/features.js'
-  ],
   modulePathIgnorePatterns: ['<rootDir>/temp'],
   testEnvironment: 'jsdom',
   transform: {
@@ -23,6 +12,17 @@ const commonConfig = {
 }
 
 module.exports = {
+  coverageDirectory: 'coverage',
+  collectCoverageFrom: [
+    'src/**/*.js',
+    '!src/**/*.test.js',
+    '!src/**/*.component-test.js',
+    '!src/index.js',
+    '!src/cdn/**/*.js',
+    '!src/features/*/index.js',
+    '!src/features/*/constants.js',
+    '!src/loaders/features/features.js'
+  ],
   projects: [
     {
       ...commonConfig,

diff --git a/src/cdn/polyfills.js b/src/cdn/polyfills.js
@@ -21,3 +21,4 @@ import 'core-js/stable/weak-set'
 import 'core-js/stable/object/get-own-property-descriptors'
 import 'core-js/stable/url'
 import 'core-js/stable/url-search-params'
+import 'core-js/stable/string/starts-with'
diff --git a/src/common/url/parse-url.js b/src/common/url/parse-url.js
@@ -3,70 +3,40 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { globalScope, isBrowserScope } from '../constants/runtime'
-
-var stringsToParsedUrls = {}
+import { globalScope } from '../constants/runtime'
 
 export function parseUrl (url) {
-  if (url in stringsToParsedUrls) {
-    return stringsToParsedUrls[url]
-  }
-
   // Return if URL is a data URL, parseUrl assumes urls are http/https
   if ((url || '').indexOf('data:') === 0) {
     return {
       protocol: 'data'
     }
   }
 
-  let urlEl
-  var location = globalScope?.location
-  var ret = {}
-
   try {
-    urlEl = new URL(url, location.href)
-  } catch (err) {
-    if (isBrowserScope) {
-    // Use an anchor dom element to resolve the url natively.
-      urlEl = document.createElement('a')
-      urlEl.href = url
-    } else {
-      return ret
+    const parsedUrl = new URL(url, location.href)
+    const returnVal = {
+      port: parsedUrl.port,
+      hostname: parsedUrl.hostname,
+      pathname: parsedUrl.pathname,
+      search: parsedUrl.search,
+      protocol: parsedUrl.protocol.slice(0, parsedUrl.protocol.indexOf(':')),
+      sameOrigin: parsedUrl.protocol === globalScope?.location?.protocol && parsedUrl.host === globalScope?.location?.host
     }
-  }
-
-  ret.port = urlEl.port
-
-  ret.search = urlEl.search
 
-  var firstSplit = urlEl.href.split('://')
-
-  if (!ret.port && firstSplit[1]) {
-    ret.port = firstSplit[1].split('/')[0].split('@').pop().split(':')[1]
-  }
-  if (!ret.port || ret.port === '0') ret.port = (firstSplit[0] === 'https' ? '443' : '80')
-
-  // Host not provided in IE for relative urls
-  ret.hostname = (urlEl.hostname || location.hostname)
-
-  ret.pathname = urlEl.pathname
-
-  ret.protocol = firstSplit[0]
-
-  // Pathname sometimes doesn't have leading slash (IE 8 and 9)
-  if (ret.pathname.charAt(0) !== '/') ret.pathname = '/' + ret.pathname
-
-  // urlEl.protocol is ':' in old ie when protocol is not specified
-  var sameProtocol = !urlEl.protocol || urlEl.protocol === ':' || urlEl.protocol === location.protocol
-  var sameDomain = urlEl.hostname === location.hostname && urlEl.port === location.port
+    if (!returnVal.port || returnVal.port === '') {
+      if (parsedUrl.protocol === 'http:') returnVal.port = '80'
+      if (parsedUrl.protocol === 'https:') returnVal.port = '443'
+    }
 
-  // urlEl.hostname is not provided by IE for relative urls, but relative urls are also same-origin
-  ret.sameOrigin = sameProtocol && (!urlEl.hostname || sameDomain)
+    if (!returnVal.pathname || returnVal.pathname === '') {
+      returnVal.pathname = '/'
+    } else if (!returnVal.pathname.startsWith('/')) {
+      returnVal.pathname = `/${returnVal.pathname}`
+    }
 
-  // Only cache if url doesn't have a path
-  if (ret.pathname === '/') {
-    stringsToParsedUrls[url] = ret
+    return returnVal
+  } catch (err) {
+    return {}
   }
-
-  return ret
 }
diff --git a/src/common/url/parse-url.test.js b/src/common/url/parse-url.test.js
@@ -1,3 +1,5 @@
+import { parseUrl } from './parse-url'
+
 afterEach(() => {
   jest.resetModules()
   jest.clearAllMocks()
@@ -67,78 +69,6 @@ const urlTests = [
   }
 ]
 
-test.each(urlTests)('verify url parsing inside browser scope', async ({ input, expected }) => {
-  jest.doMock('../constants/runtime', () => ({
-    __esModule: true,
-    isBrowserScope: true,
-    globalScope: global
-  }))
-
-  const { parseUrl } = await import('./parse-url')
-  expect(parseUrl(input)).toEqual(expected)
-})
-
-test.each(urlTests)('verify url parsing outside browser scope', async ({ input, expected }) => {
-  jest.doMock('../constants/runtime', () => ({
-    __esModule: true,
-    isBrowserScope: false,
-    globalScope: global
-  }))
-
-  const { parseUrl } = await import('./parse-url')
-  expect(parseUrl(input)).toEqual(expected)
-})
-
-test('should cache parsed urls', async () => {
-  jest.doMock('../constants/runtime', () => ({
-    __esModule: true,
-    isBrowserScope: true,
-    globalScope: global
-  }))
-
-  const input = 'http://example.com/'
-  const expected = {
-    hostname: 'example.com',
-    pathname: '/',
-    protocol: 'http',
-    port: '80',
-    sameOrigin: false,
-    search: ''
-  }
-
-  jest.spyOn(document, 'createElement')
-
-  const { parseUrl } = await import('./parse-url')
-  parseUrl(input)
-
-  expect(parseUrl(input)).toEqual(expected)
-  expect(document.createElement).toHaveBeenCalledTimes(0)
-})
-
-test('should use createElement as fallback', async () => {
-  jest.doMock('../constants/runtime', () => ({
-    __esModule: true,
-    isBrowserScope: true,
-    globalScope: global
-  }))
-
-  global.URL = jest.fn(() => { throw new Error('test') })
-
-  const input = 'http://example.com/'
-  const expected = {
-    hostname: 'example.com',
-    pathname: '/',
-    protocol: 'http',
-    port: '80',
-    sameOrigin: false,
-    search: ''
-  }
-
-  jest.spyOn(document, 'createElement')
-
-  const { parseUrl } = await import('./parse-url')
-  parseUrl(input)
-
+test.each(urlTests)('verify url parsing', async ({ input, expected }) => {
   expect(parseUrl(input)).toEqual(expected)
-  expect(document.createElement).toHaveBeenCalledTimes(1)
 })
diff --git a/tests/specs/graphql-metadata.e2e.js b/tests/specs/graphql-metadata.e2e.js
@@ -3,11 +3,13 @@ import { notIE, notMobile } from '../../tools/browser-matcher/common-matchers.mj
 // ios+android with saucelabs does not honor the window load to induce ajax into ixn reliably. omitting from test for now until more elegant solution is reached.
 describe.withBrowsersMatching([notIE, notMobile])('GraphQL metadata is appended to relevant ajax calls', () => {
   it('adds GQL metadata to both standalone and interation ajax calls', async () => {
-    await browser.url(await browser.testHandle.assetURL('test-builds/library-wrapper/apollo-client.html', { init: { ajax: { block_internal: false } } })) // Setup expects before loading the page
-
     const [ixnEvents] = await Promise.all([
       browser.testHandle.expectInteractionEvents(),
-      browser.waitForAgentLoad()
+      await browser.url(
+        await browser.testHandle.assetURL(
+          'test-builds/library-wrapper/apollo-client.html',
+          { init: { ajax: { block_internal: false } } })
+      )
     ])
 
     const [ajaxEvents] = await Promise.all([
@@ -16,29 +18,38 @@ describe.withBrowsersMatching([notIE, notMobile])('GraphQL metadata is appended
         window.sendGQL()
       })])
 
-    const ixnJson = ixnEvents.request.body[0].children.find(x => x.domain.includes('flyby-router'))
-    const ajaxJson = ajaxEvents.request.body.find(x => x.domain.includes('flyby-router')) // apollo's test server
-
     // operationName: `initialPageLoad` is called during page load (page load browser ixn)
-    expect(ixnJson.children).toEqual(expect.arrayContaining([
-      {
-        type: 'stringAttribute',
-        key: 'operationName',
-        value: 'initialPageLoad'
-      },
-      { type: 'stringAttribute', key: 'operationType', value: 'query' },
-      { type: 'stringAttribute', key: 'operationFramework', value: 'GraphQL' }
-    ]))
+    expect(ixnEvents.request.body).toEqual(expect.arrayContaining([expect.objectContaining({
+      type: 'interaction',
+      trigger: 'initialPageLoad',
+      children: expect.arrayContaining([expect.objectContaining({
+        type: 'ajax',
+        domain: 'flyby-router-demo.herokuapp.com:443',
+        children: expect.arrayContaining([
+          {
+            type: 'stringAttribute',
+            key: 'operationName',
+            value: 'initialPageLoad'
+          },
+          { type: 'stringAttribute', key: 'operationType', value: 'query' },
+          { type: 'stringAttribute', key: 'operationFramework', value: 'GraphQL' }
+        ])
+      })])
+    })]))
 
     // operationName: `standalone` is called when we execute `window.sendGQL()` (standalone ajax)
-    expect(ajaxJson.children).toEqual(expect.arrayContaining([
-      {
-        type: 'stringAttribute',
-        key: 'operationName',
-        value: 'standalone'
-      },
-      { type: 'stringAttribute', key: 'operationType', value: 'query' },
-      { type: 'stringAttribute', key: 'operationFramework', value: 'GraphQL' }
-    ]))
+    expect(ajaxEvents.request.body).toEqual(expect.arrayContaining([expect.objectContaining({
+      type: 'ajax',
+      domain: 'flyby-router-demo.herokuapp.com:443',
+      children: expect.arrayContaining([
+        {
+          type: 'stringAttribute',
+          key: 'operationName',
+          value: 'standalone'
+        },
+        { type: 'stringAttribute', key: 'operationType', value: 'query' },
+        { type: 'stringAttribute', key: 'operationFramework', value: 'GraphQL' }
+      ])
+    })]))
   })
 })
diff --git a/tools/testing-server/plugins/agent-injector/debug-shim.js b/tools/testing-server/plugins/agent-injector/debug-shim.js
@@ -32,28 +32,28 @@ module.exports = `
       origOnError(arguments)
     }
   }
-  var origLog = window.console.log
+  var origLog = window.console.log.bind(window.console)
   window.console.log = function() {
     NRDEBUG('console.log: ' + JSON.stringify(arguments))
     if (typeof origLog === 'function') {
       origLog(arguments)
     }
   }
-  var origWarn = window.console.warn
+  var origWarn = window.console.warn.bind(window.console)
   window.console.warn = function() {
     NRDEBUG('console.warn: ' + JSON.stringify(arguments))
     if (typeof origWarn === 'function') {
-      origWarn(arguments)
+      //origWarn(arguments)
     }
   }
-  var origErr = window.console.error
+  var origErr = window.console.error.bind(window.console)
   window.console.error = function() {
     NRDEBUG('console.error: ' + JSON.stringify(arguments))
     if (typeof origErr === 'function') {
       origErr(arguments)
     }
   }
-  var origTrace = window.console.trace
+  var origTrace = window.console.trace.bind(window.console)
   window.console.trace = function() {
     NRDEBUG('console.trace: ' + JSON.stringify(arguments))
     if (typeof origTrace === 'function') {