Resolving publish dependencies with a huge excludeContentIds list #10635

enonic · Jul 10, 2024 · 4c2daa5 · 4c2daa5
1 parent 64dea8f
commit 4c2daa5
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 114 deletions.
diff --git a/...java/com/enonic/xp/repo/impl/elasticsearch/query/translator/factory/DiffQueryFactory.java b/...java/com/enonic/xp/repo/impl/elasticsearch/query/translator/factory/DiffQueryFactory.java
@@ -1,6 +1,5 @@
 package com.enonic.xp.repo.impl.elasticsearch.query.translator.factory;
 
-import java.util.Set;
 import java.util.stream.Collectors;
 
 import org.elasticsearch.index.query.BoolQueryBuilder;
@@ -15,10 +14,9 @@
 
 import com.enonic.xp.branch.Branch;
 import com.enonic.xp.node.NodePath;
+import com.enonic.xp.node.NodePaths;
 import com.enonic.xp.repo.impl.StorageType;
 import com.enonic.xp.repo.impl.branch.storage.BranchIndexPath;
-import com.enonic.xp.repo.impl.version.search.ExcludeEntries;
-import com.enonic.xp.repo.impl.version.search.ExcludeEntry;
 import com.enonic.xp.repo.impl.version.search.NodeVersionDiffQuery;
 
 public class DiffQueryFactory
@@ -29,7 +27,7 @@ public class DiffQueryFactory
 
     private final NodePath nodePath;
 
-    private final ExcludeEntries excludes;
+    private final NodePaths excludes;
 
     private final StorageType childStorageType;
 
@@ -75,12 +73,12 @@ private QueryBuilder wrapInPathQueryIfNecessary( final BoolQueryBuilder sourceTa
 
         if ( this.nodePath != null && !this.nodePath.isRoot() )
         {
-            result.filter( hasPaths( ExcludeEntries.create().add( new ExcludeEntry( this.nodePath, true ) ).build() ) );
+            result.filter( hasPaths( NodePaths.from( this.nodePath ), true ) );
         }
 
         if ( !this.excludes.isEmpty() )
         {
-            result.mustNot( hasPaths( excludes ) );
+            result.mustNot( hasPaths( excludes, false ) );
         }
 
         return result;
@@ -91,25 +89,21 @@ private BoolQueryBuilder joinOnlyInQueries( final BoolQueryBuilder inSourceOnly,
         return new BoolQueryBuilder().should( inSourceOnly ).should( inTargetOnly );
     }
 
-    private QueryBuilder hasPaths( final ExcludeEntries excludeEntries )
+    private QueryBuilder hasPaths( final NodePaths excludePaths, final boolean recursive )
     {
         final BoolQueryBuilder pathQuery = new BoolQueryBuilder().should( new TermsQueryBuilder( BranchIndexPath.PATH.getPath(),
-                                                                                                 excludeEntries.getSet()
+                                                                                                 excludePaths.getSet()
                                                                                                      .stream()
-                                                                                                     .map(
-                                                                                                         excludeEntry -> excludeEntry.nodePath()
+                                                                                                     .map( excludeEntry -> excludeEntry
                                                                                                              .toString()
                                                                                                              .toLowerCase() )
                                                                                                      .collect( Collectors.toList() ) ) );
 
-        final Set<NodePath> recursivePaths =
-            excludeEntries.getSet().stream().filter( ExcludeEntry::recursive ).map( ExcludeEntry::nodePath ).collect( Collectors.toSet() );
-
-        if ( !recursivePaths.isEmpty() )
+        if ( recursive )
         {
             final String queryPath = nodePath.toString().toLowerCase();
 
-            recursivePaths.forEach( nodePath -> {
+            excludePaths.forEach( nodePath -> {
                 pathQuery.should( new WildcardQueryBuilder( BranchIndexPath.PATH.getPath(),
                                                             queryPath.endsWith( "/" ) ? queryPath + "*" : queryPath + "/*" ) );
             } );

diff --git a/...epo/src/main/java/com/enonic/xp/repo/impl/node/FindNodesWithVersionDifferenceCommand.java b/...epo/src/main/java/com/enonic/xp/repo/impl/node/FindNodesWithVersionDifferenceCommand.java
@@ -6,13 +6,12 @@
 import com.enonic.xp.node.NodeBranchEntry;
 import com.enonic.xp.node.NodeIds;
 import com.enonic.xp.node.NodePath;
+import com.enonic.xp.node.NodePaths;
 import com.enonic.xp.node.NodeVersionDiffResult;
 import com.enonic.xp.repo.impl.InternalContext;
 import com.enonic.xp.repo.impl.search.NodeSearchService;
 import com.enonic.xp.repo.impl.search.result.SearchResult;
 import com.enonic.xp.repo.impl.storage.NodeStorageService;
-import com.enonic.xp.repo.impl.version.search.ExcludeEntries;
-import com.enonic.xp.repo.impl.version.search.ExcludeEntry;
 import com.enonic.xp.repo.impl.version.search.NodeVersionDiffQuery;
 
 public class FindNodesWithVersionDifferenceCommand
@@ -50,7 +49,7 @@ public NodeVersionDiffResult execute()
     {
         final InternalContext context = InternalContext.from( ContextAccessor.current() );
 
-        final ExcludeEntries excludeEntries = getExcludePaths( context );
+        final NodePaths excludeEntries = getExcludePaths( context );
 
         final SearchResult result = this.nodeSearchService.query( NodeVersionDiffQuery.create()
                                                                       .source( source )
@@ -64,20 +63,20 @@ public NodeVersionDiffResult execute()
         return NodeVersionDiffResultFactory.create( result );
     }
 
-    private ExcludeEntries getExcludePaths( final InternalContext context )
+    private NodePaths getExcludePaths( final InternalContext context )
     {
         if ( this.excludes.isEmpty() )
         {
-            return ExcludeEntries.empty();
+            return NodePaths.empty();
         }
 
-        final ExcludeEntries.Builder builder = ExcludeEntries.create();
+        final NodePaths.Builder builder = NodePaths.create();
 
         final NodeBranchEntries result = this.nodeStorageService.getBranchNodeVersions( excludes, context );
 
         for ( final NodeBranchEntry entry : result )
         {
-            builder.add( new ExcludeEntry( entry.getNodePath(), false ) );
+            builder.addNodePath( entry.getNodePath() );
         }
 
         return builder.build();

diff --git a/...e/core-repo/src/main/java/com/enonic/xp/repo/impl/node/HasUnpublishedChildrenCommand.java b/...e/core-repo/src/main/java/com/enonic/xp/repo/impl/node/HasUnpublishedChildrenCommand.java
@@ -4,9 +4,8 @@
 import com.enonic.xp.context.ContextAccessor;
 import com.enonic.xp.node.Node;
 import com.enonic.xp.node.NodeId;
+import com.enonic.xp.node.NodePaths;
 import com.enonic.xp.repo.impl.search.result.SearchResult;
-import com.enonic.xp.repo.impl.version.search.ExcludeEntries;
-import com.enonic.xp.repo.impl.version.search.ExcludeEntry;
 import com.enonic.xp.repo.impl.version.search.NodeVersionDiffQuery;
 
 public class HasUnpublishedChildrenCommand
@@ -36,10 +35,7 @@ public boolean execute()
                                                                  .source( ContextAccessor.current().getBranch() )
                                                                  .target( target )
                                                                  .nodePath( parentNode.path() )
-                                                                 .size( 0 )
-                                                                 .excludes( ExcludeEntries.create()
-                                                                                .add( new ExcludeEntry( parentNode.path(), false ) )
-                                                                                .build() )
+                                                                 .size( 0 ).excludes( NodePaths.from( parentNode.path() ) )
                                                                  .build(), ContextAccessor.current().getRepositoryId() );
 
         return result.getTotalHits() > 0;

diff --git a/...s/core/core-repo/src/main/java/com/enonic/xp/repo/impl/version/search/ExcludeEntries.java b/...s/core/core-repo/src/main/java/com/enonic/xp/repo/impl/version/search/ExcludeEntries.java
diff --git a/...les/core/core-repo/src/main/java/com/enonic/xp/repo/impl/version/search/ExcludeEntry.java b/...les/core/core-repo/src/main/java/com/enonic/xp/repo/impl/version/search/ExcludeEntry.java
diff --git a/.../core-repo/src/main/java/com/enonic/xp/repo/impl/version/search/NodeVersionDiffQuery.java b/.../core-repo/src/main/java/com/enonic/xp/repo/impl/version/search/NodeVersionDiffQuery.java
@@ -3,6 +3,7 @@
 import com.enonic.xp.branch.Branch;
 import com.enonic.xp.node.AbstractQuery;
 import com.enonic.xp.node.NodePath;
+import com.enonic.xp.node.NodePaths;
 
 public class NodeVersionDiffQuery
     extends AbstractQuery
@@ -13,15 +14,15 @@ public class NodeVersionDiffQuery
 
     private final NodePath nodePath;
 
-    private final ExcludeEntries excludes;
+    private final NodePaths excludes;
 
     private NodeVersionDiffQuery( Builder builder )
     {
         super( builder );
         this.source = builder.source;
         this.target = builder.target;
         this.nodePath = builder.nodePath;
-        this.excludes = builder.excludes;
+        this.excludes = builder.excludes.build();
     }
 
     public Branch getSource()
@@ -39,7 +40,7 @@ public NodePath getNodePath()
         return nodePath;
     }
 
-    public ExcludeEntries getExcludes()
+    public NodePaths getExcludes()
     {
         return excludes;
     }
@@ -58,7 +59,7 @@ public static class Builder
 
         private NodePath nodePath;
 
-        private ExcludeEntries excludes = ExcludeEntries.empty();
+        private final NodePaths.Builder excludes = NodePaths.create();
 
 
         public Builder()
@@ -84,9 +85,9 @@ public Builder nodePath( final NodePath nodePath )
             return this;
         }
 
-        public Builder excludes( final ExcludeEntries excludes )
+        public Builder excludes( final NodePaths excludes )
         {
-            this.excludes = excludes;
+            this.excludes.addNodePaths( excludes.getSet() );
             return this;
         }
 

diff --git a/...es/itest/itest-core/src/test/java/com/enonic/xp/core/node/ResolveSyncWorkCommandTest.java b/...es/itest/itest-core/src/test/java/com/enonic/xp/core/node/ResolveSyncWorkCommandTest.java
@@ -1174,7 +1174,7 @@ public void change_child_order_manual_yields_children_changed()
     }
 
     @Test
-    void push_more_than_1024_nodes_at_once()
+    void resolve_with_more_than_1024_excluded_nodes_at_once()
     {
         final Node rootNode = createNodeSkipVerification( CreateNodeParams.create().name( "rootNode" ).parent( NodePath.ROOT ).build() );
 
@@ -1193,6 +1193,8 @@ void push_more_than_1024_nodes_at_once()
             .searchService( this.searchService )
             .build()
             .execute();
+
+        assertEquals( 1, syncWork.getSize() );
     }
 
     /*