You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2021/12/01 01:12:59 UTC
[GitHub] [sling-org-apache-sling-repoinit-parser] klcodanr opened a new pull request #14: SLING-10952 - Adding support for whitespace characters in group names
klcodanr opened a new pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14
Goal here is to be able to define repoinit statements for groups loaded from external sources that contain whitespace in the name. Allows group names to either be quoted or standard strings. Note this only adds creation / deletion support for groups, but theoretically you could update properties and update membership of other types of authorizables with whitespace in the name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] sonarcloud[bot] removed a comment on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1011711570
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list) [88.5% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list) [7.1% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] sonarcloud[bot] removed a comment on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-983188641
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list) [88.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list) [7.4% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] sonarcloud[bot] commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-983188641
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list) [88.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list) [7.4% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] anchela commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
anchela commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1011929603
@klcodanr , i agree with that analysis. changing the method name to what it actually is instead of what it is used for makes sense to me. thanks a lot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] sonarcloud[bot] commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1012169549
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list) [1.5% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] anchela edited a comment on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
anchela edited a comment on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1016534866
@klcodanr , i fear SLING-10996 will be a bigger thing to do. i would not include it in this improvement here, which is primarily about allowing for white space in strings like user/group names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] klcodanr commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
klcodanr commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1011611802
@anchela Good points.
Re:Naming, would you have any suggestion on the name for this class / the related tokens and methods given that it is used for Authorizable ID's and Principal Names?
It feels like it may be easiest / most logical to call it PrincipalName (e.g. PrincipalNameUtil / principalName / principalName()) as long as that's not confusing since it an also be an Authorizable ID.
Would you agree?
Re:the other changes I'll make those and send an update.
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] klcodanr merged pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
klcodanr merged pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/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-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] anchela commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
anchela commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1016534866
@klcodanr , i fear SLING-10996 will be a bigger thing to do. i would not include it in this improvement here, which is primarily about allow for white space in strings like user/group names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] klcodanr commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
klcodanr commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1016513997
@anchela I believe I made all the changes requested. However I do see [SLING-10996](https://issues.apache.org/jira/browse/SLING-10996). Should instead this be aligned with the allowed characters in a JCR local name?
I'd like to get this in, but I'd prefer to do it correctly vs having a _mostly_ though not entirely correct implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] klcodanr commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
klcodanr commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1011776778
I was thinking about this more and the name is misleading about what the purpose of the token is. While it is used for principal names and authorizable ids, it's really a "quotable string" i.e. a string that can either be quoted which allows whitespace or not which does not allow whitespace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] anchela commented on a change in pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
anchela commented on a change in pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#discussion_r782185940
##########
File path: src/main/java/org/apache/sling/repoinit/parser/impl/AuthorizableIdUtil.java
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.repoinit.parser.impl;
+
+import java.util.List;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+public class AuthorizableIdUtil {
+
+ private static final Pattern regex = Pattern.compile("\\s");
+
+ private AuthorizableIdUtil() {
+ // hidden
+ }
+
+ /**
+ * Gets the authorizable ID handling cases where the value should be quoted
+ * (e.g. the authorizable id is not just an alphanumeric string)
+ *
+ * @param authorizableId the authorizable ID to get
+ * @return the (potentially quoted) authorizable id
+ */
+ public static final String forRepoInitString(String authorizableId) {
+ return !regex.matcher(authorizableId).find() ? authorizableId : "\"" + authorizableId + "\"";
+ }
+
+ /**
+ * Gets the authorizable IDs handling cases where the value should be quoted
+ * (e.g. the authorizable id is not just an alphanumeric string)
+ *
+ * @param authorizableIds the authorizable IDs to get
+ * @return the list of (potentially quoted) authorizable ids
+ */
+ public static final List<String> forRepoInitString(List<String> authorizableIds) {
Review comment:
same as above
##########
File path: src/main/java/org/apache/sling/repoinit/parser/operations/AclGroupBase.java
##########
@@ -40,7 +41,7 @@
private final List<String> aclOptions;
protected AclGroupBase(List<AclLine> lines) {
- this(lines,new ArrayList<String>());
+ this(lines,new ArrayList<>());
Review comment:
IMHO this is an unrelated change. if that is true please move it to a separate pull request
##########
File path: src/main/java/org/apache/sling/repoinit/parser/operations/SetAclPrincipals.java
##########
@@ -33,14 +34,15 @@
private final List<String> principals;
public SetAclPrincipals(List<String> principals, List<AclLine> lines) {
- this(principals,lines,new ArrayList<String>());
+ this(principals,lines,new ArrayList<>());
Review comment:
see above
##########
File path: src/main/javacc/RepoInitGrammar.jjt
##########
@@ -771,13 +771,21 @@ void removeFromGroupStatement(List<Operation> result) :
<REMOVE>
members = principalsList()
<FROM> <GROUP>
- group = <STRING>
+ group = authorizableId()
{
result.add(new RemoveGroupMembers(members, group.image));
}
}
+Token authorizableId() :
Review comment:
same as with the utility class. the name of the method is IMO not correct as it is used both for principal names and for authorizable ids.
##########
File path: src/main/java/org/apache/sling/repoinit/parser/impl/AuthorizableIdUtil.java
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.repoinit.parser.impl;
+
+import java.util.List;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+public class AuthorizableIdUtil {
+
+ private static final Pattern regex = Pattern.compile("\\s");
+
+ private AuthorizableIdUtil() {
+ // hidden
+ }
+
+ /**
+ * Gets the authorizable ID handling cases where the value should be quoted
+ * (e.g. the authorizable id is not just an alphanumeric string)
+ *
+ * @param authorizableId the authorizable ID to get
+ * @return the (potentially quoted) authorizable id
+ */
+ public static final String forRepoInitString(String authorizableId) {
Review comment:
same here, sometimes a principal name is passed which isn't the same as the authorizable id.
i would also suggest to use notnull/nullable annotations for the param and the return value and adjust the description accordingly
##########
File path: src/main/java/org/apache/sling/repoinit/parser/operations/SetAclPrincipalBased.java
##########
@@ -33,14 +34,15 @@
private final List<String> principals;
public SetAclPrincipalBased(List<String> principals, List<AclLine> lines) {
- this(principals,lines,new ArrayList<String>());
+ this(principals,lines,new ArrayList<>());
}
public SetAclPrincipalBased(List<String> principals, List<AclLine> lines, List<String> aclOptions) {
super(lines,aclOptions);
this.principals = Collections.unmodifiableList(principals);
}
+ @Override
Review comment:
see above
##########
File path: src/main/java/org/apache/sling/repoinit/parser/operations/SetAclPrincipals.java
##########
@@ -33,14 +34,15 @@
private final List<String> principals;
public SetAclPrincipals(List<String> principals, List<AclLine> lines) {
- this(principals,lines,new ArrayList<String>());
+ this(principals,lines,new ArrayList<>());
}
public SetAclPrincipals(List<String> principals,List<AclLine> lines, List<String> aclOptions) {
super(lines,aclOptions);
this.principals = Collections.unmodifiableList(principals);
}
+ @Override
Review comment:
see above
##########
File path: src/main/java/org/apache/sling/repoinit/parser/impl/AuthorizableIdUtil.java
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.repoinit.parser.impl;
+
+import java.util.List;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+public class AuthorizableIdUtil {
Review comment:
i find the name of this class a bit unfortunate as it is used both for parsing authorizable ids as well as for principal names.
second, i would suggest to write a dedicate unit test for the methods of this utility class
##########
File path: src/main/java/org/apache/sling/repoinit/parser/operations/SetAclPrincipalBased.java
##########
@@ -33,14 +34,15 @@
private final List<String> principals;
public SetAclPrincipalBased(List<String> principals, List<AclLine> lines) {
- this(principals,lines,new ArrayList<String>());
+ this(principals,lines,new ArrayList<>());
Review comment:
see above
##########
File path: src/main/java/org/apache/sling/repoinit/parser/operations/SetAclPaths.java
##########
@@ -41,6 +41,7 @@ public SetAclPaths(List<String> paths,List<AclLine> lines, List<String> aclOptio
this.paths = Collections.unmodifiableList(paths);
}
+ @Override
Review comment:
again, IMHO this change is unrelated to the task at hand. i would prefer if it would got in a separate 'minor cleanup' PR with a dedicated ticket.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] sonarcloud[bot] commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1011711570
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list) [88.5% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list) [7.1% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] sonarcloud[bot] commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-983197154
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list) [88.5% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list) [7.1% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] sonarcloud[bot] commented on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1012162650
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list) [2.1% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-repoinit-parser] sonarcloud[bot] removed a comment on pull request #14: SLING-10952 - Adding support for whitespace characters in group names
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #14:
URL: https://github.com/apache/sling-org-apache-sling-repoinit-parser/pull/14#issuecomment-1012162650
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list) [2.1% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-repoinit-parser&pullRequest=14&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org