Merge pull request #7524 from enonic/issue-7523

WebDispatcherImpl implementation issues #7523
enonic · Oct 9, 2019 · cec6f9b · cec6f9b
2 parents 6f9fa5d + a79d359
commit cec6f9b
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 20 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,12 +1,12 @@
 package com.enonic.xp.web.impl.handler;
 
+import java.util.Comparator;
 import java.util.Iterator;
-import java.util.List;
+import java.util.NavigableSet;
+import java.util.concurrent.ConcurrentSkipListSet;
 
 import org.osgi.service.component.annotations.Component;
 
-import com.google.common.collect.Lists;
-
 import com.enonic.xp.web.WebRequest;
 import com.enonic.xp.web.WebResponse;
 import com.enonic.xp.web.handler.WebHandler;
@@ -16,30 +16,18 @@
 public final class WebDispatcherImpl
     implements WebDispatcher
 {
-    private final List<WebHandler> webHandlerList = Lists.newCopyOnWriteArrayList();
+    private final NavigableSet<WebHandler> webHandlerList = new ConcurrentSkipListSet<>( Comparator.comparingInt( WebHandler::getOrder ) );
 
     @Override
-    public synchronized void add( final WebHandler webHandler )
+    public void add( final WebHandler webHandler )
     {
         this.webHandlerList.add( webHandler );
-        sortWebHandlerList();
     }
 
     @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();
     }
 
     @Override

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,34 @@
+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 ) );
+    }
+}