From c21b33d3d4a140d2d450cfc479988d653eeb90ec Mon Sep 17 00:00:00 2001 From: Sergey Rymsha Date: Thu, 10 Oct 2019 12:21:37 +0200 Subject: [PATCH] WebDispatcherImpl implementation issues #7523 keep elements with same order avoid copying on every dispatch --- .../web/impl/handler/WebDispatcherImpl.java | 36 ++++++-------- .../xp/web/impl/handler/TestWebHandler.java | 16 ++++++- .../impl/handler/WebDispatcherImplTest.java | 48 +++++++++++++++++++ 3 files changed, 77 insertions(+), 23 deletions(-) create mode 100644 modules/web/web-impl/src/test/java/com/enonic/xp/web/impl/handler/WebDispatcherImplTest.java 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 index 09ceccdde25..f7509e7730d 100644 --- 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,30 +18,22 @@ public final class WebDispatcherImpl implements WebDispatcher { - private final List webHandlerList = Lists.newCopyOnWriteArrayList(); + private final AtomicReference> 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 @@ -47,12 +41,12 @@ 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 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 index 55832652b20..5d99ada0476 100644 --- 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,7 +5,7 @@ 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; @@ -13,10 +13,22 @@ public final class TestWebHandler 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 new file mode 100644 index 00000000000..c33119130be --- /dev/null +++ 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 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 list = StreamSupport.stream( dispatcher.spliterator(), false ).collect( Collectors.toList() ); + assertSame( webHandler0, list.get( 0 ) ); + assertSame( webHandlerAlso0, list.get( 1 ) ); + } +}