chore: improve getFieldValue (nodejs#2785)

* chore: improve getFieldValue * remove * Update benchmarks/cacheGetFieldValues.mjs
crysmags · Feb 27, 2024 · a78cd05 · a78cd05
1 parent 3451a06
commit a78cd05
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 12 deletions.
diff --git a/benchmarks/cacheGetFieldValues.mjs b/benchmarks/cacheGetFieldValues.mjs
@@ -0,0 +1,23 @@
+import { bench, group, run } from 'mitata'
+import { getFieldValues } from '../lib/cache/util.js'
+
+const values = [
+  '',
+  'foo',
+  'invälid',
+  'foo, ',
+  'foo, bar',
+  'foo, bar, baz',
+  'foo, bar, baz, ',
+  'foo, bar, baz, , '
+]
+
+group('getFieldValues', () => {
+  bench('getFieldValues', () => {
+    for (let i = 0; i < values.length; ++i) {
+      getFieldValues(values[i])
+    }
+  })
+})
+
+await run()
diff --git a/lib/cache/cache.js b/lib/cache/cache.js
@@ -1,7 +1,7 @@
 'use strict'
 
 const { kConstruct } = require('./symbols')
-const { urlEquals, fieldValues: getFieldValues } = require('./util')
+const { urlEquals, getFieldValues } = require('./util')
 const { kEnumerableProperty, isDisturbed } = require('../core/util')
 const { webidl } = require('../fetch/webidl')
 const { Response, cloneResponse, fromInnerResponse } = require('../fetch/response')

diff --git a/lib/cache/util.js b/lib/cache/util.js
@@ -23,27 +23,23 @@ function urlEquals (A, B, excludeFragment = false) {
  * @see https://github.com/chromium/chromium/blob/694d20d134cb553d8d89e5500b9148012b1ba299/content/browser/cache_storage/cache_storage_cache.cc#L260-L262
  * @param {string} header
  */
-function fieldValues (header) {
+function getFieldValues (header) {
   assert(header !== null)
 
   const values = []
 
   for (let value of header.split(',')) {
     value = value.trim()
 
-    if (!value.length) {
-      continue
-    } else if (!isValidHeaderName(value)) {
-      continue
+    if (isValidHeaderName(value)) {
+      values.push(value)
     }
-
-    values.push(value)
   }
 
   return values
 }
 
 module.exports = {
   urlEquals,
-  fieldValues
+  getFieldValues
 }
diff --git a/lib/fetch/util.js b/lib/fetch/util.js
@@ -111,9 +111,7 @@ function isValidReasonPhrase (statusText) {
  * @see https://fetch.spec.whatwg.org/#header-name
  * @param {string} potentialValue
  */
-function isValidHeaderName (potentialValue) {
-  return isValidHTTPToken(potentialValue)
-}
+const isValidHeaderName = isValidHTTPToken
 
 /**
  * @see https://fetch.spec.whatwg.org/#header-value

diff --git a/test/cache/get-field-values.js b/test/cache/get-field-values.js
@@ -0,0 +1,19 @@
+'use strict'
+
+const { deepStrictEqual, throws } = require('node:assert')
+const { test } = require('node:test')
+const { getFieldValues } = require('../../lib/cache/util')
+
+test('getFieldValues', () => {
+  throws(() => getFieldValues(null), {
+    name: 'AssertionError',
+    message: 'The expression evaluated to a falsy value:\n\n  assert(header !== null)\n'
+  })
+  deepStrictEqual(getFieldValues(''), [])
+  deepStrictEqual(getFieldValues('foo'), ['foo'])
+  deepStrictEqual(getFieldValues('invälid'), [])
+  deepStrictEqual(getFieldValues('foo, bar'), ['foo', 'bar'])
+  deepStrictEqual(getFieldValues('foo, bar, baz'), ['foo', 'bar', 'baz'])
+  deepStrictEqual(getFieldValues('foo, bar, baz, '), ['foo', 'bar', 'baz'])
+  deepStrictEqual(getFieldValues('foo, bar, baz, , '), ['foo', 'bar', 'baz'])
+})