From 7577afaafc611500e796636faf4a75c4ca23e66d Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Mon, 31 Jul 2023 20:40:39 +0500 Subject: [PATCH 1/2] Added warning upon setting polling time period below 30 ms --- .../config/PollingProjectConfigManager.java | 4 +++- .../PollingProjectConfigManagerTest.java | 22 +++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index 4ad34e7a..854dce8e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -75,7 +75,9 @@ public PollingProjectConfigManager(long period, TimeUnit timeUnit, long blocking this.blockingTimeoutPeriod = blockingTimeoutPeriod; this.blockingTimeoutUnit = blockingTimeoutUnit; this.notificationCenter = notificationCenter; - + if (TimeUnit.MILLISECONDS.convert(period, this.timeUnit) < 30) { + logger.warn("Polling intervals below 30 seconds are not recommended."); + } final ThreadFactory threadFactory = Executors.defaultThreadFactory(); this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(runnable -> { Thread thread = threadFactory.newThread(runnable); diff --git a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java index 130f8844..89dd8b2b 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java @@ -16,13 +16,15 @@ */ package com.optimizely.ab.config; +import ch.qos.logback.classic.Level; +import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.internal.NotificationRegistry; import com.optimizely.ab.notification.NotificationCenter; import com.optimizely.ab.notification.UpdateConfigNotification; -import org.junit.After; -import org.junit.Before; -import org.junit.Ignore; -import org.junit.Test; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import org.junit.*; +import org.junit.rules.ExpectedException; +import org.junit.rules.RuleChain; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; @@ -41,6 +43,13 @@ public class PollingProjectConfigManagerTest { private static final TimeUnit POLLING_UNIT = TimeUnit.MILLISECONDS; private static final int PROJECT_CONFIG_DELAY = 100; + public ExpectedException thrown = ExpectedException.none(); + public LogbackVerifier logbackVerifier = new LogbackVerifier(); + + @Rule + @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public RuleChain ruleChain = RuleChain.outerRule(thrown) + .around(logbackVerifier); private TestProjectConfigManager testProjectConfigManager; private ProjectConfig projectConfig; @@ -228,6 +237,11 @@ public void testUpdateConfigNotificationGetsTriggered() throws InterruptedExcept assertTrue(countDownLatch.await(500, TimeUnit.MILLISECONDS)); } + @Test + public void testSettingUpLowerPollingPeriodResultsInWarning() throws InterruptedException { + logbackVerifier.expectMessage(Level.WARN, "Polling intervals below 30 seconds are not recommended."); + } + @Test public void testUpdateConfigNotificationDoesNotResultInDeadlock() throws Exception { NotificationCenter notificationCenter = new NotificationCenter(); From 7ed73233e9ee23d10d7d22509c0d6a13eec79449 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Wed, 2 Aug 2023 20:19:25 +0500 Subject: [PATCH 2/2] Updated test and changed expecting time to lower than 30 seconds --- .../optimizely/ab/config/PollingProjectConfigManager.java | 2 +- .../ab/config/PollingProjectConfigManagerTest.java | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index 854dce8e..5f0c44e7 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -75,7 +75,7 @@ public PollingProjectConfigManager(long period, TimeUnit timeUnit, long blocking this.blockingTimeoutPeriod = blockingTimeoutPeriod; this.blockingTimeoutUnit = blockingTimeoutUnit; this.notificationCenter = notificationCenter; - if (TimeUnit.MILLISECONDS.convert(period, this.timeUnit) < 30) { + if (TimeUnit.SECONDS.convert(period, this.timeUnit) < 30) { logger.warn("Polling intervals below 30 seconds are not recommended."); } final ThreadFactory threadFactory = Executors.defaultThreadFactory(); diff --git a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java index 89dd8b2b..91a9b871 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java @@ -239,6 +239,8 @@ public void testUpdateConfigNotificationGetsTriggered() throws InterruptedExcept @Test public void testSettingUpLowerPollingPeriodResultsInWarning() throws InterruptedException { + long pollingPeriod = 29; + new TestProjectConfigManager(projectConfig, pollingPeriod, TimeUnit.SECONDS, pollingPeriod / 2, TimeUnit.SECONDS, new NotificationCenter()); logbackVerifier.expectMessage(Level.WARN, "Polling intervals below 30 seconds are not recommended."); } @@ -271,7 +273,11 @@ private TestProjectConfigManager(ProjectConfig projectConfig) { } private TestProjectConfigManager(ProjectConfig projectConfig, long blockPeriod, NotificationCenter notificationCenter) { - super(POLLING_PERIOD, POLLING_UNIT, blockPeriod, POLLING_UNIT, notificationCenter); + this(projectConfig, POLLING_PERIOD, POLLING_UNIT, blockPeriod, POLLING_UNIT, notificationCenter); + } + + private TestProjectConfigManager(ProjectConfig projectConfig, long pollingPeriod, TimeUnit pollingUnit, long blockPeriod, TimeUnit blockingUnit, NotificationCenter notificationCenter) { + super(pollingPeriod, pollingUnit, blockPeriod, blockingUnit, notificationCenter); this.projectConfig = projectConfig; }