You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by dk...@apache.org on 2021/05/11 19:29:20 UTC
[sling-org-apache-sling-app-cms] branch master updated (93e3b8f ->
62d9cdc)
This is an automated email from the ASF dual-hosted git repository.
dklco pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-app-cms.git.
from 93e3b8f Minor - fixing the start / stop scripts to work with the feature launcher
new 06cb31d SLING-10373 - fixes search service resource resolver cleanup
new 62d9cdc Fixing incorrect OSGi config
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/sling/cms/reference/SearchService.java | 9 +-
.../forms/impl/fields/SelectionHandler.java | 3 +-
.../cms/reference/impl/SearchServiceImpl.java | 32 +++---
.../apache/sling/cms/reference/models/Search.java | 14 ---
.../reference/components/general/search/search.jsp | 2 +-
.../reference/forms/impl/SearchServiceTest.java | 109 +++++++++++++++++++++
6 files changed, 133 insertions(+), 36 deletions(-)
create mode 100644 reference/src/test/java/org/apache/sling/cms/reference/forms/impl/SearchServiceTest.java
[sling-org-apache-sling-app-cms] 02/02: Fixing incorrect OSGi config
Posted by dk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
dklco pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-app-cms.git
commit 62d9cdcde918c0b57b608ee8fc6d63e88a34f0d5
Author: Dan Klco <kl...@adobe.com>
AuthorDate: Tue May 11 15:29:07 2021 -0400
Fixing incorrect OSGi config
---
.../apache/sling/cms/reference/forms/impl/fields/SelectionHandler.java | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/fields/SelectionHandler.java b/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/fields/SelectionHandler.java
index 8fd6e8f..3f01468 100644
--- a/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/fields/SelectionHandler.java
+++ b/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/fields/SelectionHandler.java
@@ -28,7 +28,6 @@ import org.apache.sling.api.resource.Resource;
import org.apache.sling.cms.reference.forms.FieldHandler;
import org.apache.sling.cms.reference.forms.FormException;
import org.apache.sling.cms.reference.forms.FormUtils;
-import org.apache.sling.cms.reference.impl.SearchServiceImpl.Config;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.metatype.annotations.AttributeDefinition;
@@ -38,7 +37,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Component(service = FieldHandler.class)
-@Designate(ocd = Config.class)
+@Designate(ocd = SelectionHandler.Config.class)
public class SelectionHandler implements FieldHandler {
public static final String DEFAULT_RESOURCE_TYPE = "reference/components/forms/fields/selection";
[sling-org-apache-sling-app-cms] 01/02: SLING-10373 - fixes search
service resource resolver cleanup
Posted by dk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
dklco pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-app-cms.git
commit 06cb31d5b183f6b9e9a6a2696313d92262c6a2ec
Author: Dan Klco <kl...@adobe.com>
AuthorDate: Tue May 11 15:28:52 2021 -0400
SLING-10373 - fixes search service resource resolver cleanup
---
.../apache/sling/cms/reference/SearchService.java | 9 +-
.../cms/reference/impl/SearchServiceImpl.java | 32 +++---
.../apache/sling/cms/reference/models/Search.java | 14 ---
.../reference/components/general/search/search.jsp | 2 +-
.../reference/forms/impl/SearchServiceTest.java | 109 +++++++++++++++++++++
5 files changed, 132 insertions(+), 34 deletions(-)
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 @@ import org.apache.sling.api.resource.ResourceResolver;
* 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 java.util.Collections;
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 @@ import org.slf4j.LoggerFactory;
/**
* 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 @@ public class SearchServiceImpl implements SearchService {
}
@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 @@ public class SearchServiceImpl implements SearchService {
}
@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 @@ public class Search {
private final List<Resource> results;
- private final SearchService searchService;
-
private final int start;
private final ResourceResolver resolver;
@@ -72,7 +70,6 @@ public class Search {
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 @@ public class Search {
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