[Recorder] Single quotes in the url-path - careful revert of the reverted "workaround"

sdk/test-utils/recorder/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## 1.0.0 (Unreleased)
 
+## 2020-01-28
+
+- Single quotes can be present in the url path though unusual. When the url path has single quotes, the fixture generated by "nock" is incorrect leading to invalid recordings. [#13474](https://github.com/Azure/azure-sdk-for-js/pull/13474) introduces a workaround to be applied as part of the default customizations done on the generated recordings to fix the issue.
+  (Nock Bug 🐛: https://github.com/nock/nock/issues/2136)
+
 ## 2020-12-02
 
 - Refactored the code to enable `"browser"` module replacement when used with bundlers. Previously, even browser bundled copies of the recorder would carry dependencies on several Node packages, which would lead to unresolved dependency warnings in Rollup regarding node builtins. With this change, the recorder's browser mappings will avoid this issue.

sdk/test-utils/recorder/src/baseRecorder.ts

@@ -8,7 +8,8 @@ import {
   filterSecretsFromStrings,
   filterSecretsRecursivelyFromJSON,
   generateTestRecordingFilePath,
-  decodeHexEncodingIfExistsInNockFixture
+  decodeHexEncodingIfExistsInNockFixture,
+  handleSingleQuotesInUrlPath
 } from "./utils";
 
 /**
@@ -41,7 +42,11 @@ export abstract class BaseRecorder {
   private defaultCustomizationsOnRecordings = !isBrowser()
     ? [
         // Decodes "hex" strings in the response from the recorded fixture if any exists.
-        decodeHexEncodingIfExistsInNockFixture
+        decodeHexEncodingIfExistsInNockFixture,
+        // Nock bug: Single quotes in the path of the url are not handled by nock.
+        // (Link to the bug 🐛: https://github.com/nock/nock/issues/2136)
+        // The following is the workaround we use in the recorder until nock fixes it.
+        handleSingleQuotesInUrlPath
       ]
     : [];
 

sdk/test-utils/recorder/src/utils/index.ts

@@ -389,6 +389,53 @@ export function decodeHexEncodingIfExistsInNockFixture(fixture: string): string
   return fixture;
 }
 
+/**
+ * Meant for node recordings only!
+ *
+ * Single quotes can be present in the url path though unusual.
+ * When the url path has single quotes, the fixture generated by "nock" is incorrect
+ * since it doesn't consider the case. (Nock Bug 🐛: https://github.com/nock/nock/issues/2136)
+ *  Examples below:
+ *   .delete('/Tables('node')')
+ *   .get('/Tables('node')')
+ *   .post('/Tables('node')', {"TableName":"testTablenode"})
+ *
+ * The above problem results in invalid recordings.
+ *
+ * To avoid this problem, we replace the single quotes surrounding the url-path in the recording
+ * with backticks(`). This would fix the invalid recordings.
+ *
+ * @private
+ * @param {string} fixture
+ */
+export function handleSingleQuotesInUrlPath(fixture: string): string {
+  let updatedFixture = fixture;
+  if (!isBrowser()) {
+    // Fixtures would contain url-path as shown below
+    // Case-1: .{method}('{url-path}')
+    // Case-2: .{method}('{url-path}', {json-object})
+    // Examples:
+    // .get('/Tables('node')')
+    // .post('/Tables('node')', {"TableName":"node"})
+    // .post('/Tables('node')', "--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1')")
+
+    // Case-1
+    const matches = fixture.match(/\.(get|put|post|delete)\(\'(.*)\'\)\n\s*(.query\(true\))/);
+    if (matches && matches[2]) {
+      const match = matches[2]; // Extracted url-path
+      // If the url-path contains a single quote
+      if (match.search("'") !== -1) {
+        // Replace the occurrence of surrounding single quotes with backticks
+        updatedFixture = fixture.replace("'" + match + "'", "`" + match + "`");
+      }
+    }
+
+    // Case-2
+    // TODO: To handle the presence of request bodies
+  }
+  return updatedFixture;
+}
+
 /**
  * List of binary content types.
  * Currently, "avro/binary" is the only one present.

sdk/test-utils/recorder/test/node/utils.spec.ts

@@ -3,7 +3,8 @@ import {
   isBrowser,
   testHasChanged,
   isContentTypeInNockFixture,
-  decodeHexEncodingIfExistsInNockFixture
+  decodeHexEncodingIfExistsInNockFixture,
+  handleSingleQuotesInUrlPath
 } from "../../src/utils";
 
 import { nodeRequireRecordingIfExists, findRecordingsFolderPath } from "../../src/utils/recordings";
@@ -394,4 +395,170 @@ describe("NodeJS utils", () => {
       });
     });
   });
+
+  describe("handleSingleQuotesInUrlPath", () => {
+    [
+      // {
+      //   name: `single quotes in get request in the fixture with request body`,
+      //   input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .get('/'path'', "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?><QueryRequest><Expression>select * from BlobStorage</Expression></QueryRequest>")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`,
+      //   output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .get(\`/'path'\`, "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?><QueryRequest><Expression>select * from BlobStorage</Expression></QueryRequest>")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`
+      // },
+      {
+        name: `single quotes in get request in the fixture with no request body`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .get('/'path'')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .get(\`/'path'\`)
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      },
+      {
+        name: `single quotes in delete request in the fixture with no request body`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete('/'path'pathx')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete(\`/'path'pathx\`)
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      },
+      {
+        name: `no single quotes in get request in the fixture`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete('/path')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete('/path')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      },
+      {
+        name: `more than two single quotes in delete request in the fixture`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete('/p'''a't'h')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete(\`/p'''a't'h\`)
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      }
+      // {
+      //   name: `no single quotes in the path and single quotes in the request body should not affect`,
+      //   input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .post('/$batch', "--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1') HTTP/1.1\\r\\nAccept: application/json\\r\\n")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`,
+      //   output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .post('/$batch', "--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1') HTTP/1.1\\r\\nAccept: application/json\\r\\n")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`
+      // },
+      // {
+      //   name: `single quotes in the path and the request body should work`,
+      //   input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .post('/$batch'hello'', "--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1') HTTP/1.1\\r\\nAccept: application/json\\r\\n")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`,
+      //   output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .post(\`/$batch'hello'\`, "--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1') HTTP/1.1\\r\\nAccept: application/json\\r\\n")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`
+      // },
+      // {
+      //   name: `single quotes in the path and the request body should work - without space`,
+      //   input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .post('/$batch'hello'',"--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1') HTTP/1.1\\r\\nAccept: application/json\\r\\n")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`,
+      //   output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .post(\`/$batch'hello'\`,"--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1') HTTP/1.1\\r\\nAccept: application/json\\r\\n")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`
+      // },
+      // {
+      //   name: `single quotes in the path and the request body should work`,
+      //   input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .post('/$batch'hello'', "--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1') HTTP/1.1\\r\\nAccept: application/json\\r\\n")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`,
+      //   output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+      //             .post(\`/$batch'hello'\`, "--batch_fakeId\\r\\nDELETE https://endpoint.net/node(key='batchTest',RowKey='1') HTTP/1.1\\r\\nAccept: application/json\\r\\n")
+      //             .query(true)
+      //             .reply(200, "4f626a0131c2", [
+      //             'Transfer-Encoding',
+      //             'chunked'
+      //           ]);`
+      // }
+    ].forEach((test) => {
+      it(test.name, () => {
+        chai.assert.equal(
+          handleSingleQuotesInUrlPath(test.input),
+          test.output,
+          `Output from "handleSingleQuotesInUrlPath" did not match the expected output`
+        );
+      });
+    });
+  });
 });