-
Notifications
You must be signed in to change notification settings - Fork 54
NEW (GraphEngine): @W-12408352@: Classifies "as user" DML operations as safe. #1080
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
Conversation
rmohan20
commented
May 19, 2023
- Parses "as user" and "as system" on DML operations.
- ApexFlsViolationsRule excludes "as user" DML operations from further analysis.
- New tests for "as user" and "as system" scenarios.
…from ApexFlsViolationsRule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic to parse "as user" and "as system". A DML statement with neither is considered as executing in system mode.
| if (dmlStatementVertexChildren.size() != validationType.parameterCount) { | ||
| throw new UnexpectedException( | ||
| "Unexpected count of parameters: " + dmlStatementVertexChildren.size()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check was me being extra cautious when I didn't understand ASTs better. This doesn't make sense anymore since ASTs will be standard based on compilation.
| if (LOGGER.isWarnEnabled()) { | ||
| LOGGER.warn( | ||
| "TODO: Apex value not detected for dml's child vertex: " + childVertex); | ||
| // TODO: add telemetry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todos were taken care of earlier.
| // If AccessLevel is included in the syntax, it's usually the last child. It shows up in the | ||
| // form of a VariableExpression with a ReferenceExpression child. | ||
|
|
||
| // Example: | ||
| // <DmlUpdateStatement BeginColumn="9" BeginLine="6" DefiningType="MyClass" | ||
| // DefiningType_CaseSafe="myclass" EndLine="6" EndScopes="[BlockStatement]" | ||
| // FirstChild="false" LastChild="true" childIdx="2"> | ||
| // <VariableExpression BeginColumn="24" BeginLine="6" DefiningType="MyClass" | ||
| // DefiningType_CaseSafe="myclass" EndLine="6" FirstChild="true" LastChild="false" Name="a" | ||
| // Name_CaseSafe="a" childIdx="0"> | ||
| // <EmptyReferenceExpression BeginColumn="24" BeginLine="6" | ||
| // DefiningType="MyClass" DefiningType_CaseSafe="myclass" EndLine="6" FirstChild="true" | ||
| // LastChild="true" childIdx="0"/> | ||
| // </VariableExpression> | ||
| // <VariableExpression BeginColumn="30" BeginLine="-1" DefiningType="MyClass" | ||
| // DefiningType_CaseSafe="myclass" EndLine="-1" FirstChild="false" LastChild="true" | ||
| // Name="USER_MODE" Name_CaseSafe="user_mode" childIdx="1"> | ||
| // <ReferenceExpression BeginColumn="19" BeginLine="6" | ||
| // DefiningType="MyClass" DefiningType_CaseSafe="myclass" EndLine="6" FirstChild="true" | ||
| // LastChild="true" Name="AccessLevel" Name_CaseSafe="accesslevel" Names="[AccessLevel]" | ||
| // ReferenceType="LOAD" childIdx="0"/> | ||
| // </VariableExpression> | ||
| // </DmlUpdateStatement> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to wrap this block in // spotless:off and // spotless:on to make sure it doesn't get mangled.
| // Default to System mode to begin with. | ||
| AccessLevel accessLevel = AccessLevel.SYSTEM_MODE; | ||
|
|
||
| // If AccessLevel is included in the syntax, it's usually the last child. It shows up in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Usually the last child" are there known cases where it won't be? Will those cases break here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When "as user" or "as system" is not included, the variable that contains the value for DML is the last child. If "as user" or "as system" is included, it'll be the last child. The conditionals below confirm other things in the code that follows.