You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by ri...@apache.org on 2014/06/12 00:25:24 UTC
[5/7] git commit: code review comments: clean up checkSafe and add
some comments
code review comments: clean up checkSafe and add some 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/2f976887
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/2f976887
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/2f976887
Branch: refs/heads/master
Commit: 2f97688710be65854f82119da2bf9848a83bb52a
Parents: 7dfb233
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jun 11 13:18:40 2014 -0700
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Jun 11 13:18:40 2014 -0700
----------------------------------------------------------------------
.../entity/LocalManagementContextForTests.java | 13 ++++++++---
usage/cli/src/main/java/brooklyn/cli/Main.java | 2 ++
.../java/brooklyn/test/EntityTestUtils.java | 3 +++
.../src/main/java/brooklyn/util/os/Os.java | 24 ++++++++++++--------
4 files changed, 29 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2f976887/core/src/test/java/brooklyn/test/entity/LocalManagementContextForTests.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/test/entity/LocalManagementContextForTests.java b/core/src/test/java/brooklyn/test/entity/LocalManagementContextForTests.java
index 751517b..0c79491 100644
--- a/core/src/test/java/brooklyn/test/entity/LocalManagementContextForTests.java
+++ b/core/src/test/java/brooklyn/test/entity/LocalManagementContextForTests.java
@@ -10,15 +10,22 @@ import brooklyn.management.internal.LocalManagementContext;
public class LocalManagementContextForTests extends LocalManagementContext {
public LocalManagementContextForTests(BrooklynProperties brooklynProperties) {
- super(setEmptyCatalogAsDefault(brooklynProperties));
+ super(emptyIfNull(setEmptyCatalogAsDefault(brooklynProperties)));
}
+ private static BrooklynProperties emptyIfNull(BrooklynProperties bp) {
+ if (bp!=null) return bp;
+ return BrooklynProperties.Factory.newEmpty();
+ }
+
public static BrooklynProperties setEmptyCatalogAsDefault(BrooklynProperties brooklynProperties) {
+ if (brooklynProperties==null) return null;
brooklynProperties.putIfAbsent(AbstractManagementContext.BROOKLYN_CATALOG_URL, "classpath://brooklyn-catalog-empty.xml");
return brooklynProperties;
}
-
+
public LocalManagementContextForTests() {
- this(BrooklynProperties.Factory.newEmpty());
+ this(null);
}
+
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2f976887/usage/cli/src/main/java/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/usage/cli/src/main/java/brooklyn/cli/Main.java b/usage/cli/src/main/java/brooklyn/cli/Main.java
index 038ce91..dc3d863 100644
--- a/usage/cli/src/main/java/brooklyn/cli/Main.java
+++ b/usage/cli/src/main/java/brooklyn/cli/Main.java
@@ -88,6 +88,8 @@ public class Main {
public static final int EXECUTION_ERROR = 2;
public static final int CONFIGURATION_ERROR = 3;
+ /** @deprecated since 0.7.0 will become private static, subclasses should define their own logger */
+ @Deprecated
public static final Logger log = LoggerFactory.getLogger(Main.class);
public static void main(String... args) {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2f976887/usage/test-support/src/main/java/brooklyn/test/EntityTestUtils.java
----------------------------------------------------------------------
diff --git a/usage/test-support/src/main/java/brooklyn/test/EntityTestUtils.java b/usage/test-support/src/main/java/brooklyn/test/EntityTestUtils.java
index 474c31a..bf0f0d7 100644
--- a/usage/test-support/src/main/java/brooklyn/test/EntityTestUtils.java
+++ b/usage/test-support/src/main/java/brooklyn/test/EntityTestUtils.java
@@ -25,6 +25,9 @@ import com.google.common.collect.Maps;
public class EntityTestUtils {
+ // TODO would be nice to have this... perhaps moving this class, or perhaps this whole project, to core/src/test ?
+// public static LocalManagementContext newManagementContext() { return new LocalManagementContextForTests(); }
+
// TODO Delete methods from TestUtils, to just have them here (or switch so TestUtils delegates here,
// and deprecate methods in TestUtils until deleted).
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2f976887/utils/common/src/main/java/brooklyn/util/os/Os.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/os/Os.java b/utils/common/src/main/java/brooklyn/util/os/Os.java
index 6e76724..11c956e 100644
--- a/utils/common/src/main/java/brooklyn/util/os/Os.java
+++ b/utils/common/src/main/java/brooklyn/util/os/Os.java
@@ -189,7 +189,7 @@ public class Os {
/** tries to delete a directory recursively;
* use with care - see http://stackoverflow.com/questions/8320376/why-is-files-deletedirectorycontents-deprecated-in-guava.
* <p>
- * also note this implementation refuses to delete / or ~, it needs length at least 4 for safety.
+ * also note this implementation refuses to delete / or ~ or anything else not passing {@link #checkSafe(File)}.
* if you might really want to delete something like that, use {@link #deleteRecursively(File, boolean)}.
*/
@Beta
@@ -199,21 +199,14 @@ public class Os {
/**
* as {@link #deleteRecursively(File)} but includes safety checks to prevent deletion of / or ~
- * or any path of length 4 or less.
+ * or anything else not passing {@link #checkSafe(File)}, unless the skipSafetyChecks parameter is set
*/
@Beta
public static DeletionResult deleteRecursively(File dir, boolean skipSafetyChecks) {
try {
if (dir==null) return new DeletionResult(null, true, null);
- if (!skipSafetyChecks) {
- String dp = dir.getAbsolutePath();
- if (dp.length()<=4)
- throw new IOException("Refusing instruction to delete "+dir+": name too short");
-
- if (Os.home().equals(dp))
- throw new IOException("Refusing instruction to delete "+dir+": it's the home directory");
- }
+ if (!skipSafetyChecks) checkSafe(dir);
FileUtils.deleteDirectory(dir);
return new DeletionResult(dir, true, null);
@@ -221,6 +214,17 @@ public class Os {
return new DeletionResult(dir, false, e);
}
}
+
+ /** fails if the dir is not "safe" for deletion, currently length <= 2 or the home directory */
+ protected static void checkSafe(File dir) throws IOException {
+ String dp = dir.getAbsolutePath();
+ dp = Strings.removeFromEnd(dp, "/");
+ if (dp.length()<=2)
+ throw new IOException("Refusing instruction to delete "+dir+": name too short");
+
+ if (Os.home().equals(dp))
+ throw new IOException("Refusing instruction to delete "+dir+": it's the home directory");
+ }
/**
* @see {@link #deleteRecursively(File)}