From df6674c2ec228cc7c72412982df844bb080ef7bd Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Thu, 23 Aug 2018 14:42:52 -0400 Subject: [PATCH] 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. --- .../SimpleActiveIdleMessageSourceAdvice.java | 20 +- .../util/DynamicPeriodicTrigger.java | 183 ++++++++++++++---- .../endpoint/PollerAdviceTests.java | 2 +- 3 files changed, 161 insertions(+), 44 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/aop/SimpleActiveIdleMessageSourceAdvice.java b/spring-integration-core/src/main/java/org/springframework/integration/aop/SimpleActiveIdleMessageSourceAdvice.java index b5ec20846a..82322ab71e 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/aop/SimpleActiveIdleMessageSourceAdvice.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/aop/SimpleActiveIdleMessageSourceAdvice.java @@ -16,6 +16,8 @@ package org.springframework.integration.aop; +import java.time.Duration; + import org.springframework.integration.core.MessageSource; import org.springframework.integration.util.DynamicPeriodicTrigger; import org.springframework.messaging.Message; @@ -25,22 +27,24 @@ import org.springframework.messaging.Message; * there are no messages. * * @author Gary Russell + * * @since 4.2 + * * @see DynamicPeriodicTrigger */ public class SimpleActiveIdleMessageSourceAdvice extends AbstractMessageSourceAdvice { private final DynamicPeriodicTrigger trigger; - private volatile long idlePollPeriod; + private volatile Duration idlePollPeriod; - private volatile long activePollPeriod; + private volatile Duration activePollPeriod; public SimpleActiveIdleMessageSourceAdvice(DynamicPeriodicTrigger trigger) { this.trigger = trigger; - this.idlePollPeriod = trigger.getPeriod(); - this.activePollPeriod = trigger.getPeriod(); + this.idlePollPeriod = trigger.getDuration(); + this.activePollPeriod = trigger.getDuration(); } /** @@ -49,7 +53,7 @@ public class SimpleActiveIdleMessageSourceAdvice extends AbstractMessageSourceAd * @param idlePollPeriod the period in milliseconds. */ public void setIdlePollPeriod(long idlePollPeriod) { - this.idlePollPeriod = idlePollPeriod; + this.idlePollPeriod = Duration.ofMillis(idlePollPeriod); } /** @@ -58,16 +62,16 @@ public class SimpleActiveIdleMessageSourceAdvice extends AbstractMessageSourceAd * @param activePollPeriod the period in milliseconds. */ public void setActivePollPeriod(long activePollPeriod) { - this.activePollPeriod = activePollPeriod; + this.activePollPeriod = Duration.ofMillis(activePollPeriod); } @Override public Message afterReceive(Message result, MessageSource source) { if (result == null) { - this.trigger.setPeriod(this.idlePollPeriod); + this.trigger.setDuration(this.idlePollPeriod); } else { - this.trigger.setPeriod(this.activePollPeriod); + this.trigger.setDuration(this.activePollPeriod); } return result; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/util/DynamicPeriodicTrigger.java b/spring-integration-core/src/main/java/org/springframework/integration/util/DynamicPeriodicTrigger.java index bf8cf145ee..93d87d8e46 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/util/DynamicPeriodicTrigger.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/util/DynamicPeriodicTrigger.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 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. @@ -16,6 +16,7 @@ package org.springframework.integration.util; +import java.time.Duration; import java.util.Date; import java.util.concurrent.TimeUnit; @@ -29,18 +30,21 @@ import org.springframework.util.Assert; * This is a dynamically changeable {@link Trigger}. It is based on the * {@link PeriodicTrigger} implementations. However, the fields of this dynamic * trigger are not final and the properties can be inspected and set via - * explicit getters and setters. + * explicit getters and setters. Changes to the trigger take effect after the + * next execution. * * @author Gunnar Hillert + * @author Gary Russell + * * @since 4.2 */ public class DynamicPeriodicTrigger implements Trigger { - private volatile long period; + private volatile Duration initialDuration = Duration.ofMillis(0); - private volatile TimeUnit timeUnit; + private volatile Duration duration; - private volatile long initialDelay = 0; + private volatile TimeUnit timeUnit = TimeUnit.MILLISECONDS; private volatile boolean fixedRate = false; @@ -50,7 +54,7 @@ public class DynamicPeriodicTrigger implements Trigger { * @param period Must not be negative */ public DynamicPeriodicTrigger(long period) { - this(period, TimeUnit.MILLISECONDS); + this(Duration.ofMillis(period)); } /** @@ -59,13 +63,26 @@ public class DynamicPeriodicTrigger implements Trigger { * configured on this Trigger later via {@link #setInitialDelay(long)}. * @param period Must not be negative * @param timeUnit Must not be null + * @deprecated in favor of {@link #DynamicPeriodicTrigger(Duration)}. */ + @Deprecated public DynamicPeriodicTrigger(long period, TimeUnit timeUnit) { Assert.isTrue(period >= 0, "period must not be negative"); Assert.notNull(timeUnit, "timeUnit must not be null"); this.timeUnit = timeUnit; - this.period = this.timeUnit.toMillis(period); + this.duration = Duration.ofMillis(this.timeUnit.toMillis(period)); + } + + /** + * Create a trigger with the provided duration. + * @param duration the duration. + * @since 5.1 + */ + public DynamicPeriodicTrigger(Duration duration) { + Assert.notNull(duration, "duration must not be null"); + Assert.isTrue(!duration.isNegative(), "duration must not be negative"); + this.duration = duration; } /** @@ -73,10 +90,52 @@ public class DynamicPeriodicTrigger implements Trigger { * terms of this trigger's {@link TimeUnit}. If no time unit was explicitly * provided upon instantiation, the default is milliseconds. * @param initialDelay the initial delay in milliseconds. + * @deprecated in favor of {@link #setInitialDuration(Duration)}. */ + @Deprecated public void setInitialDelay(long initialDelay) { Assert.isTrue(initialDelay >= 0, "initialDelay must not be negative"); - this.initialDelay = this.timeUnit.toMillis(initialDelay); + this.initialDuration = Duration.ofMillis(this.timeUnit.toMillis(initialDelay)); + } + + /** + * Specify the delay for the initial execution. It will be evaluated in + * terms of this trigger's {@link TimeUnit}. If no time unit was explicitly + * provided upon instantiation, the default is milliseconds. + * @param initialDuration the initial delay in milliseconds. + * @since 5.1 + */ + public void setInitialDuration(Duration initialDuration) { + Assert.notNull(initialDuration, "initialDuration must not be null"); + Assert.isTrue(!initialDuration.isNegative(), "initialDuration must not be negative"); + this.initialDuration = initialDuration; + } + + /** + * Return the duration. + * @return the duration. + * @since 5.1 + */ + public Duration getDuration() { + return this.duration; + } + + /** + * Set the duration. + * @param duration the duration. + * @since 5.1 + */ + public void setDuration(Duration duration) { + this.duration = duration; + } + + /** + * Get the initial duration. + * @return the initial duration. + * @since 5.1 + */ + public Duration getInitialDuration() { + return this.initialDuration; } /** @@ -90,23 +149,13 @@ public class DynamicPeriodicTrigger implements Trigger { } /** - * Return the time after which a task should run again. - * @param triggerContext the trigger context to determine the previous state of schedule. - * @return the the next schedule date. + * Return the period in milliseconds. + * @return the period. + * @deprecated in favor of {@link #getDuration()}. */ - @Override - public Date nextExecutionTime(TriggerContext triggerContext) { - if (triggerContext.lastScheduledExecutionTime() == null) { - return new Date(System.currentTimeMillis() + this.initialDelay); - } - else if (this.fixedRate) { - return new Date(triggerContext.lastScheduledExecutionTime().getTime() + this.period); - } - return new Date(triggerContext.lastCompletionTime().getTime() + this.period); - } - + @Deprecated public long getPeriod() { - return this.period; + return this.duration.toMillis(); } /** @@ -114,48 +163,112 @@ public class DynamicPeriodicTrigger implements Trigger { * terms of this trigger's {@link TimeUnit}. If no time unit was explicitly * provided upon instantiation, the default is milliseconds. * @param period Must not be negative + * @deprecated in favor of {@link #setDuration(Duration)}. */ + @Deprecated public void setPeriod(long period) { Assert.isTrue(period >= 0, "period must not be negative"); - this.period = this.timeUnit.toMillis(period); + this.duration = Duration.ofMillis(this.timeUnit.toMillis(period)); } + /** + * Get the time unit. + * @return the time unit. + * @deprecated - use {@link Duration} instead. + */ + @Deprecated public TimeUnit getTimeUnit() { return this.timeUnit; } + /** + * Set the time unit. + * @param timeUnit the time unit. + * @deprecated - use {@link Duration} instead. + */ + @Deprecated public void setTimeUnit(TimeUnit timeUnit) { Assert.notNull(timeUnit, "timeUnit must not be null"); this.timeUnit = timeUnit; } + /** + * Get the initial delay in milliseconds. + * @return the initial delay. + * @deprecated in favor of {@link #getInitialDuration()}. + */ + @Deprecated public long getInitialDelay() { - return this.initialDelay; + return this.initialDuration.toMillis(); } + /** + * Return whether this trigger is fixed rate. + * @return the fixed rate. + */ public boolean isFixedRate() { return this.fixedRate; } + /** + * Return the time after which a task should run again. + * @param triggerContext the trigger context to determine the previous state of schedule. + * @return the the next schedule date. + */ + @Override + public Date nextExecutionTime(TriggerContext triggerContext) { + if (triggerContext.lastScheduledExecutionTime() == null) { + return new Date(System.currentTimeMillis() + this.initialDuration.toMillis()); + } + else if (this.fixedRate) { + return new Date(triggerContext.lastScheduledExecutionTime().getTime() + this.duration.toMillis()); + } + return new Date(triggerContext.lastCompletionTime().getTime() + this.duration.toMillis()); + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((this.duration == null) ? 0 : this.duration.hashCode()); + result = prime * result + (this.fixedRate ? 1231 : 1237); + result = prime * result + ((this.initialDuration == null) ? 0 : this.initialDuration.hashCode()); + return result; + } + @Override public boolean equals(Object obj) { if (this == obj) { return true; } - if (!(obj instanceof DynamicPeriodicTrigger)) { + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { return false; } DynamicPeriodicTrigger other = (DynamicPeriodicTrigger) obj; - return this.fixedRate == other.fixedRate - && this.initialDelay == other.initialDelay - && this.period == other.period; + if (this.duration == null) { + if (other.duration != null) { + return false; + } + } + else if (!this.duration.equals(other.duration)) { + return false; + } + if (this.fixedRate != other.fixedRate) { + return false; + } + if (this.initialDuration == null) { + if (other.initialDuration != null) { + return false; + } + } + else if (!this.initialDuration.equals(other.initialDuration)) { + return false; + } + return true; } - @Override - public int hashCode() { - return (this.fixedRate ? 14 : 41) + - (int) (38 * this.period) + - (int) (43 * this.initialDelay); - } } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java b/spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java index cbaa30e1d9..2a33423bfe 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java @@ -258,7 +258,7 @@ public class PollerAdviceTests { final DynamicPeriodicTrigger trigger = new DynamicPeriodicTrigger(10); adapter.setSource(() -> { synchronized (triggerPeriods) { - triggerPeriods.add(trigger.getPeriod()); + triggerPeriods.add(trigger.getDuration().toMillis()); } Message m = null; if (latch.getCount() % 2 == 0) {