Azure / azure-functions-host

The host/runtime that powers Azure Functions
https://functions.azure.com
MIT License
1.91k stars 435 forks source link

Support json as a dataType for queueTrigger #4207

Open TylerLeonhardt opened 5 years ago

TylerLeonhardt commented 5 years ago

Repro steps

  1. Have a queue trigger
  2. Insert a json string into the queue as the message data: {"hello":"World"}

Expected behavior

Function receives the json as a json object so it can be properly turned into a hashtable for PowerShell Function users

Actual behavior

Function receives the json as a string and it gets passed in as a string.

Known workarounds

param($QueueItem)

$parsedData = $QueueItem | ConvertFrom-Json

Related information

mhoeger commented 5 years ago

@TylerLeonhardt - I think this should be behavior controlled by the language worker instead of the host? Ex: https://github.com/Azure/azure-functions-nodejs-worker/blob/b678aea76bfcba3ae082558d67e07745e29d5176/src/Converters.ts#L43

pragnagopa commented 5 years ago

@TylerLeonhardt - As @mhoeger mentioned, host sends trigger input data as string/json

https://github.com/Azure/azure-functions-language-worker-protobuf/blob/2e9cf4b3ac3864e3f5db07dffefa751d38baa368/src/proto/FunctionRpc.proto#L284

Language worker needs to control how it is presented to the function code on invocation.

Closing this as for now. Please reopen if you still think we need to address this.

TylerLeonhardt commented 5 years ago

Sorry for the delay! Not sure how I missed this.

@mhoeger I see from the snippet that the node worker treats string/json as the same and attempts to parse it regardless (even if the TypedData is a string). Thanks for that! I can replicate that in the PowerShell worker... but it seems a bit confusing from a language worker's perspective.

My understanding would be that the host would use the TypedData as json if the data were json and string if it wasn't. We do this today in the powershell worker.

If the expectation is that the worker should try to parse TypedData.string as json, then is there any particular reason for TypedData.json over just sending everything through TypedData.string?

pragnagopa commented 5 years ago

Host should set typedData.Json if the input data is a json string.

https://github.com/Azure/azure-functions-host/blob/8bd4feed5cc266ff5986ebe4ac4397cadc0eb0cf/src/WebJobs.Script/Rpc/MessageExtensions/RpcMessageConversionExtensions.cs#L63-L66

Reopening this issue to investigate.

TylerLeonhardt commented 5 years ago

Thanks @pragnagopa! We have implemented what the node worker does (thanks again for that, @mhoeger!) in https://github.com/Azure/azure-functions-powershell-worker/pull/186

daxian-dbw commented 5 years ago

A related issue: https://github.com/Azure/azure-functions-powershell-worker/issues/205

@pragnagopa when fixing this issue, please be noted that for the RpcHttp.RawBody field, it should be kept as typedData.String even if it's a valid JSON string.

PowershellNinja commented 2 weeks ago

I disagree with the OP in general as I personally find forced parsing of string values undesireable.

Regardless of that though, as it is implemented at the moment, it generates a "System.Management.Automation.OrderedHashtable", which from a PowerShell perspective seems wrong - the builtin ConvertFrom-Json Cmdlet creates a "System.Management.Automation.PSCustomObject" if used to parse a JSON string with one object in it. Examples:

PS C:\> '{"action":"a","option":"b"}' | ConvertFrom-Json

action option
------ ------
a      b
PS C:\> '{"action":"a","option":"b"}' | ConvertFrom-Json | Get-Member

   TypeName: System.Management.Automation.PSCustomObject

Name        MemberType   Definition
----        ----------   ----------
Equals      Method       bool Equals(System.Object obj)
GetHashCode Method       int GetHashCode()
GetType     Method       type GetType()
ToString    Method       string ToString()
action      NoteProperty string action=a
option      NoteProperty string option=b

So, if the trigger absolutely needs to parse an input JSON string, it should in my opinion mirror PowerShell's normal behavior and create a PSCustomObject, not convert into a Hashtable.