Hi everyone,
during the analysis of a fix for Loading... I realized that there was a design problem in xwiki-platform-notification-filters
modules: right now we have NotificationFilterPreferenceProvider
which is a public interface defining a component role, which should never have been public IMO and which is even completely useless in 16.x.
So to give a bit of history: notification filter preferences used to be stored in WatchlistClass xobjects, and then this mechanism has been changed to store the filter preferences in DB. However during a long time (from 10.11.9 to 16.0.0 according to Loading...) there was no migration provided for WatchlistClass xobjects, and instead there was a bridge for the API to retrieve the info either in DB or from xobjects. So this NotificationFilterPreferenceProvider
was the common interface used by both the deprecated bridge and the new API that retrieve preferences from DB.
I’ve no idea why it’s been made public, but for me this was a mistake: first, I don’t see a reason why an extension would want a custom storage of the filter preferences, and second right now in DefaultNotificationFilterPreferenceManager
we were calling each time all providers to perform the various API calls, which actually led to Loading.... With a new implementation of a provider we would have hit this bug probably way before.
Now, besides being public, this component role is now even completely useless: I did performed the migration of the watchlistclass xobjects as part of Loading..., so in XWiki Standard we now have a single implementation of this interface. And removing it simplify a lot the code as it removes an indirection and make all calls of the API far more easy to understand.
Note that I did perform a check in xwiki-contrib and there’s 0 hit for NotificationFilterPreferenceProvider, also it’s never been documented in our Notification API extension page.
Now I opened a draft PR containing the actual removal of this class with all changes, just to see what it would impact (built with skipping revapi and jacoco): XWIKI-22211: Deprecate NotificationFilterPreferenceProvider by surli · Pull Request #2882 · xwiki/xwiki-platform · GitHub
So I’m opening a vote for actually breaking the API to entirely remove NotificationFilterPreferenceProvider in 16.x. This vote is opened for 2 weeks, until the 27th of february.
Here’s my +1.
To be perfectly clear: it’s completely possible to deprecate only the API and keep it in our code, but I do think it’s maintaining something for no value as I strongly believe it’s never meant to be public.