Config v6 - Githubissues

adams85 commented 1 year ago

Describe the purpose of your pull request

Implements config v6 support.

Features/improvements:

[x] Fetch new config json version (config_v6.json) and update config model
[x] SDK key format validation (when client is not set up to use local-only flag overrides)
[x] AND conditions
[x] New condition types
- Prerequisite flag condition
- Segment condition
[x] Changed comparison operators
- (NOT) CONTAINS ANY OF
[x] New comparison operators
- BEFORE/AFTER
- TEXT (NOT) EQUALS (both hashed and cleartext)
- TEXT (NOT) STARTS/ENDS WITH ANY OF (both hashed and cleartext)
- ARRAY CONTAINS ANY OF (both hashed and cleartext)
[x] Percentage options within targeting rules
[x] Percentage options based on custom attributes
[x] Custom User Object attribute handling (don't force string values + autoconversions under the hood)
[x] User Object attribute/comparison value trimming
[x] Comparison value hashing changes (SHA256, salting)
[x] Flag overrides vs. config json salt and segments
[x] EvaluationDetails.MatchedTargetingRule/MatchedPercentageOption properties (rename + set combinations correctly)

Tests:

[x] Evaluation matrix tests
[x] Evaluation log tests
[x] SDK key format validation
[x] Custom User Object attribute handling (don't force string values + autoconversions under the hood)
[x] Flag overrides vs. prerequisite flags (circular dependency, comparison value type mismatch, dependent/prereq flag overriding)
[x] Flag overrides vs. config json salt and segments
[x] EvaluationDetails.MatchedTargetingRule/MatchedPercentageOption properties (rename + set combinations correctly)

Related improvements/fixes:

[ ] ~We should go to the cache in all polling modes instead of using the in memory variable (see https://trello.com/c/rreKm64A)~ (done earlier)

Breaking changes:

Rename the MatchedEvaluationRule property to MatchedTargetingRule and the MatchedEvaluationPercentageRule property to MatchedPercentageOption in EvaluationDetails.
Change config model (IConfig and related interfaces/enums).
Change nullability of User.Custom to disallow null values.
Change the User.AllAttributes property into a method (GetAllAttributes) as it allocates under the hood.

Requirement checklist (only if applicable)

[x] I have covered the applied changes with automated tests.
[x] I have executed the full automated test set against my changes.
[x] I have validated my changes against all supported platform versions.
[x] I have read and accepted the contribution agreement.

adams85 commented 1 year ago

Some benchmark results to support the arguments made for the current implementation.

Method	Runtime	LogInfo	Mean	Error	StdDev	Ratio	RatioSD	Gen0	Gen1	Allocated	Alloc Ratio
MatrixTests_ConfigV5	.NET 6.0	False	25.92 ms	2.274 ms	0.125 ms	1.00	0.00	2406.2500	-	38.88 MB	1.00
MatrixTests_ConfigV5	.NET Framework 4.8	False	56.24 ms	16.932 ms	0.928 ms	2.17	0.05	7111.1111	-	43.08 MB	1.11

MatrixTests_ConfigV6	.NET 6.0	False	11.17 ms	2.751 ms	0.151 ms	1.00	0.00	859.3750	-	13.94 MB	1.00
MatrixTests_ConfigV6	.NET Framework 4.8	False	29.70 ms	4.885 ms	0.268 ms	2.66	0.01	3218.7500	-	19.36 MB	1.39

MatrixTests_ConfigV5	.NET 6.0	True	25.64 ms	3.050 ms	0.167 ms	1.00	0.00	2406.2500	-	38.88 MB	1.00
MatrixTests_ConfigV5	.NET Framework 4.8	True	56.09 ms	1.123 ms	0.062 ms	2.19	0.02	7111.1111	-	43.08 MB	1.11

MatrixTests_ConfigV6	.NET 6.0	True	28.51 ms	0.423 ms	0.023 ms	1.00	0.00	3937.5000	31.2500	62.83 MB	1.00
MatrixTests_ConfigV6	.NET Framework 4.8	True	69.74 ms	9.644 ms	0.529 ms	2.45	0.02	12875.0000	125.0000	77.4 MB	1.23

adams85 commented 12 months ago

@configcat/ux-qa-team May I also request a review from UX perspective for this PR? More specifically, for the XML comments of the source files located under src/ConfigCatClient/Models.