feat(interceptor): duplicate query calls throw (#1630)

* feat(interceptor): duplicate query calls throw Continuation of #1626 BREAKING CHANGE: Attempting to call `Interceptor.query` twice throws an error.
nock · Jul 16, 2019 · 2a54482 · 2a54482
1 parent f015929
commit 2a54482
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 42 deletions.
diff --git a/lib/interceptor.js b/lib/interceptor.js
@@ -50,6 +50,7 @@ function Interceptor(scope, uri, method, requestBody, interceptorOptions) {
   }${uri}`
   this.basePath = this.scope.basePath
   this.path = uriIsStr ? scope.basePathname + uri : uri
+  this.queries = null
 
   this.options = interceptorOptions || {}
   this.counter = 1
@@ -470,7 +471,9 @@ Interceptor.prototype.basicAuth = function basicAuth(options) {
  * nock('http://zombo.com').get('/').query({q: 't'});
  */
 Interceptor.prototype.query = function query(queries) {
-  this.queries = this.queries || {}
+  if (this.queries !== null) {
+    throw Error(`Query parameters have already been already defined`)
+  }
 
   // Allow all query strings to match this route
   if (queries === true) {
@@ -488,17 +491,18 @@ Interceptor.prototype.query = function query(queries) {
     strFormattingFn = common.percentDecode
   }
 
-  const entries =
-    queries instanceof url.URLSearchParams ? queries : Object.entries(queries)
+  if (queries instanceof url.URLSearchParams) {
+    // Normalize the data into the shape that is matched against.
+    // Duplicate keys are handled by combining the values into an array.
+    queries = qs.parse(queries.toString())
+  } else if (!_.isPlainObject(queries)) {
+    throw Error(`Argument Error: ${queries}`)
+  }
 
-  for (const [key, value] of entries) {
+  this.queries = {}
+  for (const [key, value] of Object.entries(queries)) {
     const formatted = common.formatQueryValue(key, value, strFormattingFn)
     const [formattedKey, formattedValue] = formatted
-
-    if (formattedKey in this.queries) {
-      throw Error(`${formattedKey} already defined as a query parameter`)
-    }
-
     this.queries[formattedKey] = formattedValue
   }
 

diff --git a/tests/test_allow_unmocked_https.js b/tests/test_allow_unmocked_https.js
@@ -38,35 +38,6 @@ test('Nock with allowUnmocked and an url match', async t => {
   server.close()
 })
 
-test('Nock with allowUnmocked, url match and query false', async t => {
-  const options = {
-    key: fs.readFileSync('tests/ssl/ca.key'),
-    cert: fs.readFileSync('tests/ssl/ca.crt'),
-  }
-
-  const server = https
-    .createServer(options, (req, res) => {
-      res.writeHead(200)
-      res.end(JSON.stringify({ status: 'default' }))
-    })
-    .listen(3000)
-
-  const url = `https://127.0.0.1:3000`
-
-  nock(`${url}`, { allowUnmocked: true })
-    .get('/')
-    .query(false)
-    .reply(200, { status: 'intercepted' })
-
-  const { body } = await got(`${url}/otherpath`, {
-    rejectUnauthorized: false,
-  })
-
-  t.true(JSON.parse(body).status === 'default')
-
-  server.close()
-})
-
 test('allow unmocked option works with https', t => {
   t.plan(6)
 

diff --git a/tests/test_query.js b/tests/test_query.js
@@ -121,19 +121,46 @@ test('query() accepts URLSearchParams as input', async t => {
   scope.done()
 })
 
-test('query() throws for duplicate keys', async t => {
+test('query() throws if query params have already been defined', async t => {
+  const interceptor = nock('http://example.test').get('/?foo=bar')
+
+  t.throws(
+    () => {
+      interceptor.query({ foo: 'baz' })
+    },
+    {
+      message: 'Query parameters have already been already defined',
+    }
+  )
+})
+
+test('query() throws if query() was already called', async t => {
   const interceptor = nock('http://example.test')
     .get('/')
     .query({ foo: 'bar' })
 
   t.throws(
     () => {
-      interceptor.query({ foo: 'baz' })
+      interceptor.query({ baz: 'qux' })
+    },
+    {
+      message: 'Query parameters have already been already defined',
+    }
+  )
+})
+
+test('query() throws for invalid arguments', t => {
+  const interceptor = nock('http://example.test').get('/')
+
+  t.throws(
+    () => {
+      interceptor.query('foo=bar')
     },
     {
-      message: 'foo already defined as a query parameter',
+      message: 'Argument Error: foo=bar',
     }
   )
+  t.done()
 })
 
 test('query() matches a query string that contains special RFC3986 characters', t => {
@@ -304,7 +331,7 @@ test('query() will not match when a query string is present that was not registe
     .query({ foo: 'bar' })
     .reply(200)
 
-  mikealRequest('https://example.test/c?foo=bar&baz=foz', function(err, res) {
+  mikealRequest('https://example.test/c?foo=bar&baz=foz', function(err) {
     t.equal(
       err.message.trim(),
       `Nock: No match for request ${JSON.stringify(