Skip to content

Commit

Permalink
[app-configuration] Renaming keys => keyFilter, label => labelFilter (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
richardpark-msft authored Dec 6, 2019
1 parent 1aaa9da commit 9b80900
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 64 deletions.
6 changes: 6 additions & 0 deletions sdk/appconfiguration/app-configuration/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down
21 changes: 11 additions & 10 deletions sdk/appconfiguration/app-configuration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
},
Expand All @@ -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",
Expand All @@ -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"
}
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ export interface ListRevisionsPage extends HttpResponseField<SyncTokenHeaderFiel
// @public
export interface ListSettingsOptions extends OptionalFields {
acceptDateTime?: Date;
keys?: string[];
labels?: string[];
keyFilter?: string;
labelFilter?: string;
}

// @public
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export async function run() {

async function cleanupSampleValues(keys: string[], client: AppConfigurationClient) {
const existingSettings = await client.listConfigurationSettings({
keys: keys
keyFilter: keys.join(',')
});

for await (const setting of existingSettings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export async function run() {

async function cleanupSampleValues(keys: string[], client: AppConfigurationClient) {
const settingsIterator = await client.listConfigurationSettings({
keys: keys
keyFilter: keys.join(',')
});

for await (const setting of settingsIterator) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export async function run() {

async function cleanupSampleValues(keys: string[], client: AppConfigurationClient) {
const existingSettings = await client.listConfigurationSettings({
keys: keys
keyFilter: keys.join(',')
});

for await (const setting of existingSettings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export async function run() {
await client.setConfigurationSetting(newSetting);

const revisionsIterator = client.listRevisions({
keys: [ newSetting.key ]
keyFilter: newSetting.key
});

// show all the revisions, including the date they were set.
Expand All @@ -50,7 +50,7 @@ export async function run() {

async function cleanupSampleValues(keys: string[], client: AppConfigurationClient) {
const settingsIterator = await client.listConfigurationSettings({
keys: keys
keyFilter: keys.join(',')
});

for await (const setting of settingsIterator) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export async function run() {

async function cleanupSampleValues(keys: string[], client: AppConfigurationClient) {
const existingSettings = await client.listConfigurationSettings({
keys: keys
keyFilter: keys.join(',')
});

for await (const setting of existingSettings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export async function run() {

async function cleanupSampleValues(keys: string[], client: AppConfigurationClient) {
const existingSettings = await client.listConfigurationSettings({
keys: keys
keyFilter: keys.join(',')
});

for await (const setting of existingSettings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import * as coreHttp from "@azure/core-http";
import * as Models from "./models";

const packageName = "app-configuration";
export const packageVersion = "1.0.0-preview.9";
export const packageVersion = "1.0.0-preview.10";

export class AppConfigurationContext extends coreHttp.ServiceClient {
syncToken?: string;
Expand Down
17 changes: 2 additions & 15 deletions sdk/appconfiguration/app-configuration/src/internal/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,6 @@ export function checkAndFormatIfAndIfNoneMatch(
export function formatWildcards(
listConfigOptions: ListConfigurationSettingsOptions | ListRevisionsOptions
): Pick<AppConfigurationGetKeyValuesOptionalParams, "key" | "label" | "select" | "acceptDatetime"> {
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) {
Expand All @@ -111,8 +98,8 @@ export function formatWildcards(
}

return {
key,
label,
key: listConfigOptions.keyFilter,
label: listConfigOptions.labelFilter,
acceptDatetime,
select: fieldsToGet
};
Expand Down
37 changes: 33 additions & 4 deletions sdk/appconfiguration/app-configuration/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
12 changes: 6 additions & 6 deletions sdk/appconfiguration/app-configuration/test/helpers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
26 changes: 13 additions & 13 deletions sdk/appconfiguration/app-configuration/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);

Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -603,7 +603,7 @@ describe("AppConfigurationClient", () => {

it("by date", async () => {
let byKeyIterator = client.listConfigurationSettings({
keys: ['listConfigSettingA-*'],
keyFilter: 'listConfigSettingA-*',
acceptDateTime: listConfigSettingA.lastModified
});

Expand Down Expand Up @@ -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(
Expand All @@ -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);

Expand All @@ -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(
Expand All @@ -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);

Expand All @@ -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
});

Expand Down
Loading

0 comments on commit 9b80900

Please sign in to comment.