Re: [PR] Make the build work on more recent JDKs [sling-org-apache-sling-scripting-sightly-compiler]
sonarcloud[bot] commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler/pull/13#issuecomment-2131368553 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13) **Quality Gate failed** Failed conditions  [56.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13=new_coverage=list) (required ≥ 80%)  [9.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13=new_duplicated_lines_density=list) (required ≤ 3%) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Make the build work on more recent JDKs [sling-org-apache-sling-scripting-sightly-compiler]
kwin commented on code in PR #13: URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler/pull/13#discussion_r1614785674 ## pom.xml: ## @@ -41,27 +41,133 @@ Apache Sling Scripting HTL Compiler - -The Apache Sling Scripting HTL Compiler provides support for compiling HTML Template Language scripts into an Abstract -Syntax Tree. - +The Apache Sling Scripting HTL Compiler provides support for compiling HTML Template Language scripts into an Abstract +Syntax Tree. scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-sightly-compiler.git scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-sightly-compiler.git - https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler.git org.apache.sling.scripting.sightly.compiler-1.2.6-1.4.0 + https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler.git Review Comment: I would suggest setting an explicit java bytecode version (no need to upgrade > 8 for now) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Make the build work on more recent JDKs [sling-org-apache-sling-scripting-sightly-compiler]
csaboka commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler/pull/13#issuecomment-2131315388 Well, now I get the blame for all the code duplication and missing coverage on the code I "touched". I hope that isn't a blocker for merging this. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Make the build work on more recent JDKs [sling-org-apache-sling-scripting-sightly-compiler]
sonarcloud[bot] commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler/pull/13#issuecomment-2131313550 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13) **Quality Gate failed** Failed conditions  [55.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13=new_coverage=list) (required ≥ 80%)  [9.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13=new_duplicated_lines_density=list) (required ≤ 3%) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Make the build work on more recent JDKs [sling-org-apache-sling-scripting-sightly-compiler]
csaboka commented on code in PR #13: URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler/pull/13#discussion_r1614722374 ## pom.xml: ## @@ -25,7 +25,7 @@ org.apache.sling sling-bundle-parent -44 +52 Review Comment: Oh, after reading the linked page I can see that it still targets JDK 11, but requires JDK 17+ for the build process. I'll have a try at it. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Make the build work on more recent JDKs [sling-org-apache-sling-scripting-sightly-compiler]
csaboka commented on code in PR #13: URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler/pull/13#discussion_r1614720582 ## pom.xml: ## @@ -25,7 +25,7 @@ org.apache.sling sling-bundle-parent -44 +52 Review Comment: The latest parent only builds with Java 17 or later. Is that OK? Aren't there any clients that need to run on JDK 11? -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Make the build work on more recent JDKs [sling-org-apache-sling-scripting-sightly-compiler]
kwin commented on code in PR #13: URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler/pull/13#discussion_r1614687706 ## pom.xml: ## @@ -25,7 +25,7 @@ org.apache.sling sling-bundle-parent -44 +52 Review Comment: Can you update to latest? There are hints on what you need to consider in https://cwiki.apache.org/confluence/x/SI75E -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Make the build work on more recent JDKs [sling-org-apache-sling-scripting-sightly-compiler]
sonarcloud[bot] commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly-compiler/pull/13#issuecomment-2131264743 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13=false=true)  No data about Coverage  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-scripting-sightly-compiler=13) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
rishabhdaim commented on PR #9: URL: https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#issuecomment-2129190028 > @rishabhdaim I think once you remove the original.pid part of the code, we could merge this PR. Done in https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9/commits/69597f866d6a5646ba334cbefb3aade499a97e1d -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bump ch.qos.logback:logback-classic from 1.2.11 to 1.2.13 [sling-org-apache-sling-jcr-repoinit]
sonarcloud[bot] commented on PR #49: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/49#issuecomment-2128478041 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=49) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=49=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=49=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=49=false=true)  No data about Coverage  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=49=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=49) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12264 only call save() if there's something to save [sling-org-apache-sling-jcr-repoinit]
sonarcloud[bot] commented on PR #52: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/52#issuecomment-2128474929 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=52) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=52=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=52=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=52=false=true)  [55.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=52=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=52=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=52) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12262 use a metric to indicate repoinit failures [sling-org-apache-sling-jcr-repoinit]
sonarcloud[bot] commented on PR #51: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/51#issuecomment-2128474359 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=51) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=51=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=51=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=51=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=51=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=51=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=51) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
jsedding commented on code in PR #9: URL: https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1611777912 ## src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java: ## @@ -707,6 +716,7 @@ private UpdateResult handleUpdate(final String resourceType, dataNode.setProperty(PROP_MODIFIED, Calendar.getInstance()); dataNode.setProperty(PROP_ENC, ENCODING); dataNode.setProperty(PROP_MIME, MIME_TXT); +dataNode.setProperty(ORIGINAL_PID, id); Review Comment: I suggest we drop the original.pid. Instead we go with your approach of checking for the legacy pid (https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13/files#diff-ddf6029cf7d9aa5ed2720412a7fe8de77d7f5df784c32864c6abc54393e78321R95-R97). That isn't quite as generic as I had originally intended, but it should be good enough. Unless I am missing something. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
jsedding merged PR #53: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
jsedding commented on code in PR #13:
URL:
https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#discussion_r1611600431
##
src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigInstallTask.java:
##
@@ -92,6 +92,9 @@ public void execute(final InstallationContext ctx) {
}
Configuration config =
ConfigUtil.getConfiguration(this.getConfigurationAdmin(), this.factoryPid,
this.configPid);
+if (config == null) {
+config =
ConfigUtil.getLegacyFactoryConfig(this.getConfigurationAdmin(),
this.factoryPid, null, this.configPid);
+}
Review Comment:
I think this change here is actually "good enough".
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
jsedding commented on code in PR #53:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#discussion_r1611598811
##
src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java:
##
@@ -118,23 +116,25 @@ public NodePropertiesVisitor(Session s) {
* is the same as the autocreated default value
*
* @param n the node to check
- * @param pRelPath the property relative path to check
+ * @param propertyPath the property relative path to check
* @return true or false
*/
-protected static boolean isUnchangedAutocreatedProperty(Node n, final
String pRelPath)
+protected static boolean isUnchangedAutocreatedProperty(Node n, final
String propertyPath)
throws RepositoryException {
boolean sameAsDefault = false;
// deal with the pRelPath nesting
-Path path = Paths.get(pRelPath);
-Path parentPath = path.getParent();
-String name = path.getFileName().toString();
-if (parentPath != null) {
-String relPath = parentPath.toString();
-if (n.hasNode(relPath)) {
-n = n.getNode(relPath);
-} else {
-n = null;
+final int pos = propertyPath.lastIndexOf("/");
Review Comment:
... and then finally addressed by using the JCR API instead of
reimplementing its functionality.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
jsedding commented on code in PR #53:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#discussion_r1611596184
##
src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java:
##
@@ -118,42 +116,25 @@ public NodePropertiesVisitor(Session s) {
* is the same as the autocreated default value
*
* @param n the node to check
- * @param pRelPath the property relative path to check
+ * @param propertyPath the property relative path to check
* @return true or false
*/
-protected static boolean isUnchangedAutocreatedProperty(Node n, final
String pRelPath)
+protected static boolean isUnchangedAutocreatedProperty(Node n, final
String propertyPath)
throws RepositoryException {
-boolean sameAsDefault = false;
-
-// deal with the pRelPath nesting
-Path path = Paths.get(pRelPath);
-Path parentPath = path.getParent();
-String name = path.getFileName().toString();
-if (parentPath != null) {
-String relPath = parentPath.toString();
-if (n.hasNode(relPath)) {
-n = n.getNode(relPath);
-} else {
-n = null;
-}
-}
-
-// if the property has been set by being autocreated and the value is
still
-// the same as the default values then also allow changing the value
-if (n != null && n.hasProperty(name)) {
-@Nullable
-PropertyDefinition pd = resolvePropertyDefinition(name, n);
+if (n.hasProperty(propertyPath)) {
Review Comment:
exactly. it also handles trailing slashes. so most of the code in this
method was re-implementing functionality that is already present in JCR.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
joerghoh commented on code in PR #53:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#discussion_r1611590909
##
src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java:
##
@@ -118,42 +116,25 @@ public NodePropertiesVisitor(Session s) {
* is the same as the autocreated default value
*
* @param n the node to check
- * @param pRelPath the property relative path to check
+ * @param propertyPath the property relative path to check
* @return true or false
*/
-protected static boolean isUnchangedAutocreatedProperty(Node n, final
String pRelPath)
+protected static boolean isUnchangedAutocreatedProperty(Node n, final
String propertyPath)
throws RepositoryException {
-boolean sameAsDefault = false;
-
-// deal with the pRelPath nesting
-Path path = Paths.get(pRelPath);
-Path parentPath = path.getParent();
-String name = path.getFileName().toString();
-if (parentPath != null) {
-String relPath = parentPath.toString();
-if (n.hasNode(relPath)) {
-n = n.getNode(relPath);
-} else {
-n = null;
-}
-}
-
-// if the property has been set by being autocreated and the value is
still
-// the same as the default values then also allow changing the value
-if (n != null && n.hasProperty(name)) {
-@Nullable
-PropertyDefinition pd = resolvePropertyDefinition(name, n);
+if (n.hasProperty(propertyPath)) {
Review Comment:
so do I get that right, that you skip the handling of the relative path etc
because ``node.hasProperty`` can handle relative paths and no manual resolution
is required?
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12262 use a metric to indicate repoinit failures [sling-org-apache-sling-jcr-repoinit]
joerghoh commented on code in PR #51: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/51#discussion_r1599049824 ## pom.xml: ## @@ -158,6 +158,12 @@ 4.3.1 test + +org.apache.sling +org.apache.sling.commons.metrics +1.2.12 + Review Comment: fixed. ## src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java: ## @@ -78,14 +79,22 @@ public class RepositoryInitializerFactory implements SlingRepositoryInitializer private final Logger log = LoggerFactory.getLogger(getClass()); +private static final String METRIC_REPOINIT_FAILED = RepositoryInitializerFactory.class.getName() + ".failed"; @Reference private RepoInitParser parser; @Reference private JcrRepoInitOpsProcessor processor; + +@Reference +MetricsService metrics; private RepositoryInitializerFactory.Config config; + +// assume that repoinit succeeds ... and just this to true if it fails +private boolean aRepoInitStatementFailed = false; Review Comment: Agree on that. Replaced it with an AtomicBoolean. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
sonarcloud[bot] commented on PR #53: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#issuecomment-2126808416 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)  [83.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
jsedding commented on code in PR #53:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#discussion_r1611427296
##
src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java:
##
@@ -118,23 +116,25 @@ public NodePropertiesVisitor(Session s) {
* is the same as the autocreated default value
*
* @param n the node to check
- * @param pRelPath the property relative path to check
+ * @param propertyPath the property relative path to check
* @return true or false
*/
-protected static boolean isUnchangedAutocreatedProperty(Node n, final
String pRelPath)
+protected static boolean isUnchangedAutocreatedProperty(Node n, final
String propertyPath)
throws RepositoryException {
boolean sameAsDefault = false;
// deal with the pRelPath nesting
-Path path = Paths.get(pRelPath);
-Path parentPath = path.getParent();
-String name = path.getFileName().toString();
-if (parentPath != null) {
-String relPath = parentPath.toString();
-if (n.hasNode(relPath)) {
-n = n.getNode(relPath);
-} else {
-n = null;
+final int pos = propertyPath.lastIndexOf("/");
Review Comment:
addressed by moving to `org.apache.jackrabbit.util.Text`
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
rishabhdaim commented on code in PR #9:
URL:
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1611358227
##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrUtil.java:
##
@@ -73,4 +75,21 @@ public static Node createPath(final Session session,
}
return parentNode.getNode(relativePath);
}
+
+/**
+ * Get the PID for a configuration event by using the R7 format before
saving it.
+ *
+ * @param factoryPid factory PID of the configuration
+ * @param pid PID of the configuration
+ * @return The PID in R7 format
+ */
+public static String getPid(final String factoryPid, final String pid) {
+// if factory pid is separated from pid by a period (.), we need
replace it with a ~ so that this can be installed
+// and grouped as a factory configuration
+if (pid.startsWith(factoryPid + '.')) {
+String id = pid.substring(factoryPid.length() + 1);
+return factoryPid + "~" + id;
+}
+return pid;
Review Comment:
Addressed in
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9/commits/49da45e49774d6f9b8597bbc6376c527ef5970f3
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
reschke commented on code in PR #53:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#discussion_r1611357821
##
src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java:
##
@@ -118,23 +116,25 @@ public NodePropertiesVisitor(Session s) {
* is the same as the autocreated default value
*
* @param n the node to check
- * @param pRelPath the property relative path to check
+ * @param propertyPath the property relative path to check
* @return true or false
*/
-protected static boolean isUnchangedAutocreatedProperty(Node n, final
String pRelPath)
+protected static boolean isUnchangedAutocreatedProperty(Node n, final
String propertyPath)
throws RepositoryException {
boolean sameAsDefault = false;
// deal with the pRelPath nesting
-Path path = Paths.get(pRelPath);
-Path parentPath = path.getParent();
-String name = path.getFileName().toString();
-if (parentPath != null) {
-String relPath = parentPath.toString();
-if (n.hasNode(relPath)) {
-n = n.getNode(relPath);
-} else {
-n = null;
+final int pos = propertyPath.lastIndexOf("/");
Review Comment:
just checking: we know that there's no trailing "/" here?
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
sonarcloud[bot] commented on PR #53: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#issuecomment-2126642430 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)  [85.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
sonarcloud[bot] commented on PR #53: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#issuecomment-2126609772 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)  [85.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]
jsedding opened a new pull request, #53: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53 (no comment) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] SLING-12321 update parent to 60 [sling-org-apache-sling-engine]
joerghoh opened a new pull request, #46: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/46 (no comment) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12322 Stream Reader should Return SC_OK if content length is le… [sling-org-apache-sling-servlets-get]
abhishekgarg18 closed pull request #14: SLING-12322 Stream Reader should Return SC_OK if content length is le… URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/14 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] SLING-12322 Stream Reader should Return SC_OK if content length is le… [sling-org-apache-sling-servlets-get]
abhishekgarg18 opened a new pull request, #14: URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/14 …ss than specified range -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12317 - Make Sling Engine build with Java 21 [sling-org-apache-sling-engine]
rombert commented on PR #45: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/45#issuecomment-2124809600 @joerghoh - all done. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12317 - Make Sling Engine build with Java 21 [sling-org-apache-sling-engine]
rombert merged PR #45: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/45 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12318 apply spotless formatting [sling-org-apache-sling-engine]
rombert merged PR #44: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/44 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12317 - Make Sling Engine build with Java 21 [sling-org-apache-sling-engine]
joerghoh commented on PR #45: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/45#issuecomment-2124762983 @rombert would appreciate that. I plan to upgrade parent pom afterwards (SLING-12321) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
jsedding commented on code in PR #9:
URL:
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1609457830
##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrUtil.java:
##
@@ -73,4 +75,21 @@ public static Node createPath(final Session session,
}
return parentNode.getNode(relativePath);
}
+
+/**
+ * Get the PID for a configuration event by using the R7 format before
saving it.
+ *
+ * @param factoryPid factory PID of the configuration
+ * @param pid PID of the configuration
+ * @return The PID in R7 format
+ */
+public static String getPid(final String factoryPid, final String pid) {
+// if factory pid is separated from pid by a period (.), we need
replace it with a ~ so that this can be installed
+// and grouped as a factory configuration
+if (pid.startsWith(factoryPid + '.')) {
+String id = pid.substring(factoryPid.length() + 1);
+return factoryPid + "~" + id;
+}
+return pid;
Review Comment:
I was thinking to keep this slightly more generic (note: untested code
below!):
```suggestion
final String name;
if (pid.matches("^" + factoryPid + "[^a-zA-Z0-9\\s]")) {
name = pid.substring(factoryPid.length() + 1);
} else {
name = pid;
}
return factoryPid + "~" + name;
```
WDYT?
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12317 - Make Sling Engine build with Java 21 [sling-org-apache-sling-engine]
rombert commented on PR #45: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/45#issuecomment-2124069446 @joerghoh - do you want to merge #44 first? I can then rebase, if needed. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
rishabhdaim commented on code in PR #9: URL: https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608439188 ## src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java: ## @@ -707,6 +716,7 @@ private UpdateResult handleUpdate(final String resourceType, dataNode.setProperty(PROP_MODIFIED, Calendar.getInstance()); dataNode.setProperty(PROP_ENC, ENCODING); dataNode.setProperty(PROP_MIME, MIME_TXT); +dataNode.setProperty(ORIGINAL_PID, id); Review Comment: I didn't get any exceptions while creating a config on AEM 1, and it was successfully replicated on AEM 2. I set up both instances using Mongo document store. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
rishabhdaim commented on code in PR #13:
URL:
https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#discussion_r1608464507
##
src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigInstallTask.java:
##
@@ -92,6 +92,9 @@ public void execute(final InstallationContext ctx) {
}
Configuration config =
ConfigUtil.getConfiguration(this.getConfigurationAdmin(), this.factoryPid,
this.configPid);
+if (config == null) {
+config =
ConfigUtil.getLegacyFactoryConfig(this.getConfigurationAdmin(),
this.factoryPid, null, this.configPid);
+}
Review Comment:
I agree that I haven't checked whether the `original.pid` property is
present or not cause I didn't need it (may be we can remove it altogether).
Regarding the checking of config, the `ConfigUtil.getLegacyFactoryConfig`
does check it
[here](https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/blob/master/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java#L251-L256)
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
rishabhdaim commented on code in PR #9:
URL:
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608455987
##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrUtil.java:
##
@@ -73,4 +75,21 @@ public static Node createPath(final Session session,
}
return parentNode.getNode(relativePath);
}
+
+/**
+ * Get the PID for a configuration event by using the R7 format before
saving it.
+ *
+ * @param factoryPid factory PID of the configuration
+ * @param pid PID of the configuration
+ * @return The PID in R7 format
+ */
+public static String getPid(final String factoryPid, final String pid) {
+// if factory pid is separated from pid by a period (.), we need
replace it with a ~ so that this can be installed
+// and grouped as a factory configuration
+if (pid.startsWith(factoryPid + '.')) {
+String id = pid.substring(factoryPid.length() + 1);
+return factoryPid + "~" + id;
+}
+return pid;
Review Comment:
Addressed in
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9/commits/6b9cdd225224636759cd64a96328c9b573316aff
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
rishabhdaim commented on code in PR #9: URL: https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608439188 ## src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java: ## @@ -707,6 +716,7 @@ private UpdateResult handleUpdate(final String resourceType, dataNode.setProperty(PROP_MODIFIED, Calendar.getInstance()); dataNode.setProperty(PROP_ENC, ENCODING); dataNode.setProperty(PROP_MIME, MIME_TXT); +dataNode.setProperty(ORIGINAL_PID, id); Review Comment: I didn't get any exceptions while creating a config on AEM 1 and it gets replicated on AEM 2 successfully. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
jsedding commented on code in PR #9:
URL:
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608329623
##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrUtil.java:
##
@@ -73,4 +75,21 @@ public static Node createPath(final Session session,
}
return parentNode.getNode(relativePath);
}
+
+/**
+ * Get the PID for a configuration event by using the R7 format before
saving it.
+ *
+ * @param factoryPid factory PID of the configuration
+ * @param pid PID of the configuration
+ * @return The PID in R7 format
+ */
+public static String getPid(final String factoryPid, final String pid) {
+// if factory pid is separated from pid by a period (.), we need
replace it with a ~ so that this can be installed
+// and grouped as a factory configuration
+if (pid.startsWith(factoryPid + '.')) {
+String id = pid.substring(factoryPid.length() + 1);
+return factoryPid + "~" + id;
+}
+return pid;
Review Comment:
I would flip the logic. I.e. let a pid in the correct format through. If the
format is incorrect, normalize the pid to a name by removing a prefix of
`factoryPid` if present. Then concat the factoryPid, ~ and normalized pid.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
jsedding commented on code in PR #13:
URL:
https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#discussion_r1608323556
##
src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigInstallTask.java:
##
@@ -92,6 +92,9 @@ public void execute(final InstallationContext ctx) {
}
Configuration config =
ConfigUtil.getConfiguration(this.getConfigurationAdmin(), this.factoryPid,
this.configPid);
+if (config == null) {
+config =
ConfigUtil.getLegacyFactoryConfig(this.getConfigurationAdmin(),
this.factoryPid, null, this.configPid);
+}
Review Comment:
I don't think this is a complete fix (together with
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9).
The `original.pid` property is not leveraged. Installation would need to be
skipped if the `original.pid` property is present AND a configuration with that
PID exists in config admin.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
jsedding commented on code in PR #9: URL: https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608319420 ## src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java: ## @@ -707,6 +716,7 @@ private UpdateResult handleUpdate(final String resourceType, dataNode.setProperty(PROP_MODIFIED, Calendar.getInstance()); dataNode.setProperty(PROP_ENC, ENCODING); dataNode.setProperty(PROP_MIME, MIME_TXT); +dataNode.setProperty(ORIGINAL_PID, id); Review Comment: Does this work, seeting a property on the `jcr:content` node? I think the `nt:resource` node type is quite restrictive. IIRC Oak uses a different node-type for a file's `jcr:content` node. Maybe that one is more lenient. Alternatively, a mixin nodetype could be added to allow for the extra property. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Update dependency org.codehaus.mojo:build-helper-maven-plugin to v3.6.0 [sling-site]
rombert merged PR #164: URL: https://github.com/apache/sling-site/pull/164 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
raducotescu merged PR #43: URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
sonarcloud[bot] commented on PR #43: URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#issuecomment-2120129661 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-resource=43) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource=43=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource=43=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-resource=43=false=true)  [91.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource=43=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource=43=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-resource=43) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
raducotescu commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1606549396
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
Review Comment:
This should be now clear with the changes from
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43/commits/b54140babc116cad4cefce98c7c57670145a18bc.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
rishabhdaim opened a new pull request, #9: URL: https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9 …present -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
sonarcloud[bot] commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#issuecomment-2120007654 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-installer-factory-configuration=13=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Update dependency org.codehaus.mojo:build-helper-maven-plugin to v3.6.0 [sling-site]
renovate-bot opened a new pull request, #164:
URL: https://github.com/apache/sling-site/pull/164
[](https://renovatebot.com)
This PR contains the following updates:
| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[org.codehaus.mojo:build-helper-maven-plugin](https://www.mojohaus.org/build-helper-maven-plugin/)
([source](https://togithub.com/mojohaus/build-helper-maven-plugin)) | `3.5.0`
-> `3.6.0` |
[](https://docs.renovatebot.com/merge-confidence/)
|
[](https://docs.renovatebot.com/merge-confidence/)
|
[](https://docs.renovatebot.com/merge-confidence/)
|
[](https://docs.renovatebot.com/merge-confidence/)
|
---
> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency Dashboard
for more information.
---
### Release Notes
mojohaus/build-helper-maven-plugin
(org.codehaus.mojo:build-helper-maven-plugin)
###
[`v3.6.0`](https://togithub.com/mojohaus/build-helper-maven-plugin/releases/tag/3.6.0)
[Compare
Source](https://togithub.com/mojohaus/build-helper-maven-plugin/compare/3.5.0...3.6.0)
Changes
New features and improvements
- Deprecate remove-project-artifact goal
([#205](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/205))
[@slawekjaranowski](https://togithub.com/slawekjaranowski)
- Parallel execution of uptodate-properties
([#201](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/201))
[@mkarg](https://togithub.com/mkarg)
Dependency updates
- Bump org.codehaus.mojo:mojo-parent from 81 to 82
([#206](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/206))
[@dependabot](https://togithub.com/dependabot)
- Bump org.codehaus.mojo:mojo-parent from 78 to 81
([#204](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/204))
[@dependabot](https://togithub.com/dependabot)
- Bump org.codehaus.plexus:plexus-utils from 4.0.0 to 4.0.1
([#202](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/202))
[@dependabot](https://togithub.com/dependabot)
- Bump apache/maven-gh-actions-shared from 3 to 4
([#200](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/200))
[@dependabot](https://togithub.com/dependabot)
- Bump release-drafter/release-drafter from 5 to 6
([#195](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/195))
[@dependabot](https://togithub.com/dependabot)
- Bump org.codehaus.mojo:mojo-parent from 77 to 78
([#193](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/193))
[@dependabot](https://togithub.com/dependabot)
Maintenance
- Delete link to remove-project-artifact as is deprecated
([#210](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/210))
[@slawekjaranowski](https://togithub.com/slawekjaranowski)
- Cleanups dependencies
([#209](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/209))
[@slawekjaranowski](https://togithub.com/slawekjaranowski)
- Remove public modifiers from JUnit 5 tests
([#208](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/208))
[@slawekjaranowski](https://togithub.com/slawekjaranowski)
- Delete example about remove-project-artifact as is deprecated
([#207](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/207))
[@slawekjaranowski](https://togithub.com/slawekjaranowski)
- Fix goal in usage add-test-resource example
([#199](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/199))
[@mfussenegger](https://togithub.com/mfussenegger)
Build
- Use shared action for release drafter
([#203](https://togithub.com/mojohaus/build-helper-maven-plugin/pull/203))
[@slawekjaranowski](https://togithub.com/slawekjaranowski)
---
### Configuration
**Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).
**Automerge**: Disabled by config. Please merge this manually once you are
satisfied.
♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry
checkbox.
**Ignore**: Close this PR and you won't be reminded about this update
again.
---
- [ ] If you want to rebase/retry this PR, check this
box
---
This PR
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
anchela commented on PR #43: URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#issuecomment-2117930435 @raducotescu , as discussed... i am fine now but making the comment a bit clearer and maybe also explaining why the extra attribute on the credentials is set might make this easier to read. specially since SlingRepository.impersonateFromService which is called does not require the attribute on the credentials -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
anchela commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605249171
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
+String sudoUser = getSudoUser(authenticationInfo);
+if (sudoUser != null) {
+SimpleCredentials creds = new
SimpleCredentials(sudoUser, new char[0]);
+
creds.setAttribute(ResourceResolver.USER_IMPERSONATOR, subServiceName == null ?
Review Comment:
@raducotescu , as discussed in private ok with leaving it in since
apparently the resource-resolver api allows to retrieve the impersonator and
does so via a session attribute.
althought that feels a bit awkward to me to use a jcr-session attribute for
that which by coincidence gets populated from session attributes *autsch*
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
anchela commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605245518
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
Review Comment:
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
raducotescu commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605242759
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
Review Comment:
It is a shortcut, since impersonation would have been handled in
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/f82678ae650a67538d261bcdc5d1e254039e9a37/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java#L150,
through calling
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/f82678ae650a67538d261bcdc5d1e254039e9a37/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java#L189.
I think I should rephrase the comment to "[...] impersonation for services
[...]", since that was my intention.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
raducotescu commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605239318
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
+String sudoUser = getSudoUser(authenticationInfo);
+if (sudoUser != null) {
+SimpleCredentials creds = new
SimpleCredentials(sudoUser, new char[0]);
+
creds.setAttribute(ResourceResolver.USER_IMPERSONATOR, subServiceName == null ?
Review Comment:
That attribute seems to be retrieved via
`org.apache.sling.jcr.resource.internal.helper.jcr.JcrResourceProvider#getAttribute`
for `org.apache.sling.api.resource.ResourceResolver#getAttribute` calls.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
anchela commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605209974
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
+String sudoUser = getSudoUser(authenticationInfo);
+if (sudoUser != null) {
+SimpleCredentials creds = new
SimpleCredentials(sudoUser, new char[0]);
+
creds.setAttribute(ResourceResolver.USER_IMPERSONATOR, subServiceName == null ?
Review Comment:
why is that attribute needed?
SlingRepository.impersonateFromService just takes a credential object and
the method itself already implies that an impersonation is requested to be
performed.
the extra attribute is not needed afaik and the implementation of
SlingRepository.impersonateFromService that is executed in AEM and the default
in sling
(https://github.com/apache/sling-org-apache-sling-jcr-base/blob/master/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepository2.java#L391-L427)
don't respect it. but maybe that explains my confusion with the comment (see
above)
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
Review Comment:
the comment confuses me... what you are handling here is getServiceResolver
request that in addition comes with a hint that an impersonation should be
performed i.e. calling impersonateFromService instead of just loginService.
the first part 'impersonation for service users is the one that confuses
me impersonateFromService means that a service should perform an
impersonation of any user. from the osgi service (bundle/subservice) a given
specified user should be impersonated. it's not that the service user is in
charge here... it could be but that's an impl detail.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
sonarcloud[bot] commented on PR #43: URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#issuecomment-2117858021 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-resource=43) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource=43=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource=43=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-resource=43=false=true)  [91.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource=43=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource=43=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-resource=43) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
raducotescu opened a new pull request, #43: URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43 * for service users that want to impersonate, simply call SlingRepository#impersonateFromService -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11716 ability to cache the results of a caconfig lookup [sling-org-apache-sling-caconfig-impl]
stefanseifert commented on PR #9: URL: https://github.com/apache/sling-org-apache-sling-caconfig-impl/pull/9#issuecomment-2116999103 lgtm in general, some remarks: * making it configurable with default switched off is not really helpful - either we consider this caching safe then we can enable it by default - or if not, what can we do to make it safe * when may this caching be harmful? i see two use cases ** you have a long-running resource resolver e.g. in an OSGi service. this is an anti-pattern, but sometimes required e.g. if you are listing for resource change events. ** you are actually writing caconfig in your current request and reading it later (most time this is a rare use case) * maybe instead of having an OSGi configuration we should add a switch to the caconfig API where you can disable the caching if you are using caconfig in one of this use cases * more cosmetic: we should adapt the pattern to build cache keys to use separator chars which are not allowed in JCR paths and not "--" - to be on the safe side. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] chore(deps): update dependency org.apache.sling:org.apache.sling.jcr.resource to v3.3.0 [sling-org-apache-sling-starter]
rombert merged PR #328: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/328 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] chore(deps): update dependency org.apache.sling:org.apache.sling.jcr.resource to v3.3.0 [sling-org-apache-sling-starter]
renovate-bot opened a new pull request, #328: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/328 [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [org.apache.sling:org.apache.sling.jcr.resource](https://sling.apache.org) ([source](https://togithub.com/apache/sling-org-apache-sling-jcr-resource)) | `3.2.4` -> `3.3.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/apache/sling-org-apache-sling-starter). -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12318 apply spotless formatting [sling-org-apache-sling-engine]
sonarcloud[bot] commented on PR #44: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/44#issuecomment-2113699160 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-engine=44) **Quality Gate failed** Failed conditions  [34.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-engine=44=new_coverage=list) (required ≥ 80%) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-engine=44) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12317 - Make Sling Engine build with Java 21 [sling-org-apache-sling-engine]
sonarcloud[bot] commented on PR #45: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/45#issuecomment-2113698925 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-engine=45) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-engine=45=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-engine=45=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-engine=45=false=true)  No data about Coverage  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-engine=45=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-engine=45) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12318 apply spotless formatting [sling-org-apache-sling-engine]
joerghoh commented on PR #44: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/44#issuecomment-2113422171 @rombert When I tried to make the update of the parent pom and the test dependencies, the spotless plugin failed the build for me; and for that reason I wanted to have the formatting change before I do the update of the pom. In other words: we need #44 **and** #45 merged (preferably in this order). -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12316 servlet name should always be re-set properly [sling-org-apache-sling-engine]
joerghoh merged PR #43: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/43 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
sonarcloud[bot] commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#issuecomment-2113317468 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) **Quality Gate passed** Issues  [7 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
sonarcloud[bot] commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#issuecomment-2113316059 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) **Quality Gate passed** Issues  [7 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
sonarcloud[bot] commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#issuecomment-2113311666 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) **Quality Gate passed** Issues  [7 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
sonarcloud[bot] commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#issuecomment-2113206440 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) **Quality Gate passed** Issues  [7 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-installer-factory-configuration=13=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-installer-factory-configuration=13=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-installer-factory-configuration=13) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2112861725 FTR: 1. Yes, we want to completely remove even the shaded Guava variant, but it'll take time. 2. *Major* version updates of Guava happened ~ once every year since 2020, so the situation might not be that bad, -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2112842362 Echoing a direct conversation I had with @reschke - it's hard for me to formulate an ask to the Oak project since we can pull in a lot of components (from oak-core?) which may be using Guava without exposing it as an API. One thing we could improve for this bundle, if we chose to go the way of removing Guava usages, is to fail the build if such imports are found. I haven't found a good way of doing this though. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12317 - Make Sling Engine build with Java 21 [sling-org-apache-sling-engine]
sonarcloud[bot] commented on PR #45: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/45#issuecomment-2112761541 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-engine=45) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-engine=45=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-engine=45=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-engine=45=false=true)  No data about Coverage  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-engine=45=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-engine=45) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12318 apply spotless formatting [sling-org-apache-sling-engine]
rombert commented on PR #44: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/44#issuecomment-2112750535 @joerghoh - I think I already fixed the Java 21 build ... #45 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] SLING-12317 - Make Sling Engine build with Java 21 [sling-org-apache-sling-engine]
rombert opened a new pull request, #45: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/45 Update to the latest JMock and byte-buddy versions. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12318 apply spotless formatting [sling-org-apache-sling-engine]
joerghoh commented on PR #44: URL: https://github.com/apache/sling-org-apache-sling-engine/pull/44#issuecomment-2112724636 (failed for not building on Java 21, which I plan to fix asap) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2112496633 It's not clear to me yet what makes those Oak objects special. Or are they really special? At the end of the day, each repostiory instance will end up with some sort of (shaded) Guava requirement, until we are done with the removal. So it's not even clear whether fixing particular ones will get us anywhere. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
rombert commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#issuecomment-2112452359 @rishabhdaim - can you please merge with the latest master commit? https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/commit/fd459daca46c60aeac53c33e699a8973111ba4cf would make our testing easier. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12283 : changed the config Pid to separate factoryPID & PID wit… [sling-org-apache-sling-installer-factory-configuration]
rombert commented on code in PR #13:
URL:
https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13#discussion_r1601372208
##
src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java:
##
@@ -336,4 +337,20 @@ public static void removeRedundantProperties(final
Dictionary pr
}
}
}
+
+public static String getPid(final ConfigurationEvent event) {
+final String id;
+final String pid;
+if (event.getFactoryPid() == null ) {
Review Comment:
I think the code would be clearer if you would check if you need to after
the pid ( branch on line 347 ) and then have a fallback to `id =
event.getPid();`.
##
src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java:
##
@@ -131,10 +131,11 @@ public InstallTask createTask(final TaskResourceGroup
group) {
@Override
public void configurationEvent(final ConfigurationEvent event) {
synchronized ( Coordinator.SHARED ) {
+final String id = ConfigUtil.getPid(event);
Review Comment:
I think we need a more significant name than "id".
##
src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java:
##
@@ -131,10 +131,11 @@ public InstallTask createTask(final TaskResourceGroup
group) {
@Override
public void configurationEvent(final ConfigurationEvent event) {
synchronized ( Coordinator.SHARED ) {
+final String id = ConfigUtil.getPid(event);
if ( event.getType() == ConfigurationEvent.CM_DELETED ) {
final Coordinator.Operation op =
Coordinator.SHARED.get(event.getPid(), event.getFactoryPid(), true);
Review Comment:
It would be good to document somewhere by we pass the original PID and
Factory PID to the coordinator but the recalculated one elsewhere.
##
src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java:
##
@@ -336,4 +337,20 @@ public static void removeRedundantProperties(final
Dictionary pr
}
}
}
+
Review Comment:
Please document why this is needed
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12315: avoid new registrations during deactivation [sling-org-apache-sling-i18n]
jsedding commented on code in PR #14:
URL:
https://github.com/apache/sling-org-apache-sling-i18n/pull/14#discussion_r1601048338
##
src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java:
##
@@ -436,8 +439,10 @@ protected void deactivate() {
this.locatorPathsTracker = null;
}
-clearCache();
-this.bundleContext = null;
+synchronized(this) {
Review Comment:
Agreed that the thread-safety model of the class is confusing.
That said, I consider the `bundleContext` and `bundleServiceRegistrations`
fields to have a strong relation, because the registrations implicitly pertain
to the bundle context. The synchronized block in deactivate makes sure that no
thread-scheduling can lead to a registration being added to
`bundleServiceRegistrations` once `bundleContext` has been set to `null`. Such
registrations would never be unregistered, which is why I think it is important
to guard against this case.
In order to simplify the thread-safety model, it might be sensible to
encapsulate the concern of "bundle service registrations and deregistration"
into its own class. This class would receive the bundle context in the
constructor, removing the need for a `bundleContext` field in
`JcrResourceBundleProvider`. All registrations, unregistrations, book keeping
of registrations, and thread-safety concerns would be encapsulated. The object
would be created in the activate method and closed in the deactivate method.
WDYT? I believe that could be a start towards reducing complexity.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12315: avoid new registrations during deactivation [sling-org-apache-sling-i18n]
rombert commented on code in PR #14:
URL:
https://github.com/apache/sling-org-apache-sling-i18n/pull/14#discussion_r1599862705
##
src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java:
##
@@ -436,8 +439,10 @@ protected void deactivate() {
this.locatorPathsTracker = null;
}
-clearCache();
-this.bundleContext = null;
+synchronized(this) {
Review Comment:
I think you're looking at interleaved access between `deactivate()` and
`registerResourceBundle`, right? That is the only scenario where
`bundleServiceRegistrations` will receive new entries.
It looks like it's theoretically possible for this scenario to happen,
although I see the `bundleContext != null` checks on every code path and I am
very skeptical that a runtime compiler will reorder staments in face of
multi-thread volatile reads and writes.
I find the overall thread safety model of this class quite confusing (
multiple volatile fields, concurrent maps with Semaphore keys, synchronized
blocks ( on `this` (!) , on a private collection ) and I think expanding the
scope of the synchronized block would make things more confusing.
If you think that it's worth protecting the `bundleContext` and
`bundleServiceRegistrations` together I would prefer to not make the
`bundleContext` synchronized and instead guard access to those two fields all
the time using the same lock. Of course, that can have performance implications
so care must be taken.
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2109422262 @rombert - is there something we can do systematically in Oak (other than removing Guava completely) at this point? Also, can you file tickets for the classes where you noticed issues? -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12262 use a metric to indicate repoinit failures [sling-org-apache-sling-jcr-repoinit]
sonarcloud[bot] commented on PR #51: URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/51#issuecomment-2109187204 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=51) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=51=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=51=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=51=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=51=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=51=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=51) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107970625 Yes, indeed. So it seems we we missed the case implementation objects leak the dependency. For now we should open tickets for each that we encounter, we might be able to address those in Oak 1.64.0. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107641629 > Yes, that's why we have removed Oak from the public Oak API. I think you meant 'removed Guava', right? At any rate, for closely integrated bundles like this one it looks that we need more that public APIs. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107614606 Yes, that's why we have removed Oak from the public Oak API. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107549956 > Oak's shaded Guava lib should be treated just as any other Oak component, such as [oak-core](https://issues.apache.org/jira/browse/OAK-core). I'm not sure what's different? Because of the simplistic way the OSGi package versions are managed. Almost each shaded Guava update is a major version bump to consumers which causes headaches when trying to support multiple Oak versions. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107533908 Oak's shaded Guava lib should be treated just as any other Oak component, such as oak-core. I'm not sure what's different? -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107526113 @reschke - the problem is that there seems to be no way to have a SlingRepository implementation without pulling in Guava transitively. If you do see a way of doing that without linking to various core classes, please let me know. And the inlining is only about the shaded Guava classes, not other internals. What concerns would you have with that? -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107521572 And no, by all means do not inline internal Oak classes! -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107516285 The goal definitively way for Oak not to leak any Guava-ish stuff in the public APIs. AFAICT, this has been the case for approximately one year now. That said, there are indeed runtime dependencies; and removing them is only a mid-term goal. So... I'm not really sure what problem we're discussing here. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
rombert commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2107505437 > I'm not sure what you mean by a "large number" of source files? There are only 4 files under src/main/java/ch/qos/logback and only 1 of those files copies any code blocks from the original vendor. The other three extend the original classes and add to it or override to provide a different implementation. Oh, I misread your PR. I thought that the large number of changes was due to copying logback code; my mistake. > Anyways with that said, I am not interested in spending any more time on solving this in another way That is a shame, but I can understand your reasoning. Let's please keep this open for now and see if anyone else is willing to extend it and - indeed - review it. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107499868 @enapps-enorman - good catch with the `AuthInfoImpl`. I hand-rolled a different implementation (which does not seem to work, not sure why). But we still have the guava classess pulled in, e.g. via `org.apache.jackrabbit.oak.plugins.commit.JcrConflictHandler`. I think it is going to be very hard to make sure we don't transitively pull in Guava classes from Oak, unfortunately. Perhaps @reschke can comment on that. Regarding the actual fix, we can either - go with this fix and keep making new releases for new Oak versions - go with a wider bnd range ( and probably add extra tests ) ; we would still need to keep making releases for new Oak versions but users of older versions of Oak can upgrade to newer versions of the bundle - inline the required guava classes in this bundle so we can get wide compatibility; a good start for this is below ``` Private-Package: org.apache.jackrabbit.guava.common.base, \ org.apache.jackrabbit.guava.common.collect, \ org.apache.jackrabbit.guava.common.math, \ org.apache.jackrabbit.guava.common.base.internal, \ org.apache.jackrabbit.guava.common.primitives ``` I'll leave this with you. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12312 add jakarta.servlet support in RequestLocaleResolve [sling-org-apache-sling-i18n]
rombert commented on PR #13: URL: https://github.com/apache/sling-org-apache-sling-i18n/pull/13#issuecomment-2107431441 > Why can't the EE8 loyalists just stay on the old version of org.apache.sling.i18n? I don't think "loyalists" is the right term here. And we don't typically get to chose how our users manage their deployments. I'll start a dev@sling thread on this, I think it's important to have a clear policy. -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12315: avoid new registrations during deactivation [sling-org-apache-sling-i18n]
sonarcloud[bot] commented on PR #14: URL: https://github.com/apache/sling-org-apache-sling-i18n/pull/14#issuecomment-2107393290 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-i18n=14) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-i18n=14=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-i18n=14=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-i18n=14=false=true)  [81.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-i18n=14=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-i18n=14=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-i18n=14) -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] chore(deps): update dependency org.apache.felix:org.apache.felix.http.jetty to v5.1.12 [sling-org-apache-sling-starter]
rombert merged PR #324: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/324 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] chore(deps): update dependency org.apache.sling:org.apache.sling.engine to v2.15.14 [sling-org-apache-sling-starter]
rombert merged PR #325: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/325 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] chore(deps): update dependency org.apache.sling:slingfeature-maven-plugin to v1.8.2 [sling-org-apache-sling-starter]
rombert merged PR #326: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/326 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] chore(deps): update dependency commons-codec:commons-codec to v1.17.0 [sling-org-apache-sling-starter]
rombert merged PR #327: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/327 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] chore(deps): update dependency com.fasterxml.jackson.core:jackson-core to v2.17.1 [sling-org-apache-sling-starter]
rombert merged PR #323: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/323 -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] chore(deps): update dependency commons-codec:commons-codec to v1.17.0 [sling-org-apache-sling-starter]
renovate-bot opened a new pull request, #327: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/327 [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [commons-codec:commons-codec](https://commons.apache.org/proper/commons-codec/) ([source](https://togithub.com/apache/commons-codec)) | `1.16.1` -> `1.17.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes apache/commons-codec (commons-codec:commons-codec) ### [`v1.17.0`](https://togithub.com/apache/commons-codec/blob/HEAD/RELEASE-NOTES.txt#Apache-Commons-Codec-1170-RELEASE-NOTES) The Apache Commons Codec component contains encoder and decoders for various formats such as Base16, Base32, Base64, digest, and Hexadecimal. In addition to these widely used encoders and decoders, the codec package also maintains a collection of phonetic encoding utilities. Feature and fix release. Requires a minimum of Java 8. --- ### Configuration **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/apache/sling-org-apache-sling-starter). -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] chore(deps): update dependency org.apache.sling:slingfeature-maven-plugin to v1.8.2 [sling-org-apache-sling-starter]
renovate-bot opened a new pull request, #326: URL: https://github.com/apache/sling-org-apache-sling-starter/pull/326 [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [org.apache.sling:slingfeature-maven-plugin](https://sling.apache.org/components/slingfeature-maven-plugin/) ([source](https://togithub.com/apache/sling-slingfeature-maven-plugin)) | `1.8.0` -> `1.8.2` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/apache/sling-org-apache-sling-starter). -- 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-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
