Skip to content

Commit df6674c

Browse files
garyrussellartembilan
authored andcommitted
Fix DynamicPeriodicTrigger
- inconsistent use of internal `TimeUnit` - `setPeriod()` applied the unit, but `getPeriod()` did not This test failed... ```java @test public void testTriggerConsistency() { final DynamicPeriodicTrigger trigger = new DynamicPeriodicTrigger(10, TimeUnit.SECONDS); trigger.setPeriod(trigger.getPeriod()); assertThat(trigger.getPeriod(), equalTo(10_000)); } ``` Change the trigger to use `Duration` and deprecate the other methods.
1 parent aa371f5 commit df6674c

File tree

3 files changed

+161
-44
lines changed

3 files changed

+161
-44
lines changed

spring-integration-core/src/main/java/org/springframework/integration/aop/SimpleActiveIdleMessageSourceAdvice.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.integration.aop;
1818

19+
import java.time.Duration;
20+
1921
import org.springframework.integration.core.MessageSource;
2022
import org.springframework.integration.util.DynamicPeriodicTrigger;
2123
import org.springframework.messaging.Message;
@@ -25,22 +27,24 @@
2527
* there are no messages.
2628
*
2729
* @author Gary Russell
30+
*
2831
* @since 4.2
32+
*
2933
* @see DynamicPeriodicTrigger
3034
*/
3135
public class SimpleActiveIdleMessageSourceAdvice extends AbstractMessageSourceAdvice {
3236

3337
private final DynamicPeriodicTrigger trigger;
3438

35-
private volatile long idlePollPeriod;
39+
private volatile Duration idlePollPeriod;
3640

37-
private volatile long activePollPeriod;
41+
private volatile Duration activePollPeriod;
3842

3943

4044
public SimpleActiveIdleMessageSourceAdvice(DynamicPeriodicTrigger trigger) {
4145
this.trigger = trigger;
42-
this.idlePollPeriod = trigger.getPeriod();
43-
this.activePollPeriod = trigger.getPeriod();
46+
this.idlePollPeriod = trigger.getDuration();
47+
this.activePollPeriod = trigger.getDuration();
4448
}
4549

4650
/**
@@ -49,7 +53,7 @@ public SimpleActiveIdleMessageSourceAdvice(DynamicPeriodicTrigger trigger) {
4953
* @param idlePollPeriod the period in milliseconds.
5054
*/
5155
public void setIdlePollPeriod(long idlePollPeriod) {
52-
this.idlePollPeriod = idlePollPeriod;
56+
this.idlePollPeriod = Duration.ofMillis(idlePollPeriod);
5357
}
5458

5559
/**
@@ -58,16 +62,16 @@ public void setIdlePollPeriod(long idlePollPeriod) {
5862
* @param activePollPeriod the period in milliseconds.
5963
*/
6064
public void setActivePollPeriod(long activePollPeriod) {
61-
this.activePollPeriod = activePollPeriod;
65+
this.activePollPeriod = Duration.ofMillis(activePollPeriod);
6266
}
6367

6468
@Override
6569
public Message<?> afterReceive(Message<?> result, MessageSource<?> source) {
6670
if (result == null) {
67-
this.trigger.setPeriod(this.idlePollPeriod);
71+
this.trigger.setDuration(this.idlePollPeriod);
6872
}
6973
else {
70-
this.trigger.setPeriod(this.activePollPeriod);
74+
this.trigger.setDuration(this.activePollPeriod);
7175
}
7276
return result;
7377
}
Lines changed: 148 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.integration.util;
1818

19+
import java.time.Duration;
1920
import java.util.Date;
2021
import java.util.concurrent.TimeUnit;
2122

@@ -29,18 +30,21 @@
2930
* This is a dynamically changeable {@link Trigger}. It is based on the
3031
* {@link PeriodicTrigger} implementations. However, the fields of this dynamic
3132
* trigger are not final and the properties can be inspected and set via
32-
* explicit getters and setters.
33+
* explicit getters and setters. Changes to the trigger take effect after the
34+
* next execution.
3335
*
3436
* @author Gunnar Hillert
37+
* @author Gary Russell
38+
*
3539
* @since 4.2
3640
*/
3741
public class DynamicPeriodicTrigger implements Trigger {
3842

39-
private volatile long period;
43+
private volatile Duration initialDuration = Duration.ofMillis(0);
4044

41-
private volatile TimeUnit timeUnit;
45+
private volatile Duration duration;
4246

43-
private volatile long initialDelay = 0;
47+
private volatile TimeUnit timeUnit = TimeUnit.MILLISECONDS;
4448

4549
private volatile boolean fixedRate = false;
4650

@@ -50,7 +54,7 @@ public class DynamicPeriodicTrigger implements Trigger {
5054
* @param period Must not be negative
5155
*/
5256
public DynamicPeriodicTrigger(long period) {
53-
this(period, TimeUnit.MILLISECONDS);
57+
this(Duration.ofMillis(period));
5458
}
5559

5660
/**
@@ -59,24 +63,79 @@ public DynamicPeriodicTrigger(long period) {
5963
* configured on this Trigger later via {@link #setInitialDelay(long)}.
6064
* @param period Must not be negative
6165
* @param timeUnit Must not be null
66+
* @deprecated in favor of {@link #DynamicPeriodicTrigger(Duration)}.
6267
*/
68+
@Deprecated
6369
public DynamicPeriodicTrigger(long period, TimeUnit timeUnit) {
6470
Assert.isTrue(period >= 0, "period must not be negative");
6571
Assert.notNull(timeUnit, "timeUnit must not be null");
6672

6773
this.timeUnit = timeUnit;
68-
this.period = this.timeUnit.toMillis(period);
74+
this.duration = Duration.ofMillis(this.timeUnit.toMillis(period));
75+
}
76+
77+
/**
78+
* Create a trigger with the provided duration.
79+
* @param duration the duration.
80+
* @since 5.1
81+
*/
82+
public DynamicPeriodicTrigger(Duration duration) {
83+
Assert.notNull(duration, "duration must not be null");
84+
Assert.isTrue(!duration.isNegative(), "duration must not be negative");
85+
this.duration = duration;
6986
}
7087

7188
/**
7289
* Specify the delay for the initial execution. It will be evaluated in
7390
* terms of this trigger's {@link TimeUnit}. If no time unit was explicitly
7491
* provided upon instantiation, the default is milliseconds.
7592
* @param initialDelay the initial delay in milliseconds.
93+
* @deprecated in favor of {@link #setInitialDuration(Duration)}.
7694
*/
95+
@Deprecated
7796
public void setInitialDelay(long initialDelay) {
7897
Assert.isTrue(initialDelay >= 0, "initialDelay must not be negative");
79-
this.initialDelay = this.timeUnit.toMillis(initialDelay);
98+
this.initialDuration = Duration.ofMillis(this.timeUnit.toMillis(initialDelay));
99+
}
100+
101+
/**
102+
* Specify the delay for the initial execution. It will be evaluated in
103+
* terms of this trigger's {@link TimeUnit}. If no time unit was explicitly
104+
* provided upon instantiation, the default is milliseconds.
105+
* @param initialDuration the initial delay in milliseconds.
106+
* @since 5.1
107+
*/
108+
public void setInitialDuration(Duration initialDuration) {
109+
Assert.notNull(initialDuration, "initialDuration must not be null");
110+
Assert.isTrue(!initialDuration.isNegative(), "initialDuration must not be negative");
111+
this.initialDuration = initialDuration;
112+
}
113+
114+
/**
115+
* Return the duration.
116+
* @return the duration.
117+
* @since 5.1
118+
*/
119+
public Duration getDuration() {
120+
return this.duration;
121+
}
122+
123+
/**
124+
* Set the duration.
125+
* @param duration the duration.
126+
* @since 5.1
127+
*/
128+
public void setDuration(Duration duration) {
129+
this.duration = duration;
130+
}
131+
132+
/**
133+
* Get the initial duration.
134+
* @return the initial duration.
135+
* @since 5.1
136+
*/
137+
public Duration getInitialDuration() {
138+
return this.initialDuration;
80139
}
81140

82141
/**
@@ -90,72 +149,126 @@ public void setFixedRate(boolean fixedRate) {
90149
}
91150

92151
/**
93-
* Return the time after which a task should run again.
94-
* @param triggerContext the trigger context to determine the previous state of schedule.
95-
* @return the the next schedule date.
152+
* Return the period in milliseconds.
153+
* @return the period.
154+
* @deprecated in favor of {@link #getDuration()}.
96155
*/
97-
@Override
98-
public Date nextExecutionTime(TriggerContext triggerContext) {
99-
if (triggerContext.lastScheduledExecutionTime() == null) {
100-
return new Date(System.currentTimeMillis() + this.initialDelay);
101-
}
102-
else if (this.fixedRate) {
103-
return new Date(triggerContext.lastScheduledExecutionTime().getTime() + this.period);
104-
}
105-
return new Date(triggerContext.lastCompletionTime().getTime() + this.period);
106-
}
107-
156+
@Deprecated
108157
public long getPeriod() {
109-
return this.period;
158+
return this.duration.toMillis();
110159
}
111160

112161
/**
113162
* Specify the period of the trigger. It will be evaluated in
114163
* terms of this trigger's {@link TimeUnit}. If no time unit was explicitly
115164
* provided upon instantiation, the default is milliseconds.
116165
* @param period Must not be negative
166+
* @deprecated in favor of {@link #setDuration(Duration)}.
117167
*/
168+
@Deprecated
118169
public void setPeriod(long period) {
119170
Assert.isTrue(period >= 0, "period must not be negative");
120-
this.period = this.timeUnit.toMillis(period);
171+
this.duration = Duration.ofMillis(this.timeUnit.toMillis(period));
121172
}
122173

174+
/**
175+
* Get the time unit.
176+
* @return the time unit.
177+
* @deprecated - use {@link Duration} instead.
178+
*/
179+
@Deprecated
123180
public TimeUnit getTimeUnit() {
124181
return this.timeUnit;
125182
}
126183

184+
/**
185+
* Set the time unit.
186+
* @param timeUnit the time unit.
187+
* @deprecated - use {@link Duration} instead.
188+
*/
189+
@Deprecated
127190
public void setTimeUnit(TimeUnit timeUnit) {
128191
Assert.notNull(timeUnit, "timeUnit must not be null");
129192
this.timeUnit = timeUnit;
130193
}
131194

195+
/**
196+
* Get the initial delay in milliseconds.
197+
* @return the initial delay.
198+
* @deprecated in favor of {@link #getInitialDuration()}.
199+
*/
200+
@Deprecated
132201
public long getInitialDelay() {
133-
return this.initialDelay;
202+
return this.initialDuration.toMillis();
134203
}
135204

205+
/**
206+
* Return whether this trigger is fixed rate.
207+
* @return the fixed rate.
208+
*/
136209
public boolean isFixedRate() {
137210
return this.fixedRate;
138211
}
139212

213+
/**
214+
* Return the time after which a task should run again.
215+
* @param triggerContext the trigger context to determine the previous state of schedule.
216+
* @return the the next schedule date.
217+
*/
218+
@Override
219+
public Date nextExecutionTime(TriggerContext triggerContext) {
220+
if (triggerContext.lastScheduledExecutionTime() == null) {
221+
return new Date(System.currentTimeMillis() + this.initialDuration.toMillis());
222+
}
223+
else if (this.fixedRate) {
224+
return new Date(triggerContext.lastScheduledExecutionTime().getTime() + this.duration.toMillis());
225+
}
226+
return new Date(triggerContext.lastCompletionTime().getTime() + this.duration.toMillis());
227+
}
228+
229+
@Override
230+
public int hashCode() {
231+
final int prime = 31;
232+
int result = 1;
233+
result = prime * result + ((this.duration == null) ? 0 : this.duration.hashCode());
234+
result = prime * result + (this.fixedRate ? 1231 : 1237);
235+
result = prime * result + ((this.initialDuration == null) ? 0 : this.initialDuration.hashCode());
236+
return result;
237+
}
238+
140239
@Override
141240
public boolean equals(Object obj) {
142241
if (this == obj) {
143242
return true;
144243
}
145-
if (!(obj instanceof DynamicPeriodicTrigger)) {
244+
if (obj == null) {
245+
return false;
246+
}
247+
if (getClass() != obj.getClass()) {
146248
return false;
147249
}
148250
DynamicPeriodicTrigger other = (DynamicPeriodicTrigger) obj;
149-
return this.fixedRate == other.fixedRate
150-
&& this.initialDelay == other.initialDelay
151-
&& this.period == other.period;
251+
if (this.duration == null) {
252+
if (other.duration != null) {
253+
return false;
254+
}
255+
}
256+
else if (!this.duration.equals(other.duration)) {
257+
return false;
258+
}
259+
if (this.fixedRate != other.fixedRate) {
260+
return false;
261+
}
262+
if (this.initialDuration == null) {
263+
if (other.initialDuration != null) {
264+
return false;
265+
}
266+
}
267+
else if (!this.initialDuration.equals(other.initialDuration)) {
268+
return false;
269+
}
270+
return true;
152271
}
153272

154-
@Override
155-
public int hashCode() {
156-
return (this.fixedRate ? 14 : 41) +
157-
(int) (38 * this.period) +
158-
(int) (43 * this.initialDelay);
159-
}
160273

161274
}

spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ public void testActiveIdleAdvice() throws Exception {
258258
final DynamicPeriodicTrigger trigger = new DynamicPeriodicTrigger(10);
259259
adapter.setSource(() -> {
260260
synchronized (triggerPeriods) {
261-
triggerPeriods.add(trigger.getPeriod());
261+
triggerPeriods.add(trigger.getDuration().toMillis());
262262
}
263263
Message<Object> m = null;
264264
if (latch.getCount() % 2 == 0) {

0 commit comments

Comments
 (0)