You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by sv...@apache.org on 2016/11/29 08:21:51 UTC
[2/6] brooklyn-server git commit: BROOKLYN-379: address review
comments
BROOKLYN-379: address review comments
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/72dccf08
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/72dccf08
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/72dccf08
Branch: refs/heads/master
Commit: 72dccf08bc4198d5abf466c43e7294c838cf80b5
Parents: 6e68d74
Author: Aled Sage <al...@gmail.com>
Authored: Thu Nov 17 09:56:42 2016 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Fri Nov 25 23:28:19 2016 +0000
----------------------------------------------------------------------
.../brooklyn/core/mgmt/ha/OsgiManager.java | 5 +
.../core/mgmt/persist/OsgiClassPrefixer.java | 29 ++++-
.../mgmt/persist/OsgiClassPrefixerTest.java | 111 +++++++++++++++++++
3 files changed, 140 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/72dccf08/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java
index d58dbad..7e68a31 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java
@@ -43,6 +43,7 @@ import org.apache.brooklyn.util.core.osgi.Osgis;
import org.apache.brooklyn.util.core.osgi.Osgis.BundleFinder;
import org.apache.brooklyn.util.core.osgi.SystemFrameworkLoader;
import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.os.Os;
import org.apache.brooklyn.util.os.Os.DeletionResult;
@@ -195,6 +196,10 @@ public class OsgiManager {
Bundle b = bundle.get();
Optional<String> strippedType = osgiClassPrefixer.stripMatchingPrefix(b, type);
String typeToLoad = strippedType.isPresent() ? strippedType.get() : type;
+ if (osgiClassPrefixer.hasPrefix(typeToLoad)) {
+ bundleProblems.put(osgiBundle, new UserFacingException("Bundle does not match prefix in type name '"+typeToLoad+"'"));
+ continue;
+ }
//Extension bundles don't support loadClass.
//Instead load from the app classpath.
Class<T> clazz = SystemFrameworkLoader.get().loadClassFromBundle(typeToLoad, b);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/72dccf08/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixer.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixer.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixer.java
index c1650f4..f252f8e 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixer.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixer.java
@@ -18,11 +18,16 @@
*/
package org.apache.brooklyn.core.mgmt.persist;
+import javax.annotation.Nullable;
+
import org.apache.brooklyn.core.mgmt.rebind.dto.MementosGenerators;
import org.apache.brooklyn.util.core.ClassLoaderUtils;
import org.apache.brooklyn.util.core.osgi.Osgis;
import org.osgi.framework.Bundle;
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
import com.google.common.base.Optional;
/**
@@ -33,27 +38,41 @@ import com.google.common.base.Optional;
* In that situation, the code writing the field must include the prefix (e.g. see
* {@link MementosGenerators}.
*/
+@Beta
public class OsgiClassPrefixer {
+ private static final String DELIMITER = ":";
+
private final ClassLoaderUtils whiteListRetriever;
+ private final Function<Class<?>, Optional<Bundle>> bundleRetriever;
public OsgiClassPrefixer() {
- whiteListRetriever = new ClassLoaderUtils(getClass());
+ this(null);
+ }
+
+ @VisibleForTesting
+ protected OsgiClassPrefixer(@Nullable Function<Class<?>, Optional<Bundle>> bundleRetriever) {
+ this.whiteListRetriever = new ClassLoaderUtils(getClass());
+ this.bundleRetriever = bundleRetriever;
}
public Optional<String> getPrefix(Class<?> type) {
- Optional<Bundle> bundle = Osgis.getBundleOf(type);
+ Optional<Bundle> bundle = (bundleRetriever != null) ? bundleRetriever.apply(type) : Osgis.getBundleOf(type);
if (bundle.isPresent() && !whiteListRetriever.isBundleWhiteListed(bundle.get())) {
- return Optional.of(bundle.get().getSymbolicName() + ":");
+ return Optional.of(bundle.get().getSymbolicName() + DELIMITER);
}
return Optional.absent();
}
public Optional<String> stripMatchingPrefix(Bundle bundle, String type) {
String symbolicName = bundle.getSymbolicName();
- if (symbolicName != null && type.startsWith(symbolicName + ":")) {
- return Optional.of(type.substring((symbolicName + ":").length()));
+ if (symbolicName != null && type.startsWith(symbolicName + DELIMITER)) {
+ return Optional.of(type.substring(type.lastIndexOf(DELIMITER) + 1));
}
return Optional.absent();
}
+
+ public boolean hasPrefix(String type) {
+ return type != null && type.contains(":");
+ }
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/72dccf08/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixerTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixerTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixerTest.java
new file mode 100644
index 0000000..19a5610
--- /dev/null
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/OsgiClassPrefixerTest.java
@@ -0,0 +1,111 @@
+/*
+ * 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.brooklyn.core.mgmt.persist;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import org.apache.brooklyn.core.BrooklynVersion;
+import org.apache.brooklyn.util.osgi.OsgiUtils;
+import org.mockito.Mockito;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.Version;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Function;
+import com.google.common.base.Optional;
+
+public class OsgiClassPrefixerTest {
+
+ @Test
+ public void testGetPrefixFromNonBundle() throws Exception {
+ OsgiClassPrefixer prefixer = new OsgiClassPrefixer();
+ assertAbsent(prefixer.getPrefix(String.class));
+ }
+
+ @Test
+ public void testGetPrefixWithBundle() throws Exception {
+ final Class<?> classInBundle = String.class;
+
+ final Bundle bundle = Mockito.mock(Bundle.class);
+ Mockito.when(bundle.getSymbolicName()).thenReturn("my.symbolic.name");
+ Mockito.when(bundle.getVersion()).thenReturn(Version.valueOf("1.2.3"));
+
+ Function<Class<?>, Optional<Bundle>> bundleRetriever = new Function<Class<?>, Optional<Bundle>>() {
+ @Override public Optional<Bundle> apply(Class<?> input) {
+ return (classInBundle.equals(input)) ? Optional.of(bundle) : Optional.<Bundle>absent();
+ }
+ };
+ OsgiClassPrefixer prefixer = new OsgiClassPrefixer(bundleRetriever);
+ assertAbsent(prefixer.getPrefix(Number.class));
+ assertPresent(prefixer.getPrefix(String.class), "my.symbolic.name:");
+ }
+
+ @Test
+ public void testGetPrefixWithWhitelistedBundle() throws Exception {
+ final Bundle bundle = Mockito.mock(Bundle.class);
+ Mockito.when(bundle.getSymbolicName()).thenReturn("org.apache.brooklyn.my-bundle");
+ Mockito.when(bundle.getVersion()).thenReturn(Version.valueOf(OsgiUtils.toOsgiVersion(BrooklynVersion.get())));
+
+ Function<Class<?>, Optional<Bundle>> bundleRetriever = new Function<Class<?>, Optional<Bundle>>() {
+ @Override public Optional<Bundle> apply(Class<?> input) {
+ return Optional.of(bundle);
+ }
+ };
+ OsgiClassPrefixer prefixer = new OsgiClassPrefixer(bundleRetriever);
+ assertAbsent(prefixer.getPrefix(String.class));
+ }
+
+ @Test
+ public void testStringPrefix() throws Exception {
+ Bundle bundle = Mockito.mock(Bundle.class);
+ Mockito.when(bundle.getSymbolicName()).thenReturn("my.symbolic.name");
+ Mockito.when(bundle.getVersion()).thenReturn(Version.valueOf("1.2.3"));
+
+ OsgiClassPrefixer prefixer = new OsgiClassPrefixer();
+ assertAbsent(prefixer.stripMatchingPrefix(bundle, "my.package.MyClass"));
+ assertAbsent(prefixer.stripMatchingPrefix(bundle, "different.symbolic.name:my.package.MyClass"));
+ assertAbsent(prefixer.stripMatchingPrefix(bundle, "different.symbolic.name:1.2.3:my.package.MyClass"));
+ assertPresent(prefixer.stripMatchingPrefix(bundle, "my.symbolic.name:my.package.MyClass"), "my.package.MyClass");
+ assertPresent(prefixer.stripMatchingPrefix(bundle, "my.symbolic.name:1.2.3:my.package.MyClass"), "my.package.MyClass");
+
+ // TODO Will match any version - is that good enough?
+ // Is it the right thing to do, to make upgrades simpler?!
+ assertPresent(prefixer.stripMatchingPrefix(bundle, "my.symbolic.name:1.0.0:my.package.MyClass"), "my.package.MyClass");
+ }
+
+ @Test
+ public void testHasPrefix() throws Exception {
+ OsgiClassPrefixer prefixer = new OsgiClassPrefixer();
+ assertFalse(prefixer.hasPrefix("MyClassInDefaultPackage"));
+ assertFalse(prefixer.hasPrefix("my.package.MyClass"));
+ assertTrue(prefixer.hasPrefix("my.symbolic.name:my.package.MyClass"));
+ assertTrue(prefixer.hasPrefix("my.symbolic.name:1.2.3:my.package.MyClass"));
+ }
+
+ private void assertAbsent(Optional<?> val) {
+ assertFalse(val.isPresent(), "val="+val);
+ }
+
+ private <T> void assertPresent(Optional<T> val, T expected) {
+ assertTrue(val.isPresent(), "val="+val);
+ assertEquals(val.get(), expected);
+ }
+}