Current state of SSO or OpenID Connect support in Sling

2019-05-09 Thread Gaston Gonzalez
Hi All,

I have been researching an SSO solution for Sling for the last week and noticed 
that some work has been done around OpenID Connect. During my research I 
stumbled upon SLING-2759 and was able to get it working with Sling 11 using a 
couple of OpenID providers (e.g., Google Identity Platform and Auth0). This 
ticket has been stale since August 2018 and I was wondering if I can help 
contribute to the development of this feature. I searched the Sling dev and 
user mailing list archives and can’t seem to find any work that would supersede 
SLING-2759. 

Is SLING-2759 still the front runner for supporting Open ID Connect? 
Is there a better option on the table for supporting SSO in Sling?

I also stumbled upon an adaptTo() 2018 talk, "Modern Authentication in Sling 
with OpenID Connect and Keycloak” (https://www.youtube.com/watch?v=aaqpmmyylis 
) that seems to suggest that there 
is some interest in OpenID Connect + Sling.

Any thoughts would be appreciated.

Thanks,

Gaston Gonzalez
Senior Architect | www.headwire.com





[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Jason E Bailey (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836568#comment-16836568
 ] 

Jason E Bailey commented on SLING-8407:
---

If I remember correctly, jobs are stored in a specific location. I don't think 
this is a case where a query is even needed. We have utilities to filter and 
select child nodes from a path which utilizes a tree traversal, which is not 
the same as an oak traversal. Implementing a filter like that would prevent 
this issue and also make it independent of the type of repository that 
underlies the data store. Without sacrificing speed or efficiency.

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Closed] (SLING-8398) sling-mock: Make MockMimeTypeService compatible with org.apache.sling.commons.mime 2.2.0

2019-05-09 Thread Stefan Seifert (JIRA)


 [ 
https://issues.apache.org/jira/browse/SLING-8398?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Stefan Seifert closed SLING-8398.
-

> sling-mock: Make MockMimeTypeService compatible with 
> org.apache.sling.commons.mime 2.2.0
> 
>
> Key: SLING-8398
> URL: https://issues.apache.org/jira/browse/SLING-8398
> Project: Sling
>  Issue Type: Improvement
>  Components: Testing
>Affects Versions: Testing Sling Mock 2.3.8
>Reporter: Stefan Seifert
>Assignee: Stefan Seifert
>Priority: Major
> Fix For: Testing Sling Mock 2.3.10
>
>
> The signature of the activate method has changed again with release 2.2.0 or 
> the mime type service implementation, and it's no longer using the log 
> service.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[RESULT] [VOTE] Release Apache Sling Testing Sling Mock 2.3.10

2019-05-09 Thread Stefan Seifert
Hi,

The vote has passed with the following result :

+1 (binding): Stefan Seifert, Robert Munteanu, Radu Cotescu, Daniel Klco
+1 (non binding): Timothee Maret, Andreas Schaefer

I will copy this release to the Sling dist directory and
promote the artifacts to the central Maven repository.

stefan



Re: [VOTE] Release Apache Sling Testing Sling Mock 2.3.10

2019-05-09 Thread Andreas Schaefer
+1 (not binding)

- Andy

> On May 8, 2019, at 3:37 PM, Daniel Klco  wrote:
> 
> +1
> 
> On Wed, May 8, 2019 at 9:59 AM Radu Cotescu  wrote:
> 
>> +1
>> 
>>> On 6 May 2019, at 18:37, Stefan Seifert  wrote:
>>> 
>>> Please vote to approve this release:
>>> 
>>> [ ] +1 Approve the release
>>> [ ]  0 Don't care
>>> [ ] -1 Don't release, because ...
>>> 
>>> This majority vote is open for at least 72 hours.
>> 
>> 



[jira] [Commented] (SLING-8411) Provide a way to bifurcat a repository path to a provider mount

2019-05-09 Thread Karl Pauls (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836447#comment-16836447
 ] 

Karl Pauls commented on SLING-8411:
---

The provided pull request implements this by introducing the concept of a 
RepositoryMount. In short, the base will look for RepositoryMount services 
published with a RepositoryMount.MOUNT_POINTS_KEY service property (for the 
subpaths to take over). While one is present, it will wrap the repositories 
created with a wrapper that will bifurcate requests to the actual repository 
and the mount according to the mount points.

[~cziegeler], does that look good to you?

> Provide a way to bifurcat a repository path to a provider mount
> ---
>
> Key: SLING-8411
> URL: https://issues.apache.org/jira/browse/SLING-8411
> Project: Sling
>  Issue Type: Improvement
>  Components: JCR
>Affects Versions: JCR Base 3.0.6
>Reporter: Karl Pauls
>Assignee: Karl Pauls
>Priority: Major
> Fix For: JCR Base 3.0.8
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> While one would normally use a resource provider to extend the content tree, 
> sometimes, that can be not enough a code might make assumptions about the 
> resources being actually being backed by JCR. This can be problematic as for 
> example, a given resource can not necessarily be adapted to a Node nor be 
> found via a Session.
> Hence, a similar approach on the JCR level allowing developers to mount 
> content into sub trees of the repository itself can be helpful to be able to 
> support these kind of usecase. We should provide a hook for a repository 
> mount that takes over a given subpath by wrapping repositories returned by 
> the base and dispatching to the mount when the request is for a given subpath.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Created] (SLING-8411) Provide a way to bifurcat a repository path to a provider mount

2019-05-09 Thread Karl Pauls (JIRA)
Karl Pauls created SLING-8411:
-

 Summary: Provide a way to bifurcat a repository path to a provider 
mount
 Key: SLING-8411
 URL: https://issues.apache.org/jira/browse/SLING-8411
 Project: Sling
  Issue Type: Improvement
  Components: JCR
Affects Versions: JCR Base 3.0.6
Reporter: Karl Pauls
Assignee: Karl Pauls
 Fix For: JCR Base 3.0.8


While one would normally use a resource provider to extend the content tree, 
sometimes, that can be not enough a code might make assumptions about the 
resources being actually being backed by JCR. This can be problematic as for 
example, a given resource can not necessarily be adapted to a Node nor be found 
via a Session.

Hence, a similar approach on the JCR level allowing developers to mount content 
into sub trees of the repository itself can be helpful to be able to support 
these kind of usecase. We should provide a hook for a repository mount that 
takes over a given subpath by wrapping repositories returned by the base and 
dispatching to the mount when the request is for a given subpath.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Carsten Ziegeler (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836445#comment-16836445
 ] 

Carsten Ziegeler commented on SLING-8407:
-

The query is not provided by the caller of the API, it is constructed in the 
impl, therefore there is no point in throwing an exception if the query is not 
correct back to he caller. It's not the fault of the caller. Therefore it is 
ignored. A comment in the code sounds like a good idea

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Comment Edited] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836442#comment-16836442
 ] 

Thomas Mueller edited comment on SLING-8407 at 5/9/19 2:43 PM:
---

{noformat}
 } catch (final QuerySyntaxException qse) {
String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
if (configuration.isFailTraversal() {
// no index is available, or (unlikely), query syntax
logger.debug(message, qse);
logger.info(message);
} else {
// query syntax
logger.warn(message, qse);
}
// re-throw in any case
throw new IllegalStateException(message);
 } finally {
{noformat}



was (Author: tmueller):
 } catch (final QuerySyntaxException qse) {
String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
if (configuration.isFailTraversal() {
// no index is available, or (unlikely), query syntax
logger.debug(message, qse);
logger.info(message);
} else {
// query syntax
logger.warn(message, qse);
}
// re-throw in any case
throw new IllegalStateException(message);
 } finally {

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836442#comment-16836442
 ] 

Thomas Mueller commented on SLING-8407:
---

 } catch (final QuerySyntaxException qse) {
String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
if (configuration.isFailTraversal() {
// no index is available, or (unlikely), query syntax
logger.debug(message, qse);
logger.info(message);
} else {
// query syntax
logger.warn(message, qse);
}
// re-throw in any case
throw new IllegalStateException(message);
 } finally {

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836441#comment-16836441
 ] 

Thomas Mueller commented on SLING-8407:
---

[~egli] yes, or we could use a different configuration option for this case? I 
think throwing an exception (versus just logging) is much better if the query 
syntax is wrong. Actually I think it was an oversight to swallow the exception 
(meaning, not re-throw). In case it was intentional, I would expect there is a 
comment in the source code (similar to "fall through" comments in switch / 
case),... [~cziegeler] what do you think? I assume the code is very old so you 
will probably not remember this exact case, but given that you likely wrote the 
code, was that also your way of thinking?

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Stefan Egli (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836416#comment-16836416
 ] 

Stefan Egli commented on SLING-8407:


> {{if (configuration.isFailTraversal()) {}}

yes, that's how I envisioned it.

However, I'd argue that throwing the RuntimeException could also depend on that 
same condition, so something like
{noformat}
 } catch (final QuerySyntaxException qse) {
if (configuration.isFailTraversal() {
// no index is available
String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
logger.debug(message, qse);
logger.info(message);
throw new IllegalStateException();
} else {
logger.warn("Query syntax wrong " + buf.toString(), qse);
}
 } finally {
{noformat}

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836410#comment-16836410
 ] 

Thomas Mueller commented on SLING-8407:
---

Regarding the configuration option would this patch work? The option is enabled 
by default (given that the same option is used in SLING-7544, and there are no 
tests for older versions of Oak, I think the point of supporting older Oak 
version is moot.)

{noformat}
--- 
a/src/main/java/org/apache/sling/event/impl/jobs/config/JobManagerConfiguration.java
+++ 
b/src/main/java/org/apache/sling/event/impl/jobs/config/JobManagerConfiguration.java
@@ -117,6 +117,9 @@ public class JobManagerConfiguration {
 /** Configuration property for the scheduled jobs path. */
 public static final String PROPERTY_SCHEDULED_JOBS_PATH = 
"job.scheduled.jobs.path";
 
+/** Configuration property to fail traversal queries. */
+public static final String PROPERTY_FAIL_TRAVERSAL = "job.failTraversal";
+
 /** The jobs base path with a slash. */
 private String jobsBasePathWithSlash;
 
@@ -178,6 +181,8 @@ public class JobManagerConfiguration {
 /** The topology capabilities. */
 private volatile TopologyCapabilities topologyCapabilities;
 
+private boolean failTraversal;
+
 /**
  * Activate this component.
  * @param props Configuration properties
@@ -207,6 +212,8 @@ public class JobManagerConfiguration {
 DEFAULT_SCHEDULED_JOBS_PATH);
 this.scheduledJobsPathWithSlash = this.scheduledJobsPath + "/";
 
+this.failTraversal = 
PropertiesUtil.toBoolean(props.get(PROPERTY_FAIL_TRAVERSAL), true);
+
 this.historyCleanUpRemovedJobs = config.cleanup_period();
 
 // create initial resources
@@ -460,6 +467,15 @@ public class JobManagerConfiguration {
 return (slash ? this.scheduledJobsPathWithSlash : 
this.scheduledJobsPath);
 }
 
+/**
+ * Whether to fail traversal when querying for jobs.
+ *
+ * @return true if enabled
+ */
+public boolean isFailTraversal() {
+return failTraversal;
+}
+
 /**
  * Stop processing
  */

--- a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
+++ b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
@@ -520,6 +520,9 @@ public class JobManagerImpl
 buf.append(Job.PROPERTY_JOB_CREATED);
 buf.append(" ascending");
 }
+if (configuration.isFailTraversal()) {
+buf.append(" option(traversal fail)");
+}
 final Iterator iter = 
resolver.findResources(buf.toString(), "xpath");
 long count = 0;
 
@@ -535,7 +538,11 @@ public class JobManagerImpl
 }
  }
 } catch (final QuerySyntaxException qse) {
-logger.warn("Query syntax wrong " + buf.toString(), qse);
+// no index is available
+String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
+logger.debug(message, qse);
+logger.info(message);
+throw new IllegalStateException();
 } finally {
 resolver.close();
 }
{noformat}

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836389#comment-16836389
 ] 

Thomas Mueller commented on SLING-8407:
---

[~egli] how should the feature flag be implemented, assuming we do want to only 
use "buf.append(" option(traversal fail)");" if enabled? Would it be similar to 
how it is implemented in SLING-7544?:

{noformat}
if (this.factory.isForceNoAliasTraversal()) { queryString += " 
option(traversal fail);  }
{noformat}

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836383#comment-16836383
 ] 

Thomas Mueller commented on SLING-8407:
---

> why not delay the health checks until the repo is initialized?

[~cziegeler] I don't know of a way :-)

Even if we do delay (assuming you know of a way), then there is still the 
problem of corrupt / unavailable / deleted / disabled indexes, which right now 
are not properly handled. With the patch, they are.

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Carsten Ziegeler (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836379#comment-16836379
 ] 

Carsten Ziegeler commented on SLING-8407:
-

so we are talking about repository initialization - why not delay the health 
checks until the repo is initialized?

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Comment Edited] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836296#comment-16836296
 ] 

Thomas Mueller edited comment on SLING-8407 at 5/9/19 1:17 PM:
---

Patch for sling-org-apache-sling-event:

{noformat}
diff --git a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java 
b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
index 279d5ca..7faf515 100644
--- a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
+++ b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
@@ -520,6 +520,7 @@ public class JobManagerImpl
 buf.append(Job.PROPERTY_JOB_CREATED);
 buf.append(" ascending");
 }
+buf.append(" option(traversal fail)");
 final Iterator iter = 
resolver.findResources(buf.toString(), "xpath");
 long count = 0;
 
@@ -535,7 +536,11 @@ public class JobManagerImpl
 }
  }
 } catch (final QuerySyntaxException qse) {
-logger.warn("Query syntax wrong " + buf.toString(), qse);
+// no index is available
+String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
+logger.debug(message, qse);
+logger.info(message);
+throw new IllegalStateException(message);
 } finally {
 resolver.close();
 }
diff --git 
a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java 
b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
index c51ecd9..42af6e8 100644
--- a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
+++ b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
@@ -98,8 +98,8 @@ public abstract class AbstractJobHandlingTest {
 
 String localRepo = System.getProperty("maven.repo.local", "");
 
-final String jackrabbitVersion = "2.13.1";
-final String oakVersion = "1.5.7";
+final String jackrabbitVersion = "2.14.6";
+final String oakVersion = "1.6.0";
 
 final String slingHome = new File(buildDir + File.separatorChar + 
"sling_" + System.currentTimeMillis()).getAbsolutePath();
 
@@ -230,6 +230,7 @@ public abstract class AbstractJobHandlingTest {
 
 mavenBundle("com.google.guava", "guava", "15.0"),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-api", 
jackrabbitVersion),
+mavenBundle("org.apache.jackrabbit", "jackrabbit-data", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-jcr-commons", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-spi", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-spi-commons", 
jackrabbitVersion),
{noformat}

* Added "option(traversal fail)".
* If there is no index, then QuerySyntaxException is thrown (Sling converts all 
kinds of query exceptions to QuerySyntaxException, even if the syntax is fine - 
this is slightly confusing, but fixing that would be a backward compatibility 
problem).
* This exception is logged at info level, and then converted to 
IllegalStateException (a RuntimeException). So if there is no index, the 
error.log will show what's going on - just that this is no longer a warning, 
just an information.
* The AbstractJobHandlingTest now uses a stable Oak version instead of an 
unstable one as before. This is needed because "option(traversal fail)" was 
implemented slightly after Oak 1.5.7.
* The Jackrabbit version is updated as well, as that's the version needed by 
the newer Oak version
* Adding "jackrabbit-data" is needed as that was added slightly after Oak 1.5.7 
(part of modularization).


was (Author: tmueller):
Patch for sling-org-apache-sling-event:

{noformat}
diff --git a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java 
b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
index 279d5ca..7faf515 100644
--- a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
+++ b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
@@ -520,6 +520,7 @@ public class JobManagerImpl
 buf.append(Job.PROPERTY_JOB_CREATED);
 buf.append(" ascending");
 }
+buf.append(" option(traversal fail)");
 final Iterator iter = 
resolver.findResources(buf.toString(), "xpath");
 long count = 0;
 
@@ -535,7 +536,11 @@ public class JobManagerImpl
 }
  }
 } catch (final QuerySyntaxException qse) {
-logger.warn("Query syntax wrong " + buf.toString(), qse);
+// no index is available
+String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
+logger.debug(message, qse);
+  

[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836367#comment-16836367
 ] 

Thomas Mueller commented on SLING-8407:
---

> How do you know that no one else is calling that api?

Do you mean JobManager.findJobs? It is called by health checks (e.g. 
SLING-8408). Health checks are run early on, even during repository 
initialization. I'm not aware of other code that is calling this method during 
repository initialization. I would argue if there is such code, then that 
should also fail, until the repository is initialized.

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836363#comment-16836363
 ] 

Thomas Mueller commented on SLING-8407:
---

[~egli] looks like you found a bug in the JobManagerImpl, here:

{noformat}
buf.append(" '");
buf.append(current.getValue());
buf.append("'");
{noformat}

The second line is not escaping the value. Single quotes in the value need to 
be repeated, or a bind variable needs to be used.

Right now, the problem is (almost) silently ignored here:

{noformat}
} catch (final QuerySyntaxException qse) {
logger.warn("Query syntax wrong " + buf.toString(), qse);
} finally {
{noformat}

My change would throw an IllegalStateException for this case, which is arguably 
much better than ignoring the problem. The proper solution is of course to fix 
the code above.

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Carsten Ziegeler (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836361#comment-16836361
 ] 

Carsten Ziegeler commented on SLING-8407:
-

How do you know that no one else is calling that api?

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836358#comment-16836358
 ] 

Thomas Mueller commented on SLING-8407:
---

> It seems Oak is not ready without the index as queries can't be executed.

Let's put it that way: queries _should_ not be executed if no index is 
available. Right now, they are executed, it's just that in this case a warning 
is logged by Oak. There are multiple reasons an index is not available:

* (A) during the very first startup, the index isn't ready yet
* (B) if the index is corrupt or is not available (for remote indexes)
* (C) if the index was removed or disabled (usually due to user error)

> So we should probably block everything until the index is there.

Block everything, in my view, is the wrong solution. That would mean no part of 
the system would work (or could start up) just because one index is not 
available / available yet.

The solution I propose is to fail exactly that part of the system that needs 
the index. In this case, it affects the health check.

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Carsten Ziegeler (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836349#comment-16836349
 ] 

Carsten Ziegeler commented on SLING-8407:
-

I don't agree. It seems Oak is not ready without the index as queries can't be 
executed. So we should probably block everything until the index is there. If 
we fix it here, we have to fix every potential upstream user of this API.

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836354#comment-16836354
 ] 

Thomas Mueller commented on SLING-8407:
---

> adding option(traversal fail) by default but only if a new config option is 
> set (false by default)? 

[~egli] yes that would work. 

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Stefan Egli (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836348#comment-16836348
 ] 

Stefan Egli commented on SLING-8407:


I think we should not throw the RuntimeException due to backwards 
compatibility. We already have many cases where WARNs are logged which would 
now result in a RuntimeException (something to look at separately perhaps).

Here's an example of the exception:
{noformat}
09.05.2019 04:47:34.462 *WARN* | xxx | | [10.43.32.53 [1557370054384] POST 
/libs/wcm/core/content/reference.json HTTP/1.1] 
org.apache.sling.event.impl.jobs.JobManagerImpl : Query syntax wrong 
/jcr:root/var/eventing/jobs//element(*,slingevent:Job)[@event.job.topic = 'xxx' 
and not(@slingevent:finishedState) and ((@cq:path = '/a/b/c/d'f/my.jpg'))] 
order by @slingevent:created ascending 
org.apache.sling.api.resource.QuerySyntaxException: java.text.ParseException: 
Query: /jcr:root/var/eventing/jobs//element(*,slingevent:Job)[@event.job.topic 
= 'xxx' and not(@slingevent:finishedState) and ((@cq:path = 
'/a/b/c/d'f/my.jpg(*)'))] order by @slingevent:created ascending
{noformat}

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836333#comment-16836333
 ] 

Thomas Mueller commented on SLING-8407:
---

[~cziegeler] longer term, we want to enable "traversal fail" in general (for 
all queries) in Oak. That is, require queries to have an index.

However, the current behavior of the Sling JobManager is problematic: it 
catches this exception (which is converted to a QuerySyntaxException, even 
thought the query syntax is just fine), and basically ignores it (only logs it 
as a warning). This behavior is problematic in any case. Instead, the 
JobManager should throw such exceptions.

My patch fixes that.

> makes it impossible to use this implementation in older versions.

Right now, Sling runs tests against Oak 1.5.7. If you want to support older 
versions of Oak, then there should be tests for older versions as well.

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8408) DistributionQueueHealthCheck should deal with failing queries

2019-05-09 Thread Timothee Maret (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8408?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836334#comment-16836334
 ] 

Timothee Maret commented on SLING-8408:
---

[~tmueller] I suggest to open a PR, thanks.

> DistributionQueueHealthCheck should deal with failing queries
> -
>
> Key: SLING-8408
> URL: https://issues.apache.org/jira/browse/SLING-8408
> Project: Sling
>  Issue Type: Improvement
>  Components: Content Distribution
>Reporter: Thomas Mueller
>Priority: Major
>
> The following health check indirectly runs a queries which might fail:
>  * 
> [DistributionQueueHealthCheck|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/monitor/DistributionQueueHealthCheck.java]:
>  
> sling-org-apache-sling-distribution-core/src/main/java/org/apache/sling/distribution/monitor
> The call 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373],
>  which can throw an exception with SLING-8407, if the index is not yet 
> available. The health checks should catch this exception and return 
> HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Stefan Egli (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836332#comment-16836332
 ] 

Stefan Egli commented on SLING-8407:


Could we keep the backwards compatibility by not adding {{option(traversal 
fail)}} by default but only if a new config option is set (false by default)? 
We can then enable this in deployments where we think we need this.
/cc [~tmueller], [~cziegeler]

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836322#comment-16836322
 ] 

Thomas Mueller commented on SLING-8407:
---

[~cziegeler]

> I'm not sure if we really should go this way. It adds a dependency to some 
> Oak version which makes it impossible to use this implementation in older 
> versions.

Yes, the "option(traversal fail)" is not yet supported in older Oak version 
(1.0, 1.2, 1.4). I'm fine to add support for this there: this syntax would be 
supported, but ignored (which is fine).

> But more important it sounds like a workaround to me, It seems because the 
> health checks start too early, 

Well, there is no other API I can think of to verify at what point exactly 
indexing is done. So there is no way to ensure health checks don't start too 
early. If you have a better idea, then please tell me.

> you want to add a workaround here and there. Why not fix the root problem?

In my view, the root cause of the problem is in fact that the queries, that 
require an index, don't currently ensure that the index is available. So in 
fact we are fixing the root cause here. See also the related Oak issue 
[OAK-5889|https://issues.apache.org/jira/browse/OAK-5889].

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836304#comment-16836304
 ] 

Thomas Mueller commented on SLING-8407:
---

[~egli] could you review please, or let me know who should? (I'm not a Sling 
committer.)

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8408) DistributionQueueHealthCheck should deal with failing queries

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8408?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836305#comment-16836305
 ] 

Thomas Mueller commented on SLING-8408:
---

[~teofili] could you review please, or let me know who should? (I'm not a Sling 
committer.)

> DistributionQueueHealthCheck should deal with failing queries
> -
>
> Key: SLING-8408
> URL: https://issues.apache.org/jira/browse/SLING-8408
> Project: Sling
>  Issue Type: Improvement
>  Components: Content Distribution
>Reporter: Thomas Mueller
>Priority: Major
>
> The following health check indirectly runs a queries which might fail:
>  * 
> [DistributionQueueHealthCheck|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/monitor/DistributionQueueHealthCheck.java]:
>  
> sling-org-apache-sling-distribution-core/src/main/java/org/apache/sling/distribution/monitor
> The call 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373],
>  which can throw an exception with SLING-8407, if the index is not yet 
> available. The health checks should catch this exception and return 
> HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8408) DistributionQueueHealthCheck should deal with failing queries

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8408?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836298#comment-16836298
 ] 

Thomas Mueller commented on SLING-8408:
---

Patch for sling-org-apache-sling-distribution-cor:

{noformat}
diff --git 
a/src/main/java/org/apache/sling/distribution/monitor/DistributionQueueHealthCheck.java
 
b/src/main/java/org/apache/sling/distribution/monitor/DistributionQueueHealthCheck.java
index 38bf41e..caffc0d 100644
--- 
a/src/main/java/org/apache/sling/distribution/monitor/DistributionQueueHealthCheck.java
+++ 
b/src/main/java/org/apache/sling/distribution/monitor/DistributionQueueHealthCheck.java
@@ -124,8 +124,9 @@ public class DistributionQueueHealthCheck implements 
HealthCheck {
 } else {
 resultLog.debug("No items in queue [{}]", 
q.getName());
 }
-
-} catch (Exception e) {
+} catch (IllegalStateException e) {
+   resultLog.healthCheckError("The job index is not 
available (just yet) while inspecting replication agent [{}]", queueName);
+   } catch (Exception e) {
 resultLog.warn("Exception while inspecting 
distribution queue [{}]: {}", queueName, e);
 }
 }
{noformat}

* Catching IllegalStateException as that's what is thrown by SLING-8407 for the 
case where no index is available.
* Report this as a health check error: it means the index is not available, 
which can happen at the very first startup, or it could happen later on, if 
someone would remove the index. In both cases, the system is not in a good 
state, so reporting an error is appropriate. I would expect nobody monitors the 
health checks during the very first startup (where the repository is 
initialized), but I argue during that time the system is in fact not available.


> DistributionQueueHealthCheck should deal with failing queries
> -
>
> Key: SLING-8408
> URL: https://issues.apache.org/jira/browse/SLING-8408
> Project: Sling
>  Issue Type: Improvement
>  Components: Content Distribution
>Reporter: Thomas Mueller
>Priority: Major
>
> The following health check indirectly runs a queries which might fail:
>  * 
> [DistributionQueueHealthCheck|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/monitor/DistributionQueueHealthCheck.java]:
>  
> sling-org-apache-sling-distribution-core/src/main/java/org/apache/sling/distribution/monitor
> The call 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373],
>  which can throw an exception with SLING-8407, if the index is not yet 
> available. The health checks should catch this exception and return 
> HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836296#comment-16836296
 ] 

Thomas Mueller commented on SLING-8407:
---

Patch for sling-org-apache-sling-event:

{noformat}
diff --git a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java 
b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
index 279d5ca..7faf515 100644
--- a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
+++ b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
@@ -520,6 +520,7 @@ public class JobManagerImpl
 buf.append(Job.PROPERTY_JOB_CREATED);
 buf.append(" ascending");
 }
+buf.append(" option(traversal fail)");
 final Iterator iter = 
resolver.findResources(buf.toString(), "xpath");
 long count = 0;
 
@@ -535,7 +536,11 @@ public class JobManagerImpl
 }
  }
 } catch (final QuerySyntaxException qse) {
-logger.warn("Query syntax wrong " + buf.toString(), qse);
+// no index is available
+String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
+logger.debug(message, qse);
+logger.info(message);
+throw new IllegalStateException();
 } finally {
 resolver.close();
 }
diff --git 
a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java 
b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
index c51ecd9..42af6e8 100644
--- a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
+++ b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
@@ -98,8 +98,8 @@ public abstract class AbstractJobHandlingTest {
 
 String localRepo = System.getProperty("maven.repo.local", "");
 
-final String jackrabbitVersion = "2.13.1";
-final String oakVersion = "1.5.7";
+final String jackrabbitVersion = "2.14.6";
+final String oakVersion = "1.6.0";
 
 final String slingHome = new File(buildDir + File.separatorChar + 
"sling_" + System.currentTimeMillis()).getAbsolutePath();
 
@@ -230,6 +230,7 @@ public abstract class AbstractJobHandlingTest {
 
 mavenBundle("com.google.guava", "guava", "15.0"),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-api", 
jackrabbitVersion),
+mavenBundle("org.apache.jackrabbit", "jackrabbit-data", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-jcr-commons", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-spi", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-spi-commons", 
jackrabbitVersion),
{noformat}

* Added "option(traversal fail)".
* If there is no index, then QuerySyntaxException is thrown (Sling converts all 
kinds of query exceptions to QuerySyntaxException, even if the syntax is fine - 
this is slightly confusing, but fixing that would be a backward compatibility 
problem).
* This exception is logged at info level, and then converted to 
IllegalStateException (a RuntimeException). So if there is no index, the 
error.log will show what's going on - just that this is no longer a warning, 
just an information.
* The AbstractJobHandlingTest now uses a stable Oak version instead of an 
unstable one as before. This is needed because "option(traversal fail)" was 
implemented slightly after Oak 1.5.7.
* The Jackrabbit version is updated as well, as that's the version needed by 
the newer Oak version
* Adding "jackrabbit-data" is needed as that was added slightly after Oak 1.5.7 
(part of modularization).

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't 

[jira] [Commented] (SLING-8391) Add support for execution modes

2019-05-09 Thread Radu Cotescu (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836286#comment-16836286
 ] 

Radu Cotescu commented on SLING-8391:
-

Switched to {{master}} with [commit 
4371338|https://github.com/apache/sling-org-apache-sling-committer-cli/commit/4371338].

> Add support for execution modes
> ---
>
> Key: SLING-8391
> URL: https://issues.apache.org/jira/browse/SLING-8391
> Project: Sling
>  Issue Type: New Feature
>  Components: Tooling
>Reporter: Radu Cotescu
>Assignee: Radu Cotescu
>Priority: Major
> Fix For: Committer CLI 1.0.0
>
>
> The Commiter CLI tool should provide support for the following execution 
> modes:
>  # dry-run - default mode, which only lists the output of a command
>  # interactive - lists command output, but based on user-input can also 
> execute actions on behalf of the user (e.g. send email, release artifacts, 
> update site, etc.)
>  # auto - runs the command, assuming the default actions / answers



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (SLING-8410) Add analyser to scan for potential problems when using Apache Felix Connect

2019-05-09 Thread Carsten Ziegeler (JIRA)


 [ 
https://issues.apache.org/jira/browse/SLING-8410?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Carsten Ziegeler resolved SLING-8410.
-
Resolution: Fixed

Added new analyser in rev efb1675 which checks the above and produces warnings

> Add analyser to scan for potential problems when using Apache Felix Connect
> ---
>
> Key: SLING-8410
> URL: https://issues.apache.org/jira/browse/SLING-8410
> Project: Sling
>  Issue Type: New Feature
>  Components: Feature Model
>Reporter: Carsten Ziegeler
>Assignee: Carsten Ziegeler
>Priority: Major
> Fix For: Feature Model Analyser 1.0.2
>
>
> When running in Apache Felix Connect, the bundles are all put on a shared 
> classloader. There are potentially two problems here:
> - if a bundle embeds other jars which are part of the bundle class path, this 
> requires to exband the bundle
> - if there is more than a bundle for a package (like for private packages), 
> then these packages are merged on the classpath and the first class found wins



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Created] (SLING-8410) Add analyser to scan for potential problems when using Apache Felix Connect

2019-05-09 Thread Carsten Ziegeler (JIRA)
Carsten Ziegeler created SLING-8410:
---

 Summary: Add analyser to scan for potential problems when using 
Apache Felix Connect
 Key: SLING-8410
 URL: https://issues.apache.org/jira/browse/SLING-8410
 Project: Sling
  Issue Type: New Feature
  Components: Feature Model
Reporter: Carsten Ziegeler
Assignee: Carsten Ziegeler
 Fix For: Feature Model Analyser 1.0.2


When running in Apache Felix Connect, the bundles are all put on a shared 
classloader. There are potentially two problems here:
- if a bundle embeds other jars which are part of the bundle class path, this 
requires to exband the bundle
- if there is more than a bundle for a package (like for private packages), 
then these packages are merged on the classpath and the first class found wins



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836160#comment-16836160
 ] 

Thomas Mueller commented on SLING-8407:
---

The Oak version change (and subsequent Jackrabbit version change, plus addition 
of jackrabbit-data) will update Oak to a stable version.

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SLING-8407) JobManagerImpl.findJobs should prevent traversal

2019-05-09 Thread Thomas Mueller (JIRA)


[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836157#comment-16836157
 ] 

Thomas Mueller commented on SLING-8407:
---

Here a first patch that passes the tests. I will try to throw a special kind of 
RuntimeException instead of returning an empty list / null:

{noformat}
--- a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
+++ b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
@@ -520,6 +520,7 @@ public class JobManagerImpl
 buf.append(Job.PROPERTY_JOB_CREATED);
 buf.append(" ascending");
 }
+buf.append(" option(traversal fail)");
 final Iterator iter = 
resolver.findResources(buf.toString(), "xpath");
 long count = 0;
 
@@ -535,7 +536,10 @@ public class JobManagerImpl
 }
  }
 } catch (final QuerySyntaxException qse) {
-logger.warn("Query syntax wrong " + buf.toString(), qse);
+// no index is available
+String message = "Query " + buf.toString() + " failed: " + 
qse.getMessage();
+logger.debug(message, qse);
+logger.info(message, qse);
 } finally {
 resolver.close();
 }
diff --git 
a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java 
b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
index c51ecd9..42af6e8 100644
--- a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
+++ b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
@@ -98,8 +98,8 @@ public abstract class AbstractJobHandlingTest {
 
 String localRepo = System.getProperty("maven.repo.local", "");
 
-final String jackrabbitVersion = "2.13.1";
-final String oakVersion = "1.5.7";
+final String jackrabbitVersion = "2.14.6";
+final String oakVersion = "1.6.0";
 
 final String slingHome = new File(buildDir + File.separatorChar + 
"sling_" + System.currentTimeMillis()).getAbsolutePath();
 
@@ -230,6 +230,7 @@ public abstract class AbstractJobHandlingTest {
 
 mavenBundle("com.google.guava", "guava", "15.0"),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-api", 
jackrabbitVersion),
+mavenBundle("org.apache.jackrabbit", "jackrabbit-data", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-jcr-commons", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-spi", 
jackrabbitVersion),
 mavenBundle("org.apache.jackrabbit", "jackrabbit-spi-commons", 
jackrabbitVersion),
{noformat}

> JobManagerImpl.findJobs should prevent traversal
> 
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
>  Issue Type: Improvement
>  Components: Event
>Reporter: Thomas Mueller
>Priority: Major
>
> The method 
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
>  runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't 
> initialized yet, meaning while the index isn't available yet. What is 
> happening in this case is that the query is traversing all nodes below that 
> path, triggering a warning that the query doesn't use an index. It is 
> sometimes happening when a health check is running before the repository is 
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an 
> index. If the index isn't available yet, it should fail. Therefore, the query 
> should use "option(traversal fail)". That would result in an exception that 
> can be caught.  I will log a related issue to change the health checks to 
> process this exception and return HEALTH_CHECK_ERROR for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)