addressing comments, name/argument changes, updating docs, adding and…

… fixing test cases
apollographql · Jun 21, 2019 · 203e7fa · 203e7fa
1 parent 896c311
commit 203e7fa
Show file tree

Hide file tree

Showing 6 changed files with 358 additions and 311 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,7 @@
 
 ### vNext
 - `apollo-engine-reporting`: BREAKING CHANGE: By default, send no GraphQL variable values to Apollo's servers instead of sending all variable values. Use the new EngineReportingOption `sendVariableValues` to send some or all variable values, possibly after transforming them. 
-This replaces the `privateVariables` option, which is now deprecated. [PR #2847](https://github.com/apollographql/apollo-server/pull/2847) [PR #2472](https://github.com/apollographql/apollo-server/pull/2472)
+This replaces the `privateVariables` option, which is now deprecated. [PR #2847](https://github.com/apollographql/apollo-server/pull/2847)
 - `apollo-engine-reporting`: BREAKING CHANGE: By default, send no GraphQL headers to Apollo's servers instead of sending all. Use the new EngineReportingOption `sendHeaders` to send some or all headers and their values. 
 A replacement for the 'privateHeaders' option, which is now deprecated. [PR #2847](https://github.com/apollographql/apollo-server/pull/2847)
 ### v2.6.2

diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md
@@ -119,7 +119,7 @@ new ApolloServer({
 
 * `engine`: <`EngineReportingOptions`> | boolean
 
-  Provided the `ENGINE_API_KEY` environment variable is set, the engine reporting agent will be started automatically. The API key can also be provided as the `apiKey` field in an object passed as the `engine` field. See the [EngineReportingOptions](#enginereportingoptions) section for a full description of how to configure the reporting agent, including how to blocklist variables. When using the Engine proxy, this option should be set to false.
+  Provided the `ENGINE_API_KEY` environment variable is set, the engine reporting agent will be started automatically. The API key can also be provided as the `apiKey` field in an object passed as the `engine` field. See the [EngineReportingOptions](#enginereportingoptions) section for a full description of how to configure the reporting agent, including how to include variable values and HTTP headers. When using the Engine proxy, this option should be set to false.
 
 * `persistedQueries`: <`Object`> | false
 
@@ -342,48 +342,48 @@ addMockFunctionsToSchema({
    By default, errors sending reports to Engine servers will be logged
    to standard error. Specify this function to process errors in a different
    way.
-
-*  `privateVariables`: Array<String\> | boolean
-
-   DEPRECATING IN VERSION XX.XX.XX for `sendVariableValues`, which will support the same
-   functionalities but allow for more flexibility.
 
-   A case-sensitive list of names of variables whose values should not be sent
-   to Apollo servers, or 'true' to leave out all variables. In the former
-   case, the report will indicate that each private variable was redacted in
-   the latter case, no variables are sent at all.
-
-* `sendVariableValues`: { valueModifier: (options: { variables: Record<string, any>, operationString?: string } ) => Record<string, any> }
-                     | { exceptVariableNames: Array<String\> }
-                     | { safelistAll: boolean }
+* `sendVariableValues`: { transform: (options: { variables: Record<string, any>, operationString?: string } ) => Record<string, any> }
+                     | { exceptVariableNames: Array&lt;String&gt; }
+                     | { sendNone: true }
+                     | { sendAll: true }
 
     By default, Apollo Server does not send the values of any GraphQL variables to Apollo's servers, because variable values often contain the private data of your app's users. If you'd like variable values to be included in traces, set this option. This option can take several forms:
 
-    - { safelistAll: ... }: false to blocklist, or true to safelist all variable values
-    - { valueModifier: ... }: a custom function for modifying variable values
-    - { exceptVariableNames: ... }: a case-sensitive list of names of variables whose values should not be sent to Apollo servers
+    - { sendNone: true } to blocklist all variable values
+    - { sendAll: true } to safelist all variable values
+    - { transform: ... }: a custom function for modifying variable values. Keys that were added by the custom function will be removed, and keys that were removed will be added back with an empty value. 
+    - { exceptNames: ... }: a case-sensitive list of names of variables whose values should not be sent to Apollo servers
+
+   Defaults to blocklisting all variable values if both this parameter and
+   the to-be-deprecated `privateVariables` are not set. The report will
+   indicate each private variable key whose value was redacted by { sendNone: true } or { exceptVariableNames: [...] }.
+
+*  `privateVariables`: Array&lt;String&gt; | boolean
 
-    Defaults to blocklisting all variable values if both this parameter and
-    the to-be-deprecated `privateVariables` are not set. The report will also
-    indicate each private variable key redacted by { safelistAll: false } or { exceptVariableNames: [...] }.
+   DEPRECATING IN VERSION XX.XX.XX, to be replaced by the option `sendVariableValues`, which supports the same
+   functionalities but allow for more flexibility. Passing an array into `privateVariables` is equivalent to
+   passing in `{ exceptVariableNames: array } ` to `sendVariableValues`, and passing in `true` or `false` is equivalent
+   to passing ` { sendNone: true } ` or ` { sendAll: true }`, respectively.
 
-* `sendHeaders`: { except: Array<String\> } | { safelistAll: boolean }
-   By default, Apollo Server does not send the list of HTTP headers and values to
+* `sendHeaders`: { except: Array&lt;String&gt; } | { sendAll: boolean } | { sendNone: boolean }
+   By default, Apollo Server does not send the list of HTTP request headers and values to
    Apollo's servers, to protect private data of your app's users. If you'd like this information included in traces,
-   set this option. This option can take two forms:
+   set this option. This option can take several forms:
 
-   - {except: Array<String\>} A case-insensitive list of names of HTTP headers whose values should not be
+   - { exceptNames: Array&lt;String&gt; } A case-insensitive list of names of HTTP headers whose values should not be
    sent to Apollo servers
-   - {safelistAll: true/false} to include or leave out all HTTP headers.
+   - { sendNone : true } to drop all HTTP request headers
+   - { sendAll : true } to send the values of all HTTP request headers
 
-   Unlike with sendVariableValues, names of dropped headers are not reported.
+   Unlike with `sendVariableValues`, names of dropped headers are not reported.
+   The headers 'authorization', 'cookie', and 'set-cookie' are never reported.
 
-*  `privateHeaders`: Array<String\> | boolean
+*  `privateHeaders`: Array&lt;String&gt; | boolean
 
-   DEPRECATING IN VERSION XX.XX.XX, use 'sendHeaders' instead.
-   A case-insensitive list of names of HTTP headers whose values should not be
-   sent to Apollo servers, or 'true' to leave out all HTTP headers. Unlike
-   with privateVariables, names of dropped headers are not reported.
+   DEPRECATING IN VERSION XX.XX.XX, use `sendHeaders` instead.
+   Passing an array into `privateHeaders` is equivalent to passing ` { except: array } ` into `sendHeaders`, and
+   passing `true` or `false` is equivalent to passing in ` { sendNone: true } ` and ` { sendAll: true }`, respectively.
 
 *  `handleSignals`: boolean
 

diff --git a/packages/apollo-engine-reporting/src/__tests__/agent.test.ts b/packages/apollo-engine-reporting/src/__tests__/agent.test.ts
@@ -1,4 +1,8 @@
-import { signatureCacheKey } from '../agent';
+import {
+  signatureCacheKey,
+  handleLegacyOptions,
+  EngineReportingOptions,
+} from '../agent';
 
 describe('signature cache key', () => {
   it('generates without the operationName', () => {
@@ -11,3 +15,80 @@ describe('signature cache key', () => {
     );
   });
 });
+
+describe("test handleLegacyOptions(), which converts the deprecated privateVariable and privateHeaders options to the new options' formats", () => {
+  it('Case 1: privateVariables/privateHeaders == False; same as sendAll', () => {
+    const optionsPrivateFalse: EngineReportingOptions<any> = {
+      privateVariables: false,
+      privateHeaders: false,
+    };
+    handleLegacyOptions(optionsPrivateFalse);
+    expect(optionsPrivateFalse.privateVariables).toBe(undefined);
+    expect(optionsPrivateFalse.sendVariableValues).toEqual({ sendAll: true });
+    expect(optionsPrivateFalse.privateHeaders).toBe(undefined);
+    expect(optionsPrivateFalse.sendHeaders).toEqual({ sendAll: true });
+  });
+
+  it('Case 2: privateVariables/privateHeaders == True; same as sendNone', () => {
+    const optionsPrivateTrue: EngineReportingOptions<any> = {
+      privateVariables: true,
+      privateHeaders: true,
+    };
+    handleLegacyOptions(optionsPrivateTrue);
+    expect(optionsPrivateTrue.privateVariables).toBe(undefined);
+    expect(optionsPrivateTrue.sendVariableValues).toEqual({ sendNone: true });
+    expect(optionsPrivateTrue.privateHeaders).toBe(undefined);
+    expect(optionsPrivateTrue.sendHeaders).toEqual({ sendNone: true });
+  });
+
+  it('Case 3: privateVariables/privateHeaders set to an array', () => {
+    const privateArray: Array<String> = ['t1', 't2'];
+    const optionsPrivateArray: EngineReportingOptions<any> = {
+      privateVariables: privateArray,
+      privateHeaders: privateArray,
+    };
+    handleLegacyOptions(optionsPrivateArray);
+    expect(optionsPrivateArray.privateVariables).toBe(undefined);
+    expect(optionsPrivateArray.sendVariableValues).toEqual({
+      exceptNames: privateArray,
+    });
+    expect(optionsPrivateArray.privateHeaders).toBe(undefined);
+    expect(optionsPrivateArray.sendHeaders).toEqual({
+      exceptNames: privateArray,
+    });
+  });
+
+  it('Case 4: throws error when both the new and old options are set', () => {
+    const optionsBothVariables: EngineReportingOptions<any> = {
+      privateVariables: true,
+      sendVariableValues: { sendNone: true },
+    };
+    expect(() => {
+      handleLegacyOptions(optionsBothVariables);
+    }).toThrow();
+    const optionsBothHeaders: EngineReportingOptions<any> = {
+      privateHeaders: true,
+      sendHeaders: { sendNone: true },
+    };
+    expect(() => {
+      handleLegacyOptions(optionsBothHeaders);
+    }).toThrow();
+  });
+
+  it('Case 5: the passed in options are not modified if deprecated fields were not set', () => {
+    const optionsNotDeprecated: EngineReportingOptions<any> = {
+      sendVariableValues: { exceptNames: ['test'] },
+      sendHeaders: true,
+    };
+    const output: EngineReportingOptions<any> = {
+      sendVariableValues: { exceptNames: ['test'] },
+      sendHeaders: true,
+    };
+    handleLegacyOptions(optionsNotDeprecated);
+    expect(optionsNotDeprecated).toEqual(output);
+
+    const emptyInput: EngineReportingOptions<any> = {};
+    handleLegacyOptions(emptyInput);
+    expect(emptyInput).toEqual({});
+  });
+});