nfsantos commented on code in PR #1094:
URL: https://github.com/apache/jackrabbit-oak/pull/1094#discussion_r1329183290
##########
oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/ResultSizeTest.java:
##########
@@ -126,5 +151,90 @@ private void doTestResultSize(boolean union, int expected)
throws RepositoryExce
System.clearProperty("oak.fastQuerySize");
}
+
+ private void doTestResultSizeOption(boolean aggregateAtQueryTime) throws
RepositoryException {
+ createData();
+ int expectedForUnion = 400;
+ int expectedForTwoConditions = aggregateAtQueryTime ? 400 : 200;
+ doTestResultSizeOption(superuser, false, expectedForTwoConditions);
+ doTestResultSizeOption(superuser, true, expectedForUnion);
+ Session readOnlySession = null;
+ try {
+ readOnlySession = getHelper().getReadOnlySession();
+ assertNotNull(readOnlySession);
+ doTestResultSizeOption(readOnlySession, false, -1);
+ doTestResultSizeOption(readOnlySession, true, -1);
+ } finally {
+ if (readOnlySession != null) {
+ readOnlySession.logout();
+ }
+ }
+ }
+
+ private void doTestResultSizeOption(Session session, boolean union, int
expected) throws RepositoryException {
+ QueryManager qm = session.getWorkspace().getQueryManager();
+
+ String statement;
+ if (union) {
+ statement = "select a.[jcr:path] from [nt:base] as a where
contains(a.[text], 'Hello') UNION select a.[jcr:path] from [nt:base] as a where
contains(a.[text], 'World')";
+ } else {
+ statement = "select a.[jcr:path] from [nt:base] as a where
contains(a.[text], 'Hello World')";
+ }
+
+ Query q;
+ long result;
+ NodeIterator it;
+ StringBuilder buff;
+
+ // enabled by default now, in LuceneOakRepositoryStub. Disable global
+ System.setProperty("oak.fastQuerySize", "false");
+
+ // fast (insecure) case
+ String fastSizeResult = "";
+ q = qm.createQuery(statement + " option (insecure result size)",
Query.JCR_SQL2);
+ if (expected < 0) {
+ // if expected < 0, i.e. insufficient permissions, expect a
InvalidQueryException on execute().
+ try {
+ it = q.execute().getNodes();
+ fail("expected an InvalidQueryException caused by a
IllegalArgumentException");
+ } catch (InvalidQueryException e) {
+ assertTrue("expected an InvalidQueryException caused by a
ParseException",
+ e.getCause() instanceof IllegalArgumentException);
+ }
+ } else {
+ it = q.execute().getNodes();
+ result = it.getSize();
+ assertTrue("size: " + result + " expected around " + expected,
+ result > expected - 50 &&
+ result < expected + 50);
+ buff = new StringBuilder();
+ while (it.hasNext()) {
+ Node n = it.nextNode();
+ buff.append(n.getPath()).append('\n');
+ }
+ fastSizeResult = buff.toString();
+ q = qm.createQuery(statement + " option (insecure result size)",
Query.JCR_SQL2);
+ q.setLimit(90);
+ it = q.execute().getNodes();
+ assertEquals(90, it.getSize());
+ }
+
+ // default (secure) case
+ q = qm.createQuery(statement, Query.JCR_SQL2);
+ it = q.execute().getNodes();
+ result = it.getSize();
+ assertEquals(-1, result);
+ buff = new StringBuilder();
+ while (it.hasNext()) {
+ Node n = it.nextNode();
+ buff.append(n.getPath()).append('\n');
+ }
+ String regularResult = buff.toString();
+ if (expected >= 0) {
+ assertEquals(regularResult, fastSizeResult);
+ }
+
+ System.clearProperty("oak.fastQuerySize");
Review Comment:
I did not look at the PR in detail, I just made a comment in case you were
not familiar with the RestoreSystemProperties rule (I was not until recently
and discovering it simplified my tests). So I don't feel ready to either
approve or to make other comments about the PR. It's fine to use finally to
clear the system properties. I'd say to go ahead, you already have two
approvals.
The only suggestion I have after a very quick look is to avoid repeating the
string literal "oak.fastQuerySize". Better to have a constant defined
somewhere, to avoid copy and paste errors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
