Re: [Webmacro-devel] Bug in repeating invocations of #eval

[Alex F. <al...@fi...>]

All,

moving aside from the discussion about hosting and version control of
webmacro, and shifting across to the fixing of the bug (which I will fix in
my github tree, and then backport across to the sourceforge tree if people
want the fix), there are basically two ways to fix it:


   1. Change EvalDirective so that when it invokes setMap on the Context
   that it has created to invoke the templet on, that it copies the Map before
   invoking setMap and then utilises the copy.  This copy can then be mutated
   by the templet without effecting the original cached parsed Map.
   2. Change EvalDirective so that rather than invoking context.setMap, it
   instead invokes context.getMap().putAll(argMap) thereby copying the
   contents of the cached Map into the Map inside the Context that can then be
   invoked.

My personal preference is the second, and I would remove context.setMap
which is only used for EvalDirective.  My reasons for wanting to remove the
method are:

   1. it feels like an optimisation to avoid copying the Context in
   EvalDirective and as a side effect created this bug
   2. it doesn't smell very nice to me.  I would prefer Context to be
   responsible for data that can be contained within itself and letting a 3rd
   party drop in an unknown implementation of Map at an unknown point in the
   lifecycle of the Context just feels a bit wrong
   3. specifically in the context of the fix to EvalDirective - it just
   feels wrong that we do the following:
      1. Create a Context, that creates an empty Map
      2. Create an external Map
      3. Copy our cached Map into the external Map
      4. replace the Context Map with the external Map

as opposed to:


   1. Create a Context, that creates an empty Map
   2. copy our cached Map into the Context's Map

Let me know if anyone has any strong feelings about this and I will add in
a fix to my tree...

Alex


On 24 November 2011 17:44, Alex Fiennes <al...@fi...> wrote:

> Dear all,
>
> I have absolutely no idea whether or not there is anybody on this list, or
> if they are whether or not they are interested in things webmacro, so I
> might be talking to myself, but here goes...
>
> First off, I've found a bug.  Lets consider the following block of code:
>
> #templet $test {
>   Original: $value <br />
>   #set $value = $value + 1
>   Incremented: $value <br />
> }
> #eval $test using { "value": 1 }
>
> You run this in a servlet and it does what you would expect, ie:
>
> Original: 1
> Incremented: 2
>
>
> However if you reload the page shortly afterwards then you get the
> slightly surprising:
>
> Original: 2
> Incremented: 3
>
>
> which is much less what you would expect.
>
> I've been doing some initial poking around and from what I can see the
> problem is roughly along the lines of:
>
>    - the parser compiles the {"value": 1} into a Map and then caches this
>    - the EvalDirective passes this very same Map into the Context that is
>    used to evaluate the $test templet
>    - the $test templet makes changes to it's Context which then changes
>    the cached compiled version of {"value": 1}
>    - the next time we invoke $test, we pass in what we assume is the
>    compiled version of {"value": 1}, but it actually now contains {"value": 2}
>
> If one leaves a pause between reloads and the caching engine discards the
> cached parsed templates then the value returns back to 1.
>
> Now obviously, the EvalDirective should be taking a shallow copy of the
> params before setting this as the Map that is backing the Context that is
> used to evaluate $test.
> I've mocked this up in user space so that I can invoke:
>
> #eval $test using $Vars.copyMap({ "value": 1 })
>
> and I get the expected behaviour, ie the value doesn't increment between
> reloads.
>
> So I'm going to patch EvalDirective to sort this out which brings me onto
> my second point.
> I've been doing a bit of playing around with webmacro myself and I've got
> a clone of the git repository that Guy Bolton-King took of the sourceforge
> CVS repository and this is sitting at
> https://github.com/alex-fiennes/webmacro
> This has been a gentle experiment so far because I haven't found anything
> that is actually broken yet, but come this bug then I suspect that I might
> well start using my branch for production stuff rather than just playing
> around with ideas.
> Who else other than me is still using webmacro and should I just continue
> to work on my github branch and move forwards?  Failing that, do people
> want me to backport changes across to sourceforge?
>
> Stuff that I have been initially working on in github is:
>
>    - removing the dependencies on the EDU.oswego.cs.dl.util.concurrent
>    packages and using the java concurrent packages
>    - adding in generics declarations as required so that the package will
>    compile cleanly against modern java.
>
> What I am planning to work on is:
>
>    - adding in support for invoking varargs methods from inside templates
>    - a certain amount of refactoring to use more of google guava
>    libraries as appropriate
>    - better error reporting on nested templet invocations
>
> If people want to let me know whether or not any of this is of interest to
> anyone then we can work out how to move forwards.  If I don't hear back
> from anyone then I will assume that everyone has moved on to new pastures
> and I will just continue down the path outlined above.
>
> Alex
>
> --
> Alex Fiennes
> email: al...@fi...
> mobile:  +44 (0)7813 832662
> office: +44 (0)1875 823 310
>
>
>


-- 
Alex Fiennes
email: al...@fi...
mobile:  +44 (0)7813 832662
office: +44 (0)1875 823 310