Bug?: MSTeams notifier inconsistent shouldNotify implementation

[cdprete]

Hello guys.

I'm starting to integrate notifiers in our instance of SBA and, apart some fights over the application context regarding the registration time of the beans (composition/delegation/decorator doesn't really work currently due to order of creation of the beans), I realized that some notifiers are inconsistent in terms of when they should trigger a notification.

In particular, the MicrosoftTeamsNotifier is extending AbstractStatusChangeNotifier but then it's ignoring the fact (by overriding shouldNotify) that it should be dealing only with status updates and it's handling all of them.

Is this intentional? Why not extending AbstractEventNotifier instead then?
From the other end, assuming that's correct and that it should handle all the events, why MailNotifier doesn't do the same then and it deals, instead, only with status changes?

Finally, https://github.com/codecentric/spring-boot-admin/blob/master/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/notify/AbstractEventNotifier.java#L34 should be protected in my opinion.
At the moment, I'm forced to store it once more in a field to have access to it even if it's already available in the parent class.

[cdprete]

Regarding the implementation of a decorator, it seems like it doesn't work also because some methods are too restricted/not visible.
In particular, shouldNotify is protected, so something like:

public class MyNotifierDecorator extends AbstractEventNotifier {
    private final AbstractEventNotifier delegate; // passed by constructor

   protected boolean shouldNotify() { return delegate.shouldNotify(); } // NOK, it is protected and not accessible here

   protected Mono<Void> doNotify(...) { 
       // do something and then
       return delegate.doNotify(...); // NOK, it is protected and not accessible here
    }
}

In my case I'm trying to implement a delay-based notifier that sends the notification only if the instance is still not OK after 5 minutes.
But, if I want to cover the MailNotifier as well as the MicrosoftTeamsNotifier, I need to implement the same logic twice because the only way I've to decorate those is to extend them.
But, as we all know, multiple inheritance is not supported in Java, so something like class MyNotifier extends MailNotifier, DelayedNotifier is not possible.

[hzpz]

Besides status events, MicrosoftTeamsNotifier does also notify registered and deregistered events. That has always been the case since its inception (many, many years ago), probably because there was a requirement to do so. It extends AbstractStatusChangeNotifier to reuse the status change handling logic instead of duplicating it. This is not a bug.

To implement your decorator you could use the approach from this example using CompositeNotifier in combination with FilteringNotifier.

[cdprete]

Besides status events, MicrosoftTeamsNotifier does also notify registered and deregistered events. That has always been the case since its inception (many, many years ago), probably because there was a requirement to do so. It extends AbstractStatusChangeNotifier to reuse the status change handling logic instead of duplicating it. This is not a bug.

To implement your decorator you could use the approach from this example using CompositeNotifier in combination with FilteringNotifier.

This is not a bug.

Looks a breaking in the contract of that class to me, but if it's intentional so be it.

To implement your decorator you could use the approach from this example using CompositeNotifier in combination with FilteringNotifier.

Not really. CompositeNotifier is calling all the notifiers while I just need to decorate one (I need to delay its execution).