You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "joerghoh (via GitHub)" <gi...@apache.org> on 2023/06/10 16:14:29 UTC
[GitHub] [sling-org-apache-sling-servlets-resolver] joerghoh opened a new pull request, #37: SLING-11558 cache already resolved resources
joerghoh opened a new pull request, #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37
The ``LocationCollector`` returns now a list of ``Resources`` instead of Strings. Internally it uses a Map (stored within ``ResourceResolver.getPropertyMap()``, thus this map shares the lifecycle of the resourceResolver) to cache already resolved resources.
The benefit of this cache for a single servlet resolution is likely quite small; but I expect that the benefit in real-life applications is much higher, especially when multiple servlets of the same type are resolved in the context of a resourceresolver. I plan to do some tests next week and share numbers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1585773937
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] joerghoh commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1594311624
I did some tests on a very simple page (AEM WKND sample page), and I see a decrease in the number of access to the repository in the context of resource resolution (from around 60 to 40). This is mostly caused by avoiding to resolve the default resourcetypes over and over. In more complex pages where the same resourcetype is used more often the percentage will increase.
In the context of resource access for the whole page this improvement is very small (< 1%), but that's rather caused by inefficiencies elsewhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1594513052
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] joerghoh commented on a diff in pull request #37: SLING-11558 cache already resolved resources
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#discussion_r1237045543
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
+ if (m == null) {
+ m = new HashMap<>();
+ resolver.getPropertyMap().put(CACHE_KEY, m);
Review Comment:
The ``propertyMap`` of the ResourceResolver is "public" in a way, that everyone who has access to that ResourceResolver can read and write it. That means that any code accessing it can read and modify it.
I don't think that this is a big security concern, because at that point if you have access to that object, you have your code already running in the JVM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1627264546
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [88.1% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1635704203
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [91.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] joerghoh merged pull request #37: SLING-11558 cache already resolved resources
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh merged PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1627277136
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [91.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1606232606
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [96.1% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1585725822
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] joerghoh commented on a diff in pull request #37: SLING-11558 cache already resolved resources
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#discussion_r1241261233
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
+ if (m == null) {
+ m = new HashMap<>();
+ resolver.getPropertyMap().put(CACHE_KEY, m);
+ }
+ final Map<String,Resource> cacheMap = m;
Review Comment:
This is needed because of the "quasi-final" requirements to it because i want to use it in the lambda below.
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
Review Comment:
This is needed because of the "quasi-final" requirements to it because i want to use it in the lambda below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] joerghoh commented on a diff in pull request #37: SLING-11558 cache already resolved resources
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#discussion_r1241259558
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
Review Comment:
This is needed because of the "quasi-final" requirements to it because i want to use it in the lambda below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1600641766
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] anchela commented on a diff in pull request #37: SLING-11558 cache already resolved resources
Posted by "anchela (via GitHub)" <gi...@apache.org>.
anchela commented on code in PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#discussion_r1236998781
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -73,12 +74,13 @@ class LocationCollector {
private final List<String> result = new ArrayList<>();
private LocationCollector(String resourceType, String resourceSuperType, String baseResourceType,
- ResourceResolver resolver) {
+ ResourceResolver resolver, Map<String,Resource> cacheMap) {
Review Comment:
i would suggest to consistently add @NotNull/@Nullable annotations to the parameters. IMHO it improves maintainability of the code... when looking at the patch it seems clear that 'cacheMap' is never null. but i wasn't so sure about the other params.
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
+ if (m == null) {
+ m = new HashMap<>();
+ resolver.getPropertyMap().put(CACHE_KEY, m);
Review Comment:
i have a few concerns regarding adding the cache to the property map of the resource-resolver:
- the property map is public right? so this cache map might leak out, no?
- is there any chance that someone relies on the current values of the property map? in other words: could adding a value that is purely for internal processing cause regressions?
- i am not too familiar with the servlet-resolution bundle: but if the resource-resolver instance is shared, this cache might be updated concurrently.... so using a regular hashmap is likely causing issues with concurrent updates as it is being populated upon the 'resolveResource' call.
my gut feeling: i would feel more comfortable if the resolver would optionally allow for a separate internal caching mechanism instead of adding it to the regular property map.
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
+ if (m == null) {
+ m = new HashMap<>();
+ resolver.getPropertyMap().put(CACHE_KEY, m);
+ }
+ final Map<String,Resource> cacheMap = m;
Review Comment:
that extra assignment is not really needed, is it?
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
Review Comment:
for the sake of defensive programming you may want to check if the map entry is really of the expected type before casting it. if it isn't someone might have manipulated it (e.g. adding a property with that key.....)
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
+ if (m == null) {
+ m = new HashMap<>();
+ resolver.getPropertyMap().put(CACHE_KEY, m);
+ }
+ final Map<String,Resource> cacheMap = m;
+
+ LocationCollector collector = new LocationCollector(resourceType, resourceSuperType, baseResourceType,
+ resolver, cacheMap);
+ List<Resource> result = new ArrayList<>();
+ collector.getResolvedLocations().forEach(location -> {
+ // get the location resource, use a synthetic resource if there
+ // is no real location. There may still be children at this
+ // location
+ final String path;
+ if (location.endsWith("/")) {
+ path = location.substring(0, location.length() - 1);
+ } else {
+ path = location;
+ }
+ final Resource locationRes = getResource(resolver, path, cacheMap);
+ result.add(locationRes);
+ });
+ return result;
+ }
+
+ /**
+ * Resolve a path to a resource
+ * @param resolver
+ * @param path
+ * @return a synthetic or "real" resource
+ */
+ protected static @NotNull Resource getResource(final @NotNull ResourceResolver resolver,
+ @NotNull String path, @NotNull Map<String,Resource> cacheMap) {
+
+ if (cacheMap.containsKey(path) && cacheMap.get(path) != null) {
+ return cacheMap.get(path);
+ } else {
+ Resource res = resolver.getResource(path);
+ if (res == null) {
+ res = new SyntheticResource(resolver, path, "$synthetic$");
+ }
+ cacheMap.put(path, res);
Review Comment:
see above regarding my concern about concurrent update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1606229964
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [96.1% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] joerghoh commented on a diff in pull request #37: SLING-11558 cache already resolved resources
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#discussion_r1241269395
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
+ if (m == null) {
+ m = new HashMap<>();
+ resolver.getPropertyMap().put(CACHE_KEY, m);
+ }
+ final Map<String,Resource> cacheMap = m;
+
+ LocationCollector collector = new LocationCollector(resourceType, resourceSuperType, baseResourceType,
+ resolver, cacheMap);
+ List<Resource> result = new ArrayList<>();
+ collector.getResolvedLocations().forEach(location -> {
+ // get the location resource, use a synthetic resource if there
+ // is no real location. There may still be children at this
+ // location
+ final String path;
+ if (location.endsWith("/")) {
+ path = location.substring(0, location.length() - 1);
+ } else {
+ path = location;
+ }
+ final Resource locationRes = getResource(resolver, path, cacheMap);
+ result.add(locationRes);
+ });
+ return result;
+ }
+
+ /**
+ * Resolve a path to a resource
+ * @param resolver
+ * @param path
+ * @return a synthetic or "real" resource
+ */
+ protected static @NotNull Resource getResource(final @NotNull ResourceResolver resolver,
+ @NotNull String path, @NotNull Map<String,Resource> cacheMap) {
+
+ if (cacheMap.containsKey(path) && cacheMap.get(path) != null) {
+ return cacheMap.get(path);
+ } else {
+ Resource res = resolver.getResource(path);
+ if (res == null) {
+ res = new SyntheticResource(resolver, path, "$synthetic$");
+ }
+ cacheMap.put(path, res);
Review Comment:
As this is assigned to a ResourceResolver, it should also be affected by the "1 thread, 1 resource resolver". So there should not be any concurrent access.
Nevertheless, as there is no guarantee for it, I wrapped it into a ``SynchronizedMap`` . Instead of giving good performance with highly concurrent access, I would rather want to detect such cases if they get real bottlenecks and change the usage of the ResourceResolvers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] joerghoh commented on a diff in pull request #37: SLING-11558 cache already resolved resources
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#discussion_r1241261233
##########
src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java:
##########
@@ -208,4 +209,75 @@ private void collectPaths() {
}
return rst;
}
+
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource(@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>) resolver.getPropertyMap().get(CACHE_KEY);
+ if (m == null) {
+ m = new HashMap<>();
+ resolver.getPropertyMap().put(CACHE_KEY, m);
+ }
+ final Map<String,Resource> cacheMap = m;
Review Comment:
This is needed because of the "quasi-final" requirements to it because i want to use it in the lambda below. But has been removed with the latest refactoring anway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-resolver] sonarcloud[bot] commented on pull request #37: SLING-11558 cache already resolved resources
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #37:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/37#issuecomment-1585726425
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-resolver&pullRequest=37&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org