Skip to content

Commit

Permalink
SAK-32166 Allow import from site on custom site type. (sakaiproject#3882
Browse files Browse the repository at this point in the history
)

When you are in a site that is inheriting it’s tools from another site type through configuration:

    # This allows repository site types to have all the normal project tools.
    projectSiteType=repository,project
    projectSiteTargetType=project

The tools list presented is correct, but when you attempt to perform the import it fails. This is because when checking that the tools are valid for the site being imported into it doesn’t use this configuration to locate the list of valid tools.

Although this isn’t an API I’ve added some tests to that we can check that the internal methods are behaving as expected.
  • Loading branch information
buckett authored Feb 24, 2017
1 parent a081874 commit 30e3a1b
Show file tree
Hide file tree
Showing 4 changed files with 208 additions and 32 deletions.
24 changes: 24 additions & 0 deletions site-manage/site-manage-tool/tool/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,30 @@
<groupId>org.sakaiproject.common</groupId>
<artifactId>sakai-privacy-api</artifactId>
</dependency>
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-api-mockito</artifactId>
<version>1.6.5</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-module-junit4</artifactId>
<version>1.6.5</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<version>1.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>1.10.19</version>
<scope>test</scope>
</dependency>

</dependencies>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.Hashtable;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.ListIterator;
import java.util.Locale;
Expand Down Expand Up @@ -321,12 +322,12 @@ public class SiteAction extends PagedResourceActionII {
/** Name of state attribute for Site Information */
private static final String STATE_SITE_INFO = "site.info";

private static final String STATE_SITE_TYPE = "site-type";
static final String STATE_SITE_TYPE = "site-type";

/** Name of state attribute for possible site types */
private static final String STATE_SITE_TYPES = "site_types";

private static final String STATE_DEFAULT_SITE_TYPE = "default_site_type";
static final String STATE_DEFAULT_SITE_TYPE = "default_site_type";

private static final String STATE_PUBLIC_CHANGEABLE_SITE_TYPES = "changeable_site_types";

Expand Down Expand Up @@ -7728,10 +7729,12 @@ private void resetVisitedTemplateListToIndex(SessionState state, String index) {

/**
* toolId might be of form original tool id concatenated with number
* find whether there is an counterpart in the the multipleToolIdSet
* @param state
* @param toolId
* @return
* find whether there is an counterpart in the the multipleToolIdSet.
* Also only returns tools that are considered valid tools for this site type.
*
* @param state The session state to get the site type from.
* @param toolId The tool ID to find.
* @return <code>null</code> if the tool couldn't be found or the matched tool ID.
*/
private String findOriginalToolId(SessionState state, String toolId) {
// treat home tool differently
Expand All @@ -7741,11 +7744,11 @@ private String findOriginalToolId(SessionState state, String toolId) {
}
else
{
Set toolRegistrationList = ToolManager.findTools(Collections.singleton(state.getAttribute(STATE_SITE_TYPE)), null);
Set<Tool>toolRegistrationSet = getToolRegistrations(state, (String) state.getAttribute(STATE_SITE_TYPE));
String rv = null;
if (toolRegistrationList != null)
if (toolRegistrationSet != null)
{
for (Iterator i=toolRegistrationList.iterator(); rv == null && i.hasNext();)
for (Iterator i=toolRegistrationSet.iterator(); rv == null && i.hasNext();)
{
Tool tool = (Tool) i.next();
String tId = tool.getId();
Expand All @@ -7756,22 +7759,26 @@ private String findOriginalToolId(SessionState state, String toolId) {
}
}

// replace fake tool ids with real ones. Don't duplicate, since several fake tool ids may appear for the same real one
// it's far from clear that we need to be using fake tool ids at all. But I don't know the code well enough
// to get rid of the fakes completely.
// modified to exclude nulls as sakai.assignment is still in the list (as of Nov 2014) and is unresolvable
private List<String> originalToolIds(List<String>toolIds, SessionState state) {
Set<String>found = new HashSet<String>();
List<String>rv = new ArrayList<String>();
/**
* This extracts original tools IDs from compound IDs. The compound tools IDs are used when there
* is more than once copy of a tool on a site. For example lessonbuilder and web content can have
* multiple pages in the site on which they appear. It also filters out any tool IDs that aren't
* considered valid in this site.
*
* @param toolIds The list of tool IDs
* @param state The session state.
* @return A filtered list of tool IDs.
*/
List<String> originalToolIds(List<String>toolIds, SessionState state) {
Set<String>rv = new LinkedHashSet<>();

for (String toolId: toolIds) {
String origToolId = findOriginalToolId(state, toolId);
if (StringUtils.isNotBlank(origToolId) && !found.contains(origToolId)) {
rv.add(origToolId);
found.add(origToolId);
}
String origToolId = findOriginalToolId(state, toolId);
if (StringUtils.isNotBlank(origToolId)) {
rv.add(origToolId);
}
}
return rv;
return new ArrayList<>(rv);
}

private String originalToolId(String toolId, String toolRegistrationId) {
Expand All @@ -7782,15 +7789,17 @@ private String originalToolId(String toolId, String toolRegistrationId) {
}
else if (toolId.indexOf(toolRegistrationId) != -1 && isMultipleInstancesAllowed(toolRegistrationId))
{
// the multiple tool id format is of SITE_IDTOOL_IDx, where x is an intger >= 1
if (toolId.endsWith(toolRegistrationId))
{
// get the site id part out
// the multiple tool id format is of {page_id}{tool_id}
// get the page id part out
String uuid = toolId.replaceFirst(toolRegistrationId, "");
if (uuid != null && uuid.length() == UUID_LENGTH)
rv = toolRegistrationId;
} else
}
else
{
// the multiple tool id format is of {tool_id}{x}, where x is an intger >= 1
String suffix = toolId.substring(toolId.indexOf(toolRegistrationId) + toolRegistrationId.length());
try
{
Expand Down Expand Up @@ -11569,10 +11578,12 @@ else if (multiAllowed && toolId.startsWith(toolRegId))
* @param siteType The type of the site to get the list of tools for.
* @return A Set of possible tools.
*/
private Set<Tool> getToolRegistrations(SessionState state, String siteType) {
Set categories = new HashSet();
Set<Tool> getToolRegistrations(SessionState state, String siteType) {
if (siteType == null) {
return Collections.emptySet();
}
Set<String> categories = new HashSet<>();
// UMICH 1035
categories.add(siteType);
categories.add(SiteTypeUtil.getTargetSiteType(siteType));
Set toolRegistrationSet = ToolManager.findTools(categories, null);
if ((toolRegistrationSet == null || toolRegistrationSet.size() == 0)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
package org.sakaiproject.site.tool;

import org.hamcrest.collection.IsCollectionContaining;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.sakaiproject.component.cover.ComponentManager;
import org.sakaiproject.component.cover.ServerConfigurationService;
import org.sakaiproject.event.api.SessionState;
import org.sakaiproject.site.cover.SiteService;
import org.sakaiproject.tool.api.Session;
import org.sakaiproject.tool.api.SessionManager;
import org.sakaiproject.tool.api.Tool;
import org.sakaiproject.tool.cover.ToolManager;

import java.util.*;

import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;
import static org.sakaiproject.site.tool.SiteAction.STATE_DEFAULT_SITE_TYPE;
import static org.sakaiproject.site.tool.SiteAction.STATE_SITE_TYPE;

/**
* Tests for SiteAction
*/
@SuppressWarnings("deprecation")
@RunWith(PowerMockRunner.class)
@PrepareForTest({ComponentManager.class})
public class SiteActionTestTools {

private SiteAction siteAction;

@Before
public void setUp() {
PowerMockito.mockStatic(ComponentManager.class);
// A mock component manager.
when(ComponentManager.get(any(Class.class))).then(new Answer<Object>() {
private Map<Class, Object> mocks = new HashMap<>();
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
Class classToMock = (Class) invocation.getArguments()[0];
return mocks.computeIfAbsent(classToMock, k -> mock(classToMock));
}
});
// Mock the Session so that ResourceLoader doesn't NPE on init
when(ComponentManager.get(SessionManager.class).getCurrentSession()).thenReturn(mock(Session.class));
siteAction = new SiteAction();
}

@Test
public void testGetToolRegistrationsSimple() {
// Normal project site type
Tool projectTool = mock(Tool.class);
when(ToolManager.findTools(singleton("project"), null)).thenReturn(singleton(projectTool));
when(SiteService.getSiteTypeStrings("project")).thenReturn(singletonList("project"));
when(ServerConfigurationService.getString("projectSiteTargetType", "project")).thenReturn("project");

SessionState state = mock(SessionState.class);
Set<Tool> project = siteAction.getToolRegistrations(state, "project");
assertThat(project, IsCollectionContaining.hasItems(projectTool));
}

@Test
public void testGetToolRegistrationNone() {
// Site type that doesn't exist
SessionState state = mock(SessionState.class);
Set<Tool> other = siteAction.getToolRegistrations(state, "other");
assertTrue(other.isEmpty());
}

@Test
public void testGetToolRegistrationDefault() {
// Check that we fallback to the default site type
Tool projectTool = mock(Tool.class);
when(ToolManager.findTools(singleton("project"), null)).thenReturn(singleton(projectTool));
when(SiteService.getSiteTypeStrings("new")).thenReturn(Arrays.asList("project", "new"));
when(ServerConfigurationService.getString("projectSiteTargetType", "project")).thenReturn("project");

SessionState state = mock(SessionState.class);
when(state.getAttribute(STATE_DEFAULT_SITE_TYPE)).thenReturn("project");
Set<Tool> tools = siteAction.getToolRegistrations(state, "new");
assertThat(tools, IsCollectionContaining.hasItems(projectTool));
}

@Test
public void testOriginalToolIds() {
// Normal project site type
Tool singleTool = mock(Tool.class);
Tool multipleTool = mock(Tool.class);
when(singleTool.getId()).thenReturn("sakai.single");
when(multipleTool.getId()).thenReturn("sakai.multiple");
when(ToolManager.findTools(singleton("project"), null)).thenReturn(new HashSet<>(Arrays.asList(singleTool, multipleTool)));
when(ToolManager.getTool("sakai.single")).thenReturn(singleTool);
when(ToolManager.getTool("sakai.multiple")).thenReturn(multipleTool);
Properties singleToolProperties = new Properties();
singleToolProperties.put("allowMultipleInstances", "false");
when(singleTool.getRegisteredConfig()).thenReturn(singleToolProperties);
Properties multipleToolProperties = new Properties();
multipleToolProperties.put("allowMultipleInstances", "true");
when(multipleTool.getRegisteredConfig()).thenReturn(multipleToolProperties);

when(SiteService.getSiteTypeStrings("project")).thenReturn(singletonList("project"));
when(ServerConfigurationService.getString("projectSiteTargetType", "project")).thenReturn("project");

SessionState state = mock(SessionState.class);
when(state.getAttribute(STATE_SITE_TYPE)).thenReturn("project");

List<String> filteredToolIds;
filteredToolIds = siteAction.originalToolIds(singletonList("not.present"), state);
assertTrue(filteredToolIds.isEmpty());

filteredToolIds = siteAction.originalToolIds(singletonList("sakai.single"), state);
assertEquals(Arrays.asList("sakai.single"), filteredToolIds);

filteredToolIds = siteAction.originalToolIds(Arrays.asList("sakai.single", "not.present"), state);
assertEquals(Arrays.asList("sakai.single"), filteredToolIds);

filteredToolIds = siteAction.originalToolIds(Arrays.asList("sakai.single1", "not.present"), state);
assertEquals(Arrays.asList(), filteredToolIds);

filteredToolIds = siteAction.originalToolIds(Arrays.asList("sakai.multiple", "not.present"), state);
assertEquals(Arrays.asList("sakai.multiple"), filteredToolIds);

filteredToolIds = siteAction.originalToolIds(Arrays.asList("sakai.multiple1", "sakai.multiple2"), state);
assertEquals(Arrays.asList("sakai.multiple"), filteredToolIds);

// Has to be 36 characters long.
filteredToolIds = siteAction.originalToolIds(Arrays.asList("012345678901234567890123456789012345sakai.multiple"), state);
assertEquals(Arrays.asList("sakai.multiple"), filteredToolIds);

}



}
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@

import java.util.List;

import org.sakaiproject.component.api.ServerConfigurationService;
import org.sakaiproject.component.cover.ComponentManager;
import org.sakaiproject.site.api.SiteService;

public class SiteTypeUtil {

private static org.sakaiproject.site.api.SiteService siteService = (org.sakaiproject.site.api.SiteService) ComponentManager
.get("org.sakaiproject.site.api.SiteService");
private static SiteService siteService = ComponentManager.get(SiteService.class);

private static org.sakaiproject.component.api.ServerConfigurationService serverConfigurationService = (org.sakaiproject.component.api.ServerConfigurationService) ComponentManager
.get("org.sakaiproject.component.api.ServerConfigurationService");
private static ServerConfigurationService serverConfigurationService = ComponentManager.get(ServerConfigurationService.class);

/**
* get all site type strings associated with course site type
Expand Down

0 comments on commit 30e3a1b

Please sign in to comment.