You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by st...@apache.org on 2021/04/21 07:55:17 UTC
[sling-org-apache-sling-discovery-oak] branch master updated:
SLING-10204 : less noisy logging on unregistration (#2)
This is an automated email from the ASF dual-hosted git repository.
stefanegli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-discovery-oak.git
The following commit(s) were added to refs/heads/master by this push:
new 46748c1 SLING-10204 : less noisy logging on unregistration (#2)
46748c1 is described below
commit 46748c1602962d3fceb62af35f3a1b3607196fea
Author: stefan-egli <st...@apache.org>
AuthorDate: Wed Apr 21 09:55:09 2021 +0200
SLING-10204 : less noisy logging on unregistration (#2)
* SLING-10204 : minimal update on pom to support Java 9/11
* SLING-10204 : update parent to 29 to update maven-scr-plugin to get FELIX-5118 fix
* SLING-10204 : in PropertyProvider osgi handlers, catch RuntimeException and instead log less noisy (WARN not ERROR, and without stacktrace) as this can happen at unregistration time - allow to get stacktrace when log level is at DEBUG (but log at WARN still as that level is more appropriate)
* SLING-10204 : in callers to getDescriptorFrom, catch Exception and instead log less noisy (WARN not ERROR, and without stacktrace) as this can happen at unregistration time - allow to get stacktrace when log level is at DEBUG (but log at WARN still as that level is more appropriate)
* SLING-10204 : adding some simple tests
* SLING-10204 : adding argLine to surefire plugin to activate jacoco execution - as suggested in stackoverflow.com/questions/18107375
* SLING-10204 : improved newly introduced log messages with a WARN stating that DEBUG exposes stacktraces
* SLING-10204 : dollar argLine instead of at argLine
* SLING-10204 : try newer jacoco-maven-plugin with dollar argLine
* SLING-10204 : newer jacoco-maven-plugin with at argLine, as suggested
* SLING-10204 : newer surefire plugin version
* SLING-10204 : try setting jacoco.command instead of relying on argLine
* SLING-10204 : some more test coverage
---
pom.xml | 89 +++++++++++++++++++++-
.../sling/discovery/oak/OakDiscoveryService.java | 26 ++++++-
.../discovery/oak/TopologyWebConsolePlugin.java | 9 ++-
.../oak/cluster/OakClusterViewService.java | 27 +++++--
.../discovery/oak/OakDiscoveryServiceTest.java | 30 ++++++++
.../oak/TopologyWebConsolePluginTest.java | 55 +++++++++++++
.../oak/cluster/OakClusterViewServiceTest.java | 78 +++++++++++++++++++
7 files changed, 299 insertions(+), 15 deletions(-)
diff --git a/pom.xml b/pom.xml
index 49d0643..f9ffd4e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -23,7 +23,7 @@
<parent>
<groupId>org.apache.sling</groupId>
<artifactId>sling</artifactId>
- <version>26</version>
+ <version>29</version>
<relativePath />
</parent>
@@ -52,11 +52,35 @@
<build>
<plugins>
<plugin>
+ <groupId>org.jacoco</groupId>
+ <artifactId>jacoco-maven-plugin</artifactId>
+ <version>0.8.6</version>
+ <executions>
+ <execution>
+ <id>prepare-agent</id>
+ <configuration>
+ <propertyName>jacoco.command</propertyName>
+ </configuration>
+ <goals>
+ <goal>prepare-agent</goal>
+ </goals>
+ </execution>
+ <execution>
+ <id>report</id>
+ <phase>post-integration-test</phase>
+ <goals>
+ <goal>report</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
+ <plugin>
<groupId>org.apache.maven.plugins</groupId>
- <artifactId>maven-surefire-plugin</artifactId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>3.0.0-M4</version>
<configuration>
<redirectTestOutputToFile>true</redirectTestOutputToFile>
- <argLine>-Xmx2048m</argLine>
+ <argLine>${jacoco.command} -Xmx2048m</argLine>
<excludedGroups>${sling.excluded.surefire.groups}</excludedGroups>
</configuration>
</plugin>
@@ -89,6 +113,61 @@
</instructions>
</configuration>
</plugin>
+ <plugin>
+ <artifactId>maven-antrun-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>set-bundle-required-execution-environment</id>
+ <phase>initialize</phase>
+ <goals>
+ <goal>run</goal>
+ </goals>
+ <configuration>
+ <exportAntProperties>true</exportAntProperties>
+ <target>
+ <!-- apply fix as suggested in this comment:
+ https://issues.apache.org/jira/browse/MANTRUN-200?focusedCommentId=17016706&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17016706 -->
+ <property name="plugin_classpath" refid="maven.plugin.classpath"/>
+ <script language="javascript" classpath="${plugin_classpath}">var System = java.lang.System;
+ var bree = "JavaSE-1.6";
+ var animalSnifferSignaturesVersion = "6";
+ var slingJavaVersion = System.getProperty("sling.java.version");
+ if (!slingJavaVersion) {
+ slingJavaVersion = project.getProperty("sling.java.version");
+ }
+ if (slingJavaVersion == "7") {
+ animalSnifferSignaturesVersion = "7";
+ bree = "JavaSE-1.7";
+ System.out.println("Setting Bundle-RequiredExecutionEnvironment=" + bree + " from sling.java.version=" + slingJavaVersion);
+ } else if ( slingJavaVersion == "8") {
+ animalSnifferSignaturesVersion = "8";
+ bree = "JavaSE-1.8";
+ System.out.println("Setting Bundle-RequiredExecutionEnvironment=" + bree + " from sling.java.version=" + slingJavaVersion);
+ } else if (slingJavaVersion != "6") {
+ System.out.println("Unsupported sling.java.version=" + slingJavaVersion + "; must be 6, 7 or 8.");
+ throw "sling.java.version must be 6, 7 or 8";
+ }
+ project.setProperty("sling.bree", bree);
+ project.setProperty("sling.animalSignatures.version", animalSnifferSignaturesVersion);</script>
+ </target>
+ </configuration>
+ </execution>
+ </executions>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.ant</groupId>
+ <artifactId>ant-apache-regexp</artifactId>
+ <version>1.7.1</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>jakarta-regexp</groupId>
+ <artifactId>jakarta-regexp</artifactId>
+ <version>1.4</version>
+ <scope>compile</scope>
+ </dependency>
+ </dependencies>
+ </plugin>
</plugins>
</build>
<profiles>
@@ -110,6 +189,7 @@
<dependency>
<groupId>biz.aQute</groupId>
<artifactId>bndlib</artifactId>
+ <version>1.50.0</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
@@ -119,10 +199,12 @@
<dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.core</artifactId>
+ <version>4.1.0</version>
</dependency>
<dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.compendium</artifactId>
+ <version>4.1.0</version>
</dependency>
<dependency>
<groupId>org.apache.sling</groupId>
@@ -219,6 +301,7 @@
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>servlet-api</artifactId>
+ <version>2.4</version>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
diff --git a/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java b/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java
index 0b4093f..9913462 100644
--- a/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java
+++ b/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java
@@ -390,7 +390,13 @@ public class OakDiscoveryService extends BaseDiscoveryService {
propertyProvider);
synchronized (lock) {
- this.bindPropertyProviderInteral(propertyProvider, props);
+ try {
+ this.bindPropertyProviderInteral(propertyProvider, props);
+ } catch (RuntimeException re) {
+ // SLING-10204 : catch and instead log less noisy as this can legitimately happen
+ logger.warn("bindPropertyProvider: got RuntimeException (enable debug logging to see stacktrace) : " + re);
+ logger.debug("bindPropertyProvider: RuntimeException stacktrace", re);
+ }
}
}
@@ -417,8 +423,14 @@ public class OakDiscoveryService extends BaseDiscoveryService {
propertyProvider);
synchronized (lock) {
- this.unbindPropertyProviderInternal(propertyProvider, props, false);
- this.bindPropertyProviderInteral(propertyProvider, props);
+ try {
+ this.unbindPropertyProviderInternal(propertyProvider, props, false);
+ this.bindPropertyProviderInteral(propertyProvider, props);
+ } catch (RuntimeException re) {
+ // SLING-10204 : catch and instead log less noisy as this can legitimately happen
+ logger.warn("updatePropertyProvider: got RuntimeException (enable debug logging to see stacktrace) : " + re);
+ logger.debug("updatePropertyProvider: RuntimeException stacktrace", re);
+ }
}
}
@@ -430,7 +442,13 @@ public class OakDiscoveryService extends BaseDiscoveryService {
logger.debug("unbindPropertyProvider: Releasing PropertyProvider {}",
propertyProvider);
synchronized (lock) {
- this.unbindPropertyProviderInternal(propertyProvider, props, true);
+ try {
+ this.unbindPropertyProviderInternal(propertyProvider, props, true);
+ } catch (RuntimeException re) {
+ // SLING-10204 : catch and instead log less noisy as this can legitimately happen
+ logger.warn("unbindPropertyProvider: got RuntimeException (enable debug logging to see stacktrace) : " + re);
+ logger.debug("unbindPropertyProvider: RuntimeException stacktrace", re);
+ }
}
}
diff --git a/src/main/java/org/apache/sling/discovery/oak/TopologyWebConsolePlugin.java b/src/main/java/org/apache/sling/discovery/oak/TopologyWebConsolePlugin.java
index 2a3e073..d671bd2 100644
--- a/src/main/java/org/apache/sling/discovery/oak/TopologyWebConsolePlugin.java
+++ b/src/main/java/org/apache/sling/discovery/oak/TopologyWebConsolePlugin.java
@@ -775,6 +775,11 @@ public class TopologyWebConsolePlugin extends AbstractWebConsolePlugin implement
ResourceResolver resourceResolver = null;
try{
resourceResolver = getResourceResolver();
+ } catch(Exception e) {
+ logger.error("addDiscoveryLiteHistoryEntry: Exception: "+e, e);
+ return;
+ }
+ try {
DiscoveryLiteDescriptor descriptor =
DiscoveryLiteDescriptor.getDescriptorFrom(resourceResolver);
@@ -790,7 +795,9 @@ public class TopologyWebConsolePlugin extends AbstractWebConsolePlugin implement
discoveryLiteHistory.remove(0);
}
} catch(Exception e) {
- logger.error("addDiscoveryLiteHistoryEntry: Exception: "+e, e);
+ // SLING-10204 : log less noisy as this can legitimately happen
+ logger.warn("addDiscoveryLiteHistoryEntry: got Exception (enable debug logging to see stacktrace) : " + e);
+ logger.debug("addDiscoveryLiteHistoryEntry: Exception stacktrace", e);
} finally {
if (resourceResolver != null) {
resourceResolver.close();
diff --git a/src/main/java/org/apache/sling/discovery/oak/cluster/OakClusterViewService.java b/src/main/java/org/apache/sling/discovery/oak/cluster/OakClusterViewService.java
index 2632806..ea91586 100644
--- a/src/main/java/org/apache/sling/discovery/oak/cluster/OakClusterViewService.java
+++ b/src/main/java/org/apache/sling/discovery/oak/cluster/OakClusterViewService.java
@@ -110,8 +110,25 @@ public class OakClusterViewService implements ClusterViewService {
ResourceResolver resourceResolver = null;
try{
resourceResolver = getResourceResolver();
- DiscoveryLiteDescriptor descriptor =
- DiscoveryLiteDescriptor.getDescriptorFrom(resourceResolver);
+ } catch (Exception e) {
+ logger.error("getLocalClusterView: repository exception: "+e, e);
+ throw new UndefinedClusterViewException(Reason.REPOSITORY_EXCEPTION, "Exception while processing descriptor: "+e);
+ }
+ DiscoveryLiteDescriptor descriptor = null;
+ try {
+ descriptor = DiscoveryLiteDescriptor.getDescriptorFrom(resourceResolver);
+ } catch (Exception e) {
+ // SLING-10204 : log less noisy as this can legitimately happen
+ logger.warn("getLocalClusterView: got Exception (enable debug logging to see stacktrace) : " + e);
+ logger.debug("getLocalClusterView: Exception stacktrace", e);
+ throw new UndefinedClusterViewException(Reason.REPOSITORY_EXCEPTION, "Exception while processing descriptor: "+e);
+ } finally {
+ logger.trace("getLocalClusterView: end");
+ if (resourceResolver!=null) {
+ resourceResolver.close();
+ }
+ }
+ try {
if (lastSeqNum!=descriptor.getSeqNum()) {
logger.info("getLocalClusterView: sequence number change detected - clearing idmap cache");
idMapService.clearCache();
@@ -122,11 +139,7 @@ public class OakClusterViewService implements ClusterViewService {
logger.info("getLocalClusterView: undefined clusterView: "+e.getReason()+" - "+e.getMessage());
throw e;
} catch (Exception e) {
- if (e.getMessage() != null && e.getMessage().contains("No Descriptor value available")) {
- logger.warn("getLocalClusterView: repository exception: "+e);
- } else {
- logger.error("getLocalClusterView: repository exception: "+e, e);
- }
+ logger.error("getLocalClusterView: repository exception: "+e, e);
throw new UndefinedClusterViewException(Reason.REPOSITORY_EXCEPTION, "Exception while processing descriptor: "+e);
} finally {
logger.trace("getLocalClusterView: end");
diff --git a/src/test/java/org/apache/sling/discovery/oak/OakDiscoveryServiceTest.java b/src/test/java/org/apache/sling/discovery/oak/OakDiscoveryServiceTest.java
index cfff2cc..9a75491 100644
--- a/src/test/java/org/apache/sling/discovery/oak/OakDiscoveryServiceTest.java
+++ b/src/test/java/org/apache/sling/discovery/oak/OakDiscoveryServiceTest.java
@@ -25,7 +25,9 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.UUID;
import org.apache.sling.api.resource.LoginException;
@@ -38,6 +40,7 @@ import org.apache.sling.discovery.base.its.setup.OSGiMock;
import org.apache.sling.discovery.base.its.setup.VirtualInstance;
import org.apache.sling.discovery.base.its.setup.mock.DummyResourceResolverFactory;
import org.apache.sling.discovery.base.its.setup.mock.MockFactory;
+import org.apache.sling.discovery.base.its.setup.mock.PropertyProviderImpl;
import org.apache.sling.discovery.commons.providers.BaseTopologyView;
import org.apache.sling.discovery.commons.providers.base.DummyListener;
import org.apache.sling.discovery.commons.providers.spi.base.DescriptorHelper;
@@ -51,6 +54,7 @@ import org.apache.sling.discovery.oak.its.setup.OakVirtualInstanceBuilder;
import org.apache.sling.discovery.oak.its.setup.SimulatedLease;
import org.apache.sling.discovery.oak.its.setup.SimulatedLeaseCollection;
import org.junit.Test;
+import org.osgi.framework.Constants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -384,4 +388,30 @@ public class OakDiscoveryServiceTest {
assertEquals(1, listener.countEvents());
return listener.getLastView();
}
+
+ @Test
+ public void testPropertyProviderRegistrations() throws Exception{
+ OakVirtualInstanceBuilder builder =
+ (OakVirtualInstanceBuilder) new OakVirtualInstanceBuilder()
+ .setDebugName("instance1")
+ .newRepository("/foo/barrio/foo/", true)
+ .setConnectorPingInterval(999)
+ .setConnectorPingTimeout(999);
+ VirtualInstance instance = builder.build();
+ OakDiscoveryService discoveryService = (OakDiscoveryService) instance.getDiscoveryService();
+ discoveryService.bindPropertyProvider(null, null);
+ discoveryService.unbindPropertyProvider(null, null);
+ discoveryService.updatedPropertyProvider(null, null);
+
+ PropertyProviderImpl p = new PropertyProviderImpl();
+ discoveryService.bindPropertyProvider(p, null);
+ discoveryService.unbindPropertyProvider(p, null);
+ discoveryService.updatedPropertyProvider(p, null);
+
+ Map<String, Object> m = new HashMap<>();
+ m.put(Constants.SERVICE_ID, 42L);
+ discoveryService.bindPropertyProvider(p, m);
+ discoveryService.unbindPropertyProvider(p, m);
+ discoveryService.updatedPropertyProvider(p, m);
+ }
}
diff --git a/src/test/java/org/apache/sling/discovery/oak/TopologyWebConsolePluginTest.java b/src/test/java/org/apache/sling/discovery/oak/TopologyWebConsolePluginTest.java
new file mode 100644
index 0000000..c06927a
--- /dev/null
+++ b/src/test/java/org/apache/sling/discovery/oak/TopologyWebConsolePluginTest.java
@@ -0,0 +1,55 @@
+/*
+ * 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.discovery.oak;
+
+import java.util.UUID;
+
+import org.apache.sling.discovery.TopologyEvent;
+import org.apache.sling.discovery.base.its.setup.mock.MockFactory;
+import org.apache.sling.discovery.commons.providers.DefaultClusterView;
+import org.apache.sling.discovery.commons.providers.DummyTopologyView;
+import org.junit.Test;
+
+public class TopologyWebConsolePluginTest {
+
+ @Test
+ /** trivial test that covers the first new Exception catch case */
+ public void testNoResourceResolverFactory() throws Exception {
+ final DefaultClusterView cluster = new DefaultClusterView(UUID.randomUUID().toString());
+ final DummyTopologyView view = new DummyTopologyView()
+ .addInstance(UUID.randomUUID().toString(), cluster, true, true);
+ final TopologyWebConsolePlugin p = new TopologyWebConsolePlugin();
+ p.activate(MockFactory.mockBundleContext());
+ p.handleTopologyEvent(new TopologyEvent(TopologyEvent.Type.TOPOLOGY_INIT, null, view));
+ p.resourceResolverFactory = MockFactory.mockResourceResolverFactory();
+ p.handleTopologyEvent(new TopologyEvent(TopologyEvent.Type.TOPOLOGY_INIT, null, view));
+ }
+
+ @Test
+ /** trivial test that covers the 2nd new Exception catch case */
+ public void testResourceResolverFactory() throws Exception {
+ final DefaultClusterView cluster = new DefaultClusterView(UUID.randomUUID().toString());
+ final DummyTopologyView view = new DummyTopologyView()
+ .addInstance(UUID.randomUUID().toString(), cluster, true, true);
+ final TopologyWebConsolePlugin p = new TopologyWebConsolePlugin();
+ p.resourceResolverFactory = MockFactory.mockResourceResolverFactory();
+ p.activate(MockFactory.mockBundleContext());
+ p.handleTopologyEvent(new TopologyEvent(TopologyEvent.Type.TOPOLOGY_INIT, null, view));
+ }
+}
diff --git a/src/test/java/org/apache/sling/discovery/oak/cluster/OakClusterViewServiceTest.java b/src/test/java/org/apache/sling/discovery/oak/cluster/OakClusterViewServiceTest.java
new file mode 100644
index 0000000..4131721
--- /dev/null
+++ b/src/test/java/org/apache/sling/discovery/oak/cluster/OakClusterViewServiceTest.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.discovery.oak.cluster;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+import org.apache.sling.discovery.base.commons.ClusterViewService;
+import org.apache.sling.discovery.base.commons.UndefinedClusterViewException;
+import org.apache.sling.discovery.base.its.setup.VirtualInstance;
+import org.apache.sling.discovery.base.its.setup.mock.DummyResourceResolverFactory;
+import org.apache.sling.discovery.oak.its.setup.OakVirtualInstanceBuilder;
+import org.junit.Test;
+
+public class OakClusterViewServiceTest {
+
+ @Test
+ public void testLocalClusterView() throws Exception {
+ OakVirtualInstanceBuilder builder1 =
+ (OakVirtualInstanceBuilder) new OakVirtualInstanceBuilder()
+ .setDebugName("instance1")
+ .newRepository("/foo/barrio/foo/", true)
+ .setConnectorPingInterval(999)
+ .setConnectorPingTimeout(999);
+ VirtualInstance instance1 = builder1.build();
+
+ ClusterViewService cvs1 = instance1.getClusterViewService();
+
+ try {
+ cvs1.getLocalClusterView();
+ fail("should throw UndefinedClusterViewException");
+ } catch (UndefinedClusterViewException e) {
+ // ok
+ }
+
+ OakVirtualInstanceBuilder builder2 =
+ (OakVirtualInstanceBuilder) new OakVirtualInstanceBuilder()
+ .setDebugName("instance2")
+ .useRepositoryOf(instance1)
+ .setConnectorPingInterval(999)
+ .setConnectorPingTimeout(999);
+ VirtualInstance instance2 = builder2.build();
+
+ instance1.heartbeatsAndCheckView();
+ instance2.heartbeatsAndCheckView();
+ instance1.heartbeatsAndCheckView();
+ instance2.heartbeatsAndCheckView();
+
+ assertNotNull(cvs1.getLocalClusterView());
+ }
+
+ @Test
+ public void testGetResourceResolverException() throws Exception {
+ OakClusterViewService cvs = OakClusterViewService.testConstructor(null, null, null, null);
+ try {
+ cvs.getLocalClusterView();
+ fail("should throw UndefinedClusterViewException");
+ } catch (UndefinedClusterViewException e) {
+ // ok
+ }
+ }
+}