Resolving publish dependencies with a huge excludeContentIds list #10635

enonic · Aug 6, 2024 · 062ca3a · 062ca3a
1 parent 3d52e53
commit 062ca3a
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 65 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,11 +1,14 @@
 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;
-import org.elasticsearch.index.query.FilteredQueryBuilder;
 import org.elasticsearch.index.query.HasChildQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.index.query.TermQueryBuilder;
+import org.elasticsearch.index.query.TermsQueryBuilder;
 import org.elasticsearch.index.query.WildcardQueryBuilder;
 
 import com.google.common.base.Preconditions;
@@ -62,56 +65,54 @@ private QueryBuilder createDiffQuery()
 
     private BoolQueryBuilder onlyInQuery( final Branch source, final Branch target )
     {
-        return new BoolQueryBuilder().
-            must( isInBranch( source ) ).
-            mustNot( isInBranch( target ) );
+        return new BoolQueryBuilder().must( isInBranch( source ) ).mustNot( isInBranch( target ) );
     }
 
-    private FilteredQueryBuilder wrapInPathQueryIfNecessary( final BoolQueryBuilder sourceTargetCompares )
+    private QueryBuilder wrapInPathQueryIfNecessary( final BoolQueryBuilder sourceTargetCompares )
     {
 
-        final BoolQueryBuilder pathFilter = new BoolQueryBuilder();
-
-        boolean addedPathFilter = false;
+        final BoolQueryBuilder result = QueryBuilders.boolQuery().filter( sourceTargetCompares );
 
         if ( this.nodePath != null && !this.nodePath.isRoot() )
         {
-            addedPathFilter = true;
-            pathFilter.
-                must( hasPath( this.nodePath, true ) );
+            result.filter( hasPaths( ExcludeEntries.create().add( new ExcludeEntry( this.nodePath, true ) ).build() ) );
         }
 
         if ( !this.excludes.isEmpty() )
         {
-            addedPathFilter = true;
-            for ( final ExcludeEntry exclude : excludes )
-            {
-                pathFilter.
-                    mustNot( hasPath( exclude.getNodePath(), exclude.isRecursive() ) );
-            }
+            result.mustNot( hasPaths( excludes ) );
         }
 
-        return addedPathFilter
-            ? new FilteredQueryBuilder( pathFilter, sourceTargetCompares )
-            : new FilteredQueryBuilder( QueryBuilders.matchAllQuery(), sourceTargetCompares );
+        return result;
     }
 
     private BoolQueryBuilder joinOnlyInQueries( final BoolQueryBuilder inSourceOnly, final BoolQueryBuilder inTargetOnly )
     {
         return new BoolQueryBuilder().should( inSourceOnly ).should( inTargetOnly );
     }
 
-    private QueryBuilder hasPath( final NodePath nodePath, final boolean recursive )
+    private QueryBuilder hasPaths( final ExcludeEntries excludeEntries )
     {
-        final String queryPath = nodePath.toString().toLowerCase();
-
-        final BoolQueryBuilder pathQuery = new BoolQueryBuilder().
-            should( new TermQueryBuilder( BranchIndexPath.PATH.getPath(), queryPath ) );
-
-        if ( recursive )
+        final BoolQueryBuilder pathQuery = new BoolQueryBuilder().should( new TermsQueryBuilder( BranchIndexPath.PATH.getPath(),
+                                                                                                 excludeEntries.getSet()
+                                                                                                     .stream()
+                                                                                                     .map(
+                                                                                                         excludeEntry -> excludeEntry.nodePath()
+                                                                                                             .toString()
+                                                                                                             .toLowerCase() )
+                                                                                                     .collect( Collectors.toList() ) ) );
+
+        final Set<NodePath> recursivePaths =
+            excludeEntries.getSet().stream().filter( ExcludeEntry::recursive ).map( ExcludeEntry::nodePath ).collect( Collectors.toSet() );
+
+        if ( !recursivePaths.isEmpty() )
         {
-            pathQuery.should( new WildcardQueryBuilder( BranchIndexPath.PATH.getPath(),
-                                                        queryPath.endsWith( "/" ) ? queryPath + "*" : queryPath + "/*" ) );
+            final String queryPath = nodePath.toString().toLowerCase();
+
+            recursivePaths.forEach( nodePath -> {
+                pathQuery.should( new WildcardQueryBuilder( BranchIndexPath.PATH.getPath(),
+                                                            queryPath.endsWith( "/" ) ? queryPath + "*" : queryPath + "/*" ) );
+            } );
         }
 
         return new HasChildQueryBuilder( childStorageType.getName(), pathQuery );

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
@@ -1,38 +1,41 @@
 package com.enonic.xp.repo.impl.version.search;
 
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 
-public class ExcludeEntries
+import com.google.common.collect.ImmutableSet;
+
+public final class ExcludeEntries
     implements Iterable<ExcludeEntry>
 {
     private final Set<ExcludeEntry> excludeEntries;
 
+    private static final ExcludeEntries EMPTY = create().build();
+
     @Override
     public Iterator<ExcludeEntry> iterator()
     {
         return excludeEntries.iterator();
     }
 
-    public static ExcludeEntries empty()
+    private ExcludeEntries( final Builder builder )
     {
-        return new ExcludeEntries( new HashSet<>() );
+        excludeEntries = builder.excludeEntries.build();
     }
 
     public boolean isEmpty()
     {
         return this.excludeEntries.isEmpty();
     }
 
-    private ExcludeEntries( final Set<ExcludeEntry> excludeEntries )
+    public static ExcludeEntries empty()
     {
-        this.excludeEntries = excludeEntries;
+        return EMPTY;
     }
 
-    private ExcludeEntries( final Builder builder )
+    public Set<ExcludeEntry> getSet()
     {
-        excludeEntries = builder.excludeEntries;
+        return excludeEntries;
     }
 
     public static Builder create()
@@ -43,7 +46,7 @@ public static Builder create()
 
     public static final class Builder
     {
-        private final Set<ExcludeEntry> excludeEntries = new HashSet<>();
+        private final ImmutableSet.Builder<ExcludeEntry> excludeEntries = ImmutableSet.builder();
 
         private Builder()
         {
@@ -66,4 +69,4 @@ public ExcludeEntries build()
             return new ExcludeEntries( this );
         }
     }
-}
+}
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
@@ -2,25 +2,6 @@
 
 import com.enonic.xp.node.NodePath;
 
-public class ExcludeEntry
+public record ExcludeEntry(NodePath nodePath, boolean recursive)
 {
-    private final NodePath nodePath;
-
-    private final boolean recursive;
-
-    public ExcludeEntry( final NodePath nodePath, final boolean recursive )
-    {
-        this.nodePath = nodePath;
-        this.recursive = recursive;
-    }
-
-    public NodePath getNodePath()
-    {
-        return nodePath;
-    }
-
-    public boolean isRecursive()
-    {
-        return recursive;
-    }
 }
diff --git a/modules/itest/itest-core/src/test/java/com/enonic/xp/core/AbstractNodeTest.java b/modules/itest/itest-core/src/test/java/com/enonic/xp/core/AbstractNodeTest.java
@@ -4,6 +4,7 @@
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.Consumer;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
@@ -520,20 +521,27 @@ protected void assertOrder( final FindNodesByQueryResult result, NodeId... ids )
 
     protected final void createNodes( final Node parent, final int numberOfNodes, final int maxLevels, final int level )
     {
+        this.createNodes( parent, numberOfNodes, maxLevels, level, ( child ) -> {
+        } );
+    }
+
+    protected final void createNodes( final Node parent, final int numberOfNodes, final int maxLevels, final int level,
+                                      final Consumer<Node> childConsumer )
+    {
+
         for ( int i = 0; i < numberOfNodes; i++ )
         {
             final PropertyTree data = new PropertyTree();
             data.addReference( "myRef", new Reference( parent.id() ) );
 
-            final Node node = createNodeSkipVerification( CreateNodeParams.create().
-                name( "nodeName_" + level + "-" + i ).
-                parent( parent.path() ).
-                data( data ).
-                build() );
+            final Node node = createNodeSkipVerification(
+                CreateNodeParams.create().name( "nodeName_" + level + "-" + i ).parent( parent.path() ).data( data ).build() );
+
+            childConsumer.accept( node );
 
             if ( level < maxLevels )
             {
-                createNodes( node, numberOfNodes, maxLevels, level + 1 );
+                createNodes( node, numberOfNodes, maxLevels, level + 1, childConsumer );
             }
         }
     }

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
@@ -1173,6 +1173,28 @@ public void change_child_order_manual_yields_children_changed()
             child( "node1_1", "node1_2", "node1_3", "node1_4" ) );
     }
 
+    @Test
+    void push_more_than_1024_nodes_at_once()
+    {
+        final Node rootNode = createNodeSkipVerification( CreateNodeParams.create().name( "rootNode" ).parent( NodePath.ROOT ).build() );
+
+        final Set<NodeId> childrenIds = new HashSet<>();
+
+        createNodes( rootNode, 10, 3, 1, ( child ) -> childrenIds.add( child.id() ) );
+
+        refresh();
+
+        final ResolveSyncWorkResult syncWork = ResolveSyncWorkCommand.create()
+            .nodeId( rootNode.id() )
+            .target( WS_OTHER )
+            .excludedNodeIds( NodeIds.from( childrenIds ) )
+            .indexServiceInternal( this.indexServiceInternal )
+            .storageService( this.storageService )
+            .searchService( this.searchService )
+            .build()
+            .execute();
+    }
+
     /*
      * s1
      ** a1