You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Dmitri Blinov (JIRA)" <ji...@apache.org> on 2018/04/06 14:07:00 UTC
[jira] [Comment Edited] (JEXL-256) Jexl should not try to resolve a
variable from Context when Context.has() returns false
[ https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16428367#comment-16428367 ]
Dmitri Blinov edited comment on JEXL-256 at 4/6/18 2:06 PM:
------------------------------------------------------------
I understand your logic, but I thought that if JexlContext has defined two methods for variables, like {{get()}} and {{has()}}, that would mean if we check for a variable, we should first call {{has()}} and only if it returns {{true}} we should call {{get()}}. Your explanation has your reason, that we optimistically try to {{get()}} first and if {{null}} has returned, we use {{has()}} to check if there is the variable missing or its value is {{null}}. I have implemented the context {{get()}} method to log an error message if there is an attempt to address the variable that does not exist within context. And as wrote previously, I noticed that there are log messages about missing context variables which names are parts of a longer antish style variable names. The idea is to check for {{has()}} first, so that developers writing their own context implementation could rely on {{has()}} as a guarding method. I think this is similar to {{ConcurrentHashMap}} implementation where {{get()}} method can throw NPE if there is no key, so we should rely on {{containsKey()}} before we actually try to get anything from the map
was (Author: dmitri_blinov):
I understand your logic, but I thought that if JexlContext has defined two methods for variables, like {{get()}} and {{has()}}, that would mean if we check for a variable, we should first call {{has()}} and only if it returns {{true}} we should call {{get()}}. Your explanation has your reason, that we optimistically try to {{get()}} first and if {{null}} has returned, we use {{has()}} to check if there is the variable missing or its value is {{nul}}l. I have implemented the context {{get()}} method to log an error message if there is an attempt to address the variable that does not exist within context. And as wrote previously, I noticed that there are log messages about missing context variables which names are parts of a longer antish style variable names. The idea is to check for {{has()}} first, so that developers writing their own context implementation could rely on {{has()}} as a guarding method. I think this is similar to {{ConcurrentHashMap}} implementation where {{get()}} method can throw NPE if there is no key, so we should rely on {{containsKey()}} before we actually try to get anything from the map
> Jexl should not try to resolve a variable from Context when Context.has() returns false
> ---------------------------------------------------------------------------------------
>
> Key: JEXL-256
> URL: https://issues.apache.org/jira/browse/JEXL-256
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 3.1
> Reporter: Dmitri Blinov
> Priority: Major
>
> I have bumped into the problem when my {{Context.get()}} sometimes reports access to variables that are reported by the {{Context.has()}} method as not existent, and are not supposed to be in the context, mainly parts of ant-ish variable paths. I assume that once {{Context.has()}} have reported {{false}} no attempt from the Jexl to call {{Context.get()}} with the same parameter should be made.
> I think the problem lies in {{Interpreter.java}} which first calls {{Context.get()}} and only if it returns null, which should not necceserily mean the variable does not exist, checks {{Context.has()}}.
> {code}
> @Override
> protected Object visit(ASTIdentifier node, Object data) {
> cancelCheck(node);
> String name = node.getName();
> if (data == null) {
> int symbol = node.getSymbol();
> if (symbol >= 0) {
> return frame.get(symbol);
> }
> Object value = context.get(name);
> if (value == null
> && !(node.jjtGetParent() instanceof ASTReference)
> && !context.has(name)
> && !node.isTernaryProtected()) {
> return unsolvableVariable(node, name, true);
> }
> return value;
> } else {
> return getAttribute(data, name, node);
> }
> }
> {code}
> So I suggest to change the code to something like this
> {code}
> @Override
> protected Object visit(ASTIdentifier node, Object data) {
> cancelCheck(node);
> String name = node.getName();
> if (data == null) {
> int symbol = node.getSymbol();
> if (symbol >= 0) {
> return frame.get(symbol);
> }
> if (!context.has(name)
> && !(node.jjtGetParent() instanceof ASTReference)
> && !node.isTernaryProtected()) {
> return unsolvableVariable(node, name, true);
> }
> return context.get(name);
> } else {
> return getAttribute(data, name, node);
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)