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

BUG: CC007 incorrectly errors on valid comparisons #410

Closed
MonochromeChameleon opened this issue Dec 8, 2023 · 12 comments
Closed

BUG: CC007 incorrectly errors on valid comparisons #410

MonochromeChameleon opened this issue Dec 8, 2023 · 12 comments
Assignees

Comments

@MonochromeChameleon
Copy link
Contributor

#403 introduced checking for Condition statement, but incorrectly flags comparisons between two flow variables as invalid. In the PR description it says:

Unknown or invalid operands. Eg fault.name = null is fine, fault.name = invalid_client is invalid. (The RHS should be quoted)

but if invalid_client were a flow variable, this comparison would be valid.

@ssvaidyanathan
Copy link
Collaborator

@MonochromeChameleon - I dont think you have a flow variable on both sides of the operation. In your case invalid_client must be a string for the condition to work

@MonochromeChameleon
Copy link
Contributor Author

@ssvaidyanathan that's just the example from the error comment, but in the more general case, you should definitely be able to compare two flow variables, e.g.

<Step>
    <Name>rf-invalid-format</Name>
    <Condition>detected_format != required_format</Condition>
</Step>

@ghost
Copy link

ghost commented Jan 9, 2024

I'm having a similar issue, same code error but when I use multiline conditions or the "not" operator.

Ex:

<Condition>
    proxy.pathsuffix MatchesPath "example" and 
    request.verb = "GET"
</Condition>

and

<Condition>not(proxy.pathsuffix MatchesPath "example")</Condition>

@ssvaidyanathan
Copy link
Collaborator

@MonochromeChameleon - Apigee as a product doesnt support comparing two flow variables from what I understand.

@ssvaidyanathan
Copy link
Collaborator

@matnsc - I have submitted a PR to allow not.. For now you can use ! instead of not. The multi line condition should be available in 2.46.3. Can you please try and let us know. Its part of #402

@Polaratom
Copy link

Polaratom commented Jan 10, 2024

@ssvaidyanathan APIGEE does support using two flow variables, In my case I check org and env names

I expect org and env names in resource URI, and then use flow variables organization.name and environment.name against the extracted values form resource URI but due to this it's failing

@ssvaidyanathan
Copy link
Collaborator

ssvaidyanathan commented Jan 10, 2024

@ssvaidyanathan APIGEE does support using two flow variables, In my case I check org and env names

I expect org and env names in resource URI, and then use flow variables organization.name and environment.name against the extracted values form resource URI but due to this it's failing

@Polaratom - Can you share an example of what you mean please?

@DinoChiesa
Copy link
Collaborator

I have run some tests and found these results for what Apigee accepts on the right-hand-side of operators:

| operator              | RHS                 |
| --------------------- | ------------------- |
| Equals                | literal or variable |
| GreaterThanOrEquals   | literal or variable |
| GreaterThan           | literal or variable |
| LesserThanOrEquals    | literal or variable |
| LesserThan            | literal or variable |
| StartsWith            | literal or variable |
| NotEquals             | literal or variable |
| EqualsCaseInsensitive | literal only        |
| JavaRegex             | literal only        |
| MatchesPath           | literal only        |
| Matches               | literal only        |

@ghost
Copy link

ghost commented Jan 11, 2024

@matnsc - I have submitted a PR to allow not.. For now you can use ! instead of not. The multi line condition should be available in 2.46.3. Can you please try and let us know. Its part of #402

I did test the lastest PR (#416) in some of our proxies that use multiline, not (and other named operators) and double equals which previously failed CC007 tests, now it's working as expected.

Latest version (2.46.3):
before

PR #416:
after

@DinoChiesa
Copy link
Collaborator

Excellent, thank you for the confirmation.
@MonochromeChameleon - Can we close this one out now?

@ssvaidyanathan
Copy link
Collaborator

Just released v2.47.0
Please test using this version and let us know if you find any issues.

@MonochromeChameleon
Copy link
Contributor Author

Excellent, thank you for the confirmation. @MonochromeChameleon - Can we close this one out now?

Yep, all working for me now - thanks!

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

4 participants