Skip to content

Commit

Permalink
XWIKI-21880: Cannot delete user defined filters (#2881)
Browse files Browse the repository at this point in the history 
  * Refactor DefaultNotificationFilterPreferenceManager to never throw
    immediately an exception thrown by a provider, but to only do it if
all providers failed
  * Provide a new test covering this behaviour
  * Small test improvment
  * Improve a bit the logic to use standard exception if there's only
    one provider
  • Loading branch information
@surli
surli authored Feb 13, 2024
1 parent c595079 commit 103650d
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 90 deletions.
189 changes: 109 additions & 80 deletions .../org/xwiki/notifications/filters/internal/DefaultNotificationFilterPreferenceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
*/
package org.xwiki.notifications.filters.internal;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.inject.Inject;
Expand Down Expand Up @@ -56,55 +58,109 @@
@Singleton
public class DefaultNotificationFilterPreferenceManager implements NotificationFilterPreferenceManager
{
private static final String ENABLE_ERROR_MESSAGE = "Failed to enable or disabled the filter preference [{}].";

@Inject
private ComponentManager componentManager;

@Inject
private Logger logger;

@Override
public Collection<NotificationFilterPreference> getFilterPreferences(DocumentReference user)
throws NotificationException
@FunctionalInterface
interface ProviderCallable
{
Set<NotificationFilterPreference> filterPreferences = new HashSet<>();

try {
List<NotificationFilterPreferenceProvider> providers
= componentManager.getInstanceList(NotificationFilterPreferenceProvider.class);
void doInProvider(NotificationFilterPreferenceProvider provider) throws NotificationException;
}

for (NotificationFilterPreferenceProvider provider : providers) {
filterPreferences.addAll(provider.getFilterPreferences(user));
}
@FunctionalInterface
interface RetrieveWithProviderCallable<E>
{
Collection<E> retrieveWithProvider(NotificationFilterPreferenceProvider provider)
throws NotificationException;
}

return filterPreferences;
private List<NotificationFilterPreferenceProvider> getProviderList() throws NotificationException
{
try {
return componentManager.getInstanceList(NotificationFilterPreferenceProvider.class);
} catch (ComponentLookupException e) {
throw new NotificationException(
String.format("Unable to fetch a list of notification preference providers with user [%s].", user));
throw new NotificationException("Error when trying to load the list of providers", e);
}
}

@Override
public Collection<NotificationFilterPreference> getFilterPreferences(WikiReference wikiReference)
throws NotificationException
private String getProviderDebugMessage(String loggerMessage, NotificationFilterPreferenceProvider provider)
{
Set<NotificationFilterPreference> filterPreferences = new HashSet<>();
return String.format("%s with provider %s", loggerMessage, provider);
}

try {
List<NotificationFilterPreferenceProvider> providers
= componentManager.getInstanceList(NotificationFilterPreferenceProvider.class);
private String getExceptionMessage(String loggerMessage, List<NotificationException> exceptions)
{
return String.format("%s - All providers called failed, see exceptions: [%s].",
loggerMessage,
exceptions.stream().map(ExceptionUtils::getRootCauseMessage).collect(Collectors.joining(",")));
}

for (NotificationFilterPreferenceProvider provider : providers) {
filterPreferences.addAll(provider.getFilterPreferences(wikiReference));
private void providerExceptionWrapper(ProviderCallable callable, String loggerMessage) throws NotificationException
{
boolean allFailing = true;
List<NotificationException> exceptions = new ArrayList<>();
List<NotificationFilterPreferenceProvider> providerList = getProviderList();
if (providerList.size() > 1) {
for (NotificationFilterPreferenceProvider provider : providerList) {
try {
callable.doInProvider(provider);
allFailing = false;
} catch (NotificationException e) {
this.logger.debug(getProviderDebugMessage(loggerMessage, provider), e);
exceptions.add(e);
}
}
if (allFailing) {
throw new NotificationException(getExceptionMessage(loggerMessage, exceptions));
}
} else {
callable.doInProvider(providerList.get(0));
}
}

return filterPreferences;
} catch (ComponentLookupException e) {
throw new NotificationException(
String.format("Unable to fetch a list of notification preference providers with wiki [%s].",
wikiReference));
private <E> Collection<E> retrieveWithProviderExceptionWrapper(RetrieveWithProviderCallable<E> callable,
String loggerMessage) throws NotificationException
{
boolean allFailing = true;
List<NotificationException> exceptions = new ArrayList<>();
Set<E> result = new HashSet<>();
List<NotificationFilterPreferenceProvider> providerList = getProviderList();
if (providerList.size() > 1) {
for (NotificationFilterPreferenceProvider provider : getProviderList()) {
try {
result.addAll(callable.retrieveWithProvider(provider));
allFailing = false;
} catch (NotificationException e) {
this.logger.debug(getProviderDebugMessage(loggerMessage, provider), e);
exceptions.add(e);
}
}
if (allFailing) {
throw new NotificationException(getExceptionMessage(loggerMessage, exceptions));
}
} else {
result.addAll(callable.retrieveWithProvider(providerList.get(0)));
}
return result;
}

@Override
public Collection<NotificationFilterPreference> getFilterPreferences(DocumentReference user)
throws NotificationException
{
return this.retrieveWithProviderExceptionWrapper(provider -> provider.getFilterPreferences(user),
String.format("Error when trying to get filter preferences for user [%s]", user));
}

@Override
public Collection<NotificationFilterPreference> getFilterPreferences(WikiReference wikiReference)
throws NotificationException
{
return this.retrieveWithProviderExceptionWrapper(provider -> provider.getFilterPreferences(wikiReference),
String.format("Error when trying to get filter preferences for wiki [%s]", wikiReference));
}

@Override
Expand Down Expand Up @@ -177,79 +233,52 @@ public void deleteFilterPreference(DocumentReference user, String filterPreferen
public void deleteFilterPreferences(DocumentReference user, Set<String> filterPreferenceIds)
throws NotificationException
{
try {
for (NotificationFilterPreferenceProvider provider
: componentManager.<NotificationFilterPreferenceProvider>getInstanceList(
NotificationFilterPreferenceProvider.class)) {
provider.deleteFilterPreferences(user, filterPreferenceIds);
}
} catch (ComponentLookupException e) {
logger.info("Failed to remove the user filter preferences {}.", filterPreferenceIds, e);
}
this.providerExceptionWrapper(provider -> provider.deleteFilterPreferences(user, filterPreferenceIds),
String.format("Error when trying to remove filter preferences %s for user [%s]", filterPreferenceIds,
user));
}

@Override
public void deleteFilterPreference(WikiReference wikiReference, String filterPreferenceId)
throws NotificationException
{
try {
for (NotificationFilterPreferenceProvider provider
: componentManager.<NotificationFilterPreferenceProvider>getInstanceList(
NotificationFilterPreferenceProvider.class)) {
provider.deleteFilterPreference(wikiReference, filterPreferenceId);
}

} catch (ComponentLookupException e) {
logger.info("Failed to remove the wiki filter preference [{}].", filterPreferenceId, e);
}
this.providerExceptionWrapper(provider -> provider.deleteFilterPreference(wikiReference, filterPreferenceId),
String.format("Error when trying to remove filter preference [%s] for wiki [%s]", filterPreferenceId,
wikiReference));
}


@Override
public void setFilterPreferenceEnabled(DocumentReference user, String filterPreferenceId, boolean enabled)
throws NotificationException
{
try {
for (NotificationFilterPreferenceProvider provider
: componentManager.<NotificationFilterPreferenceProvider>getInstanceList(
NotificationFilterPreferenceProvider.class)) {
provider.setFilterPreferenceEnabled(user, filterPreferenceId, enabled);
}

} catch (ComponentLookupException e) {
logger.info(ENABLE_ERROR_MESSAGE, filterPreferenceId, e);
}
this.providerExceptionWrapper(provider ->
provider.setFilterPreferenceEnabled(user, filterPreferenceId, enabled),
String.format("Error when trying to set filter preference [%s] enabled to [%s] for user [%s]",
enabled,
filterPreferenceId,
user));
}

@Override
public void setFilterPreferenceEnabled(WikiReference wikiReference, String filterPreferenceId, boolean enabled)
throws NotificationException
{
try {
for (NotificationFilterPreferenceProvider provider
: componentManager.<NotificationFilterPreferenceProvider>getInstanceList(
NotificationFilterPreferenceProvider.class)) {
provider.setFilterPreferenceEnabled(wikiReference, filterPreferenceId, enabled);
}

} catch (ComponentLookupException e) {
logger.info(ENABLE_ERROR_MESSAGE, filterPreferenceId, e);
}
this.providerExceptionWrapper(provider ->
provider.setFilterPreferenceEnabled(wikiReference, filterPreferenceId, enabled),
String.format("Error when trying to set filter preference [%s] enabled to [%s] for wiki [%s]",
enabled,
filterPreferenceId,
wikiReference));
}

@Override
public void setStartDateForUser(DocumentReference user, Date startDate) throws NotificationException
{
try {
List<NotificationFilterPreferenceProvider> providers
= componentManager.getInstanceList(NotificationFilterPreferenceProvider.class);

for (NotificationFilterPreferenceProvider provider : providers) {
provider.setStartDateForUser(user, startDate);
}
} catch (ComponentLookupException e) {
throw new NotificationException(String.format("Unable to set the starting date for filter preferences"
+ " with user [%s].", user));
}
this.providerExceptionWrapper(provider ->
provider.setStartDateForUser(user, startDate),
String.format("Error when trying to set start date to [%s] for user [%s]",
startDate,
user));
}
}
52 changes: 42 additions & 10 deletions .../xwiki/notifications/filters/internal/DefaultNotificationFilterPreferenceManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,26 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.internal.util.collections.Sets;
import org.xwiki.component.manager.ComponentManager;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.WikiReference;
import org.xwiki.notifications.NotificationException;
import org.xwiki.notifications.filters.NotificationFilter;
import org.xwiki.notifications.filters.NotificationFilterPreference;
import org.xwiki.notifications.filters.NotificationFilterPreferenceProvider;
import org.xwiki.notifications.filters.NotificationFilterType;
import org.xwiki.test.annotation.BeforeComponent;
import org.xwiki.test.junit5.mockito.ComponentTest;
import org.xwiki.test.junit5.mockito.InjectComponentManager;
import org.xwiki.test.junit5.mockito.InjectMockComponents;
import org.xwiki.test.junit5.mockito.MockComponent;
import org.xwiki.test.mockito.MockitoComponentManager;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand All @@ -68,12 +70,6 @@ public class DefaultNotificationFilterPreferenceManagerTest
@InjectComponentManager
private MockitoComponentManager componentManager;

@BeforeComponent
void beforeComponent() throws Exception
{
this.componentManager.registerComponent(ComponentManager.class, this.componentManager);
}

@BeforeEach
public void setUp() throws Exception
{
Expand Down Expand Up @@ -154,6 +150,42 @@ void deleteFilterPreference() throws Exception
verify(testProvider).deleteFilterPreferences(testUser, Set.of("myFilter"));
}

@Test
void deleteFilterPreferenceProviderException() throws Exception
{
String providerName1 = "providerName1";
String providerName2 = "providerName2";

NotificationFilterPreferenceProvider provider1 =
componentManager.registerMockComponent(NotificationFilterPreferenceProvider.class, providerName1);
NotificationFilterPreferenceProvider provider2 =
componentManager.registerMockComponent(NotificationFilterPreferenceProvider.class, providerName2);

String filterId = "filterId";
doThrow(new NotificationException("error provider1")).when(provider1).deleteFilterPreferences(testUser,
Set.of(filterId));
filterPreferenceManager.deleteFilterPreference(testUser, filterId);
verify(testProvider).deleteFilterPreferences(testUser, Set.of(filterId));
verify(provider1).deleteFilterPreferences(testUser, Set.of(filterId));
verify(provider2).deleteFilterPreferences(testUser, Set.of(filterId));

doThrow(new NotificationException("error provider2")).when(provider2).deleteFilterPreferences(testUser,
Set.of(filterId));
filterPreferenceManager.deleteFilterPreference(testUser, filterId);
verify(testProvider, times(2)).deleteFilterPreferences(testUser, Set.of(filterId));
verify(provider1, times(2)).deleteFilterPreferences(testUser, Set.of(filterId));
verify(provider2, times(2)).deleteFilterPreferences(testUser, Set.of(filterId));

doThrow(new NotificationException("error testprovider")).when(testProvider).deleteFilterPreferences(testUser,
Set.of(filterId));
NotificationException notificationException = assertThrows(NotificationException.class,
() -> filterPreferenceManager.deleteFilterPreference(testUser, filterId));
assertEquals("Error when trying to remove filter preferences [filterId] for user [wiki:test.user] - "
+ "All providers called failed, see exceptions: [NotificationException: error testprovider,"
+ "NotificationException: error provider1,NotificationException: error provider2].",
notificationException.getMessage());
}

@Test
void setFilterPreferenceEnabled() throws Exception
{
Expand Down Expand Up @@ -199,9 +231,9 @@ void saveFilterPreferences() throws Exception
String providerName2 = "providerName2";

NotificationFilterPreferenceProvider provider1 =
this.componentManager.registerMockComponent(NotificationFilterPreferenceProvider.class, providerName1);
componentManager.registerMockComponent(NotificationFilterPreferenceProvider.class, providerName1);
NotificationFilterPreferenceProvider provider2 =
this.componentManager.registerMockComponent(NotificationFilterPreferenceProvider.class, providerName2);
componentManager.registerMockComponent(NotificationFilterPreferenceProvider.class, providerName2);

when(pref1.getProviderHint()).thenReturn(providerName1);
when(pref2.getProviderHint()).thenReturn(providerName2);
Expand Down

0 comments on commit 103650d

Please sign in to comment.