From 29b60c293294635d08077a802fe3aa8bbecb2724 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Fri, 23 Mar 2018 15:21:06 -0400 Subject: [PATCH] INT-4438: No lifecycle twice in the same role JIRA: https://jira.spring.io/browse/INT-4438 The `SmartLifecycleRoleController` is based on the `MultiValueMap` which used internally a `List` for the values. With such an architecture we can add the same value several times. On the other hand we are iterating over `Lifecycle`s in the role and build a `Map` for their running status. In this case when `NamesComponent`s return the same name the Java `Collectors.toMap()` fails with a duplicate key error. In any cases it would be better do not allow to add the same lifecylce several time to the role or different with the same name. * Add search logic to the `addLifecycleToRole()` to fail fast with the `IllegalArgumentException` because a lifecycle with the same name is already present in the role **Cherry-pick to 5.0.x** * Remove redundant `this.initialized = false` from the `AbstractPollingEndpoint.doStop()` Add `allEndpointsRunning()` verification to the `EndpointRoleParserTests` Polishing --- .../endpoint/AbstractPollingEndpoint.java | 3 +- .../support/SmartLifecycleRoleController.java | 29 +++++++- .../config/xml/EndpointRoleParserTests.java | 6 +- .../SmartLifecycleRoleControllerTests.java | 74 ++++++++++++++++++- 4 files changed, 104 insertions(+), 8 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java index 593b007776..c4319616cb 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -239,7 +239,6 @@ public abstract class AbstractPollingEndpoint extends AbstractEndpoint implement this.runningTask.cancel(true); } this.runningTask = null; - this.initialized = false; } private boolean doPoll() { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/SmartLifecycleRoleController.java b/spring-integration-core/src/main/java/org/springframework/integration/support/SmartLifecycleRoleController.java index 6903cca591..a7cdcfe270 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/SmartLifecycleRoleController.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/SmartLifecycleRoleController.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2017 the original author or authors. + * Copyright 2015-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,6 +43,7 @@ import org.springframework.integration.leader.event.OnGrantedEvent; import org.springframework.integration.leader.event.OnRevokedEvent; import org.springframework.integration.support.context.NamedComponent; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -101,7 +102,31 @@ public class SmartLifecycleRoleController implements ApplicationListener lifecycles = this.lifecycles.get(role); + if (CollectionUtils.isEmpty(lifecycles)) { + this.lifecycles.add(role, lifecycle); + } + else { + lifecycles + .stream() + .filter(e -> + e == lifecycle || + (e instanceof NamedComponent && lifecycle instanceof NamedComponent + && ((NamedComponent) e).getComponentName() + .equals(((NamedComponent) lifecycle).getComponentName()))) + .findFirst() + .ifPresent(e -> { + throw new IllegalArgumentException("Cannot add the Lifecycle '" + + (lifecycle instanceof NamedComponent + ? ((NamedComponent) lifecycle).getComponentName() + : lifecycle) + + "' to the role '" + role + "' because a Lifecycle with the name '" + + (e instanceof NamedComponent ? ((NamedComponent) e).getComponentName() : e) + + "' is already present."); + }); + + lifecycles.add(lifecycle); + } } /** diff --git a/spring-integration-core/src/test/java/org/springframework/integration/config/xml/EndpointRoleParserTests.java b/spring-integration-core/src/test/java/org/springframework/integration/config/xml/EndpointRoleParserTests.java index 0521c95466..6698a821da 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/config/xml/EndpointRoleParserTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/config/xml/EndpointRoleParserTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2015-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,6 +33,8 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; /** * @author Gary Russell + * @author Artem Bilan + * * @since 4.2 * */ @@ -105,6 +107,8 @@ public class EndpointRoleParserTests { assertFalse(this.out3.isRunning()); assertFalse(this.out4.isRunning()); assertFalse(this.bridge.isRunning()); + + assertFalse(this.controller.allEndpointsRunning("cluster")); } public static class Sink { diff --git a/spring-integration-core/src/test/java/org/springframework/integration/support/SmartLifecycleRoleControllerTests.java b/spring-integration-core/src/test/java/org/springframework/integration/support/SmartLifecycleRoleControllerTests.java index bcf4c34861..b365ebf250 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/support/SmartLifecycleRoleControllerTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/support/SmartLifecycleRoleControllerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2015-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.integration.support; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -24,11 +25,14 @@ import org.junit.Test; import org.mockito.InOrder; import org.springframework.context.SmartLifecycle; +import org.springframework.integration.support.context.NamedComponent; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; /** * @author Gary Russell + * @author Artem Bilan + * * @since 4.2 * */ @@ -39,8 +43,8 @@ public class SmartLifecycleRoleControllerTests { SmartLifecycle lc1 = mock(SmartLifecycle.class); when(lc1.getPhase()).thenReturn(2); SmartLifecycle lc2 = mock(SmartLifecycle.class); - when(lc1.getPhase()).thenReturn(1); - MultiValueMap map = new LinkedMultiValueMap(); + when(lc2.getPhase()).thenReturn(1); + MultiValueMap map = new LinkedMultiValueMap<>(); map.add("foo", lc1); map.add("foo", lc2); SmartLifecycleRoleController controller = new SmartLifecycleRoleController(map); @@ -53,4 +57,68 @@ public class SmartLifecycleRoleControllerTests { inOrder.verify(lc2).stop(); } + + @Test + public void allEndpointRunning() { + SmartLifecycle lc1 = new PseudoSmartLifecycle(); + SmartLifecycle lc2 = new PseudoSmartLifecycle(); + MultiValueMap map = new LinkedMultiValueMap<>(); + map.add("foo", lc1); + map.add("foo", lc2); + try { + new SmartLifecycleRoleController(map); + } + catch (Exception e) { + assertThat(e.getMessage()) + .contains("Cannot add the Lifecycle 'bar' to the role 'foo' " + + "because a Lifecycle with the name 'bar' is already present."); + } + } + + private static class PseudoSmartLifecycle implements SmartLifecycle, NamedComponent { + + boolean running; + + @Override + public boolean isAutoStartup() { + return false; + } + + @Override + public void stop(Runnable callback) { + running = false; + } + + @Override + public void start() { + running = true; + } + + @Override + public void stop() { + running = false; + } + + @Override + public boolean isRunning() { + return running; + } + + @Override + public int getPhase() { + return 0; + } + + @Override + public String getComponentName() { + return "bar"; + } + + @Override + public String getComponentType() { + return PseudoSmartLifecycle.class.getName(); + } + + } + }