Skip to content

Commit

Permalink
SAK-33973 EntityBroker Search endpoint was broken (sakaiproject#5361)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ern
ern authored Mar 2, 2018
1 parent 7b6a2e6 commit 80f9f46
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 64 deletions.
4 changes: 4 additions & 0 deletions entitybroker/utils/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@
<groupId>org.sakaiproject.kernel</groupId>
<artifactId>sakai-kernel-api</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
<!-- required for bean cloning and reflection -->
<dependency>
<groupId>org.azeckoski</groupId>
Expand Down
80 changes: 40 additions & 40 deletions entitybroker/utils/src/java/org/sakaiproject/entitybroker/util/request/RequestUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.apache.commons.lang3.StringUtils;
import org.sakaiproject.entitybroker.EntityView;
import org.sakaiproject.entitybroker.entityprovider.extension.Formats;
import org.sakaiproject.entitybroker.entityprovider.extension.RequestStorage;
Expand Down Expand Up @@ -385,8 +386,9 @@ private static synchronized HashSet<String> getIgnoreSet() {
*/
public static Search makeSearchFromRequestParams(Map<String, Object> params) {
Search search = new Search();
int page = -1;
int limit = -1;
int limit, page, start;
page = start = 0;
limit = 10;
try {
if (params != null) {
for (Entry<String, Object> entry : params.entrySet()) {
Expand All @@ -409,17 +411,15 @@ public static Search makeSearchFromRequestParams(Map<String, Object> params) {
|| "count".equals(key)
|| "itemsPerPage".equals(key)) {
try {
limit = Integer.valueOf(value.toString()).intValue();
search.setLimit(limit);
limit = Integer.valueOf(value.toString());
} catch (NumberFormatException e) {
log.warn("Invalid non-number passed in for _limit/_perpage param: " + value + ":" + e);
}
continue;
} else if ("_start".equals(key)
|| "startIndex".equals(key)) {
try {
int start = Integer.valueOf(value.toString()).intValue();
search.setStart(start);
start = Integer.valueOf(value.toString());
} catch (NumberFormatException e) {
log.warn("Invalid non-number passed in for '_start' param: " + value + ":" + e);
}
Expand All @@ -428,47 +428,43 @@ public static Search makeSearchFromRequestParams(Map<String, Object> params) {
|| "page".equals(key)
|| "startPage".equals(key)) {
try {
page = Integer.valueOf(value.toString()).intValue();
page = Integer.valueOf(value.toString());
} catch (NumberFormatException e) {
log.warn("Invalid non-number passed in for '_page' param: " + value + ":" + e);
}
continue;
} else if ("_order".equals(key)
|| "_sort".equals(key)
|| "sort".equals(key)) {
if (value != null) {
String val = value.toString();
String[] sortBy = new String[] {val};
if (val.indexOf(',') > 0) {
// multiple sort params
sortBy = val.split(",");
}
try {
for (int i = 0; i < sortBy.length; i++) {
String sortItem = sortBy[i].trim();
if (sortItem.endsWith("_reverse")) {
search.addOrder( new Order(sortItem.substring(0, sortItem.length()-8), false) );
} else if (sortItem.endsWith("_desc")) {
search.addOrder( new Order(sortItem.substring(0, sortItem.length()-5), false) );
} else if (sortItem.endsWith("_asc")) {
search.addOrder( new Order(sortItem.substring(0, sortItem.length()-4)) );
} else {
search.addOrder( new Order(sortItem) );
}
String val = value.toString();
String[] sortBy = new String[] {val};
if (val.indexOf(',') > 0) {
// multiple sort params
sortBy = val.split(",");
}
try {
for (String sortItem : sortBy) {
sortItem = StringUtils.trimToEmpty(sortItem);
if (sortItem.endsWith("_reverse")) {
search.addOrder(new Order(sortItem.substring(0, sortItem.length() - 8), false));
} else if (sortItem.endsWith("_desc")) {
search.addOrder(new Order(sortItem.substring(0, sortItem.length() - 5), false));
} else if (sortItem.endsWith("_asc")) {
search.addOrder(new Order(sortItem.substring(0, sortItem.length() - 4)));
} else {
search.addOrder(new Order(sortItem));
}
} catch (RuntimeException e) {
log.warn("WARN Failed while getting the sort/order param: " + val + ":" + e);
}
} catch (RuntimeException e) {
log.warn("Failure while getting the sort/order param: {}:{}", val, e.getMessage());
}
continue;
} else if ("_searchTerms".equals(key)
|| "searchTerms".equals(key)) {
// indicates a space delimited list of search terms
if (value != null) {
String val = value.toString();
String[] terms = val.split(" ");
search.addRestriction( new Restriction("searchTerms", terms) );
}
String val = value.toString();
String[] terms = val.split(" ");
search.addRestriction( new Restriction("searchTerms", terms) );
continue;
}
}
Expand All @@ -479,15 +475,19 @@ public static Search makeSearchFromRequestParams(Map<String, Object> params) {
// failed to translate the request to a search, not really much to do here
log.warn("Could not translate entity request into search params: " + e.getMessage() + ":" + e);
}
// translate page into start/limit

// if paging has been specified ignore start
int end;
if (page > 0) {
if (limit <= -1) {
limit = 10; // set to a default value
search.setLimit(limit);
log.warn("Page is set without a limit per page, setting per page limit to default value of 10");
}
search.setStart( (page-1) * limit );
// translate page into start/end
start = ((page - 1) * limit);
end = page * limit;
} else {
end = start + limit;
}
search.setStart(start);
search.setLimit(end);

return search;
}

Expand Down
12 changes: 6 additions & 6 deletions entitybroker/utils/src/test/org/sakaiproject/entitybroker/util/request/RequestUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void testMakeSearchFromRequestStorage() {
assertEquals(1, search.getRestrictions().length);
assertNotNull( search.getRestrictionByProperty("test") );
assertEquals("stuff", search.getRestrictionByProperty("test").value);
assertEquals(10, search.getLimit());
assertEquals(15, search.getLimit());
assertEquals(5, search.getStart());

params.clear();
Expand All @@ -113,7 +113,7 @@ public void testMakeSearchFromRequestStorage() {
assertEquals(1, search.getRestrictions().length);
assertNotNull( search.getRestrictionByProperty("test") );
assertEquals("stuff", search.getRestrictionByProperty("test").value);
assertEquals(5, search.getLimit());
assertEquals(15, search.getLimit());
assertEquals(10, search.getStart());
}

Expand All @@ -131,7 +131,7 @@ public void testMakeSearchFromRequestStorageOrder() {
assertEquals(1, search.getRestrictions().length);
assertNotNull( search.getRestrictionByProperty("test") );
assertEquals("stuff", search.getRestrictionByProperty("test").value);
assertEquals(0, search.getLimit());
assertEquals(10, search.getLimit());
assertEquals(0, search.getStart());
assertEquals(1, search.getOrders().length);
assertEquals("name", search.getOrders()[0].getProperty());
Expand All @@ -147,7 +147,7 @@ public void testMakeSearchFromRequestStorageOrder() {
assertEquals(1, search.getRestrictions().length);
assertNotNull( search.getRestrictionByProperty("test") );
assertEquals("stuff", search.getRestrictionByProperty("test").value);
assertEquals(0, search.getLimit());
assertEquals(10, search.getLimit());
assertEquals(0, search.getStart());
assertEquals(1, search.getOrders().length);
assertEquals("name", search.getOrders()[0].getProperty());
Expand All @@ -163,7 +163,7 @@ public void testMakeSearchFromRequestStorageOrder() {
assertEquals(1, search.getRestrictions().length);
assertNotNull( search.getRestrictionByProperty("test") );
assertEquals("stuff", search.getRestrictionByProperty("test").value);
assertEquals(0, search.getLimit());
assertEquals(10, search.getLimit());
assertEquals(0, search.getStart());
assertEquals(2, search.getOrders().length);
assertEquals("name", search.getOrders()[0].getProperty());
Expand All @@ -181,7 +181,7 @@ public void testMakeSearchFromRequestStorageOrder() {
assertEquals(1, search.getRestrictions().length);
assertNotNull( search.getRestrictionByProperty("test") );
assertEquals("stuff", search.getRestrictionByProperty("test").value);
assertEquals(0, search.getLimit());
assertEquals(10, search.getLimit());
assertEquals(0, search.getStart());
assertEquals(3, search.getOrders().length);
assertEquals("name", search.getOrders()[0].getProperty());
Expand Down
2 changes: 1 addition & 1 deletion .../search-impl/impl/src/java/org/sakaiproject/search/entitybroker/SearchEntityProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public List<SearchResultEntity> search(EntityReference ref, Search search) {
*
* @return a list of supported tools
*/
@EntityCustomAction(action = "tools", viewKey = EntityView.VIEW_SHOW)
@EntityCustomAction(action = "tools", viewKey = EntityView.VIEW_LIST)
public Set<String> getTools() {
List<EntityContentProducer> entityContentProducers = searchIndexBuilder.getContentProducers();
Set<String> tools = new HashSet<String>(entityContentProducers.size());
Expand Down
33 changes: 16 additions & 17 deletions search/search-impl/impl/src/webapp/WEB-INF/siteAdapterComponents.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,21 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">

<bean id="siteAdapter"
class="org.sakaiproject.search.component.adapter.site.SiteContentProducer"
init-method="init" >
<property name="siteService" ><ref bean="org.sakaiproject.site.api.SiteService"/></property>

<property name="entityManager"><ref bean="org.sakaiproject.entity.api.EntityManager" /></property>
<property name="searchService"><ref bean="org.sakaiproject.search.api.SearchService"/></property>
<property name="searchIndexBuilder"><ref bean="org.sakaiproject.search.api.SearchIndexBuilder" /></property>
<property name="serverConfigurationService"><ref bean="org.sakaiproject.component.api.ServerConfigurationService"/></property>

</bean>

<bean parent="org.sakaiproject.entitybroker.entityprovider.AbstractEntityProvider"
class="org.sakaiproject.search.entitybroker.SearchEntityProvider">
<property name="searchService" ref="org.sakaiproject.search.api.SearchService" />
<property name="siteService" ref="org.sakaiproject.site.api.SiteService" />
<property name="userDirectoryService" ref="org.sakaiproject.user.api.UserDirectoryService" />
<bean id="siteAdapter"
class="org.sakaiproject.search.component.adapter.site.SiteContentProducer"
init-method="init">
<property name="siteService" ref="org.sakaiproject.site.api.SiteService"/>
<property name="entityManager" ref="org.sakaiproject.entity.api.EntityManager"/>
<property name="searchService" ref="org.sakaiproject.search.api.SearchService"/>
<property name="searchIndexBuilder" ref="org.sakaiproject.search.api.SearchIndexBuilder"/>
<property name="serverConfigurationService" ref="org.sakaiproject.component.api.ServerConfigurationService"/>
</bean>

<bean parent="org.sakaiproject.entitybroker.entityprovider.AbstractEntityProvider"
class="org.sakaiproject.search.entitybroker.SearchEntityProvider">
<property name="searchIndexBuilder" ref="org.sakaiproject.search.api.SearchIndexBuilder"/>
<property name="searchService" ref="org.sakaiproject.search.api.SearchService"/>
<property name="siteService" ref="org.sakaiproject.site.api.SiteService"/>
<property name="userDirectoryService" ref="org.sakaiproject.user.api.UserDirectoryService"/>
</bean>
</beans>

0 comments on commit 80f9f46

Please sign in to comment.