ava-innersource / Liquid-Application-Framework-1.0-deprecated

Liquid is a framework to speed up the development of microservices
MIT License
25 stars 14 forks source link

LightWorker accepts *any* attribute in the place of MessageBusAttribute #195

Open bruno-brant opened 4 years ago

bruno-brant commented 4 years ago

In order to use LiquidWorker, you need to annotate the class with an attribute - usually MessageBusAttribute. However, what we discovered is that the code that attempts to find the attribute, well, accepts any attribute:

https://github.com/Avanade/Liquid-Application-Framework/blob/ac834f46809cd445dda8783ff616ea4ae7af6b33/src/Liquid.Activation/Worker/LightWorker.cs#L38-L45

As you can see, the code gets all attributes (ReflectedType.CustomAttributes) and, if there's any, then it gets the first, and uses the first attribute as the tag name... whatever it is. And, of course, the code will break if there's no attributes.

So, while this is what is expected...

[MessageBus("BusNameInConfig")]
public class MyWorker : ILightWorker
{

}

...this is also acceptable...

[Obsolete("BusTag")]
public class Worker : ILightWorker
{
    // ...
}

...and this will fail:

[Obsolete, MessageBus("BusTag")]
public class Worker : ILightWorker
{
    // ...
}
bruno-brant commented 4 years ago

I'm reluctant to tag this as a bug, for if the user correctly annotates the class with MessageBusAttribute, then, everything works, although -- as the example above shows -- there are many conditions where this can lead to obscure errors during startup.

So I'm tagging it as a priority enhancement.

bruno-brant commented 4 years ago

(closed by mistake)

guilhermeluizsp commented 4 years ago

Let's not forget that the section 24.2 of the C# specification explicitly says that the order of the attributes is not guaranteed, so, even if we do something like [MessageBus("Tag"), MyOtherAttribute], we still may get errors at runtime.

bruno-brant commented 4 years ago

Let's not forget that the section 24.2 of the C# specification explicitly says that the order of the attributes is not guaranteed, so, even if we do something like [MessageBus("Tag"), MyOtherAttribute], we still may get errors at runtime.

Oh yes, indeed, I've forgotten about this.

For that reason I'm elevating this issue to a bug.