From 9b80900b7284d17fef80a9b33da4e146cbe15f1a Mon Sep 17 00:00:00 2001 From: Richard Park <51494936+richardpark-msft@users.noreply.github.com> Date: Fri, 6 Dec 2019 12:56:10 -0800 Subject: [PATCH] [app-configuration] Renaming keys => keyFilter, label => labelFilter (#6417) As part of a .net API review we discovered some inconsistent behavior with how we handle filters, especially with regards to proper escaping. This PR renames those fields and also makes them strings rather than arrays while also documenting the format allowed for the underlying string. (also, fixing an odd issue where `nock` was failing. Adding the dependency and removing a method that didn't need to be called) Fixes #6384 --- .../app-configuration/CHANGELOG.md | 6 +++ .../app-configuration/package.json | 21 ++++++----- .../review/app-configuration.api.md | 4 +- .../samples/getSettingOnlyIfChanged.ts | 2 +- .../app-configuration/samples/helloworld.ts | 2 +- .../samples/helloworldWithLabels.ts | 2 +- .../samples/listRevisions.ts | 4 +- .../samples/optimisticConcurrencyViaEtag.ts | 2 +- .../samples/setReadOnlySample.ts | 2 +- .../generated/src/appConfigurationContext.ts | 2 +- .../app-configuration/src/internal/helpers.ts | 17 +-------- .../app-configuration/src/models.ts | 37 +++++++++++++++++-- .../app-configuration/test/helpers.spec.ts | 12 +++--- .../app-configuration/test/index.spec.ts | 26 ++++++------- .../test/internal/synctokenpolicy.spec.ts | 7 ++-- .../app-configuration/test/package.spec.ts | 5 ++- .../app-configuration/test/testHelpers.ts | 2 +- 17 files changed, 89 insertions(+), 64 deletions(-) diff --git a/sdk/appconfiguration/app-configuration/CHANGELOG.md b/sdk/appconfiguration/app-configuration/CHANGELOG.md index 09613abc2fbc..12f9bb4fa268 100644 --- a/sdk/appconfiguration/app-configuration/CHANGELOG.md +++ b/sdk/appconfiguration/app-configuration/CHANGELOG.md @@ -1,3 +1,9 @@ +# 1.0.0-preview.10 (TBD) + +- Specifying filters for listConfigurationSettings() or listRevisions() is + now done with the `keyFilter` or `labelFilter` strings rather than `keys` + and `labels` as they were previously. + # 1.0.0-preview.9 (2019-12-03) - Updated to use OpenTelemetry 0.2 via `@azure/core-tracing`. diff --git a/sdk/appconfiguration/app-configuration/package.json b/sdk/appconfiguration/app-configuration/package.json index 5bdd3e28e00e..b2168db4e387 100644 --- a/sdk/appconfiguration/app-configuration/package.json +++ b/sdk/appconfiguration/app-configuration/package.json @@ -2,7 +2,7 @@ "name": "@azure/app-configuration", "author": "Microsoft Corporation", "description": "An isomorphic client library for the Azure App Configuration service.", - "version": "1.0.0-preview.9", + "version": "1.0.0-preview.10", "sdk-type": "client", "keywords": [ "node", @@ -60,12 +60,20 @@ }, "sideEffects": false, "autoPublish": false, + "//metadata": { + "constantPaths": [ + { + "path": "src/generated/src/appConfigurationContext.ts", + "prefix": "packageVersion" + } + ] + }, "dependencies": { "@azure/core-asynciterator-polyfill": "^1.0.0", "@azure/core-http": "^1.0.0", "@azure/core-paging": "^1.0.0", - "@azure/identity": "^1.0.0", "@azure/core-tracing": "1.0.0-preview.7", + "@azure/identity": "^1.0.0", "@opentelemetry/types": "^0.2.0", "tslib": "^1.9.3" }, @@ -84,6 +92,7 @@ "mocha": "^6.2.2", "mocha-junit-reporter": "^1.18.0", "mocha-multi": "^1.1.3", + "nock": "^11.7.0", "nyc": "^14.0.0", "prettier": "^1.16.4", "rimraf": "^3.0.0", @@ -96,13 +105,5 @@ "ts-node": "^8.3.0", "typescript": "~3.6.4", "uglify-js": "^3.4.9" - }, - "//metadata": { - "constantPaths": [ - { - "path": "src/generated/src/appConfigurationContext.ts", - "prefix": "packageVersion" - } - ] } } diff --git a/sdk/appconfiguration/app-configuration/review/app-configuration.api.md b/sdk/appconfiguration/app-configuration/review/app-configuration.api.md index 71ffafec0023..ce2fa48de4bf 100644 --- a/sdk/appconfiguration/app-configuration/review/app-configuration.api.md +++ b/sdk/appconfiguration/app-configuration/review/app-configuration.api.md @@ -128,8 +128,8 @@ export interface ListRevisionsPage extends HttpResponseField { - let key; - - if (listConfigOptions.keys) { - // TODO: escape commas? - key = listConfigOptions.keys.join(","); - } - - let label; - - if (listConfigOptions.labels) { - label = listConfigOptions.labels.join(","); - } - let fieldsToGet: (keyof KeyValue)[] | undefined; if (listConfigOptions.fields) { @@ -111,8 +98,8 @@ export function formatWildcards( } return { - key, - label, + key: listConfigOptions.keyFilter, + label: listConfigOptions.labelFilter, acceptDatetime, select: fieldsToGet }; diff --git a/sdk/appconfiguration/app-configuration/src/models.ts b/sdk/appconfiguration/app-configuration/src/models.ts index 0b5c30fe3d9f..045a77bbc309 100644 --- a/sdk/appconfiguration/app-configuration/src/models.ts +++ b/sdk/appconfiguration/app-configuration/src/models.ts @@ -233,14 +233,43 @@ export interface ListSettingsOptions extends OptionalFields { acceptDateTime?: Date; /** - * Filters for wildcard matching (using *) against keys. These conditions are logically OR'd against each other. + * Filters for keys. There are two types of matching: + * + * 1. Exact matching. Up to 5 key names are allowed, separated by commas (',') + * 2. Wildcard matching. A single wildcard expression can be specified. + * + * | Value | Matches | + * |--------------|---------------------------------------| + * | omitted or * | Matches any key | + * | abc | Matches a key named abc | + * | abc* | Matches key names that start with abc | + * | *abc | Matches key names that end with abc | + * | *abc* | Matches key names that contain abc | + * + * These characters are reserved and must be prefixed with backslash in order + * to be specified: * or \ or , */ - keys?: string[]; + keyFilter?: string; /** - * Filters for wildcard matching (using *) against labels. These conditions are logically OR'd against each other. + * Filters for labels. There are two types of matching: + * + * 1. Exact matching. Up to 5 labels are allowed, separated by commas (',') + * 2. Wildcard matching. A single wildcard expression can be specified. + * + * | Value | Matches | + * |--------------|---------------------------------------------------| + * | omitted or * | Matches any key | + * | %00 | Matches any key without a label | + * | prod | Matches a key with label named prod | + * | prod* | Matches key with label names that start with prod | + * | *prod | Matches key with label names that end with prod | + * | *prod* | Matches key with label names that contain prod | + * + * These characters are reserved and must be prefixed with backslash in order + * to be specified: * or \ or , */ - labels?: string[]; + labelFilter?: string; } /** diff --git a/sdk/appconfiguration/app-configuration/test/helpers.spec.ts b/sdk/appconfiguration/app-configuration/test/helpers.spec.ts index f665db039e13..a0d0458eb9b2 100644 --- a/sdk/appconfiguration/app-configuration/test/helpers.spec.ts +++ b/sdk/appconfiguration/app-configuration/test/helpers.spec.ts @@ -85,8 +85,8 @@ describe("helper methods", () => { describe("formatWildcards", () => { it("undefined", () => { const result = formatWildcards({ - keys: undefined, - labels: undefined + keyFilter: undefined, + labelFilter: undefined }); assert.ok(!result.key); @@ -95,8 +95,8 @@ describe("helper methods", () => { it("single values only", () => { const result = formatWildcards({ - keys: ["key1"], - labels: ["label1"] + keyFilter: "key1", + labelFilter: "label1" }); assert.equal("key1", result.key); @@ -105,8 +105,8 @@ describe("helper methods", () => { it("multiple values", () => { const result = formatWildcards({ - keys: ["key1", "key2"], - labels: ["label1", "label2"] + keyFilter: "key1,key2", + labelFilter: "label1,label2" }); assert.equal("key1,key2", result.key); diff --git a/sdk/appconfiguration/app-configuration/test/index.spec.ts b/sdk/appconfiguration/app-configuration/test/index.spec.ts index 66a0b99a427b..ccf638b4e807 100644 --- a/sdk/appconfiguration/app-configuration/test/index.spec.ts +++ b/sdk/appconfiguration/app-configuration/test/index.spec.ts @@ -472,7 +472,7 @@ describe("AppConfigurationClient", () => { it("exact match on label", async () => { // query with a direct label match - let byLabelIterator = client.listConfigurationSettings({ labels: [uniqueLabel] }); + let byLabelIterator = client.listConfigurationSettings({ labelFilter: uniqueLabel }); const byLabelSettings = await toSortedArray(byLabelIterator); assertEqualSettings( @@ -497,7 +497,7 @@ describe("AppConfigurationClient", () => { it("label wildcards", async () => { // query with a direct label match let byLabelIterator = client.listConfigurationSettings({ - labels: ["*" + uniqueLabel.substring(1)] + labelFilter: "*" + uniqueLabel.substring(1) }); const byLabelSettings = await toSortedArray(byLabelIterator); @@ -521,7 +521,7 @@ describe("AppConfigurationClient", () => { }); it("exact match on key", async () => { - let byKeyIterator = client.listConfigurationSettings({ keys: [`listConfigSettingA-${now}`] }); + let byKeyIterator = client.listConfigurationSettings({ keyFilter: `listConfigSettingA-${now}` }); const byKeySettings = await toSortedArray(byKeyIterator); assertEqualSettings( @@ -545,7 +545,7 @@ describe("AppConfigurationClient", () => { it("key wildcards", async () => { // query with a key wildcard - let byKeyIterator = client.listConfigurationSettings({ keys: [`*istConfigSettingA-${now}`] }); + let byKeyIterator = client.listConfigurationSettings({ keyFilter: `*istConfigSettingA-${now}` }); const byKeySettings = await toSortedArray(byKeyIterator); assertEqualSettings( @@ -570,7 +570,7 @@ describe("AppConfigurationClient", () => { it("filter on fields", async () => { // only fill in the 'readOnly' field (which is really the locked field in the REST model) let byKeyIterator = client.listConfigurationSettings({ - keys: [`listConfigSettingA-${now}`], + keyFilter: `listConfigSettingA-${now}`, fields: ["key", "label", "isReadOnly"] }); let settings = await toSortedArray(byKeyIterator); @@ -586,7 +586,7 @@ describe("AppConfigurationClient", () => { // only fill in the 'readOnly' field (which is really the locked field in the REST model) byKeyIterator = client.listConfigurationSettings({ - keys: [`listConfigSettingA-${now}`], + keyFilter: `listConfigSettingA-${now}`, fields: ["key", "label", "value"] }); settings = await toSortedArray(byKeyIterator); @@ -603,7 +603,7 @@ describe("AppConfigurationClient", () => { it("by date", async () => { let byKeyIterator = client.listConfigurationSettings({ - keys: ['listConfigSettingA-*'], + keyFilter: 'listConfigSettingA-*', acceptDateTime: listConfigSettingA.lastModified }); @@ -698,7 +698,7 @@ describe("AppConfigurationClient", () => { }); it("exact match on label", async () => { - const revisionsWithLabelIterator = await client.listRevisions({ labels: [labelA] }); + const revisionsWithLabelIterator = await client.listRevisions({ labelFilter: labelA }); const revisions = await toSortedArray(revisionsWithLabelIterator); assertEqualSettings( @@ -712,7 +712,7 @@ describe("AppConfigurationClient", () => { it("label wildcards", async () => { const revisionsWithLabelIterator = await client.listRevisions({ - labels: ["*" + labelA.substring(1)] + labelFilter: "*" + labelA.substring(1) }); const revisions = await toSortedArray(revisionsWithLabelIterator); @@ -726,7 +726,7 @@ describe("AppConfigurationClient", () => { }); it("exact match on key", async () => { - const revisionsWithKeyIterator = await client.listRevisions({ keys: [key] }); + const revisionsWithKeyIterator = await client.listRevisions({ keyFilter: key }); const revisions = await toSortedArray(revisionsWithKeyIterator); assertEqualSettings( @@ -742,7 +742,7 @@ describe("AppConfigurationClient", () => { it("key wildcards", async () => { const revisionsWithKeyIterator = await client.listRevisions({ - keys: ["*" + key.substring(1)] + keyFilter: "*" + key.substring(1) }); const revisions = await toSortedArray(revisionsWithKeyIterator); @@ -759,14 +759,14 @@ describe("AppConfigurationClient", () => { it("accepts operation options", async () => { await assertThrowsAbortError(async () => { - const iter = client.listRevisions({ labels: [labelA], requestOptions: { timeout: 1 } }); + const iter = client.listRevisions({ labelFilter: labelA, requestOptions: { timeout: 1 } }); await iter.next(); }); }); it("by date", async () => { let byKeyIterator = client.listRevisions({ - keys: [key], + keyFilter: key, acceptDateTime: originalSetting.lastModified }); diff --git a/sdk/appconfiguration/app-configuration/test/internal/synctokenpolicy.spec.ts b/sdk/appconfiguration/app-configuration/test/internal/synctokenpolicy.spec.ts index 5e21b64f6d43..f05e700201b2 100644 --- a/sdk/appconfiguration/app-configuration/test/internal/synctokenpolicy.spec.ts +++ b/sdk/appconfiguration/app-configuration/test/internal/synctokenpolicy.spec.ts @@ -104,7 +104,7 @@ describe("sync tokens", () => { }) .get(/.*/) .reply(418); - + await assertThrowsRestError( async () => await client.getConfigurationSetting({ @@ -114,7 +114,6 @@ describe("sync tokens", () => { ); assert.ok(policyScope.isDone()); - policyScope.cleanAll(); }); it("addConfigurationSetting", async () => { @@ -171,7 +170,7 @@ describe("sync tokens", () => { .reply(200, '', { 'sync-token': 'listConfigurationSetting=value;sn=1'}); const iterator = client.listConfigurationSettings({ - keys: ["doesntmatter"] + keyFilter: "doesntmatter" }); await iterator.next(); @@ -184,7 +183,7 @@ describe("sync tokens", () => { .reply(200, '', { 'sync-token': 'listRevisions=value;sn=1'}); const iterator = client.listRevisions({ - keys: ["doesntmatter"] + keyFilter: "doesntmatter" }); await iterator.next(); diff --git a/sdk/appconfiguration/app-configuration/test/package.spec.ts b/sdk/appconfiguration/app-configuration/test/package.spec.ts index 99a9f841c8b3..05a30ee4bb43 100644 --- a/sdk/appconfiguration/app-configuration/test/package.spec.ts +++ b/sdk/appconfiguration/app-configuration/test/package.spec.ts @@ -1,12 +1,15 @@ import { join } from "path"; import { readFileSync } from "fs"; import * as assert from "assert"; + // this constant isn't normally exported by the generator so you'll need to do it // by hand if you regenerate the models. import { packageVersion } from "../src/generated/src/appConfigurationContext"; describe("packagejson related tests", () => { - it("user agent string", () => { + // if this test is failing you need to update the contant `packageVersion` referenced above + // in the generated code. + it("user agent string matches the package version", () => { const packageJsonFilePath = join(__dirname, "../package.json"); const rawFileContents = readFileSync(packageJsonFilePath, { encoding: "utf-8" }); const packageJsonContents = JSON.parse(rawFileContents); diff --git a/sdk/appconfiguration/app-configuration/test/testHelpers.ts b/sdk/appconfiguration/app-configuration/test/testHelpers.ts index a635e76a6630..acb9630bcd09 100644 --- a/sdk/appconfiguration/app-configuration/test/testHelpers.ts +++ b/sdk/appconfiguration/app-configuration/test/testHelpers.ts @@ -67,7 +67,7 @@ export function createAppConfigurationClientForTests(options?: AppConfigurationC export async function deleteKeyCompletely(keys: string[], client: AppConfigurationClient) { const settingsIterator = await client.listConfigurationSettings({ - keys: keys + keyFilter: keys.join(",") }); for await (const setting of settingsIterator) {