anchela commented on code in PR #158:
URL:
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/158#discussion_r1112682836
##########
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
finally: there is not 'else'.... it would be good to either refactor the
code that it's a simple if-else of alternatively add a comment about your
'else' that is missing. what happens in that case?
--
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]
