Re: [PR] SLING-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]

2024-05-23 Thread via GitHub


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-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]

2024-05-23 Thread via GitHub


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]

2024-05-23 Thread via GitHub


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]

2024-05-23 Thread via GitHub


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-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]

2024-05-23 Thread via GitHub


sonarcloud[bot] commented on PR #53:
URL: 
https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#issuecomment-2126808416

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53)
 **Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [1 New 
issue](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=WONTFIX)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [83.3% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [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]

2024-05-23 Thread via GitHub


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-12323 - [RepoInit] Avoid java.nio.file.Path for parsing repository paths [sling-org-apache-sling-jcr-repoinit]

2024-05-23 Thread via GitHub


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]

2024-05-23 Thread via GitHub


sonarcloud[bot] commented on PR #53:
URL: 
https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#issuecomment-2126642430

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53)
 **Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [1 New 
issue](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=WONTFIX)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [85.7% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [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]

2024-05-23 Thread via GitHub


sonarcloud[bot] commented on PR #53:
URL: 
https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/53#issuecomment-2126609772

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-repoinit=53)
 **Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [1 New 
issue](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit=53=WONTFIX)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit=53=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [85.7% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit=53=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [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