ArchUnitNET icon indicating copy to clipboard operation
ArchUnitNET copied to clipboard

HaveAnyAttributes seams to check that all exist is that how it should work?

Open jnormen opened this issue 1 year ago • 7 comments

When I do:

HaveAnyAttributes(....) It always require me to have the first attribute set. but when I also add a list of other attributes it force me to have them as well.

.HaveAnyAttributes(typeof(GetAttribute), typeof(PostAttribute), typeof(PutAttribute), typeof(DeleteAttribute))

 For me any sounds like if you find any of those act on them, but not it seams like it force me to have all those 4. 
 This seams odd. Why it's called Any if it not just check if any of them exist and then to the check? 

jnormen avatar Apr 22 '24 16:04 jnormen

Hi @jnormen, thank you for your question. To me the method HaveAnyAttributes(...) works exactly as one would expect - the condition is fulfilled if any of the attributes is present for your class, interface, etc. You can find the implementation at the following line: https://github.com/TNG/ArchUnitNET/blob/fabf95b02eba6fa2b7dc2b75549662b8d12a4706/ArchUnitNET/Fluent/Syntax/Elements/ObjectConditionsDefinition.cs#L1095

You can also check the following simple Xunit example. The first test passes since the classes all have at least one of the attributes Test1 and Test2. The attribute Test3 is not used anywhere and therefore simply ignored. The second test fails since the classes in question have neither of the three attributes:

[Fact]
public void HaveAnyAttributesPassingTest()
{
    var classesWithAtLeastOneAttribute = Classes()
        .That()
        .Are(typeof(ClassWithAttribute1), typeof(ClassWithAttribute2), typeof(ClassWithBothAttributes))
        .Should()
        .HaveAnyAttributes(typeof(Test1), typeof(Test2), typeof(Test3));

    classesWithAtLeastOneAttribute.Check(Architecture);
}

[Fact]
public void HaveAnyAttributesFailingTest()
{
    var classesWithoutOneOfTheAttributes = Classes()
        .That()
        .Are(typeof(ClassWithoutAttributes), typeof(ClassWithDifferentAttribute))
        .Should()
        .HaveAnyAttributes(typeof(Test1), typeof(Test2), typeof(Test3));

    classesWithoutOneOfTheAttributes.Check(Architecture);
}

[Test1]
internal class ClassWithAttribute1 { }

[Test2]
[Test4]
internal class ClassWithAttribute2 { }

[Test1]
[Test2]
internal class ClassWithBothAttributes { }

internal class ClassWithoutAttributes { }

[Test4]
internal class ClassWithDifferentAttribute{ }

internal class Test1 : Attribute { }

internal class Test2 : Attribute { }

internal class Test3 : Attribute { }

internal class Test4 : Attribute { }

If you still observe unexpected behavior with the method feel free to add a minimal example to your question.

mak638 avatar May 25 '24 19:05 mak638

 [Fact]
    public void Interfaces_Using_Refit_Should_End_With_Api()
    {
        Interfaces()
            .That()
            .HaveAnyAttributes(typeof(GetAttribute))
            .Should()
            .HaveNameEndingWith("Client")
            .Because("All interfaces with Refit attributes should end with 'Api'")
            .Check(Architecture);
    }

GetAttribute is a Refit Atribute. When I do like this the test expect there shall be an Interface using this attribute. If I add that I want any of the get, post, put attributes then it force me to have all 3 in my code.

So if I do: .HaveAnyAttributes(typeof(GetAttribute),typeof(PostAttribute))

And I have a GetAttribut no Post it complains there is no Post. And if I have a Post but no Get it says the opposite. So for me I'm forced to have all the attributes not just one of them.

for example:

 public interface ISvsClient
{
    [Post("/v1/balance-inquiry")]
    public Task<BalanceEnquiryResponse> GetBalanceEnquiryAsync(SvsBalanceEnquiryRequest svsBalanceEnquiryRequest);
}
[Fact]
    public void Interfaces_Using_Refit_Should_End_With_Api()
    {
        Interfaces()
            .That()
            .HaveAnyAttributes(typeof(GetAttribute),typeof(PostAttribute))
            .Should()
            .HaveNameEndingWith("Client")
            .Because("All interfaces with Refit attributes should end with 'Api'")
            .Check(Architecture);
    }

ArchUnitNET.Domain.Exceptions.TypeDoesNotExistInArchitecture: Type Refit.GetAttribute does not exist in provided architecture or is no attribute.

When I wrote the issue I did not check the source code, and now when I answer I haven't checked the source code either, but I see that I use Interface() not classes and are not sure if the implementation is different. I'm on the move now so just wanted to add a fast example as is.

I did not use the .Are(typeof(ClassWithAttribute1), typeof(ClassWithAttribute2), typeof(ClassWithBothAttributes)) things. maybe that something I need to add?

jnormen avatar May 26 '24 10:05 jnormen

Hi @jnormen,

could you quickly share with us how you load your classes? Are you sure that the Refit Attributes are correctly loaded and part of the analysed architecture?

fgather avatar Sep 03 '24 07:09 fgather

I use Refit. So it's interfaces.

You put attributes on them like this:

public interface IHelloClient { [Get("/hello/{greet}")] public Task<Response<string>> GetHelloAsync(string greet, CancellationToken cancellationToken = default); }

then I do like this: ` private static readonly Architecture Architecture = new ArchLoader().LoadAssemblies(typeof(Program).Assembly).Build();

[Fact]
public void Interfaces_Using_Refit_Should_End_With_Client()
{
    Interfaces()
        .That()
        .HaveAnyAttributes(typeof(GetAttribute))
        .Should()
        .HaveNameEndingWith("Client")
        .Because("All interfaces with Refit attributes should end with 'Client'")
        .Check(Architecture);`

This works, but I want to do like this: .HaveAnyAttributes(typeof(GetAttribute),typeof(PostAttribute),typeof(PutAttribute), typeof(DeleteAttribute) )

In this case it force me to have all set. Any for me is like, if any of them exist, then it should act. But in my case I'm forced to have all of them set.

I want to use it as a generic verification. Not all clients will have all those attributes set. I tried Or too but same problem.

So what I want to do is: If any of those attributes exist then it shall act and check that the name end with Client.

If that's not how any works, then the naming is strange and the bug it more typo :)

jnormen avatar Sep 03 '24 08:09 jnormen

Hi @jnormen,

since the attributes from Refit are added to the methods of the interface, this has to be taken into account when writing the test. So this test would check, that any methods with any of these attributes are defined in interfaces ending with Api:

MethodMembers()
    .That()
    .HaveAnyAttributes(
        typeof(GetAttribute),
        typeof(PostAttribute),
        typeof(PutAttribute),
        typeof(DeleteAttribute)
    )            
    .And()
    .AreDeclaredIn(Interfaces())
    .Should()
    .BeDeclaredIn("^.*Api$", true)
    .Because("All interfaces with Refit attributes should end with 'Api'")
    .Check(Architecture);

~I could not reproduce the TypeDoesNotExistInArchitecture exception. In a simple example with one classlib and one test assembly this worked without any problems. We would need an example (zip file or public github repository) that reproduces the issue to further investigate this.~ -> We did now also find an example where we get the exception. We will need to have a closer look as to why this happens.

alexanderlinne avatar Sep 09 '24 16:09 alexanderlinne

I don't understand how you get this to work... I still get the same error. For me it does not act on ANY it act on all. If there is a Get no problem, if I add any and Post it gives me this error:

ArchUnitNET.Domain.Exceptions.TypeDoesNotExistInArchitecture: Type Refit.PostAttribute does not exist in provided architecture or is no attribute.

If I add a Post method in refit interface it complains that a Put does not exist.

ANY for me is if there is a get then act, if there is a post, then act, if there is a delete than act. If none of them exist complain. But the ANY still force me to add all of them not one of them.

For exmaple:

I have a Refit Interface with a get set when I run this code:

MethodMembers() .That() .HaveAnyAttributes( typeof(GetAttribute), typeof(PostAttribute), typeof(PutAttribute), typeof(DeleteAttribute) ) .And() .AreDeclaredIn(Interfaces()) .Should() .BeDeclaredIn("^.*Api$", true) .Because("All interfaces with Refit attributes should end with 'Api'") .Check(Architecture);

I get
ArchUnitNET.Domain.Exceptions.TypeDoesNotExistInArchitecture: Type Refit.PostAttribute does not exist in provided architecture or is no attribute.

If I also add a Post attribute on a method in my interface it complains that a Put does not exist. 

I expect a ANY is iF any of them exist then act...  If there is none of them = error. if one of them exist, then pass. 

jnormen avatar Sep 10 '24 08:09 jnormen

Since we do not have a full example it is hard to reproduce your exact issue on our side. Could you please run the attached sample project, for me this neither throws any exception nor does the test fail.

LibaryTest.zip

alexanderlinne avatar Sep 20 '24 16:09 alexanderlinne