Why does EventGridTriggerBinding need to support multiple types to convert from?

[drub0y]

Hi all, I was attempting to rebase PR #12 on top of the latest changes and ran into an interesting problem. A unit test was written for the EvenGridTriggerBinding::BindAsync method here:

azure-functions-eventgrid-extension/test/Extension.tests/UnitTests.cs

Lines 20 to 44 in 9a9b3ee

	public async Task bindAsyncTest()
	{
	MethodBase methodbase = this.GetType().GetMethod("DummyMethod", BindingFlags.NonPublic | BindingFlags.Instance);
	ParameterInfo[] arrayParam = methodbase.GetParameters();
	ITriggerBinding binding = new EventGridTriggerBinding(arrayParam[0], null, null);
	// given GventGridEvent
	EventGridEvent eve = JsonConvert.DeserializeObject<EventGridEvent>(FakeData.singleEvent);
	JObject data = eve.Data;
	ITriggerData triggerDataWithEvent = await binding.BindAsync(eve, null);
	Assert.Equal(data, triggerDataWithEvent.BindingData["data"]);
	ITriggerData triggerDataWithString = await binding.BindAsync(FakeData.singleEvent, null);
	Assert.Equal(data, triggerDataWithString.BindingData["data"]);
	// test invalid, batch of events
	FormatException formatException = await Assert.ThrowsAsync<FormatException>(() => binding.BindAsync(FakeData.arrayOfOneEvent, null));
	Assert.Equal($"Unable to parse {FakeData.arrayOfOneEvent} to {typeof(EventGridEvent)}", formatException.Message);
	// test invalid, random object
	var testObject = new TestClass();
	InvalidOperationException invalidException = await Assert.ThrowsAsync<InvalidOperationException>(() => binding.BindAsync(testObject, null));
	Assert.Equal($"Unable to bind {testObject} to type {arrayParam[0].ParameterType}", invalidException.Message);
	}

This unit test now passes in two totally different types of source values which the "old" EventGridTriggerBinding did support. They are string and EventGridEvent.

The question I have is: why does this binding need to support more than one? The binding itself is an internal class that closely cooperates with EventGridExtensionConfig's IAsyncConverter<HttpRequestMessage, HttpResponseMessage> whose responsibility it is to provide the value into the binding... albeit indirectly. Seen here:

azure-functions-eventgrid-extension/src/EventGridExtension/EventGridExtensionConfig.cs

Lines 85 to 96 in 9a9b3ee

	string jsonArray = await req.Content.ReadAsStringAsync();
	List<EventGridEvent> events = JsonConvert.DeserializeObject<List<EventGridEvent>>(jsonArray);
	foreach (var ev in events)
	{
	TriggeredFunctionData triggerData = new TriggeredFunctionData
	{
	TriggerValue = ev
	};
	await _listeners[functionName].Executor.TryExecuteAsync(triggerData, CancellationToken.None);
	}

In the current implementation, the EventGridExtensionConfig is always translating the incoming HttpRequestMessage body from JSON into a strongly typed List<EventGridEvent> and then triggering the functions with the individual EventGridEvent instances. Given that, the binding will never see string parameter.

So, that said, while the binding implementation supports converting the raw value from a string, it will never have to do that AFAICT and, therefore, the unit test that was added covers a scenario that doesn't really exist and should thus be removed. Additionally I would suggest that the test method is extremely overloaded with way too many responsibilities and should be refactored into multiple tests.

[drub0y]

In PR #12 I'm going to go ahead and refactor this test to work against my implementation. I'll push an update to the PR with those changes and await any other feedback the team might have to offer.

[watashiSHUN]

...I should track better of the issues
The answer lies in b6af0df
in short, there is more than 1 way to call bindAsync, one of which is through admin/functions/{name} api which allows user to pass the function argument in HTTP request body, function runtime will first parse it to string and then invoke user function (reference)

[sandromastronardi]

How about the cloudevent schema? i dont see that in the code currently. I see documentation stating that i could just use CloudEvent but the validation fails because there is nothing in there for cloud events?