Skip to content

Commit 0c20bc4

Browse files
committed
ref(spring): Use MonitorConfigUtils in SentryCheckInAdvice
Keep only the Spring-specific part in the three advices (finding @scheduled, reading its attributes, resolving placeholders) and move the conversion edge-case tests to core.
1 parent 055f4fb commit 0c20bc4

6 files changed

Lines changed: 48 additions & 456 deletions

File tree

‎sentry-spring-7/src/main/java/io/sentry/spring7/checkin/SentryCheckInAdvice.java‎

Lines changed: 16 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,15 @@
77
import io.sentry.IScopes;
88
import io.sentry.ISentryLifecycleToken;
99
import io.sentry.MonitorConfig;
10-
import io.sentry.MonitorSchedule;
11-
import io.sentry.MonitorScheduleUnit;
1210
import io.sentry.ScopesAdapter;
1311
import io.sentry.SentryLevel;
1412
import io.sentry.protocol.SentryId;
1513
import io.sentry.time.Stopwatch;
14+
import io.sentry.util.MonitorConfigUtils;
1615
import io.sentry.util.Objects;
1716
import io.sentry.util.TracingUtils;
1817
import java.lang.reflect.Method;
1918
import java.time.Duration;
20-
import java.util.HashMap;
21-
import java.util.Locale;
22-
import java.util.Map;
2319
import java.util.Set;
2420
import java.util.concurrent.TimeUnit;
2521
import org.aopalliance.intercept.MethodInterceptor;
@@ -33,7 +29,6 @@
3329
import org.springframework.core.annotation.AnnotationUtils;
3430
import org.springframework.scheduling.annotation.Scheduled;
3531
import org.springframework.util.ObjectUtils;
36-
import org.springframework.util.StringUtils;
3732
import org.springframework.util.StringValueResolver;
3833

3934
/**
@@ -43,18 +38,6 @@
4338
@ApiStatus.Internal
4439
@Open
4540
public class SentryCheckInAdvice implements MethodInterceptor, EmbeddedValueResolverAware {
46-
private static final @NotNull Map<String, String> CRON_MACROS = new HashMap<>();
47-
48-
static {
49-
CRON_MACROS.put("@yearly", "0 0 1 1 *");
50-
CRON_MACROS.put("@annually", "0 0 1 1 *");
51-
CRON_MACROS.put("@monthly", "0 0 1 * *");
52-
CRON_MACROS.put("@weekly", "0 0 * * 0");
53-
CRON_MACROS.put("@daily", "0 0 * * *");
54-
CRON_MACROS.put("@midnight", "0 0 * * *");
55-
CRON_MACROS.put("@hourly", "0 * * * *");
56-
}
57-
5841
private final @NotNull IScopes scopes;
5942

6043
private @Nullable StringValueResolver resolver;
@@ -149,40 +132,21 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
149132
return null;
150133
}
151134
final @NotNull Scheduled scheduled = schedules.iterator().next();
135+
// timeUnit only exists from Spring 5.3.10, so read it as an attribute
136+
final @Nullable Object timeUnitAttribute =
137+
AnnotationUtils.getAnnotationAttributes(scheduled).get("timeUnit");
138+
final @NotNull TimeUnit timeUnit =
139+
timeUnitAttribute instanceof TimeUnit
140+
? (TimeUnit) timeUnitAttribute
141+
: TimeUnit.MILLISECONDS;
152142
final @Nullable String cron = resolve(scheduled.cron());
153-
final @Nullable Long fixedRate =
154-
periodMillis(scheduled, scheduled.fixedRate(), scheduled.fixedRateString());
155-
final @Nullable Long fixedDelay =
156-
periodMillis(scheduled, scheduled.fixedDelay(), scheduled.fixedDelayString());
157-
final boolean hasCron = cron != null && !cron.isEmpty();
158-
159-
if (hasCron && cron != null && fixedRate == null && fixedDelay == null) {
160-
final @Nullable String crontab = toCrontab(cron);
161-
if (crontab == null) {
162-
return null;
163-
}
164-
final @NotNull MonitorConfig config = new MonitorConfig(MonitorSchedule.crontab(crontab));
165-
final @Nullable String zone = resolve(scheduled.zone());
166-
if (zone != null && !zone.isEmpty()) {
167-
config.setTimezone(zone);
168-
}
169-
return config;
170-
}
171-
final @Nullable Long period = fixedRate != null ? fixedRate : fixedDelay;
172-
if (!hasCron && period != null && (fixedRate == null || fixedDelay == null)) {
173-
final long millis = period;
174-
final long minuteMillis = TimeUnit.MINUTES.toMillis(1);
175-
if (millis < minuteMillis || millis % minuteMillis != 0) {
176-
return null;
177-
}
178-
final long minutes = millis / minuteMillis;
179-
if (minutes > Integer.MAX_VALUE) {
180-
return null;
181-
}
182-
return new MonitorConfig(
183-
MonitorSchedule.interval((int) minutes, MonitorScheduleUnit.MINUTE));
184-
}
185-
return null;
143+
final @Nullable String zone =
144+
cron != null && !cron.isEmpty() ? resolve(scheduled.zone()) : null;
145+
return MonitorConfigUtils.fromSchedule(
146+
cron,
147+
zone,
148+
periodMillis(scheduled.fixedRate(), scheduled.fixedRateString(), timeUnit),
149+
periodMillis(scheduled.fixedDelay(), scheduled.fixedDelayString(), timeUnit));
186150
} catch (RuntimeException e) {
187151
scopes
188152
.getOptions()
@@ -196,14 +160,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
196160
}
197161

198162
private @Nullable Long periodMillis(
199-
final @NotNull Scheduled scheduled, final long value, final @NotNull String valueString) {
200-
// timeUnit only exists from Spring 5.3.10, so read it as an attribute
201-
final @Nullable Object timeUnitAttribute =
202-
AnnotationUtils.getAnnotationAttributes(scheduled).get("timeUnit");
203-
final @NotNull TimeUnit timeUnit =
204-
timeUnitAttribute instanceof TimeUnit
205-
? (TimeUnit) timeUnitAttribute
206-
: TimeUnit.MILLISECONDS;
163+
final long value, final @NotNull String valueString, final @NotNull TimeUnit timeUnit) {
207164
if (value >= 0) {
208165
return timeUnit.toMillis(value);
209166
}
@@ -218,34 +175,6 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
218175
return timeUnit.toMillis(Long.parseLong(trimmed));
219176
}
220177

221-
/**
222-
* Converts a Spring cron expression (6 fields, seconds first) to a 5 field crontab. Returns null
223-
* unless the seconds field is a single fixed number, since crontab cannot express sub-minute
224-
* schedules.
225-
*/
226-
private static @Nullable String toCrontab(final @NotNull String cron) {
227-
final @NotNull String trimmed = cron.trim();
228-
if (Scheduled.CRON_DISABLED.equals(trimmed)) {
229-
return null;
230-
}
231-
if (trimmed.startsWith("@")) {
232-
return CRON_MACROS.get(trimmed.toLowerCase(Locale.ROOT));
233-
}
234-
final @NotNull String[] fields = StringUtils.tokenizeToStringArray(trimmed, " \t");
235-
if (fields.length != 6 || !fields[0].matches("\\d{1,2}")) {
236-
return null;
237-
}
238-
final @NotNull StringBuilder crontab = new StringBuilder();
239-
for (int i = 1; i < fields.length; i++) {
240-
if (i > 1) {
241-
crontab.append(' ');
242-
}
243-
// Spring treats '?' like '*'
244-
crontab.append("?".equals(fields[i]) ? "*" : fields[i]);
245-
}
246-
return crontab.toString();
247-
}
248-
249178
private @Nullable String resolve(final @NotNull String value) {
250179
if (resolver == null || value.isEmpty()) {
251180
return value;

‎sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryCheckInAdviceTest.kt‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -253,35 +253,6 @@ class SentryCheckInAdviceTest {
253253
assertNull(config.timezone)
254254
}
255255

256-
@Test
257-
fun `cron question mark is converted to asterisk`() {
258-
val config = inProgressMonitorConfig { sampleServiceScheduled.cronQuestionMark() }
259-
assertEquals("0 9 * * *", config?.schedule?.value)
260-
}
261-
262-
@Test
263-
fun `cron macro is converted to crontab`() {
264-
val config = inProgressMonitorConfig { sampleServiceScheduled.cronMacro() }
265-
assertEquals("0 * * * *", config?.schedule?.value)
266-
}
267-
268-
@Test
269-
fun `cron with variable seconds sends no monitor config`() {
270-
assertNull(inProgressMonitorConfig { sampleServiceScheduled.cronEverySeconds() })
271-
}
272-
273-
@Test
274-
fun `disabled cron sends no monitor config`() {
275-
assertNull(inProgressMonitorConfig { sampleServiceScheduled.cronDisabled() })
276-
}
277-
278-
@Test
279-
fun `cron zone is sent as timezone`() {
280-
val config = inProgressMonitorConfig { sampleServiceScheduled.cronWithZone() }
281-
assertEquals("0 2 * * *", config?.schedule?.value)
282-
assertEquals("Europe/Vienna", config?.timezone)
283-
}
284-
285256
@Test
286257
fun `cron and zone placeholders are resolved`() {
287258
val config = inProgressMonitorConfig { sampleServiceScheduled.cronFromProperties() }
@@ -312,16 +283,6 @@ class SentryCheckInAdviceTest {
312283
assertEquals("minute", config?.schedule?.unit)
313284
}
314285

315-
@Test
316-
fun `sub-minute fixed rate sends no monitor config`() {
317-
assertNull(inProgressMonitorConfig { sampleServiceScheduled.fixedRateSeconds() })
318-
}
319-
320-
@Test
321-
fun `fixed rate not in whole minutes sends no monitor config`() {
322-
assertNull(inProgressMonitorConfig { sampleServiceScheduled.fixedRateNotWholeMinutes() })
323-
}
324-
325286
@Test
326287
fun `multiple schedules send no monitor config`() {
327288
assertNull(inProgressMonitorConfig { sampleServiceScheduled.multipleSchedules() })
@@ -421,24 +382,6 @@ class SentryCheckInAdviceTest {
421382

422383
@SentryCheckIn("cron") @Scheduled(cron = "0 15 10 * * MON-FRI") open fun cron() {}
423384

424-
@SentryCheckIn("cron_question_mark")
425-
@Scheduled(cron = "0 0 9 ? * *")
426-
open fun cronQuestionMark() {}
427-
428-
@SentryCheckIn("cron_macro") @Scheduled(cron = "@hourly") open fun cronMacro() {}
429-
430-
@SentryCheckIn("cron_every_seconds")
431-
@Scheduled(cron = "*/30 * * * * *")
432-
open fun cronEverySeconds() {}
433-
434-
@SentryCheckIn("cron_disabled")
435-
@Scheduled(cron = Scheduled.CRON_DISABLED)
436-
open fun cronDisabled() {}
437-
438-
@SentryCheckIn("cron_zone")
439-
@Scheduled(cron = "0 0 2 * * *", zone = "Europe/Vienna")
440-
open fun cronWithZone() {}
441-
442385
@SentryCheckIn("cron_properties")
443386
@Scheduled(cron = "\${my.cron.schedule}", zone = "\${my.cron.zone}")
444387
open fun cronFromProperties() {}
@@ -453,14 +396,6 @@ class SentryCheckInAdviceTest {
453396
@Scheduled(fixedDelayString = "\${my.cron.delay}")
454397
open fun fixedDelayFromProperties() {}
455398

456-
@SentryCheckIn("fixed_rate_seconds")
457-
@Scheduled(fixedRate = 30_000)
458-
open fun fixedRateSeconds() {}
459-
460-
@SentryCheckIn("fixed_rate_odd")
461-
@Scheduled(fixedRate = 90_000)
462-
open fun fixedRateNotWholeMinutes() {}
463-
464399
@SentryCheckIn("multiple")
465400
@Scheduled(cron = "0 0 1 * * *")
466401
@Scheduled(cron = "0 0 13 * * *")

0 commit comments

Comments
 (0)