You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rave.apache.org by ja...@apache.org on 2011/08/17 20:58:35 UTC

svn commit: r1158868 - in /incubator/rave/trunk/rave-portal/src: main/java/org/apache/rave/portal/model/util/ main/java/org/apache/rave/portal/repository/ main/java/org/apache/rave/portal/repository/impl/ main/java/org/apache/rave/portal/service/ main/...

Author: jasha
Date: Wed Aug 17 18:58:34 2011
New Revision: 1158868

URL: http://svn.apache.org/viewvc?rev=1158868&view=rev
Log:
RAVE-196 Use pagination in widget store to prevent a large list of widgets

Added:
    incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/model/util/SearchResultTest.java
Modified:
    incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/util/SearchResult.java
    incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/WidgetRepository.java
    incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java
    incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/WidgetService.java
    incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultWidgetService.java
    incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java
    incubator/rave/trunk/rave-portal/src/main/webapp/WEB-INF/views/store.jsp
    incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/repository/JpaWidgetRepositoryTest.java
    incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/service/WidgetServiceTest.java
    incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/web/controller/WidgetStoreControllerTest.java

Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/util/SearchResult.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/util/SearchResult.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/util/SearchResult.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/util/SearchResult.java Wed Aug 17 18:58:34 2011
@@ -61,4 +61,15 @@ public class SearchResult<T extends Basi
     public void setPageSize(int pageSize) {
         this.pageSize = pageSize;
     }
+
+    public int getNumberOfPages() {
+        if (pageSize == 0) {
+            return 0;
+        }
+        int numberOfPages = totalResults / pageSize;
+        if (totalResults % pageSize > 0) {
+            numberOfPages++;
+        }
+        return numberOfPages;
+    }
 }

Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/WidgetRepository.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/WidgetRepository.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/WidgetRepository.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/WidgetRepository.java Wed Aug 17 18:58:34 2011
@@ -25,13 +25,23 @@ import org.apache.rave.portal.model.Widg
 
 public interface WidgetRepository extends Repository<Widget> {
     /**
-     * Gets a list of all widgets in the repository
+     * Gets a list of <strong>all</strong> widgets in the repository
      *
      * @return a valid List
      */
     List<Widget> getAll();
 
     /**
+     * List of Widgets with the same condition as in {@link #getAll()}
+     * but with a limited amount of Widgets.
+     *
+     * @param offset   start point within the total resultset
+     * @param pageSize maximum number of items to be returned (for paging)
+     * @return a List of widgets with of at most the number of items in pageSize
+     */
+    List<Widget> getLimitedList(int offset, int pageSize);
+
+    /**
      * @return the total number of {@link Widget}'s in the repository. Useful for paging.
      */
     int getCountAll();

Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java Wed Aug 17 18:58:34 2011
@@ -28,25 +28,43 @@ import javax.persistence.TypedQuery;
 import org.apache.rave.persistence.jpa.AbstractJpaRepository;
 import org.apache.rave.portal.model.Widget;
 import org.apache.rave.portal.repository.WidgetRepository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Repository;
 
 @Repository
 public class JpaWidgetRepository extends AbstractJpaRepository<Widget> implements WidgetRepository {
 
+    private final Logger log = LoggerFactory.getLogger(JpaWidgetRepository.class);
+
+    private int LARGE_PAGESIZE = 1000;
+
     public JpaWidgetRepository() {
         super(Widget.class);
     }
 
     @Override
     public List<Widget> getAll() {
+        log.warn("Requesting potentially large resultset of Widget. No pagesize set.");
         TypedQuery<Widget> query = manager.createNamedQuery("Widget.getAll", Widget.class);
         return query.getResultList();
     }
 
     @Override
+    public List<Widget> getLimitedList(int offset, int pageSize) {
+        TypedQuery<Widget> query = manager.createNamedQuery("Widget.getAll", Widget.class);
+        query.setFirstResult(offset);
+        if (pageSize >= LARGE_PAGESIZE) {
+            log.warn("Requesting potentially large resultset of Widgets. Pagesize is {}", pageSize);
+        }
+        query.setMaxResults(pageSize);
+        return query.getResultList();
+    }
+
+    @Override
     public int getCountAll() {
         Query query = manager.createNamedQuery("Widget.countAll");
-        Number countResult=(Number) query.getSingleResult();
+        Number countResult = (Number) query.getSingleResult();
         return countResult.intValue();
     }
 
@@ -54,12 +72,11 @@ public class JpaWidgetRepository extends
     public List<Widget> getByFreeTextSearch(String searchTerm, int offset, int pageSize) {
         TypedQuery<Widget> query = manager.createNamedQuery("Widget.getByFreeText", Widget.class);
         query.setParameter("searchTerm", "%" + searchTerm.toLowerCase() + "%");
-        if (offset > 0) {
-            query.setFirstResult(offset);
-        }
-        if (pageSize > 0) {
-            query.setMaxResults(pageSize);
+        if (pageSize >= LARGE_PAGESIZE) {
+            log.warn("Requesting potentially large resultset of Widgets. Pagesize is {}", pageSize);
         }
+        query.setFirstResult(offset);
+        query.setMaxResults(pageSize);
         return query.getResultList();
     }
 
@@ -67,7 +84,7 @@ public class JpaWidgetRepository extends
     public int getCountFreeTextSearch(String searchTerm) {
         Query query = manager.createNamedQuery("Widget.countByFreeText");
         query.setParameter("searchTerm", "%" + searchTerm.toLowerCase() + "%");
-        Number countResult=(Number) query.getSingleResult();
+        Number countResult = (Number) query.getSingleResult();
         return countResult.intValue();
     }
 }

Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/WidgetService.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/WidgetService.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/WidgetService.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/WidgetService.java Wed Aug 17 18:58:34 2011
@@ -29,11 +29,23 @@ public interface WidgetService {
 
     /**
      * Gets a {@link SearchResult} for {@link Widget}'s that a user can add to their context
+     * <p/>
+     * May return a very large resultset
      *
      * @return SearchResult
      */
     SearchResult<Widget> getAllWidgets();
 
+
+    /**
+     * Gets a limited {@link SearchResult} for {@link Widget}'s  that a user can add to their context.
+     *
+     * @param offset start point within the resultset (for paging)
+     * @param pageSize   maximum number of items to be returned (for paging)
+     * @return SearchResult
+     */
+    SearchResult<Widget> getLimitedListOfWidgets(int offset, int pageSize);
+
     /**
      * Gets a SearchResult for {@link Widget}'s by performing a free text search
      *

Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultWidgetService.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultWidgetService.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultWidgetService.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultWidgetService.java Wed Aug 17 18:58:34 2011
@@ -41,24 +41,29 @@ public class DefaultWidgetService implem
 
     @Override
     public SearchResult<Widget> getAllWidgets() {
-        int count = widgetRepository.getCountAll();
-        List<Widget> widgets = widgetRepository.getAll();
+        final int count = widgetRepository.getCountAll();
+        final List<Widget> widgets = widgetRepository.getAll();
         return new SearchResult<Widget>(widgets, count);
     }
 
     @Override
+    public SearchResult<Widget> getLimitedListOfWidgets(int offset, int pageSize) {
+        final int count = widgetRepository.getCountAll();
+        final List<Widget> widgets = widgetRepository.getLimitedList(offset, pageSize);
+        final SearchResult<Widget> searchResult = new SearchResult<Widget>(widgets, count);
+        searchResult.setPageSize(pageSize);
+        return searchResult;
+    }
+
+    @Override
     public SearchResult<Widget> getWidgetsByFreeTextSearch(String searchTerm, int offset, int pageSize) {
-        int count;
-        List<Widget> widgets;
         if (StringUtils.isBlank(searchTerm)) {
-            count = widgetRepository.getCountAll();
-            // TODO a getAll with paging support
-            widgets = widgetRepository.getAll();
-
-        } else {
-            count = widgetRepository.getCountFreeTextSearch(searchTerm);
-            widgets = widgetRepository.getByFreeTextSearch(searchTerm, offset, pageSize);
+            return getLimitedListOfWidgets(offset, pageSize);
         }
+
+        final int count = widgetRepository.getCountFreeTextSearch(searchTerm);
+        final List<Widget>widgets = widgetRepository.getByFreeTextSearch(searchTerm, offset, pageSize);
+
         final SearchResult<Widget> searchResult = new SearchResult<Widget>(widgets, count);
         searchResult.setPageSize(pageSize);
         return searchResult;

Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java Wed Aug 17 18:58:34 2011
@@ -49,11 +49,14 @@ public class WidgetStoreController {
      *
      * @param model           model map
      * @param referringPageId the source {@link org.apache.rave.portal.model.Page } ID
+     * @param offset          offset within the total amount of results (to enable paging)
      * @return the view name of the main store page
      */
     @RequestMapping(method = RequestMethod.GET)
-    public String view(Model model, @RequestParam long referringPageId) {
-        model.addAttribute(ModelKeys.WIDGETS, widgetService.getAllWidgets());
+    public String view(Model model, @RequestParam long referringPageId,
+                       @RequestParam(required = false, defaultValue = "0") int offset) {
+        model.addAttribute(ModelKeys.WIDGETS,
+                widgetService.getLimitedListOfWidgets(offset, MAXIMUM_WIDGETS_PER_PAGE));
         model.addAttribute(ModelKeys.REFERRING_PAGE_ID, referringPageId);
         return ViewNames.STORE;
     }

Modified: incubator/rave/trunk/rave-portal/src/main/webapp/WEB-INF/views/store.jsp
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/webapp/WEB-INF/views/store.jsp?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/webapp/WEB-INF/views/store.jsp (original)
+++ incubator/rave/trunk/rave-portal/src/main/webapp/WEB-INF/views/store.jsp Wed Aug 17 18:58:34 2011
@@ -103,19 +103,18 @@
             </c:forEach>
         </ul>
         
-        <c:if test="${fn:length(widgets.resultSet) lt widgets.totalResults and widgets.pageSize gt 0}">
+        <c:if test="${widgets.numberOfPages gt 1}">
             <div class="storeBox">
-            <ul class="paging">
-                <c:set var="nrOfPages" value="${widgets.totalResults / widgets.pageSize}"/>
-                <c:forEach var="i" begin="0" end="${nrOfPages}">
-                    <c:url var="pageUrl" value="/app/store/search">
-                        <c:param name="referringPageId" value="${referringPageId}"/>
-                        <c:param name="searchTerm" value="${searchTerm}"/>
-                        <c:param name="offset" value="${i * widgets.pageSize}"/>
-                    </c:url>
-                    <li><a href="<c:out value="${pageUrl}"/>">${i + 1}</a></li>
-                </c:forEach>
-            </ul>
+                <ul class="paging">
+                    <c:forEach var="i" begin="0" end="${widgets.numberOfPages - 1}">
+                        <c:url var="pageUrl" value="">
+                            <c:param name="referringPageId" value="${referringPageId}"/>
+                            <c:param name="searchTerm" value="${searchTerm}"/>
+                            <c:param name="offset" value="${i * widgets.pageSize}"/>
+                        </c:url>
+                        <li><a href="<c:out value="${pageUrl}"/>">${i + 1}</a></li>
+                    </c:forEach>
+                </ul>
             </div>
         </c:if>
     </c:if>

Added: incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/model/util/SearchResultTest.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/model/util/SearchResultTest.java?rev=1158868&view=auto
==============================================================================
--- incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/model/util/SearchResultTest.java (added)
+++ incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/model/util/SearchResultTest.java Wed Aug 17 18:58:34 2011
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.rave.portal.model.util;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.rave.persistence.BasicEntity;
+import org.junit.Before;
+import org.junit.Test;
+
+import static junit.framework.Assert.assertEquals;
+
+/**
+ * Test for {@link SearchResult}
+ */
+public class SearchResultTest {
+    SearchResult searchResult;
+
+    @Test
+    public void testGetNumberOfPages_noFractal() throws Exception {
+        searchResult.setPageSize(15);
+        assertEquals("Total results is 1 * page size", 1, searchResult.getNumberOfPages());
+
+    }
+
+    @Test
+    public void testGetNumberOfPages_addPage() throws Exception {
+        searchResult.setPageSize(10);
+        assertEquals("Total results is 1.5 * page size", 2, searchResult.getNumberOfPages());
+    }
+
+
+    @Test
+    public void testGetNumberOfPages_noPageSize() throws Exception {
+        searchResult.setPageSize(0);
+        assertEquals("Division by 0", 0, searchResult.getNumberOfPages());
+    }
+
+    @Before
+    @SuppressWarnings("unchecked")
+    public void setup() {
+        List<? extends BasicEntity> results = new ArrayList<BasicEntity>();
+        searchResult = new SearchResult(results, 15);
+    }
+
+}
+

Modified: incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/repository/JpaWidgetRepositoryTest.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/repository/JpaWidgetRepositoryTest.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/repository/JpaWidgetRepositoryTest.java (original)
+++ incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/repository/JpaWidgetRepositoryTest.java Wed Aug 17 18:58:34 2011
@@ -33,6 +33,7 @@ import org.springframework.test.context.
 import org.springframework.transaction.annotation.Transactional;
 
 import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertNotNull;
 import static junit.framework.Assert.assertTrue;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
@@ -87,6 +88,14 @@ public class JpaWidgetRepositoryTest {
     }
 
     @Test
+    public void getLimitedList() {
+        final int pageSize = 3;
+        List<Widget> widgets = repository.getLimitedList(0, pageSize);
+        assertNotNull(widgets);
+        assertTrue(widgets.size() <= pageSize);
+    }
+
+    @Test
     public void countAll() {
         int count = repository.getCountAll();
         assertTrue(count > 4);

Modified: incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/service/WidgetServiceTest.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/service/WidgetServiceTest.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/service/WidgetServiceTest.java (original)
+++ incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/service/WidgetServiceTest.java Wed Aug 17 18:58:34 2011
@@ -36,6 +36,7 @@ import static org.hamcrest.CoreMatchers.
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.CoreMatchers.sameInstance;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 
 /**
@@ -63,6 +64,23 @@ public class WidgetServiceTest {
     }
 
     @Test
+    public void getLimitedListOfWidgets() {
+        Widget widget1 = new Widget(1L,"http://example.com/widget1.xml");
+        Widget widget2 = new Widget(2L,"http://example.com/widget2.xml");
+        List<Widget> widgets = new ArrayList<Widget>();
+        widgets.add(widget1);
+        widgets.add(widget2);
+        final int pageSize = 10;
+        expect(repository.getLimitedList(0, pageSize)).andReturn(widgets);
+        replay(repository);
+
+        SearchResult<Widget> result = service.getLimitedListOfWidgets(0, pageSize);
+        assertEquals(pageSize, result.getPageSize());
+        assertSame(widgets, result.getResultSet());
+
+    }
+
+    @Test
     public void getWidget() {
         Widget w = new Widget();
         expect(repository.get(1L)).andReturn(w);

Modified: incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/web/controller/WidgetStoreControllerTest.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/web/controller/WidgetStoreControllerTest.java?rev=1158868&r1=1158867&r2=1158868&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/web/controller/WidgetStoreControllerTest.java (original)
+++ incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/web/controller/WidgetStoreControllerTest.java Wed Aug 17 18:58:34 2011
@@ -67,10 +67,10 @@ public class WidgetStoreControllerTest {
         List<Widget> widgets = new ArrayList<Widget>();
         SearchResult<Widget> emptyResult = new SearchResult<Widget>(widgets, 0);
 
-        expect(widgetService.getAllWidgets()).andReturn(emptyResult);
+        expect(widgetService.getLimitedListOfWidgets(0, 10)).andReturn(emptyResult);
         replay(widgetService);
 
-        String view = controller.view(model, REFERRER_ID);
+        String view = controller.view(model, REFERRER_ID, 0);
 
         verify(widgetService);
         assertThat(view, is(equalTo(ViewNames.STORE)));