[jira] [Created] (SLING-12350) Make the HTL Compiler build work on more recent JDKs
Csaba Varga created SLING-12350: --- Summary: Make the HTL Compiler build work on more recent JDKs Key: SLING-12350 URL: https://issues.apache.org/jira/browse/SLING-12350 Project: Sling Issue Type: Improvement Components: HTL Affects Versions: Scripting HTL Compiler 1.2.14-1.4.0 Reporter: Csaba Varga Fix For: Scripting HTL Compiler 1.2.16-1.4.0 The current build of the HTL compiler artifact fails on JDK 17 and later because it tries to access standard library internals via reflection. It should be updated so that it can be built on recent JDKs. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Created] (SLING-12325) The HTL parser can get confused when HTML markup is present in string literals
Csaba Varga created SLING-12325:
---
Summary: The HTL parser can get confused when HTML markup is
present in string literals
Key: SLING-12325
URL: https://issues.apache.org/jira/browse/SLING-12325
Project: Sling
Issue Type: Bug
Components: HTL
Reporter: Csaba Varga
In some cases, HTL that's perfectly valid based on the spec will cause a
compilation error. For example, this will be rejected when used outside of an
attribute:
{code:java}
${'Click here.' @ format='/', context='html'} {code}
The HTML parsing phase will consider the first closing brace to be the end of
the expression, and the part as a tag outside of an expression. The next
phase will see this expression:
{code:java}
${'Click here {code}
and reject it as syntactically invalid.
The HTML parsing phase needs to be more careful about expression syntax. I
think it will need to look out for quotes and backslashes to safely tell
whether a closing brace is an expression terminator or just a regular character
inside a string literal.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SLING-12266) Cache initial repository state to improve JCR_OAK performance
[ https://issues.apache.org/jira/browse/SLING-12266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17836117#comment-17836117 ] Csaba Varga commented on SLING-12266: - [~sseifert] : Removed the "more aggressive" commit as requested. While there, I also rebased my code on the latest master. (I accidentally started from an outdated commit originally.) > Cache initial repository state to improve JCR_OAK performance > - > > Key: SLING-12266 > URL: https://issues.apache.org/jira/browse/SLING-12266 > Project: Sling > Issue Type: Improvement > Components: Testing >Affects Versions: Testing Sling Mock 3.4.18 >Reporter: Csaba Varga >Assignee: Stefan Seifert >Priority: Minor > > A lot of effort goes into preparing an Oak Mock repository from scratch: node > types need to be registered, indexes need to be created, and all this happens > over several commits. None of this work depends on the test case itself, so > it will always result in the exact same repository state. We could take the > root NodeState from the first repository we build, then build subsequent > repositories on top of it, avoiding most of the redundant work. Commits can > be relatively expensive even in memory, so each one we avoid can save a lot > of time in the long term. > > This would require extending the contract between Testing Sling Mock and the > ResourceResolverTypeAdapters, to add optional "make snapshot" and "build repo > from snapshot" operations. For adapters that don't support them, we would > keep rebuilding things from scratch. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (SLING-12266) Cache initial repository state to improve JCR_OAK performance
[ https://issues.apache.org/jira/browse/SLING-12266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17835759#comment-17835759 ] Csaba Varga commented on SLING-12266: - I separated that change to a different commit because I wasn't sure about it, either. On one hand, it's the only way I can see to avoid running some code that parses a large CND file and tries to (redundantly) register the built-in node types. On the other hand, it _is_ a bit ugly, and it doesn't save that much time, according to my quick and dirty performance tests. I can drop that part from my PR if that's the consensus. > Cache initial repository state to improve JCR_OAK performance > - > > Key: SLING-12266 > URL: https://issues.apache.org/jira/browse/SLING-12266 > Project: Sling > Issue Type: Improvement > Components: Testing >Affects Versions: Testing Sling Mock 3.4.18 >Reporter: Csaba Varga >Assignee: Stefan Seifert >Priority: Minor > > A lot of effort goes into preparing an Oak Mock repository from scratch: node > types need to be registered, indexes need to be created, and all this happens > over several commits. None of this work depends on the test case itself, so > it will always result in the exact same repository state. We could take the > root NodeState from the first repository we build, then build subsequent > repositories on top of it, avoiding most of the redundant work. Commits can > be relatively expensive even in memory, so each one we avoid can save a lot > of time in the long term. > > This would require extending the contract between Testing Sling Mock and the > ResourceResolverTypeAdapters, to add optional "make snapshot" and "build repo > from snapshot" operations. For adapters that don't support them, we would > keep rebuilding things from scratch. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (SLING-12266) Cache initial repository state to improve JCR_OAK performance
[ https://issues.apache.org/jira/browse/SLING-12266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17835676#comment-17835676 ] Csaba Varga commented on SLING-12266: - [~sseifert] : I have opened two PRs, as this will need changes to both sling-mock and sling-mock-oak. Looks like I will need to work a bit on test coverage before the build becomes green, but in the meantime, feel free to review the overall approach. > Cache initial repository state to improve JCR_OAK performance > - > > Key: SLING-12266 > URL: https://issues.apache.org/jira/browse/SLING-12266 > Project: Sling > Issue Type: Improvement > Components: Testing >Affects Versions: Testing Sling Mock 3.4.18 >Reporter: Csaba Varga >Priority: Minor > > A lot of effort goes into preparing an Oak Mock repository from scratch: node > types need to be registered, indexes need to be created, and all this happens > over several commits. None of this work depends on the test case itself, so > it will always result in the exact same repository state. We could take the > root NodeState from the first repository we build, then build subsequent > repositories on top of it, avoiding most of the redundant work. Commits can > be relatively expensive even in memory, so each one we avoid can save a lot > of time in the long term. > > This would require extending the contract between Testing Sling Mock and the > ResourceResolverTypeAdapters, to add optional "make snapshot" and "build repo > from snapshot" operations. For adapters that don't support them, we would > keep rebuilding things from scratch. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (SLING-12266) Cache initial repository state to improve JCR_OAK performance
[ https://issues.apache.org/jira/browse/SLING-12266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17825676#comment-17825676 ] Csaba Varga commented on SLING-12266: - I would be interested in writing code for this, if it seems worthwhile. I think it could speed things up nicely, but I may be missing some potential issue here. > Cache initial repository state to improve JCR_OAK performance > - > > Key: SLING-12266 > URL: https://issues.apache.org/jira/browse/SLING-12266 > Project: Sling > Issue Type: Improvement > Components: Testing >Affects Versions: Testing Sling Mock 3.4.18 >Reporter: Csaba Varga >Priority: Minor > > A lot of effort goes into preparing an Oak Mock repository from scratch: node > types need to be registered, indexes need to be created, and all this happens > over several commits. None of this work depends on the test case itself, so > it will always result in the exact same repository state. We could take the > root NodeState from the first repository we build, then build subsequent > repositories on top of it, avoiding most of the redundant work. Commits can > be relatively expensive even in memory, so each one we avoid can save a lot > of time in the long term. > > This would require extending the contract between Testing Sling Mock and the > ResourceResolverTypeAdapters, to add optional "make snapshot" and "build repo > from snapshot" operations. For adapters that don't support them, we would > keep rebuilding things from scratch. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Created] (SLING-12266) Cache initial repository state to improve JCR_OAK performance
Csaba Varga created SLING-12266: --- Summary: Cache initial repository state to improve JCR_OAK performance Key: SLING-12266 URL: https://issues.apache.org/jira/browse/SLING-12266 Project: Sling Issue Type: Improvement Components: Testing Affects Versions: Testing Sling Mock 3.4.18 Reporter: Csaba Varga A lot of effort goes into preparing an Oak Mock repository from scratch: node types need to be registered, indexes need to be created, and all this happens over several commits. None of this work depends on the test case itself, so it will always result in the exact same repository state. We could take the root NodeState from the first repository we build, then build subsequent repositories on top of it, avoiding most of the redundant work. Commits can be relatively expensive even in memory, so each one we avoid can save a lot of time in the long term. This would require extending the contract between Testing Sling Mock and the ResourceResolverTypeAdapters, to add optional "make snapshot" and "build repo from snapshot" operations. For adapters that don't support them, we would keep rebuilding things from scratch. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (SLING-12265) Node type registration is inefficient during unit tests when using the JCR_OAK resolver type
[ https://issues.apache.org/jira/browse/SLING-12265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17825350#comment-17825350 ] Csaba Varga commented on SLING-12265: - I have already made some experimental changes on this today, and my unit tests complete noticeably faster with my tentative improvement. I will open a PR today or tomorrow with the proposed changes. > Node type registration is inefficient during unit tests when using the > JCR_OAK resolver type > > > Key: SLING-12265 > URL: https://issues.apache.org/jira/browse/SLING-12265 > Project: Sling > Issue Type: Improvement > Components: Testing >Affects Versions: Testing Sling Mock 3.4.18 >Reporter: Csaba Varga >Priority: Minor > Labels: performance > > I did some profiling on my company's slow-running unit tests today, and found > that 70+% of the CPU time is spent inside NodeTypeDefinitionScanner, more > specifically in the commit() call triggered by it. Our test classpath > includes AEM Mocks and the Cloud SDK, resulting in 30+ CND files being > detected, and as many commits done preparing the repository before each test. > (Slightly more, because some commits fail and get retried later.) > It should be possible to parse each CND into memory structures in a separate > pass, then create the node types all at once in a single commit. This > wouldn't just eliminate the extra commits, but would also remove the need to > retry, since all dependencies would also be provided in the same call. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Created] (SLING-12265) Node type registration is inefficient during unit tests when using the JCR_OAK resolver type
Csaba Varga created SLING-12265: --- Summary: Node type registration is inefficient during unit tests when using the JCR_OAK resolver type Key: SLING-12265 URL: https://issues.apache.org/jira/browse/SLING-12265 Project: Sling Issue Type: Improvement Components: Testing Affects Versions: Testing Sling Mock 3.4.18 Reporter: Csaba Varga I did some profiling on my company's slow-running unit tests today, and found that 70+% of the CPU time is spent inside NodeTypeDefinitionScanner, more specifically in the commit() call triggered by it. Our test classpath includes AEM Mocks and the Cloud SDK, resulting in 30+ CND files being detected, and as many commits done preparing the repository before each test. (Slightly more, because some commits fail and get retried later.) It should be possible to parse each CND into memory structures in a separate pass, then create the node types all at once in a single commit. This wouldn't just eliminate the extra commits, but would also remove the need to retry, since all dependencies would also be provided in the same call. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (SLING-11352) Cannot create path-only mapping rule in /etc/map
[
https://issues.apache.org/jira/browse/SLING-11352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17609444#comment-17609444
]
Csaba Varga commented on SLING-11352:
-
I have opened a PR with a proposed fix:
[https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/84]
Could someone please take a look at it?
> Cannot create path-only mapping rule in /etc/map
>
>
> Key: SLING-11352
> URL: https://issues.apache.org/jira/browse/SLING-11352
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.8.4
>Reporter: Csaba Varga
>Priority: Minor
> Time Spent: 20m
> Remaining Estimate: 0h
>
> I am trying to set up a mapping to make some of our site hierarchy appear on
> a different URL than what would be dictated by the hierarchy. My goal is to
> map the path /content/mycompany/locales/some_locale/sectionA to the URL
> /some_locale/sectionB . (I don't want to override the protocol, host and port
> fields.) Because we support multiple locales, I need to use a regex for
> setting up the matches. Here is my original attempt:
> {code:java}
> /etc/map/my_mapping
> +-- sling:internalRedirect =
> /content/mycompany/locales/([-a-z]+)/sectionA
> +-- sling:match = /$1/sectionB{code}
> When this node is present, /content/mycompany/locales/en-us/sectionA/foo gets
> mapped to /en-us://sectionB/foo instead of the intended /en-us/sectionB/foo.
> I can work around the issue by avoiding forward slashes in sling:match, but
> this is ugly and unclear :
> {code:java}
> /etc/map/my_mapping
> +-- sling:internalRedirect =
> /content/mycompany/locales/([-a-z]+)(/)sectionA
> +-- sling:match = /$1$2sectionB {code}
> Is this kind of mapping supported at all? I would rather avoid adding the
> hostname to mapping rules because it is environment-specific.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SLING-11352) Cannot create path-only mapping rule in /etc/map
[
https://issues.apache.org/jira/browse/SLING-11352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17543933#comment-17543933
]
Csaba Varga commented on SLING-11352:
-
Looks like the issue is in the MapEntry.toURI method. Its comment says that it
is only supposed to process absolute URIs and should return null for regular
paths like "/the/path". It actually returns "/the://path" when given the input
"/the/path".
toURI tries four regexes, but the fourth one is overly generic and will match
anything that contains a forward slash anywhere but at the very beginning. The
regexes look like they are meant to describe the whole string, so maybe they
should be matched using Matcher.matches instead of Matcher.find. That way, none
of them would match "/the/path" and the method would start working as
documented.
> Cannot create path-only mapping rule in /etc/map
>
>
> Key: SLING-11352
> URL: https://issues.apache.org/jira/browse/SLING-11352
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.8.4
>Reporter: Csaba Varga
>Priority: Minor
>
> I am trying to set up a mapping to make some of our site hierarchy appear on
> a different URL than what would be dictated by the hierarchy. My goal is to
> map the path /content/mycompany/locales/some_locale/sectionA to the URL
> /some_locale/sectionB . (I don't want to override the protocol, host and port
> fields.) Because we support multiple locales, I need to use a regex for
> setting up the matches. Here is my original attempt:
> {code:java}
> /etc/map/my_mapping
> +-- sling:internalRedirect =
> /content/mycompany/locales/([-a-z]+)/sectionA
> +-- sling:match = /$1/sectionB{code}
> When this node is present, /content/mycompany/locales/en-us/sectionA/foo gets
> mapped to /en-us://sectionB/foo instead of the intended /en-us/sectionB/foo.
> I can work around the issue by avoiding forward slashes in sling:match, but
> this is ugly and unclear :
> {code:java}
> /etc/map/my_mapping
> +-- sling:internalRedirect =
> /content/mycompany/locales/([-a-z]+)(/)sectionA
> +-- sling:match = /$1$2sectionB {code}
> Is this kind of mapping supported at all? I would rather avoid adding the
> hostname to mapping rules because it is environment-specific.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
[jira] [Updated] (SLING-11352) Cannot create path-only mapping rule in /etc/map
[
https://issues.apache.org/jira/browse/SLING-11352?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Csaba Varga updated SLING-11352:
Description:
I am trying to set up a mapping to make some of our site hierarchy appear on a
different URL than what would be dictated by the hierarchy. My goal is to map
the path /content/mycompany/locales/some_locale/sectionA to the URL
/some_locale/sectionB . (I don't want to override the protocol, host and port
fields.) Because we support multiple locales, I need to use a regex for setting
up the matches. Here is my original attempt:
{code:java}
/etc/map/my_mapping
+-- sling:internalRedirect = /content/mycompany/locales/([-a-z]+)/sectionA
+-- sling:match = /$1/sectionB{code}
When this node is present, /content/mycompany/locales/en-us/sectionA/foo gets
mapped to /en-us://sectionB/foo instead of the intended /en-us/sectionB/foo.
I can work around the issue by avoiding forward slashes in sling:match, but
this is ugly and unclear :
{code:java}
/etc/map/my_mapping
+-- sling:internalRedirect =
/content/mycompany/locales/([-a-z]+)(/)sectionA
+-- sling:match = /$1$2sectionB {code}
Is this kind of mapping supported at all? I would rather avoid adding the
hostname to mapping rules because it is environment-specific.
was:
I am trying to set up a mapping to make some of our site hierarchy appear on a
different URL than what would be dictated by the hierarchy. My goal is to map
the path /content/mycompany/locales/some_locale/sectionA to the URL
/some_locale/sectionB . (I don't want to override the protocol, host and port
fields.) Because we support multiple locales, I need to use a regex for setting
up the matches. Here is my original attempt:
{code:java}
/etc/map/my_mapping
+-- sling:internalRedirect = /content/mycompany/locales/([-a-z]+)/sectionA
+-- sling:match = /$1/sectionB{code}
When this node is present, /content/mycompany/locales/en-us/sectionA/foo gets
mapped to en-us://sectionB/foo instead of the intended /en-us/sectionB/foo.
I can work around the issue by avoiding forward slashes in sling:match, but
this is ugly and unclear :
{code:java}
/etc/map/my_mapping
+-- sling:internalRedirect =
/content/mycompany/locales/([-a-z]+)(/)sectionA
+-- sling:match = /$1$2sectionB {code}
Is this kind of mapping supported at all? I would rather avoid adding the
hostname to mapping rules because it is environment-specific.
> Cannot create path-only mapping rule in /etc/map
>
>
> Key: SLING-11352
> URL: https://issues.apache.org/jira/browse/SLING-11352
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.8.4
>Reporter: Csaba Varga
>Priority: Minor
>
> I am trying to set up a mapping to make some of our site hierarchy appear on
> a different URL than what would be dictated by the hierarchy. My goal is to
> map the path /content/mycompany/locales/some_locale/sectionA to the URL
> /some_locale/sectionB . (I don't want to override the protocol, host and port
> fields.) Because we support multiple locales, I need to use a regex for
> setting up the matches. Here is my original attempt:
> {code:java}
> /etc/map/my_mapping
> +-- sling:internalRedirect =
> /content/mycompany/locales/([-a-z]+)/sectionA
> +-- sling:match = /$1/sectionB{code}
> When this node is present, /content/mycompany/locales/en-us/sectionA/foo gets
> mapped to /en-us://sectionB/foo instead of the intended /en-us/sectionB/foo.
> I can work around the issue by avoiding forward slashes in sling:match, but
> this is ugly and unclear :
> {code:java}
> /etc/map/my_mapping
> +-- sling:internalRedirect =
> /content/mycompany/locales/([-a-z]+)(/)sectionA
> +-- sling:match = /$1$2sectionB {code}
> Is this kind of mapping supported at all? I would rather avoid adding the
> hostname to mapping rules because it is environment-specific.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
[jira] [Created] (SLING-11352) Cannot create path-only mapping rule in /etc/map
Csaba Varga created SLING-11352:
---
Summary: Cannot create path-only mapping rule in /etc/map
Key: SLING-11352
URL: https://issues.apache.org/jira/browse/SLING-11352
Project: Sling
Issue Type: Bug
Components: ResourceResolver
Affects Versions: Resource Resolver 1.8.4
Reporter: Csaba Varga
I am trying to set up a mapping to make some of our site hierarchy appear on a
different URL than what would be dictated by the hierarchy. My goal is to map
the path /content/mycompany/locales/some_locale/sectionA to the URL
/some_locale/sectionB . (I don't want to override the protocol, host and port
fields.) Because we support multiple locales, I need to use a regex for setting
up the matches. Here is my original attempt:
{code:java}
/etc/map/my_mapping
+-- sling:internalRedirect = /content/mycompany/locales/([-a-z]+)/sectionA
+-- sling:match = /$1/sectionB{code}
When this node is present, /content/mycompany/locales/en-us/sectionA/foo gets
mapped to en-us://sectionB/foo instead of the intended /en-us/sectionB/foo.
I can work around the issue by avoiding forward slashes in sling:match, but
this is ugly and unclear :
{code:java}
/etc/map/my_mapping
+-- sling:internalRedirect =
/content/mycompany/locales/([-a-z]+)(/)sectionA
+-- sling:match = /$1$2sectionB {code}
Is this kind of mapping supported at all? I would rather avoid adding the
hostname to mapping rules because it is environment-specific.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
[jira] [Commented] (SLING-10265) JUNIT5 Test cases are failing when I use code "new AemContext(ResourceResolverType.JCR_OAK)"
[ https://issues.apache.org/jira/browse/SLING-10265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17326436#comment-17326436 ] Csaba Varga commented on SLING-10265: - I'm not a Sling developer, but I've run into the exact same issue this week. In my case, it was caused by two incompatible sets of Sling API classes being on the classpath. One was included via Sling Mocks, the other was embedded into the AEM SDK API JAR. I could fix my issue by explicitly including org.apache.sling.api as a dependency before Sling Mocks and specifying a more recent version than what Sling Mocks would have pulled. This more recent version, while still technically a duplicate, played nicely with the classes coming from the AEM SDK. I hope this helps. > JUNIT5 Test cases are failing when I use code "new > AemContext(ResourceResolverType.JCR_OAK)" > > > Key: SLING-10265 > URL: https://issues.apache.org/jira/browse/SLING-10265 > Project: Sling > Issue Type: Bug > Components: Apache Sling Testing Clients >Affects Versions: Testing Sling Mock 2.6.0 >Reporter: subramanya >Priority: Blocker > > I am unable to run test cases using JUNIT5 with JCR_OAK repository > implementation in the test case code . > Repository using JCR_MOCK works, the snippet below works > private final AemContext aemContext = new > AemContext(ResourceResolverType.JCR_MOCK); > Where as this one throws exception: > private final AemContext aemContext = new > AemContext(ResourceResolverType.JCR_OAK); > This is the error I am getting > java.lang.RuntimeException: Unable to initialize JCR_OAK resource resolver > factory: Unable to invoke method 'activate' for class > org.apache.sling.testing.mock.sling.oak.OakMockSlingRepository Caused by: > java.lang.NoSuchMethodError: > org.apache.jackrabbit.oak.plugins.nodetype.constraint.Constraints.asPredicate(ILjava/lang/String;)Ljava/util/function/Predicate; > summary JUNIT5 Test cases are failing when I use new > AemContext(ResourceResolverType.JCR_OAK) -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (SLING-7815) CLONE - ResourceResolver.clone(null) should not share the same JCR session
[
https://issues.apache.org/jira/browse/SLING-7815?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16750311#comment-16750311
]
Csaba Varga commented on SLING-7815:
[~cziegeler]: Can we have a decision on this, please? This ticket and the
corresponding PR has been in limbo ever since September, neither getting merged
nor getting rejected. I can make changes if the current code is not acceptable,
of course, but it feels like this ticket is staying open without any good
reason.
> CLONE - ResourceResolver.clone(null) should not share the same JCR session
> --
>
> Key: SLING-7815
> URL: https://issues.apache.org/jira/browse/SLING-7815
> Project: Sling
> Issue Type: Improvement
> Components: API, JCR, ResourceResolver
>Affects Versions: JCR Resource 3.0.14
>Reporter: Alexander Klimetschek
>Assignee: Carsten Ziegeler
>Priority: Major
> Fix For: JCR Resource 3.0.18
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SLING-7815) CLONE - ResourceResolver.clone(null) should not share the same JCR session
[
https://issues.apache.org/jira/browse/SLING-7815?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16601673#comment-16601673
]
Csaba Varga commented on SLING-7815:
[~rombert]: I had some time this weekend to have a look at this.
Because of a stupid mistake in my code, the JCR resource provider could close
the session from under itself. This case was even triggered with a regular
username+password login, but it haven't occurred to me to test for it!
I've created a pull request with the proposed fixed code:
[https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/4]
This passes the (now extended) unit tests, and I can also launch the starter
app properly using this version of the bundle, so hopefully it works properly
now. Do let me know if I should validate it some other way as well.
> CLONE - ResourceResolver.clone(null) should not share the same JCR session
> --
>
> Key: SLING-7815
> URL: https://issues.apache.org/jira/browse/SLING-7815
> Project: Sling
> Issue Type: Improvement
> Components: API, JCR, ResourceResolver
>Affects Versions: JCR Resource 3.0.14
>Reporter: Alexander Klimetschek
>Assignee: Carsten Ziegeler
>Priority: Major
> Fix For: JCR Resource 3.0.18
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SLING-3524) ResourceResolver.clone(null) should not share the same JCR session
[
https://issues.apache.org/jira/browse/SLING-3524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16461478#comment-16461478
]
Csaba Varga commented on SLING-3524:
Pushed a new test case that will exercise the JCR login and logout
functionality in all the eight combinations we were talking about. I hope I
could express all API guarantees as tests, so they won't get broken again in
the future.
> ResourceResolver.clone(null) should not share the same JCR session
> --
>
> Key: SLING-3524
> URL: https://issues.apache.org/jira/browse/SLING-3524
> Project: Sling
> Issue Type: Improvement
> Components: API, JCR, ResourceResolver
>Affects Versions: Resource Resolver 1.0.6
>Reporter: Alexander Klimetschek
>Priority: Major
> Fix For: API 2.18.2, JCR Resource 3.0.10, Resource Resolver 1.6.2
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SLING-3524) ResourceResolver.clone(null) should not share the same JCR session
[
https://issues.apache.org/jira/browse/SLING-3524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16461281#comment-16461281
]
Csaba Varga commented on SLING-3524:
I've added a [new
commit|https://github.com/csaboka/sling-org-apache-sling-resourceresolver/commit/0f444aa5c6537d480badcf4ca7b2af557f09d48a]
to clear the CLONE key from the authentication info before passing it to the
resource resolver. I hope I've caught all the paths from external clients to
the ResourceResolverImpl constructor. The upside is that now the code has a
central place where new "forbidden properties" can be added as necessary.
> ResourceResolver.clone(null) should not share the same JCR session
> --
>
> Key: SLING-3524
> URL: https://issues.apache.org/jira/browse/SLING-3524
> Project: Sling
> Issue Type: Improvement
> Components: API, JCR, ResourceResolver
>Affects Versions: Resource Resolver 1.0.6
>Reporter: Alexander Klimetschek
>Priority: Major
> Fix For: API 2.18.2, JCR Resource 3.0.10, Resource Resolver 1.6.2
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SLING-3524) ResourceResolver.clone(null) should not share the same JCR session
[
https://issues.apache.org/jira/browse/SLING-3524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16460196#comment-16460196
]
Csaba Varga commented on SLING-3524:
{quote}Unit tests covering all paths would be good, I think.
{quote}
I'll try to have a shot at it, then. With a parameterized test class covering
all eight combinations, I could probably get rid of the current testLeakOnSudo
and testNoSessionSharing tests. (They would be covered by the new test class,
which would test leaks and sharing in all the relevant combinations.)
{quote}AFAICS, the cell in bold is the only new combination as part of this
patch.
{quote}
Yes, that's the intention. If any other combination starts working differently,
it's most likely a bug.
> ResourceResolver.clone(null) should not share the same JCR session
> --
>
> Key: SLING-3524
> URL: https://issues.apache.org/jira/browse/SLING-3524
> Project: Sling
> Issue Type: Improvement
> Components: JCR, ResourceResolver
>Affects Versions: Resource Resolver 1.0.6
>Reporter: Alexander Klimetschek
>Priority: Major
> Time Spent: 40m
> Remaining Estimate: 0h
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SLING-3524) ResourceResolver.clone(null) should not share the same JCR session
[
https://issues.apache.org/jira/browse/SLING-3524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16459515#comment-16459515
]
Csaba Varga commented on SLING-3524:
{quote}I think the "needsCloning/needsSudo/explicitSessionUsed" might need some
explanation in comments, it's not straightforward to follow.
{quote}
Fair point. I've added some comments now. explicitSessionUsed is already
commented in the Javadoc of handleImpersonation(). Does it need some
clarification?
{quote}Are all possible combinations covered correctly?
{quote}
I think so, but I've gone through them this morning to double check. I believe
we have three independent variables to consider, giving eight cases in total:
* Is it a normal login or is a session given in the auth info? (For this
variable, I believe we can treat service logins and administrative logins as
normal logins.)
* Does the login request impersonation or not?
* Is the login caused by a clone() call?
This is what happens in the eight cases:
||Scenario||No clone||Clone||
|Normal login, no sudo|logoutSession: true
no impersonation call
doLogoutSession: true|logoutSession: true
no impersonation call
doLogoutSession: true|
|Normal login with sudo|logoutSession: true
original session impersonated then closed, USER_IMPERSONATOR set
doLogoutSession: true|logoutSession: true
original session impersonated then closed, USER_IMPERSONATOR set
doLogoutSession: true|
|Session login, no sudo|logoutSession: false
session used as-is
doLogoutSession: false|logoutSession: false
session self-impersonated
doLogoutSession: true|
|Session login with sudo|logoutSession: false
session impersonated, USER_IMPERSONATOR set
doLogoutSession: true|logoutSession: false
session impersonated, USER_IMPERSONATOR set
doLogoutSession: true|
The only case when cloning behavior is different from normal behavior is when
you pass a session but you don't want to impersonate. If you don't pass a
session, cloning will just log you in again with your credentials, just like
before the patch. If you pass a session and you request impersonation,
session.impersonate() was already called before this patch, and will keep being
called after it.
Am I missing something? If these are all the factors we need to worry about, do
you think it's worthwhile to build a parameterized unit test that covers all
possible combinations?
> ResourceResolver.clone(null) should not share the same JCR session
> --
>
> Key: SLING-3524
> URL: https://issues.apache.org/jira/browse/SLING-3524
> Project: Sling
> Issue Type: Improvement
> Components: JCR, ResourceResolver
>Affects Versions: Resource Resolver 1.0.6
>Reporter: Alexander Klimetschek
>Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SLING-3524) ResourceResolver.clone(null) should not share the same JCR session
[
https://issues.apache.org/jira/browse/SLING-3524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16458499#comment-16458499
]
Csaba Varga commented on SLING-3524:
Here are my pull requests for the proposed fix:
[https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/3]
[https://github.com/apache/sling-org-apache-sling-api/pull/3]
[https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/6]
I'm not sure if this is the right approach to the problem, but it seemed the
simplest.
The idea is to introduce a new key to the authentication info map, to indicate
that the login is happening because of a clone() call. Resource providers can
then adjust their behavior based on the presence of this key. The JCR resource
provider checks the presence of this flag, in addition to the presence of an
explicitly provided JCR session in the login info. If both are present, the
session is cloned using the self-impersonation trick to make sure it's not
shared with the originating resolver.
To allow proper behavior for clones of clones, the JCR resource provider also
updates the authentication info with the cloned session. I'm not sure how
"clean" this approach is, since the API doesn't say whether mutation of the
authentication info is supported as part of a provider login. It works with the
current implementation, but it may still violate the spirit of the API.
> ResourceResolver.clone(null) should not share the same JCR session
> --
>
> Key: SLING-3524
> URL: https://issues.apache.org/jira/browse/SLING-3524
> Project: Sling
> Issue Type: Improvement
> Components: JCR, ResourceResolver
>Affects Versions: Resource Resolver 1.0.6
>Reporter: Alexander Klimetschek
>Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SLING-3524) ResourceResolver.clone(null) should not share the same JCR session
[
https://issues.apache.org/jira/browse/SLING-3524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16458477#comment-16458477
]
Csaba Varga commented on SLING-3524:
I believe the problem is really about the statefulness of the authentication
info. The clone() method assumes that none of the authentication info has any
state, so just copying the references into a new map will give you a suitable
new authentication info. This works well most of the time when you authenticate
using strings, but breaks down with a JCR session object because it's _very_
stateful.
I'm working on a proposed patch that lets individual resource providers work
around this limitation by letting them distinguish a clone operation from a
regular login. In case of clone, stateful things like JCR sessions can be
properly cloned to avoid any inadvertent shared state between the old and new
resolvers. Providers that don't use stateful objects in their login info can
just keep working without any changes.
> ResourceResolver.clone(null) should not share the same JCR session
> --
>
> Key: SLING-3524
> URL: https://issues.apache.org/jira/browse/SLING-3524
> Project: Sling
> Issue Type: Improvement
> Components: JCR, ResourceResolver
>Affects Versions: Resource Resolver 1.0.6
>Reporter: Alexander Klimetschek
>Priority: Major
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SLING-7591) Session leak when a ResourceResolver is created from a Session then gets cloned
[ https://issues.apache.org/jira/browse/SLING-7591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16441347#comment-16441347 ] Csaba Varga commented on SLING-7591: Created PR with proposed fix: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/2 > Session leak when a ResourceResolver is created from a Session then gets > cloned > --- > > Key: SLING-7591 > URL: https://issues.apache.org/jira/browse/SLING-7591 > Project: Sling > Issue Type: Bug > Components: JCR, ResourceResolver >Affects Versions: JCR Resource 3.0.8 >Reporter: Csaba Varga >Priority: Major > > The following steps will cause a new JCR session to be opened then leaked: > # Create a ResourceResolver out of an existing JCR session (using > AUTHENTICATION_INFO_SESSION) > # Call clone() on the ResourceResolver created in step 1, passing the > ResourceResolverFactory.USER_IMPERSONATION key with a username to impersonate > # Close the ResourceResolver created in step 2. > The expected behavior is that since step 2 opened a new JCR Session in the > background, step 3 should close it. The actual behavior is that the JCR > session stays open without an obvious way to close it, and these leaked > sessions can pile up if this sequence is executed often. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SLING-7591) Session leak when a ResourceResolver is created from a Session then gets cloned
[ https://issues.apache.org/jira/browse/SLING-7591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16441265#comment-16441265 ] Csaba Varga commented on SLING-7591: I'll try to create a pull request with a unit test demonstration and a proposed fix. This is my first time working on this code base, so I'm not sure when I can complete it. > Session leak when a ResourceResolver is created from a Session then gets > cloned > --- > > Key: SLING-7591 > URL: https://issues.apache.org/jira/browse/SLING-7591 > Project: Sling > Issue Type: Bug > Components: JCR, ResourceResolver >Affects Versions: JCR Resource 3.0.8 >Reporter: Csaba Varga >Priority: Major > > The following steps will cause a new JCR session to be opened then leaked: > # Create a ResourceResolver out of an existing JCR session (using > AUTHENTICATION_INFO_SESSION) > # Call clone() on the ResourceResolver created in step 1, passing the > ResourceResolverFactory.USER_IMPERSONATION key with a username to impersonate > # Close the ResourceResolver created in step 2. > The expected behavior is that since step 2 opened a new JCR Session in the > background, step 3 should close it. The actual behavior is that the JCR > session stays open without an obvious way to close it, and these leaked > sessions can pile up if this sequence is executed often. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (SLING-7591) Session leak when a ResourceResolver is created from a Session then gets cloned
Csaba Varga created SLING-7591: -- Summary: Session leak when a ResourceResolver is created from a Session then gets cloned Key: SLING-7591 URL: https://issues.apache.org/jira/browse/SLING-7591 Project: Sling Issue Type: Bug Components: JCR, ResourceResolver Affects Versions: JCR Resource 3.0.8 Reporter: Csaba Varga The following steps will cause a new JCR session to be opened then leaked: # Create a ResourceResolver out of an existing JCR session (using AUTHENTICATION_INFO_SESSION) # Call clone() on the ResourceResolver created in step 1, passing the ResourceResolverFactory.USER_IMPERSONATION key with a username to impersonate # Close the ResourceResolver created in step 2. The expected behavior is that since step 2 opened a new JCR Session in the background, step 3 should close it. The actual behavior is that the JCR session stays open without an obvious way to close it, and these leaked sessions can pile up if this sequence is executed often. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
