groovy-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thierry Hanser <Thierry.Han...@lhasalimited.org>
Subject RE: Failing to change the value of a Java variable from wihtin a Groovy script (potential issue?) [POTENTIAL FIX]
Date Tue, 05 Apr 2016 18:00:34 GMT
Hi everyone,

After further investigating the unexpected behaviour due to the getProperty()/setProperty()
behaviour dissymmetry, I understood the underlying ground which is indeed compatible with
what Jochen described.

 In the Script class, getProperty() implicitly checks if the variable exists in the binding
using the exception try/catch. If it exists it gets the value from the binding and if it does
not exists if delegates to the super class (GroovyObjectSupport) which in turn delegates to
the meta class which eventually calls getX(). 

On the other hand setProperty() simply sets the variable in the binding regardless of its
existence prior to the assignment. At this stage the meta class gets no chance to act when
the variable does not exists which introduces a non symmetrical behaviour between getProperty()
and setProperty().

In the Script class:

    public Object getProperty(String property) {
        try {
            return binding.getVariable(property);   // <== property exists -> value
from binding
        } catch (MissingPropertyException e) {
            return super.getProperty(property);     // <== property does not exist ->
value from the super class which delegates to the meta class

        }
    }

    public void setProperty(String property, Object newValue) {
        if ("binding".equals(property))
            setBinding((Binding) newValue);
        else if("metaClass".equals(property))
            setMetaClass((MetaClass)newValue);
        else
            binding.setVariable(property, newValue); // <== change/set value in binding
regardless of the existence of the variable in the binding or not
    }

The solution is indeed close to what Jochen suggested. I just think that for the sake of symmetry
all we need to do is to check whether the variable exists in the binding and behave accordingly.
If it exists change its value in the binding, if it does not exist delegate to the  meta class
(or call super.super). This would ensure a consistent (symmetrical) behaviour between getProperty()
and setProperty(). 

@Override
public void setProperty(String property, Object newValue) 
{	
   if ("binding".equals(property)) setBinding((Binding) newValue);
   else if("metaClass".equals(property)) setMetaClass((MetaClass)newValue);
   else if(getBinding().hasVariable(property))  getBinding().setVariable(property, newValue);
 // <== if variable exist => set binding value
   else getMetaClass().setProperty(this, property, newValue); // <== variable does not
exist => delegate to meta class
}

Note that the origin of the dissymmetry is actually a consequence of the dissymmetry between
the getVariable() and the setVariable() methods in the Binding class. getVariable() throws
an exception when the variable does not exist where as setVariable() does not (it adds the
variable to the linked map). The former exception is used by getProperty() in Script to trigger
the delegation to the meta class but this can be used by setProperty() since no exception
is thrown by setVariable() in Binding. 

Finally to be I would suggest to not use the exception of the getVariable() method (in Binding)
to decide articulate the behaviour of the getProperty() method (in Script). To be fully symmetrical
and consistent I would suggest to also fix the getProperty() method in the following manner:

@Override
public Object getProperty(String property)
{
   if(getBinding().hasVariable(property))  return getBinding().getVariable(property);
   else return  getMetaClass().getProperty(this, property);
}

Another positive consequence is that the binding variables become consistent since script
level remain separate from the dynamically introduced binding.
If we wanted both variable/property spaces to be automatically merged and in sync at assignment
time then we would have to remove the existence test in the setProperty() code (but I am not
sure this is what we want; it is a higher level design decision)

@Override
public void setProperty(String property, Object newValue) 
{	
   if ("binding".equals(property)) setBinding((Binding) newValue);
   else if("metaClass".equals(property)) setMetaClass((MetaClass)newValue);
   else   
   {      
      getBinding().setVariable(property, newValue);  // <== update binding
      getMetaClass().setProperty(this, property, newValue); // <== notify meta class
   }
}

Unless I missed out some other subtle aspects, I think this should have been the original
implementation of the Script setProperty(). 
Could this be a candidate for a fix?   (BTW, this solution works and provides the same behaviour
with and without the CompileStatic option activated)

Thank you all for your help.

Thierry



-----Original Message-----
From: Thierry Hanser [mailto:Thierry.Hanser@lhasalimited.org] 
Sent: 05 April 2016 17:24
To: users@groovy.apache.org
Subject: RE: Failing to change the value of a Java variable from wihtin a Groovy script (potential
issue?)

Thank you for your explanations Jochen.

Your assumption is correct except that I think you meant to say that *The subsequent "println
'2 = ' + x" will then get the value property instead of asking for the binding.* since the
value ('one') is the one from the unchanged variable/property 'x'.

I am confused by the lack of symmetry in the handling of the binding/properties value.

"print x" in a script will call getX() (if available) whereas "x = 'two'" will call setProperty()
which will add 'x' (if not present) to the binding but will not subsequently call setX().
As a result when you use the value of a property 'x' in a script you are guaranteed to call
getX() but if you assign a value to 'x' you are not guaranteed that setX() is called !

This dissymmetry seems odd to me (and obviously leads to unexpected behaviour).

Note that without static compilation the same dissymmetry is observed, however for some reason
the property 'x' is then properly updated but still without calling setX()

Really confusing to me !!!

I understand there are issues with the CompileStatic option. Do we know what level of prioritization
are these issues in the bigger Groovy roadmap?

Thanks a lot,

Thierry

--

Groovy script:

"println '1 = ' + x; x = 'two'; println binding.variables;  println '2 = ' + x\n";

With static compilation:

init x: one
Getting x : one
1 = one
Setting property : x = two
[x:two]
Getting x : one
2 = one

Without static compilation:

init x: one
Getting x : one
1 = one
Setting property : x = two
[x:two]
2 = two


Java:

public abstract class JavaScriptDemo extends Script {
    public String x;

    public JavaScriptDemo()
    {
                x="one";
                System.out.println("init x: " + x);
    }

    public Object process(Closure code)
    {
        System.out.println("Processing with x : " + x);
        return code.call();
    }

    public void setX(String text)
    {
        System.out.println("Setting x : "  + x);
        x = text;
    }

    public String getX()
    {
        System.out.println("Getting x : "  + x);
        return x;
    }


        @Override
        public void setProperty(String property, Object value)
        {
        System.out.println("Setting property : "  + property + " = " + value);
        super.setProperty(property, value);
        }


        /**
     * Demo main
     * @param args
     * @throws Throwable
     */
    public static void main(String...args) throws Throwable
    {
                ////
                // Compilation configuration
                CompilerConfiguration configuration = new CompilerConfiguration();
                //configuration.addCompilationCustomizers(new ASTTransformationCustomizer(CompileStatic.class));
                configuration.setScriptBaseClass(JavaScriptDemo.class.getName());
                GroovyShell shell = new GroovyShell(JavaScriptDemo.class.getClassLoader(),
configuration);

                // source code
                String scriptSource= "println '1 = ' + x; x = 'two'; println binding.variables;
 println '2 = ' + x\n";

                // compile the source code and run the compiled script
                JavaScriptDemo compiledScript = (JavaScriptDemo)shell.parse(scriptSource);
        compiledScript.run();
    }
}

-----Original Message-----
From: Jochen Theodorou [mailto:blackdrag@gmx.org]
Sent: 09 March 2016 13:37
To: users@groovy.apache.org
Subject: Re: Failing to change the value of a Java variable from wihtin a Groovy script (potential
issue?)



On 08.03.2016 17:31, Thierry Hanser wrote:
[...]
> *In Groovy*
>
> println '1 = ' + x
> x = 'two'
> println '2 = ' + x
>
> *Output*:
>
> init x: one <- initial value assignement OK
>
> 1 = one <- successfully accessing 'x' from within the compiled script 
> OK
>
>                  the Groovy script has picked up the value of the Java 
> variable;
>
>                  the implicit getX() has been called
>
> 2 = one<- should be 'two' as per Groovy code (second line)
>
>                  but is unchanged ???

I think I know the problem...

When using a normal script your code above would have failed, because the binding does not
contain x. But what exactly happens if the binding does not contain x? Groovy will ask the
script class for a property of that name. Just to point it out very much: The property lookup
is done only after binding is asked!

What happens in the set case then? The set case will always set a variable in the binding!

This explaines why for you "println '1 = ' + x " gives "one", but the important part is that
then "x = 'two'" will leave that property untouched and goes directly to the binding. The
subsequent "println '2 = ' + x" will then get the value from the binding instead of asking
for the property.

The solution would be to overwrite setProperty to first try setting the property on the script
class, basically: a try with this.getMetaClass().setProperty(this, property, newValue); and
a super.setProperty in the exception case.

And it seems the static compiler has big issues with handling a script base class

bye Jochen
________________________________
Switchboard: +44 (0)113 394 6020
Technical Support: +44 (0)113 394 6030
________________________________
Lhasa Limited, a not-for-profit organisation, promotes scientific knowledge & understanding
through the development of computer-aided reasoning & information systems in chemistry
& the life sciences. Registered Charity Number 290866. Registered Office: Granary Wharf
House, 2 Canal Wharf, Leeds, LS11 5PS. Company Registration Number 01765239. Registered in
England and Wales.
This communication, including any associated attachments, is intended for the use of the addressee
only and may contain confidential, privileged or copyright material. If you are not the intended
recipient, you must not copy this message or attachment or disclose the contents to any other
person. If you have received this transmission in error, please notify the sender immediately
and delete the message and any attachment from your system. Except where specifically stated,
any views expressed in this message are those of the individual sender and do not represent
the views of Lhasa Limited. Lhasa Limited cannot accept liability for any statements made
which are the sender's own. Lhasa Limited does not guarantee that electronic communications,
including any attachments, are free of viruses. Virus scanning is recommended and is the responsibility
of the recipient.

Mime
View raw message