Allow global variables in modules to use context

Author: larsga

src/main/java/com/schibsted/spt/data/jslt/Parser.java

@@ -187,6 +187,7 @@ public Parser withNamedModules(Map<String, Module> thisModules) {
    */
   public Expression compile() {
     ParseContext ctx = new ParseContext(functions, source, resolver, modules,
+                                        new ArrayList(),
                                         new PreparationContext());
     return ParserImpl.compileExpression(ctx, new JsltParser(reader));
   }

src/main/java/com/schibsted/spt/data/jslt/impl/ExpressionImpl.java

@@ -35,7 +35,7 @@ public class ExpressionImpl implements Expression {
   private Map<String, Function> functions;
   private ExpressionNode actual;
   private int stackFrameSize;
-  private JsonNode[] globalStackFrame;
+  private JstlFile[] fileModules;
 
   // contains the mapping from external parameters (variables set from
   // outside at query-time) to slots, so that we can put the
@@ -79,12 +79,15 @@ public JsonNode apply(Scope scope, JsonNode input) {
     if (input == null)
       input = NullNode.instance;
 
-    // if imported modules have global variables we need to set those
-    // before we start
-    if (globalStackFrame != null)
-      scope.insertModuleGlobals(globalStackFrame);
+    // evaluate lets in global modules
+    if (fileModules != null) {
+      for (int ix = 0; ix < fileModules.length; ix++)
+        fileModules[ix].evaluateLetsOnly(scope, input);
+    }
 
+    // evaluate own lets
     NodeUtils.evalLets(scope, input, lets);
+
     return actual.apply(scope, input);
   }
 
@@ -112,10 +115,8 @@ public void prepare(PreparationContext ctx) {
    * ExpressionImpl is a module. Called once during compilation.
    * The values are then remembered forever.
    */
-  public void evaluateLetsOnly(Scope scope) {
-    // the context node is null: all references to it in modules are
-    // verboten, anyway
-    NodeUtils.evalLets(scope, null, lets);
+  public void evaluateLetsOnly(Scope scope, JsonNode input) {
+    NodeUtils.evalLets(scope, input, lets);
   }
 
   public void optimize() {
@@ -149,7 +150,8 @@ public int getStackFrameSize() {
     return stackFrameSize;
   }
 
-  public void setInitialScope(Scope startScope) {
-    this.globalStackFrame = startScope.getGlobalStackFrame();
+  public void setGlobalModules(List<JstlFile> fileModules) {
+    this.fileModules = new JstlFile[fileModules.size()];
+    this.fileModules = fileModules.toArray(this.fileModules);
   }
 }

src/main/java/com/schibsted/spt/data/jslt/impl/JstlFile.java

@@ -64,7 +64,7 @@ public JsonNode call(JsonNode input, JsonNode[] arguments) {
     return body.apply(arguments[0]);
   }
 
-  public void evaluateLetsOnly(Scope scope) {
-    body.evaluateLetsOnly(scope);
+  public void evaluateLetsOnly(Scope scope, JsonNode input) {
+    body.evaluateLetsOnly(scope, input);
   }
 }

src/main/java/com/schibsted/spt/data/jslt/impl/ParseContext.java

@@ -16,6 +16,7 @@
 package com.schibsted.spt.data.jslt.impl;
 
 import java.util.Map;
+import java.util.List;
 import java.util.HashMap;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -39,10 +40,18 @@ public class ParseContext {
    */
   private String source;
   /**
-   * Imported modules listed under their prefixes.
+   * Imported modules listed under their prefixes. This is scoped per
+   * source file, since each has a different name-module mapping.
    */
   private Map<String, Module> modules;
-  private Collection<FunctionExpression> funcalls; // delayed function resolution
+  /**
+   * Tracks all loaded JSLT files. Shared between all contexts.
+   */
+  private List<JstlFile> files;
+  /**
+   * Function expressions, used for delayed name-to-function resolution.
+   */
+  private Collection<FunctionExpression> funcalls;
   private ParseContext parent;
   private ResourceResolver resolver;
   /**
@@ -57,13 +66,15 @@ public class ParseContext {
   public ParseContext(Collection<Function> extensions, String source,
                       ResourceResolver resolver,
                       Map<String, Module> namedModules,
+                      List<JstlFile> files,
                       PreparationContext preparationContext) {
     this.extensions = extensions;
     this.functions = new HashMap();
     for (Function func : extensions)
       functions.put(func.getName(), func);
 
     this.source = source;
+    this.files = files;
     this.funcalls = new ArrayList();
     this.modules = new HashMap();
     this.resolver = resolver;
@@ -75,7 +86,7 @@ public ParseContext(Collection<Function> extensions, String source,
 
   public ParseContext(String source) {
     this(Collections.EMPTY_SET, source, new ClasspathResourceResolver(),
-         new HashMap(), new PreparationContext());
+         new HashMap(), new ArrayList(), new PreparationContext());
   }
 
   public void setParent(ParseContext parent) {
@@ -165,11 +176,11 @@ public ResourceResolver getResolver() {
     return resolver;
   }
 
-  public Scope evaluateGlobalModuleVariables(int stackFrameSize) {
-    Scope scope = Scope.getRoot(stackFrameSize);
-    for (Module m : modules.values())
-      if (m instanceof JstlFile)
-        ((JstlFile) m).evaluateLetsOnly(scope);
-    return scope;
+  public List<JstlFile> getFiles() {
+    return files;
+  }
+
+  public void registerJsltFile(JstlFile file) {
+    files.add(file);
   }
 }

src/main/java/com/schibsted/spt/data/jslt/impl/Scope.java

@@ -66,21 +66,4 @@ public void setValue(int slot, JsonNode value) {
     else
       localStackFrames.peek()[slot] = value;
   }
-
-  public void insertModuleGlobals(JsonNode[] globals) {
-    for (int ix = 0; ix < globals.length; ix++)
-      if (globals[ix] != null)
-        globalStackFrame[ix] = globals[ix];
-  }
-
-  public JsonNode[] getGlobalStackFrame() {
-    return globalStackFrame;
-  }
-
-  public boolean hasGlobalValuesSet() {
-    for (int ix = 0; ix < globalStackFrame.length; ix++)
-      if (globalStackFrame[ix] != null)
-        return true;
-    return false;
-  }
 }

src/main/java/com/schibsted/spt/data/jslt/parser/ParserImpl.java

@@ -52,14 +52,9 @@ public static Expression compileExpression(ParseContext ctx, JsltParser parser)
     try {
       parser.Start();
       ExpressionImpl expr = compile(ctx, (SimpleNode) parser.jjtree.rootNode());
-
-      // we need to evaluate the global variables in all the modules,
-      // if there are any, once and for all, and remember their values
-      Scope scope = ctx.evaluateGlobalModuleVariables(expr.getStackFrameSize());
-      if (scope.hasGlobalValuesSet())
-        expr.setInitialScope(scope);
-
+      expr.setGlobalModules(ctx.getFiles());
       return expr;
+
     } catch (ParseException e) {
       throw new JsltException("Parse error: " + e.getMessage(),
                               makeLocation(ctx, e.currentToken));
@@ -72,7 +67,7 @@ private static ExpressionImpl compileImport(Collection<Function> functions,
                                              ParseContext parent,
                                              String jslt) {
     try (Reader reader = parent.getResolver().resolve(jslt)) {
-      ParseContext ctx = new ParseContext(functions, jslt, parent.getResolver(), parent.getNamedModules(), parent.getPreparationContext());
+      ParseContext ctx = new ParseContext(functions, jslt, parent.getResolver(), parent.getNamedModules(), parent.getFiles(), parent.getPreparationContext());
       ctx.setParent(parent);
       return compileModule(ctx, new JsltParser(reader));
     } catch (IOException e) {
@@ -552,6 +547,7 @@ private static void processImports(ParseContext ctx, SimpleNode parent) {
         JstlFile file = doImport(ctx, source, node, prefix);
         ctx.registerModule(prefix, file);
         ctx.addDeclaredFunction(prefix, file);
+        ctx.registerJsltFile(file);
       }
     }
   }

src/test/resources/function-declaration-tests.yaml

@@ -124,6 +124,30 @@ tests:
      m:variable($global)
    output: "\"variable is global\""
 
+  # in this one we don't define the global variable in the query
+  -
+   input: {}
+   query: >
+     import "module-with-global-var.jslt" as m
+     m:variable("variable is")
+   output: "\"variable is global\""
+
+  -
+   input: {}
+   query: >
+     import "module-as-function-with-global.jslt" as m
+     m("variable is")
+   output: "\"variable is global\""
+
+  -
+   input: {}
+   query: >
+     import "module-with-global-var.jslt" as m
+     import "module-also-with-global-var.jslt" as m2
+     let global = "variable is"
+     m:variable($global) + " " + m2:andfunc($global) + " " + $global
+   output: "\"variable is global and variable is variable is\""
+
   -
    input: {}
    query: >
@@ -145,6 +169,20 @@ tests:
      m:foo(2)
    output: 6
 
+  -
+   input: "{\"foo\" : 22}"
+   query: >
+     import "module-with-global-var-dot.jslt" as m
+     m:foo()
+   output: 22
+
+  -
+   input: "{\"foo\" : 22}"
+   query: >
+     import "module-imports-other-module.jslt" as m
+     m:foo()
+   output: 22
+
   -
    input: {}
    query: >

src/test/resources/module-also-with-global-var.jslt

@@ -0,0 +1,6 @@
+
+// Test that we can use global variables in modules
+def andfunc(text)
+  $global + $text
+
+let global = "and "

src/test/resources/module-as-function-with-global.jslt

@@ -0,0 +1,4 @@
+
+let global = "global"
+
+. + " " + $global

src/test/resources/module-imports-other-module.jslt

@@ -0,0 +1,5 @@
+
+import "module-with-global-var-dot.jslt" as m
+
+def foo()
+  m:foo()

src/test/resources/module-with-global-var-dot.jslt

@@ -0,0 +1,5 @@
+
+let foo = .foo
+
+def foo()
+  $foo

src/test/resources/module-with-global-var.jslt

@@ -1,6 +1,6 @@
 
+let global = "global"
+
 // Test that we can use global variables in modules
 def variable(text)
   $text + " " + $global
-
-let global = "global"