Repository: brooklyn-server Updated Branches: refs/heads/master aa43a5feb -> 9c9331c1b
Fix rebind EntitySpec with policies ref Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/59800ab5 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/59800ab5 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/59800ab5 Branch: refs/heads/master Commit: 59800ab51f8dbe09fcff6ea3eee86f48effdd0a9 Parents: aa43a5f Author: Aled Sage <aled.s...@gmail.com> Authored: Mon Nov 6 13:11:20 2017 +0000 Committer: Aled Sage <aled.s...@gmail.com> Committed: Mon Nov 6 14:56:45 2017 +0000 ---------------------------------------------------------------------- .../apache/brooklyn/api/entity/EntitySpec.java | 25 +++- .../mgmt/rebind/AbstractRebindHistoricTest.java | 11 +- .../rebind/RebindHistoricEntitySpecTest.java | 134 +++++++++++++++++++ .../core/mgmt/rebind/enricher-abcdefghij | 25 ++++ ...ityspec-containing-empty-policies-wj5s8u9h73 | 49 +++++++ ...th-entityspec-containing-policies-aeifj99fjd | 53 ++++++++ .../brooklyn/core/mgmt/rebind/policy-awmsgjxp8i | 25 ++++ 7 files changed, 319 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/59800ab5/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java b/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java index 7e11387..43df6fe 100644 --- a/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java +++ b/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java @@ -29,9 +29,13 @@ import javax.annotation.Nullable; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationSpec; +import org.apache.brooklyn.api.policy.Policy; import org.apache.brooklyn.api.policy.PolicySpec; +import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.api.sensor.EnricherSpec; import org.apache.brooklyn.util.collections.MutableList; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.Function; import com.google.common.base.Throwables; @@ -55,6 +59,8 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E private static final long serialVersionUID = -2247153452919128990L; + private static final Logger LOG = LoggerFactory.getLogger(EntitySpec.class); + /** * Creates a new {@link EntitySpec} instance for an entity of the given type. The returned * {@link EntitySpec} can then be customized. @@ -112,12 +118,29 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E private final List<Entity> members = Lists.newArrayList(); private final List<Group> groups = Lists.newArrayList(); private volatile boolean immutable; - + + // Kept for backwards compatibility of persisted state + private List<Policy> policies; + private List<Enricher> enrichers; + public EntitySpec(Class<T> type) { super(type); } @Override + protected Object readResolve() { + if (policies != null && policies.size() > 0) { + LOG.warn("NOT SUPPORTED: EntitySpec "+this+" has hard-coded policies, rather than use of PolicySpec - policies will be ignored ("+policies+")"); + policies = null; + } + if (enrichers != null && enrichers.size() > 0) { + LOG.warn("NOT SUPPORTED: EntitySpec "+this+" has hard-coded enrichers, rather than use of EnricherSpec - enrichers will be ignored ("+enrichers+")"); + enrichers = null; + } + return super.readResolve(); + } + + @Override protected EntitySpec<T> copyFrom(EntitySpec<T> otherSpec) { super.copyFrom(otherSpec) .additionalInterfaces(otherSpec.getAdditionalInterfaces()) http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/59800ab5/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/AbstractRebindHistoricTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/AbstractRebindHistoricTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/AbstractRebindHistoricTest.java index 1a0face..9df6357 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/AbstractRebindHistoricTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/AbstractRebindHistoricTest.java @@ -18,7 +18,9 @@ */ package org.apache.brooklyn.core.mgmt.rebind; -import com.google.common.io.Files; +import java.io.File; +import java.io.FileInputStream; + import org.apache.brooklyn.api.mgmt.rebind.RebindExceptionHandler; import org.apache.brooklyn.api.mgmt.rebind.RebindManager; import org.apache.brooklyn.api.objs.BrooklynObjectType; @@ -26,7 +28,7 @@ import org.apache.brooklyn.core.test.entity.TestApplication; import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.stream.Streams; -import java.io.File; +import com.google.common.io.Files; public abstract class AbstractRebindHistoricTest extends RebindTestFixtureWithApp { @@ -50,6 +52,11 @@ public abstract class AbstractRebindHistoricTest extends RebindTestFixtureWithAp Files.write(memento.getBytes(), persistedFile); } + protected String readMemento(BrooklynObjectType type, String id) throws Exception { + File persistedFile = getPersistanceFile(type, id); + return Streams.readFullyStringAndClose(new FileInputStream(persistedFile)); + } + protected File getPersistanceFile(BrooklynObjectType type, String id) { String dir; switch (type) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/59800ab5/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindHistoricEntitySpecTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindHistoricEntitySpecTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindHistoricEntitySpecTest.java new file mode 100644 index 0000000..7a11cde --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindHistoricEntitySpecTest.java @@ -0,0 +1,134 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.mgmt.rebind; + +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import org.apache.brooklyn.api.entity.EntitySpec; +import org.apache.brooklyn.api.mgmt.rebind.RebindExceptionHandler; +import org.apache.brooklyn.api.mgmt.rebind.RebindManager; +import org.apache.brooklyn.api.mgmt.rebind.mementos.EntityMemento; +import org.apache.brooklyn.api.objs.BrooklynObjectType; +import org.apache.brooklyn.core.test.entity.TestApplication; +import org.apache.brooklyn.core.test.entity.TestApplicationNoEnrichersImpl; +import org.apache.brooklyn.entity.stock.BasicEntity; +import org.apache.brooklyn.test.LogWatcher; +import org.apache.brooklyn.test.LogWatcher.EventPredicates; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.common.base.Optional; +import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; + +import ch.qos.logback.classic.spi.ILoggingEvent; + +public class RebindHistoricEntitySpecTest extends AbstractRebindHistoricTest { + @Override + @BeforeMethod(alwaysRun=true) + public void setUp() throws Exception { + super.setUp(); + } + + /** + * In https://github.com/apache/brooklyn-server/pull/859, we deleted EntitySpec.policies + * and EntitySpec.enrichers fields. Unfortunately that broke backwards compatibility: + * persisted state that included these fields would no longer rebind. + * + * This included for an empty list of policies, i.e. {@code <policies/>}. + * + * An alternative fix would have been to for XmlMementoSerializer.SpecConverter to have + * omitted those fields. However, it would then have been very hard to warn if we come + * across a non-empty list of policies or enrichers. + */ + @Test + public void testEntitySpecWithEmptyPoliciesList() throws Exception { + addMemento(BrooklynObjectType.ENTITY, "entity-with-entityspec-containing-empty-policies", "wj5s8u9h73"); + rebind(); + } + + /** + * EntitySpec includes a hard-coded reference to a policy and to an enricher. + * + * Ensure we log.warn if there are policies/enrichers. We discard them, as this is + * no longer supported. It would have been dangerous to add them: the entity spec would + * have had a hard-coded policy id, so if the spec was used to create more than one entity + * (e.g. in a cluster's memberSpec) then they would have shared the same policy instance, + * which would have gone badly wrong (e.g. calling policy.setEntity multiple times). + * + * Also this causes a rebind problem - dangling reference to the policy - because rebind does + * {@link RebindIteration#instantiateMementos()} (to instantiate the {@link EntityMemento} etc) + * before it does {@link RebindIteration#instantiateAdjuncts(org.apache.brooklyn.core.mgmt.rebind.RebindIteration.BrooklynObjectInstantiator)}. + * Therefore the rebindContext does not yet know about that policy instance. + * Note that the policies of an entity are referenced via {@link EntityMemento#getPolicies()}, + * so we did not try to resolve them during instantiation of the EntityMemento. + * In contrast, the EntitySpec's reference is for a field of type {@code List<Policy>} so needs to + * be resolved immediately (and because we don't use DynamicProxies for policies, we haven't + * created a place-holder). + * + * An alternative impl would be to fail to instantiate the EntitySpec (thus failing rebind). + * But given rebind would have failed previously (with the dangling-policy-ref described + * above), then I think it is fine to just discard these! + */ + @Test + public void testEntitySpecWithPolicyInstance() throws Exception { + // Need to ignore dangling-policy-ref (see javadoc above) + RebindExceptionHandler exceptionHandler = RebindExceptionHandlerImpl.builder() + .danglingRefFailureMode(RebindManager.RebindFailureMode.CONTINUE) + .rebindFailureMode(RebindManager.RebindFailureMode.FAIL_AT_END) + .addConfigFailureMode(RebindManager.RebindFailureMode.FAIL_AT_END) + .addPolicyFailureMode(RebindManager.RebindFailureMode.FAIL_AT_END) + .loadPolicyFailureMode(RebindManager.RebindFailureMode.FAIL_AT_END) + .build(); + + addMemento(BrooklynObjectType.ENRICHER, "enricher", "abcdefghij"); + addMemento(BrooklynObjectType.POLICY, "policy", "awmsgjxp8i"); + addMemento(BrooklynObjectType.ENTITY, "entity-with-entityspec-containing-policies", "aeifj99fjd"); + + Predicate<ILoggingEvent> filter = EventPredicates.containsMessage("NOT SUPPORTED"); + LogWatcher watcher = new LogWatcher(EntitySpec.class.getName(), ch.qos.logback.classic.Level.WARN, filter); + watcher.start(); + try { + rebind(RebindOptions.create().exceptionHandler(exceptionHandler)); + watcher.assertHasEventEventually(); + + Optional<ILoggingEvent> policiesWarning = Iterables.tryFind(watcher.getEvents(), EventPredicates.containsMessage("policies will be ignored")); + Optional<ILoggingEvent> enrichersWarning = Iterables.tryFind(watcher.getEvents(), EventPredicates.containsMessage("enrichers will be ignored")); + assertTrue(policiesWarning.isPresent()); + assertTrue(enrichersWarning.isPresent()); + } finally { + watcher.close(); + } + } + + // Check that a normal EntitySpec does not include "policies" or "enrichers" in its persisted state + @Test + public void testVanillaEntitySpec() throws Exception { + TestApplication app = mgmt().getEntityManager().createEntity(EntitySpec.create(TestApplication.class).impl(TestApplicationNoEnrichersImpl.class) + .configure("myEntitySpec", EntitySpec.create(BasicEntity.class))); + + RebindTestUtils.waitForPersisted(app); + String memento = readMemento(BrooklynObjectType.ENTITY, app.getId()); + assertFalse(memento.contains("<policies")); + assertFalse(memento.contains("<enrichers")); + + rebind(); + } +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/59800ab5/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/enricher-abcdefghij ---------------------------------------------------------------------- diff --git a/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/enricher-abcdefghij b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/enricher-abcdefghij new file mode 100644 index 0000000..1c465bc --- /dev/null +++ b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/enricher-abcdefghij @@ -0,0 +1,25 @@ +<!-- +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. +--> + +<enricher> + <brooklynVersion>0.11.0-SNAPSHOT</brooklynVersion> + <type>org.apache.brooklyn.core.test.policy.TestEnricher</type> + <id>abcdefghij</id> + <displayName>My Enricher</displayName> +</enricher> http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/59800ab5/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/entity-with-entityspec-containing-empty-policies-wj5s8u9h73 ---------------------------------------------------------------------- diff --git a/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/entity-with-entityspec-containing-empty-policies-wj5s8u9h73 b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/entity-with-entityspec-containing-empty-policies-wj5s8u9h73 new file mode 100644 index 0000000..f810d27 --- /dev/null +++ b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/entity-with-entityspec-containing-empty-policies-wj5s8u9h73 @@ -0,0 +1,49 @@ +<!-- +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. +--> + +<entity> + <brooklynVersion>1.0.0-SNAPSHOT</brooklynVersion> + <type>org.apache.brooklyn.core.test.entity.TestApplicationNoEnrichersImpl</type> + <id>wj5s8u9h73</id> + <displayName>TestApplicationNoEnrichersImpl:wj5s</displayName> + <config> + <myEntitySpec> + <org.apache.brooklyn.api:org.apache.brooklyn.api.entity.EntitySpec> + <type>org.apache.brooklyn.entity.stock.BasicEntity</type> + <catalogItemIdSearchPath class="MutableSet"/> + <tags class="MutableSet"/> + <parameters class="ImmutableList"/> + <flags/> + <config/> + <policySpecs/> + <enricherSpecs/> + <locations/> + <locationSpecs/> + <additionalInterfaces/> + <entityInitializers/> + <children/> + <members/> + <groups/> + <immutable>false</immutable> + <policies/> + <enrichers/> + </org.apache.brooklyn.api:org.apache.brooklyn.api.entity.EntitySpec> + </myEntitySpec> + </config> +</entity> http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/59800ab5/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/entity-with-entityspec-containing-policies-aeifj99fjd ---------------------------------------------------------------------- diff --git a/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/entity-with-entityspec-containing-policies-aeifj99fjd b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/entity-with-entityspec-containing-policies-aeifj99fjd new file mode 100644 index 0000000..4dbd1ef --- /dev/null +++ b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/entity-with-entityspec-containing-policies-aeifj99fjd @@ -0,0 +1,53 @@ +<!-- +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. +--> + +<entity> + <brooklynVersion>1.0.0-SNAPSHOT</brooklynVersion> + <type>org.apache.brooklyn.core.test.entity.TestApplicationNoEnrichersImpl</type> + <id>wj5s8u9h73</id> + <displayName>TestApplicationNoEnrichersImpl:wj5s</displayName> + <config> + <myEntitySpec> + <org.apache.brooklyn.api:org.apache.brooklyn.api.entity.EntitySpec> + <type>org.apache.brooklyn.entity.stock.BasicEntity</type> + <catalogItemIdSearchPath class="MutableSet"/> + <tags class="MutableSet"/> + <parameters class="ImmutableList"/> + <flags/> + <config/> + <policySpecs/> + <enricherSpecs/> + <locations/> + <locationSpecs/> + <additionalInterfaces/> + <entityInitializers/> + <children/> + <members/> + <groups/> + <immutable>false</immutable> + <policies> + <org.apache.brooklyn.core.test.policy.TestPolicy>awmsgjxp8i</org.apache.brooklyn.core.test.policy.TestPolicy> + </policies> + <enrichers> + <org.apache.brooklyn.core.test.policy.TestEnricher>abcdefghij</org.apache.brooklyn.core.test.policy.TestEnricher> + </enrichers> + </org.apache.brooklyn.api:org.apache.brooklyn.api.entity.EntitySpec> + </myEntitySpec> + </config> +</entity> http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/59800ab5/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/policy-awmsgjxp8i ---------------------------------------------------------------------- diff --git a/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/policy-awmsgjxp8i b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/policy-awmsgjxp8i new file mode 100644 index 0000000..5792b6d --- /dev/null +++ b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/policy-awmsgjxp8i @@ -0,0 +1,25 @@ +<!-- +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. +--> + +<policy> + <brooklynVersion>0.11.0-SNAPSHOT</brooklynVersion> + <type>org.apache.brooklyn.core.test.policy.TestPolicy</type> + <id>awmsgjxp8i</id> + <displayName>My Policy</displayName> +</policy>
