feat: getOptimizelyConfig API to get static experiments and features data

[fayyazarshad]

Summary

Added OptimizelyConfigService which generates OptimizelyConfig Object containing experiments and features data. This is the first step in the development of getOptimizelyConfig api. This service always generates a new OptimzelyConfig object for now. Next PR will contain caching implementation for OptimizelyConfig object.

Test plan

Added Unit Tests

[coveralls]

Pull Request Test Coverage Report for Build 1325

• 132 of 157 (84.08%) changed or added relevant lines in 7 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.2%) to 89.519%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyExperiment.java	14	17	82.35%
core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyFeature.java	17	20	85.0%
core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyVariable.java	15	18	83.33%
core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyVariation.java	16	19	84.21%
core-api/src/main/java/com/optimizely/ab/Optimizely.java	0	5	0.0%
core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigService.java	62	70	88.57%

Totals
Change from base Build 1323:	-0.2%
Covered Lines:	22446
Relevant Lines:	25074

---

💛 - Coveralls

[jaeopt]

Overall looks good! A few small changes required.

[zashraf1985]

LGTM

[jaeopt]

A small change required

[jaeopt]

LGTM

[mikecdavis]

I'm concerned with the test coverage. I think we should have more explicit test cases and scenarios. I would also like to see .stream() used as opposed to .forEach(...) when building the transformations. I provided an example where the method structure first addresses any pre-conditions then builds the response. I think that pattern can be applied to the other getters and will be cleaner overall.

[mikecdavis]

OptimizelyConfigService looks great. Thanks for making those changes, I just noted a few minor changes there.

I would have liked to see better unit test coverage of the individual methods: e.g.

• testGenerateFeatureKeyToVariablesMap
• testGetExperimentFeatureKey
• testGetExperimentsMap
• testGetVariationsMap
• testGetMergedVariablesMap
• testGetExperimentsMapForFeature
• testGetFeatureVariablesMap
• testGetFeaturesMap

The current approach is a higher level "integration" style test relying on validConfigJsonV4(). As a result the coverage feels more like a sanity check than a comprehensive intentional test suite. We also don't have explicit unit tests on the individual POJO constructors and getters. The coverage is assumed through the higher level testing, but again its implicit coverage not explicit.

[zashraf1985]

@mikecdavis ... Thanks for your feedback... You are right about unit tests not looking like the way they should in java. I should have been more watchful about it. I have been juggling between reviewing and implementing optimizely config on multiple languages so may be i mostly leaned towards porting some or most of the stuff instead of totally rethinking how it should be implemented somewhat differently in different languages. This is valuable feedback for me both in context of the current feature and also for the future work.

[mikecdavis]

Sorry to keep being a stickler about this, but we need more granular, explicit tests.

Must have:

• The model classes need tests for the constructors and getters.

Nice to have:

• The Service tests should not rely on validConfigJsonV4(), this is a bad precedent that we need to try to break. Instead, construct explicit input and output. Our dependency on validConfigJsonV4() is brittle and lacks comprehension.

[mikecdavis]

A few typos, but LGTM.

[fayyazarshad]

@jaeopt can you please review and approve, as its blocked upon your change request

[jaeopt]

LGTM

[zashraf1985]

@fayyazarshad this should be 2016-2020