Problem about EventBus Method Invoke

Topics: Core
Jun 14, 2013 at 10:39 AM
I have a question about TryInvokeMethod in Class DefaultWanerDaoEventBus.
private static bool TryInvokeMethod(IEventHandler eventHandler, Type interfaceType, string methodName, IDictionary<string, object> arguments, out IEnumerable returnValue)
{
            MethodInfo method = _interfaceMethodsCache.GetOrAdd(String.Concat(eventHandler.GetType().FullName + "_" + interfaceType.Name, "_", methodName, "_", String.Join("_", arguments.Keys)), GetMatchingMethod(eventHandler, interfaceType, methodName, arguments));

            if (method != null)
            {
                var parameters = new List<object>();
                foreach (var methodParameter in method.GetParameters())
                {
                    parameters.Add(arguments[methodParameter.Name]);
                }
                var result = method.Invoke(eventHandler, parameters.ToArray());
                returnValue = result as IEnumerable;
                //because the type of string is assignable from IEnumerable, is it a Orchard's bug?
                if (returnValue == null && result != null)
                    returnValue = new[] { result };
                return true;
            }
            returnValue = null;
            return false;
}
Look at this line:
var result = method.Invoke(eventHandler, parameters.ToArray());
returnValue = result as IEnumerable;
If result is a string type, returnValue will be converted to IEnumerable, so the out IEnumerable returnValue is wrong I think.
Is it a Orchard bug?
Developer
Jun 18, 2013 at 10:59 PM
Edited Jun 18, 2013 at 11:07 PM
Where does the DefaultWanerDaoEventBus come from? You mean DefaultOrchardEventBus (because this is the default implementation), right?

Anyway - it's not a bug and it's by design.

It's a bit cleaner to treat every return value as a collection. Most of the time there will be more than one handler responding to some event, so in the end an IEnumerable needs to be returned anyway - you want to receive and aggregate of all results. But if multiple handlers return an actual IEnumerable each, we want to return a concatenated collection, not a collection of IEnumerables.

Of course we could do that type check in NotifyHandlers method later and simply return an object from TryInvokeMethod but that would not change anything.
Jun 19, 2013 at 10:33 AM
I have understood what you just said. But here is my core question. if i want to get a return value of string type, obviously string is a implement of IEnumerable, but in fact, i will get IEnumerable<char> in NotifyHandlers:
foreach (var eventHandler in eventHandlers)
            {
                IEnumerable returnValue;
                if (TryNotifyHandler(eventHandler, messageName, interfaceName, methodName, eventData, out returnValue))
                {
                    if (returnValue != null)
                    {
                        foreach (var value in returnValue)// returnValue is just a string type
                        {
                            yield return value// will return char type, but i return a string value.
                        }
                    }
                }
            }
Developer
Jun 19, 2013 at 3:03 PM
Right, now I get your point. We should add an explicit check for string type there.
Please file a bug and put a link to this thread.