From b9291e2b43c86be30d099305f3ed1502ce2e5d5b Mon Sep 17 00:00:00 2001 From: Gary Roybal Date: Tue, 11 Jun 2024 10:58:59 -0700 Subject: [PATCH] fix/issue-#2800: Fix bug with favorites getting deleted when a favorited portlet gets deleted and a user subequently favorites another portlet or unfavorites a portlet (#2801) --- .../portlets/favorites/FavoritesUtils.java | 4 + .../remoting/UpdatePreferencesServlet.java | 104 ++++-- .../UpdatePreferencesServletTest.java | 302 +++++++++++++++++- .../node/UserLayoutChannelDescription.java | 11 +- .../layout/dlm/MissingPortletDefinition.java | 4 +- 5 files changed, 380 insertions(+), 45 deletions(-) diff --git a/uPortal-api/uPortal-api-internal/src/main/java/org/apereo/portal/portlets/favorites/FavoritesUtils.java b/uPortal-api/uPortal-api-internal/src/main/java/org/apereo/portal/portlets/favorites/FavoritesUtils.java index 83e20c40166..7c30e15308d 100644 --- a/uPortal-api/uPortal-api-internal/src/main/java/org/apereo/portal/portlets/favorites/FavoritesUtils.java +++ b/uPortal-api/uPortal-api-internal/src/main/java/org/apereo/portal/portlets/favorites/FavoritesUtils.java @@ -54,6 +54,10 @@ public class FavoritesUtils { private Logger logger = LoggerFactory.getLogger(getClass()); + public void setPortletDefinitionRegistry(IPortletDefinitionRegistry registry) { + this.portletRegistry = registry; + } + /** * Get the favorite collections of portlets (i.e. suitable folders ("tabs") in the user layout.) * Suitable layout nodes are of type folder with @type attribute favorite_collection. diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/layout/dlm/remoting/UpdatePreferencesServlet.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/layout/dlm/remoting/UpdatePreferencesServlet.java index 24b2d971ebf..1d6ef923665 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/layout/dlm/remoting/UpdatePreferencesServlet.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/layout/dlm/remoting/UpdatePreferencesServlet.java @@ -39,6 +39,7 @@ import org.apereo.portal.layout.IUserLayoutManager; import org.apereo.portal.layout.IUserLayoutStore; import org.apereo.portal.layout.PortletSubscribeIdResolver; +import org.apereo.portal.layout.dlm.MissingPortletDefinition; import org.apereo.portal.layout.dlm.UserPrefsHandler; import org.apereo.portal.layout.node.IUserLayoutChannelDescription; import org.apereo.portal.layout.node.IUserLayoutFolderDescription; @@ -51,13 +52,14 @@ import org.apereo.portal.portlet.registry.IPortletWindowRegistry; import org.apereo.portal.portlets.favorites.FavoritesUtils; import org.apereo.portal.security.IAuthorizationPrincipal; +import org.apereo.portal.security.IAuthorizationService; import org.apereo.portal.security.IPermission; import org.apereo.portal.security.IPerson; import org.apereo.portal.security.PermissionHelper; -import org.apereo.portal.services.AuthorizationServiceFacade; import org.apereo.portal.services.GroupService; import org.apereo.portal.user.IUserInstance; import org.apereo.portal.user.IUserInstanceManager; +import org.apereo.portal.utils.personalize.IPersonalizer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -86,6 +88,7 @@ public class UpdatePreferencesServlet { protected final Logger logger = LoggerFactory.getLogger(getClass()); + private IAuthorizationService authorizationService; private IPortletDefinitionRegistry portletDefinitionRegistry; private IUserIdentityStore userIdentityStore; private IUserInstanceManager userInstanceManager; @@ -94,6 +97,7 @@ public class UpdatePreferencesServlet { private MessageSource messageSource; private IPortletWindowRegistry portletWindowRegistry; private FavoritesUtils favoritesUtils; + private IPersonalizer personalizer; @Value("${org.apereo.portal.layout.dlm.remoting.addedWindowState:null}") private String addedPortletWindowState; @@ -109,6 +113,11 @@ private void initAddedPortletWindowState() { } } + @Autowired + public void setAuthorizationService(IAuthorizationService authorizationService) { + this.authorizationService = authorizationService; + } + @Autowired public void setUserLayoutStore(IUserLayoutStore userLayoutStore) { this.userLayoutStore = userLayoutStore; @@ -150,6 +159,11 @@ public void setFavoritesUtils(FavoritesUtils favoritesUtils) { this.favoritesUtils = favoritesUtils; } + @Autowired + public void setPersonalizer(IPersonalizer personalizer) { + this.personalizer = personalizer; + } + /** Default name given to newly created tabs. */ protected static final String DEFAULT_TAB_NAME = "New Tab"; // TODO: Requires i18n! @@ -606,13 +620,15 @@ public ModelAndView addFavorite( final IUserLayoutManager ulm = upm.getUserLayoutManager(); final IUserLayoutChannelDescription channel = - new UserLayoutChannelDescription(person, pdef, request.getSession()); + new UserLayoutChannelDescription(person, pdef, request.getSession(), personalizer); // get favorite tab final String favoriteTabNodeId = favoritesUtils.getFavoriteTabNodeId(ulm.getUserLayout()); if (favoriteTabNodeId != null) { - // add portlet to favorite tab + removeOrphanedFavoritesFromLayout(ulm); + + // add new favorite portlet to favorite tab final IUserLayoutNodeDescription node = addNodeToTab(ulm, channel, favoriteTabNodeId); if (node == null) { @@ -696,12 +712,13 @@ public ModelAndView removeFavorite( String resp = new String(); + removeOrphanedFavoritesFromLayout(ulm, favoritePortlets); + + boolean saveNeeded = false; for (IUserLayoutChannelDescription deleteNode : favoritesToDelete) { - if (deleteNode != null && deleteNode instanceof UserLayoutChannelDescription) { - UserLayoutChannelDescription channelDescription = - (UserLayoutChannelDescription) deleteNode; + if (deleteNode != null) { try { - if (!ulm.deleteNode(channelDescription.getChannelSubscribeId())) { + if (!ulm.deleteNode(deleteNode.getChannelSubscribeId())) { logger.warn( "Error deleting the node {} from favorites for user {}", @@ -718,27 +735,30 @@ public ModelAndView removeFavorite( "Can''t remove favorite", locale); } else { + saveNeeded = true; // load success message - - resp = - getMessage( - "success.remove.portlet", - "Removed from Favorites successfully", - locale); } - // save the user's layout + } catch (PortalException e) { + return handlePersistError(request, response, e); + } + } + if (saveNeeded) { + try { ulm.saveUserLayout(); } catch (PortalException e) { return handlePersistError(request, response, e); } + resp = + getMessage( + "success.remove.portlet", + "Removed from Favorites successfully", + locale); } } return new ModelAndView("jsonView", Collections.singletonMap("response", resp)); } - // save the user's layout - ulm.saveUserLayout(); return new ModelAndView( "jsonView", Collections.singletonMap( @@ -789,7 +809,7 @@ else if (fname != null) IUserLayoutChannelDescription channel = new UserLayoutChannelDescription( - getPerson(ui, response), definition, request.getSession()); + getPerson(ui, response), definition, request.getSession(), personalizer); IUserLayoutNodeDescription node; if (isTab(ulm, destinationId)) { @@ -1453,13 +1473,12 @@ protected String getMessage(String key, String argument, String defaultMessage, } private IAuthorizationPrincipal getUserPrincipal(final String userName) { - final IEntity user = GroupService.getEntity(userName, IPerson.class); + final IEntity user = this.personEntityService.getPersonEntity(userName); if (user == null) { return null; } - final AuthorizationServiceFacade authService = AuthorizationServiceFacade.instance(); - return authService.newPrincipal(user); + return authorizationService.newPrincipal(user); } private String getTabIdFromName(IUserLayout userLayout, String tabName) { @@ -1595,4 +1614,49 @@ private boolean attemptNodeMove( private boolean isFolder(IUserLayoutManager ulm, String id) { return ulm.getNode(id).getType().equals(IUserLayoutNodeDescription.LayoutNodeType.FOLDER); } + + private void removeOrphanedFavoritesFromLayout(IUserLayoutManager ulm) { + List favoritesPortlets = + favoritesUtils.getFavoritePortletLayoutNodes(ulm.getUserLayout()); + removeOrphanedFavoritesFromLayout(ulm, favoritesPortlets); + } + + private void removeOrphanedFavoritesFromLayout( + IUserLayoutManager ulm, List favoritesPortlets) { + // remove orphaned favorites (meaning they are favorites for portlets that have been + // removed/deleted) ...keep in mind that ulm.saveLayout() will need to be called + // in order to actually save the changes to the layout + for (IUserLayoutNodeDescription node : favoritesPortlets) { + if (node instanceof IUserLayoutChannelDescription) { + IUserLayoutChannelDescription channelDescription = + (IUserLayoutChannelDescription) node; + if (channelDescription + .getChannelPublishId() + .equals(MissingPortletDefinition.CHANNEL_ID)) { + ulm.deleteNode(channelDescription.getChannelSubscribeId()); + } + } + } + } + + // for unit testing begin + + public interface IPersonEntityService { + IEntity getPersonEntity(String userName); + } + + public class GroupServiceBasedPersonEntityService implements IPersonEntityService { + @Override + public IEntity getPersonEntity(String userName) { + return GroupService.getEntity(userName, IPerson.class); + } + } + + private IPersonEntityService personEntityService = new GroupServiceBasedPersonEntityService(); + + public void setPersonEntityService(IPersonEntityService personEntityService) { + this.personEntityService = personEntityService; + } + + // for unit testing end } diff --git a/uPortal-api/uPortal-api-rest/src/test/java/org/apereo/portal/layout/dlm/remoting/UpdatePreferencesServletTest.java b/uPortal-api/uPortal-api-rest/src/test/java/org/apereo/portal/layout/dlm/remoting/UpdatePreferencesServletTest.java index 934723da9d8..30a4a5ebd2c 100644 --- a/uPortal-api/uPortal-api-rest/src/test/java/org/apereo/portal/layout/dlm/remoting/UpdatePreferencesServletTest.java +++ b/uPortal-api/uPortal-api-rest/src/test/java/org/apereo/portal/layout/dlm/remoting/UpdatePreferencesServletTest.java @@ -14,25 +14,52 @@ */ package org.apereo.portal.layout.dlm.remoting; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import java.io.IOException; +import java.util.Arrays; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Vector; import javax.portlet.WindowState; import org.apereo.portal.IUserIdentityStore; import org.apereo.portal.UserInstance; +import org.apereo.portal.UserPreferencesManager; +import org.apereo.portal.groups.IEntity; import org.apereo.portal.layout.IStylesheetUserPreferencesService; +import org.apereo.portal.layout.IUserLayout; +import org.apereo.portal.layout.IUserLayoutManager; import org.apereo.portal.layout.IUserLayoutStore; +import org.apereo.portal.layout.dlm.MissingPortletDefinition; +import org.apereo.portal.layout.node.IUserLayoutChannelDescription; +import org.apereo.portal.layout.node.IUserLayoutFolderDescription; +import org.apereo.portal.layout.node.IUserLayoutNodeDescription; +import org.apereo.portal.portlet.om.IPortletDefinition; +import org.apereo.portal.portlet.om.IPortletDefinitionId; +import org.apereo.portal.portlet.om.IPortletType; import org.apereo.portal.portlet.registry.IPortletDefinitionRegistry; import org.apereo.portal.portlet.registry.IPortletWindowRegistry; +import org.apereo.portal.portlet.registry.PortletDefinitionRegistryImpl; import org.apereo.portal.portlets.favorites.FavoritesUtils; +import org.apereo.portal.security.IAuthorizationPrincipal; +import org.apereo.portal.security.IAuthorizationService; +import org.apereo.portal.security.IPermission; import org.apereo.portal.security.IPerson; import org.apereo.portal.security.ISecurityContext; import org.apereo.portal.security.provider.PersonImpl; import org.apereo.portal.user.IUserInstance; import org.apereo.portal.user.IUserInstanceManager; +import org.apereo.portal.utils.personalize.IPersonalizer; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,6 +70,7 @@ public class UpdatePreferencesServletTest { @InjectMocks UpdatePreferencesServlet updatePreferencesServlet; + @Mock private IAuthorizationService authorizationService; @Mock private IPortletDefinitionRegistry portletDefinitionRegistry; @Mock private IUserIdentityStore userIdentityStore; @Mock private IUserInstanceManager userInstanceManager; @@ -51,29 +79,154 @@ public class UpdatePreferencesServletTest { @Mock private MessageSource messageSource; @Mock private IPortletWindowRegistry portletWindowRegistry; @Mock private WindowState addedWindowState; - @Mock private FavoritesUtils favoritesUtils; + @Mock private IPersonalizer personalizer; @Mock protected final Logger logger = LoggerFactory.getLogger(getClass()); @Mock protected ISecurityContext m_securityContext = null; + @Mock IPerson person; + @Mock IEntity personEntity; + @Mock IUserInstance userInstance; + @Mock IPortletType portletType; + @Mock IPortletDefinitionId portletDefinitionId; + @Mock IPortletDefinition portletDefForPortletBeingFavorited; + @Mock IPortletDefinition portletDefForFavoritedPortlet1; + @Mock IPortletDefinition portletDefForFavoritedPortlet2; + @Mock IPortletDefinition portletDefForFavoritedPortlet3; + @Mock IAuthorizationPrincipal authPrincipal; + @Mock UserPreferencesManager upm; + @Mock IUserLayoutManager ulm; + @Mock IUserLayout userLayout; + @Mock IUserLayoutFolderDescription favoritesFolderNodeDescription; + @Mock IUserLayoutChannelDescription favoritedPortlet1ChannelDescription; + @Mock IUserLayoutChannelDescription favoritedPortlet2ChannelDescription; + @Mock IUserLayoutChannelDescription favoritedPortlet3ChannelDescription; + @Mock IUserLayoutChannelDescription newFavoritedPortletChannelDescription; + @Mock IUserLayoutChannelDescription orphanedFavoritedPortletChannelDescription; + + private AutoCloseable closeable; private MockHttpServletRequest req; private MockHttpServletResponse res; + private final String favoritedPortlet1Id = "111"; + private final String favoritedPortlet2Id = "222"; + private final String favoritedPortlet3Id = "333"; + private final String favoritedPortlet1Fname = "fname-" + favoritedPortlet1Id; + private final String favoritedPortlet2Fname = "fname-" + favoritedPortlet2Id; + private final String favoritedPortlet3Fname = "fname-" + favoritedPortlet3Id; + private final String getFavoritedPortlet1SubscribeId = "1111"; + private final String getFavoritedPortlet2SubscribeId = "2222"; + private final String getFavoritedPortlet3SubscribeId = "3333"; + private final String orphanedFavoritedPortletSubscribeId = "123"; + private final String favoritesFolderNodeId = "7"; + private final String favoritesFolderColumnNodeId = "77"; + private final String idForPortletBeingFavorited = "777"; + private final String userLayoutRootId = "1"; + private FavoritesUtils favoritesUtils; @Before public void setup() { updatePreferencesServlet = new UpdatePreferencesServlet(); res = new MockHttpServletResponse(); req = new MockHttpServletRequest(); - MockitoAnnotations.initMocks(this); + closeable = MockitoAnnotations.openMocks(this); + favoritesUtils = new FavoritesUtils(); + favoritesUtils.setPortletDefinitionRegistry(portletDefinitionRegistry); + updatePreferencesServlet.setFavoritesUtils(favoritesUtils); + when(favoritesFolderNodeDescription.getName()).thenReturn("Favorites"); + when(favoritesFolderNodeDescription.getType()) + .thenReturn(IUserLayoutNodeDescription.LayoutNodeType.FOLDER); + when(((IUserLayoutFolderDescription) favoritesFolderNodeDescription).getFolderType()) + .thenReturn("favorites"); + when(favoritesFolderNodeDescription.getId()).thenReturn(favoritesFolderNodeId); + when(userLayout.getNodeDescription(favoritesFolderNodeId)) + .thenReturn(favoritesFolderNodeDescription); + when(userInstanceManager.getUserInstance(req)).thenReturn(userInstance); + when(userInstance.getPerson()).thenReturn(person); + when(portletDefinitionId.getStringId()).thenReturn(idForPortletBeingFavorited); + when(portletDefForPortletBeingFavorited.getPortletDefinitionId()) + .thenReturn(portletDefinitionId); + when(portletDefinitionRegistry.getPortletDefinition(idForPortletBeingFavorited)) + .thenReturn(portletDefForPortletBeingFavorited); + when(portletDefForPortletBeingFavorited.getType()).thenReturn(portletType); + when(authorizationService.newPrincipal(any())).thenReturn(authPrincipal); + when(authPrincipal.hasPermission( + eq(IPermission.PORTAL_SYSTEM), + eq(IPermission.PORTLET_FAVORITE_ACTIVITY), + any())) + .thenReturn(true); + when(userInstance.getPreferencesManager()).thenReturn(upm); + when(upm.getUserLayoutManager()).thenReturn(ulm); + when(ulm.getUserLayout()).thenReturn(userLayout); + when(userLayout.getRootId()).thenReturn(userLayoutRootId); + when(favoritedPortlet1ChannelDescription.getChannelPublishId()) + .thenReturn(favoritedPortlet1Id); + when(favoritedPortlet2ChannelDescription.getChannelPublishId()) + .thenReturn(favoritedPortlet2Id); + when(favoritedPortlet3ChannelDescription.getChannelPublishId()) + .thenReturn(favoritedPortlet3Id); + when(orphanedFavoritedPortletChannelDescription.getChannelPublishId()) + .thenReturn(MissingPortletDefinition.CHANNEL_ID); + when(favoritedPortlet1ChannelDescription.getFunctionalName()) + .thenReturn(favoritedPortlet1Fname); + when(favoritedPortlet2ChannelDescription.getFunctionalName()) + .thenReturn(favoritedPortlet2Fname); + when(favoritedPortlet3ChannelDescription.getFunctionalName()) + .thenReturn(favoritedPortlet3Fname); + when(orphanedFavoritedPortletChannelDescription.getFunctionalName()) + .thenReturn(MissingPortletDefinition.FNAME); + when(favoritedPortlet1ChannelDescription.getChannelSubscribeId()) + .thenReturn(getFavoritedPortlet1SubscribeId); + when(favoritedPortlet2ChannelDescription.getChannelSubscribeId()) + .thenReturn(getFavoritedPortlet2SubscribeId); + when(favoritedPortlet3ChannelDescription.getChannelSubscribeId()) + .thenReturn(getFavoritedPortlet3SubscribeId); + when(orphanedFavoritedPortletChannelDescription.getChannelSubscribeId()) + .thenReturn(orphanedFavoritedPortletSubscribeId); + when(portletDefinitionRegistry.getPortletDefinition(favoritedPortlet1Id)) + .thenReturn(portletDefForFavoritedPortlet1); + when(portletDefinitionRegistry.getPortletDefinition(favoritedPortlet2Id)) + .thenReturn(portletDefForFavoritedPortlet2); + when(portletDefinitionRegistry.getPortletDefinition(favoritedPortlet3Id)) + .thenReturn(portletDefForFavoritedPortlet3); + when(portletDefForFavoritedPortlet1.getType()).thenReturn(portletType); + when(portletDefForFavoritedPortlet2.getType()).thenReturn(portletType); + when(portletDefForFavoritedPortlet3.getType()).thenReturn(portletType); + when(portletDefForFavoritedPortlet1.getPortletDefinitionId()) + .thenReturn(portletDefinitionId); + when(portletDefForFavoritedPortlet2.getPortletDefinitionId()) + .thenReturn(portletDefinitionId); + when(portletDefForFavoritedPortlet3.getPortletDefinitionId()) + .thenReturn(portletDefinitionId); + when(portletDefForFavoritedPortlet1.getFName()).thenReturn(favoritedPortlet1Fname); + when(portletDefForFavoritedPortlet2.getFName()).thenReturn(favoritedPortlet2Fname); + when(portletDefForFavoritedPortlet3.getFName()).thenReturn(favoritedPortlet3Fname); + when(userLayout.getNodeDescription(favoritedPortlet1Id)) + .thenReturn(favoritedPortlet1ChannelDescription); + when(userLayout.getNodeDescription(favoritedPortlet2Id)) + .thenReturn(favoritedPortlet2ChannelDescription); + when(userLayout.getNodeDescription(favoritedPortlet3Id)) + .thenReturn(favoritedPortlet3ChannelDescription); + when(userLayout.getNodeDescription(MissingPortletDefinition.CHANNEL_ID)) + .thenReturn(orphanedFavoritedPortletChannelDescription); + when(ulm.addNode(any(), eq(favoritesFolderColumnNodeId), eq((String) null))) + .thenReturn(newFavoritedPortletChannelDescription); + when(ulm.deleteNode(any())).thenReturn(true); + when(newFavoritedPortletChannelDescription.getId()).thenReturn(idForPortletBeingFavorited); } @Test(expected = NullPointerException.class) public void testRemoveElement() throws IOException { - Mockito.when(userInstanceManager.getUserInstance(req)).thenReturn(null); - ModelAndView modelAndView = updatePreferencesServlet.removeElement(req, res); + when(userInstanceManager.getUserInstance(req)).thenReturn(null); + updatePreferencesServlet.removeElement(req, res); + } + + @After + public void tearDown() throws Exception { + closeable.close(); } @Test(expected = NullPointerException.class) - public void testRemoveByFNameNull() throws IOException { - ModelAndView modelAndView = updatePreferencesServlet.removeByFName(req, res, "fname"); + public void testRemoveByFNameNullForUserInstanceNotFound() throws IOException { + when(userInstanceManager.getUserInstance(req)).thenReturn(null); + updatePreferencesServlet.removeByFName(req, res, "fname"); } @Test(expected = NullPointerException.class) @@ -83,24 +236,80 @@ public void testRemoveByFName() throws IOException { person.setFullName("john doe"); IUserInstance userInstance = new UserInstance(person, null, null); - Mockito.when(userInstanceManager.getUserInstance(req)).thenReturn(userInstance); - ModelAndView modelAndView = updatePreferencesServlet.removeByFName(req, res, "fname"); + when(userInstanceManager.getUserInstance(req)).thenReturn(userInstance); + updatePreferencesServlet.removeByFName(req, res, "fname"); } @Test(expected = NullPointerException.class) public void testMovePortletAjax() { req.setLocalName("en-US"); - ModelAndView modelAndView = - updatePreferencesServlet.movePortletAjax( - req, res, "sourceId", "prevNodeIs", "nextNodeId"); + updatePreferencesServlet.movePortletAjax(req, res, "sourceId", "prevNodeIs", "nextNodeId"); } @Test(expected = NullPointerException.class) public void testMoveElement() throws IOException { req.setLocalName("en-US"); - Mockito.when(userInstanceManager.getUserInstance(req)).thenReturn(null); - ModelAndView modelAndView = - updatePreferencesServlet.moveElement(req, res, "sourceId", "get", "elementId"); + when(userInstanceManager.getUserInstance(req)).thenReturn(null); + updatePreferencesServlet.moveElement(req, res, "sourceId", "get", "elementId"); + } + + @Test(expected = IllegalArgumentException.class) + public void testAddFavoriteWithNullPortletId() throws IOException { + updatePreferencesServlet.setPortletDefinitionRegistry(new PortletDefinitionRegistryImpl()); + updatePreferencesServlet.setPersonEntityService( + this.createPersonEntityService(personEntity)); + updatePreferencesServlet.addFavorite(null, req, res); + } + + @Test + public void testAddFavoriteSucceeds() throws IOException { + req.setLocalName("en-US"); + updatePreferencesServlet.setPersonEntityService( + this.createPersonEntityService(personEntity)); + whenFavoritesFolderColumnNodeIsPresent(); + whenFavoritedPortletsArePresent( + favoritedPortlet1Id, favoritedPortlet2Id, favoritedPortlet3Id); + ModelAndView mav = + updatePreferencesServlet.addFavorite(idForPortletBeingFavorited, req, res); + assertNotNull(mav); + assertEquals("jsonView", mav.getViewName()); + assertEquals(idForPortletBeingFavorited, mav.getModel().get("newNodeId")); + verify(ulm).addNode(any(), eq(favoritesFolderColumnNodeId), eq((String) null)); + verify(ulm).saveUserLayout(); + } + + @Test + public void testAddFavoriteRemovesFavoriteForPortletThatHasBeenDeleted() throws IOException { + req.setLocalName("en-US"); + updatePreferencesServlet.setPersonEntityService( + this.createPersonEntityService(personEntity)); + whenFavoritesFolderColumnNodeIsPresent(); + whenFavoritedPortletsArePresent( + favoritedPortlet1Id, MissingPortletDefinition.CHANNEL_ID, favoritedPortlet3Id); + ModelAndView mav = + updatePreferencesServlet.addFavorite(idForPortletBeingFavorited, req, res); + assertNotNull(mav); + assertEquals("jsonView", mav.getViewName()); + assertEquals(idForPortletBeingFavorited, mav.getModel().get("newNodeId")); + verify(ulm).deleteNode(orphanedFavoritedPortletSubscribeId); + verify(ulm).addNode(any(), eq(favoritesFolderColumnNodeId), eq((String) null)); + verify(ulm).saveUserLayout(); + } + + @Test + public void testRemoveFavoriteSucceeds() throws IOException { + req.setLocalName("en-US"); + updatePreferencesServlet.setPersonEntityService( + this.createPersonEntityService(personEntity)); + whenFavoritesFolderColumnNodeIsPresent(); + whenFavoritedPortletsArePresent( + favoritedPortlet1Id, favoritedPortlet2Id, favoritedPortlet3Id); + ModelAndView mav = updatePreferencesServlet.removeFavorite(favoritedPortlet2Id, req, res); + assertNotNull(mav); + assertEquals("jsonView", mav.getViewName()); + verify(ulm).deleteNode(getFavoritedPortlet2SubscribeId); + verify(ulm).saveUserLayout(); + verify(messageSource).getMessage(eq("success.remove.portlet"), any(), any(), any()); } @Test(expected = NullPointerException.class) @@ -110,7 +319,66 @@ public void testRemoveFavorite() throws IOException { person.setUserName("jdoe"); person.setFullName("john doe"); IUserInstance userInstance = new UserInstance(person, null, null); - Mockito.when(userInstanceManager.getUserInstance(req)).thenReturn(userInstance); - ModelAndView modelAndView = updatePreferencesServlet.removeFavorite("channelId", req, res); + when(userInstanceManager.getUserInstance(req)).thenReturn(userInstance); + updatePreferencesServlet.removeFavorite("channelId", req, res); + } + + @Test(expected = IllegalArgumentException.class) + public void testRemoveFavoriteWithNullPortletId() throws IOException { + updatePreferencesServlet.setPortletDefinitionRegistry(new PortletDefinitionRegistryImpl()); + updatePreferencesServlet.setPersonEntityService( + this.createPersonEntityService(personEntity)); + updatePreferencesServlet.removeFavorite(null, req, res); + } + + @Test + public void testRemoveFavoriteRemovesFavoriteForPortletThatHasBeenDeleted() throws IOException { + req.setLocalName("en-US"); + updatePreferencesServlet.setPersonEntityService( + this.createPersonEntityService(personEntity)); + whenFavoritesFolderColumnNodeIsPresent(); + whenFavoritedPortletsArePresent( + favoritedPortlet1Id, MissingPortletDefinition.CHANNEL_ID, favoritedPortlet3Id); + ModelAndView mav = updatePreferencesServlet.removeFavorite(favoritedPortlet3Id, req, res); + assertNotNull(mav); + assertEquals("jsonView", mav.getViewName()); + verify(ulm).deleteNode(orphanedFavoritedPortletSubscribeId); + verify(ulm).deleteNode(getFavoritedPortlet3SubscribeId); + verify(ulm).saveUserLayout(); + verify(messageSource).getMessage(eq("success.remove.portlet"), any(), any(), any()); + } + + private UpdatePreferencesServlet.IPersonEntityService createPersonEntityService( + IEntity personEntity) { + return new UpdatePreferencesServlet.IPersonEntityService() { + @Override + public IEntity getPersonEntity(String username) { + return personEntity; + } + }; + } + + private Enumeration createNodeIdsEnumeration(String... nodeIds) { + return new Vector(new HashSet(Arrays.asList(nodeIds))).elements(); + } + + private void whenFavoritesFolderColumnNodeIsPresent() { + when(userLayout.getChildIds(userLayoutRootId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderNodeId)); + when(userLayout.getChildIds(favoritesFolderNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderColumnNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderColumnNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderColumnNodeId)); + when(ulm.getChildIds(favoritesFolderNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderColumnNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderColumnNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritesFolderColumnNodeId)); + } + + private void whenFavoritedPortletsArePresent(String... favoritedPortletIds) { + when(userLayout.getChildIds(favoritesFolderColumnNodeId)) + .thenReturn(this.createNodeIdsEnumeration(favoritedPortletIds)); } } diff --git a/uPortal-layout/uPortal-layout-core/src/main/java/org/apereo/portal/layout/node/UserLayoutChannelDescription.java b/uPortal-layout/uPortal-layout-core/src/main/java/org/apereo/portal/layout/node/UserLayoutChannelDescription.java index 0b384ebced7..0640b87d42b 100644 --- a/uPortal-layout/uPortal-layout-core/src/main/java/org/apereo/portal/layout/node/UserLayoutChannelDescription.java +++ b/uPortal-layout/uPortal-layout-core/src/main/java/org/apereo/portal/layout/node/UserLayoutChannelDescription.java @@ -25,7 +25,6 @@ import org.apereo.portal.portlet.om.IPortletDefinition; import org.apereo.portal.portlet.om.IPortletDefinitionParameter; import org.apereo.portal.security.IPerson; -import org.apereo.portal.spring.locator.ApplicationContextLocator; import org.apereo.portal.utils.personalize.IPersonalizer; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -48,7 +47,6 @@ public class UserLayoutChannelDescription extends UserLayoutNodeDescription private boolean hasHelp = false; private boolean hasAbout = false; private boolean isSecure = false; - private IPersonalizer personalizer; public UserLayoutChannelDescription() { super(); @@ -60,12 +58,13 @@ public UserLayoutChannelDescription() { * @param person personalization details * @param definition * @param session lightweight caching + * @param personalizer */ public UserLayoutChannelDescription( - IPerson person, IPortletDefinition definition, HttpSession session) { - this.personalizer = - ApplicationContextLocator.getApplicationContext().getBean(IPersonalizer.class); - + IPerson person, + IPortletDefinition definition, + HttpSession session, + IPersonalizer personalizer) { this.title = personalizer.personalize(person, definition.getTitle(), session); this.name = definition.getName(); this.description = personalizer.personalize(person, definition.getDescription(), session); diff --git a/uPortal-layout/uPortal-layout-impl/src/main/java/org/apereo/portal/layout/dlm/MissingPortletDefinition.java b/uPortal-layout/uPortal-layout-impl/src/main/java/org/apereo/portal/layout/dlm/MissingPortletDefinition.java index c549ae10943..6f06fbe23f1 100644 --- a/uPortal-layout/uPortal-layout-impl/src/main/java/org/apereo/portal/layout/dlm/MissingPortletDefinition.java +++ b/uPortal-layout/uPortal-layout-impl/src/main/java/org/apereo/portal/layout/dlm/MissingPortletDefinition.java @@ -36,8 +36,8 @@ */ public class MissingPortletDefinition implements IPortletDefinition { - private static final String FNAME = "DLMStaticMissingChannel"; - /* package-private */ static final String CHANNEL_ID = "-1"; + public static final String FNAME = "DLMStaticMissingChannel"; + public static final String CHANNEL_ID = "-1"; public static final IPortletDefinition INSTANCE = new MissingPortletDefinition();