This is an automated email from the ASF dual-hosted git repository. cziegeler pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
The following commit(s) were added to refs/heads/master by this push: new 0c081b4 SLING-7727 : Make ordering of providers for adaptTo detection predicatable 0c081b4 is described below commit 0c081b4a280335e80073ef15a487974ff2b9b4db Author: Carsten Ziegeler <czieg...@adobe.com> AuthorDate: Tue Jun 12 16:53:12 2018 +0200 SLING-7727 : Make ordering of providers for adaptTo detection predicatable --- .../impl/providers/ResourceProviderHandler.java | 10 +-- .../impl/providers/ResourceProviderStorage.java | 27 ++++++-- .../providers/ResourceProviderStorageTest.java | 78 ++++++++++++++++++++++ 3 files changed, 106 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderHandler.java b/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderHandler.java index 73090f7..50dd9d1 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderHandler.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderHandler.java @@ -128,16 +128,18 @@ public class ResourceProviderHandler implements Comparable<ResourceProviderHandl @Override public int compareTo(final ResourceProviderHandler o) { - if ( this.getInfo() == null ) { - if ( o.getInfo() == null ) { + final ResourceProviderInfo localInfo = this.info; + final ResourceProviderInfo otherInfo = o.info; + if ( localInfo == null ) { + if ( otherInfo == null ) { return 0; } return 1; } - if ( o.getInfo() == null ) { + if ( otherInfo == null ) { return -1; } - return this.getInfo().compareTo(o.getInfo()); + return localInfo.compareTo(otherInfo); } /** diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderStorage.java b/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderStorage.java index dac3616..4d59966 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderStorage.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderStorage.java @@ -19,6 +19,8 @@ package org.apache.sling.resourceresolver.impl.providers; import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; import java.util.List; import org.apache.sling.api.resource.runtime.dto.AuthType; @@ -46,10 +48,10 @@ public class ResourceProviderStorage { public ResourceProviderStorage(List<ResourceProviderHandler> handlers) { this.allHandlers = handlers; - this.authRequiredHandlers = new ArrayList<ResourceProviderHandler>(); - this.adaptableHandlers = new ArrayList<ResourceProviderHandler>(); - this.attributableHandlers = new ArrayList<ResourceProviderHandler>(); - this.languageQueryableHandlers = new ArrayList<ResourceProviderHandler>(); + this.authRequiredHandlers = new ArrayList<>(); + this.adaptableHandlers = new ArrayList<>(); + this.attributableHandlers = new ArrayList<>(); + this.languageQueryableHandlers = new ArrayList<>(); for (ResourceProviderHandler h : allHandlers) { ResourceProviderInfo info = h.getInfo(); if (info.getAuthType() == AuthType.required) { @@ -66,7 +68,22 @@ public class ResourceProviderStorage { this.languageQueryableHandlers.add(h); } } - this.handlersTree = new PathTree<ResourceProviderHandler>(handlers); + Collections.sort(this.adaptableHandlers, new Comparator<ResourceProviderHandler>() { + + @Override + public int compare(final ResourceProviderHandler o1, final ResourceProviderHandler o2) { + final ResourceProviderInfo i1 = o1.getInfo(); + final ResourceProviderInfo i2 = o2.getInfo(); + if ( i1 == null ) { + return i2 == null ? 0 : -1; + } + if ( i2 == null ) { + return 1; + } + return i2.getServiceReference().compareTo(i1.getServiceReference()); + } + }); + this.handlersTree = new PathTree<>(handlers); } public List<ResourceProviderHandler> getAllHandlers() { diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderStorageTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderStorageTest.java new file mode 100644 index 0000000..a0b6b69 --- /dev/null +++ b/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderStorageTest.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sling.resourceresolver.impl.providers; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.sling.spi.resource.provider.ResourceProvider; +import org.junit.Test; +import org.osgi.framework.ServiceReference; + +public class ResourceProviderStorageTest { + + @Test public void testAdaptableOrdering() throws Exception { + final ServiceReference r1 = mock(ServiceReference.class); + when(r1.getProperty(ResourceProvider.PROPERTY_ADAPTABLE)).thenReturn(Boolean.TRUE); + final ServiceReference r2 = mock(ServiceReference.class); + when(r2.getProperty(ResourceProvider.PROPERTY_ADAPTABLE)).thenReturn(Boolean.TRUE); + final ServiceReference r3 = mock(ServiceReference.class); + when(r3.getProperty(ResourceProvider.PROPERTY_ADAPTABLE)).thenReturn(Boolean.TRUE); + when(r1.compareTo(r1)).thenReturn(0); + when(r1.compareTo(r2)).thenReturn(-1); + when(r1.compareTo(r3)).thenReturn(-1); + when(r2.compareTo(r2)).thenReturn(0); + when(r2.compareTo(r1)).thenReturn(1); + when(r2.compareTo(r3)).thenReturn(-1); + when(r3.compareTo(r3)).thenReturn(0); + when(r3.compareTo(r1)).thenReturn(1); + when(r3.compareTo(r2)).thenReturn(1); + + final ResourceProviderInfo i1 = new ResourceProviderInfo(r1); + final ResourceProviderInfo i2 = new ResourceProviderInfo(r2); + final ResourceProviderInfo i3 = new ResourceProviderInfo(r3); + + final List<ResourceProviderHandler> handlers = new ArrayList<>(); + // first in right order + handlers.add(new ResourceProviderHandler(null, i3)); + handlers.add(new ResourceProviderHandler(null, i2)); + handlers.add(new ResourceProviderHandler(null, i1)); + + final List<ResourceProviderHandler> correctOrder = new ArrayList<>(handlers); + assertEquals(correctOrder, new ResourceProviderStorage(handlers).getAdaptableHandlers()); + + // reverse order + handlers.clear(); + handlers.add(correctOrder.get(2)); + handlers.add(correctOrder.get(1)); + handlers.add(correctOrder.get(0)); + assertEquals(correctOrder, new ResourceProviderStorage(handlers).getAdaptableHandlers()); + + // arbitrary order + handlers.clear(); + handlers.add(correctOrder.get(2)); + handlers.add(correctOrder.get(0)); + handlers.add(correctOrder.get(1)); + assertEquals(correctOrder, new ResourceProviderStorage(handlers).getAdaptableHandlers()); + } +} \ No newline at end of file -- To stop receiving notification emails like this one, please contact cziege...@apache.org.