SLING-10373 - fixes search service resource resolver cleanup
diff --git a/reference/src/main/java/org/apache/sling/cms/reference/SearchService.java b/reference/src/main/java/org/apache/sling/cms/reference/SearchService.java
index 271f514..3a3ebaf 100644
--- a/reference/src/main/java/org/apache/sling/cms/reference/SearchService.java
+++ b/reference/src/main/java/org/apache/sling/cms/reference/SearchService.java
@@ -23,14 +23,7 @@
* Service for managing the search user.
*/
public interface SearchService {
-
- /**
- * Closes the resource resolver if appropriate
- *
- * @param resolver the resource resolver used in search
- */
- void closeResolver(ResourceResolver resolver);
-
+
/**
* Gets either the service user resource resolver of the request resource
* resolver depending if the service user is configured.
diff --git a/reference/src/main/java/org/apache/sling/cms/reference/impl/SearchServiceImpl.java b/reference/src/main/java/org/apache/sling/cms/reference/impl/SearchServiceImpl.java
index 805a87f..c9fa7c2 100644
--- a/reference/src/main/java/org/apache/sling/cms/reference/impl/SearchServiceImpl.java
+++ b/reference/src/main/java/org/apache/sling/cms/reference/impl/SearchServiceImpl.java
@@ -20,6 +20,8 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.request.SlingRequestEvent;
+import org.apache.sling.api.request.SlingRequestListener;
import org.apache.sling.api.resource.LoginException;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.api.resource.ResourceResolverFactory;
@@ -37,16 +39,17 @@
/**
* Implementation of the SearchService
*/
-@Component(service = { SearchService.class })
+@Component(service = { SearchService.class, SlingRequestListener.class })
@Designate(ocd = Config.class)
-public class SearchServiceImpl implements SearchService {
+public class SearchServiceImpl implements SearchService, SlingRequestListener {
private static final Logger log = LoggerFactory.getLogger(SearchServiceImpl.class);
- @Reference
- private ResourceResolverFactory factory;
+ private final ResourceResolverFactory factory;
- private Config config;
+ private final Config config;
+
+ private static final String RESOURCE_RESOLVER_ATTR = SearchServiceImpl.class.getName() + ":ResourceResolver";
@ObjectClassDefinition(name = "%cms.reference.search.name", description = "%cms.reference.search.description", localization = "OSGI-INF/l10n/bundle")
public @interface Config {
@@ -56,17 +59,21 @@
}
@Activate
- public void init(Config config) {
+ public SearchServiceImpl(@Reference ResourceResolverFactory factory, Config config) {
+ this.factory = factory;
this.config = config;
}
@Override
public ResourceResolver getResourceResolver(SlingHttpServletRequest request) {
+
if (config != null && StringUtils.isNotBlank(config.searchServiceUsername())) {
try {
log.debug("Retrieving Service User {}", config.searchServiceUsername());
- return factory.getServiceResourceResolver(Collections.singletonMap(ResourceResolverFactory.SUBSERVICE,
- (Object) config.searchServiceUsername()));
+ ResourceResolver resolver = factory.getServiceResourceResolver(
+ Collections.singletonMap(ResourceResolverFactory.SUBSERVICE, config.searchServiceUsername()));
+ request.setAttribute(RESOURCE_RESOLVER_ATTR, resolver);
+ return resolver;
} catch (LoginException e) {
log.warn("Failed to retrieve Service User {}, falling back to request user",
config.searchServiceUsername(), e);
@@ -79,11 +86,14 @@
}
@Override
- public void closeResolver(ResourceResolver resolver) {
- if (resolver != null && resolver.isLive() && StringUtils.isNotBlank(config.searchServiceUsername())
- && config.searchServiceUsername().equals(resolver.getUserID())) {
+ public void onEvent(SlingRequestEvent sre) {
+ if (sre.getType() == SlingRequestEvent.EventType.EVENT_DESTROY
+ && sre.getServletRequest().getAttribute(RESOURCE_RESOLVER_ATTR) != null
+ && sre.getServletRequest().getAttribute(RESOURCE_RESOLVER_ATTR) instanceof ResourceResolver) {
+ ResourceResolver resolver = (ResourceResolver) sre.getServletRequest().getAttribute(RESOURCE_RESOLVER_ATTR);
resolver.close();
}
+
}
}
diff --git a/reference/src/main/java/org/apache/sling/cms/reference/models/Search.java b/reference/src/main/java/org/apache/sling/cms/reference/models/Search.java
index 08fa94a..6f053bb 100644
--- a/reference/src/main/java/org/apache/sling/cms/reference/models/Search.java
+++ b/reference/src/main/java/org/apache/sling/cms/reference/models/Search.java
@@ -62,8 +62,6 @@
private final List<Resource> results;
- private final SearchService searchService;
-
private final int start;
private final ResourceResolver resolver;
@@ -72,7 +70,6 @@
public Search(@Self SlingHttpServletRequest request, @ValueMapValue @Named("limit") int limit,
@OSGiService SearchService searchService, @ValueMapValue @Named("basePath") String basePath) {
this.request = request;
- this.searchService = searchService;
Set<String> distinct = new HashSet<>();
String term = Text.escapeIllegalXpathSearchChars(request.getParameter(TERM_PARAMETER)).replace("'", "''");
@@ -156,17 +153,6 @@
return request.getParameter(TERM_PARAMETER);
}
- /**
- * This is a horrible hack to close the resource resolver used for retrieving
- * the search results
- *
- * @return true, always
- */
- public String getFinalize() {
- searchService.closeResolver(resolver);
- return "";
- }
-
public boolean isFirst() {
return page == 0;
}
diff --git a/reference/src/main/resources/jcr_root/apps/reference/components/general/search/search.jsp b/reference/src/main/resources/jcr_root/apps/reference/components/general/search/search.jsp
index f9f42d1..f160511 100644
--- a/reference/src/main/resources/jcr_root/apps/reference/components/general/search/search.jsp
+++ b/reference/src/main/resources/jcr_root/apps/reference/components/general/search/search.jsp
@@ -38,4 +38,4 @@
</div>
<sling:call script="pagination.jsp" />
</div>
-</c:if>${search.finalize}
\ No newline at end of file
+</c:if>
\ No newline at end of file
diff --git a/reference/src/test/java/org/apache/sling/cms/reference/forms/impl/SearchServiceTest.java b/reference/src/test/java/org/apache/sling/cms/reference/forms/impl/SearchServiceTest.java
new file mode 100644
index 0000000..db98963
--- /dev/null
+++ b/reference/src/test/java/org/apache/sling/cms/reference/forms/impl/SearchServiceTest.java
@@ -0,0 +1,109 @@
+/*
+ * 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.cms.reference.forms.impl;
+
+import java.lang.annotation.Annotation;
+
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.request.SlingRequestEvent;
+import org.apache.sling.api.request.SlingRequestListener;
+import org.apache.sling.api.request.SlingRequestEvent.EventType;
+import org.apache.sling.api.resource.LoginException;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.cms.reference.SearchService;
+import org.apache.sling.cms.reference.impl.SearchServiceImpl;
+import org.apache.sling.cms.reference.impl.SearchServiceImpl.Config;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class SearchServiceTest {
+
+ private ResourceResolverFactory factory;
+ private SlingHttpServletRequest request;
+ private ResourceResolver resolver;
+
+ @Before
+ public void init() {
+ factory = Mockito.mock(ResourceResolverFactory.class);
+ request = Mockito.mock(SlingHttpServletRequest.class);
+ resolver = Mockito.mock(ResourceResolver.class);
+ }
+
+ @Test
+ public void testNoConfig() {
+ SearchService search = new SearchServiceImpl(factory, new ConfigImpl(null));
+ search.getResourceResolver(request);
+ Mockito.verify(request).getResourceResolver();
+ }
+
+ @Test
+ public void testWithConfig() throws LoginException {
+ SearchService search = new SearchServiceImpl(factory, new ConfigImpl("hello-world"));
+ search.getResourceResolver(request);
+ Mockito.verify(factory).getServiceResourceResolver(Mockito.anyMap());
+ }
+
+ @Test
+ public void testInvalidUser() throws LoginException {
+ SearchService search = new SearchServiceImpl(factory, new ConfigImpl("invalid"));
+ Mockito.when(factory.getServiceResourceResolver(Mockito.anyMap())).thenThrow(new LoginException("Bad user"));
+ search.getResourceResolver(request);
+ Mockito.verify(request).getResourceResolver();
+ }
+
+ @Test
+ public void testRequestListener() throws LoginException {
+ String name = SearchServiceImpl.class.getName() + ":ResourceResolver";
+ SlingRequestListener listener = new SearchServiceImpl(factory, new ConfigImpl("invalid"));
+
+ listener.onEvent(new SlingRequestEvent(null, request, EventType.EVENT_DESTROY));
+ Mockito.verify(resolver, Mockito.never()).close();
+
+ Mockito.when(request.getAttribute(name)).thenReturn(null);
+ listener.onEvent(new SlingRequestEvent(null, request, EventType.EVENT_DESTROY));
+ Mockito.verify(resolver, Mockito.never()).close();
+
+ listener.onEvent(new SlingRequestEvent(null, request, EventType.EVENT_INIT));
+ Mockito.verify(resolver, Mockito.never()).close();
+
+ Mockito.when(request.getAttribute(name)).thenReturn(resolver);
+ listener.onEvent(new SlingRequestEvent(null, request, EventType.EVENT_DESTROY));
+ Mockito.verify(resolver).close();
+ }
+
+}
+
+class ConfigImpl implements Config {
+ private final String name;
+
+ ConfigImpl(String name) {
+ this.name = name;
+ }
+
+ @Override
+ public Class<? extends Annotation> annotationType() {
+ return null;
+ }
+
+ @Override
+ public String searchServiceUsername() {
+ return name;
+ }
+
+}
\ No newline at end of file
