This is an automated email from the ASF dual-hosted git repository.
chetanm pushed a commit to branch SLING-3049
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-log.git
The following commit(s) were added to refs/heads/SLING-3049 by this push:
new 70f44c8 SLING-3049 - Make Logback Stacktrace Packaging data support
OSGi aware
70f44c8 is described below
commit 70f44c8d7f6889a1065268f0716ec56691268ef1
Author: Chetan Mehrotra <[email protected]>
AuthorDate: Tue Oct 31 19:46:35 2017 +0530
SLING-3049 - Make Logback Stacktrace Packaging data support OSGi aware
Ensure that bundle info is only provided if there is a single owner of
any package. If same package is provided by multiple bundles then no bundle
info would be provided
---
pom.xml | 7 ++++
.../internal/stacktrace/PackageInfoCollector.java | 23 ++++++++---
.../logback/integration/PackagingDataTestUtil.java | 37 +++++++++++++----
.../stacktrace/PackageInfoCollectorTest.java | 48 +++++++++++++++++++++-
4 files changed, 100 insertions(+), 15 deletions(-)
diff --git a/pom.xml b/pom.xml
index d56aa44..c761e76 100644
--- a/pom.xml
+++ b/pom.xml
@@ -48,6 +48,7 @@
<slf4j.version>1.7.21</slf4j.version>
<logback.version>1.2.3</logback.version>
<pax-exam.version>3.5.0</pax-exam.version>
+ <sling.java.version>8</sling.java.version>
<bundle.build.dir>
${basedir}/target
@@ -392,6 +393,12 @@
<version>1.3</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>2.11.0</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<profiles>
diff --git
a/src/main/java/org/apache/sling/commons/log/logback/internal/stacktrace/PackageInfoCollector.java
b/src/main/java/org/apache/sling/commons/log/logback/internal/stacktrace/PackageInfoCollector.java
index e80eed1..cd78ba9 100644
---
a/src/main/java/org/apache/sling/commons/log/logback/internal/stacktrace/PackageInfoCollector.java
+++
b/src/main/java/org/apache/sling/commons/log/logback/internal/stacktrace/PackageInfoCollector.java
@@ -18,15 +18,18 @@
*/
package org.apache.sling.commons.log.logback.internal.stacktrace;
-import java.util.Map;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
import org.osgi.framework.Bundle;
import org.osgi.framework.hooks.weaving.WeavingHook;
import org.osgi.framework.hooks.weaving.WovenClass;
public class PackageInfoCollector implements WeavingHook{
- private final Map<String, String> pkgInfoMapping = new
ConcurrentHashMap<>();
+ private final ConcurrentMap<String, Set<String>> pkgInfoMapping = new
ConcurrentHashMap<>();
@Override
public void weave(WovenClass wovenClass) {
@@ -38,7 +41,10 @@ public class PackageInfoCollector implements WeavingHook{
}
void add(Bundle bundle, String className) {
- pkgInfoMapping.put(getPackageName(className), getInfo(bundle));
+ String packageName = getPackageName(className);
+
+ Set<String> infos = pkgInfoMapping.computeIfAbsent(packageName, k ->
Collections.synchronizedSet(new HashSet<>()));
+ infos.add(getInfo(bundle));
}
String getBundleInfo(String className) {
@@ -46,10 +52,17 @@ public class PackageInfoCollector implements WeavingHook{
return null;
}
String packageName = getPackageName(className);
- return pkgInfoMapping.get(packageName);
+ Set<String> infos = pkgInfoMapping.get(packageName);
+
+ //If multiple infos are found then we cannot determine the exact
version
+ //so better not to provide any info
+ if (infos == null || infos.size() > 1 || infos.size() == 0) {
+ return null;
+ }
+ return infos.iterator().next();
}
- static String getInfo(Bundle bundle) {
+ private static String getInfo(Bundle bundle) {
return bundle.getSymbolicName() + ":" + bundle.getVersion();
}
diff --git
a/src/test/java/org/apache/sling/commons/log/logback/integration/PackagingDataTestUtil.java
b/src/test/java/org/apache/sling/commons/log/logback/integration/PackagingDataTestUtil.java
index 05de164..8e9b0da 100644
---
a/src/test/java/org/apache/sling/commons/log/logback/integration/PackagingDataTestUtil.java
+++
b/src/test/java/org/apache/sling/commons/log/logback/integration/PackagingDataTestUtil.java
@@ -20,10 +20,9 @@
package org.apache.sling.commons.log.logback.integration;
import java.io.InputStream;
+import java.net.URL;
-import
org.apache.sling.commons.log.logback.integration.bundle.PackageDataActivator;
-import org.apache.sling.commons.log.logback.integration.bundle.TestRunnable;
-import org.ops4j.pax.tinybundles.core.InnerClassStrategy;
+import org.ops4j.pax.tinybundles.core.TinyBundle;
import org.osgi.framework.Constants;
import static org.ops4j.pax.tinybundles.core.TinyBundles.bundle;
@@ -39,12 +38,32 @@ public class PackagingDataTestUtil {
public static final String TEST_BUNDLE_NAME = "packagedatatest";
public static InputStream createTestBundle() {
- return bundle()
- .set(Constants.BUNDLE_ACTIVATOR,
PackageDataActivator.class.getName())
+ //Avoid referring to test bundle classes otherwise they get loaded in
2 bundles i.e.
+ //pax exam probe bundle and our packagedatatest. So we refer only by
class name strings
+ String activatorClassName =
"org.apache.sling.commons.log.logback.integration.bundle.PackageDataActivator";
+ TinyBundle tb = bundle()
+ .set(Constants.BUNDLE_ACTIVATOR, activatorClassName)
.set(Constants.BUNDLE_SYMBOLICNAME, TEST_BUNDLE_NAME)
- .set(Constants.BUNDLE_VERSION, TEST_BUNDLE_VERSION)
- .add(PackageDataActivator.class, InnerClassStrategy.NONE)
- .add(TestRunnable.class, InnerClassStrategy.NONE)
- .build(withBnd());
+ .set(Constants.BUNDLE_VERSION, TEST_BUNDLE_VERSION);
+ add(tb,
"org.apache.sling.commons.log.logback.integration.bundle.TestRunnable");
+ add(tb, activatorClassName);
+ return tb.build(withBnd());
+ }
+
+ private static void add(TinyBundle tb, String className) {
+ String name = asResource(className);
+ tb.add(name, asResourceURL(name));
+ }
+
+ private static String asResource( String klass ) {
+ return klass.replace( '.', '/' ) + ".class";
+ }
+
+ private static URL asResourceURL(String klass ) {
+ URL u = PackagingDataTestUtil.class.getResource("/" + klass);
+ if (u == null) {
+ throw new RuntimeException("No resource found for "+klass);
+ }
+ return u;
}
}
diff --git
a/src/test/java/org/apache/sling/commons/log/logback/internal/stacktrace/PackageInfoCollectorTest.java
b/src/test/java/org/apache/sling/commons/log/logback/internal/stacktrace/PackageInfoCollectorTest.java
index f3ec3f5..ec982be 100644
---
a/src/test/java/org/apache/sling/commons/log/logback/internal/stacktrace/PackageInfoCollectorTest.java
+++
b/src/test/java/org/apache/sling/commons/log/logback/internal/stacktrace/PackageInfoCollectorTest.java
@@ -20,14 +20,60 @@
package org.apache.sling.commons.log.logback.internal.stacktrace;
import org.junit.Test;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.Version;
+import org.osgi.framework.hooks.weaving.WovenClass;
+import org.osgi.framework.wiring.BundleWiring;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
public class PackageInfoCollectorTest {
+ private PackageInfoCollector collector = new PackageInfoCollector();
+
@Test
public void packageName() throws Exception{
assertEquals("com.foo",
PackageInfoCollector.getPackageName("com.foo.TestClass"));
assertEquals("",
PackageInfoCollector.getPackageName("packageInDefaultClass"));
}
+ @Test
+ public void getBundleInfo() throws Exception{
+ Bundle bundle = newBundle("foo.bundle", "0.0.7");
+ collector.weave(newWovenClass(bundle, "com.example.Foo"));
+
+
assertEquals("foo.bundle:0.0.7",collector.getBundleInfo("com.example.Foo"));
+
assertEquals("foo.bundle:0.0.7",collector.getBundleInfo("com.example.Bar"));
+ assertNull(collector.getBundleInfo("com.example2.Bar"));
+ }
+
+ /**
+ * For case where same package is present in multiple bundles then no
bundle info is provided
+ */
+ @Test
+ public void duplicates() throws Exception{
+ collector.weave(newWovenClass(newBundle("foo.bundle", "0.0.7"),
"com.example.Foo"));
+ collector.weave(newWovenClass(newBundle("foo.bundle2", "0.0.8"),
"com.example.Bar"));
+
+ assertNull(collector.getBundleInfo("com.example.Bar"));
+ }
+
+ private Bundle newBundle(String name, String version){
+ Bundle b = mock(Bundle.class);
+ when(b.getSymbolicName()).thenReturn(name);
+ when(b.getVersion()).thenReturn(Version.parseVersion(version));
+ return b;
+ }
+
+ private WovenClass newWovenClass(Bundle bundle, String className){
+ WovenClass woven = mock(WovenClass.class);
+ BundleWiring wiring = mock(BundleWiring.class);
+ when(woven.getBundleWiring()).thenReturn(wiring);
+
+ when(wiring.getBundle()).thenReturn(bundle);
+ when(woven.getClassName()).thenReturn(className);
+ return woven;
+ }
}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
