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

@in operator is unable to be applied for (int, list(uint)) #416

Open
buildbreaker opened this issue Jun 29, 2023 · 1 comment
Open

@in operator is unable to be applied for (int, list(uint)) #416

buildbreaker opened this issue Jun 29, 2023 · 1 comment

Comments

@buildbreaker
Copy link
Contributor

buildbreaker commented Jun 29, 2023

I am looking to use cel-java to evaluate if a value is in a list. The problem that I'm running into is the different conversions between the different types. This fails on the Script build line:

check failed: ERROR: <input>:1:5: found no matching overload for '@in' applied to '(int, list(uint))'
 | !(3 in rules.repeated_fixed32) ? 'value must be in list' : ''
 | ....^
org.projectnessie.cel.tools.ScriptCreateException: check failed: ERROR: <input>:1:5: found no matching overload for '@in' applied to '(int, list(uint))'
 | !(3 in rules.repeated_fixed32) ? 'value must be in list' : ''
 | ....^

What I am also noticing is that if I change the script to be a unit (intList() test), it still fails due to cel-java interpreting the list type to be int rather than uint. Instead of failing on build time, it incorrectly materializes the uint list as a list of integers, then it cannot find the unit in the list because of the type difference.. From my investigation it seems like this is intentional behavior as we do end up hitting this code path for equality between UintT and IntT.

Here are tests that I am using to demonstrate this behavior:

  @Test
  public void uintList() throws ScriptException {
    TestAllTypesProto.TestAllTypes rule = TestAllTypesProto.TestAllTypes.newBuilder()
            .addRepeatedFixed32(2)
            .addRepeatedFixed32(3)
            .build();
    ScriptHost scriptHost = ScriptHost.newBuilder().build();
    Script script =
        scriptHost
            .buildScript("!(3 in rules.repeated_fixed32) ? 'value must be in list' : ''")
            .withTypes(rule.getDefaultInstanceForType())
            .withDeclarations(
                Decls.newVar(
                    "rules", Decls.newObjectType(rule.getDescriptorForType().getFullName())))
            .build();
    Map<String, Object> arguments = new HashMap<>();
    arguments.put("rules", rule);

    String result = script.execute(String.class, arguments);
    assertThat(result).isEqualTo("");
  }

  @Test
  public void intList() throws ScriptException {
    TestAllTypesProto.TestAllTypes rule = TestAllTypesProto.TestAllTypes.newBuilder()
            .addRepeatedFixed32(2)
            .addRepeatedFixed32(3)
            .build();
    ScriptHost scriptHost = ScriptHost.newBuilder().build();
    Script script =
            scriptHost
                    .buildScript("!(3u in rules.repeated_fixed32) ? 'value must be in list' : ''")
                    .withTypes(rule.getDefaultInstanceForType())
                    .withDeclarations(
                            Decls.newVar(
                                    "rules", Decls.newObjectType(rule.getDescriptorForType().getFullName())))
                    .build();
    Map<String, Object> arguments = new HashMap<>();
    arguments.put("rules", rule);

    String result = script.execute(String.class, arguments);
    assertThat(result).isEqualTo("");
  }

Thank you for your quick responses!

@Alfus
Copy link

Alfus commented Jun 29, 2023

Note that the type checker is correct, and the underlying problem is that {uint} in {list(uint)} is not evaluating correctly, and always returns false for protobuf repeated uint fields. Though at runtime, {double} in {list(int)} and {uint} in {list(int)} should work in CEL, regardless. So one fix might be to match the expected behavior, though I wonder what these return at runtime:

  • type(rules.repeated_fixed32) - I would guess list(int) or list(dyn) though it should be list(uint) (list(dyn) would be ok)
  • type(rules.repeated_fixed32[0]) - I would guess int though it should be uint

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