Skip to content

Commit

Permalink
INT-4438: No lifecycle twice in the same role
Browse files Browse the repository at this point in the history
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
  • Loading branch information
artembilan authored and garyrussell committed Apr 2, 2018
1 parent 9ce329f commit 29b60c2
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -239,7 +239,6 @@ protected void doStop() {
this.runningTask.cancel(true);
}
this.runningTask = null;
this.initialized = false;
}

private boolean doPoll() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -43,6 +43,7 @@
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;

Expand Down Expand Up @@ -101,7 +102,31 @@ public void setApplicationContext(ApplicationContext applicationContext) throws
* @param lifecycle the {@link SmartLifecycle}.
*/
public final void addLifecycleToRole(String role, SmartLifecycle lifecycle) {
this.lifecycles.add(role, lifecycle);
List<SmartLifecycle> 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);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -33,6 +33,8 @@

/**
* @author Gary Russell
* @author Artem Bilan
*
* @since 4.2
*
*/
Expand Down Expand Up @@ -105,6 +107,8 @@ public void test() {
assertFalse(this.out3.isRunning());
assertFalse(this.out4.isRunning());
assertFalse(this.bridge.isRunning());

assertFalse(this.controller.allEndpointsRunning("cluster"));
}

public static class Sink {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand All @@ -24,11 +25,14 @@
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
*
*/
Expand All @@ -39,8 +43,8 @@ public void testOrder() {
SmartLifecycle lc1 = mock(SmartLifecycle.class);
when(lc1.getPhase()).thenReturn(2);
SmartLifecycle lc2 = mock(SmartLifecycle.class);
when(lc1.getPhase()).thenReturn(1);
MultiValueMap<String, SmartLifecycle> map = new LinkedMultiValueMap<String, SmartLifecycle>();
when(lc2.getPhase()).thenReturn(1);
MultiValueMap<String, SmartLifecycle> map = new LinkedMultiValueMap<>();
map.add("foo", lc1);
map.add("foo", lc2);
SmartLifecycleRoleController controller = new SmartLifecycleRoleController(map);
Expand All @@ -53,4 +57,68 @@ public void testOrder() {
inOrder.verify(lc2).stop();
}


@Test
public void allEndpointRunning() {
SmartLifecycle lc1 = new PseudoSmartLifecycle();
SmartLifecycle lc2 = new PseudoSmartLifecycle();
MultiValueMap<String, SmartLifecycle> 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();
}

}

}

0 comments on commit 29b60c2

Please sign in to comment.