Skip to content

Commit bff6667

Browse files
msohailhussainMichael Ng
msohailhussain
authored and
Michael Ng
committed
fix: issues related to blocking timeout (#172)
1 parent 3fe2d2c commit bff6667

File tree

3 files changed

+89
-17
lines changed

3 files changed

+89
-17
lines changed

‎OptimizelySDK.Tests/ConfigTest/HttpProjectConfigManagerTest.cs‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ public void TestHttpConfigManagerDoesNotWaitForTheConfigWhenDeferIsTrue()
130130
.WithSdkKey("QBw9gFM8oTn7ogY9ANCC1z")
131131
.WithLogger(LoggerMock.Object)
132132
.WithPollingInterval(TimeSpan.FromSeconds(2))
133-
.WithBlockingTimeoutPeriod(TimeSpan.FromSeconds(0))
133+
// negligible timeout
134+
.WithBlockingTimeoutPeriod(TimeSpan.FromMilliseconds(50))
134135
.WithStartByDefault()
135136
.Build(false);
136137

@@ -176,6 +177,52 @@ public void TestHttpConfigManagerDoesNotSendConfigUpdateNotificationWhenDatafile
176177
NotificationCallbackMock.Verify(nc => nc.TestConfigUpdateCallback(), Times.Never);
177178
Assert.NotNull(httpManager.GetConfig()); Assert.NotNull(httpManager.GetConfig());
178179
}
179-
#endregion
180-
}
180+
181+
[Test]
182+
public void TestDefaultBlockingTimeoutWhileProvidingZero()
183+
{
184+
var httpManager = new HttpProjectConfigManager.Builder()
185+
.WithSdkKey("QBw9gFM8oTn7ogY9ANCC1z")
186+
.WithDatafile(TestData.Datafile)
187+
.WithLogger(LoggerMock.Object)
188+
.WithPollingInterval(TimeSpan.FromMilliseconds(1000))
189+
.WithBlockingTimeoutPeriod(TimeSpan.FromMilliseconds(0))
190+
.WithStartByDefault(true)
191+
.Build(true);
192+
193+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, $"Blocking timeout is not valid, using default blocking timeout {TimeSpan.FromSeconds(15).TotalMilliseconds}ms"));
194+
}
195+
196+
[Test]
197+
public void TestDefaultPeriodWhileProvidingZero()
198+
{
199+
var httpManager = new HttpProjectConfigManager.Builder()
200+
.WithSdkKey("QBw9gFM8oTn7ogY9ANCC1z")
201+
.WithDatafile(TestData.Datafile)
202+
.WithLogger(LoggerMock.Object)
203+
.WithPollingInterval(TimeSpan.FromMilliseconds(0))
204+
.WithBlockingTimeoutPeriod(TimeSpan.FromMilliseconds(1000))
205+
.WithStartByDefault(true)
206+
.Build(true);
207+
208+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, $"Period is not valid for periodic calls, using default period {TimeSpan.FromMinutes(5).TotalMilliseconds}ms"));
209+
}
210+
211+
[Test]
212+
public void TestDefaultPeriodWhileProvidingNegative()
213+
{
214+
var httpManager = new HttpProjectConfigManager.Builder()
215+
.WithSdkKey("QBw9gFM8oTn7ogY9ANCC1z")
216+
.WithDatafile(TestData.Datafile)
217+
.WithLogger(LoggerMock.Object)
218+
.WithPollingInterval(TimeSpan.FromMilliseconds(-1))
219+
.WithBlockingTimeoutPeriod(TimeSpan.FromMilliseconds(1000))
220+
.WithStartByDefault(true)
221+
.Build(true);
222+
223+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, $"Period is not valid for periodic calls, using default period {TimeSpan.FromMinutes(5).TotalMilliseconds}ms"));
224+
}
225+
226+
#endregion
227+
}
181228
}

‎OptimizelySDK/Config/HttpProjectConfigManager.cs‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,18 @@ protected override ProjectConfig Poll()
122122

123123
public class Builder
124124
{
125+
private const long MAX_MILLISECONDS_LIMIT = 4294967294;
126+
private readonly TimeSpan DEFAULT_PERIOD = TimeSpan.FromMinutes(5);
127+
private readonly TimeSpan DEFAULT_BLOCKINGOUT_PERIOD = TimeSpan.FromSeconds(15);
128+
125129
private string Datafile;
126130
private string SdkKey;
127131
private string Url;
128132
private string Format = "https://cdn.optimizely.com/datafiles/{0}.json";
129133
private ILogger Logger;
130134
private IErrorHandler ErrorHandler;
131-
private TimeSpan Period = TimeSpan.FromMinutes(5);
132-
private TimeSpan BlockingTimeoutSpan = TimeSpan.FromSeconds(15);
135+
private TimeSpan Period;
136+
private TimeSpan BlockingTimeoutSpan;
133137
private bool AutoUpdate = true;
134138
private bool StartByDefault;
135139
private NotificationCenter NotificationCenter;
@@ -143,42 +147,49 @@ public Builder WithBlockingTimeoutPeriod(TimeSpan blockingTimeoutSpan)
143147
public Builder WithDatafile(string datafile)
144148
{
145149
Datafile = datafile;
150+
146151
return this;
147152
}
148153

149154
public Builder WithSdkKey(string sdkKey)
150155
{
151156
SdkKey = sdkKey;
157+
152158
return this;
153159
}
154160

155161
public Builder WithUrl(string url)
156162
{
157163
Url = url;
164+
158165
return this;
159166
}
160167

161168
public Builder WithPollingInterval(TimeSpan period)
162-
{
169+
{
163170
Period = period;
171+
164172
return this;
165173
}
166174

167175
public Builder WithFormat(string format)
168176
{
169177
Format = format;
178+
170179
return this;
171180
}
172181

173182
public Builder WithLogger(ILogger logger)
174183
{
175184
Logger = logger;
185+
176186
return this;
177187
}
178188

179189
public Builder WithErrorHandler(IErrorHandler errorHandler)
180190
{
181191
ErrorHandler = errorHandler;
192+
182193
return this;
183194
}
184195

@@ -222,20 +233,35 @@ public HttpProjectConfigManager Build()
222233
public HttpProjectConfigManager Build(bool defer)
223234
{
224235
HttpProjectConfigManager configManager = null;
236+
225237
if (Logger == null)
226238
Logger = new DefaultLogger();
227239

240+
if (ErrorHandler == null)
241+
ErrorHandler = new DefaultErrorHandler();
242+
228243
if (string.IsNullOrEmpty(Url) && string.IsNullOrEmpty(SdkKey))
229244
{
230-
ErrorHandler.HandleError(new Exception("SdkKey cannot be null"));
231-
throw new Exception("SdkKey cannot be null");
245+
ErrorHandler.HandleError(new Exception("SdkKey cannot be null"));
232246
}
233247
else if (!string.IsNullOrEmpty(SdkKey))
234248
{
235249
Url = string.Format(Format, SdkKey);
236250
}
251+
252+
if (Period.TotalMilliseconds <= 0 || Period.TotalMilliseconds > MAX_MILLISECONDS_LIMIT) {
253+
Logger.Log(LogLevel.INFO, $"Period is not valid for periodic calls, using default period {DEFAULT_PERIOD.TotalMilliseconds}ms");
254+
Period = DEFAULT_PERIOD;
255+
}
237256

238257

258+
if (BlockingTimeoutSpan.TotalMilliseconds <= 0 || BlockingTimeoutSpan.TotalMilliseconds > MAX_MILLISECONDS_LIMIT) {
259+
Logger.Log(LogLevel.INFO, $"Blocking timeout is not valid, using default blocking timeout {DEFAULT_BLOCKINGOUT_PERIOD.TotalMilliseconds}ms");
260+
BlockingTimeoutSpan = DEFAULT_BLOCKINGOUT_PERIOD;
261+
}
262+
263+
264+
239265
configManager = new HttpProjectConfigManager(Period, Url, BlockingTimeoutSpan, AutoUpdate, Logger, ErrorHandler);
240266

241267
if (Datafile != null)

‎OptimizelySDK/Config/PollingProjectConfigManager.cs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,7 @@ public ProjectConfig GetConfig()
136136
/// <param name="projectConfig">ProjectConfig</param>
137137
/// <returns>true if the ProjectConfig saved successfully, false otherwise</returns>
138138
public bool SetConfig(ProjectConfig projectConfig)
139-
{
140-
// trigger now, due because of delayed latency response
141-
if (scheduleWhenFinished && IsStarted) {
142-
// Can't directly call Run, it will be part of previous thread then.
143-
// Call immediately, because it's due now.
144-
scheduleWhenFinished = false;
145-
SchedulerService.Change(TimeSpan.FromSeconds(0), PollingInterval);
146-
}
147-
139+
{
148140
if (projectConfig == null)
149141
return false;
150142

@@ -191,6 +183,13 @@ public virtual void Run()
191183
Logger.Log(LogLevel.ERROR, "Unable to get project config. Error: " + exception.GetAllMessages());
192184
} finally {
193185
Interlocked.Exchange(ref resourceInUse, 0);
186+
187+
// trigger now, due because of delayed latency response
188+
if (scheduleWhenFinished && IsStarted) {
189+
// Call immediately, because it's due now.
190+
scheduleWhenFinished = false;
191+
SchedulerService.Change(TimeSpan.FromSeconds(0), PollingInterval);
192+
}
194193
}
195194
}
196195
else {

0 commit comments

Comments
 (0)