-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[Unified Recorder] Allow passing undefined to sanitizers and other minor changes #19561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
76f0656
596cae3
7aae2bb
17ec69e
8ef19eb
305cfc3
a272f86
ab3af41
e6798e1
7400079
43918fb
82d2c1f
b2ff65f
2f88a54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { | |
| getTestMode, | ||
| ProxyToolSanitizers, | ||
| RecorderError, | ||
| RegexSanitizer, | ||
| sanitizerKeywordMapping, | ||
| SanitizerOptions | ||
| } from "./utils/utils"; | ||
|
|
@@ -14,7 +15,7 @@ import { | |
| * Sanitizer class to handle communication with the proxy-tool relating to the sanitizers adding/resetting, etc. | ||
| */ | ||
| export class Sanitizer { | ||
| constructor(private url: string, private httpClient: HttpClient) {} | ||
| constructor(private url: string, private httpClient: HttpClient) { } | ||
| private recordingId: string | undefined; | ||
|
|
||
| setRecordingId(recordingId: string): void { | ||
|
|
@@ -74,12 +75,13 @@ export class Sanitizer { | |
| const replacers = options[prop]; | ||
| if (replacers) { | ||
| return Promise.all( | ||
| replacers.map((replacer: unknown) => | ||
| this.addSanitizer({ | ||
| replacers.map((replacer: RegexSanitizer) => { | ||
| if (!replacer.regex) return; | ||
| return this.addSanitizer({ | ||
| sanitizer: sanitizerKeywordMapping[prop], | ||
| body: JSON.stringify(replacer) | ||
| }) | ||
| ) | ||
| }) | ||
| ); | ||
| } else return; | ||
| }) | ||
|
|
@@ -140,9 +142,10 @@ export class Sanitizer { | |
| * - generalRegexSanitizer is applied for each of the parts with the real and fake values that are parsed | ||
| */ | ||
| async addConnectionStringSanitizer( | ||
| actualConnString: string, | ||
| actualConnString: string | undefined, | ||
| fakeConnString: string | ||
| ): Promise<void> { | ||
| if (!actualConnString) return; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we throw if it's undefined in live/record mode?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| // extract connection string parts and match call | ||
| const pairsMatched = getRealAndFakePairs(actualConnString, fakeConnString); | ||
| await this.addSanitizers({ | ||
|
|
@@ -160,9 +163,8 @@ export class Sanitizer { | |
| body: string | undefined; | ||
| }): Promise<void> { | ||
| if (this.recordingId !== undefined) { | ||
| const uri = `${this.url}${paths.admin}${ | ||
| options.sanitizer !== "Reset" ? paths.addSanitizer : paths.reset | ||
| }`; | ||
| const uri = `${this.url}${paths.admin}${options.sanitizer !== "Reset" ? paths.addSanitizer : paths.reset | ||
| }`; | ||
| const req = this._createRecordingRequest(uri); | ||
| if (options.sanitizer !== "Reset") { | ||
| req.headers.set("x-abstraction-identifier", options.sanitizer); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import { isPlaybackMode } from "./utils"; | ||
|
|
||
| /** | ||
| * Usage - `await delay(<milliseconds>)` | ||
| * This `delay` has no effect if the `TEST_MODE` is `"playback"`. | ||
| * If the `TEST_MODE` is not `"playback"`, `delay` is a wrapper for setTimeout that resolves a promise after t milliseconds. | ||
| * | ||
| * @param {number} milliseconds The number of milliseconds to be delayed. | ||
| * @returns {Promise<T>} Resolved promise | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return type needs update
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed |
||
| */ | ||
| export function delay(milliseconds: number): Promise<void> | null { | ||
| return isPlaybackMode() ? null : new Promise((resolve) => setTimeout(resolve, milliseconds)); | ||
| } | ||
|
HarshaNalluru marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,7 +97,7 @@ export interface RegexSanitizer { | |
| /** | ||
| * A regex. Can be defined as a simple regex replace OR if groupForReplace is set, a subsitution operation. | ||
| */ | ||
| regex: string; | ||
| regex?: string; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it optional because it is not used in playback mode? if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only meant for convenience while writing the tests. If regex is undefined, the sanitizer won't be added. For |
||
| /** | ||
| * The capture group that needs to be operated upon. Do not set if you're invoking a simple replacement operation. | ||
| */ | ||
|
|
@@ -149,7 +149,7 @@ interface ConnectionStringSanitizer { | |
| /** | ||
| * Real connection string with all the secrets | ||
| */ | ||
| actualConnString: string; | ||
| actualConnString?: string; | ||
| /** | ||
| * Fake connection string - with all the parts of the connection string mapped to fake values | ||
| */ | ||
|
|
@@ -181,7 +181,17 @@ export interface SanitizerOptions { | |
| * Regardless, there are examples present in `recorder-new/test/testProxyTests.spec.ts`. | ||
| */ | ||
| bodyRegexSanitizers?: RegexSanitizer[]; | ||
|
|
||
| /** | ||
| * This sanitizer offers regex update of a specific JTokenPath. | ||
| * | ||
| * EG: "TableName" within a json response body having its value replaced by whatever substitution is offered. | ||
| * This simply means that if you are attempting to replace a specific key wholesale, this sanitizer will be simpler | ||
| * than configuring a BodyRegexSanitizer that has to match against the full "KeyName": "Value" that is part of the json structure. | ||
| * | ||
| * Further reading is available [here](https://www.newtonsoft.com/json/help/html/SelectToken.htm#SelectTokenJSONPath). | ||
| * | ||
| * If the body is NOT a JSON object, this sanitizer will NOT be applied. | ||
| */ | ||
| bodyKeySanitizers?: BodyKeySanitizer[]; | ||
| /** | ||
| * TODO | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we throw if it's undefined in live/record mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm, good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only record mode