You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by md...@apache.org on 2010/12/15 11:25:19 UTC
svn commit: r1049473 - in /jackrabbit/trunk:
jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/
jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/
jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/c...
Author: mduerig
Date: Wed Dec 15 10:25:19 2010
New Revision: 1049473
URL: http://svn.apache.org/viewvc?rev=1049473&view=rev
Log:
JCR-2844: MaxCount not working correctly in user/group query when restricting to group members
Added:
jackrabbit/trunk/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/iterator/BoundedIterator.java
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerSearchTest.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java?rev=1049473&r1=1049472&r2=1049473&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java Wed Dec 15 10:25:19 2010
@@ -25,6 +25,7 @@ import org.apache.jackrabbit.core.NodeIm
import org.apache.jackrabbit.core.SessionImpl;
import org.apache.jackrabbit.core.security.user.XPathQueryBuilder.Condition;
import org.apache.jackrabbit.core.security.user.XPathQueryBuilder.RelationOp;
+import org.apache.jackrabbit.spi.commons.iterator.BoundedIterator;
import org.apache.jackrabbit.spi.commons.iterator.Iterators;
import org.apache.jackrabbit.spi.commons.iterator.Predicate;
import org.apache.jackrabbit.spi.commons.iterator.Predicates;
@@ -105,23 +106,20 @@ public class XPathQueryEvaluator impleme
return Iterators.empty();
}
- if (maxCount > 0) {
- query.setLimit(maxCount);
- }
-
- // If we are scoped to a group and have an offset, we need to skip to that offset
- // here (inefficient!) otherwise we can apply the offset in the query
+ // If we are scoped to a group and have a limit, we have to apply the limit
+ // here (inefficient!) otherwise we can apply the limit in the query
if (builder.getGroupName() == null) {
if (offset > 0) {
query.setOffset(offset);
}
+ if (maxCount > 0) {
+ query.setLimit(maxCount);
+ }
return toAuthorizables(execute(query));
} else {
- Iterator<Authorizable> result = filter(toAuthorizables(execute(query)), builder.getGroupName(), builder.isDeclaredMembersOnly());
- for (int c = 0; c < offset && result.hasNext(); c++) {
- result.next();
- }
- return result;
+ Iterator<Authorizable> result = toAuthorizables(execute(query));
+ Iterator<Authorizable> filtered = filter(result, builder.getGroupName(), builder.isDeclaredMembersOnly());
+ return BoundedIterator.create(offset, maxCount, filtered);
}
}
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerSearchTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerSearchTest.java?rev=1049473&r1=1049472&r2=1049473&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerSearchTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/UserManagerSearchTest.java Wed Dec 15 10:25:19 2010
@@ -726,6 +726,29 @@ public class UserManagerSearchTest exten
assertFalse(result.hasNext());
}
+ public void testScopeWithMax() throws RepositoryException {
+ final int offset = 0;
+ final int count = 22;
+
+ Iterator<Authorizable> result = userMgr.findAuthorizables(new Query() {
+ public <T> void build(QueryBuilder<T> builder) {
+ builder.setScope("vertebrates", false);
+ builder.setSortOrder("profile/@weight", Direction.ASCENDING);
+ builder.setLimit(offset, count);
+ }
+ });
+
+ Iterator<Authorizable> expected = userMgr.findAuthorizables(new Query() {
+ public <T> void build(QueryBuilder<T> builder) {
+ builder.setScope("vertebrates", false);
+ builder.setSortOrder("profile/@weight", Direction.ASCENDING);
+ }
+ });
+
+ assertSameElements(expected, result);
+ assertFalse(result.hasNext());
+ }
+
//------------------------------------------< private >---
private static void addMembers(Group group, Authorizable... authorizables) throws RepositoryException {
Added: jackrabbit/trunk/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/iterator/BoundedIterator.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/iterator/BoundedIterator.java?rev=1049473&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/iterator/BoundedIterator.java (added)
+++ jackrabbit/trunk/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/iterator/BoundedIterator.java Wed Dec 15 10:25:19 2010
@@ -0,0 +1,115 @@
+/*
+ * 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.jackrabbit.spi.commons.iterator;
+
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+
+/**
+ * Implements a bounded iterator which only returns a maximum number of element from an underlying iterator
+ * starting at a given offset.
+ *
+ * @param <T> element type
+ */
+public class BoundedIterator<T> implements Iterator<T> {
+ private final Iterator<T> iterator;
+ private final long offset;
+ private final long max;
+ private int pos;
+ private T next;
+
+ /**
+ * Create a new bounded iterator with a given offset and maximum
+ *
+ * @param offset offset to start iteration at. Must be non negative
+ * @param max maximum elements this iterator should return. Set to -1 for all
+ * @param iterator the underlying iterator
+ * @throws IllegalArgumentException if offset is negative
+ */
+ public BoundedIterator(long offset, long max, Iterator<T> iterator) {
+ if (offset < 0) {
+ throw new IllegalArgumentException("Offset must not be negative");
+ }
+
+ this.iterator = iterator;
+ this.offset = offset;
+ this.max = max;
+ }
+
+ /**
+ * Factory for creating a bounded iterator.
+ * @see #BoundedIterator(long, long, java.util.Iterator)
+ *
+ * @param offset offset to start iteration at. Must be non negative
+ * @param max maximum elements this iterator should return. Set to -1 for all
+ * @param iterator the underlying iterator
+ * @param <T> element type
+ * @return an iterator which only returns the elements in the given bounds
+ */
+ public static <T> Iterator<T> create(long offset, long max, Iterator<T> iterator) {
+ if (offset == 0 && max == -1) {
+ return iterator;
+ }
+ else {
+ return new BoundedIterator<T>(offset, max, iterator);
+ }
+ }
+
+ public boolean hasNext() {
+ if (next == null) {
+ fetchNext();
+ }
+
+ return next != null;
+ }
+
+ public T next() {
+ if (!hasNext()) {
+ throw new NoSuchElementException();
+ }
+
+ return consumeNext();
+ }
+
+ public void remove() {
+ throw new UnsupportedOperationException();
+ }
+
+ //------------------------------------------< private >---
+
+ private void fetchNext() {
+ for (; pos < offset && iterator.hasNext(); pos++) {
+ next = iterator.next();
+ }
+
+ if (pos < offset || !iterator.hasNext() || max >= 0 && pos - offset + 1 > max) {
+ next = null;
+ }
+ else {
+ next = iterator.next();
+ pos++;
+ }
+
+ }
+
+ private T consumeNext() {
+ T element = next;
+ next = null;
+ return element;
+ }
+}