brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [12/18] incubator-brooklyn git commit: Catalog versioning - consistently use symbolicName and other minor changes
Date Thu, 13 Nov 2014 22:23:45 GMT
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 <svetoslav.neykov@cloudsoftcorp.com>
Authored: Wed Nov 12 12:59:33 2014 +0200
Committer: Svetoslav Neykov <svetoslav.neykov@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("_");
-    }
 }


Mime
View raw message