anchela commented on code in PR #158:
URL:
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/158#discussion_r1112675670
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ cp.addSegment(
+ part,
+ primaryTypeMap.get(repositoryPath),
+ mixinTypeMap.get(repositoryPath)
+ );
+ }else{
Review Comment:
formatting
##########
src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java:
##########
@@ -286,11 +295,9 @@ public void convert(@NotNull File... contentPackages)
throws IOException, Conver
// analyze sub-content packages in order to filter out
// possible outdated conflicting packages
recollectorVaultPackageScanner.traverse(pack);
-
logger.info("content-package '{}' successfully read!",
contentPackage);
aclManager.reset();
- bundleSlingInitialContentExtractor.reset();
Review Comment:
@niekraaijmakers , the method '`reset`' on the
`BundleSlingInitialContentExtractor` is now no longer used anywhere..... but it
is still present in the code.
please remove it....
and on a second look: could it be a problem that the reset method is never
called? after all it was there for a reason, wasn't it?
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ cp.addSegment(
+ part,
+ primaryTypeMap.get(repositoryPath),
+ mixinTypeMap.get(repositoryPath)
+ );
+ }else{
+ cp.addSegment(part, null);
+ }
+ }
+
+ private void collectTypeDataForSegment() {
+ for (VaultPackageAssembler packageAssembler : packageAssemblers) {
+
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ boolean merge = true;
+ for(PathFilterSet set:
packageAssembler.getFilter().getFilterSets()){
+ if(set.covers(repositoryPath) && (set.getImportMode() !=
ImportMode.MERGE && set.getImportMode() != ImportMode.MERGE_PROPERTIES)){
Review Comment:
formatting
in addition: this condition is too complex to read -> i would suggest to
extract it into a separate method that helps understanding what you are doing.
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ cp.addSegment(
+ part,
+ primaryTypeMap.get(repositoryPath),
+ mixinTypeMap.get(repositoryPath)
+ );
+ }else{
+ cp.addSegment(part, null);
+ }
+ }
+
+ private void collectTypeDataForSegment() {
+ for (VaultPackageAssembler packageAssembler : packageAssemblers) {
+
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ boolean merge = true;
+ for(PathFilterSet set:
packageAssembler.getFilter().getFilterSets()){
+ if(set.covers(repositoryPath) && (set.getImportMode() !=
ImportMode.MERGE && set.getImportMode() != ImportMode.MERGE_PROPERTIES)){
+ //found a path with a mode other than merge, proceed
to replace the type definitions
+ merge = false;
+ }else if(set.covers(repositoryPath)){
+ // merge is flipped to true again by another filter
defined after the previous ones
+ merge = true;
}
}
+ if(merge){
+ // we already got the path defined by an earlier package.
only proceed to replace if the filter is not merge.
+ continue;
+ }
}
- if (!segmentAdded) {
- //use sling:Folder (defined by repo-init runtime module)
- cp.addSegment(part, null);
+
+ File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
+ if (currentContent.exists() && currentContent.isFile()) {
+ //collect the primary Data up ahead
+ collectTypeData(repositoryPath, currentContent);
}
}
- return foundType;
}
- private static boolean addSegment(@NotNull CreatePath cp, @NotNull String
part, @NotNull File currentContent) {
+
+ private boolean collectTypeData(String repositoryPath, @NotNull File
currentContent) {
Review Comment:
please add annotation to repositoryPath param. i believe you don't expect
this to be null and thus it should have notnull annotation.
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
Review Comment:
formatting
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ cp.addSegment(
+ part,
+ primaryTypeMap.get(repositoryPath),
+ mixinTypeMap.get(repositoryPath)
+ );
+ }else{
+ cp.addSegment(part, null);
+ }
+ }
+
+ private void collectTypeDataForSegment() {
+ for (VaultPackageAssembler packageAssembler : packageAssemblers) {
+
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ boolean merge = true;
+ for(PathFilterSet set:
packageAssembler.getFilter().getFilterSets()){
+ if(set.covers(repositoryPath) && (set.getImportMode() !=
ImportMode.MERGE && set.getImportMode() != ImportMode.MERGE_PROPERTIES)){
+ //found a path with a mode other than merge, proceed
to replace the type definitions
+ merge = false;
+ }else if(set.covers(repositoryPath)){
+ // merge is flipped to true again by another filter
defined after the previous ones
+ merge = true;
}
}
+ if(merge){
+ // we already got the path defined by an earlier package.
only proceed to replace if the filter is not merge.
+ continue;
Review Comment:
wait-what.... ? ah.... that's the outer for-loop.....
that code looks quite brittle IMHO. plese refactor the code such that you
don't need the 'continue' and make the code easy to understand/read/maintain.
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
Review Comment:
i believe you want 'part' to be notnull... -> add nullable or notnull
annotations.
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
Review Comment:
please add notnull/nullable annotation. (i believe you want it to be notnull)
missing space before {
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ cp.addSegment(
+ part,
+ primaryTypeMap.get(repositoryPath),
+ mixinTypeMap.get(repositoryPath)
+ );
+ }else{
+ cp.addSegment(part, null);
+ }
+ }
+
+ private void collectTypeDataForSegment() {
+ for (VaultPackageAssembler packageAssembler : packageAssemblers) {
+
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ boolean merge = true;
+ for(PathFilterSet set:
packageAssembler.getFilter().getFilterSets()){
Review Comment:
formatting
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ cp.addSegment(
+ part,
+ primaryTypeMap.get(repositoryPath),
+ mixinTypeMap.get(repositoryPath)
+ );
+ }else{
+ cp.addSegment(part, null);
+ }
+ }
+
+ private void collectTypeDataForSegment() {
+ for (VaultPackageAssembler packageAssembler : packageAssemblers) {
+
+ if(primaryTypeMap.containsKey(repositoryPath)){
Review Comment:
formatting... you even don't do it consistently in your own code :(
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ cp.addSegment(
+ part,
+ primaryTypeMap.get(repositoryPath),
+ mixinTypeMap.get(repositoryPath)
+ );
+ }else{
+ cp.addSegment(part, null);
+ }
+ }
+
+ private void collectTypeDataForSegment() {
+ for (VaultPackageAssembler packageAssembler : packageAssemblers) {
+
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ boolean merge = true;
+ for(PathFilterSet set:
packageAssembler.getFilter().getFilterSets()){
+ if(set.covers(repositoryPath) && (set.getImportMode() !=
ImportMode.MERGE && set.getImportMode() != ImportMode.MERGE_PROPERTIES)){
+ //found a path with a mode other than merge, proceed
to replace the type definitions
+ merge = false;
+ }else if(set.covers(repositoryPath)){
+ // merge is flipped to true again by another filter
defined after the previous ones
+ merge = true;
}
}
+ if(merge){
Review Comment:
formatting
##########
src/main/java/org/apache/sling/feature/cpconverter/repoinit/createpath/CreatePathSegmentProcessor.java:
##########
@@ -27,52 +29,97 @@
import java.io.FileInputStream;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import static org.apache.jackrabbit.vault.util.Constants.DOT_CONTENT_XML;
public class CreatePathSegmentProcessor {
- private CreatePathSegmentProcessor() {
+ private final RepoPath path;
+ private final Collection<VaultPackageAssembler> packageAssemblers;
+ private final CreatePath cp;
+ private boolean foundType = false;
+ private String repositoryPath = "";
+
+ private Map<String,String> primaryTypeMap = new LinkedHashMap<>();
+ private Map<String,List<String>> mixinTypeMap = new LinkedHashMap<>();
+
+ public CreatePathSegmentProcessor(@NotNull RepoPath path,
+ @NotNull
Collection<VaultPackageAssembler> packageAssemblers,
+ @NotNull CreatePath cp) {
+ this.path = path;
+ this.packageAssemblers = packageAssemblers;
+ this.cp = cp;
}
/**
* Process segments of a repopath to createpath, checking
packageassemblers for existing primaryType definitions.
- *
- * @param path
- * @param packageAssemblers
- * @param cp
* @return
*/
- public static boolean processSegments(@NotNull RepoPath path, @NotNull
Collection<VaultPackageAssembler> packageAssemblers, @NotNull CreatePath cp) {
- String repositoryPath = "";
- boolean foundType = false;
+ public boolean processSegments() {
for (final String part : path.getSegments()) {
- final String platformName =
PlatformNameFormat.getPlatformName(part);
- repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+ repositoryPath = processSegment(part);
+ }
+ return foundType;
+ }
- boolean segmentAdded = false;
- //loop all package assemblers and check if .content.xml is defined
- for (VaultPackageAssembler packageAssembler : packageAssemblers) {
- File currentContent =
packageAssembler.getFileEntry(repositoryPath.concat(ConverterConstants.SLASH).concat(DOT_CONTENT_XML));
- if (currentContent.exists() && currentContent.isFile()) {
- //add segment if jcr:primaryType is defined.
- segmentAdded = addSegment(cp, part, currentContent);
- if (segmentAdded) {
- foundType = true;
- break;
+ @NotNull
+ private String processSegment(String part) {
+ final String platformName = PlatformNameFormat.getPlatformName(part);
+ repositoryPath =
repositoryPath.concat(ConverterConstants.SLASH).concat(platformName);
+
+ //loop all package assemblers and check if .content.xml is defined
+ collectTypeDataForSegment();
+ addSegment(part);
+
+ return repositoryPath;
+ }
+
+ private void addSegment(String part){
+ //add segment if jcr:primaryType is defined.
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ cp.addSegment(
+ part,
+ primaryTypeMap.get(repositoryPath),
+ mixinTypeMap.get(repositoryPath)
+ );
+ }else{
+ cp.addSegment(part, null);
+ }
+ }
+
+ private void collectTypeDataForSegment() {
+ for (VaultPackageAssembler packageAssembler : packageAssemblers) {
+
+ if(primaryTypeMap.containsKey(repositoryPath)){
+ boolean merge = true;
+ for(PathFilterSet set:
packageAssembler.getFilter().getFilterSets()){
+ if(set.covers(repositoryPath) && (set.getImportMode() !=
ImportMode.MERGE && set.getImportMode() != ImportMode.MERGE_PROPERTIES)){
+ //found a path with a mode other than merge, proceed
to replace the type definitions
+ merge = false;
+ }else if(set.covers(repositoryPath)){
Review Comment:
formatting
in addition: you have twice the condition `set.covers(repositoryPath)` in
both the if and the else-if. that doesn't look good. i believe this could be
heavily improved in terms of readability if you would extract that into a
separate method.... e.g. 'doMerge'
also note: merge is already the default in the init of the boolean.... so
this looks a bit odd at a first glance and becomes only understandable in the
context of the for-loop and with the comment.
added benefit of extracting into a separate method: the complexity of the
method does down, which will avoid having sonar complaining about 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
