From ac1bb50f7ee29e5d6dc37d6dfab47a3140c4e706 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Tue, 15 Aug 2017 18:56:30 -0700 Subject: [PATCH 1/8] Add metadata + tests --- package.json | 4 +- src/metadata.ts | 132 ++++++++++++++++++++++++++---- test/test-metadata.ts | 182 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 301 insertions(+), 17 deletions(-) create mode 100644 test/test-metadata.ts diff --git a/package.json b/package.json index bc02fdd6..a92c3267 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,8 @@ }, "dependencies": { "@types/async": "^2.0.41", - "async": "^2.5.0" + "@types/lodash": "^4.14.73", + "async": "^2.5.0", + "lodash": "^4.17.4" } } diff --git a/src/metadata.ts b/src/metadata.ts index ffa78996..1936bfa2 100644 --- a/src/metadata.ts +++ b/src/metadata.ts @@ -1,35 +1,135 @@ +import { forOwn } from 'lodash'; + export type MetadataValue = string | Buffer; export interface MetadataObject { - [propName: string]: Array; + [key: string]: Array; +} + +function cloneMetadataObject(repr: MetadataObject): MetadataObject { + const result: MetadataObject = {}; + forOwn(repr, (value, key) => { + // v.slice copies individual buffer values in value. + // TODO(kjin): Is this necessary + result[key] = value.map(v => { + if (v instanceof Buffer) { + return v.slice(); + } else { + return v; + } + }); + }); + return result; +} + +function isLegal(legalChars: Array, str: string): boolean { + for (let i = 0; i < str.length; i++) { + const legalCharsIndex = str.charCodeAt(i) >> 3; + if (!(1 << (str.charCodeAt(i) & 7) & legalChars[legalCharsIndex])) { + return false; + } + } + return true; +} + +const legalKeyChars = [ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x60, 0xff, 0x03, 0x00, 0x00, 0x00, + 0x80, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 +]; +const legalNonBinValueChars = [ + 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 +]; + +function isLegalKey(key: string): boolean { + return key.length > 0 && isLegal(legalKeyChars, key); +} + +function isLegalNonBinaryValue(value: string): boolean { + return isLegal(legalNonBinValueChars, value); +} + +function isBinaryKey(key: string): boolean { + return key.endsWith('-bin'); +} + +function normalizeKey(key: string): string { + return key.toLowerCase(); +} + +function validate(key: string, value?: MetadataValue): void { + if (!isLegalKey(key)) { + throw new Error('Metadata key"' + key + '" contains illegal characters'); + } + if (value != null) { + if (isBinaryKey(key)) { + if (!(value instanceof Buffer)) { + throw new Error('keys that end with \'-bin\' must have Buffer values'); + } + } else { + if (value instanceof Buffer) { + throw new Error( + 'keys that don\'t end with \'-bin\' must have String values'); + } + if (!isLegalNonBinaryValue(value)) { + throw new Error('Metadata string value "' + value + + '" contains illegal characters'); + } + } + } } export class Metadata { - static createMetadata(): Metadata { - return new Metadata(); + constructor(private readonly internalRepr: MetadataObject = {}) {} + + set(key: string, value: MetadataValue): void { + key = normalizeKey(key); + validate(key, value); + this.internalRepr[key] = [value]; } - set(_key: string, _value: MetadataValue): void { - throw new Error('Not implemented'); + add(key: string, value: MetadataValue): void { + key = normalizeKey(key); + validate(key, value); + if (!this.internalRepr[key]) { + this.internalRepr[key] = [value]; + } else { + this.internalRepr[key].push(value); + } } - add(_key: string, _value: MetadataValue): void { - throw new Error('Not implemented'); + remove(key: string): void { + key = normalizeKey(key); + validate(key); + if (Object.prototype.hasOwnProperty.call(this.internalRepr, key)) { + delete this.internalRepr[key]; + } } - remove(_key: string): void { - throw new Error('Not implemented'); + get(key: string): Array { + key = normalizeKey(key); + validate(key); + if (Object.prototype.hasOwnProperty.call(this.internalRepr, key)) { + return this.internalRepr[key]; + } else { + return []; + } } - get(_key: string): Array { - throw new Error('Not implemented'); - } - - getMap(): MetadataObject { - throw new Error('Not implemented'); + getMap(): { [key: string]: MetadataValue } { + const result: { [key: string]: MetadataValue } = {}; + forOwn(this.internalRepr, function(values, key) { + if(values.length > 0) { + const v = values[0]; + result[key] = v instanceof Buffer ? v.slice() : v; + } + }); + return result; } clone(): Metadata { - throw new Error('Not implemented'); + return new Metadata(cloneMetadataObject(this.internalRepr)); } } diff --git a/test/test-metadata.ts b/test/test-metadata.ts new file mode 100644 index 00000000..17c89a48 --- /dev/null +++ b/test/test-metadata.ts @@ -0,0 +1,182 @@ +import * as assert from 'assert'; +import { Metadata } from '../src/metadata'; + +describe('Metadata', () => { + let metadata: Metadata; + + beforeEach(() => { + metadata = new Metadata(); + }); + + describe('set', () => { + it('Only accepts string values for non "-bin" keys', () => { + assert.throws(() => { + metadata.set('key', new Buffer('value')); + }); + assert.doesNotThrow(() => { + metadata.set('key', 'value'); + }); + }); + + it('Only accepts Buffer values for "-bin" keys', () => { + assert.throws(() => { + metadata.set('key-bin', 'value'); + }); + assert.doesNotThrow(() => { + metadata.set('key-bin', new Buffer('value')); + }); + }); + + it('Rejects invalid keys', () => { + assert.throws(() => { + metadata.set('key$', 'value'); + }); + assert.throws(() => { + metadata.set('', 'value'); + }); + }); + + it('Rejects values with non-ASCII characters', () => { + assert.throws(() => { + metadata.set('key', 'résumé'); + }); + }); + + it('Saves values that can be retrieved', () => { + metadata.set('key', 'value'); + assert.deepEqual(metadata.get('key'), ['value']); + }); + + it('Overwrites previous values', () => { + metadata.set('key', 'value1'); + metadata.set('key', 'value2'); + assert.deepEqual(metadata.get('key'), ['value2']); + }); + + it('Normalizes keys', () => { + metadata.set('Key', 'value1'); + assert.deepEqual(metadata.get('key'), ['value1']); + metadata.set('KEY', 'value2'); + assert.deepEqual(metadata.get('key'), ['value2']); + }); + }); + + describe('add', () => { + it('Only accepts string values for non "-bin" keys', () => { + assert.throws(() => { + metadata.add('key', new Buffer('value')); + }); + assert.doesNotThrow(() => { + metadata.add('key', 'value'); + }); + }); + + it('Only accepts Buffer values for "-bin" keys', () => { + assert.throws(() => { + metadata.add('key-bin', 'value'); + }); + assert.doesNotThrow(() => { + metadata.add('key-bin', new Buffer('value')); + }); + }); + + it('Rejects invalid keys', () => { + assert.throws(() => { + metadata.add('key$', 'value'); + }); + assert.throws(() => { + metadata.add('', 'value'); + }); + }); + + it('Saves values that can be retrieved', () => { + metadata.add('key', 'value'); + assert.deepEqual(metadata.get('key'), ['value']); + }); + + it('Combines with previous values', () => { + metadata.add('key', 'value1'); + metadata.add('key', 'value2'); + assert.deepEqual(metadata.get('key'), ['value1', 'value2']); + }); + + it('Normalizes keys', () => { + metadata.add('Key', 'value1'); + assert.deepEqual(metadata.get('key'), ['value1']); + metadata.add('KEY', 'value2'); + assert.deepEqual(metadata.get('key'), ['value1', 'value2']); + }); + }); + + describe('remove', () => { + it('clears values from a key', () => { + metadata.add('key', 'value'); + metadata.remove('key'); + assert.deepEqual(metadata.get('key'), []); + }); + + it('Normalizes keys', () => { + metadata.add('key', 'value'); + metadata.remove('KEY'); + assert.deepEqual(metadata.get('key'), []); + }); + }); + + describe('get', () => { + beforeEach(() => { + metadata.add('key', 'value1'); + metadata.add('key', 'value2'); + metadata.add('key-bin', new Buffer('value')); + }); + + it('gets all values associated with a key', () => { + assert.deepEqual(metadata.get('key'), ['value1', 'value2']); + }); + + it('Normalizes keys', () => { + assert.deepEqual(metadata.get('KEY'), ['value1', 'value2']); + }); + + it('returns an empty list for non-existent keys', () => { + assert.deepEqual(metadata.get('non-existent-key'), []); + }); + + it('returns Buffers for "-bin" keys', () => { + assert.ok(metadata.get('key-bin')[0] instanceof Buffer); + }); + }); + + describe('getMap', () => { + it('gets a map of keys to values', () => { + metadata.add('key1', 'value1'); + metadata.add('Key2', 'value2'); + metadata.add('KEY3', 'value3'); + assert.deepEqual(metadata.getMap(), + {key1: 'value1', + key2: 'value2', + key3: 'value3'}); + }); + }); + + describe('clone', () => { + it('retains values from the original', () => { + metadata.add('key', 'value'); + const copy = metadata.clone(); + assert.deepEqual(copy.get('key'), ['value']); + }); + + it('Does not see newly added values', () => { + metadata.add('key', 'value1'); + const copy = metadata.clone(); + metadata.add('key', 'value2'); + assert.deepEqual(copy.get('key'), ['value1']); + }); + + it('Does not add new values to the original', () => { + metadata.add('key', 'value1'); + const copy = metadata.clone(); + copy.add('key', 'value2'); + assert.deepEqual(metadata.get('key'), ['value1']); + }); + }); +}); \ No newline at end of file From 72f4bf47957f6b210d45e61e8945106fae9a0761 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Tue, 15 Aug 2017 19:07:59 -0700 Subject: [PATCH 2/8] Added comments --- src/metadata.ts | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/metadata.ts b/src/metadata.ts index 1936bfa2..5b3c10bd 100644 --- a/src/metadata.ts +++ b/src/metadata.ts @@ -81,15 +81,32 @@ function validate(key: string, value?: MetadataValue): void { } } +/** + * A class for storing metadata. Keys are normalized to lowercase ASCII. + */ export class Metadata { constructor(private readonly internalRepr: MetadataObject = {}) {} + /** + * Sets the given value for the given key by replacing any other values + * associated with that key. Normalizes the key. + * @param key The key to whose value should be set. + * @param value The value to set. Must be a buffer if and only + * if the normalized key ends with '-bin'. + */ set(key: string, value: MetadataValue): void { key = normalizeKey(key); validate(key, value); this.internalRepr[key] = [value]; } + /** + * Adds the given value for the given key by appending to a list of previous + * values associated with that key. Normalizes the key. + * @param key The key for which a new value should be appended. + * @param value The value to add. Must be a buffer if and only + * if the normalized key ends with '-bin'. + */ add(key: string, value: MetadataValue): void { key = normalizeKey(key); validate(key, value); @@ -100,6 +117,10 @@ export class Metadata { } } + /** + * Removes the given key and any associated values. Normalizes the key. + * @param key The key whose values should be removed. + */ remove(key: string): void { key = normalizeKey(key); validate(key); @@ -108,6 +129,11 @@ export class Metadata { } } + /** + * Gets a list of all values associated with the key. Normalizes the key. + * @param key The key whose value should be retrieved. + * @return A list of values associated with the given key. + */ get(key: string): Array { key = normalizeKey(key); validate(key); @@ -118,6 +144,11 @@ export class Metadata { } } + /** + * Gets a plain object mapping each key to the first value associated with it. + * This reflects the most common way that people will want to see metadata. + * @return A key/value mapping of the metadata. + */ getMap(): { [key: string]: MetadataValue } { const result: { [key: string]: MetadataValue } = {}; forOwn(this.internalRepr, function(values, key) { @@ -129,6 +160,10 @@ export class Metadata { return result; } + /** + * Clones the metadata object. + * @return The newly cloned object. + */ clone(): Metadata { return new Metadata(cloneMetadataObject(this.internalRepr)); } From 0a1eb630f3b9e28706933d4982c0f09a34ec9455 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Thu, 17 Aug 2017 11:55:16 -0700 Subject: [PATCH 3/8] add Metadata#merge --- src/metadata.ts | 17 +++++++++++++++-- test/test-metadata.ts | 37 ++++++++++++++++++++++++++++++++++--- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/metadata.ts b/src/metadata.ts index 5b3c10bd..bac6ef66 100644 --- a/src/metadata.ts +++ b/src/metadata.ts @@ -85,7 +85,7 @@ function validate(key: string, value?: MetadataValue): void { * A class for storing metadata. Keys are normalized to lowercase ASCII. */ export class Metadata { - constructor(private readonly internalRepr: MetadataObject = {}) {} + constructor(protected readonly internalRepr: MetadataObject = {}) {} /** * Sets the given value for the given key by replacing any other values @@ -151,7 +151,7 @@ export class Metadata { */ getMap(): { [key: string]: MetadataValue } { const result: { [key: string]: MetadataValue } = {}; - forOwn(this.internalRepr, function(values, key) { + forOwn(this.internalRepr, (values, key) => { if(values.length > 0) { const v = values[0]; result[key] = v instanceof Buffer ? v.slice() : v; @@ -167,4 +167,17 @@ export class Metadata { clone(): Metadata { return new Metadata(cloneMetadataObject(this.internalRepr)); } + + /** + * Merges all key-value pairs from a given Metadata object into this one. + * If both this object and the given object have values in the same key, + * values from the other Metadata object will be appended to this object's + * values. + * @param other A Metadata object. + */ + merge(other: Metadata): void { + forOwn(other.internalRepr, (values, key) => { + this.internalRepr[key] = (this.internalRepr[key] || []).concat(values); + }); + } } diff --git a/test/test-metadata.ts b/test/test-metadata.ts index 17c89a48..ee4b25c6 100644 --- a/test/test-metadata.ts +++ b/test/test-metadata.ts @@ -1,5 +1,11 @@ import * as assert from 'assert'; -import { Metadata } from '../src/metadata'; +import * as metadata from '../src/metadata'; + +class Metadata extends metadata.Metadata { + getInternalRepresentation() { + return this.internalRepr; + } +} describe('Metadata', () => { let metadata: Metadata; @@ -150,11 +156,12 @@ describe('Metadata', () => { it('gets a map of keys to values', () => { metadata.add('key1', 'value1'); metadata.add('Key2', 'value2'); - metadata.add('KEY3', 'value3'); + metadata.add('KEY3', 'value3a'); + metadata.add('KEY3', 'value3b'); assert.deepEqual(metadata.getMap(), {key1: 'value1', key2: 'value2', - key3: 'value3'}); + key3: 'value3a'}); }); }); @@ -179,4 +186,28 @@ describe('Metadata', () => { assert.deepEqual(metadata.get('key'), ['value1']); }); }); + + describe('merge', () => { + it('appends values from a given metadata object', () => { + metadata.add('key1', 'value1'); + metadata.add('Key2', 'value2a'); + metadata.add('KEY3', 'value3a'); + metadata.add('key4', 'value4'); + const metadata2 = new Metadata(); + metadata2.add('KEY1', 'value1'); + metadata2.add('key2', 'value2b'); + metadata2.add('key3', 'value3b'); + metadata2.add('key5', 'value5a'); + metadata2.add('key5', 'value5b'); + const metadata2IR = metadata2.getInternalRepresentation(); + metadata.merge(metadata2); + // Ensure metadata2 didn't change + assert.deepEqual(metadata2.getInternalRepresentation(), metadata2IR); + assert.deepEqual(metadata.get('key1'), ['value1', 'value1']); + assert.deepEqual(metadata.get('key2'), ['value2a', 'value2b']); + assert.deepEqual(metadata.get('key3'), ['value3a', 'value3b']); + assert.deepEqual(metadata.get('key4'), ['value4']); + assert.deepEqual(metadata.get('key5'), ['value5a', 'value5b']); + }); + }); }); \ No newline at end of file From 44b97e660b3d6d1c70af7d4b9578ada89f075310 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Thu, 17 Aug 2017 12:07:14 -0700 Subject: [PATCH 4/8] Use regexes to validate metadata --- src/metadata.ts | 25 ++----------------------- test/test-metadata.ts | 12 ++++++++++++ 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/metadata.ts b/src/metadata.ts index bac6ef66..62e61096 100644 --- a/src/metadata.ts +++ b/src/metadata.ts @@ -22,33 +22,12 @@ function cloneMetadataObject(repr: MetadataObject): MetadataObject { return result; } -function isLegal(legalChars: Array, str: string): boolean { - for (let i = 0; i < str.length; i++) { - const legalCharsIndex = str.charCodeAt(i) >> 3; - if (!(1 << (str.charCodeAt(i) & 7) & legalChars[legalCharsIndex])) { - return false; - } - } - return true; -} - -const legalKeyChars = [ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x60, 0xff, 0x03, 0x00, 0x00, 0x00, - 0x80, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 -]; -const legalNonBinValueChars = [ - 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 -]; - function isLegalKey(key: string): boolean { - return key.length > 0 && isLegal(legalKeyChars, key); + return !!key.match(/^[0-9a-z_.-]+$/); } function isLegalNonBinaryValue(value: string): boolean { - return isLegal(legalNonBinValueChars, value); + return !!value.match(/^[ -~]+$/); } function isBinaryKey(key: string): boolean { diff --git a/test/test-metadata.ts b/test/test-metadata.ts index ee4b25c6..3d28cd40 100644 --- a/test/test-metadata.ts +++ b/test/test-metadata.ts @@ -1,4 +1,5 @@ import * as assert from 'assert'; +import { range } from 'lodash'; import * as metadata from '../src/metadata'; class Metadata extends metadata.Metadata { @@ -7,6 +8,11 @@ class Metadata extends metadata.Metadata { } } +const validKeyChars = '0123456789abcdefghijklmnopqrstuvwxyz_-.'; +const validNonBinValueChars = range(0x20, 0x7f) + .map(code => String.fromCharCode(code)) + .join(''); + describe('Metadata', () => { let metadata: Metadata; @@ -34,6 +40,9 @@ describe('Metadata', () => { }); it('Rejects invalid keys', () => { + assert.doesNotThrow(() => { + metadata.set(validKeyChars, 'value'); + }); assert.throws(() => { metadata.set('key$', 'value'); }); @@ -43,6 +52,9 @@ describe('Metadata', () => { }); it('Rejects values with non-ASCII characters', () => { + assert.doesNotThrow(() => { + metadata.set('key', validNonBinValueChars); + }); assert.throws(() => { metadata.set('key', 'résumé'); }); From 9403167d5f442fdaf539fdd03c752f12e2928026 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Thu, 17 Aug 2017 13:55:16 -0700 Subject: [PATCH 5/8] add toHttp2Headers --- package.json | 1 + src/metadata.ts | 20 ++++++++++++++++++++ test/test-metadata.ts | 27 +++++++++++++++++++++++++++ tsconfig.json | 5 ++++- 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index a92c3267..e68c401b 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "gulp-tslint": "^8.1.1", "gulp-typescript": "^3.2.1", "gulp-util": "^3.0.8", + "h2-types": "git+https://github.com/kjin/node-h2-types.git", "merge2": "^1.1.0", "mocha": "^3.5.0", "through2": "^2.0.3", diff --git a/src/metadata.ts b/src/metadata.ts index 62e61096..b395c0f8 100644 --- a/src/metadata.ts +++ b/src/metadata.ts @@ -1,4 +1,5 @@ import { forOwn } from 'lodash'; +import * as http2 from 'http2'; export type MetadataValue = string | Buffer; @@ -159,4 +160,23 @@ export class Metadata { this.internalRepr[key] = (this.internalRepr[key] || []).concat(values); }); } + + /** + * Creates an OutgoingHttpHeaders object that can be used with the http2 API. + */ + toHttp2Headers(): http2.OutgoingHttpHeaders { + const result: http2.OutgoingHttpHeaders = {}; + forOwn(this.internalRepr, (values, key) => { + // We assume that the user's interaction with this object is limited to + // through its public API (i.e. keys and values are already validated). + result[key] = values.map((value) => { + if (value instanceof Buffer) { + return value.toString('base64'); + } else { + return value; + } + }); + }); + return result; + } } diff --git a/test/test-metadata.ts b/test/test-metadata.ts index 3d28cd40..7d6d3851 100644 --- a/test/test-metadata.ts +++ b/test/test-metadata.ts @@ -222,4 +222,31 @@ describe('Metadata', () => { assert.deepEqual(metadata.get('key5'), ['value5a', 'value5b']); }); }); + + describe('toHttp2Headers', () => { + it('creates an http2.OutgoingHttpHeaders object', () => { + metadata.add('key1', 'value1'); + metadata.add('Key2', 'value2'); + metadata.add('KEY3', 'value3a'); + metadata.add('key3', 'value3b'); + metadata.add('key-bin', Buffer.from(range(0, 16))); + metadata.add('key-bin', Buffer.from(range(16, 32))); + metadata.add('key-bin', Buffer.from(range(0, 32))); + const headers = metadata.toHttp2Headers(); + assert.deepEqual(headers, { + key1: ['value1'], + key2: ['value2'], + key3: ['value3a', 'value3b'], + 'key-bin': [ + 'AAECAwQFBgcICQoLDA0ODw==', + 'EBESExQVFhcYGRobHB0eHw==', + 'AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=' + ] + }); + }); + + it('creates an empty header object from empty Metadata', () => { + assert.deepEqual(metadata.toHttp2Headers(), {}); + }); + }); }); \ No newline at end of file diff --git a/tsconfig.json b/tsconfig.json index cc6364d9..e189ea83 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,10 @@ { "extends": "./node_modules/google-ts-style/tsconfig-google.json", "compilerOptions": { - "lib": [ "es6" ] + "lib": [ "es6" ], + "typeRoots": [ + "node_modules/h2-types", "node_modules/@types" + ] }, "include": [ "src/**/*.ts", From ae20e248eac4b41623959a7d548dc04acbe1059c Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Thu, 17 Aug 2017 14:07:43 -0700 Subject: [PATCH 6/8] Modifying call credentials to reflect metadata changes --- src/call-credentials.ts | 9 ++------- test/test-call-credentials.ts | 33 ++++----------------------------- 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/src/call-credentials.ts b/src/call-credentials.ts index cd79315b..6e196823 100644 --- a/src/call-credentials.ts +++ b/src/call-credentials.ts @@ -63,15 +63,10 @@ class CallCredentialsImpl { cb(err || new Error('Unknown error')); return; } else { - const result = Metadata.createMetadata(); + const result: Metadata = new Metadata(); metadataArray.forEach((metadata) => { if (metadata) { - const metadataObj = metadata.getMap(); - Object.keys(metadataObj).forEach((key) => { - metadataObj[key].forEach((value) => { - result.add(key, value); - }); - }); + result.merge(metadata); } }); cb(null, result); diff --git a/test/test-call-credentials.ts b/test/test-call-credentials.ts index 155fb1a6..13b2302d 100644 --- a/test/test-call-credentials.ts +++ b/test/test-call-credentials.ts @@ -1,28 +1,7 @@ import { Metadata } from '../src/metadata'; import { CallCredentials, CallMetadataGenerator } from '../src/call-credentials'; -import { mockFunction } from './common'; import * as assert from 'assert'; -class MetadataMock extends Metadata { - constructor(private obj: { [propName: string]: Array } = {}) { - super(); - } - - add(key: string, value: string) { - if (!this.obj[key]) { - this.obj[key] = [value]; - } else { - this.obj[key].push(value); - } - } - clone() { return new MetadataMock(Object.create(this.obj)); }; - get(key: string) { return this.obj[key]; } - getMap() { return this.obj; } - set() { mockFunction() } - remove() { mockFunction() } -} -Metadata.createMetadata = () => new MetadataMock(); - // Returns a Promise that resolves to an object containing either an error or // metadata function generateMetadata( @@ -40,7 +19,7 @@ function generateMetadata( function makeGenerator(props: Array): CallMetadataGenerator { return (options: { [propName: string]: string }, cb) => { - const metadata: Metadata = new MetadataMock(); + const metadata: Metadata = new Metadata(); props.forEach((prop) => { if (options[prop]) { metadata.add(prop, options[prop]); @@ -52,7 +31,7 @@ function makeGenerator(props: Array): CallMetadataGenerator { function makeAfterMsElapsedGenerator(ms: number): CallMetadataGenerator { return (_options, cb) => { - const metadata = new MetadataMock(); + const metadata = new Metadata(); metadata.add('msElapsed', `${ms}`); setTimeout(() => cb(null, metadata), ms); }; @@ -101,9 +80,7 @@ describe('CallCredentials', () => { assert.ok(!err); assert.ok(metadata); if (metadata) { - assert.deepEqual(metadata.getMap(), { - name: ['foo'] - }); + assert.deepEqual(metadata.get('name'), ['foo']); } } ); @@ -153,9 +130,7 @@ describe('CallCredentials', () => { assert.ok(!err); assert.ok(metadata); if (metadata) { - assert.deepEqual(metadata.getMap(), { - msElapsed: expected - }); + assert.deepEqual(metadata.get('msElapsed'), expected); } })); }); From 95827662b4178fc5f42195ed715ae7df0377ea13 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Thu, 17 Aug 2017 14:09:59 -0700 Subject: [PATCH 7/8] add newline --- test/test-metadata.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-metadata.ts b/test/test-metadata.ts index 7d6d3851..6c3e99cb 100644 --- a/test/test-metadata.ts +++ b/test/test-metadata.ts @@ -249,4 +249,4 @@ describe('Metadata', () => { assert.deepEqual(metadata.toHttp2Headers(), {}); }); }); -}); \ No newline at end of file +}); From fb38744985f4c93d82557738a061b856eb09691f Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Wed, 23 Aug 2017 10:51:37 -0700 Subject: [PATCH 8/8] Add fromHttp2Headers --- src/metadata.ts | 29 +++++++++++++++++++++++++++++ test/test-metadata.ts | 43 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/metadata.ts b/src/metadata.ts index b395c0f8..e9e71208 100644 --- a/src/metadata.ts +++ b/src/metadata.ts @@ -179,4 +179,33 @@ export class Metadata { }); return result; } + + /** + * Returns a new Metadata object based fields in a given IncomingHttpHeaders + * object. + * @param headers An IncomingHttpHeaders object. + */ + static fromHttp2Headers(headers: http2.IncomingHttpHeaders): Metadata { + const result = new Metadata(); + forOwn(headers, (values, key) => { + if (isBinaryKey(key)) { + if (Array.isArray(values)) { + values.forEach((value) => { + result.add(key, Buffer.from(value, 'base64')); + }) + } else { + result.add(key, Buffer.from(values, 'base64')); + } + } else { + if (Array.isArray(values)) { + values.forEach((value) => { + result.add(key, value); + }) + } else { + result.add(key, values); + } + } + }); + return result; + } } diff --git a/test/test-metadata.ts b/test/test-metadata.ts index 6c3e99cb..f884c14d 100644 --- a/test/test-metadata.ts +++ b/test/test-metadata.ts @@ -1,4 +1,5 @@ import * as assert from 'assert'; +import * as http2 from 'http2'; import { range } from 'lodash'; import * as metadata from '../src/metadata'; @@ -6,6 +7,13 @@ class Metadata extends metadata.Metadata { getInternalRepresentation() { return this.internalRepr; } + + static fromHttp2Headers(headers: http2.IncomingHttpHeaders): Metadata { + const result = metadata.Metadata.fromHttp2Headers(headers) as Metadata; + result.getInternalRepresentation = + Metadata.prototype.getInternalRepresentation; + return result; + } } const validKeyChars = '0123456789abcdefghijklmnopqrstuvwxyz_-.'; @@ -224,7 +232,7 @@ describe('Metadata', () => { }); describe('toHttp2Headers', () => { - it('creates an http2.OutgoingHttpHeaders object', () => { + it('creates an OutgoingHttpHeaders object with expected values', () => { metadata.add('key1', 'value1'); metadata.add('Key2', 'value2'); metadata.add('KEY3', 'value3a'); @@ -249,4 +257,37 @@ describe('Metadata', () => { assert.deepEqual(metadata.toHttp2Headers(), {}); }); }); + + describe('fromHttp2Headers', () => { + it('creates a Metadata object with expected values', () => { + const headers = { + key1: 'value1', + key2: ['value2'], + key3: ['value3a', 'value3b'], + 'key-bin': [ + 'AAECAwQFBgcICQoLDA0ODw==', + 'EBESExQVFhcYGRobHB0eHw==', + 'AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=' + ] + }; + const metadataFromHeaders = Metadata.fromHttp2Headers(headers); + const internalRepr = metadataFromHeaders.getInternalRepresentation(); + assert.deepEqual(internalRepr, { + key1: ['value1'], + key2: ['value2'], + key3: ['value3a', 'value3b'], + 'key-bin': [ + Buffer.from(range(0, 16)), + Buffer.from(range(16, 32)), + Buffer.from(range(0, 32)) + ] + }); + }); + + it('creates an empty Metadata object from empty headers', () => { + const metadataFromHeaders = Metadata.fromHttp2Headers({}); + const internalRepr = metadataFromHeaders.getInternalRepresentation(); + assert.deepEqual(internalRepr, {}); + }); + }); });