Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Added set polling interval and set blocking timeout period method in OptimizelyFactory #264

Merged
merged 4 commits into from
May 12, 2021
Merged

Feat: Added set polling interval and set blocking timeout period method in OptimizelyFactory #264

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16cd373
Added set polling interval method and set blocking timeout period in …
mnoman09 May 6, 2021
0d78c85
Fixed UnitTests
mnoman09 May 6, 2021
5bdbe43
headers updated
mnoman09 May 10, 2021
6c56c88
addressed feedback
msohailhussain May 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions OptimizelySDK.Tests/OptimizelyFactoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ public void TestOptimizelyInstanceUsingConfigFile()
optimizely.Dispose();
}


[Test]
public void TestProjectConfigManagerUsingSDKKey()
{
Expand All @@ -78,8 +77,8 @@ public void TestProjectConfigManagerUsingSDKKey()
Url = "https://cdn.optimizely.com/datafiles/my-sdk-key.json",
LastModified = "",
AutoUpdate = true,
BlockingTimeout = TimeSpan.FromSeconds(15),
PollingInterval = TimeSpan.FromMinutes(5)
BlockingTimeout = TimeSpan.FromSeconds(30),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update header.

PollingInterval = TimeSpan.FromMilliseconds(2023)
};

Assert.AreEqual(actualConfigManagerProps, expectedConfigManagerProps);
Expand All @@ -102,15 +101,40 @@ public void TestProjectConfigManagerWithDatafileAccessToken()
LastModified = "",
DatafileAccessToken = "access-token",
AutoUpdate = true,
BlockingTimeout = TimeSpan.FromSeconds(15),
PollingInterval = TimeSpan.FromMinutes(5)
BlockingTimeout = TimeSpan.FromSeconds(30),
PollingInterval = TimeSpan.FromMilliseconds(2023)
};

Assert.AreEqual(actualConfigManagerProps, expectedConfigManagerProps);

optimizely.Dispose();
}

[Test]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add one more priority test. when polling interval is set using method then configelement value is discarded.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these tests already show that configElement values will be discarded once we set the polling interval using optimizelyFactory.
Value given in ConfigElement is 2 second, where as we can see that at line 130 and 116 after setting polling Interval using OptlyFactory all other values remained same only polling and blocking interval are changed.

public void TestOptimizelyInstanceUsingConfigFileWithBlockingAndPollingInterval()
{
OptimizelyFactory.SetBlockingTimeOutPeriod(TimeSpan.FromSeconds(30));
OptimizelyFactory.SetPollingInterval(TimeSpan.FromMilliseconds(2023));
var optimizely = OptimizelyFactory.NewDefaultInstance();
// Check values are loaded from app.config or not.
var projectConfigManager = optimizely.ProjectConfigManager as HttpProjectConfigManager;
Assert.NotNull(projectConfigManager);

var actualConfigManagerProps = new ProjectConfigManagerProps(projectConfigManager);
var expectedConfigManagerProps = new ProjectConfigManagerProps
{
Url = "www.testurl.com",
LastModified = "",
AutoUpdate = true,
DatafileAccessToken = "testingtoken123",
BlockingTimeout = TimeSpan.FromSeconds(30),
PollingInterval = TimeSpan.FromMilliseconds(2023)
};

Assert.AreEqual(actualConfigManagerProps, expectedConfigManagerProps);
optimizely.Dispose();
}

[Test]
public void TestProjectConfigManagerWithCustomProjectConfigManager()
{
Expand Down
23 changes: 19 additions & 4 deletions OptimizelySDK/OptimizelyFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public static class OptimizelyFactory
{
private static int MaxEventBatchSize;
private static TimeSpan MaxEventFlushInterval;
private static TimeSpan PollingInterval;
private static TimeSpan BlockingTimeOutPeriod;
private static ILogger OptimizelyLogger;
private const string ConfigSectionName = "optlySDKConfigSection";

Expand All @@ -49,6 +51,16 @@ public static void SetFlushInterval(TimeSpan flushInterval)
MaxEventFlushInterval = flushInterval;
}

public static void SetPollingInterval(TimeSpan pollingInterval)
{
PollingInterval = pollingInterval;
}

public static void SetBlockingTimeOutPeriod(TimeSpan blockingTimeOutPeriod)
{
BlockingTimeOutPeriod = blockingTimeOutPeriod;
}

public static void SetLogger(ILogger logger)
{
OptimizelyLogger = logger;
Expand Down Expand Up @@ -76,13 +88,12 @@ public static Optimizely NewDefaultInstance()
var eventDispatcher = new DefaultEventDispatcher(logger);
var builder = new HttpProjectConfigManager.Builder();
var notificationCenter = new NotificationCenter();

var configManager = builder
.WithSdkKey(httpProjectConfigElement.SDKKey)
.WithUrl(httpProjectConfigElement.Url)
.WithFormat(httpProjectConfigElement.Format)
.WithPollingInterval(TimeSpan.FromMilliseconds(httpProjectConfigElement.PollingInterval))
.WithBlockingTimeoutPeriod(TimeSpan.FromMilliseconds(httpProjectConfigElement.BlockingTimeOutPeriod))
.WithPollingInterval(PollingInterval == TimeSpan.Zero ? TimeSpan.FromMilliseconds(httpProjectConfigElement.PollingInterval) : PollingInterval)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PLease update header.

.WithBlockingTimeoutPeriod(BlockingTimeOutPeriod == TimeSpan.Zero ? TimeSpan.FromMilliseconds(httpProjectConfigElement.BlockingTimeOutPeriod) : BlockingTimeOutPeriod)
#if !NET40 && !NET35
.WithAccessToken(httpProjectConfigElement.DatafileAccessToken)
#endif
Expand Down Expand Up @@ -111,7 +122,7 @@ public static Optimizely NewDefaultInstance()
}
#endif

public static Optimizely NewDefaultInstance(string sdkKey)
public static Optimizely NewDefaultInstance(string sdkKey)
{
return NewDefaultInstance(sdkKey, null);
}
Expand All @@ -128,6 +139,8 @@ public static Optimizely NewDefaultInstance(string sdkKey, string fallback, stri
.WithSdkKey(sdkKey)
.WithDatafile(fallback)
.WithLogger(logger)
.WithPollingInterval(PollingInterval)
.WithBlockingTimeoutPeriod(BlockingTimeOutPeriod)
Comment on lines +142 to +143
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused - why don't have the config reading unlike the above:
.WithPollingInterval(PollingInterval == TimeSpan.Zero ? TimeSpan.FromMilliseconds(httpProjectConfigElement.PollingInterval) : PollingInterval)
This can have different configuration paths depending on factory method parameters. It can be confusing unless we document very clearly about them.

.WithErrorHandler(errorHandler)
.WithAccessToken(datafileAuthToken)
.WithNotificationCenter(notificationCenter)
Expand Down Expand Up @@ -160,6 +173,8 @@ public static Optimizely NewDefaultInstance(string sdkKey, string fallback)
.WithSdkKey(sdkKey)
.WithDatafile(fallback)
.WithLogger(logger)
.WithPollingInterval(PollingInterval)
.WithBlockingTimeoutPeriod(BlockingTimeOutPeriod)
.WithErrorHandler(errorHandler)
.WithNotificationCenter(notificationCenter)
.Build(true);
Expand Down