You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2014/11/13 23:23:45 UTC
[12/18] incubator-brooklyn git commit: Catalog versioning -
consistently use symbolicName and other minor changes
Catalog versioning - consistently use symbolicName and other minor changes
Addressing first review comments.
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/497496c1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/497496c1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/497496c1
Branch: refs/heads/master
Commit: 497496c1563a10608628c566fe17e7be8c68f2bf
Parents: 9c6539f
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Wed Nov 12 12:59:33 2014 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Thu Nov 13 11:49:54 2014 +0200
----------------------------------------------------------------------
api/src/main/java/brooklyn/catalog/CatalogItem.java | 2 +-
.../catalog/internal/BasicBrooklynCatalog.java | 13 ++++++++-----
.../brooklyn/catalog/internal/CatalogBundleDto.java | 12 ++++++------
.../catalog/internal/CatalogItemComparator.java | 3 +--
.../persister/BrooklynMementoPersisterToMultiFile.java | 2 +-
.../BrooklynMementoPersisterToObjectStore.java | 4 +---
.../main/java/brooklyn/management/ha/OsgiManager.java | 10 +++++-----
core/src/main/java/brooklyn/util/osgi/Osgis.java | 2 +-
.../camp/brooklyn/catalog/CatalogYamlEntityTest.java | 4 ++--
.../src/main/java/brooklyn/util/io/FileUtil.java | 8 --------
10 files changed, 26 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/api/src/main/java/brooklyn/catalog/CatalogItem.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/catalog/CatalogItem.java b/api/src/main/java/brooklyn/catalog/CatalogItem.java
index c58dcd3..d06a682 100644
--- a/api/src/main/java/brooklyn/catalog/CatalogItem.java
+++ b/api/src/main/java/brooklyn/catalog/CatalogItem.java
@@ -37,7 +37,7 @@ public interface CatalogItem<T,SpecT> extends BrooklynObject, Rebindable {
}
public static interface CatalogBundle {
- public String getName();
+ public String getSymbolicName();
public String getVersion();
public String getUrl();
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
index ed85e76..bc43682 100644
--- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
@@ -193,7 +193,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
private String getDefaultVersion(String symbolicName) {
Iterable<CatalogItem<Object, Object>> versions = getCatalogItems(CatalogPredicates.symbolicName(Predicates.equalTo(symbolicName)));
- ImmutableSortedSet<CatalogItem<?, ?>> orderedVersions = ImmutableSortedSet.orderedBy(new CatalogItemComparator()).addAll(versions).build();
+ ImmutableSortedSet<CatalogItem<?, ?>> orderedVersions = ImmutableSortedSet.orderedBy(CatalogItemComparator.INSTANCE).addAll(versions).build();
if (!orderedVersions.isEmpty()) {
return orderedVersions.iterator().next().getVersion();
} else {
@@ -469,11 +469,14 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
if (possibleVersion.isAbsent() && Strings.isBlank(version)) {
throw new IllegalArgumentException("'version' attribute missing in 'brooklyn.catalog' section.");
} else if (possibleVersion.isPresent()) {
- if (Strings.isNonBlank(version)) {
- throw new IllegalArgumentException("Can't use both attribute 'version' and versioned id");
- }
//could be coalesced to a number - can be one of Integer, Double, String
- version = possibleVersion.get().toString();
+ String versionProperty = possibleVersion.get().toString();
+
+ if (!Strings.isBlank(version) && !versionProperty.equals(version)) {
+ throw new IllegalArgumentException("Discrepency between version set in id/name " + version + " and version property " + versionProperty);
+ }
+
+ version = versionProperty;
}
CatalogUtils.installLibraries(mgmt, libraries);
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java b/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java
index 9f65f80..35353c4 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java
@@ -24,7 +24,7 @@ import com.google.common.base.Preconditions;
import brooklyn.catalog.CatalogItem.CatalogBundle;
public class CatalogBundleDto implements CatalogBundle {
- private String name;
+ private String symbolicName;
private String version;
private String url;
@@ -38,19 +38,19 @@ public class CatalogBundleDto implements CatalogBundle {
Preconditions.checkNotNull(version, "version");
}
- this.name = name;
+ this.symbolicName = name;
this.version = version;
this.url = url;
}
@Override
public boolean isNamed() {
- return name != null && version != null;
+ return symbolicName != null && version != null;
}
@Override
- public String getName() {
- return name;
+ public String getSymbolicName() {
+ return symbolicName;
}
@Override
@@ -66,7 +66,7 @@ public class CatalogBundleDto implements CatalogBundle {
@Override
public String toString() {
return Objects.toStringHelper(this)
- .add("name", name)
+ .add("symbolicName", symbolicName)
.add("version", version)
.add("url", url)
.toString();
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java b/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java
index 4ac03f9..a202b92 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java
@@ -25,7 +25,6 @@ import brooklyn.util.text.NaturalOrderComparator;
public class CatalogItemComparator implements Comparator<CatalogItem<?, ?>> {
private static final String SNAPSHOT = "SNAPSHOT";
- private static final Comparator<String> COMPARATOR = new NaturalOrderComparator();
public static final CatalogItemComparator INSTANCE = new CatalogItemComparator();
@@ -40,7 +39,7 @@ public class CatalogItemComparator implements Comparator<CatalogItem<?, ?>> {
boolean isV1Snapshot = v1.contains(SNAPSHOT);
boolean isV2Snapshot = v2.contains(SNAPSHOT);
if (isV1Snapshot == isV2Snapshot) {
- return -COMPARATOR.compare(v1, v2);
+ return -NaturalOrderComparator.INSTANCE.compare(v1, v2);
} else if (isV1Snapshot) {
return 1;
} else {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToMultiFile.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToMultiFile.java b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToMultiFile.java
index ac49879..74817c5 100644
--- a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToMultiFile.java
+++ b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToMultiFile.java
@@ -580,7 +580,7 @@ public class BrooklynMementoPersisterToMultiFile implements BrooklynMementoPersi
}
private File getFileFor(File parent, String id) {
- return new File(parent, FileUtil.getSafeFileName(id));
+ return new File(parent, Strings.makeValidFilename(id));
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
index 28a2810..38cc12d 100644
--- a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
+++ b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
@@ -31,7 +31,6 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
-import java.util.regex.Pattern;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -57,7 +56,6 @@ import brooklyn.mementos.Memento;
import brooklyn.util.collections.MutableMap;
import brooklyn.util.exceptions.CompoundRuntimeException;
import brooklyn.util.exceptions.Exceptions;
-import brooklyn.util.io.FileUtil;
import brooklyn.util.text.Strings;
import brooklyn.util.time.Duration;
import brooklyn.util.time.Time;
@@ -659,7 +657,7 @@ public class BrooklynMementoPersisterToObjectStore implements BrooklynMementoPer
}
private String getPath(String subPath, String id) {
- return subPath+"/"+FileUtil.getSafeFileName(id);
+ return subPath+"/"+Strings.makeValidFilename(id);
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/core/src/main/java/brooklyn/management/ha/OsgiManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/OsgiManager.java b/core/src/main/java/brooklyn/management/ha/OsgiManager.java
index c3a2faa..1042d1f 100644
--- a/core/src/main/java/brooklyn/management/ha/OsgiManager.java
+++ b/core/src/main/java/brooklyn/management/ha/OsgiManager.java
@@ -112,10 +112,10 @@ public class OsgiManager {
String nv = b.getSymbolicName()+":"+b.getVersion().toString();
if (bundle.isNamed() &&
- (!bundle.getName().equals(b.getSymbolicName()) ||
+ (!bundle.getSymbolicName().equals(b.getSymbolicName()) ||
!bundle.getVersion().equals(b.getVersion().toString()))) {
log.warn("Bundle at " + bundle.getUrl() + " installed as " + nv +
- " but user explicitly requested " + bundle.getName() + ":" + bundle.getVersion());
+ " but user explicitly requested " + bundle.getSymbolicName() + ":" + bundle.getVersion());
}
List<Bundle> matches = Osgis.bundleFinder(framework)
@@ -139,7 +139,7 @@ public class OsgiManager {
if (bundleUrl != null) {
VersionedName nv = urlToBundleIdentifier.get(bundleUrl);
if (nv!=null) {
- if (bundle.isNamed() && !nv.equals(bundle.getName(), bundle.getVersion())) {
+ if (bundle.isNamed() && !nv.equals(bundle.getSymbolicName(), bundle.getVersion())) {
throw new IllegalStateException("Bundle from "+bundleUrl+" already installed as "+nv+" but user explicitly requested "+bundle);
}
Maybe<Bundle> installedBundle = Osgis.bundleFinder(framework).requiringFromUrl(bundleUrl).find();
@@ -157,7 +157,7 @@ public class OsgiManager {
}
}
} else {
- Maybe<Bundle> installedBundle = Osgis.bundleFinder(framework).symbolicName(bundle.getName()).version(bundle.getVersion()).find();
+ Maybe<Bundle> installedBundle = Osgis.bundleFinder(framework).symbolicName(bundle.getSymbolicName()).version(bundle.getVersion()).find();
if (installedBundle.isPresent()) {
log.trace("Bundle "+bundle+" installed from "+installedBundle.get().getLocation());
} else {
@@ -231,7 +231,7 @@ public class OsgiManager {
if (catalogBundle.getUrl() != null) {
bundleFinder.requiringFromUrl(catalogBundle.getUrl());
} else {
- bundleFinder.symbolicName(catalogBundle.getName()).version(catalogBundle.getVersion());
+ bundleFinder.symbolicName(catalogBundle.getSymbolicName()).version(catalogBundle.getVersion());
}
return bundleFinder.find();
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/core/src/main/java/brooklyn/util/osgi/Osgis.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/osgi/Osgis.java b/core/src/main/java/brooklyn/util/osgi/Osgis.java
index dfcea83..c6c9d9a 100644
--- a/core/src/main/java/brooklyn/util/osgi/Osgis.java
+++ b/core/src/main/java/brooklyn/util/osgi/Osgis.java
@@ -163,7 +163,7 @@ public class Osgis {
public BundleFinder bundle(CatalogBundle bundle) {
if (bundle.isNamed()) {
- symbolicName(bundle.getName());
+ symbolicName(bundle.getSymbolicName());
version(bundle.getVersion());
}
if (bundle.getUrl() != null) {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
index c0083de..4ce917b 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
@@ -211,7 +211,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
"- type: " + SIMPLE_ENTITY_TYPE);
fail();
} catch (IllegalStateException e) {
- Assert.assertEquals(e.getMessage(), "Bundle CatalogBundleDto{name=" + nonExistentId + ", version=" + nonExistentVersion + ", url=null} not previously registered, but URL is empty.");
+ Assert.assertEquals(e.getMessage(), "Bundle CatalogBundleDto{symbolicName=" + nonExistentId + ", version=" + nonExistentVersion + ", url=null} not previously registered, but URL is empty.");
}
}
@@ -300,7 +300,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
"- type: " + SIMPLE_ENTITY_TYPE);
fail();
} catch (IllegalStateException e) {
- assertEquals(e.getMessage(), "Bundle CatalogBundleDto{name=" + nonExistentId + ", version=" + nonExistentVersion + ", url=null} " +
+ assertEquals(e.getMessage(), "Bundle CatalogBundleDto{symbolicName=" + nonExistentId + ", version=" + nonExistentVersion + ", url=null} " +
"not previously registered, but URL is empty.");
}
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/497496c1/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/io/FileUtil.java b/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
index aed8450..66e7fc1 100644
--- a/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
+++ b/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
@@ -45,11 +45,6 @@ public class FileUtil {
private static final Logger LOG = LoggerFactory.getLogger(FileUtil.class);
- //Linux allows any characters except /
- //Windows reserves the following set: < > : " / \ | ? *
- //Object stores: ???, better be conservative
- private static final Pattern FILE_NAME_BLACKLIST_CHARACTERS = Pattern.compile("[^\\w\\d \\-_.()\\[\\]$!]");
-
private static boolean loggedSetFilePermissionsWarning = false;
// When we move to java 7, we can use Files.setPosixFilePermissions
@@ -204,7 +199,4 @@ public class FileUtil {
}
}
- public static String getSafeFileName(String str) {
- return FILE_NAME_BLACKLIST_CHARACTERS.matcher(str).replaceAll("_");
- }
}