Skip to content

Commit

Permalink
Clean up camel case and typed data parsing
Browse files Browse the repository at this point in the history
Main points:
- We don't want `toCamelCase` to call `fromRpcTypedData` because it traverses an entire object recursively and only the top level objects should be RpcTypedData
- We will remove triggerMetadata from http/timer triggers. That information is best found on the request/timer objects instead

Related to all of these bugs:
Azure/azure-functions-nodejs-worker#607
Azure/azure-functions-nodejs-worker#274
Azure/azure-functions-nodejs-worker#204
Azure/azure-functions-nodejs-worker#388
  • Loading branch information
ejizba committed Aug 25, 2022
1 parent 068dcf4 commit efa5fb3
Show file tree
Hide file tree
Showing 16 changed files with 598 additions and 249 deletions.
23 changes: 8 additions & 15 deletions src/InvocationContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,26 @@

import * as types from '@azure/functions';
import { InvocationContextInit, LogHandler, RetryContext, TraceContext, TriggerMetadata } from '@azure/functions';
import { RpcInvocationRequest } from '@azure/functions-core';
import { convertKeysToCamelCase } from './converters/convertKeysToCamelCase';
import { fromRpcRetryContext, fromRpcTraceContext } from './converters/fromRpcContext';

export class InvocationContext implements types.InvocationContext {
invocationId: string;
functionName: string;
triggerMetadata: TriggerMetadata;
traceContext?: TraceContext;
retryContext?: RetryContext;
extraInputs: InvocationContextExtraInputs;
extraOutputs: InvocationContextExtraOutputs;
retryContext?: RetryContext;
traceContext?: TraceContext;
triggerMetadata?: TriggerMetadata;
#userLogHandler: LogHandler;

constructor(init: InvocationContextInit & RpcInvocationRequest) {
constructor(init: InvocationContextInit) {
this.invocationId = init.invocationId;
this.functionName = init.functionName;
this.triggerMetadata = init.triggerMetadata ? convertKeysToCamelCase(init.triggerMetadata) : {};
if (init.retryContext) {
this.retryContext = fromRpcRetryContext(init.retryContext);
}
if (init.traceContext) {
this.traceContext = fromRpcTraceContext(init.traceContext);
}
this.#userLogHandler = init.logHandler;
this.extraInputs = new InvocationContextExtraInputs();
this.extraOutputs = new InvocationContextExtraOutputs();
this.retryContext = init.retryContext;
this.traceContext = init.traceContext;
this.triggerMetadata = init.triggerMetadata;
this.#userLogHandler = init.logHandler;
}

log(...args: unknown[]): void {
Expand Down
55 changes: 31 additions & 24 deletions src/InvocationModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,55 +14,62 @@ import {
} from '@azure/functions-core';
import { format } from 'util';
import { returnBindingKey } from './constants';
import { convertKeysToCamelCase } from './converters/convertKeysToCamelCase';
import { fromRpcRetryContext, fromRpcTraceContext } from './converters/fromRpcContext';
import { fromRpcTriggerMetadata } from './converters/fromRpcTriggerMetadata';
import { fromRpcTypedData } from './converters/fromRpcTypedData';
import { toCamelCaseValue } from './converters/toCamelCase';
import { toRpcHttp } from './converters/toRpcHttp';
import { toRpcTypedData } from './converters/toRpcTypedData';
import { HttpRequest } from './http/HttpRequest';
import { InvocationContext } from './InvocationContext';
import { nonNullProp } from './utils/nonNull';
import { isTimerTrigger, isTrigger } from './utils/isTrigger';
import { nonNullProp, nonNullValue } from './utils/nonNull';

export class InvocationModel implements coreTypes.InvocationModel {
#isDone = false;
#coreCtx: CoreInvocationContext;
#functionName: string;
#bindings: Record<string, RpcBindingInfo>;
#triggerType: string;

constructor(coreCtx: CoreInvocationContext) {
this.#coreCtx = coreCtx;
this.#functionName = nonNullProp(coreCtx.metadata, 'name');
this.#bindings = nonNullProp(coreCtx.metadata, 'bindings');
const triggerBinding = nonNullValue(
Object.values(this.#bindings).find((b) => isTrigger(b.type)),
'triggerBinding'
);
this.#triggerType = nonNullProp(triggerBinding, 'type');
}

// eslint-disable-next-line @typescript-eslint/require-await
async getArguments(): Promise<InvocationArguments> {
const req = this.#coreCtx.request;

const context = new InvocationContext({
...this.#coreCtx.request,
invocationId: nonNullProp(this.#coreCtx, 'invocationId'),
functionName: this.#functionName,
logHandler: (level: RpcLogLevel, ...args: unknown[]) => this.#userLog(level, ...args),
retryContext: fromRpcRetryContext(req.retryContext),
traceContext: fromRpcTraceContext(req.traceContext),
triggerMetadata: fromRpcTriggerMetadata(req.triggerMetadata, this.#triggerType),
});

const inputs: any[] = [];
if (this.#coreCtx.request.inputData) {
for (const binding of this.#coreCtx.request.inputData) {
if (binding.data && binding.name) {
let input: any;
const bindingType = this.#bindings[binding.name].type?.toLowerCase();
if (binding.data.http) {
input = new HttpRequest(binding.data.http);
} else if (bindingType === 'timertrigger') {
// TODO: Don't hard code fix for camelCase https://github.com/Azure/azure-functions-nodejs-worker/issues/188
input = convertKeysToCamelCase(binding.data);
} else {
input = fromRpcTypedData(binding.data);
}

if (bindingType && /trigger/i.test(bindingType)) {
inputs.push(input);
} else {
context.extraInputs.set(binding.name, input);
}
const inputs: unknown[] = [];
if (req.inputData) {
for (const binding of req.inputData) {
const bindingName = nonNullProp(binding, 'name');
let input: unknown = fromRpcTypedData(binding.data);

const bindingType = this.#bindings[bindingName].type;
if (isTimerTrigger(bindingType)) {
input = toCamelCaseValue(input);
}

if (isTrigger(bindingType)) {
inputs.push(input);
} else {
context.extraInputs.set(bindingName, input);
}
}
}
Expand Down
28 changes: 0 additions & 28 deletions src/converters/convertKeysToCamelCase.ts

This file was deleted.

38 changes: 23 additions & 15 deletions src/converters/fromRpcContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,19 @@ import { Exception, RetryContext, TraceContext } from '@azure/functions';
import { RpcException, RpcRetryContext, RpcTraceContext } from '@azure/functions-core';
import { copyPropIfDefined, nonNullProp } from '../utils/nonNull';

export function fromRpcRetryContext(retryContext: RpcRetryContext): RetryContext {
const result: RetryContext = {
retryCount: nonNullProp(retryContext, 'retryCount'),
maxRetryCount: nonNullProp(retryContext, 'maxRetryCount'),
};
if (retryContext.exception) {
result.exception = fromRpcException(retryContext.exception);
export function fromRpcRetryContext(retryContext: RpcRetryContext | null | undefined): RetryContext | undefined {
if (!retryContext) {
return undefined;
} else {
const result: RetryContext = {
retryCount: nonNullProp(retryContext, 'retryCount'),
maxRetryCount: nonNullProp(retryContext, 'maxRetryCount'),
};
if (retryContext.exception) {
result.exception = fromRpcException(retryContext.exception);
}
return result;
}
return result;
}

function fromRpcException(exception: RpcException): Exception {
Expand All @@ -24,12 +28,16 @@ function fromRpcException(exception: RpcException): Exception {
return result;
}

export function fromRpcTraceContext(traceContext: RpcTraceContext): TraceContext {
const result: TraceContext = {};
copyPropIfDefined(traceContext, result, 'traceParent');
copyPropIfDefined(traceContext, result, 'traceState');
if (traceContext.attributes) {
result.attributes = traceContext.attributes;
export function fromRpcTraceContext(traceContext: RpcTraceContext | null | undefined): TraceContext | undefined {
if (!traceContext) {
return undefined;
} else {
const result: TraceContext = {};
copyPropIfDefined(traceContext, result, 'traceParent');
copyPropIfDefined(traceContext, result, 'traceState');
if (traceContext.attributes) {
result.attributes = traceContext.attributes;
}
return result;
}
return result;
}
27 changes: 27 additions & 0 deletions src/converters/fromRpcTriggerMetadata.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License.

import { TriggerMetadata } from '@azure/functions';
import { RpcTypedData } from '@azure/functions-core';
import { isHttpTrigger, isTimerTrigger } from '../utils/isTrigger';
import { fromRpcTypedData } from './fromRpcTypedData';
import { toCamelCaseKey, toCamelCaseValue } from './toCamelCase';

export function fromRpcTriggerMetadata(
triggerMetadata: Record<string, RpcTypedData> | null | undefined,
triggerType: string
): TriggerMetadata | undefined {
// For http and timer triggers, we will avoid using `triggerMetadata` for a few reasons:
// 1. It uses `toCamelCase` methods, which can lead to weird casing bugs
// 2. It's generally a large medley of properties that is difficult for us to document/type
// 3. We can represent that information on the request & timer objects instead
if (!triggerMetadata || isHttpTrigger(triggerType) || isTimerTrigger(triggerType)) {
return undefined;
} else {
const result: TriggerMetadata = {};
for (const [key, value] of Object.entries(triggerMetadata)) {
result[toCamelCaseKey(key)] = toCamelCaseValue(fromRpcTypedData(value));
}
return result;
}
}
65 changes: 37 additions & 28 deletions src/converters/fromRpcTypedData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,43 @@
// Licensed under the MIT License.

import { RpcTypedData } from '@azure/functions-core';
import { isLong } from 'long';
import { HttpRequest } from '../http/HttpRequest';
import { isDefined } from '../utils/nonNull';

export function fromRpcTypedData(typedData?: RpcTypedData, convertStringToJson = true) {
typedData = typedData || {};
let str = typedData.string || typedData.json;
if (str !== undefined) {
if (convertStringToJson) {
try {
if (str != null) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
str = JSON.parse(str);
}
} catch (err) {
// ignore
}
}
return str;
} else if (typedData.bytes) {
return Buffer.from(<Buffer>typedData.bytes);
} else if (typedData.collectionBytes && typedData.collectionBytes.bytes) {
const byteCollection = typedData.collectionBytes.bytes;
return byteCollection.map((element) => Buffer.from(<Buffer>element));
} else if (typedData.collectionString && typedData.collectionString.string) {
return typedData.collectionString.string;
} else if (typedData.collectionDouble && typedData.collectionDouble.double) {
return typedData.collectionDouble.double;
} else if (typedData.collectionSint64 && typedData.collectionSint64.sint64) {
const longCollection = typedData.collectionSint64.sint64;
return longCollection.map((element) => (isLong(element) ? element.toString() : element));
export function fromRpcTypedData(data: RpcTypedData | null | undefined): unknown {
if (!data) {
return undefined;
} else if (isDefined(data.string)) {
return tryJsonParse(data.string);
} else if (isDefined(data.json)) {
return JSON.parse(data.json);
} else if (isDefined(data.bytes)) {
return Buffer.from(data.bytes);
} else if (isDefined(data.stream)) {
return Buffer.from(data.stream);
} else if (isDefined(data.http)) {
return new HttpRequest(data.http);
} else if (isDefined(data.int)) {
return data.int;
} else if (isDefined(data.double)) {
return data.double;
} else if (data.collectionBytes && isDefined(data.collectionBytes.bytes)) {
return data.collectionBytes.bytes.map((d) => Buffer.from(d));
} else if (data.collectionString && isDefined(data.collectionString.string)) {
return data.collectionString.string.map(tryJsonParse);
} else if (data.collectionDouble && isDefined(data.collectionDouble.double)) {
return data.collectionDouble.double;
} else if (data.collectionSint64 && isDefined(data.collectionSint64.sint64)) {
return data.collectionSint64.sint64;
} else {
return undefined;
}
}

function tryJsonParse(data: string): unknown {
try {
return JSON.parse(data);
} catch {
return data;
}
}
20 changes: 20 additions & 0 deletions src/converters/toCamelCase.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License.

export function toCamelCaseValue(data: unknown): unknown {
if (typeof data !== 'object' || data === null) {
return data;
} else if (Array.isArray(data)) {
return data.map(toCamelCaseValue);
} else {
const result = {};
for (const [key, value] of Object.entries(data)) {
result[toCamelCaseKey(key)] = toCamelCaseValue(value);
}
return result;
}
}

export function toCamelCaseKey(key: string): string {
return key.charAt(0).toLowerCase() + key.slice(1);
}
3 changes: 2 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import { CoreInvocationContext, FunctionCallback } from '@azure/functions-core';
import { returnBindingKey, version } from './constants';
import { InvocationModel } from './InvocationModel';
import { getRandomHexString } from './utils/getRandomHexString';
import { isTrigger } from './utils/isTrigger';

export { HttpRequest } from './http/HttpRequest';
export { InvocationContext } from './InvocationContext';
Expand Down Expand Up @@ -212,7 +213,7 @@ export namespace app {
bindings[trigger.name] = {
...trigger,
direction: 'in',
type: /trigger$/i.test(trigger.type) ? trigger.type : trigger.type + 'Trigger',
type: isTrigger(trigger.type) ? trigger.type : trigger.type + 'Trigger',
};

if (options.extraInputs) {
Expand Down
14 changes: 14 additions & 0 deletions src/utils/isTrigger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License.

export function isTrigger(typeName: string | undefined | null): boolean {
return !!typeName && /trigger$/i.test(typeName);
}

export function isHttpTrigger(typeName: string | undefined | null): boolean {
return typeName?.toLowerCase() === 'httptrigger';
}

export function isTimerTrigger(typeName: string | undefined | null): boolean {
return typeName?.toLowerCase() === 'timertrigger';
}
4 changes: 4 additions & 0 deletions src/utils/nonNull.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@ export function copyPropIfDefined<TData, TKey extends keyof TData>(source: TData
destination[key] = source[key];
}
}

export function isDefined<T>(data: T | undefined | null): data is T {
return data !== null && data !== undefined;
}
Loading

0 comments on commit efa5fb3

Please sign in to comment.