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

116 dml statements dont validate the expressions are sobjects #233

Conversation

kjonescertinia
Copy link
Contributor

@kjonescertinia kjonescertinia commented Sep 23, 2023

This improves validation of the DML statements insert, update, upsert, delete, undelete & merge. Most require a SObject or SObject list but merge takes two expression, the first has to be a specific SObject while the second should be a matching SObject or SObject List.

The validation are written so that if we can't get a type for the expression then we don't error. The expression(s) should have logged errors if needed.

@kjonescertinia kjonescertinia changed the base branch from main to 115-runas-statement-is-missing-validation September 23, 2023 21:33
Base automatically changed from 115-runas-statement-is-missing-validation to main September 25, 2023 16:07
(false, verifyResult)
} else {
(true, verifyResult)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's probably a future refactor in here to split off the isDefined / static type checking as it's in every method. Just nitpicks, looks fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, think we are trapped a bit here between two different design options, returning an Option[ExprContext] to allow for failure to verify and returning an Any to mask that failure. Not sure I want to try sort that just yet but we might be able to simplify some verify logic if we did.

.contains(sObjectTypeName.name)
) Some(sObjectTypeName)
else None
}
Copy link
Contributor

@pwrightcertinia pwrightcertinia Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also maybe a getSObjectTypeName() that returns an Option if it's not could be used for the isSObjectOrSObjectList method and this, there's a bit of common behaviour between these two. More nitpicky

@kjonescertinia kjonescertinia merged commit 19edf62 into main Oct 4, 2023
@kjonescertinia kjonescertinia deleted the 116-dml-statements-dont-validate-the-expressions-are-sobjects branch October 4, 2023 09:10
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

Successfully merging this pull request may close these issues.

2 participants