diff --git a/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java b/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java index 90eff4bed8..104fdbfdd5 100644 --- a/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java +++ b/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java @@ -45,6 +45,7 @@ public interface CatalogItem extends BrooklynObject, Rebindable { public static enum CatalogItemType { TEMPLATE, + APPLICATION, ENTITY, POLICY, ENRICHER, @@ -63,7 +64,7 @@ public static CatalogItemType ofTargetClass(Class type if (Policy.class.isAssignableFrom(type)) return POLICY; if (Enricher.class.isAssignableFrom(type)) return ENRICHER; if (Location.class.isAssignableFrom(type)) return LOCATION; - if (Application.class.isAssignableFrom(type)) return TEMPLATE; + if (Application.class.isAssignableFrom(type)) return APPLICATION; if (Entity.class.isAssignableFrom(type)) return ENTITY; return null; } diff --git a/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObjectType.java b/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObjectType.java index f4a175648c..4bde7d6708 100644 --- a/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObjectType.java +++ b/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObjectType.java @@ -99,9 +99,10 @@ public static BrooklynObjectType of(CatalogItemType t) { switch (t) { case ENRICHER: return BrooklynObjectType.ENRICHER; case ENTITY: return BrooklynObjectType.ENTITY; + case APPLICATION: return BrooklynObjectType.ENTITY; + case TEMPLATE: return BrooklynObjectType.ENTITY; case LOCATION: return BrooklynObjectType.LOCATION; case POLICY: return BrooklynObjectType.POLICY; - case TEMPLATE: return BrooklynObjectType.ENTITY; default: return BrooklynObjectType.UNKNOWN; } } diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java index 89f282b262..e456493a31 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java @@ -22,6 +22,7 @@ import java.io.File; import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; @@ -35,6 +36,7 @@ import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.ConfigPredicates; import org.apache.brooklyn.core.config.MapConfigKey; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityAsserts; @@ -43,6 +45,7 @@ import org.apache.brooklyn.core.test.entity.TestEntityImpl; import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess; import org.apache.brooklyn.entity.stock.BasicApplication; +import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool; import org.apache.brooklyn.util.os.Os; @@ -556,6 +559,9 @@ public void testEntityRuntimeInheritanceOptions() throws Exception { " type: java.util.Map", " inheritance.runtime: not_reinherited"); + ImmutableMap defaultKV = ImmutableMap.of("myDefaultKey", "myDefaultVal"); + ImmutableMap myKV = ImmutableMap.of("mykey", "myval"); + // Test retrieval of defaults { String yaml = Joiner.on("\n").join( @@ -567,10 +573,10 @@ public void testEntityRuntimeInheritanceOptions() throws Exception { Entity app = createStartWaitAndLogApplication(yaml); TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); - assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-merged", ImmutableMap.of("myDefaultKey", "myDefaultVal")); - assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-overwrite", ImmutableMap.of("myDefaultKey", "myDefaultVal")); - assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-never", ImmutableMap.of("myDefaultKey", "myDefaultVal")); - assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-not-reinherited", ImmutableMap.of("myDefaultKey", "myDefaultVal")); + assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-merged", defaultKV); + assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-overwrite", defaultKV); + assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-never", defaultKV); + assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-not-reinherited", defaultKV); } // Test override defaults in parent entity @@ -593,10 +599,10 @@ public void testEntityRuntimeInheritanceOptions() throws Exception { Entity app = createStartWaitAndLogApplication(yaml); TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); - assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-merged", ImmutableMap.of("mykey", "myval")); - assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-overwrite", ImmutableMap.of("mykey", "myval")); - assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-never", ImmutableMap.of("myDefaultKey", "myDefaultVal")); - assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-not-reinherited", ImmutableMap.of("mykey", "myval")); + assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-merged", myKV); + assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-overwrite", myKV); + assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-never", defaultKV); + assertDeclaredKeyAndAnonymousKeyValuesEqual(entity, "map.type-not-reinherited", myKV); } // Test merging/overriding of explicit config @@ -649,28 +655,60 @@ public void testEntityRuntimeInheritanceOptions() throws Exception { " map.type-not-reinherited:", " mykey: myval", " brooklyn.children:", + " - type: " + TestEntity.class.getName(), + "- type: entity-with-keys", + " brooklyn.children:", " - type: " + TestEntity.class.getName()); Entity app = createStartWaitAndLogApplication(yaml); - TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); - TestEntity child = (TestEntity) Iterables.getOnlyElement(entity.getChildren()); + Iterator children = app.getChildren().iterator(); + TestEntity entityMyValues = (TestEntity) children.next(); + TestEntity childMyValues = (TestEntity) Iterables.getOnlyElement(entityMyValues.getChildren()); + TestEntity entityNoValues = (TestEntity) children.next(); + TestEntity childNoValues = (TestEntity) Iterables.getOnlyElement(entityNoValues.getChildren()); - // Not using `assertConfigEquals(child, ...)`, bacause child.getEntityType().getConfigKey() - // will not find these keys. - // - // TODO Note the different behaviour for "never" and "not-reinherited" keys for if an - // untyped key is used, versus the strongly typed key. We can live with that - I wouldn't - // expect an implementation of TestEntity to try to use such keys that it doesn't know - // about! - assertEquals(child.config().get(entity.getEntityType().getConfigKey("map.type-merged")), ImmutableMap.of("mykey", "myval")); - assertEquals(child.config().get(entity.getEntityType().getConfigKey("map.type-overwrite")), ImmutableMap.of("mykey", "myval")); - assertEquals(child.config().get(entity.getEntityType().getConfigKey("map.type-never")), ImmutableMap.of("myDefaultKey", "myDefaultVal")); - assertEquals(child.config().get(entity.getEntityType().getConfigKey("map.type-not-reinherited")), ImmutableMap.of("myDefaultKey", "myDefaultVal")); + // looking first at child with MY values (from runtime mgmt parent) + + // using keys from cNV we see the inherited values or if not inherited we don't see the keys + assertEquals(childMyValues.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-merged")), myKV); + assertEquals(childMyValues.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-overwrite")), myKV); + assertEquals(childMyValues.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-never")), null); + assertEquals(childMyValues.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-not-reinherited")), null); - assertEquals(child.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-merged")), ImmutableMap.of("mykey", "myval")); - assertEquals(child.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-overwrite")), ImmutableMap.of("mykey", "myval")); - assertEquals(child.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-never")), null); - assertEquals(child.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-not-reinherited")), null); + // using keys from eNV as originally defined, we get the runtime inherited values or original default values + // the use of non-inherited keys is weird but allows us to assert the right things happen depending which + // keys we use (this of course is also why we don't use `assertConfigEquals(child, ...)` + assertEquals(childMyValues.config().get(entityNoValues.getEntityType().getConfigKey("map.type-merged")), myKV); + assertEquals(childMyValues.config().get(entityNoValues.getEntityType().getConfigKey("map.type-overwrite")), myKV); + assertEquals(childMyValues.config().get(entityNoValues.getEntityType().getConfigKey("map.type-never")), defaultKV); + assertEquals(childMyValues.config().get(entityNoValues.getEntityType().getConfigKey("map.type-not-reinherited")), defaultKV); + + // if we use eMV keys, the "defaults" we see are the values it has set, since we change defaults (2018-11) + assertEquals(childMyValues.config().get(entityMyValues.getEntityType().getConfigKey("map.type-merged")), myKV); + assertEquals(childMyValues.config().get(entityMyValues.getEntityType().getConfigKey("map.type-overwrite")), myKV); + assertEquals(childMyValues.config().get(entityMyValues.getEntityType().getConfigKey("map.type-never")), myKV); + assertEquals(childMyValues.config().get(entityMyValues.getEntityType().getConfigKey("map.type-not-reinherited")), myKV); + + // looking at child with NO values + + // using keys from cNV we see the default values or if not inherited we don't see the keys + assertEquals(childNoValues.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-merged")), defaultKV); + assertEquals(childNoValues.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-overwrite")), defaultKV); + assertEquals(childNoValues.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-never")), null); + assertEquals(childNoValues.config().get(ConfigKeys.newConfigKey(Map.class, "map.type-not-reinherited")), null); + + // using keys from eNV as originally defined, we get the runtime inherited values or original default values + assertEquals(childNoValues.config().get(entityNoValues.getEntityType().getConfigKey("map.type-merged")), defaultKV); + assertEquals(childNoValues.config().get(entityNoValues.getEntityType().getConfigKey("map.type-overwrite")), defaultKV); + assertEquals(childNoValues.config().get(entityNoValues.getEntityType().getConfigKey("map.type-never")), defaultKV); + assertEquals(childNoValues.config().get(entityNoValues.getEntityType().getConfigKey("map.type-not-reinherited")), defaultKV); + + // if we use eMV keys, the "defaults" we see are the values it has set, since we change defaults (2018-11) + // (this is a weird one, we wouldn't do this in real life!) + assertEquals(childNoValues.config().get(entityMyValues.getEntityType().getConfigKey("map.type-merged")), myKV); + assertEquals(childNoValues.config().get(entityMyValues.getEntityType().getConfigKey("map.type-overwrite")), myKV); + assertEquals(childNoValues.config().get(entityMyValues.getEntityType().getConfigKey("map.type-never")), myKV); + assertEquals(childNoValues.config().get(entityMyValues.getEntityType().getConfigKey("map.type-not-reinherited")), myKV); } // Test inheritance by child with duplicated keys that have no defaults - does it get runtime-management parent's defaults? @@ -687,8 +725,8 @@ public void testEntityRuntimeInheritanceOptions() throws Exception { // Note this merges the default values from the parent and child (i.e. the child's default // value does not override the parent's. - assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-merged", ImmutableMap.of("myDefaultKey", "myDefaultVal")); - assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-overwrite", ImmutableMap.of("myDefaultKey", "myDefaultVal")); + assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-merged", defaultKV); + assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-overwrite", defaultKV); assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-never", null); assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-not-reinherited", null); } @@ -715,8 +753,8 @@ public void testEntityRuntimeInheritanceOptions() throws Exception { TestEntity child = (TestEntity) Iterables.getOnlyElement(entity.getChildren()); - assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-merged", ImmutableMap.of("mykey", "myval")); - assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-overwrite", ImmutableMap.of("mykey", "myval")); + assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-merged", myKV); + assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-overwrite", myKV); assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-never", null); assertDeclaredKeyAndAnonymousKeyValuesEqual(child, "map.type-not-reinherited", null); } @@ -741,7 +779,7 @@ public void testExtendsSuperTypeConfigSimple() throws Exception { } @Test - public void testExtendsSuperTypeConfigMixingLongOverridingShortNames() throws Exception { + public Entity testExtendsSuperTypeConfigMixingLongOverridingShortNames() throws Exception { ImmutableMap expectedEnv = ImmutableMap.of("ENV1", "myEnv1", "ENV2", "myEnv2"); // super-type has env; sub-type shell.env @@ -756,8 +794,21 @@ public void testExtendsSuperTypeConfigMixingLongOverridingShortNames() throws Ex Entity app = createStartWaitAndLogApplication(yaml); Entity entity = Iterables.getOnlyElement(app.getChildren()); EntityAsserts.assertConfigEquals(entity, EmptySoftwareProcess.SHELL_ENVIRONMENT, expectedEnv); + return entity; } - + + /** EntitySpecs track flags and config separately, with the merge occurring on Entity creation, + * so from the spec alone it is not straightforward to include flags in the determination of the default value. + * Thus default values do _not_ include flags and this test has only the shell.env KV pair in the default + * (so size 1, not size 2 which we want) + */ + @Test(groups="Broken") + public void testDefaultValueWhenExtendingSuperTypeConfigMixingLongOverridingShortNames() throws Exception { + Entity entity = testExtendsSuperTypeConfigMixingLongOverridingShortNames(); + ConfigKey key = Iterables.getOnlyElement(entity.config().findKeysDeclared(ConfigPredicates.nameEqualTo(EmptySoftwareProcess.SHELL_ENVIRONMENT.getName()))); + Asserts.assertSize((Map)key.getDefaultValue(), 2); + } + @Test public void testExtendsSuperTypeConfigMixingShortOverridingLongName() throws Exception { ImmutableMap expectedEnv = ImmutableMap.of("ENV1", "myEnv1", "ENV2", "myEnv2"); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java index 87be1a45dc..3c8297f214 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java @@ -28,6 +28,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -175,7 +176,12 @@ public void testConfigParameterOverridingJavaListedInType() throws Exception { // it was using the `MapConfigKey`. Unfortunately the `MapConfigKey` uses a different structure // for storing its data (flattening out the map). So the subsequent lookup failed. // - // There are three parts to fixing this: + // 2018-11 This is now working as the BasicConfigKey acts better; + // however more complicated tests could be written for inconsistencies between YAML "Map" + // parameters (ie BasicConfigKey) and java MapConfigKey. + // Remember these notes if we encounter those! + // + // These are the anticipated steps to fixing this better: // 1. [DONE] In `entity.config().set(key, val)`, replace `key` with the entity's own key // (i.e. the same logic that will subsequently be used in the `get`). // 2. [DONE] In `BrooklynComponentTemplateResolver.findAllFlagsAndConfigKeyValues`, respect @@ -186,19 +192,7 @@ public void testConfigParameterOverridingJavaListedInType() throws Exception { // 4. [TODO] Major overhaul of the ConfigKey name versus `SetFromFlag` alias. It is currently // confusing in when reading the config values what the precedence is because there are // different names that are only understood by some things. - @Test(groups="Broken") public void testConfigParameterOverridingJavaMapConfigKey() throws Exception { - runConfigParameterOverridingJavaMapConfigKey(true); - } - - @Test - public void testConfigParameterOverridingJavaMapConfigKeyWithoutRebindValueCheck() throws Exception { - // A cut-down test of what is actually working just now (so we can detect any - // further regressions!) - runConfigParameterOverridingJavaMapConfigKey(false); - } - - protected void runConfigParameterOverridingJavaMapConfigKey(boolean assertReboundVal) throws Exception { addCatalogItems( "brooklyn.catalog:", " itemType: entity", @@ -227,15 +221,7 @@ protected void runConfigParameterOverridingJavaMapConfigKey(boolean assertReboun // Rebind, and then check again that the config key is listed Entity newApp = rebind(); TestEntity newEntity = (TestEntity) Iterables.getOnlyElement(newApp.getChildren()); - if (assertReboundVal) { - assertKeyEquals(newEntity, TestEntity.CONF_MAP_THING.getName(), "myDescription", java.util.Map.class, null, ImmutableMap.of("mykey", "myval")); - } else { - // TODO delete duplication from `assertKeyEquals`, when the above works! - ConfigKey key = newEntity.getEntityType().getConfigKey(TestEntity.CONF_MAP_THING.getName()); - assertEquals(key.getDescription(), "myDescription"); - assertEquals(key.getType(), java.util.Map.class); - assertEquals(key.getDefaultValue(), null); - } + assertKeyEquals(newEntity, TestEntity.CONF_MAP_THING.getName(), "myDescription", java.util.Map.class, null, ImmutableMap.of("mykey", "myval")); } @Test @@ -1357,16 +1343,22 @@ public static interface TestEntityWithPinnedConfig extends Entity { public static class TestEntityWithPinnedConfigImpl extends TestEntityImpl implements TestEntityWithPinnedConfig { } - protected void assertKeyEquals(Entity entity, String keyName, String expectedDescription, Class expectedType, T expectedDefaultVal, T expectedEntityVal) { + protected void assertKeyEquals(Entity entity, String keyName, String expectedDescription, Class expectedType, T expectedDefaultValIfNoEntityVal, T expectedEntityVal) { ConfigKey key = entity.getEntityType().getConfigKey(keyName); assertNotNull(key, "No key '"+keyName+"'; keys="+entity.getEntityType().getConfigKeys()); assertEquals(key.getName(), keyName); assertEquals(key.getDescription(), expectedDescription); assertEquals(key.getType(), expectedType); - assertEquals(key.getDefaultValue(), expectedDefaultVal); - assertEquals(entity.config().get(key), expectedEntityVal); + + if (Objects.equals(key.getDefaultValue(), expectedEntityVal!=null ? expectedEntityVal : expectedDefaultValIfNoEntityVal)) { + // fine - if the entity has a value, we want the default value to have changed to be that actual value; + // this is because types defined in yaml, when subtyped, should see the value the parent set. + // if there is no value then the original default is expected. + } else { + Assert.fail("Default value for "+key+" not as expected: "+key.getDefaultValue()); + } } @ImplementedBy(TestEntityWithUninheritedConfigImpl.class) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java index 53a6be84ce..571076d0ab 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java @@ -293,6 +293,8 @@ public void testDeferredSupplierToConfig() throws Exception { " - $brooklyn:config(\"myOtherConf\")", " test.confSetPlain:", " - $brooklyn:config(\"myOtherConf\")", + " test.confSetStringPlain:", + " - $brooklyn:config(\"myOtherConf\")", " myOtherConf: myOther"); final Entity app = createStartWaitAndLogApplication(yaml); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java index 67cb48fe88..f39c3da6fe 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java @@ -227,8 +227,26 @@ protected void doDslAttributeWhenReadyPersistedAsDeferredSupplier(boolean resolv // foo // // + + // may be longer if there is a constraint, inheritance, esp if it is a BasicConfigKeyOverwriting storing the parent's definition also, eg + // (with some cleanups now since 2018-11 the BasicConfigKey will prefer nulls for lesser-used fields with common default values) +// +// test.confName +// java.lang.String +// Configuration key, my name +// +// false +// +// test.confName +// java.lang.String +// Configuration key, my name +// defaultval +// false +// +// + - Assert.assertTrue(testConfNamePersistedState.length() < 400, "persisted state too long: " + testConfNamePersistedState); + Assert.assertTrue(testConfNamePersistedState.length() < 1200, "persisted state too long ("+testConfNamePersistedState.length()+"): " + testConfNamePersistedState); Assert.assertFalse(testConfNamePersistedState.contains("bar"), "value 'bar' leaked in persisted state"); } diff --git a/core/pom.xml b/core/pom.xml index e7e7236201..28de44e9a6 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -261,9 +261,9 @@ groovy-eclipse-compiler - + **/*.groovy - + true false ${java.version} diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java b/core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java index 4756007e50..eb44df4ed6 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java @@ -141,8 +141,10 @@ public String toString() { public static final Predicate>> IS_TEMPLATE = CatalogPredicates.>isCatalogItemType(CatalogItemType.TEMPLATE); + public static final Predicate>> IS_APPLICATION = + CatalogPredicates.>isCatalogItemType(CatalogItemType.APPLICATION); public static final Predicate>> IS_ENTITY = - CatalogPredicates.>isCatalogItemType(CatalogItemType.ENTITY); + CatalogPredicates.>isCatalogItemType(CatalogItemType.ENTITY); public static final Predicate>> IS_POLICY = CatalogPredicates.>isCatalogItemType(CatalogItemType.POLICY); public static final Predicate>> IS_ENRICHER = diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java index 5b98cb0b81..504baeb779 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java @@ -954,6 +954,9 @@ private void collectCatalogItemsFromItemMetadataBlock(String sourceYaml, Managed if (itemType==CatalogItemType.TEMPLATE) { tags.add(BrooklynTags.CATALOG_TEMPLATE); + itemType = CatalogItemType.APPLICATION; + } + if (itemType==CatalogItemType.APPLICATION) { itemType = CatalogItemType.ENTITY; superTypes.add(Application.class); } @@ -1371,7 +1374,7 @@ private boolean attemptType(String key, CatalogItemType candidateCiType) { } catch (Exception e) { Exceptions.propagateIfFatal(e); // record the error if we have reason to expect this guess to succeed - if (item.containsKey("services") && (candidateCiType==CatalogItemType.ENTITY || candidateCiType==CatalogItemType.TEMPLATE)) { + if (item.containsKey("services") && (candidateCiType==CatalogItemType.ENTITY || candidateCiType==CatalogItemType.APPLICATION || candidateCiType==CatalogItemType.TEMPLATE)) { // explicit services supplied, so plan should have been parseable for an entity or a a service errors.add(e); } else if (catalogItemType!=null && key!=null) { @@ -2079,6 +2082,7 @@ public CatalogItem apply(@Nullable CatalogItemDo item) { dto.setSymbolicName(dto.getJavaType()); switch (dto.getCatalogItemType()) { case TEMPLATE: + case APPLICATION: case ENTITY: dto.setPlanYaml("services: [{ type: "+dto.getJavaType()+" }]"); break; diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogApplicationItemDto.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogApplicationItemDto.java new file mode 100644 index 0000000000..eabcd6d19d --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogApplicationItemDto.java @@ -0,0 +1,42 @@ +/* + * 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.catalog.internal; + +import org.apache.brooklyn.api.entity.Application; +import org.apache.brooklyn.api.entity.EntitySpec; + +public class CatalogApplicationItemDto extends CatalogItemDtoAbstract> { + + @Override + public CatalogItemType getCatalogItemType() { + return CatalogItemType.APPLICATION; + } + + @Override + public Class getCatalogItemJavaType() { + return Application.class; + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + @Override + public Class> getSpecType() { + return (Class)EntitySpec.class; + } + +} diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemBuilder.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemBuilder.java index f08ef17da2..28f9090233 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemBuilder.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemBuilder.java @@ -33,6 +33,7 @@ public static CatalogItemBuilder newItem(CatalogItemType itemType, String sym Preconditions.checkNotNull(itemType, "itemType required"); switch (itemType) { case ENTITY: return newEntity(symbolicName, version); + case APPLICATION: return newApplication(symbolicName, version); case TEMPLATE: return newTemplate(symbolicName, version); case POLICY: return newPolicy(symbolicName, version); case ENRICHER: return newEnricher(symbolicName, version); @@ -47,6 +48,12 @@ public static CatalogItemBuilder newEntity(String symbolic .version(version); } + public static CatalogItemBuilder newApplication(String symbolicName, String version) { + return new CatalogItemBuilder(new CatalogApplicationItemDto()) + .symbolicName(symbolicName) + .version(version); + } + public static CatalogItemBuilder newTemplate(String symbolicName, String version) { return new CatalogItemBuilder(new CatalogTemplateItemDto()) .symbolicName(symbolicName) diff --git a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java index cbcb94c304..ea47dff2d2 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java @@ -270,7 +270,10 @@ private BasicConfigKey(Class typeC, TypeToken typeT, String name, String d public BasicConfigKey(Builder builder) { this.name = checkNotNull(builder.name, "name"); - this.deprecatedNames = checkNotNull(builder.deprecatedNames, "deprecatedNames"); + + this.deprecatedNames = builder.deprecatedNames; + getDeprecatedNames(); // invoke method to tidy up for persistence + TypeTokens.checkCompatibleOneNonNull(builder.type, builder.typeToken); this.type = builder.type; this.typeToken = builder.typeToken; @@ -279,20 +282,28 @@ public BasicConfigKey(Builder builder) { this.reconfigurable = builder.reconfigurable; this.runtimeInheritance = builder.runtimeInheritance; this.typeInheritance = builder.typeInheritance; + // Note: it's intentionally possible to have default values that are not valid // per the configured constraint. If validity were checked here any class that // contained a weirdly-defined config key would fail to initialise. - this.constraint = checkNotNull(builder.constraint, "constraint"); + this.constraint = builder.constraint; + getConstraint(); // invoke method to tidy up for persistence } /** @see ConfigKey#getName() */ @Override public String getName() { return name; } /** @see ConfigKey#getDeprecatedNames() */ - @Override public Collection getDeprecatedNames() { - // check for null, for backwards compatibility of serialized state - if (deprecatedNames == null) deprecatedNames = ImmutableList.of(); - return deprecatedNames; + @Override public synchronized Collection getDeprecatedNames() { + // check for empty list, for backwards compatibility of serialized state; prefer "null" so we don't get noise in serialised state + if (deprecatedNames!=null) { + if (deprecatedNames.isEmpty()) { + deprecatedNames = null; + } else { + return deprecatedNames; + } + } + return ImmutableList.of(); } /** @see ConfigKey#getTypeName() */ @@ -374,13 +385,16 @@ public ConfigInheritance getParentInheritance() { /** @see ConfigKey#getConstraint() */ @Override @Nonnull - public Predicate getConstraint() { - // Could be null after rebinding + public synchronized Predicate getConstraint() { if (constraint != null) { - return constraint; - } else { - return Predicates.alwaysTrue(); + if (Predicates.alwaysTrue().equals(constraint)) { + // prefer null so persisted state is cleaner + constraint = null; + } else { + return constraint; + } } + return Predicates.alwaysTrue(); } /** @see ConfigKey#isValueValid(T) */ @@ -476,5 +490,33 @@ public BasicConfigKeyOverwriting(ConfigKey key, String newDescription, T defa public ConfigKey getParentKey() { return parentKey; } + + @Override + public boolean isSet(Map vals) { + if (parentKey instanceof ConfigKeySelfExtracting) { + return ((ConfigKeySelfExtracting)parentKey).isSet(vals); + } + return super.isSet(vals); + } + + @Override + public T extractValue(Map vals, ExecutionContext exec) { + // if we are unsure and want to log both, then + T v1, v2 = null; + if (parentKey instanceof ConfigKeySelfExtracting) { + v2 = ((ConfigKeySelfExtracting)parentKey).extractValue(vals, exec); + return v2; + } + v1 = super.extractValue(vals, exec); + // uncomment the "return v2" above, and these lines below, if we want to reenable reporting + // of cases where these values are different. based on spot-checks and thinking it through, + // i (alex) think v2 is always correct if applicable, but leaving this in for a while after 2018-11 + // in case it warrants further investigation. +// if (v2!=null && !Objects.equal(v1, v2)) { +// log.warn("Different values extracted for "+this+": "+v1+" ("+JavaClassNames.cleanSimpleClassName(v1)+") / "+v2+" ("+JavaClassNames.cleanSimpleClassName(v2)+")"); +// return v2; +// } + return v1; + } } } diff --git a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java index 9e68099179..ab4076cb56 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java @@ -44,6 +44,7 @@ import org.apache.brooklyn.core.config.ConfigKeys.InheritanceContext; import org.apache.brooklyn.core.config.Sanitizer; import org.apache.brooklyn.core.config.StructuredConfigKey; +import org.apache.brooklyn.core.config.BasicConfigKey.BasicConfigKeyOverwriting; import org.apache.brooklyn.core.objs.BrooklynObjectInternal; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; @@ -177,10 +178,11 @@ public Object setConfig(final ConfigKey key, Object v) { ConfigKey ownKey = getKeyAtContainer(getContainer(), key); if (ownKey==null) ownKey = key; - Object val = coerceConfigVal(ownKey, v); + Object val = coerceConfigValOnWrite(ownKey, v); Object oldVal; - if (ownKey instanceof StructuredConfigKey) { - oldVal = ((StructuredConfigKey)ownKey).applyValueToMap(val, ownConfig); + Maybe structured = getStructuredKeyMaybe(ownKey); + if (structured.isPresent()) { + oldVal = structured.get().applyValueToMap(val, ownConfig); } else { oldVal = ownConfig.put(ownKey, val); } @@ -248,17 +250,48 @@ public Maybe getConfigRaw(ConfigKey key, boolean includeInherited) { return getParentInternal().config().getInternalConfigMap().getConfigRaw(key, includeInherited); } - protected Object coerceConfigVal(ConfigKey key, Object v) { + // In general we coerce on write to detect errors fast, but there are a lot of exceptions + // where we aren't strict on write, such as deferred suppliers and deep config. + protected Object coerceConfigValOnWrite(ConfigKey key, Object v) { if ((v instanceof Future) || (v instanceof DeferredSupplier) || (v instanceof TaskFactory)) { // no coercion for these (coerce on exit) return v; - } else if (key instanceof StructuredConfigKey) { - // no coercion for these structures (they decide what to do) + } else if (getStructuredKeyMaybe(key).isPresent()) { + // no coercion for these structures (they decide what coercion to enforce when elements are set) return v; - } else if ((v instanceof Map || v instanceof Iterable) && key.getType().isInstance(v)) { - // don't do coercion on put for these, if the key type is compatible, - // because that will force resolution deeply + } else if (v instanceof Map && key.getType().isInstance(v)) { + // don't do coercion on put for maps, as we may have deferred suppliers internally + // (we could implement deep coercion which excludes futures but not realllly needed) return v; + } else if (v instanceof Iterable && Iterable.class.isAssignableFrom(key.getType())) { + // as for maps, but do handle set/list differences + // (prior to 2018-11 we applied the same check as for Map for Iterable, which meant + // setting a List in a ConfigKey did not come in to this block, but rather it + // fell through to the coercion block below; if the list had a Future in it, and + // were being asked to coerce to a Set, it would of course fail to coerce, + // and no attempt would be made to coerce to a Set; on read it will attempt to coerce + // the values to String but it would not coerce the List to a Set; it never happened arose + // as a problem IRL because we were good about using SetConfigKey, but the addition of tests of + // TestEntity.CONF_SET_STRING_PLAIN demonstrated the problem in existing code that was revealed + // by setting defaults and using BasicConfigKeyOverwriting; the new logic below corrects this problem + try { + // try to coerce on input, to detect errors sooner + return TypeCoercions.coerce(v, key.getTypeToken()); + } catch (Exception e) { + // could not coerce, something in the iterable is not coercible; + // for now however assume it is a future and proceed without flagging an error + if (key.getType().isInstance(v)) { + return v; + } + if (key.getType().isAssignableFrom(MutableList.class)) { + return MutableList.copyOf((Iterable)v).asUnmodifiable(); + } + if (key.getType().isAssignableFrom(MutableSet.class)) { + return MutableSet.copyOf((Iterable)v).asUnmodifiable(); + } + // it's a weird sort of collection-to-collection mapping, so just fail + throw new IllegalArgumentException("Cannot coerce or set iterable types, "+v+" to "+key, e); + } } else { try { // try to coerce on input, to detect errors sooner @@ -275,6 +308,15 @@ protected Object coerceConfigVal(ConfigKey key, Object v) { } } + private Maybe getStructuredKeyMaybe(ConfigKey key) { + // check if it is structured _or effectively so_ + if (key instanceof StructuredConfigKey) return Maybe.of((StructuredConfigKey)key); + if (key instanceof BasicConfigKeyOverwriting) { + return getStructuredKeyMaybe(((BasicConfigKeyOverwriting)key).getParentKey()); + } + return Maybe.absent(); + } + @Override public Map asMapWithStringKeys() { return mapViewWithStringKeys; diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java index fa7cecd251..493b838238 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java @@ -356,7 +356,8 @@ private static Map checkConstructorFlags(Map flags, Entity parent) { } /** - * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly + * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly, + * (and used for internal private configuration from entity specs which use flags to set keys) */ @Override @Deprecated diff --git a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java index 51e2e616c4..9e49aac0cc 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java @@ -89,7 +89,7 @@ public LocationConfigMap submap(Predicate> filter) { } @Override - protected Object coerceConfigVal(ConfigKey key, Object v) { + protected Object coerceConfigValOnWrite(ConfigKey key, Object v) { if ((Class.class.isAssignableFrom(key.getType()) || Function.class.isAssignableFrom(key.getType())) && v instanceof String) { // strings can be written where classes/functions are permitted; // this because an occasional pattern only for locations because validation wasn't enforced there @@ -98,7 +98,7 @@ protected Object coerceConfigVal(ConfigKey key, Object v) { return v; } - return super.coerceConfigVal(key, v); + return super.coerceConfigValOnWrite(key, v); } @Override diff --git a/core/src/main/java/org/apache/brooklyn/enricher/stock/aggregator/AggregationJob.java b/core/src/main/java/org/apache/brooklyn/enricher/stock/aggregator/AggregationJob.java index ed8c00677e..a7027ffba6 100644 --- a/core/src/main/java/org/apache/brooklyn/enricher/stock/aggregator/AggregationJob.java +++ b/core/src/main/java/org/apache/brooklyn/enricher/stock/aggregator/AggregationJob.java @@ -52,7 +52,7 @@ public final class AggregationJob implements Runnable { public static BasicAttributeSensorAndConfigKey>> DASHBOARD_POLICY_HIGHLIGHTS = new BasicAttributeSensorAndConfigKey(List.class, "dashboard.policyHighlights", - "Highlights from policies. List of Masps, where each map should contain text and category"); + "Highlights from policies. List of Maps, where each map should contain text and category"); private final Entity entity; diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindConfigInheritanceTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindConfigInheritanceTest.java index 97faeec5ef..ff7c4fcb0f 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindConfigInheritanceTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindConfigInheritanceTest.java @@ -33,6 +33,7 @@ import org.apache.brooklyn.core.entity.EntityAsserts; import org.apache.brooklyn.core.test.entity.TestEntity; import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.stream.Streams; import org.apache.brooklyn.util.text.Strings; @@ -52,7 +53,7 @@ public class RebindConfigInheritanceTest extends RebindTestFixtureWithApp { private static final Logger log = LoggerFactory.getLogger(RebindConfigInheritanceTest.class); ConfigKey key1 = ConfigKeys.builder(String.class, "key1").runtimeInheritance(BasicConfigInheritance.NEVER_INHERITED).build(); - String origMemento, newMemento; + String inMementoRaw, expectedOutMemento, newMemento; Application rebindedApp; @Override @@ -63,10 +64,20 @@ public void setUp() throws Exception { protected void doReadConfigInheritance(String label, String entityId) throws Exception { String mementoFilename = "config-inheritance-"+label+"-"+entityId; - origMemento = Streams.readFullyString(getClass().getResourceAsStream(mementoFilename)); + inMementoRaw = Streams.readFullyString(getClass().getResourceAsStream(mementoFilename)); File persistedEntityFile = new File(mementoDir, Os.mergePaths("entities", entityId)); - Files.write(origMemento.getBytes(), persistedEntityFile); + Files.write(inMementoRaw.getBytes(), persistedEntityFile); + try { + expectedOutMemento = Streams.readFullyString(getClass().getResourceAsStream(mementoFilename+"-out")); + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + expectedOutMemento = inMementoRaw; + } + if (expectedOutMemento.startsWith("")+3).trim(); + } + expectedOutMemento = Strings.replaceAllNonRegex(expectedOutMemento, "$VERSION", BrooklynVersion.get()); // we'll make assertions on what we've loaded rebind(); @@ -84,8 +95,8 @@ public void testPreBasicConfigInheritance_2016_07() throws Exception { ConfigKey k = Iterables.getOnlyElement( rebindedApp.config().findKeysDeclared(ConfigPredicates.nameEqualTo("my.config.inheritanceMerged")) ); - Asserts.assertStringContains(origMemento, ""); - Asserts.assertStringDoesNotContain(origMemento, BasicConfigInheritance.DEEP_MERGE.getClass().getName()); + Asserts.assertStringContains(inMementoRaw, ""); + Asserts.assertStringDoesNotContain(inMementoRaw, BasicConfigInheritance.DEEP_MERGE.getClass().getName()); // should now convert it to BasicConfigInheritance.DEEP_MERGE Asserts.assertStringDoesNotContain(newMemento, "ConfigInheritance$Merged"); @@ -102,9 +113,9 @@ public void testBasicConfigInheritance_2016_10() throws Exception { checkNewAppNonInheritingKey1(rebindedApp); - Asserts.assertStringContains(origMemento, "isReinherited"); - Asserts.assertStringDoesNotContain(origMemento, "NEVER_INHERITED"); - Asserts.assertStringDoesNotContain(origMemento, "NeverInherited"); + Asserts.assertStringContains(inMementoRaw, "isReinherited"); + Asserts.assertStringDoesNotContain(inMementoRaw, "NEVER_INHERITED"); + Asserts.assertStringDoesNotContain(inMementoRaw, "NeverInherited"); // should write it out as NeverInherited Asserts.assertStringDoesNotContain(newMemento, "isReinherited"); @@ -116,9 +127,7 @@ public void testReadConfigInheritance_2016_11() throws Exception { doReadConfigInheritance("basic-2016-11", "kmpez5fznt"); checkNewAppNonInheritingKey1(rebindedApp); - String origMementoTidied = origMemento.substring(origMemento.indexOf("")); - origMementoTidied = Strings.replaceAllNonRegex(origMementoTidied, "VERSION", BrooklynVersion.get()); - Asserts.assertEquals(origMementoTidied, newMemento.replaceAll("\n.*searchPath.*\n", "\n")); + Asserts.assertEquals(newMemento.replaceAll("\n.*searchPath.*\n", "\n"), expectedOutMemento); } @Test diff --git a/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java b/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java index 404d4f8205..065cbd488b 100644 --- a/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java +++ b/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java @@ -84,7 +84,7 @@ public > SpecT c if (item.getCatalogItemType()==CatalogItemType.ENTITY) { return (SpecT)toEntitySpec(parseXml(item.getPlanYaml()), 1); } - if (item.getCatalogItemType()==CatalogItemType.TEMPLATE) { + if (item.getCatalogItemType()==CatalogItemType.TEMPLATE || item.getCatalogItemType()==CatalogItemType.APPLICATION) { return (SpecT)toEntitySpec(parseXml(item.getPlanYaml()), 0); } throw new UnsupportedTypePlanException("Type "+item.getCatalogItemType()+" not supported"); diff --git a/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java b/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java index bfada48325..743d5f499f 100644 --- a/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java +++ b/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java @@ -67,6 +67,7 @@ public interface TestEntity extends Entity, Startable, EntityLocal, EntityIntern public static final BasicConfigKey CONF_MAP_PLAIN = new BasicConfigKey(Map.class, "test.confMapPlain", "Configuration key that's a plain map", ImmutableMap.of()); public static final BasicConfigKey CONF_LIST_PLAIN = new BasicConfigKey(List.class, "test.confListPlain", "Configuration key that's a plain list", ImmutableList.of()); public static final BasicConfigKey CONF_SET_PLAIN = new BasicConfigKey(Set.class, "test.confSetPlain", "Configuration key that's a plain set", ImmutableSet.of()); + public static final BasicConfigKey> CONF_SET_STRING_PLAIN = new BasicConfigKey>(new TypeToken>() {}, "test.confSetStringPlain", "Configuration key that's a plain set of strings", ImmutableSet.of()); public static final MapConfigKey CONF_MAP_THING = new MapConfigKey(String.class, "test.confMapThing", "Configuration key that's a map thing"); public static final MapConfigKey CONF_MAP_OBJ_THING = new MapConfigKey(Object.class, "test.confMapObjThing", "Configuration key that's a map thing with objects"); public static final ListConfigKey CONF_LIST_THING = new ListConfigKey(String.class, "test.confListThing", "Configuration key that's a list thing"); diff --git a/core/src/test/java/org/apache/brooklyn/util/core/osgi/OsgiTestBase.java b/core/src/test/java/org/apache/brooklyn/util/core/osgi/OsgiTestBase.java index 4bc1e583c3..0569ffb15b 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/osgi/OsgiTestBase.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/osgi/OsgiTestBase.java @@ -16,18 +16,19 @@ package org.apache.brooklyn.util.core.osgi; import java.io.File; -import java.io.IOException; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.test.support.TestResourceUnavailableException; import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.io.FileUtil; import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.osgi.OsgiTestResources; -import org.apache.commons.io.FileUtils; import org.osgi.framework.Bundle; import org.osgi.framework.BundleException; import org.osgi.framework.launch.Framework; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; @@ -40,7 +41,7 @@ public class OsgiTestBase { public static final String BROOKLYN_OSGI_TEST_A_0_1_0_PATH = OsgiTestResources.BROOKLYN_OSGI_TEST_A_0_1_0_PATH; public static final String BROOKLYN_OSGI_TEST_A_0_1_0_URL = "classpath:"+BROOKLYN_OSGI_TEST_A_0_1_0_PATH; - protected Bundle install(String url) throws BundleException { + protected Bundle install(String url) { try { return Osgis.install(framework, url); } catch (Exception e) { @@ -48,7 +49,7 @@ protected Bundle install(String url) throws BundleException { } } - protected Bundle installFromClasspath(String resourceName) throws BundleException { + protected Bundle installFromClasspath(String resourceName) { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), resourceName); try { return Osgis.install(framework, String.format("classpath:%s", resourceName)); @@ -67,17 +68,13 @@ public void setUp() throws Exception { } @AfterMethod(alwaysRun = true) - public void tearDown() throws BundleException, IOException, InterruptedException { + public void tearDown() { tearDownOsgiFramework(framework, storageTempDir); } - public static void tearDownOsgiFramework(Framework framework, File storageTempDir) throws BundleException, InterruptedException, IOException { + public static void tearDownOsgiFramework(Framework framework, File storageTempDir) { Osgis.ungetFramework(framework); - framework = null; - if (storageTempDir != null) { - FileUtils.deleteDirectory(storageTempDir); - storageTempDir = null; - } + FileUtil.deleteDirectory(storageTempDir); } public static void preinstallLibrariesLowLevelToPreventCatalogBomParsing(ManagementContext mgmt, String ...libraries) { diff --git a/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java b/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java index 3f99ee3ac2..311bce7d38 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java @@ -35,6 +35,7 @@ import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport; import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; +import org.apache.brooklyn.test.DisableOnWindows; import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.core.task.BasicExecutionContext; import org.apache.brooklyn.util.core.task.ssh.SshTasks; @@ -133,6 +134,7 @@ public void tearDown() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testRemoveRequireTtyFromSudoersFile() throws Exception { String cmds = BashCommands.dontRequireTtyForSudo(); @@ -154,6 +156,7 @@ public void testRemoveRequireTtyFromSudoersFile() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a whoami command available on localhost") public void testSudo() throws Exception { ByteArrayOutputStream outStream = new ByteArrayOutputStream(); ByteArrayOutputStream errStream = new ByteArrayOutputStream(); @@ -177,6 +180,7 @@ public void testDownloadUrl() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testDownloadFirstSuccessfulFile() throws Exception { List cmds = BashCommands.commandsToDownloadUrlsAs( ImmutableList.of(sourceNonExistantFileUrl, sourceFileUrl1, sourceFileUrl2), @@ -188,6 +192,7 @@ public void testDownloadFirstSuccessfulFile() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs an ssh server listening on port 22 on localhost") public void testDownloadToStdout() throws Exception { ProcessTaskWrapper t = SshTasks.newSshExecTaskFactory(loc, "cd "+destFile.getParentFile().getAbsolutePath(), @@ -199,6 +204,7 @@ public void testDownloadToStdout() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs an ssh server listening on port 22 on localhost") public void testAlternativesWhereFirstSucceeds() throws Exception { ProcessTaskWrapper t = SshTasks.newSshExecTaskFactory(loc) .add(BashCommands.alternatives(Arrays.asList("echo first", "exit 88"))) @@ -213,6 +219,7 @@ public void testAlternativesWhereFirstSucceeds() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs an ssh server listening on port 22 on localhost") public void testAlternatives() throws Exception { ProcessTaskWrapper t = SshTasks.newSshExecTaskFactory(loc) .add(BashCommands.alternatives(Arrays.asList("asdfj_no_such_command_1", "exit 88"))) @@ -224,6 +231,7 @@ public void testAlternatives() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs an ssh server listening on port 22 on localhost") public void testRequireTestHandlesFailure() throws Exception { ProcessTaskWrapper t = SshTasks.newSshExecTaskFactory(loc) .add(BashCommands.requireTest("-f "+sourceNonExistantFile.getPath(), @@ -236,6 +244,7 @@ public void testRequireTestHandlesFailure() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs an ssh server listening on port 22 on localhost") public void testRequireTestHandlesSuccess() throws Exception { ProcessTaskWrapper t = SshTasks.newSshExecTaskFactory(loc) .add(BashCommands.requireTest("-f "+sourceFile1.getPath(), @@ -247,6 +256,7 @@ public void testRequireTestHandlesSuccess() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs an ssh server listening on port 22 on localhost") public void testRequireFileHandlesFailure() throws Exception { ProcessTaskWrapper t = SshTasks.newSshExecTaskFactory(loc) .add(BashCommands.requireFile(sourceNonExistantFile.getPath())).newTask(); @@ -260,6 +270,7 @@ public void testRequireFileHandlesFailure() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs an ssh server listening on port 22 on localhost") public void testRequireFileHandlesSuccess() throws Exception { ProcessTaskWrapper t = SshTasks.newSshExecTaskFactory(loc) .add(BashCommands.requireFile(sourceFile1.getPath())).newTask(); @@ -270,6 +281,7 @@ public void testRequireFileHandlesSuccess() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs an ssh server listening on port 22 on localhost") public void testRequireFailureExitsImmediately() throws Exception { ProcessTaskWrapper t = SshTasks.newSshExecTaskFactory(loc) .add(BashCommands.requireTest("-f "+sourceNonExistantFile.getPath(), @@ -284,6 +296,7 @@ public void testRequireFailureExitsImmediately() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testPipeMultiline() throws Exception { String output = execRequiringZeroAndReturningStdout(loc, BashCommands.pipeTextTo("hello world\n"+"and goodbye\n", "wc")).get(); @@ -292,6 +305,7 @@ public void testPipeMultiline() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testWaitForFileContentsWhenAbortingOnFail() throws Exception { String fileContent = "mycontents"; String cmd = BashCommands.waitForFileContents(destFile.getAbsolutePath(), fileContent, Duration.ONE_SECOND, true); @@ -305,6 +319,7 @@ public void testWaitForFileContentsWhenAbortingOnFail() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testWaitForFileContentsWhenNotAbortingOnFail() throws Exception { String fileContent = "mycontents"; String cmd = BashCommands.waitForFileContents(destFile.getAbsolutePath(), fileContent, Duration.ONE_SECOND, false); @@ -318,6 +333,7 @@ public void testWaitForFileContentsWhenNotAbortingOnFail() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testWaitForFileContentsWhenContentsAppearAfterStart() throws Exception { String fileContent = "mycontents"; @@ -335,6 +351,7 @@ public void testWaitForFileContentsWhenContentsAppearAfterStart() throws Excepti } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testWaitForFileExistsWhenAbortingOnFail() throws Exception { String cmd = BashCommands.waitForFileExists(destFile.getAbsolutePath(), Duration.ONE_SECOND, true); @@ -347,6 +364,7 @@ public void testWaitForFileExistsWhenAbortingOnFail() throws Exception { } @Test(groups="Integration") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testWaitForFileExistsWhenNotAbortingOnFail() throws Exception { String cmd = BashCommands.waitForFileExists(destFile.getAbsolutePath(), Duration.ONE_SECOND, false); @@ -359,6 +377,7 @@ public void testWaitForFileExistsWhenNotAbortingOnFail() throws Exception { } @Test(groups="Integration", dependsOnMethods="testSudo") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testWaitForPortFreeWhenAbortingOnTimeout() throws Exception { ServerSocket serverSocket = openServerSocket(); try { @@ -378,6 +397,7 @@ public void testWaitForPortFreeWhenAbortingOnTimeout() throws Exception { } @Test(groups="Integration", dependsOnMethods="testSudo") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testWaitForPortFreeWhenNotAbortingOnTimeout() throws Exception { ServerSocket serverSocket = openServerSocket(); try { @@ -397,6 +417,7 @@ public void testWaitForPortFreeWhenNotAbortingOnTimeout() throws Exception { } @Test(groups="Integration", dependsOnMethods="testSudo") + @DisableOnWindows(reason = "Needs a bash shell available on localhost") public void testWaitForPortFreeWhenFreedAfterStart() throws Exception { ServerSocket serverSocket = openServerSocket(); try { diff --git a/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/config-inheritance-basic-2016-11-kmpez5fznt b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/config-inheritance-basic-2016-11-kmpez5fznt index 2dc8996482..cf635eae8c 100644 --- a/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/config-inheritance-basic-2016-11-kmpez5fznt +++ b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/config-inheritance-basic-2016-11-kmpez5fznt @@ -18,7 +18,7 @@ under the License. --> - VERSION + 0.10.0-SNAPSHOT org.apache.brooklyn.core.test.entity.TestApplicationNoEnrichersImpl kmpez5fznt TestApplicationNoEnrichersImpl:kmpe diff --git a/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/config-inheritance-basic-2016-11-kmpez5fznt-out b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/config-inheritance-basic-2016-11-kmpez5fznt-out new file mode 100644 index 0000000000..a1b177d1b4 --- /dev/null +++ b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/config-inheritance-basic-2016-11-kmpez5fznt-out @@ -0,0 +1,45 @@ + + + + $VERSION + org.apache.brooklyn.core.test.entity.TestApplicationNoEnrichersImpl + kmpez5fznt + TestApplicationNoEnrichersImpl:kmpe + + 1 + + + kmpez5fznt + kmpez5fznt + + + + + + + + key1 + java.lang.String + false + + + + + \ No newline at end of file diff --git a/parent/pom.xml b/parent/pom.xml index e1b3737d36..971c76ebaa 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -1110,6 +1110,7 @@ **/src/main/license/** **/src/test/license/** **/MANIFEST.MF + **/META-INF/services/** **/test-output/** **/*.pem.pub **/*.pem @@ -1385,6 +1386,7 @@ maven-surefire-plugin ${surefire.version} + false listener diff --git a/policy/src/main/java/org/apache/brooklyn/policy/failover/PropagatePrimaryEnricher.java b/policy/src/main/java/org/apache/brooklyn/policy/failover/PropagatePrimaryEnricher.java index ca2a99a1b4..6ecdca2409 100644 --- a/policy/src/main/java/org/apache/brooklyn/policy/failover/PropagatePrimaryEnricher.java +++ b/policy/src/main/java/org/apache/brooklyn/policy/failover/PropagatePrimaryEnricher.java @@ -90,6 +90,7 @@ public synchronized void onEvent(SensorEvent event) { Collection> sensorsToRemove = config().get(PROPAGATING); if (sensorsToRemove!=null) { for (Sensor s: sensorsToRemove) { + // TODO - allow strings above also if (s instanceof AttributeSensor) { ((EntityInternal)entity).sensors().remove((AttributeSensor)s); } @@ -119,7 +120,7 @@ protected void addPropagatorEnricher(Entity primary) { Collection> sensorsToPropagate = getConfig(PROPAGATING); if (sensorsToPropagate==null || sensorsToPropagate.isEmpty()) { - log.warn(""); + log.warn("Nothing found to propagate from primary "+primary+" at "+entity); return; } spec.configure(Propagator.PROPAGATING, sensorsToPropagate); diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java index 1b7fe35b25..abe783c271 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java @@ -53,6 +53,7 @@ import org.apache.brooklyn.core.config.ConfigPredicates; import org.apache.brooklyn.core.config.ConstraintViolationException; import org.apache.brooklyn.core.entity.Attributes; +import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.entity.EntityPredicates; import org.apache.brooklyn.core.entity.lifecycle.Lifecycle; import org.apache.brooklyn.core.entity.trait.Startable; @@ -335,9 +336,10 @@ private EntitySummary addConfigByGlobs(EntitySummary result, Entity entity, List extraConfigGlobs.stream().forEach(g -> { extraConfigPreds.add( ConfigPredicates.nameMatchesGlob(g) ); }); entity.config().findKeysDeclared(Predicates.or(extraConfigPreds)).forEach(key -> { if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_CONFIG, new EntityAndItem(entity, key.getName()))) { - Object v = entity.config().get(key); + Maybe vRaw = ((EntityInternal)entity).config().getRaw(key); + Object v = vRaw.isPresent() ? vRaw.get() : entity.config().get(key); if (v!=null) { - v = resolving(v).preferJson(true).asJerseyOutermostReturnValue(false).raw(true).context(entity).immediately(true).timeout(Duration.ZERO).resolve(); + v = resolving(v, mgmt()).preferJson(true).asJerseyOutermostReturnValue(false).raw(true).context(entity).immediately(true).timeout(Duration.ZERO).resolve(); configs.put(key.getName(), v); } } diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java index 3684a3eb4e..d559a44e0c 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java @@ -294,6 +294,7 @@ public static CatalogItemSummary catalogItemSummary(BrooklynRestResourceUtils b, try { switch (item.getCatalogItemType()) { case TEMPLATE: + case APPLICATION: case ENTITY: return catalogEntitySummary(b, item, ub); case POLICY: @@ -369,6 +370,7 @@ private static URI getSelfLink(CatalogItem item, UriBuilder ub) { String itemId = item.getId(); switch (item.getCatalogItemType()) { case TEMPLATE: + case APPLICATION: return serviceUriBuilder(ub, CatalogApi.class, "getApplication").build(itemId, item.getVersion()); case ENTITY: return serviceUriBuilder(ub, CatalogApi.class, "getEntity").build(itemId, item.getVersion()); diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/TypeTransformer.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/TypeTransformer.java index e8bfb5098b..2796acdf3c 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/TypeTransformer.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/TypeTransformer.java @@ -23,6 +23,7 @@ import java.net.URI; import java.util.Collections; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -87,7 +88,11 @@ private static T embellish(T result, RegisteredType item result.setExtraField("template", true); } if (item.getIconUrl()!=null) { - result.setIconUrl(tidyIconLink(b, item, item.getIconUrl(), ub)); + String tidiedUrl = tidyIconLink(b, item, item.getIconUrl(), ub); + result.setIconUrl(tidiedUrl); + if (!Objects.equals(item.getIconUrl(), tidiedUrl)) { + result.setExtraField("iconUrlSource", item.getIconUrl()); + } } if (detail) { diff --git a/server-cli/src/main/java/org/apache/brooklyn/cli/ItemLister.java b/server-cli/src/main/java/org/apache/brooklyn/cli/ItemLister.java index 6e31e5327c..4c22a6c60e 100644 --- a/server-cli/src/main/java/org/apache/brooklyn/cli/ItemLister.java +++ b/server-cli/src/main/java/org/apache/brooklyn/cli/ItemLister.java @@ -239,7 +239,7 @@ protected Map populateDescriptors() throws MalformedURLException Map itemDescriptor = ItemDescriptors.toItemDescriptor(catalog, item, headingsOnly); itemCount++; - if (item.getCatalogItemType() == CatalogItem.CatalogItemType.ENTITY || item.getCatalogItemType() == CatalogItem.CatalogItemType.TEMPLATE) { + if (item.getCatalogItemType() == CatalogItem.CatalogItemType.ENTITY || item.getCatalogItemType() == CatalogItem.CatalogItemType.TEMPLATE || item.getCatalogItemType() == CatalogItem.CatalogItemType.APPLICATION) { entities.add(itemDescriptor); } else if (item.getCatalogItemType() == CatalogItem.CatalogItemType.POLICY) { policies.add(itemDescriptor); diff --git a/test-support/src/main/java/org/apache/brooklyn/test/DisableOnWindows.java b/test-support/src/main/java/org/apache/brooklyn/test/DisableOnWindows.java new file mode 100644 index 0000000000..9fa2998cdd --- /dev/null +++ b/test-support/src/main/java/org/apache/brooklyn/test/DisableOnWindows.java @@ -0,0 +1,40 @@ +/* + * 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.test; + +import java.lang.annotation.*; + +/** + * Used to indicate that a test should ne be executed on Windows. + * + *

You must add the following to the class where this annotation is being used: + * {@code @Listeners(DisableOnWindows)}

+ */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +@Documented +public @interface DisableOnWindows { + + /**3 + * Explain the reason for the test being disabled. + * + *

e.g. "Needs an ssh server listening on port 22 on localhost."

+ */ + String reason(); +} diff --git a/test-support/src/main/java/org/apache/brooklyn/test/DisableOnWindowsListener.java b/test-support/src/main/java/org/apache/brooklyn/test/DisableOnWindowsListener.java new file mode 100644 index 0000000000..743edde9bd --- /dev/null +++ b/test-support/src/main/java/org/apache/brooklyn/test/DisableOnWindowsListener.java @@ -0,0 +1,57 @@ +/* + * 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.test; + +import org.apache.brooklyn.util.os.Os; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.IAnnotationTransformer; +import org.testng.annotations.ITestAnnotation; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; + +/** + * Scans all tests annotated with {@link DisableOnWindows} and, on Windows, sets {@code enabled=false} if the current + * environment is Windows.. + */ +public class DisableOnWindowsListener implements IAnnotationTransformer { + + private static final Logger LOG = LoggerFactory.getLogger(DisableOnWindowsListener.class); + + @Override + public void transform( + final ITestAnnotation annotation, + final Class testClass, + final Constructor testConstructor, + final Method testMethod + ) { + if (testMethod != null ) { + final DisableOnWindows disableOnWindows = testMethod.getAnnotation(DisableOnWindows.class); + if (disableOnWindows != null && Os.isMicrosoftWindows()) { + annotation.setEnabled(false); + LOG.info(String.format("Disabled: %s.%s - %s", + testMethod.getDeclaringClass().getName(), + testMethod.getName(), + disableOnWindows.reason())); + } + } + } + +} diff --git a/test-support/src/main/resources/META-INF/services/org.testng.ITestNGListener b/test-support/src/main/resources/META-INF/services/org.testng.ITestNGListener new file mode 100644 index 0000000000..584861a390 --- /dev/null +++ b/test-support/src/main/resources/META-INF/services/org.testng.ITestNGListener @@ -0,0 +1 @@ +org.apache.brooklyn.test.DisableOnWindowsListener \ No newline at end of file diff --git a/test-support/src/test/java/org/apache/brooklyn/test/DisableOnWindowsTest.java b/test-support/src/test/java/org/apache/brooklyn/test/DisableOnWindowsTest.java new file mode 100644 index 0000000000..9929f666ef --- /dev/null +++ b/test-support/src/test/java/org/apache/brooklyn/test/DisableOnWindowsTest.java @@ -0,0 +1,41 @@ +/* + * 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.test; + +import org.apache.brooklyn.util.os.Os; +import org.testng.annotations.Test; + +import static org.apache.brooklyn.test.Asserts.assertTrue; +import static org.apache.brooklyn.test.Asserts.fail; + +public class DisableOnWindowsTest { + + @Test + public void alwaysRun() { + assertTrue(true); + } + + @Test + @DisableOnWindows(reason = "unit test") + public void isDisabledOnWindows() { + if (Os.isMicrosoftWindows()) { + fail("Test should have been disabled on windows"); + } + } +} diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java b/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java index a4908f2498..176ad821a5 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java @@ -184,4 +184,21 @@ private static boolean createNewFile(File file) throws IOException { } return file.createNewFile(); } + + /** + * Recursively delete a directory. + * + *

Any IOException that might be raised is logged and not thrown.

+ * + * @param file The directory to be deleted + */ + public static void deleteDirectory(final File file) { + if (file != null) { + try { + FileUtils.deleteDirectory(file); + } catch (IOException e) { + LOG.warn(e.getMessage()); + } + } + } } diff --git a/utils/rt-felix/src/test/java/org/apache/brooklyn/rt/felix/EmbeddedFelixFrameworkTest.java b/utils/rt-felix/src/test/java/org/apache/brooklyn/rt/felix/EmbeddedFelixFrameworkTest.java index 11dfe2777c..0c837cd8b4 100644 --- a/utils/rt-felix/src/test/java/org/apache/brooklyn/rt/felix/EmbeddedFelixFrameworkTest.java +++ b/utils/rt-felix/src/test/java/org/apache/brooklyn/rt/felix/EmbeddedFelixFrameworkTest.java @@ -16,7 +16,6 @@ package org.apache.brooklyn.rt.felix; import java.io.File; -import java.io.IOException; import java.io.InputStream; import java.net.URL; import java.util.Enumeration; @@ -25,11 +24,10 @@ import org.apache.brooklyn.test.support.TestResourceUnavailableException; import org.apache.brooklyn.util.collections.MutableSet; +import org.apache.brooklyn.util.io.FileUtil; import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.osgi.OsgiTestResources; import org.apache.brooklyn.util.stream.Streams; -import org.apache.commons.io.FileUtils; -import org.osgi.framework.BundleException; import org.osgi.framework.launch.Framework; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,17 +56,13 @@ public void setUp() throws Exception { } @AfterMethod(alwaysRun = true) - public void tearDown() throws BundleException, IOException, InterruptedException { + public void tearDown() { tearDownOsgiFramework(framework, storageTempDir); } - public static void tearDownOsgiFramework(Framework framework, File storageTempDir) throws BundleException, InterruptedException, IOException { + private static void tearDownOsgiFramework(Framework framework, File storageTempDir) { EmbeddedFelixFramework.stopFramework(framework); - framework = null; - if (storageTempDir != null) { - FileUtils.deleteDirectory(storageTempDir); - storageTempDir = null; - } + FileUtil.deleteDirectory(storageTempDir); } @Test