Improve Trace for Content and Node API #10033 (#10035)

modules/core/core-api/src/main/java/com/enonic/xp/node/MultiRepoNodeQuery.java

@@ -1,5 +1,7 @@
 package com.enonic.xp.node;
 
+import java.util.Objects;
+
 public class MultiRepoNodeQuery
 {
     private final SearchTargets searchTargets;
@@ -8,8 +10,8 @@ public class MultiRepoNodeQuery
 
     public MultiRepoNodeQuery( final SearchTargets searchTargets, final NodeQuery nodeQuery )
     {
-        this.searchTargets = searchTargets;
-        this.nodeQuery = nodeQuery;
+        this.searchTargets = Objects.requireNonNull( searchTargets );
+        this.nodeQuery = Objects.requireNonNull( nodeQuery );
     }
 
     public SearchTargets getSearchTargets()

modules/core/core-api/src/main/java/com/enonic/xp/node/NodeService.java

@@ -112,6 +112,7 @@ public interface NodeService
 
     LoadNodeResult loadNode( LoadNodeParams params );
 
+    @Deprecated
     NodesHasChildrenResult hasChildren( Nodes nodes );
 
     NodeCommitEntry commit( NodeCommitEntry nodeCommitEntry, RoutableNodeVersionIds routableNodeVersionIds );

modules/core/core-api/src/main/java/com/enonic/xp/node/NodesHasChildrenResult.java

@@ -5,6 +5,7 @@
 
 import com.google.common.collect.ImmutableMap;
 
+@Deprecated
 public class NodesHasChildrenResult
 {
     private final ImmutableMap<NodeId, Boolean> valueMap;
@@ -51,4 +52,4 @@ public NodesHasChildrenResult build()
     }
 
 
-}
+}

modules/core/core-api/src/main/java/com/enonic/xp/trace/Tracer.java

@@ -1,7 +1,9 @@
 package com.enonic.xp.trace;
 
 import java.util.concurrent.Callable;
+import java.util.function.BiConsumer;
 import java.util.function.Consumer;
+import java.util.function.Supplier;
 
 public final class Tracer
 {
@@ -98,6 +100,41 @@ public static <T> T trace( final String name, final TraceRunnable<T> runnable )
         return trace( newTrace( name ), runnable );
     }
 
+    public static <T> T trace( final String name, final Consumer<Trace> before, final Supplier<T> main, final BiConsumer<Trace, T> after )
+    {
+        final Trace trace = newTrace( name );
+
+        if ( trace != null )
+        {
+            before.accept( trace );
+            final Trace current = current();
+
+            try
+            {
+                setCurrent( trace );
+                startTrace( trace );
+                final T result = main.get();
+                after.accept( trace, result );
+                return result;
+            }
+            finally
+            {
+                endTrace( trace );
+                setCurrent( current );
+            }
+        }
+        else
+        {
+            return main.get();
+        }
+    }
+
+    public static <T> T trace( final String name, final Consumer<Trace> before, final Supplier<T> main )
+    {
+        return trace( name, before, main, ( trace, t ) -> {
+        } );
+    }
+
     public static <T> T traceEx( final String name, final Callable<T> callable )
         throws Exception
     {

modules/core/core-api/src/test/java/com/enonic/xp/trace/TracerTest.java

@@ -1,40 +1,55 @@
 package com.enonic.xp.trace;
 
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InOrder;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-
-public class TracerTest
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.same;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class TracerTest
 {
-    private TraceManager manager;
+    @Mock
+    TraceManager manager;
 
-    private Trace trace;
+    @Mock
+    Trace trace;
 
     @BeforeEach
-    public void setUp()
+    void setUp()
     {
-        this.manager = Mockito.mock( TraceManager.class );
         Tracer.setManager( this.manager );
-
-        this.trace = Mockito.mock( Trace.class );
-        Mockito.when( this.manager.newTrace( Mockito.any(), Mockito.any() ) ).thenReturn( trace );
     }
 
     @AfterEach
-    public void tearDown()
+    void tearDown()
     {
         Tracer.setManager( null );
     }
 
     @Test
-    public void testEnabled()
+    void testEnabled()
     {
         assertTrue( Tracer.isEnabled() );
 
@@ -43,23 +58,25 @@ public void testEnabled()
     }
 
     @Test
-    public void testNewTrace()
+    void testNewTrace()
     {
+        when( this.manager.newTrace( any(), any() ) ).thenReturn( trace );
+
         assertSame( this.trace, Tracer.newTrace( "test" ) );
 
         Tracer.setManager( null );
         assertNull( Tracer.newTrace( "test" ) );
     }
 
     @Test
-    public void testCurrent()
+    void testCurrent()
     {
         assertNull( Tracer.current() );
         Tracer.trace( this.trace, () -> assertSame( this.trace, Tracer.current() ) );
     }
 
     @Test
-    public void withCurrent()
+    void withCurrent()
     {
         Tracer.withCurrent( ( t ) ->
                             {
@@ -70,17 +87,72 @@ public void withCurrent()
     }
 
     @Test
-    public void traceNull()
+    void traceNull()
     {
         Tracer.trace( (Trace) null, () ->
         {
         } );
     }
 
     @Test
-    public void testTrace()
+    void trace_disabled( @Mock final Consumer<Trace> before, @Mock final BiConsumer<Trace, Object> after, @Mock final Supplier<Object> call )
+    {
+        Tracer.setManager( null );
+
+        Tracer.trace( "disabled", before, call, after );
+        verifyNoInteractions( before, after );
+        verify( call, times( 1 ) ).get();
+    }
+
+    @Test
+    void trace_disabled( @Mock final Consumer<Trace> before, @Mock final Supplier<Object> call )
+    {
+        Tracer.setManager( null );
+
+        Tracer.trace( "disabled", before, call );
+        verifyNoInteractions( before );
+        verify( call, times( 1 ) ).get();
+    }
+
+    @Test
+    void trace_enabled( @Mock final Consumer<Trace> before, @Mock final BiConsumer<Trace, Object> after, @Mock final Supplier<Object> call )
+    {
+        when( this.manager.newTrace( eq("enabled"), any() ) ).thenReturn( trace );
+
+        Object result = mock( Object.class );
+        when( call.get() ).thenReturn( result );
+
+        Tracer.trace( "enabled", before, call, after );
+
+        final InOrder inOrder = inOrder( before, after, call, result );
+        inOrder.verify( before, times( 1 ) ).accept( same( trace ) );
+        inOrder.verify( call, times( 1 ) ).get();
+        inOrder.verify( after, times( 1 ) ).accept( same( trace ), same( result ) );
+        inOrder.verifyNoMoreInteractions();
+    }
+
+    @Test
+    void trace_enabled( @Mock final Consumer<Trace> before, @Mock final Supplier<Object> call )
+    {
+        when( this.manager.newTrace( eq("enabled"), any() ) ).thenReturn( trace );
+
+        Object result = mock( Object.class );
+        when( call.get() ).thenReturn( result );
+
+        Tracer.trace( "enabled", before, call );
+
+        final InOrder inOrder = inOrder( before, call, result );
+        inOrder.verify( before, times( 1 ) ).accept( same( trace ) );
+        inOrder.verify( call, times( 1 ) ).get();
+        inOrder.verifyNoMoreInteractions();
+    }
+
+    @Test
+    void testTrace()
         throws Exception
     {
+        when( this.manager.newTrace( any(), any() ) ).thenReturn( trace );
+
         Tracer.trace( "test", () -> assertSame( this.trace, Tracer.current() ) );
 
         final int return1 = Tracer.trace( "test", () -> 1 );

modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentNodeTranslator.java

@@ -1,5 +1,7 @@
 package com.enonic.xp.core.impl.content;
 
+import java.util.function.Function;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -14,7 +16,6 @@
 import com.enonic.xp.node.NodePath;
 import com.enonic.xp.node.NodeService;
 import com.enonic.xp.node.Nodes;
-import com.enonic.xp.node.NodesHasChildrenResult;
 
 public class ContentNodeTranslator
 {
@@ -34,12 +35,11 @@ public Contents fromNodes( final Nodes nodes, final boolean resolveHasChildren )
     {
         if ( resolveHasChildren )
         {
-            final NodesHasChildrenResult nodesHasChildren = this.nodeService.hasChildren( nodes );
-            return doTranslate( nodes, nodesHasChildren );
+            return doTranslate( nodes, nodeService::hasChildren );
         }
         else
         {
-            return doTranslate( nodes, NodesHasChildrenResult.empty() );
+            return doTranslate( nodes, n -> false );
         }
     }
 
@@ -54,15 +54,15 @@ public Content fromNode( final Node node, final boolean resolveHasChildren, fina
         return doTranslate( node, hasChildren, allowAltRootPath );
     }
 
-    private Contents doTranslate( final Nodes nodes, final NodesHasChildrenResult nodeHasChildren )
+    private Contents doTranslate( final Nodes nodes, final Function<Node, Boolean> hasChildrenFn )
     {
         final Contents.Builder contents = Contents.create();
 
         for ( final Node node : nodes )
         {
             try
             {
-                contents.add( doTranslate( node, nodeHasChildren.hasChild( node.id() ) ) );
+                contents.add( doTranslate( node, hasChildrenFn.apply( node ) ) );
             }
             catch ( final ContentNotFoundException e )
             {