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.

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

The logic for 'in' is incorrect #24

Closed
fivesmallq opened this issue Jan 30, 2024 · 1 comment
Closed

The logic for 'in' is incorrect #24

fivesmallq opened this issue Jan 30, 2024 · 1 comment

Comments

@fivesmallq
Copy link

fivesmallq commented Jan 30, 2024

version: growthbook-c-sharp 1.0.0

When I was dealing with conditions involving 'in', I found that the results were not as expected.
I suspect that during the check for whether an element is contained in an array, the ToString method converted the array into a string type, and then it was processed using string methods.

see different:

return conditionValue.ToString().Contains(actualValue.ToString());

return conditionList.FirstOrDefault(j => j.ToString().Equals(attributeValue?.ToString())) != null;

test code:

[TestMethod]
        public void TestFeature()
        {
            var featureString = """
                                {
                                  "defaultValue": false,
                                  "rules": [
                                    {
                                      "condition": {
                                        "userId": {
                                          "$in": [
                                            "ac1",
                                            "ac2",
                                            "ac3"
                                          ]
                                        }
                                      },
                                      "force": true
                                    }
                                  ]
                                }
                                """;
            var feature = JsonConvert.DeserializeObject<Feature>(featureString);
            var staticFeatures = new Dictionary<string, Feature>
            {
                {
                    "test-in-op", feature
                }
            };
            var context = new Context
            {
                Enabled = true,
                Url = "",
                Features = staticFeatures
            };

            var growthBook = new GrowthBook.GrowthBook(context);
            var flag = growthBook.IsOn("test-in-op");
            Assert.IsFalse(flag);
            
            // set userId
            context.Attributes.Add("userId", "");
            var errorFlag = growthBook.IsOn("test-in-op");
            Assert.IsFalse(errorFlag); // assert failed, it should be false
        }
@Norhaven
Copy link
Collaborator

@fivesmallq Good catch, thanks! It was correctly identifying both cases as not having an actual array value, but only did a null check on the value prior to comparison which meant that an empty string value would pass and would then be found within the subsequent string comparison. Should be fixed with the PR above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants