Merge pull request #7528 from enonic/issue-7523

WebDispatcherImpl implementation issues #7523
enonic · Oct 14, 2019 · 9238b62 · 9238b62
2 parents ce28a1e + c21b33d
commit 9238b62
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 23 deletions.
diff --git a/modules/web/web-impl/src/main/java/com/enonic/xp/web/impl/handler/WebDispatcherImpl.java b/modules/web/web-impl/src/main/java/com/enonic/xp/web/impl/handler/WebDispatcherImpl.java
@@ -1,11 +1,13 @@
 package com.enonic.xp.web.impl.handler;
 
+import java.util.Comparator;
 import java.util.Iterator;
-import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
 
 import org.osgi.service.component.annotations.Component;
 
-import com.google.common.collect.Lists;
+import com.google.common.collect.ImmutableList;
 
 import com.enonic.xp.web.WebRequest;
 import com.enonic.xp.web.WebResponse;
@@ -16,43 +18,35 @@
 public final class WebDispatcherImpl
     implements WebDispatcher
 {
-    private final List<WebHandler> webHandlerList = Lists.newCopyOnWriteArrayList();
+    private final AtomicReference<ImmutableList<WebHandler>> webHandlerListRef = new AtomicReference<>( ImmutableList.of() );
 
     @Override
-    public synchronized void add( final WebHandler webHandler )
+    public void add( final WebHandler webHandler )
     {
-        this.webHandlerList.add( webHandler );
-        sortWebHandlerList();
+        webHandlerListRef.updateAndGet( oldWebHandlers -> Stream.concat( oldWebHandlers.stream(), Stream.of( webHandler ) ).
+            sorted( Comparator.comparingInt( WebHandler::getOrder ) ).
+            collect( ImmutableList.toImmutableList() ) );
     }
 
     @Override
-    public synchronized void remove( final WebHandler webHandler )
+    public void remove( final WebHandler webHandler )
     {
-        this.webHandlerList.remove( webHandler );
-        sortWebHandlerList();
-    }
-
-    private void sortWebHandlerList()
-    {
-        this.webHandlerList.sort( this::compare );
-    }
-
-    private int compare( final WebHandler webHandler1, final WebHandler webHandler2 )
-    {
-        return webHandler1.getOrder() - webHandler2.getOrder();
+        webHandlerListRef.updateAndGet( oldWebHandlers -> oldWebHandlers.stream().filter( w -> w != webHandler ).
+            sorted( Comparator.comparingInt( WebHandler::getOrder ) ).
+            collect( ImmutableList.toImmutableList() ) );
     }
 
     @Override
     public WebResponse dispatch( final WebRequest req, final WebResponse res )
         throws Exception
     {
         ServletRequestHolder.setRequest( req.getRawRequest() );
-        return new WebHandlerChainImpl( this.webHandlerList ).handle( req, res );
+        return new WebHandlerChainImpl( this.webHandlerListRef.get() ).handle( req, res );
     }
 
     @Override
     public Iterator<WebHandler> iterator()
     {
-        return this.webHandlerList.iterator();
+        return this.webHandlerListRef.get().iterator();
     }
 }
diff --git a/modules/web/web-impl/src/test/java/com/enonic/xp/web/impl/handler/TestWebHandler.java b/modules/web/web-impl/src/test/java/com/enonic/xp/web/impl/handler/TestWebHandler.java
@@ -5,18 +5,30 @@
 import com.enonic.xp.web.handler.WebHandler;
 import com.enonic.xp.web.handler.WebHandlerChain;
 
-public final class TestWebHandler
+public class TestWebHandler
     implements WebHandler
 {
     protected WebResponse response;
 
     protected RequestVerifier verifier = req -> {
     };
 
+    private final int order;
+
+    public TestWebHandler()
+    {
+        this( 0 );
+    }
+
+    public TestWebHandler( final int order )
+    {
+        this.order = order;
+    }
+
     @Override
     public int getOrder()
     {
-        return 0;
+        return order;
     }
 
     @Override

diff --git a/modules/web/web-impl/src/test/java/com/enonic/xp/web/impl/handler/WebDispatcherImplTest.java b/modules/web/web-impl/src/test/java/com/enonic/xp/web/impl/handler/WebDispatcherImplTest.java
@@ -0,0 +1,48 @@
+package com.enonic.xp.web.impl.handler;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
+
+import org.junit.jupiter.api.Test;
+
+import com.enonic.xp.web.handler.WebHandler;
+
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+class WebDispatcherImplTest
+{
+    @Test
+    void orderedProperly()
+    {
+        final WebDispatcherImpl dispatcher = new WebDispatcherImpl();
+        TestWebHandler webHandlerMin = new TestWebHandler( WebHandler.MIN_ORDER );
+        dispatcher.add( webHandlerMin );
+        TestWebHandler webHandlerMax = new TestWebHandler( WebHandler.MAX_ORDER );
+        dispatcher.add( webHandlerMax );
+        TestWebHandler webHandler0 = new TestWebHandler( 0 );
+        dispatcher.add( webHandler0 );
+        TestWebHandler webHandler1 = new TestWebHandler( 1 );
+        dispatcher.add( webHandler1 );
+
+        List<WebHandler> list = StreamSupport.stream( dispatcher.spliterator(), false ).collect( Collectors.toList() );
+        assertSame( webHandlerMin, list.get( 0 ) );
+        assertSame( webHandler0, list.get( 1 ) );
+        assertSame( webHandler1, list.get( 2 ) );
+        assertSame( webHandlerMax, list.get( 3 ) );
+    }
+
+    @Test
+    void supportsEqualOderElements()
+    {
+        final WebDispatcherImpl dispatcher = new WebDispatcherImpl();
+        TestWebHandler webHandler0 = new TestWebHandler( 0 );
+        dispatcher.add( webHandler0 );
+        TestWebHandler webHandlerAlso0 = new TestWebHandler( 0 );
+        dispatcher.add( webHandlerAlso0 );
+
+        List<WebHandler> list = StreamSupport.stream( dispatcher.spliterator(), false ).collect( Collectors.toList() );
+        assertSame( webHandler0, list.get( 0 ) );
+        assertSame( webHandlerAlso0, list.get( 1 ) );
+    }
+}